fix: Windows-native CI findings on latest master
Local run of check:ci:windows-complete (the windows-native gate) on latest master surfaced five Windows-only failures, all unreachable by current CI because the native windows job is disabled and the wine gate only covers build+site. - install-lefthook/translation-pairing-merge specs junctioned the real scripts/ and tsx package into fixtures; Windows recursive deletion (Node rmSync and git worktree remove) follows MOUNT_POINT junctions and deleted the repository's own directories mid-run. Fixtures now unlink their reparse points before any recursive removal (shared helper in scripts/test-fixture-cleanup.ts). - workflow-workerthread spawned its worker with an empty env; on Windows os.tmpdir() then degrades to the literal relative path undefined\temp, so tsx wrote its transform cache into a cwd-relative undefined/ directory inside the repo. The worker env now injects the host temp path on win32 (workerSpawnEnv, platform-parameterized and unit-tested on both arms). - workspace-context spec did not stub USERPROFILE (win32 homedir) or a set DSH_HOME, leaking the developer machine's real ~/.dsh/AGENTS.md into discovery. - ui-trajectory client-bundle spec mounted the built artifact without the remote/settingsScope provides the locale plugin needs, so the plugin never activated and no view registered. - subagent temp-fixture cleanup lacked the maxRetries Windows handle release needs under load (EPERM); added retries to the three affected specs and the fixture-cleanup helper.
This commit is contained in:
@@ -578,10 +578,12 @@ describe('workspace context instruction discovery', () => {
|
||||
const root = await tempRepo()
|
||||
const emptyHome = await tempRepo()
|
||||
// Isolate the default-home fallback: blank DSH_HOME is treated as unset, and
|
||||
// HOME points at an empty dir so the default ~/.dsh holds no global scope.
|
||||
// Symlinks are followed, so a real ~/.dsh/AGENTS.md would otherwise leak in.
|
||||
// the home dirs point at an empty dir so the default ~/.dsh holds no global
|
||||
// scope. Windows homedir() reads USERPROFILE (not HOME), so both must be
|
||||
// stubbed or a real ~/.dsh/AGENTS.md would otherwise leak in.
|
||||
vi.stubEnv('DSH_HOME', '')
|
||||
vi.stubEnv('HOME', emptyHome)
|
||||
if (process.platform === 'win32') vi.stubEnv('USERPROFILE', emptyHome)
|
||||
try {
|
||||
const cwd = join(root, 'child')
|
||||
await mkdir(cwd, { recursive: true })
|
||||
@@ -622,6 +624,8 @@ describe('workspace context instruction discovery', () => {
|
||||
try {
|
||||
await write(join(home, '.dsh/AGENTS.md'), 'global default rule')
|
||||
|
||||
// A set DSH_HOME would override the homedir default and relabel the home.
|
||||
vi.stubEnv('DSH_HOME', '')
|
||||
vi.resetModules()
|
||||
vi.doMock('node:os', () => ({ homedir: () => home }))
|
||||
const isolated = await import('@deepseek-ai/dsh-agent-instructions')
|
||||
@@ -629,6 +633,7 @@ describe('workspace context instruction discovery', () => {
|
||||
|
||||
expect(files.map(file => file.displayPath)).toEqual(['~/.dsh/AGENTS.md'])
|
||||
} finally {
|
||||
vi.unstubAllEnvs()
|
||||
vi.doUnmock('node:os')
|
||||
vi.resetModules()
|
||||
await rm(root, { recursive: true, force: true })
|
||||
|
||||
@@ -53,7 +53,7 @@ class GatedAdapter extends LlmAdapter {
|
||||
|
||||
const roots: string[] = []
|
||||
afterEach(() => {
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true })
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 })
|
||||
})
|
||||
|
||||
/** Boot the full continuable stack: loop, persistence, providers, and subagents. */
|
||||
|
||||
@@ -28,7 +28,7 @@ type Script = ConstructorParameters<typeof MockAdapter>[0]
|
||||
|
||||
const roots: string[] = []
|
||||
afterEach(() => {
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true })
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 })
|
||||
})
|
||||
|
||||
/** Boot the continuable stack with real JSONL session persistence. */
|
||||
|
||||
@@ -47,7 +47,7 @@ const testToolSignal = new AbortController().signal
|
||||
|
||||
const roots: string[] = []
|
||||
afterEach(() => {
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true })
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 })
|
||||
})
|
||||
|
||||
async function setupWith(adapter: MockAdapter | GatedAdapter) {
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
* @module @deepseek-ai/dsh-workflow-worker-thread/host
|
||||
*/
|
||||
|
||||
import { tmpdir } from 'node:os'
|
||||
import { Worker } from 'node:worker_threads'
|
||||
import type { WorkerOptions } from 'node:worker_threads'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
@@ -28,18 +29,42 @@ interface ChildRecord {
|
||||
disposal?: Promise<void>
|
||||
}
|
||||
|
||||
/**
|
||||
* The scrubbed worker environment: no ambient credentials, no loader flags.
|
||||
* Windows derives `os.tmpdir()` from `TMP`/`TEMP` and falls back to the
|
||||
* literal relative path `undefined\temp` when the environment is empty, so
|
||||
* tsx's transform cache would land in a cwd-relative `undefined/temp`
|
||||
* directory; the host's real temp path (not a credential) is injected there.
|
||||
* The unbuilt shape additionally forwards `TSX_TSCONFIG_PATH` for path
|
||||
* resolution.
|
||||
* @param platform - host platform; overridable so tests exercise both peer arms.
|
||||
* @returns the scrubbed worker environment object.
|
||||
*/
|
||||
export function workerSpawnEnv(platform: NodeJS.Platform = process.platform): NodeJS.ProcessEnv {
|
||||
const env: NodeJS.ProcessEnv = {}
|
||||
if (platform === 'win32') {
|
||||
const tmp = tmpdir()
|
||||
env.TMP = tmp
|
||||
env.TEMP = tmp
|
||||
}
|
||||
if (process.env.TSX_TSCONFIG_PATH !== undefined) {
|
||||
env.TSX_TSCONFIG_PATH = process.env.TSX_TSCONFIG_PATH
|
||||
}
|
||||
return env
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a built worker bundle or an unbuilt bootstrap that installs both tsx
|
||||
* transforms inside the worker. Both shapes clear `execArgv` and the ambient
|
||||
* environment; the unbuilt shape forwards only `TSX_TSCONFIG_PATH` for path
|
||||
* resolution.
|
||||
* environment (the worker only sees the platform temp path and, unbuilt,
|
||||
* `TSX_TSCONFIG_PATH`).
|
||||
* @param init - the run payload, passed as `workerData`.
|
||||
* @returns the entry path or URL and the Worker options to spawn it with.
|
||||
*/
|
||||
function resolveWorkerSpawn(init: WorkerInit): { entry: string | URL; options: WorkerOptions } {
|
||||
/* v8 ignore next 3 -- the built-output arm: tests always run unbuilt (src/); the built-worker e2e exercises this shape for real */
|
||||
if (!import.meta.url.endsWith('.ts')) {
|
||||
return { entry: fileURLToPath(new URL('./worker.cjs', import.meta.url)), options: { workerData: init, env: {}, execArgv: [] } }
|
||||
return { entry: fileURLToPath(new URL('./worker.cjs', import.meta.url)), options: { workerData: init, env: workerSpawnEnv(), execArgv: [] } }
|
||||
}
|
||||
// Resolve tsx only for unbuilt consumers and install it before importing TS.
|
||||
const workerEntry = new URL('./worker.ts', import.meta.url)
|
||||
@@ -56,7 +81,7 @@ function resolveWorkerSpawn(init: WorkerInit): { entry: string | URL; options: W
|
||||
entry: new URL(`data:text/javascript,${encodeURIComponent(bootstrap)}`),
|
||||
options: {
|
||||
workerData: init,
|
||||
env: process.env.TSX_TSCONFIG_PATH === undefined ? {} : { TSX_TSCONFIG_PATH: process.env.TSX_TSCONFIG_PATH },
|
||||
env: workerSpawnEnv(),
|
||||
execArgv: [],
|
||||
},
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import type { Worker } from 'node:worker_threads'
|
||||
import { Context } from '@deepseek-ai/cordis'
|
||||
@@ -9,6 +10,7 @@ import type { SubagentCapabilities, SubagentProvider, SubagentResult, SubagentRu
|
||||
import type { WorkflowMeta, WorkflowResult, WorkflowResultInfo, WorkflowRun, WorkflowRunInfo } from '@deepseek-ai/dsh-workflow'
|
||||
import * as workerEngineModule from '../src/index.ts'
|
||||
import WorkerThreadWorkflowEngine, { type Config } from '../src/index.ts'
|
||||
import { workerSpawnEnv } from '../src/host.ts'
|
||||
import { HostToWorkerType, WorkerToHostType } from '../src/protocol.ts'
|
||||
import { SessionId } from '@deepseek-ai/dsh-session'
|
||||
|
||||
@@ -559,24 +561,49 @@ describe('dsh-workflow-worker-thread', () => {
|
||||
expect(result.value).toBe('fine')
|
||||
})
|
||||
|
||||
it('the worker spawns with an EMPTY environment: an escaped script finds no ambient credentials', async () => {
|
||||
it('the worker spawns with a scrubbed environment: an escaped script finds no ambient credentials', async () => {
|
||||
const { ctx, parent } = await setup()
|
||||
// A canary in the HARNESS process's env: with an inherited environment
|
||||
// the escape below would read it back (exactly how DEEPSEEK_API_KEY
|
||||
// would leak); env: {} in the spawn options is what keeps it out.
|
||||
// would leak); the worker env keeps every ambient variable out. Windows
|
||||
// additionally receives the host temp path (TMP/TEMP) so `os.tmpdir()`
|
||||
// inside the worker resolves instead of degrading to a cwd-relative
|
||||
// `undefined\temp` (tsx writes its transform cache there).
|
||||
process.env.WORKFLOW_ENV_CANARY = 'leak me'
|
||||
try {
|
||||
const result = await run(ctx, parent, scripted(`
|
||||
const proc = ${ESCAPE}
|
||||
return { canary: proc.env.WORKFLOW_ENV_CANARY ?? null, keys: Object.keys(proc.env).length }
|
||||
return { canary: proc.env.WORKFLOW_ENV_CANARY ?? null, keys: Object.keys(proc.env).sort() }
|
||||
`))
|
||||
expect(result.stopReason).toBe('completed')
|
||||
expect(result.value).toEqual({ canary: null, keys: 0 })
|
||||
const expectedKeys = process.platform === 'win32' ? ['TEMP', 'TMP'] : []
|
||||
expect(result.value).toEqual({ canary: null, keys: expectedKeys })
|
||||
} finally {
|
||||
delete process.env.WORKFLOW_ENV_CANARY
|
||||
}
|
||||
})
|
||||
|
||||
it('workerSpawnEnv injects the host temp path on win32 and leaves the POSIX peer empty', () => {
|
||||
const tmp = tmpdir()
|
||||
expect(workerSpawnEnv('win32')).toEqual({ TMP: tmp, TEMP: tmp })
|
||||
expect(workerSpawnEnv('linux')).toEqual({})
|
||||
})
|
||||
|
||||
it('workerSpawnEnv forwards TSX_TSCONFIG_PATH when the snapshot harness pins it', () => {
|
||||
const tsconfig = fileURLToPath(new URL('../../../../tsconfig.json', import.meta.url))
|
||||
vi.stubEnv('TSX_TSCONFIG_PATH', tsconfig)
|
||||
try {
|
||||
expect(workerSpawnEnv('linux')).toEqual({ TSX_TSCONFIG_PATH: tsconfig })
|
||||
expect(workerSpawnEnv('win32')).toEqual({
|
||||
TMP: tmpdir(),
|
||||
TEMP: tmpdir(),
|
||||
TSX_TSCONFIG_PATH: tsconfig,
|
||||
})
|
||||
} finally {
|
||||
vi.unstubAllEnvs()
|
||||
}
|
||||
})
|
||||
|
||||
it('the unbuilt worker forwards exactly TSX_TSCONFIG_PATH through the scrub: the paths-map pin survives, secrets do not', async () => {
|
||||
const { ctx, parent } = await setup()
|
||||
// The ACP snapshot harness runs the parent with its cwd OUTSIDE the
|
||||
@@ -589,10 +616,13 @@ describe('dsh-workflow-worker-thread', () => {
|
||||
try {
|
||||
const result = await run(ctx, parent, scripted(`
|
||||
const proc = ${ESCAPE}
|
||||
return { keys: Object.keys(proc.env), tsconfig: proc.env.TSX_TSCONFIG_PATH }
|
||||
return { keys: Object.keys(proc.env).sort(), tsconfig: proc.env.TSX_TSCONFIG_PATH }
|
||||
`))
|
||||
expect(result.stopReason).toBe('completed')
|
||||
expect(result.value).toEqual({ keys: ['TSX_TSCONFIG_PATH'], tsconfig })
|
||||
const expectedKeys = process.platform === 'win32'
|
||||
? ['TEMP', 'TMP', 'TSX_TSCONFIG_PATH']
|
||||
: ['TSX_TSCONFIG_PATH']
|
||||
expect(result.value).toEqual({ keys: expectedKeys, tsconfig })
|
||||
} finally {
|
||||
delete process.env.TSX_TSCONFIG_PATH
|
||||
delete process.env.WORKFLOW_ENV_CANARY
|
||||
|
||||
@@ -16,6 +16,7 @@ import { tmpdir } from 'node:os'
|
||||
import { dirname, isAbsolute, join, resolve } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { removeFixtureSafely, unlinkFixtureLinks } from './test-fixture-cleanup.ts'
|
||||
|
||||
const installer = fileURLToPath(new URL('./install-lefthook.mjs', import.meta.url))
|
||||
const pairingMergeDriver = 'scripts/merge-translation-pairing-driver.sh %O %A %B %P'
|
||||
@@ -40,7 +41,7 @@ interface CommandResult {
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const fixture of fixtures.splice(0)) rmSync(fixture, { recursive: true, force: true })
|
||||
for (const fixture of fixtures.splice(0)) removeFixtureSafely(fixture)
|
||||
})
|
||||
|
||||
function commandResult(command: string, args: string[], cwd: string, env: NodeJS.ProcessEnv): CommandResult {
|
||||
@@ -282,6 +283,10 @@ describe('worktree-local Lefthook installer', { timeout: 15_000 }, () => {
|
||||
expect(gitResult(fixture, fixture.main, ['config', '--file', commonConfig, '--get', 'core.bare']).status).toBe(1)
|
||||
|
||||
const mainHookBeforeRemoval = readFileSync(join(mainHooks, 'pre-commit'), 'utf8')
|
||||
// Windows Git follows the fixture's MOUNT_POINT junctions into their real
|
||||
// targets while removing a worktree; unlink them first so the removal
|
||||
// cannot delete the repository's scripts/ or tsx package.
|
||||
unlinkFixtureLinks(fixture.linked)
|
||||
git(fixture, fixture.main, ['worktree', 'remove', '--force', fixture.linked])
|
||||
expect(readFileSync(join(mainHooks, 'pre-commit'), 'utf8')).toBe(mainHookBeforeRemoval)
|
||||
expect(readFileSync(legacyHook, 'utf8')).toBe('#!/bin/sh\n# legacy hook\n')
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
/**
|
||||
* Junction-safe fixture cleanup for Windows. Test fixtures junction the REAL
|
||||
* `scripts/`, `node_modules`, and tsx package directories so installer probes
|
||||
* resolve through them; Windows recursive deletion — both Node's `rmSync` and
|
||||
* Git's `worktree remove` — follows MOUNT_POINT junctions into their targets
|
||||
* and would delete the repository's own directories. POSIX `unlink`/`rm`
|
||||
* already remove symlinks without following them, so the walk is a no-op
|
||||
* there.
|
||||
*/
|
||||
|
||||
import { lstatSync, readdirSync, rmSync, unlinkSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
|
||||
/**
|
||||
* Recursively unlink every symbolic link (junction) under `path`.
|
||||
* @param path - the fixture tree whose reparse points are unlinked.
|
||||
*/
|
||||
export function unlinkFixtureLinks(path: string): void {
|
||||
const visit = (entry: string): void => {
|
||||
let stat: ReturnType<typeof lstatSync>
|
||||
try {
|
||||
stat = lstatSync(entry)
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return
|
||||
throw error
|
||||
}
|
||||
if (stat.isSymbolicLink() || !stat.isDirectory()) {
|
||||
if (stat.isSymbolicLink()) unlinkSync(entry)
|
||||
return
|
||||
}
|
||||
for (const child of readdirSync(entry)) visit(join(entry, child))
|
||||
}
|
||||
visit(path)
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove one fixture tree after its junctions are unlinked (see
|
||||
* {@link unlinkFixtureLinks}). Retries the removal: Windows releases child
|
||||
* process and antivirus file handles asynchronously, and an unretried
|
||||
* `rmSync` fails immediately with EPERM under load.
|
||||
* @param path - the fixture tree to remove.
|
||||
*/
|
||||
export function removeFixtureSafely(path: string): void {
|
||||
unlinkFixtureLinks(path)
|
||||
rmSync(path, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 })
|
||||
}
|
||||
@@ -1,7 +1,14 @@
|
||||
/** Integration coverage for automatic and explicit pairing-record conflict resolution. */
|
||||
|
||||
import { execFileSync, spawnSync } from 'node:child_process'
|
||||
import { chmodSync, mkdtempSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
|
||||
import {
|
||||
chmodSync,
|
||||
mkdtempSync,
|
||||
mkdirSync,
|
||||
readFileSync,
|
||||
symlinkSync,
|
||||
writeFileSync,
|
||||
} from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { delimiter, dirname, join } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
@@ -15,6 +22,7 @@ import {
|
||||
renderTranslationPairingRecord,
|
||||
translationPairPaths,
|
||||
} from './translation-pairing-record.ts'
|
||||
import { removeFixtureSafely } from './test-fixture-cleanup.ts'
|
||||
|
||||
const driver = fileURLToPath(new URL('./merge-translation-pairing.ts', import.meta.url))
|
||||
const driverLauncher = fileURLToPath(new URL('./merge-translation-pairing-driver.sh', import.meta.url))
|
||||
@@ -28,7 +36,7 @@ interface Fixture {
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const fixture of fixtures.splice(0)) rmSync(fixture, { recursive: true, force: true })
|
||||
for (const fixture of fixtures.splice(0)) removeFixtureSafely(fixture)
|
||||
})
|
||||
|
||||
function git(fixture: Fixture, args: string[]): string {
|
||||
|
||||
Reference in New Issue
Block a user