diff --git a/packages/acp-bridge/src/sessionArtifacts.test.ts b/packages/acp-bridge/src/sessionArtifacts.test.ts index 8e43369525..ffb0375058 100644 --- a/packages/acp-bridge/src/sessionArtifacts.test.ts +++ b/packages/acp-bridge/src/sessionArtifacts.test.ts @@ -6,7 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createHash } from 'node:crypto'; -import { promises as fs } from 'node:fs'; +import { promises as fs, type BigIntStats, type Stats } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { pathToFileURL } from 'node:url'; @@ -46,6 +46,30 @@ describe('SessionArtifactStore', () => { .slice(0, 16); } + // Forward the options through: the store lstats with { bigint: true } + // for the identity check, and a spy that dropped it would hand back + // numeric stats that never equal the bigint fstat — hiding a broken + // comparison behind a type mismatch. + function spyOnLstatForwarding( + hook: ( + entry: Parameters[0], + stat: Stats | BigIntStats, + ) => void | Promise, + ) { + const originalLstat = fs.lstat.bind(fs); + return vi.spyOn(fs, 'lstat').mockImplementation((async ( + entry: Parameters[0], + options?: Parameters[1], + ) => { + const stat = await originalLstat( + entry, + options as Parameters[1], + ); + await hook(entry, stat); + return stat; + }) as typeof fs.lstat); + } + it('lists, removes, and idempotently ignores missing artifact deletes', async () => { const store = new SessionArtifactStore({ sessionId: 's1', @@ -3244,16 +3268,13 @@ describe('SessionArtifactStore', () => { vi.useFakeTimers(); vi.setSystemTime(new Date(Date.now() + 6_000)); - const originalLstat = fs.lstat.bind(fs); let swapped = false; - const lstatSpy = vi.spyOn(fs, 'lstat').mockImplementation(async (entry) => { - const stat = await originalLstat(entry); + const lstatSpy = spyOnLstatForwarding(async (entry) => { if (!swapped && String(entry) === realTarget) { swapped = true; await fs.writeFile(replacement, 'after'); await fs.rename(replacement, target); } - return stat; }); try { @@ -3266,6 +3287,89 @@ describe('SessionArtifactStore', () => { } }); + it('detects a swap whose file ids differ only above 2^53', async () => { + const store = new SessionArtifactStore({ + sessionId: 's7-bigint-identity', + workspaceCwd: workspace, + }); + await fs.writeFile(path.join(workspace, 'big.txt'), 'content'); + const created = await store.upsertMany([ + { title: 'Big', workspacePath: 'big.txt' }, + ]); + const artifactId = created.changes[0]!.artifactId; + const realTarget = await fs.realpath(path.join(workspace, 'big.txt')); + + // Injected ids on both sides of 2^53: the replacement rounds to the SAME + // Number as the original, so only the bigint comparison can see the swap. + const preOpenIno = 2n ** 53n; + const injectIno = (stat: Stats | BigIntStats, ino: bigint): void => { + stat.ino = typeof stat.ino === 'bigint' ? ino : Number(ino); + }; + const lstatSpy = spyOnLstatForwarding((entry, stat) => { + if (String(entry) === realTarget) { + injectIno(stat, preOpenIno); + } + }); + const originalOpen = fs.open.bind(fs); + const openSpy = vi.spyOn(fs, 'open').mockImplementation((async ( + entry: Parameters[0], + flags?: Parameters[1], + mode?: Parameters[2], + ) => { + const handle = await originalOpen( + entry, + flags as Parameters[1], + mode, + ); + if (String(entry) !== realTarget) { + return handle; + } + const originalStat = handle.stat.bind(handle); + handle.stat = (async (options?: Parameters[0]) => { + const stat = await originalStat(options); + injectIno(stat, preOpenIno + 1n); + return stat; + }) as typeof handle.stat; + return handle; + }) as typeof fs.open); + + vi.useFakeTimers(); + vi.setSystemTime(new Date(Date.now() + 6_000)); + + try { + const artifact = await store.get(artifactId); + expect(artifact).toMatchObject({ id: artifactId, status: 'missing' }); + } finally { + lstatSpy.mockRestore(); + openSpy.mockRestore(); + vi.useRealTimers(); + } + }); + + it('keeps an unswapped file available through an options-forwarding lstat spy', async () => { + const store = new SessionArtifactStore({ + sessionId: 's7-bigint-forwarding', + workspaceCwd: workspace, + }); + await fs.writeFile(path.join(workspace, 'keep.txt'), 'content'); + const created = await store.upsertMany([ + { title: 'Keep', workspacePath: 'keep.txt' }, + ]); + const artifactId = created.changes[0]!.artifactId; + + vi.useFakeTimers(); + vi.setSystemTime(new Date(Date.now() + 6_000)); + const lstatSpy = spyOnLstatForwarding(() => {}); + + try { + const artifact = await store.get(artifactId); + expect(artifact).toMatchObject({ id: artifactId, status: 'available' }); + } finally { + lstatSpy.mockRestore(); + vi.useRealTimers(); + } + }); + it('rejects workspace paths that become symlinks before open', async () => { const store = new SessionArtifactStore({ sessionId: 's7-open-nofollow', diff --git a/packages/acp-bridge/src/sessionArtifacts.ts b/packages/acp-bridge/src/sessionArtifacts.ts index 5085bd965f..14f7585128 100644 --- a/packages/acp-bridge/src/sessionArtifacts.ts +++ b/packages/acp-bridge/src/sessionArtifacts.ts @@ -5,7 +5,12 @@ */ import { createHash } from 'node:crypto'; -import { constants as fsConstants, promises as fs, type Stats } from 'node:fs'; +import { + constants as fsConstants, + promises as fs, + type BigIntStats, + type Stats, +} from 'node:fs'; import type { FileHandle } from 'node:fs/promises'; import path from 'node:path'; import { @@ -3139,16 +3144,21 @@ async function getWorkspaceStatus( if (!relative || isOutsidePath(relative)) { return { status: 'missing', escaped: true }; } - const preOpenStat = await fs.lstat(realPath); + // Identity is compared on bigint stats: NTFS file ids are 64-bit and the + // Number spelling loses precision above 2^53, so two files created close + // together can round to the SAME numeric ino and defeat the swap check. + const preOpenStat = await fs.lstat(realPath, { bigint: true }); const handle = await fs.open( realPath, fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW, ); try { - const stat = await handle.stat(); - if (!isSameFile(preOpenStat, stat)) { + if (!isSameFile(preOpenStat, await handle.stat({ bigint: true }))) { return { status: 'missing', escaped: true }; } + // The identity fstat cannot be reused here: bigint stats return BigInt + // fields and truncate mtimeMs, breaking the Number comparisons below. + const stat = await handle.stat(); if (!stat.isFile()) { throw new Error('path is not a regular file'); } @@ -3219,7 +3229,7 @@ async function getWorkspaceStatus( } } -function isSameFile(before: Stats, after: Stats): boolean { +function isSameFile(before: BigIntStats, after: BigIntStats): boolean { return before.dev === after.dev && before.ino === after.ino; } diff --git a/packages/channels/dws/src/dws-event-stream.test.ts b/packages/channels/dws/src/dws-event-stream.test.ts index bd65299881..195b5ca7fe 100644 --- a/packages/channels/dws/src/dws-event-stream.test.ts +++ b/packages/channels/dws/src/dws-event-stream.test.ts @@ -81,6 +81,29 @@ describe('DWS event process', () => { }); }); + it('reports a clean exit instead of a stale stderr error', async () => { + const onLine = vi.fn(); + const onError = vi.fn(); + const fixture = fileURLToPath( + new URL('./fixtures/dws-event-clean-exit-source.mjs', import.meta.url), + ); + const subscription = await startDwsEventProcess( + process.execPath, + [fixture], + onLine, + onError, + ); + + await subscription.closed; + + expect(onLine).toHaveBeenCalledWith('{"type":"final"}'); + expect(onError).toHaveBeenCalledOnce(); + expect(onError.mock.calls[0]?.[0]).toMatchObject({ + message: 'DWS event consumer stopped (0).', + retryable: undefined, + }); + }); + it('preserves a terminal error across stdout buffered after exit', async () => { let releaseFirstLine!: () => void; const firstLineBlocked = new Promise((resolve) => { diff --git a/packages/channels/dws/src/dws-event-stream.ts b/packages/channels/dws/src/dws-event-stream.ts index 44e7c6d5ac..b730543767 100644 --- a/packages/channels/dws/src/dws-event-stream.ts +++ b/packages/channels/dws/src/dws-event-stream.ts @@ -216,7 +216,15 @@ export const startDwsEventProcess: DwsEventProcessStarter = ( if (state === 'pending') { settleStartupError(lastError ?? processError(code)); } else if (state === 'ready' && !stopping) { - reportError(lastError ?? processError(code)); + // A clean exit certifies any recorded stderr error as stale: the + // consumer chose to finish after writing it. The live-line clear in + // the stdout handler cannot be relied on for the final line — on + // Windows the pipe routinely drains only after `exitCode` is set, + // so its liveness gate (which protects errors from being cleared + // by lines buffered across a crash) also swallows that clear. + reportError( + code === 0 ? processError(code) : (lastError ?? processError(code)), + ); } }); }); diff --git a/packages/channels/dws/src/fixtures/dws-event-clean-exit-source.mjs b/packages/channels/dws/src/fixtures/dws-event-clean-exit-source.mjs new file mode 100644 index 0000000000..af4f32cb85 --- /dev/null +++ b/packages/channels/dws/src/fixtures/dws-event-clean-exit-source.mjs @@ -0,0 +1,15 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import process from 'node:process'; + +process.stderr.write('[event] ready\n'); +process.stdout.write('{"type":"final"}\n', () => { + process.stderr.write( + '{"message":"stale stderr error","retryable":false}\n', + () => process.exit(0), + ); +}); diff --git a/packages/cli/src/commands/review/lib/worktree.test.ts b/packages/cli/src/commands/review/lib/worktree.test.ts index 94441ad1fd..946020fc8f 100644 --- a/packages/cli/src/commands/review/lib/worktree.test.ts +++ b/packages/cli/src/commands/review/lib/worktree.test.ts @@ -39,6 +39,15 @@ import { worktreeResidue, } from './worktree.js'; +// Replaces a gitfile that `git worktree add` created. On Windows git marks +// the linked worktree's `.git` hidden, and opening a hidden file for truncate +// (writeFileSync's CREATE_ALWAYS) fails with EPERM — so unlink first and let +// the rewrite create a fresh, unhidden file. +function overwriteGitfile(gitfilePath: string, content: string): void { + rmSync(gitfilePath, { force: true }); + writeFileSync(gitfilePath, content); +} + describe('worktreeResidue', () => { let repo: string; // The tree under measurement is a LINKED worktree — the production shape: @@ -245,7 +254,7 @@ describe('worktreeResidue', () => { ]); gitRepo('config', 'core.worktree', tree); - writeFileSync(join(tree, '.git'), `gitdir: ${join(repo, '.git')}\n`); + overwriteGitfile(join(tree, '.git'), `gitdir: ${join(repo, '.git')}\n`); const got = worktreeResidue(tree); @@ -296,7 +305,7 @@ describe('worktreeResidue', () => { writeFileSync(join(admin, 'commondir'), '../..\n'); writeFileSync(join(admin, 'HEAD'), `${forgedHead}\n`); copyFileSync(join(repo, '.git', 'index'), join(admin, 'index')); - writeFileSync(join(tree, '.git'), `gitdir: ${admin}\n`); + overwriteGitfile(join(tree, '.git'), `gitdir: ${admin}\n`); // Unpinned, the forge's index answers clean — and an unanchored clean // verdict is exactly the one the probe refuses (#9557). @@ -333,7 +342,7 @@ describe('worktreeResidue', () => { const admin = readFileSync(join(sibling, '.git'), 'utf8') .trim() .replace(/^gitdir:\s*/, ''); - writeFileSync(join(tree, '.git'), `gitdir: ${admin}\n`); + overwriteGitfile(join(tree, '.git'), `gitdir: ${admin}\n`); const got = worktreeResidue(tree); @@ -468,7 +477,7 @@ describe('worktreeResidue', () => { // absolutely, because its old relative spelling no longer resolves from // outside the repo. renameSync(tree, join(outside, 'review-wt')); - writeFileSync( + overwriteGitfile( join(outside, 'review-wt', '.git'), `gitdir: ${join(repo, '.git', 'worktrees', 'review-wt')}\n`, ); diff --git a/packages/cli/src/serve/daemon-git-worktree-guard.test.ts b/packages/cli/src/serve/daemon-git-worktree-guard.test.ts index a21d3df023..9df0e137c8 100644 --- a/packages/cli/src/serve/daemon-git-worktree-guard.test.ts +++ b/packages/cli/src/serve/daemon-git-worktree-guard.test.ts @@ -39,6 +39,16 @@ const outsideRepo = path.join(temporaryRoot, 'outside', 'repo'); // A second outside checkout whose path contains no Git word, so a test using // it cannot pass because `\bgit\b` happened to match inside the path. const plainOutsidePath = path.join(temporaryRoot, 'elsewhere', 'checkout'); +// Spelling for paths interpolated into COMMAND STRINGS on the lanes that +// execute through bash: an unquoted backslash is a POSIX escape, so a native +// Windows path pasted into a command reaches the tokenizer as `C:temprepo` +// and the test stops exercising what it names — resolvable targets become +// unresolvable ones, and cwd shifts silently fail in place. Forward slashes +// survive bash and resolve identically on Windows. Filesystem setup and +// denial-reason assertions keep the native spelling: the guard canonicalizes +// targets through realpath before printing them. +const cmdPath = (value: string): string => + process.platform === 'win32' ? value.replaceAll('\\', '/') : value; mkdirSync(path.join(outsideRepo, '.git'), { recursive: true }); mkdirSync(insideNested, { recursive: true }); @@ -68,17 +78,19 @@ afterAll(async () => { describe('createDaemonToolGuard', () => { it.each([ - () => `git -C ${outsideRepo} reset --hard`, - () => `git -C${outsideRepo} checkout -- .`, + () => `git -C ${cmdPath(outsideRepo)} reset --hard`, + () => `git -C${cmdPath(outsideRepo)} checkout -- .`, () => - `git --work-tree=${outsideRepo} --git-dir=${path.join(outsideRepo, '.git')} clean -fd`, - () => `git --git-dir ${path.join(outsideRepo, '.git')} commit -m x`, - () => `git --namespace foo -C ${outsideRepo} reset --hard`, - () => `git --super-prefix=foo --work-tree=${outsideRepo} clean -fd`, + `git --work-tree=${cmdPath(outsideRepo)} --git-dir=${cmdPath(path.join(outsideRepo, '.git'))} clean -fd`, + () => + `git --git-dir ${cmdPath(path.join(outsideRepo, '.git'))} commit -m x`, + () => `git --namespace foo -C ${cmdPath(outsideRepo)} reset --hard`, + () => + `git --super-prefix=foo --work-tree=${cmdPath(outsideRepo)} clean -fd`, // `grep` runs the target repo's diff..textconv programs and // `status` refreshes the target index + runs its core.fsmonitor. - () => `git -C ${outsideRepo} grep --textconv pattern`, - () => `git -C ${outsideRepo} status --porcelain`, + () => `git -C ${cmdPath(outsideRepo)} grep --textconv pattern`, + () => `git -C ${cmdPath(outsideRepo)} status --porcelain`, ])('denies relocated mutating Git command %#', async (buildCommand) => { const guard = createDaemonToolGuard(); @@ -171,7 +183,7 @@ describe('createDaemonToolGuard', () => { it.each([ // Git treats an empty `-C` as a no-op and applies the next relocation. - () => `git -C "" -C ${outsideRepo} reset --hard`, + () => `git -C "" -C ${cmdPath(outsideRepo)} reset --hard`, ])('denies relocations masked by token edge cases %#', async (command) => { const guard = createDaemonToolGuard(); @@ -185,7 +197,7 @@ describe('createDaemonToolGuard', () => { // is ordinary argv text and the divergent-syntax gate owns the shape. it .runIf(bashSemanticsLane) - .each([() => `git -C ${outsideRepo} reset --hard # note`])( + .each([() => `git -C ${cmdPath(outsideRepo)} reset --hard # note`])( 'denies relocations masked by token edge cases on the bash lanes %#', async (command) => { const guard = createDaemonToolGuard(); @@ -412,7 +424,9 @@ describe('createDaemonToolGuard', () => { const guard = createDaemonToolGuard(); await expect( - guard(request(`cd ${effectiveCwd} && sh -c 'git reset --hard'`)), + guard( + request(`cd ${cmdPath(effectiveCwd)} && sh -c 'git reset --hard'`), + ), ).resolves.toEqual({ allowed: true }); }, ); @@ -491,17 +505,17 @@ describe('createDaemonToolGuard', () => { guard(request(`sh -c 'cd ${outsideRepo}'; git reset --hard`)), ).resolves.toEqual({ allowed: true }); await expect( - guard(request(`cd ${effectiveCwd} && git reset --hard`)), + guard(request(`cd ${cmdPath(effectiveCwd)} && git reset --hard`)), ).resolves.toEqual({ allowed: true }); }, ); it.runIf(bashSemanticsLane).each([ () => `git -C \\ -${outsideRepo} reset --hard`, +${cmdPath(outsideRepo)} reset --hard`, () => `g\\ -it -C ${outsideRepo} reset --hard`, +it -C ${cmdPath(outsideRepo)} reset --hard`, ])( 'joins backslash continuations before parsing %#', async (buildCommand) => { @@ -608,12 +622,12 @@ it -C ${outsideRepo} reset --hard`, const guard = createDaemonToolGuard(); await expect( - guard(request(`git -C nested -C ${outsideRepo} reset --hard`)), + guard(request(`git -C nested -C ${cmdPath(outsideRepo)} reset --hard`)), ).resolves.toMatchObject({ allowed: false }); await expect( guard( request( - `git -C ${outsideRepo} -C ${path.relative(outsideRepo, effectiveCwd)} reset --hard`, + `git -C ${cmdPath(outsideRepo)} -C ${cmdPath(path.relative(outsideRepo, effectiveCwd))} reset --hard`, ), ), ).resolves.toEqual({ allowed: true }); @@ -1546,11 +1560,12 @@ it -C ${outsideRepo} reset --hard`, // An unrecognized program word hides what runs, so a git mention only // survives while the shell is provably still inside the boundary. it.each([ - () => `cd ${outsideRepo} && nice git reset --hard`, - () => `cd ${outsideRepo} && ionice -c3 git reset --hard`, - () => `cd ${outsideRepo} && echo x | xargs -I{} git reset --hard`, - () => `cd ${outsideRepo} && find . -maxdepth 0 -exec git reset --hard ;`, - () => `cd ${outsideRepo} && stdbuf -o0 git reset --hard`, + () => `cd ${cmdPath(outsideRepo)} && nice git reset --hard`, + () => `cd ${cmdPath(outsideRepo)} && ionice -c3 git reset --hard`, + () => `cd ${cmdPath(outsideRepo)} && echo x | xargs -I{} git reset --hard`, + () => + `cd ${cmdPath(outsideRepo)} && find . -maxdepth 0 -exec git reset --hard ;`, + () => `cd ${cmdPath(outsideRepo)} && stdbuf -o0 git reset --hard`, () => 'cd - && nice git reset --hard', ])( 'denies an unrecognized program running Git after a cwd shift %#', @@ -1605,7 +1620,11 @@ it -C ${outsideRepo} reset --hard`, guard(request('export FOO=bar && git commit -m x')), ).resolves.toEqual({ allowed: true }); await expect( - guard(request(`export GIT_WORK_TREE=${insideNested} && git commit -m x`)), + guard( + request( + `export GIT_WORK_TREE=${cmdPath(insideNested)} && git commit -m x`, + ), + ), ).resolves.toEqual({ allowed: true }); }); @@ -1666,11 +1685,14 @@ it -C ${outsideRepo} reset --hard`, // The relocated read-only allowance covers subcommands that neither write // files nor run target-repository programs — flags can revoke both. it.each([ - () => `git -C ${outsideRepo} cat-file --textconv --path=f.txt HEAD:f.txt`, - () => `git -C ${outsideRepo} cat-file --filters --path=f.txt HEAD:f.txt`, - () => `git -C ${outsideRepo} rev-parse --output=${outsideRepo}/o.txt HEAD`, () => - `git -C ${outsideRepo} cat-file --output ${outsideRepo}/o.txt -p HEAD`, + `git -C ${cmdPath(outsideRepo)} cat-file --textconv --path=f.txt HEAD:f.txt`, + () => + `git -C ${cmdPath(outsideRepo)} cat-file --filters --path=f.txt HEAD:f.txt`, + () => + `git -C ${cmdPath(outsideRepo)} rev-parse --output=${cmdPath(outsideRepo)}/o.txt HEAD`, + () => + `git -C ${cmdPath(outsideRepo)} cat-file --output ${cmdPath(outsideRepo)}/o.txt -p HEAD`, ])( 'denies a relocated read-only subcommand carrying a disqualifying flag %#', async (buildCommand) => { @@ -1697,8 +1719,8 @@ it -C ${outsideRepo} reset --hard`, // `ls-files` executes the target repository's core.fsmonitor hook — the // same property that excluded `status` (measured on git 2.47.3). it.each([ - () => `git -C ${outsideRepo} ls-files`, - () => `git -C ${outsideRepo} ls-files --others`, + () => `git -C ${cmdPath(outsideRepo)} ls-files`, + () => `git -C ${cmdPath(outsideRepo)} ls-files --others`, ])('denies a relocated ls-files %#', async (buildCommand) => { const guard = createDaemonToolGuard(); @@ -1714,11 +1736,11 @@ it -C ${outsideRepo} reset --hard`, // subcommand stays out of the read-only set because the flag is one token // away from any describe a model writes. it.each([ - () => `git -C ${outsideRepo} describe`, - () => `git -C ${outsideRepo} describe --tags`, - () => `git -C ${outsideRepo} describe --dirty`, - () => `git -C ${outsideRepo} describe --always --dirty`, - () => `git -C ${outsideRepo} describe --broken`, + () => `git -C ${cmdPath(outsideRepo)} describe`, + () => `git -C ${cmdPath(outsideRepo)} describe --tags`, + () => `git -C ${cmdPath(outsideRepo)} describe --dirty`, + () => `git -C ${cmdPath(outsideRepo)} describe --always --dirty`, + () => `git -C ${cmdPath(outsideRepo)} describe --broken`, ])('denies a relocated describe %#', async (buildCommand) => { const guard = createDaemonToolGuard(); @@ -1752,7 +1774,7 @@ it -C ${outsideRepo} reset --hard`, () => `xargs -I{} sh -c 'cd ${outsideRepo} && git reset --hard'`, // `executableBaseName` lowercases, so an uppercase program word resolves // to the same binary on a case-insensitive filesystem. - () => `cd ${outsideRepo} && nice GIT reset --hard`, + () => `cd ${cmdPath(outsideRepo)} && nice GIT reset --hard`, ])( 'denies a relocated mutation concealed in an unrecognized program %#', async (buildCommand) => { @@ -1766,8 +1788,8 @@ it -C ${outsideRepo} reset --hard`, // A program word the daemon cannot read is as opaque as an unrecognized one. it.each([ - () => `cd ${outsideRepo} && $CMD git reset --hard`, - () => `cd ${outsideRepo} && command $CMD git reset --hard`, + () => `cd ${cmdPath(outsideRepo)} && $CMD git reset --hard`, + () => `cd ${cmdPath(outsideRepo)} && command $CMD git reset --hard`, ])( 'denies a dynamic program running Git after a cwd shift %#', async (buildCommand) => { @@ -1840,8 +1862,9 @@ it -C ${outsideRepo} reset --hard`, // An unmodelled value-taking global option makes its value look like the // subcommand, which ends option parsing and hides the relocation after it. it.each([ - () => `git --shallow-file /tmp/shallow -C ${outsideRepo} reset --hard`, - () => `git --attr-source HEAD -C ${outsideRepo} reset --hard`, + () => + `git --shallow-file /tmp/shallow -C ${cmdPath(outsideRepo)} reset --hard`, + () => `git --attr-source HEAD -C ${cmdPath(outsideRepo)} reset --hard`, ])( 'parses relocations after value-taking global options %#', async (buildCommand) => { @@ -2030,11 +2053,12 @@ it -C ${outsideRepo} reset --hard`, // Values assigned earlier in the same command are visible to the guard. it.each([ - () => `X='git reset --hard'; cd ${outsideRepo}; $X`, + () => `X='git reset --hard'; cd ${cmdPath(outsideRepo)}; $X`, () => `X=git; Y='-C ${outsideRepo} reset --hard'; $X $Y`, () => - `eval 'GIT_WORK_TREE=${outsideRepo}'; export GIT_WORK_TREE; git reset --hard`, - () => `GIT_WORK_TREE=${outsideRepo}; export $NAME; git reset --hard`, + `eval 'GIT_WORK_TREE=${cmdPath(outsideRepo)}'; export GIT_WORK_TREE; git reset --hard`, + () => + `GIT_WORK_TREE=${cmdPath(outsideRepo)}; export $NAME; git reset --hard`, ])('resolves a relocation through shell variables %#', async (build) => { const guard = createDaemonToolGuard(); @@ -2402,7 +2426,7 @@ it -C ${outsideRepo} reset --hard`, for (const command of [ "env -iS 'git status'", // A `cd` target the guard already knows the value of. - `d=${insideNested}; cd $d; git status`, + `d=${cmdPath(insideNested)}; cd $d; git status`, // `set +a` turns allexport back off. `set -a; set +a; GIT_WORK_TREE=${outsideRepo}; echo done`, // Definitions used inside the boundary stay allowed. @@ -2431,9 +2455,10 @@ it -C ${outsideRepo} reset --hard`, () => `echo "$'$(GIT_DIR=${plainOutside}/.git git reset --hard HEAD~1)'"`, // The export attribute sticks to the name, so a LATER assignment to it // reaches the git subprocess. - () => `export GIT_DIR; GIT_DIR=${plainOutside}; git reset --hard`, () => - `export GIT_WORK_TREE; GIT_WORK_TREE=${plainOutside}; git reset --hard`, + `export GIT_DIR; GIT_DIR=${cmdPath(plainOutside)}; git reset --hard`, + () => + `export GIT_WORK_TREE; GIT_WORK_TREE=${cmdPath(plainOutside)}; git reset --hard`, // Both sides of a pipe run in subshells: the parent stays outside. () => `cd ${plainOutside}; echo x | cd ${effectiveCwd}; git commit -m x`, // A bare digit before a spaced redirect is a real argv word. diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 2e895300e8..f60d8ac862 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -1655,7 +1655,9 @@ describe('Server Config (config.ts)', () => { }); expect(child.getCwd()).toBe('/tmp/derived'); - expect(parent.getCwd()).toBe(baseParams.targetDir); + // The constructor resolves targetDir, so on win32 the POSIX fixture + // spelling comes back drive-qualified — compare the resolved form. + expect(parent.getCwd()).toBe(path.resolve(baseParams.targetDir)); expect(Object.getPrototypeOf(child)).toBe(parent); }); diff --git a/packages/core/src/extension/extension-git-client.ts b/packages/core/src/extension/extension-git-client.ts index 6cf31f537a..eef66b1b4c 100644 --- a/packages/core/src/extension/extension-git-client.ts +++ b/packages/core/src/extension/extension-git-client.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as os from 'node:os'; import type { SimpleGit, SimpleGitFactory, SimpleGitOptions } from 'simple-git'; import type { ExtensionInstallMetadata } from '../config/config.js'; import type { GitCredential } from './extension-git-credentials.js'; @@ -126,7 +125,11 @@ export function createExtensionGitClient( const environment: Record = { GIT_CONFIG_NOSYSTEM: '1', - GIT_CONFIG_GLOBAL: os.devNull, + // The literal '/dev/null' on every platform, NOT os.devNull: Git for + // Windows special-cases the POSIX spelling in its compat layer, while + // os.devNull's win32 value ('\\\\.\\nul') is rejected as + // "fatal: unable to access '\\.\nul': Invalid argument". + GIT_CONFIG_GLOBAL: '/dev/null', }; for (const key of [ 'PATH', diff --git a/packages/node-repl/src/node-repl.semantics.test.ts b/packages/node-repl/src/node-repl.semantics.test.ts index 2b414bb094..3c4ecbb151 100644 --- a/packages/node-repl/src/node-repl.semantics.test.ts +++ b/packages/node-repl/src/node-repl.semantics.test.ts @@ -370,8 +370,22 @@ describe('binding semantics through the real kernel', () => { expect(r.status).toBe('ok'); expect(textOf(r.events).trim()).toBe('via-symlink'); } finally { - fs.rmSync(store, { recursive: true, force: true }); - fs.rmSync(work, { recursive: true, force: true }); + // The kernel child's cwd is `work`; on Windows a directory that is any + // process's cwd cannot be removed (EBUSY). Dispose first, then retry + // the removal across the child's asynchronous teardown. + manager.dispose(); + fs.rmSync(store, { + recursive: true, + force: true, + maxRetries: 10, + retryDelay: 200, + }); + fs.rmSync(work, { + recursive: true, + force: true, + maxRetries: 10, + retryDelay: 200, + }); } }); diff --git a/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.test.ts b/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.test.ts index a47ec7383c..921d51d5d6 100644 --- a/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.test.ts +++ b/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.test.ts @@ -59,6 +59,13 @@ vi.mock('node:url', async () => { if (process.platform !== 'win32' && /^[a-zA-Z]:\\/.test(filePath)) { return actual.pathToFileURL(filePath, { windows: true }); } + // The mirror case: fixtures spell workspace paths POSIX-style, and on + // win32 the real pathToFileURL would drive-qualify them against the + // process cwd (file:///C:/workspace/…). Parse them as POSIX so the + // expected URLs read the same on every host. + if (process.platform === 'win32' && filePath.startsWith('/')) { + return actual.pathToFileURL(filePath, { windows: false }); + } return actual.pathToFileURL(filePath); }, };