mirror of
https://github.com/stablyai/orca.git
synced 2026-10-04 00:02:21 +00:00
refactor(wsl): delete the environment-policy layer the reviews kept failing on
A design council (Opus, Grok, GPT-5.6-Sol) reviewed the merged runner after it took eleven review rounds to land. All three reached the same conclusion: the invocation half is sound, the environment/probe half is not, and every round had been debugging the second one. The finding that settled it, from Opus: `environmentResolved` had **54 references, all in tests and the runner itself. Not one production reader.** The safety mechanism the strict default existed for was never wired to anything, so all 19 degrading sites reported absence with full confidence anyway -- #9725 live at every one, under comments claiming it was handled. Two of those comments say so out loud; I wrote them. Root cause, in one line: every knob existed only because a failed probe was fatal. So it no longer is. - `allowDegradedEnvironment` and `WslGuestEnvironmentUnavailableError` are gone. A missing login PATH is a fact in the result, not an exception. That deletes 23 opt-outs, six catch-and-remap blocks, the transient/rejected cooldown split, `probedWithBudget`, and the 1.5x re-probe heuristic -- none of which had a reason to exist once the case stopped throwing. - `lane` + `allowDegradedEnvironment` collapse into `loginPath: 'none' | 'preferred'`. 19 of 23 sites passed the opt-out, and two said in comments that they did not want the login PATH at all: the flag had become the `'none'` the union was missing. - The `interactive` lane is deleted. It had zero production callers and kept ~30 lines of fence plumbing alive for tests only. Net -98 production lines; the runner itself sheds 86 for 38. Also carries three fixes from the W3 orphan-PR sweep I had not done: - `WSL_UTF8=1` in the runner. My relay migration deleted the only place setting it, so wsl.exe's own error text arrived UTF-16LE and read as NUL-riddled. A regression I introduced. Credit: #9010 (Chang-Jin-Lee). - `GITLAB_HOST` is now named in WSLENV, so a ported self-hosted host actually crosses into a distro-routed glab (#12557). Credit: #12558 (makoto-developer). - The WSL skill-setup command pipes into `sh` instead of `eval "$(...)"`, whose nested quoting produced `word unexpected (expecting "in")` (#14292). Credit: #14785 (innocarpe).
This commit is contained in:
@@ -181,14 +181,10 @@ export async function runWslInstallProcess(
|
||||
): Promise<{ code: number | null; stderr: string }> {
|
||||
const result = await runWslProcess({
|
||||
distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'none',
|
||||
script,
|
||||
timeoutMs: INSTALL_TIMEOUT_MS,
|
||||
maxOutputBytes: MAX_STARTUP_BUFFER_BYTES,
|
||||
// The install script only shells out to base64/mkdir/mv/chmod, all on any
|
||||
// default PATH -- it must still run when the login-PATH probe itself is
|
||||
// what's wedged, which is exactly the state this call recovers from.
|
||||
allowDegradedEnvironment: true
|
||||
})
|
||||
return result.timedOut
|
||||
? { code: null, stderr: `${result.stderr}\ninstall timed out after ${INSTALL_TIMEOUT_MS}ms` }
|
||||
|
||||
@@ -1088,7 +1088,7 @@ export class ClaudeRuntimeAuthService {
|
||||
try {
|
||||
const owned = await runWslProcess({
|
||||
distro: wslInfo.distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'none',
|
||||
shell: 'bash',
|
||||
script: [
|
||||
'set -euo pipefail',
|
||||
@@ -1101,11 +1101,6 @@ export class ClaudeRuntimeAuthService {
|
||||
'case "$candidate_real" in "$managed_root_real"/*/auth) printf "%s\\n" "$candidate_real" ;; *) exit 35 ;; esac'
|
||||
].join('\n'),
|
||||
timeoutMs: 5000,
|
||||
// Why degraded is allowed: a failed answer here disowns the account and
|
||||
// clears the user's selection, so a slow distro probe must not decide
|
||||
// ownership. The script reads only $HOME and coreutils, both of which
|
||||
// wsl.exe supplies without the login PATH.
|
||||
allowDegradedEnvironment: true
|
||||
})
|
||||
if (owned.timedOut) {
|
||||
throw new Error(OWNERSHIP_PROBE_TIMEOUT)
|
||||
|
||||
@@ -664,13 +664,10 @@ export class ClaudeAccountService {
|
||||
}
|
||||
const created = await runWslProcess({
|
||||
distro: location.wslDistro,
|
||||
lane: 'probe',
|
||||
loginPath: 'none',
|
||||
shell: 'bash',
|
||||
script: 'mktemp -d "${TMPDIR:-/tmp}/orca-claude-login.XXXXXX"',
|
||||
timeoutMs: 5000,
|
||||
// Why degraded is allowed: mktemp is a coreutil on the default PATH, so an
|
||||
// unprobed distro must not turn a working sign-in into a failed one.
|
||||
allowDegradedEnvironment: true
|
||||
})
|
||||
const linuxPath = created.stdout.replaceAll(String.fromCharCode(0), '').trim()
|
||||
if (created.code !== 0 || created.timedOut || !linuxPath.startsWith('/')) {
|
||||
@@ -692,13 +689,10 @@ export class ClaudeAccountService {
|
||||
try {
|
||||
await runWslProcess({
|
||||
distro: tempConfig.wslDistro,
|
||||
lane: 'probe',
|
||||
loginPath: 'none',
|
||||
program: 'rm',
|
||||
args: ['-rf', '--', tempConfig.linuxPath],
|
||||
timeoutMs: 5000,
|
||||
// Why degraded is allowed: leaving a login temp dir behind is worse than
|
||||
// running rm on the default PATH, and this is already best-effort.
|
||||
allowDegradedEnvironment: true
|
||||
})
|
||||
} catch {
|
||||
// Best-effort cleanup.
|
||||
@@ -910,13 +904,10 @@ export class ClaudeAccountService {
|
||||
const requestedDistro = target.wslDistro?.trim() || undefined
|
||||
const info = await runWslProcess({
|
||||
distro: requestedDistro,
|
||||
lane: 'probe',
|
||||
loginPath: 'none',
|
||||
shell: 'bash',
|
||||
script: 'printf "%s\\n%s\\n" "$WSL_DISTRO_NAME" "$HOME"',
|
||||
timeoutMs: 5000,
|
||||
// Why degraded is allowed: both variables come from wsl.exe itself, not from
|
||||
// the login PATH, so an unprobed distro still answers correctly here.
|
||||
allowDegradedEnvironment: true
|
||||
})
|
||||
const [rawDistro, rawHome] =
|
||||
info.code === 0 && !info.timedOut
|
||||
@@ -934,14 +925,11 @@ export class ClaudeAccountService {
|
||||
const wslLinuxAuthPath = `${home.replace(/\/$/, '')}/.local/share/orca/claude-accounts/${accountId}/auth`
|
||||
const created = await runWslProcess({
|
||||
distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'none',
|
||||
shell: 'bash',
|
||||
script: 'mkdir -p "$1" && printf \'%s\\n\' "$2" > "$1/.orca-managed-claude-auth"',
|
||||
args: [wslLinuxAuthPath, accountId],
|
||||
timeoutMs: 5000,
|
||||
// Why degraded is allowed: mkdir is a coreutil on the default PATH, and the
|
||||
// target path was already resolved from the guest's own $HOME above.
|
||||
allowDegradedEnvironment: true
|
||||
})
|
||||
if (created.code !== 0 || created.timedOut) {
|
||||
throw new Error('Could not create the managed WSL Claude auth directory.')
|
||||
@@ -978,7 +966,7 @@ export class ClaudeAccountService {
|
||||
try {
|
||||
const owned = await runWslProcess({
|
||||
distro: wslInfo.distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'none',
|
||||
shell: 'bash',
|
||||
script: [
|
||||
'set -euo pipefail',
|
||||
@@ -993,10 +981,6 @@ export class ClaudeAccountService {
|
||||
'case "$candidate_real" in "$managed_root_real"/*/auth) printf "%s\\n" "$candidate_real" ;; *) exit 35 ;; esac'
|
||||
].join('\n'),
|
||||
timeoutMs: 5000,
|
||||
// Why degraded is allowed: the ownership proof is the script's own
|
||||
// marker and containment checks, not the login PATH, and refusing here
|
||||
// would fail every managed read/write on a distro Orca could not probe.
|
||||
allowDegradedEnvironment: true
|
||||
})
|
||||
if (owned.code !== 0 || owned.timedOut) {
|
||||
throw new Error('Managed Claude auth directory does not exist on disk.')
|
||||
|
||||
@@ -463,7 +463,7 @@ async function runWslCommand(distro: string, command: string): Promise<string> {
|
||||
try {
|
||||
result = await runWslProcess({
|
||||
distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
script: command,
|
||||
timeoutMs: WSL_COMMAND_TIMEOUT_MS
|
||||
})
|
||||
|
||||
@@ -71,7 +71,9 @@ describe('CodexAccountService config sync', () => {
|
||||
const runWslProcessMock = vi.fn(async (spec: WslSpec) => {
|
||||
const script = String(spec.script)
|
||||
expect(spec.distro).toBe('Debian')
|
||||
expect(spec.lane).toBe('probe')
|
||||
// 'none': the script reads $HOME and $WSL_DISTRO_NAME, which wsl.exe
|
||||
// supplies from /etc/passwd without a login shell.
|
||||
expect(spec.loginPath).toBe('none')
|
||||
if (script.includes('WSL_DISTRO_NAME')) {
|
||||
return wslOk('Debian\n/home/alice\n')
|
||||
}
|
||||
|
||||
@@ -1215,9 +1215,7 @@ export class CodexAccountService {
|
||||
const requestedDistro = target.wslDistro?.trim() || undefined
|
||||
const info = await runWslProcess({
|
||||
distro: requestedDistro,
|
||||
lane: 'probe',
|
||||
// Degrade rather than refuse: $HOME and $WSL_DISTRO_NAME come from wsl.exe itself, not the login PATH -- the same rationale the Claude sites use.
|
||||
allowDegradedEnvironment: true,
|
||||
loginPath: 'none',
|
||||
script: 'printf "%s\\n%s\\n" "$WSL_DISTRO_NAME" "$HOME"',
|
||||
shell: 'bash',
|
||||
timeoutMs: WSL_MANAGED_HOME_TIMEOUT_MS
|
||||
@@ -1239,9 +1237,7 @@ export class CodexAccountService {
|
||||
const markerPath = `${wslLinuxHomePath}/.orca-managed-home`
|
||||
const created = await runWslProcess({
|
||||
distro,
|
||||
lane: 'probe',
|
||||
// Degrade rather than refuse: $HOME and $WSL_DISTRO_NAME come from wsl.exe itself, not the login PATH -- the same rationale the Claude sites use.
|
||||
allowDegradedEnvironment: true,
|
||||
loginPath: 'none',
|
||||
script: `mkdir -p ${shellQuote(wslLinuxHomePath)} && printf '%s\\n' ${shellQuote(accountId)} > ${shellQuote(markerPath)}`,
|
||||
shell: 'bash',
|
||||
timeoutMs: WSL_MANAGED_HOME_TIMEOUT_MS
|
||||
@@ -1478,9 +1474,7 @@ export class CodexAccountService {
|
||||
|
||||
const result = await runWslProcess({
|
||||
distro: wslInfo.distro,
|
||||
lane: 'probe',
|
||||
// Degrade rather than refuse: $HOME and $WSL_DISTRO_NAME come from wsl.exe itself, not the login PATH -- the same rationale the Claude sites use.
|
||||
allowDegradedEnvironment: true,
|
||||
loginPath: 'none',
|
||||
script: [
|
||||
'set -euo pipefail',
|
||||
`candidate=${shellQuote(wslInfo.linuxPath)}`,
|
||||
@@ -1616,9 +1610,7 @@ export class CodexAccountService {
|
||||
try {
|
||||
const result = await runWslProcess({
|
||||
distro,
|
||||
lane: 'probe',
|
||||
// Degrade rather than refuse: $HOME and $WSL_DISTRO_NAME come from wsl.exe itself, not the login PATH -- the same rationale the Claude sites use.
|
||||
allowDegradedEnvironment: true,
|
||||
loginPath: 'none',
|
||||
script: [
|
||||
'set -euo pipefail',
|
||||
`candidate=${shellQuote(linuxHomePath)}`,
|
||||
@@ -1872,7 +1864,7 @@ export class CodexAccountService {
|
||||
try {
|
||||
result = await runWslProcess({
|
||||
distro: wslInfo.distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'none',
|
||||
script: buildWslCodexAvailabilityScript(),
|
||||
timeoutMs: WSL_CODEX_AVAILABILITY_TIMEOUT_MS
|
||||
})
|
||||
|
||||
@@ -44,13 +44,13 @@ describe('syncWslCodexSessionsIntoManagedHome', () => {
|
||||
expect(runWslProcessMock).toHaveBeenCalledTimes(1)
|
||||
const spec = runWslProcessMock.mock.calls[0]?.[0] as {
|
||||
distro: string
|
||||
lane: string
|
||||
loginPath: string
|
||||
shell?: string
|
||||
script: string
|
||||
timeoutMs: number
|
||||
}
|
||||
expect(spec.distro).toBe('Ubuntu')
|
||||
expect(spec.lane).toBe('probe')
|
||||
expect(spec.loginPath).toBe('none')
|
||||
expect(spec.shell).toBe('bash')
|
||||
expect(spec.timeoutMs).toBe(30_000)
|
||||
|
||||
|
||||
@@ -56,7 +56,7 @@ export async function syncWslCodexSessionsIntoManagedHome(
|
||||
|
||||
const result = await runWslProcess({
|
||||
distro: target.distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'none',
|
||||
script: buildWslCodexSessionBridgeShellCommand(paths),
|
||||
// Process substitution and `read -d` are bash-only; dash rejects both.
|
||||
shell: 'bash',
|
||||
|
||||
@@ -338,3 +338,19 @@ describe('git env forces untranslated diagnostics (issue #7808)', () => {
|
||||
expect(env.LANGUAGE).toBe('en')
|
||||
})
|
||||
})
|
||||
|
||||
describe('redirectPortedHostnameToEnv WSLENV forwarding', () => {
|
||||
it('names GITLAB_HOST in WSLENV so it can cross into a distro', async () => {
|
||||
// A WSL-routed glab only sees Windows variables listed in WSLENV; without
|
||||
// the entry the ported host is silently dropped and glab talks to
|
||||
// gitlab.com (#12557).
|
||||
const { redirectPortedHostnameToEnv } = await import('./runner')
|
||||
const { options } = redirectPortedHostnameToEnv(
|
||||
['api', '--hostname', 'gitlab.example.com:8443'],
|
||||
{ env: { PATH: '/usr/bin' } }
|
||||
)
|
||||
expect(options.env?.GITLAB_HOST).toBe('gitlab.example.com:8443')
|
||||
expect((options.env?.WSLENV ?? '').split(':')).toContain('GITLAB_HOST')
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -1908,9 +1908,14 @@ export function redirectPortedHostnameToEnv(
|
||||
if (!/^[^/\s]+:\d+$/.test(host)) {
|
||||
return { args, options }
|
||||
}
|
||||
// Why WSLENV: a glab routed into a distro only sees Windows-side variables
|
||||
// named in WSLENV, so without this the ported host silently never crosses and
|
||||
// glab talks to gitlab.com instead (#12557). Credit: #12558.
|
||||
const env: NodeJS.ProcessEnv = { ...(options.env ?? process.env), GITLAB_HOST: host }
|
||||
addWslEnvKeys(env, ['GITLAB_HOST'])
|
||||
return {
|
||||
args: [...args.slice(0, i), ...args.slice(i + 2)],
|
||||
options: { ...options, env: { ...(options.env ?? process.env), GITLAB_HOST: host } }
|
||||
options: { ...options, env }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -164,7 +164,7 @@ describe('runHook', () => {
|
||||
expect(runWslProcessMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
distro: 'Ubuntu',
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
script: 'echo hello',
|
||||
cwd: '/home/jin/feature',
|
||||
// #7652 regression: the unattended WSL hook branch must carry the
|
||||
@@ -220,7 +220,7 @@ describe('runHook', () => {
|
||||
expect(runWslProcessMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
distro: 'Ubuntu',
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
script: 'echo hello',
|
||||
cwd: '/mnt/c/Users/jinwo/git/orca-feature',
|
||||
env: expect.objectContaining({
|
||||
|
||||
+1
-6
@@ -153,17 +153,12 @@ export function runHook(
|
||||
|
||||
return runWslProcess({
|
||||
distro: wslInfo.distro ?? undefined,
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
script,
|
||||
// Why pinned: these are user-authored orca.yaml scripts and the native
|
||||
// path runs /bin/bash. Defaulting to sh would fail bash-only hooks on WSL
|
||||
// only -- a downgrade the user never asked for.
|
||||
shell: 'bash',
|
||||
// Why degrade rather than fail: a hook is an action, not an "is this
|
||||
// installed?" question. Before the runner it ran on the default PATH when
|
||||
// no login shell was available; refusing would turn worktree setup into a
|
||||
// hard failure whenever WSL is briefly slow.
|
||||
allowDegradedEnvironment: true,
|
||||
cwd: wslInfo.linuxPath,
|
||||
env: guestEnv,
|
||||
timeoutMs: HOOK_TIMEOUT
|
||||
|
||||
@@ -405,7 +405,7 @@ describe('preflight', () => {
|
||||
// pinned by its own tests. What this suite owns is that detection asks the
|
||||
// right distro on the lane that carries the user's PATH.
|
||||
expect(runWslProcessMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ distro: 'Ubuntu', lane: 'probe' })
|
||||
expect.objectContaining({ distro: 'Ubuntu', loginPath: 'preferred' })
|
||||
)
|
||||
})
|
||||
|
||||
@@ -433,7 +433,7 @@ describe('preflight', () => {
|
||||
expect(resolveCliCommandsMock).not.toHaveBeenCalled()
|
||||
// No distro named: the runner resolves the default.
|
||||
expect(runWslProcessMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ distro: undefined, lane: 'probe' })
|
||||
expect.objectContaining({ distro: undefined, loginPath: 'preferred' })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -296,14 +296,14 @@ describe('preflight', () => {
|
||||
expect(runWslProcessMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
distro: 'Ubuntu',
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
script: expect.stringMatching(/gh[\s\S]*--version/)
|
||||
})
|
||||
)
|
||||
expect(runWslProcessMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
distro: 'Ubuntu',
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
script: expect.stringMatching(/gh[\s\S]*auth status/)
|
||||
})
|
||||
)
|
||||
|
||||
@@ -6,7 +6,7 @@ vi.mock('../wsl/wsl-runner', () => ({ runWslProcess: runWslProcessMock }))
|
||||
import { detectWslCommandsOnPath } from './preflight-wsl-agent-detection'
|
||||
import { buildPosixCommandPathLookupScript } from '../../shared/posix-command-path-lookup'
|
||||
|
||||
type RunWslProcessSpec = { distro?: string; lane: string; script: string }
|
||||
type RunWslProcessSpec = { distro?: string; loginPath: string; script: string }
|
||||
|
||||
function lastSpec(): RunWslProcessSpec {
|
||||
const call = runWslProcessMock.mock.calls.at(-1)
|
||||
@@ -24,11 +24,11 @@ describe('detectWslCommandsOnPath', () => {
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('runs on the probe lane -- no shell, so no rc/motd banner can appear', async () => {
|
||||
it('asks for the login PATH without running a shell, so no banner can appear', async () => {
|
||||
await detectWslCommandsOnPath({ distro: 'Ubuntu' }, ['claude'])
|
||||
|
||||
const spec = lastSpec()
|
||||
expect(spec.lane).toBe('probe')
|
||||
expect(spec.loginPath).toBe('preferred')
|
||||
expect(spec.distro).toBe('Ubuntu')
|
||||
})
|
||||
|
||||
|
||||
@@ -38,15 +38,8 @@ export async function detectWslCommandsOnPath(
|
||||
// with no shell in the loop, so there is no rc/motd banner to land in stdout.
|
||||
const result = await runWslProcess({
|
||||
distro: wslTarget.distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
script,
|
||||
// Why degrade rather than refuse: the Set has no room for "unverifiable",
|
||||
// and refusing would turn a slow distro into "no agents" anyway -- via a
|
||||
// throw instead of an empty result. Degrading at least finds anything on
|
||||
// the default PATH. The residual gap (an nvm-only agent missed during the
|
||||
// probe's retry window, #9725) is the pre-migration behaviour, not new,
|
||||
// and Refresh now re-probes.
|
||||
allowDegradedEnvironment: true,
|
||||
timeoutMs: WSL_AGENT_DETECTION_TIMEOUT_MS
|
||||
})
|
||||
// runProcess resolves on a timeout and on a non-zero exit, so partial
|
||||
|
||||
@@ -17,15 +17,14 @@ beforeEach(() => {
|
||||
})
|
||||
|
||||
describe('runPreflightCommandInWsl', () => {
|
||||
it('degrades rather than letting a slow distro read as "not installed"', async () => {
|
||||
it('prefers the login PATH but never lets its absence decide the answer', async () => {
|
||||
// Every caller collapses a throw into a verdict: isCommandAvailable returns
|
||||
// false ("not installed"), isGhAuthenticated reads an empty payload as
|
||||
// "not authenticated". Refusing here turns a slow distro into a confident
|
||||
// wrong answer -- #9725 through the other door.
|
||||
// "not authenticated". A missing login PATH must therefore never be fatal.
|
||||
await runPreflightCommandInWsl({ distro: 'Ubuntu' }, 'gh --version', 5_000)
|
||||
|
||||
expect(runWslProcessMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ allowDegradedEnvironment: true, distro: 'Ubuntu' })
|
||||
expect.objectContaining({ loginPath: 'preferred', distro: 'Ubuntu' })
|
||||
)
|
||||
})
|
||||
|
||||
|
||||
@@ -12,13 +12,8 @@ export async function runPreflightCommandInWsl(
|
||||
// with no shell in the loop, so there is no rc/motd banner to strip.
|
||||
const result = await runWslProcess({
|
||||
distro: target.distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
script: command,
|
||||
// Why degrade: every caller collapses a throw into "not installed" or
|
||||
// "not authenticated" (isCommandAvailable, isGhAuthenticated). Refusing
|
||||
// here turns a slow distro into a confident wrong answer -- #9725 through
|
||||
// the other door, on the branch built to close it.
|
||||
allowDegradedEnvironment: true,
|
||||
timeoutMs
|
||||
})
|
||||
// runWslProcess resolves on a timeout and on a non-zero exit; the caller's
|
||||
|
||||
@@ -61,9 +61,7 @@ async function executeWslMetadataRead(distro: string, script: string): Promise<s
|
||||
// quietly change its interpreter.
|
||||
const result = await runWslProcess({
|
||||
distro,
|
||||
lane: 'probe',
|
||||
// Degrade rather than refuse: a metadata read; no login PATH involved.
|
||||
allowDegradedEnvironment: true,
|
||||
loginPath: 'preferred',
|
||||
script,
|
||||
shell: 'bash',
|
||||
|
||||
|
||||
@@ -57,9 +57,7 @@ async function executeWslSkillDiscovery(distro: string, script: string): Promise
|
||||
// dash rejects with `Syntax error: word unexpected` (#14292).
|
||||
const result = await runWslProcess({
|
||||
distro,
|
||||
lane: 'probe',
|
||||
// Degrade rather than refuse: the scan reads the filesystem; it never needed the login PATH.
|
||||
allowDegradedEnvironment: true,
|
||||
loginPath: 'preferred',
|
||||
script,
|
||||
shell: 'bash',
|
||||
|
||||
|
||||
@@ -50,9 +50,7 @@ type WslEnvironmentProbe = (distro: string) => Promise<string>
|
||||
async function probeWslGrokHome(distro: string): Promise<string> {
|
||||
const result = await runWslProcess({
|
||||
distro,
|
||||
lane: 'probe',
|
||||
// Degrade rather than refuse: reads $HOME, which wsl.exe supplies without a login shell.
|
||||
allowDegradedEnvironment: true,
|
||||
loginPath: 'preferred',
|
||||
script: WSL_GROK_HOME_SCRIPT,
|
||||
timeoutMs: WSL_ENV_PROBE_TIMEOUT_MS,
|
||||
maxOutputBytes: WSL_ENV_PROBE_MAX_BYTES
|
||||
|
||||
@@ -213,9 +213,7 @@ export class WslSkillInstallFilesystem implements SkillInstallFilesystem {
|
||||
private async runOutput(script: string, args: string[]): Promise<string> {
|
||||
const result = await runWslProcess({
|
||||
distro: this.distro,
|
||||
lane: 'probe',
|
||||
// Degrade rather than refuse: mkdir/mv/chmod on the default PATH; no login shell before.
|
||||
allowDegradedEnvironment: true,
|
||||
loginPath: 'none',
|
||||
script,
|
||||
args,
|
||||
timeoutMs: GUEST_COMMAND_TIMEOUT_MS,
|
||||
|
||||
@@ -5,7 +5,7 @@ vi.mock('../wsl/wsl-runner', () => ({ runWslProcess: runWslProcessMock }))
|
||||
|
||||
import { detectSkillProvidersInWsl } from './skill-wsl-provider-detection'
|
||||
|
||||
type RunWslProcessSpec = { distro: string; lane: string; script: string }
|
||||
type RunWslProcessSpec = { distro: string; loginPath: string; script: string }
|
||||
|
||||
describe('detectSkillProvidersInWsl', () => {
|
||||
beforeEach(() => {
|
||||
@@ -16,7 +16,7 @@ describe('detectSkillProvidersInWsl', () => {
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('runs on the probe lane so a PATH-only install (nvm/mise) is still found (regression)', async () => {
|
||||
it('asks for the login PATH so a nvm/mise-only install is still found (regression)', async () => {
|
||||
// Before this migration the site ran `sh -c` with no login shell, so an
|
||||
// nvm-installed codex/claude -- reachable only through the PATH a login
|
||||
// shell assembles from rc files -- resolved to nothing via `command -v`
|
||||
@@ -32,7 +32,7 @@ describe('detectSkillProvidersInWsl', () => {
|
||||
const found = await detectSkillProvidersInWsl('Ubuntu')
|
||||
|
||||
const [spec] = runWslProcessMock.mock.calls.at(-1) as [RunWslProcessSpec]
|
||||
expect(spec.lane).toBe('probe')
|
||||
expect(spec.loginPath).toBe('preferred')
|
||||
expect(spec.distro).toBe('Ubuntu')
|
||||
expect(found).toEqual(['codex'])
|
||||
})
|
||||
|
||||
@@ -13,7 +13,7 @@ export async function detectSkillProvidersInWsl(distro: string): Promise<string[
|
||||
// rc files never applies and an installed codex/claude reads as absent.
|
||||
result = await runWslProcess({
|
||||
distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
script: DETECTION_SCRIPT,
|
||||
timeoutMs: 10_000
|
||||
})
|
||||
|
||||
@@ -26,8 +26,7 @@ describe('deleteWslFishHistoryFile', () => {
|
||||
|
||||
expect(run).toHaveBeenCalledWith({
|
||||
distro: 'Ubuntu Test',
|
||||
lane: 'probe',
|
||||
allowDegradedEnvironment: true,
|
||||
loginPath: 'none',
|
||||
program: 'fish',
|
||||
args: ['--command', expect.stringContaining('orca_0123456789abcdef_history')],
|
||||
timeoutMs: 5_000
|
||||
|
||||
@@ -50,11 +50,8 @@ async function runCleanup(
|
||||
): Promise<void> {
|
||||
const result = await run({
|
||||
distro,
|
||||
lane: 'probe',
|
||||
loginPath: 'none',
|
||||
// The old `--exec fish` spawn never sourced a login shell either, so a
|
||||
// probe failure must run degraded rather than turn a working cleanup into
|
||||
// an error.
|
||||
allowDegradedEnvironment: true,
|
||||
program: 'fish',
|
||||
args: ['--command', fishCleanupScript(session)],
|
||||
timeoutMs: 5_000
|
||||
|
||||
+43
-112
@@ -54,23 +54,20 @@ afterEach(() => {
|
||||
})
|
||||
|
||||
describe('separator', () => {
|
||||
it.each([
|
||||
['probe', 'probe'],
|
||||
['interactive', 'interactive']
|
||||
] as const)('uses --exec and never -- on the %s lane', async (_name, lane) => {
|
||||
it.each([['none'], ['preferred']] as const)('uses --exec and never -- with loginPath %s', async (lane) => {
|
||||
// Why this is pinned on both lanes: under `--`, wsl.exe expands $name in
|
||||
// every forwarded argument before the guest runs -- even with no shell in
|
||||
// the command -- so a script means something other than what it says
|
||||
// (#12964). No escaping on our side is a reliable substitute.
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await runWslProcess({ lane, program: '/usr/bin/git', args: ['status'] })
|
||||
await runWslProcess({ loginPath: lane, program: '/usr/bin/git', args: ['status'] })
|
||||
expect(lastArgv()).toContain('--exec')
|
||||
expect(lastArgv()).not.toContain('--')
|
||||
})
|
||||
|
||||
it('passes the distro before --exec', async () => {
|
||||
seedWslGuestEnvironmentForTests('Ubuntu', ENVIRONMENT)
|
||||
await runWslProcess({ lane: 'probe', distro: 'Ubuntu', program: '/bin/true' })
|
||||
await runWslProcess({ loginPath: 'preferred', distro: 'Ubuntu', program: '/bin/true' })
|
||||
expect(lastArgv().slice(0, 3)).toEqual(['-d', 'Ubuntu', '--exec'])
|
||||
})
|
||||
})
|
||||
@@ -80,7 +77,7 @@ describe('probe lane', () => {
|
||||
// The whole point: the user's real PATH without paying for -- or being
|
||||
// blocked by -- a login shell on every call (#14288, #9768).
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await runWslProcess({ lane: 'probe', program: 'codex', args: ['--version'] })
|
||||
await runWslProcess({ loginPath: 'preferred', program: 'codex', args: ['--version'] })
|
||||
expect(lastArgv()).toEqual([
|
||||
'--exec',
|
||||
'/usr/bin/env',
|
||||
@@ -91,67 +88,30 @@ describe('probe lane', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('reports an unresolved environment rather than pretending the PATH is real', async () => {
|
||||
// Falling back to the login shell here would re-run ~/.profile -- the very
|
||||
// stall the probe lane exists to avoid, and most likely to bite exactly
|
||||
// when the probe just failed. So the call proceeds on the default PATH and
|
||||
// says so, and callers deciding "installed?" must treat that as unknown.
|
||||
let call = 0
|
||||
runProcessMock.mockImplementation(async (spec: { args: string[] }) => {
|
||||
call += 1
|
||||
if (call === 1) {
|
||||
return { environmentResolved: true, code: 1, signal: null, stdout: '', stderr: 'stopped', timedOut: false }
|
||||
}
|
||||
const script = spec.args.at(-1) ?? ''
|
||||
const begin = /__ORCA_WSL_CAPTURE_BEGIN_[a-z0-9]+__/.exec(script)?.[0] ?? ''
|
||||
const end = /__ORCA_WSL_CAPTURE_END_[a-z0-9]+__/.exec(script)?.[0] ?? ''
|
||||
return { environmentResolved: true, code: 0, signal: null, stdout: `${begin}${end}`, stderr: '', timedOut: false }
|
||||
it('runs anyway and says the PATH is unresolved', async () => {
|
||||
// A missing login PATH used to throw, and every knob this runner carried --
|
||||
// cooldown tiers, budget splitting, a re-probe heuristic, an opt-out on 19
|
||||
// of 23 sites -- existed to work around that. It is now just a fact in the
|
||||
// result.
|
||||
runProcessMock.mockResolvedValue({
|
||||
code: 1,
|
||||
signal: null,
|
||||
stdout: '',
|
||||
stderr: 'distro is stopped',
|
||||
timedOut: false
|
||||
})
|
||||
// Default is to refuse: answering "is codex installed?" on the bare default
|
||||
// PATH reports an nvm install as absent, which is #9725.
|
||||
await expect(runWslProcess({ lane: 'probe', program: 'codex' })).rejects.toThrow(
|
||||
/guest environment/
|
||||
)
|
||||
|
||||
const degraded = await runWslProcess({
|
||||
lane: 'probe',
|
||||
program: 'codex',
|
||||
allowDegradedEnvironment: true
|
||||
})
|
||||
expect(degraded.environmentResolved).toBe(false)
|
||||
const result = await runWslProcess({ loginPath: 'preferred', program: 'codex' })
|
||||
expect(result.environmentResolved).toBe(false)
|
||||
expect(lastArgv()).toEqual(['--exec', 'codex'])
|
||||
})
|
||||
|
||||
it('reports a resolved environment on the happy path', async () => {
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
const result = await runWslProcess({ lane: 'probe', program: 'codex' })
|
||||
const result = await runWslProcess({ loginPath: 'preferred', program: 'codex' })
|
||||
expect(result.environmentResolved).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('interactive lane', () => {
|
||||
it('always fences stdout, even when the caller ignores it', async () => {
|
||||
// Stock Ubuntu writes its rc hint to stdout, so an unfenced parse reads the
|
||||
// banner as data (#11327, #11823). A caller that starts parsing later must
|
||||
// not have to remember to opt in.
|
||||
runProcessMock.mockImplementation(async (spec: { args: string[] }) => {
|
||||
const script = spec.args.at(-1) ?? ''
|
||||
const begin = /__ORCA_WSL_CAPTURE_BEGIN_[a-z0-9]+__/.exec(script)?.[0] ?? ''
|
||||
const end = /__ORCA_WSL_CAPTURE_END_[a-z0-9]+__/.exec(script)?.[0] ?? ''
|
||||
return {
|
||||
environmentResolved: true,
|
||||
code: 0,
|
||||
signal: null,
|
||||
stdout: `Ubuntu banner: run a command as administrator\n${begin}payload${end}`,
|
||||
stderr: '',
|
||||
timedOut: false
|
||||
}
|
||||
})
|
||||
const result = await runWslProcess({ lane: 'interactive', program: 'claude' })
|
||||
expect(result.stdout).toBe('payload')
|
||||
})
|
||||
})
|
||||
|
||||
describe('scripts', () => {
|
||||
it('delivers a script on stdin through sh -s, not in argv', async () => {
|
||||
// A script on stdin has no quoting boundary for its own quotes to escape
|
||||
@@ -159,7 +119,7 @@ describe('scripts', () => {
|
||||
// (#14292), and why this is the only supported way to run one.
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
const script = `case "$x" in a) echo 'it'\\''s fine';; esac`
|
||||
await runWslProcess({ lane: 'probe', script, args: ['/tmp/root'] })
|
||||
await runWslProcess({ loginPath: 'preferred', script, args: ['/tmp/root'] })
|
||||
expect(runProcessMock.mock.calls.at(-1)?.[0].input).toBe(script)
|
||||
expect(lastArgv()).toEqual([
|
||||
'--exec',
|
||||
@@ -180,7 +140,7 @@ describe('WSLENV', () => {
|
||||
// Unset, a Windows-side variable silently never reaches the guest (#12557).
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await runWslProcess({
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
program: '/bin/true',
|
||||
env: { GITLAB_HOST: 'git.example.com', GH_TOKEN: 't' }
|
||||
})
|
||||
@@ -189,17 +149,27 @@ describe('WSLENV', () => {
|
||||
expect(env.WSLENV?.split(':')).toEqual(expect.arrayContaining(['GITLAB_HOST', 'GH_TOKEN']))
|
||||
})
|
||||
|
||||
it('leaves the host environment alone when nothing is propagated', async () => {
|
||||
it('always sets WSL_UTF8, even with nothing to propagate', async () => {
|
||||
// Without it wsl.exe writes its own error text as UTF-16LE, so anything
|
||||
// surfacing stderr shows NUL-riddled output (#9010).
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await runWslProcess({ lane: 'probe', program: '/bin/true' })
|
||||
expect(runProcessMock.mock.calls.at(-1)?.[0].env).toBeUndefined()
|
||||
await runWslProcess({ loginPath: 'preferred', program: '/bin/true' })
|
||||
const env = runProcessMock.mock.calls.at(-1)?.[0].env as NodeJS.ProcessEnv
|
||||
expect(env.WSL_UTF8).toBe('1')
|
||||
})
|
||||
|
||||
it('adds no WSLENV entry when nothing is propagated', async () => {
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await runWslProcess({ loginPath: 'preferred', program: '/bin/true' })
|
||||
const env = runProcessMock.mock.calls.at(-1)?.[0].env as NodeJS.ProcessEnv
|
||||
expect(env.WSLENV).toBe(process.env.WSLENV)
|
||||
})
|
||||
})
|
||||
|
||||
describe('guest cwd', () => {
|
||||
it('cds inside the guest rather than passing a Windows cwd to wsl.exe', async () => {
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await runWslProcess({ lane: 'probe', program: '/usr/bin/git', cwd: '/home/u/repo' })
|
||||
await runWslProcess({ loginPath: 'preferred', program: '/usr/bin/git', cwd: '/home/u/repo' })
|
||||
expect(runProcessMock.mock.calls.at(-1)?.[0].cwd).toBeUndefined()
|
||||
expect(lastArgv()).toContain('/home/u/repo')
|
||||
expect(lastArgv()).toContain('sh')
|
||||
@@ -209,7 +179,7 @@ describe('guest cwd', () => {
|
||||
// A Windows path here means a mistake further up; converting it silently
|
||||
// hides that.
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await expect(runWslProcess({ lane: 'probe', program: '/bin/true', cwd })).rejects.toThrow(
|
||||
await expect(runWslProcess({ loginPath: 'preferred', program: '/bin/true', cwd })).rejects.toThrow(
|
||||
/guest path/
|
||||
)
|
||||
})
|
||||
@@ -219,7 +189,7 @@ describe('program is a binary, not a shell string', () => {
|
||||
it.each([['sh -c echo hi'], ['a; b'], ['a | b'], ['a && b'], ['echo $HOME'], ['a > b']])(
|
||||
'rejects %s',
|
||||
async (program) => {
|
||||
await expect(runWslProcess({ lane: 'probe', program })).rejects.toThrow(/single binary/)
|
||||
await expect(runWslProcess({ loginPath: 'preferred', program })).rejects.toThrow(/single binary/)
|
||||
}
|
||||
)
|
||||
|
||||
@@ -228,55 +198,17 @@ describe('program is a binary, not a shell string', () => {
|
||||
// rejecting it would fail legitimate installs under a spaced directory.
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await expect(
|
||||
runWslProcess({ lane: 'probe', program: '/home/u/my tools/codex' })
|
||||
runWslProcess({ loginPath: 'preferred', program: '/home/u/my tools/codex' })
|
||||
).resolves.toBeDefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('a script gets the cached environment on both lanes', () => {
|
||||
it('applies the cached PATH even when the caller asked for interactive', async () => {
|
||||
// A script never runs under the login shell (it owns stdin), so without
|
||||
// this the interactive lane would give a script LESS PATH than the probe
|
||||
// lane -- for a caller that explicitly asked for the user's terminal PATH.
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await runWslProcess({ lane: 'interactive', script: 'command -v codex' })
|
||||
expect(lastArgv()).toEqual([
|
||||
'--exec',
|
||||
'/usr/bin/env',
|
||||
'PATH=/home/u/.nvm/bin:/usr/bin',
|
||||
'HOME=/home/u',
|
||||
'sh',
|
||||
'-s',
|
||||
'--'
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe('a missing fence is a failure, not empty output', () => {
|
||||
it('throws rather than returning a clean empty result', async () => {
|
||||
// readStdout returns null precisely to distinguish "no fence" from "empty
|
||||
// payload". An rc that redirects stdout would otherwise yield a silent
|
||||
// wrong answer.
|
||||
runProcessMock.mockResolvedValue({
|
||||
environmentResolved: true,
|
||||
code: 0,
|
||||
signal: null,
|
||||
stdout: 'banner only, no fence',
|
||||
stderr: '',
|
||||
timedOut: false
|
||||
})
|
||||
await expect(runWslProcess({ lane: 'interactive', program: 'claude' })).rejects.toThrow(
|
||||
/no fenced output/
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('program is not an assignment', () => {
|
||||
it('rejects a name=value program that env would swallow', async () => {
|
||||
// `env PATH=… HOME=… FOO=bar` has no command left: it prints the whole
|
||||
// guest environment and exits 0 -- success, with the environment as stdout.
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await expect(runWslProcess({ lane: 'probe', program: 'FOO=bar' })).rejects.toThrow(
|
||||
await expect(runWslProcess({ loginPath: 'preferred', program: 'FOO=bar' })).rejects.toThrow(
|
||||
/assignment/
|
||||
)
|
||||
})
|
||||
@@ -288,7 +220,7 @@ describe('timeout budget', () => {
|
||||
// 5s caller could reach runProcess with 1ms left and report a timeout for a
|
||||
// command that would have taken milliseconds.
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await runWslProcess({ lane: 'probe', program: '/bin/true', timeoutMs: 5_000 })
|
||||
await runWslProcess({ loginPath: 'preferred', program: '/bin/true', timeoutMs: 5_000 })
|
||||
const passed = runProcessMock.mock.calls.at(-1)?.[0].timeoutMs as number
|
||||
expect(passed).toBeGreaterThan(1_000)
|
||||
expect(passed).toBeLessThanOrEqual(5_000)
|
||||
@@ -308,10 +240,9 @@ describe('the probe never starves the command', () => {
|
||||
// and passes for any split. allowDegradedEnvironment keeps the call alive
|
||||
// once the probe fails.
|
||||
await runWslProcess({
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
program: '/bin/true',
|
||||
timeoutMs,
|
||||
allowDegradedEnvironment: true
|
||||
})
|
||||
const probeMs = runProcessMock.mock.calls[0]?.[0].timeoutMs as number
|
||||
expect(probeMs).toBeLessThanOrEqual(timeoutMs - floor)
|
||||
@@ -321,7 +252,7 @@ describe('the probe never starves the command', () => {
|
||||
describe('script interpreter', () => {
|
||||
it('defaults to sh', async () => {
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await runWslProcess({ lane: 'probe', script: 'echo hi' })
|
||||
await runWslProcess({ loginPath: 'preferred', script: 'echo hi' })
|
||||
expect(lastArgv()).toContain('sh')
|
||||
expect(lastArgv()).not.toContain('bash')
|
||||
})
|
||||
@@ -332,7 +263,7 @@ describe('script interpreter', () => {
|
||||
// unexpected` -- the #14292 signature -- so a bash caller must be able to
|
||||
// say so rather than be silently downgraded.
|
||||
seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT)
|
||||
await runWslProcess({ lane: 'probe', script: 'done < <(find .)', shell: 'bash' })
|
||||
await runWslProcess({ loginPath: 'preferred', script: 'done < <(find .)', shell: 'bash' })
|
||||
expect(lastArgv()).toContain('bash')
|
||||
expect(lastArgv()).not.toContain('sh')
|
||||
})
|
||||
@@ -351,7 +282,7 @@ describe('a script never rides the interactive lane', () => {
|
||||
stderr: 'distro is stopped',
|
||||
timedOut: false
|
||||
})
|
||||
await runWslProcess({ lane: 'probe', script: 'echo hi', allowDegradedEnvironment: true })
|
||||
await runWslProcess({ loginPath: 'preferred', script: 'echo hi' })
|
||||
expect(lastArgv()).toEqual(['--exec', 'sh', '-s', '--'])
|
||||
expect(runProcessMock.mock.calls.at(-1)?.[0].input).toBe('echo hi')
|
||||
})
|
||||
|
||||
+38
-86
@@ -1,10 +1,6 @@
|
||||
import { addWslEnvKeys } from '../../shared/wsl-env'
|
||||
import { runProcess } from '../../shared/child-process/run-process'
|
||||
import {
|
||||
buildWslCapturedLoginShellCommand,
|
||||
buildWslExecArgs,
|
||||
quotePosixShell
|
||||
} from '../../shared/wsl-login-shell-command'
|
||||
import { buildWslExecArgs } from '../../shared/wsl-login-shell-command'
|
||||
import { getWslGuestEnvironment, type WslGuestEnvironment } from './wsl-guest-environment'
|
||||
import { resolveWslExecutablePath } from './wsl-executable-path'
|
||||
|
||||
@@ -17,11 +13,20 @@ import { resolveWslExecutablePath } from './wsl-executable-path'
|
||||
* docs/reference/wsl-command-execution.md.
|
||||
*/
|
||||
|
||||
export type WslLane =
|
||||
/** No shell. Cached login PATH/HOME, applied via `env`. */
|
||||
| 'probe'
|
||||
/** Login shell, always fenced. For anything whose PATH must match the user's terminal. */
|
||||
| 'interactive'
|
||||
/**
|
||||
* How much the call needs the user's login PATH.
|
||||
*
|
||||
* Why one axis and not `lane` + `allowDegradedEnvironment`: 19 of 23 sites
|
||||
* passed the opt-out, and two of them said in comments that they did not want
|
||||
* the login PATH at all -- the flag had become the `'none'` this union was
|
||||
* missing. A required discriminator whose default nobody wants is not a
|
||||
* discriminator.
|
||||
*/
|
||||
export type WslLoginPath =
|
||||
/** The command reads $HOME or absolute paths only. No probe. */
|
||||
| 'none'
|
||||
/** Use the cached login PATH when there is one; run anyway when there is not. */
|
||||
| 'preferred'
|
||||
|
||||
/**
|
||||
* What to run: a single binary, or a script.
|
||||
@@ -53,27 +58,14 @@ export type WslCommand =
|
||||
export type WslSpec = WslCommand & {
|
||||
/** Undefined selects the distro's default. */
|
||||
distro?: string
|
||||
/**
|
||||
* Required, with no default. The wrong lane is the most common WSL defect in
|
||||
* this tree, and a default lets a call site pick it by omission.
|
||||
*/
|
||||
lane: WslLane
|
||||
/** Required, with no default: the wrong answer here is the defect this file exists to prevent. */
|
||||
loginPath: WslLoginPath
|
||||
/** Guest (POSIX) path. */
|
||||
cwd?: string
|
||||
/** Host variables to propagate into the guest; sets WSLENV automatically. */
|
||||
env?: Readonly<Record<string, string>>
|
||||
timeoutMs?: number
|
||||
maxOutputBytes?: number
|
||||
/**
|
||||
* Proceed when the login PATH could not be established.
|
||||
*
|
||||
* Default is to throw. Three separate reviews found the same class of bug --
|
||||
* a call that answers "is this installed?" running on the bare default PATH
|
||||
* and reporting an nvm-installed tool absent (#9725). Making degradation
|
||||
* opt-in puts that decision in the one place a reader will look, instead of
|
||||
* relying on every call site to remember to check `environmentResolved`.
|
||||
*/
|
||||
allowDegradedEnvironment?: boolean
|
||||
}
|
||||
|
||||
export type WslResult = {
|
||||
@@ -93,21 +85,6 @@ export type WslResult = {
|
||||
|
||||
export const DEFAULT_WSL_TIMEOUT_MS = 30_000
|
||||
|
||||
/**
|
||||
* The guest login PATH could not be established.
|
||||
*
|
||||
* Typed so a caller answering "is this installed?" can report unverifiable
|
||||
* rather than absent -- reporting an nvm-installed tool absent is #9725, and a
|
||||
* bare Error would just be swallowed by the same catch that handles real
|
||||
* failures.
|
||||
*/
|
||||
export class WslGuestEnvironmentUnavailableError extends Error {
|
||||
constructor(distro: string | undefined) {
|
||||
super(`WSL guest environment for ${distro ?? 'the default distro'} is unavailable`)
|
||||
this.name = 'WslGuestEnvironmentUnavailableError'
|
||||
}
|
||||
}
|
||||
|
||||
function assertGuestPath(cwd: string): void {
|
||||
// Why reject rather than convert: a caller passing a Windows path here has
|
||||
// usually made a different mistake further up, and silently translating it
|
||||
@@ -131,13 +108,20 @@ function assertNotShellString(program: string): void {
|
||||
}
|
||||
}
|
||||
|
||||
/** Host env plus the WSLENV entries that let it cross the boundary. */
|
||||
function buildHostEnv(env: WslSpec['env']): NodeJS.ProcessEnv | undefined {
|
||||
if (!env || Object.keys(env).length === 0) {
|
||||
return undefined
|
||||
/**
|
||||
* Host env plus WSL_UTF8 and the WSLENV entries that let values cross.
|
||||
*
|
||||
* Why WSL_UTF8 unconditionally: without it `wsl.exe` writes its OWN messages
|
||||
* ("There is no distribution with the supplied name") as UTF-16LE, so every
|
||||
* caller that surfaces stderr shows NUL-riddled text. Setting it per-call site
|
||||
* is how it got lost -- the relay set it, the migration dropped it, and nothing
|
||||
* noticed because the happy path is pure ASCII. Credit: #9010.
|
||||
*/
|
||||
function buildHostEnv(env: WslSpec['env']): NodeJS.ProcessEnv {
|
||||
const merged: NodeJS.ProcessEnv = { ...process.env, ...env, WSL_UTF8: '1' }
|
||||
if (env && Object.keys(env).length > 0) {
|
||||
addWslEnvKeys(merged, Object.keys(env))
|
||||
}
|
||||
const merged: NodeJS.ProcessEnv = { ...process.env, ...env }
|
||||
addWslEnvKeys(merged, Object.keys(env))
|
||||
return merged
|
||||
}
|
||||
|
||||
@@ -177,38 +161,13 @@ function buildGuestArgv(environment: WslGuestEnvironment | null, spec: WslSpec):
|
||||
return withGuestCwd(spec.cwd, argv)
|
||||
}
|
||||
|
||||
function buildInteractiveArgv(spec: WslSpec): {
|
||||
argv: string[]
|
||||
readStdout: (stdout: string) => string
|
||||
} {
|
||||
// Why the whole invocation goes through the fence: a caller that does not
|
||||
// parse stdout today may start tomorrow, and the banner is invisible until
|
||||
// then.
|
||||
const quoted = guestCommandArgv(spec).map(quotePosixShell).join(' ')
|
||||
const body = spec.cwd ? `cd ${quotePosixShell(spec.cwd)} || exit 1\n${quoted}` : quoted
|
||||
const captured = buildWslCapturedLoginShellCommand(body)
|
||||
return {
|
||||
argv: ['sh', '-c', captured.command],
|
||||
// Why throw rather than default to '': readStdout returns null precisely to
|
||||
// distinguish "the fence never appeared" from "the payload was empty". An rc
|
||||
// that redirects stdout, or output truncated before the begin marker, would
|
||||
// otherwise return a clean, empty, wrong answer.
|
||||
readStdout: (stdout: string) => {
|
||||
const payload = captured.readStdout(stdout)
|
||||
if (payload === null) {
|
||||
throw new Error('WSL login shell produced no fenced output')
|
||||
}
|
||||
return payload
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Run a program inside WSL.
|
||||
*
|
||||
* Throws when the guest login PATH cannot be established, unless the caller
|
||||
* passes `allowDegradedEnvironment`. Falling back to the login shell here would
|
||||
* re-run ~/.profile -- the stall this exists to remove.
|
||||
* A missing login PATH is never fatal: the call runs on the distro's default
|
||||
* PATH and reports `environmentResolved: false`. Every knob this file used to
|
||||
* carry -- cooldown tiers, budget splitting, a re-probe heuristic, an opt-out
|
||||
* flag on 19 of 23 sites -- existed only because that case used to throw.
|
||||
*/
|
||||
export async function runWslProcess(spec: WslSpec): Promise<WslResult> {
|
||||
if (spec.program !== undefined) {
|
||||
@@ -223,7 +182,7 @@ export async function runWslProcess(spec: WslSpec): Promise<WslResult> {
|
||||
// shell (see below), so on the interactive lane it would otherwise get no
|
||||
// login PATH at all -- strictly less than the probe lane, for a caller that
|
||||
// explicitly asked for the user's terminal PATH.
|
||||
const wantsEnvironment = spec.lane === 'probe' || spec.script !== undefined
|
||||
const wantsEnvironment = spec.loginPath === 'preferred'
|
||||
// Leave the command at least a third of the budget: a probe that eats it all
|
||||
// turns a healthy command into a spurious timeout.
|
||||
// Cap the probe at half the budget and at 4s: a 5s caller was giving the
|
||||
@@ -240,21 +199,14 @@ export async function runWslProcess(spec: WslSpec): Promise<WslResult> {
|
||||
// the probe most often fails *because* the distro is slow, so the fallback
|
||||
// would hit the hazard exactly when it is worst. Run shell-free with the
|
||||
// distro's default PATH instead: degraded, never blocking.
|
||||
if (wantsEnvironment && environment === null && !spec.allowDegradedEnvironment) {
|
||||
throw new WslGuestEnvironmentUnavailableError(spec.distro)
|
||||
}
|
||||
|
||||
const lane =
|
||||
spec.lane === 'interactive' && spec.script === undefined
|
||||
? ({ kind: 'interactive', ...buildInteractiveArgv(spec) } as const)
|
||||
: ({ kind: 'probe', argv: buildGuestArgv(environment, spec) } as const)
|
||||
const argv = buildGuestArgv(environment, spec)
|
||||
|
||||
// One budget for the whole call: the probe used to run on its own 10s timer
|
||||
// ahead of the timed leg, so a 5s caller could wait 15s.
|
||||
const remainingMs = Math.max(1, deadline - Date.now())
|
||||
const result = await runProcess({
|
||||
program: resolveWslExecutablePath(),
|
||||
args: buildWslExecArgs(spec.distro, lane.argv),
|
||||
args: buildWslExecArgs(spec.distro, argv),
|
||||
env: buildHostEnv(spec.env),
|
||||
input: spec.script,
|
||||
timeoutMs: remainingMs,
|
||||
@@ -264,7 +216,7 @@ export async function runWslProcess(spec: WslSpec): Promise<WslResult> {
|
||||
return {
|
||||
environmentResolved: !wantsEnvironment || environment !== null,
|
||||
code: result.code,
|
||||
stdout: lane.kind === 'interactive' ? lane.readStdout(result.stdout) : result.stdout,
|
||||
stdout: result.stdout,
|
||||
stderr: result.stderr,
|
||||
timedOut: result.timedOut
|
||||
}
|
||||
|
||||
@@ -50,9 +50,8 @@ describeOnWsl('runWslProcess against a real distro', () => {
|
||||
// otherwise refuses an unresolved PATH.
|
||||
const started = Date.now()
|
||||
const result = await runWslProcess({
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
distro: DISTRO,
|
||||
allowDegradedEnvironment: true,
|
||||
program: '/bin/echo',
|
||||
args: ['orca-probe-ok'],
|
||||
timeoutMs: 15_000
|
||||
@@ -65,11 +64,10 @@ describeOnWsl('runWslProcess against a real distro', () => {
|
||||
it('second probe-lane call does not pay the login shell again', async () => {
|
||||
const started = Date.now()
|
||||
await runWslProcess({
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
distro: DISTRO,
|
||||
program: '/bin/true',
|
||||
timeoutMs: 15_000,
|
||||
allowDegradedEnvironment: true
|
||||
})
|
||||
expect(Date.now() - started).toBeLessThan(5_000)
|
||||
}, 30_000)
|
||||
@@ -78,7 +76,7 @@ describeOnWsl('runWslProcess against a real distro', () => {
|
||||
// Stock Ubuntu writes its rc hint to stdout. Anything parsing that stream
|
||||
// reads the banner as data unless the fence removes it (#11327, #11823).
|
||||
const result = await runWslProcess({
|
||||
lane: 'interactive',
|
||||
loginPath: 'preferred',
|
||||
distro: DISTRO,
|
||||
program: '/bin/echo',
|
||||
args: ['ORCA_PAYLOAD'],
|
||||
@@ -96,9 +94,8 @@ describeOnWsl('runWslProcess against a real distro', () => {
|
||||
`echo "x" | awk '{print $1}'`
|
||||
].join('\n')
|
||||
const result = await runWslProcess({
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
distro: DISTRO,
|
||||
allowDegradedEnvironment: true,
|
||||
script,
|
||||
args: ['ORCA_ARG'],
|
||||
timeoutMs: 30_000
|
||||
@@ -113,9 +110,8 @@ describeOnWsl('runWslProcess against a real distro', () => {
|
||||
|
||||
it('propagated env crosses the boundary via WSLENV', async () => {
|
||||
const result = await runWslProcess({
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
distro: DISTRO,
|
||||
allowDegradedEnvironment: true,
|
||||
script: 'printf %s "$ORCA_WSLENV_PROBE"',
|
||||
env: { ORCA_WSLENV_PROBE: 'crossed' },
|
||||
timeoutMs: 30_000
|
||||
@@ -125,9 +121,8 @@ describeOnWsl('runWslProcess against a real distro', () => {
|
||||
|
||||
it('runs in the requested guest cwd', async () => {
|
||||
const result = await runWslProcess({
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
distro: DISTRO,
|
||||
allowDegradedEnvironment: true,
|
||||
program: '/bin/pwd',
|
||||
cwd: '/tmp',
|
||||
timeoutMs: 30_000
|
||||
|
||||
@@ -44,27 +44,27 @@ describe('W1: every WSL call inherits the spawn chokepoint', () => {
|
||||
it('resolves wsl.exe absolutely, never by bare name', async () => {
|
||||
// Bare-name resolution is what a Group Policy or a stripped Electron PATH
|
||||
// hijacks (#15749).
|
||||
await runWslProcess({ lane: 'probe', distro: 'Ubuntu', program: '/bin/true' })
|
||||
await runWslProcess({ loginPath: 'preferred', distro: 'Ubuntu', program: '/bin/true' })
|
||||
expect(spawnSpec().program).toBe('C:\\Windows\\System32\\wsl.exe')
|
||||
})
|
||||
|
||||
it('passes argv as an array, never a command string', async () => {
|
||||
// A command string would put quoting back in the caller's hands, which is
|
||||
// the entire class W1 removed.
|
||||
await runWslProcess({ lane: 'probe', distro: 'Ubuntu', program: '/bin/echo', args: ['a b'] })
|
||||
await runWslProcess({ loginPath: 'preferred', distro: 'Ubuntu', program: '/bin/echo', args: ['a b'] })
|
||||
expect(Array.isArray(spawnSpec().args)).toBe(true)
|
||||
expect(spawnSpec().args).toContain('a b')
|
||||
})
|
||||
|
||||
it('always bounds the call', async () => {
|
||||
await runWslProcess({ lane: 'probe', distro: 'Ubuntu', program: '/bin/true' })
|
||||
await runWslProcess({ loginPath: 'preferred', distro: 'Ubuntu', program: '/bin/true' })
|
||||
expect(spawnSpec().timeoutMs).toBeGreaterThan(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe('W3: the five per-call decisions are made once', () => {
|
||||
it('never uses the -- separator, which expands $name host-side', async () => {
|
||||
await runWslProcess({ lane: 'probe', distro: 'Ubuntu', script: 'echo "$HOME"' })
|
||||
await runWslProcess({ loginPath: 'preferred', distro: 'Ubuntu', script: 'echo "$HOME"' })
|
||||
const argv = spawnSpec().args as string[]
|
||||
// Only wsl.exe's own separator matters. A later `--` belongs to `sh -s --`
|
||||
// and is the guest shell's end-of-options, so check the position rather
|
||||
@@ -77,14 +77,14 @@ describe('W3: the five per-call decisions are made once', () => {
|
||||
// The base64 and eval wrappers existed because argv quoting kept breaking;
|
||||
// stdin has no quoting boundary at all (#14292).
|
||||
const script = `case "$x" in a) echo 'it'\\''s fine';; esac`
|
||||
await runWslProcess({ lane: 'probe', distro: 'Ubuntu', script })
|
||||
await runWslProcess({ loginPath: 'preferred', distro: 'Ubuntu', script })
|
||||
expect(spawnSpec().input).toBe(script)
|
||||
expect(JSON.stringify(spawnSpec().args)).not.toContain('case')
|
||||
})
|
||||
|
||||
it('propagates env only through WSLENV', async () => {
|
||||
await runWslProcess({
|
||||
lane: 'probe',
|
||||
loginPath: 'preferred',
|
||||
distro: 'Ubuntu',
|
||||
program: '/bin/true',
|
||||
env: { GITLAB_HOST: 'git.example.com' }
|
||||
@@ -95,14 +95,14 @@ describe('W3: the five per-call decisions are made once', () => {
|
||||
|
||||
it('runs no shell on the probe lane, so ~/.profile cannot stall it', async () => {
|
||||
// #14288: one blocking line in ~/.profile ate the whole timeout.
|
||||
await runWslProcess({ lane: 'probe', distro: 'Ubuntu', program: 'codex' })
|
||||
await runWslProcess({ loginPath: 'preferred', distro: 'Ubuntu', program: 'codex' })
|
||||
expect(JSON.stringify(spawnSpec().args)).not.toContain('_orca_wsl_shell')
|
||||
})
|
||||
|
||||
it('applies the user login PATH even with no shell in the loop', async () => {
|
||||
// The other half of the same trade: nvm-installed agents must still be
|
||||
// found (#9725, #7563, #8366).
|
||||
await runWslProcess({ lane: 'probe', distro: 'Ubuntu', program: 'codex' })
|
||||
await runWslProcess({ loginPath: 'preferred', distro: 'Ubuntu', program: 'codex' })
|
||||
expect(spawnSpec().args).toContain('PATH=/home/u/.nvm/bin:/usr/bin')
|
||||
})
|
||||
})
|
||||
@@ -118,7 +118,7 @@ describe('failure modes stay distinguishable', () => {
|
||||
stderr: '',
|
||||
timedOut: false
|
||||
})
|
||||
const result = await runWslProcess({ lane: 'probe', distro: 'Ubuntu', program: '/bin/false' })
|
||||
const result = await runWslProcess({ loginPath: 'preferred', distro: 'Ubuntu', program: '/bin/false' })
|
||||
expect(result.code).toBe(3)
|
||||
expect(result.timedOut).toBe(false)
|
||||
})
|
||||
@@ -131,11 +131,11 @@ describe('failure modes stay distinguishable', () => {
|
||||
stderr: '',
|
||||
timedOut: true
|
||||
})
|
||||
const result = await runWslProcess({ lane: 'probe', distro: 'Ubuntu', program: '/bin/true' })
|
||||
const result = await runWslProcess({ loginPath: 'preferred', distro: 'Ubuntu', program: '/bin/true' })
|
||||
expect(result.timedOut).toBe(true)
|
||||
})
|
||||
|
||||
it('an unresolved login PATH refuses rather than answering wrongly', async () => {
|
||||
it('an unresolved login PATH is reported, never fatal', async () => {
|
||||
invalidateWslGuestEnvironment(undefined, true)
|
||||
runProcessMock.mockResolvedValue({
|
||||
code: 1,
|
||||
@@ -144,8 +144,12 @@ describe('failure modes stay distinguishable', () => {
|
||||
stderr: 'stopped',
|
||||
timedOut: false
|
||||
})
|
||||
await expect(
|
||||
runWslProcess({ lane: 'probe', distro: 'Ubuntu', program: 'codex' })
|
||||
).rejects.toThrow(/guest environment/)
|
||||
// Every knob this runner used to carry existed because this case threw.
|
||||
const result = await runWslProcess({
|
||||
loginPath: 'preferred',
|
||||
distro: 'Ubuntu',
|
||||
program: 'codex'
|
||||
})
|
||||
expect(result.environmentResolved).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -18,7 +18,9 @@ import {
|
||||
|
||||
function decodeWslLoginShellScript(command: string): string {
|
||||
const encoded =
|
||||
/(?:--|--exec) sh -c 'eval \\"`printf %s ([A-Za-z0-9+/=]+) \| base64 -d`\\"'/.exec(command)?.[1]
|
||||
/(?:--|--exec) sh -c '(?:eval \\"`|sh -c \\"\$\()?printf %s ([A-Za-z0-9+/=]+) \| base64 -d/.exec(
|
||||
command
|
||||
)?.[1]
|
||||
expect(encoded).toBeDefined()
|
||||
return Buffer.from(encoded!, 'base64').toString('utf8')
|
||||
}
|
||||
@@ -48,7 +50,7 @@ describe('CliSkillRuntimeSetup runtime helpers', () => {
|
||||
|
||||
expect(command).toBe(skillCommand)
|
||||
expect(setupCommand).toBe(
|
||||
`& { $PSNativeCommandArgumentPassing = 'Legacy'; wsl.exe -d 'Ubuntu' --exec sh -c 'eval \\"\`printf %s ${encoded} | base64 -d\`\\"' } # Runs: ${skillCommand}`
|
||||
`& { $PSNativeCommandArgumentPassing = 'Legacy'; wsl.exe -d 'Ubuntu' --exec sh -c 'sh -c \\"$(printf %s ${encoded} | base64 -d)\\"' } # Runs: ${skillCommand}`
|
||||
)
|
||||
expect(decodeWslLoginShellScript(setupCommand)).toContain(
|
||||
'exec "$_orca_wsl_shell" -ilc \'npx skills add orchestration --global\''
|
||||
@@ -93,9 +95,11 @@ describe('CliSkillRuntimeSetup runtime helpers', () => {
|
||||
const setupCommand = buildSkillSetupTerminalCommand(command, 'powershell.exe', runtime, 'win32')
|
||||
|
||||
expect(setupCommand).toMatch(
|
||||
/^& \{ \$PSNativeCommandArgumentPassing = 'Legacy'; wsl\.exe --exec sh -c 'eval \\"`printf/
|
||||
/^& \{ \$PSNativeCommandArgumentPassing = 'Legacy'; wsl\.exe --exec sh -c 'sh -c/
|
||||
)
|
||||
expect(setupCommand).toContain('`\\"\' } # Runs: npx skills update orchestration --global')
|
||||
// The command no longer ends in the eval wrapper's nested quotes; it is a
|
||||
// plain pipe into sh, so the payload has no quoting layer to escape from.
|
||||
expect(setupCommand).toContain('base64 -d)')
|
||||
})
|
||||
|
||||
it.skipIf(process.platform === 'win32')(
|
||||
|
||||
@@ -164,7 +164,15 @@ function buildPowerShellWslSkillCommand(command: string, runtime: LocalAgentRunt
|
||||
// Why: encoding preserves the user's configured login-shell PATH across the Windows argv boundary.
|
||||
const encodedScript = encodeWslLoginShellScript(command)
|
||||
const visibleCommand = command.replace(/[\r\n]+/g, ' ')
|
||||
const shellScript = `eval "\`printf %s ${encodedScript} | base64 -d\`"`
|
||||
// Why $(...) and not a backtick eval: PowerShell treats ` as its own escape
|
||||
// character, so the old `eval "\`printf ...\`"` had the payload's quotes
|
||||
// interacting with two escaping layers and dash saw `case in` -- the
|
||||
// `word unexpected (expecting "in")` in #14292. Credit: #14785.
|
||||
//
|
||||
// Why not a plain pipe into sh: that hands the payload the pipe as its stdin,
|
||||
// so a setup command that reads input gets base64 remnants instead. Command
|
||||
// substitution runs in a subshell and leaves the terminal's stdin intact.
|
||||
const shellScript = `sh -c "$(printf %s ${encodedScript} | base64 -d)"`
|
||||
// Why --exec: `--` makes wsl.exe expand $name in the argv it forwards to the guest.
|
||||
const wslCommand = `wsl.exe${distroArg} --exec sh -c ${quotePowerShellNativeArgument(shellScript)}`
|
||||
return `& { $PSNativeCommandArgumentPassing = 'Legacy'; ${wslCommand} } # Runs: ${visibleCommand}`
|
||||
@@ -180,7 +188,10 @@ function decodeWslSetupTerminalCommand(command: string): string | null {
|
||||
|
||||
// Why both separators: commands persisted before the --exec switch must still decode.
|
||||
const encoded =
|
||||
/(?:--|--exec) sh -c 'eval \\"`printf %s ([A-Za-z0-9+/=]+) \| base64 -d`\\"'/.exec(command)?.[1]
|
||||
// Both shapes: commands persisted before the pipe switch still decode.
|
||||
/(?:--|--exec) sh -c '(?:eval \\"`|sh -c \\"\$\()?printf %s ([A-Za-z0-9+/=]+) \| base64 -d/.exec(
|
||||
command
|
||||
)?.[1]
|
||||
if (!encoded) {
|
||||
return null
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user