Skip to content

Commit 64a944d

Browse files
committed
fix: make Windows CI paths and broker handling portable
1 parent 73b4151 commit 64a944d

11 files changed

Lines changed: 129 additions & 51 deletions

scripts/lib/fs.mjs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,14 +189,13 @@ export async function withFileLock(lockPath, operation, options = {}) {
189189

190190
/** @param {string} directory */
191191
async function syncDirectory(directory) {
192-
let handle;
192+
const handle = await open(directory, 'r');
193193
try {
194-
handle = await open(directory, 'r');
195194
await handle.sync();
196195
} catch (error) {
197-
if (!isNodeError(error, 'EINVAL') && !isNodeError(error, 'ENOTSUP')) throw error;
196+
if (!isNodeError(error, 'EINVAL') && !isNodeError(error, 'ENOTSUP') && !isNodeError(error, 'EPERM')) throw error;
198197
} finally {
199-
if (handle) await handle.close();
198+
await handle.close();
200199
}
201200
}
202201

scripts/lib/review.mjs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,10 @@ async function secureArtifactRoot(storageDirectory, directory, create) {
147147
}
148148
/** @param {string} path */
149149
async function defaultSyncDirectory(path) {
150-
/** @type {import('node:fs/promises').FileHandle|undefined} */ let handle;
151-
try { handle = await open(path, 'r'); await handle.sync(); }
152-
catch (error) { if (!['EINVAL', 'ENOTSUP'].includes(errorCode(error) ?? '')) throw error; }
153-
finally { await handle?.close(); }
150+
const handle = await open(path, 'r');
151+
try { await handle.sync(); }
152+
catch (error) { if (!['EINVAL', 'ENOTSUP', 'EPERM'].includes(errorCode(error) ?? '')) throw error; }
153+
finally { await handle.close(); }
154154
}
155155

156156
/** @param {any} snapshot @param {string} command @param {{beforeMessageIds?:Set<string>,inputId?:string,stateRevision?:number}} [turnBoundary] */

scripts/zcode-broker.mjs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ export async function ensureZCodeBroker(options) {
118118
}
119119

120120
export class ZCodeBroker {
121-
/** @param {{endpoint:string,brokerToken:string,launch:{command:string,args:string[],target?:string},workspace:string,env?:NodeJS.ProcessEnv,idleTimeoutMs?:number,maxFrameBytes?:number,maxOutboundBytes?:number,drainTimeoutMs?:number,instanceId?:string}} options */
122-
constructor(options) { if (typeof options?.brokerToken !== 'string' || options.brokerToken.length < 32 || !validWireOption(options?.maxFrameBytes, 16 * 1024 * 1024) || !validWireOption(options?.maxOutboundBytes, 64 * 1024 * 1024) || !validDrainOption(options?.drainTimeoutMs)) throw brokerInputError(); this.options = options; this.ownershipPath = options.ownershipPath ?? `${options.endpoint}.owners.json`; this.ownershipStoreEstablished = false; this.server = null; this.protocol = null; this.protocolPromise = null; this.sockets = new Set(); this.socketWriters = new WeakMap(); this.authenticated = new WeakSet(); this.socketOwnerIds = new WeakMap(); this.sessionOwners = new Map(); this.permissionPending = new Map(); this.localTasks = new Set(); this.nextPermissionId = 1_000_000_000; this.owners = 0; this.activeSessions = new Set(); this.fastIdleRequested = false; this.idleTimer = null; this.closing = false; this.closePromise = null; }
121+
/** @param {{endpoint:string,ownershipPath?:string,brokerToken:string,launch:{command:string,args:string[],target?:string},workspace:string,env?:NodeJS.ProcessEnv,idleTimeoutMs?:number,maxFrameBytes?:number,maxOutboundBytes?:number,drainTimeoutMs?:number,instanceId?:string}} options */
122+
constructor(options) { if (typeof options?.brokerToken !== 'string' || options.brokerToken.length < 32 || !validWireOption(options?.maxFrameBytes, 16 * 1024 * 1024) || !validWireOption(options?.maxOutboundBytes, 64 * 1024 * 1024) || !validDrainOption(options?.drainTimeoutMs) || isWindowsNamedPipe(options?.endpoint) && (typeof options?.ownershipPath !== 'string' || !options.ownershipPath)) throw brokerInputError(); this.options = options; this.ownershipPath = options.ownershipPath ?? `${options.endpoint}.owners.json`; this.ownershipStoreEstablished = false; this.server = null; this.protocol = null; this.protocolPromise = null; this.sockets = new Set(); this.socketWriters = new WeakMap(); this.authenticated = new WeakSet(); this.socketOwnerIds = new WeakMap(); this.sessionOwners = new Map(); this.permissionPending = new Map(); this.localTasks = new Set(); this.nextPermissionId = 1_000_000_000; this.owners = 0; this.activeSessions = new Set(); this.fastIdleRequested = false; this.idleTimer = null; this.closing = false; this.closePromise = null; }
123123

124124
async start() {
125125
if (this.server) return this;
@@ -309,6 +309,7 @@ export class ZCodeBroker {
309309
function writeLocal(socket, value) { if (!socket.writable) return; try { socket.zcodeWriter?.write(`${JSON.stringify(value)}\n`); } catch { socket.destroy(); } }
310310
function validWireOption(value, maximum) { return value === undefined || Number.isSafeInteger(value) && value >= 128 && value <= maximum; }
311311
function validDrainOption(value) { return value === undefined || Number.isSafeInteger(value) && value >= 1 && value <= MAX_DRAIN_TIMEOUT_MS; }
312+
function isWindowsNamedPipe(endpoint) { return typeof endpoint === 'string' && endpoint.toLowerCase().startsWith('\\\\.\\pipe\\'); }
312313
function isProcessAlive(pid) { try { process.kill(pid, 0); return true; } catch { return false; } }
313314
function safeTokenEqual(left, right) { const a = Buffer.from(left); const b = Buffer.from(right); return a.length === b.length && timingSafeEqual(a, b); }
314315
function offeredDeny(request) { return request.options?.find((option) => option.response?.decision === 'deny')?.response ?? { decision: 'deny' }; }

tests/background-worker.test.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ test('background startup timeout terminates and reaps the unacknowledged worker'
2626
test('background startup schedules the production acknowledgement deadline at 30 seconds', async (t) => {
2727
const directory = await mkdtemp(join(tmpdir(), 'zcode-background-worker-default-'));
2828
const worker = join(directory, 'worker.mjs'); let scheduled;
29-
t.after(() => rm(directory, { force: true, recursive: true }));
29+
t.after(async () => { await new Promise((resolvePromise) => setTimeout(resolvePromise, 80)); await rm(directory, { force: true, recursive: true }); });
3030
await writeFile(worker, "import { writeSync } from 'node:fs'; writeSync(4, 'ready\\n'); setTimeout(() => {}, 20);\n");
3131
const result = await startBackgroundWorker({ companionPath: worker, jobId: 'b'.repeat(64), executionCapability: 'private-capability', cwd: directory, env: process.env,
3232
dependencies: { setTimeout: (callback, milliseconds) => { scheduled = milliseconds; return globalThis.setTimeout(callback, milliseconds); }, clearTimeout: (timer) => globalThis.clearTimeout(timer) } });

tests/codex-app-server.test.mjs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,10 @@ test('terminates the child on success, JSON-RPC error, malformed output, oversiz
6161
const directory = await mkdtemp(join(tmpdir(), 'codex-app-lifecycle-')); const record = join(directory, 'record.jsonl'); await writeFile(record, '');
6262
const promise = readCodexThread('thread-1', { executable: process.execPath, args: [fake], env: { ...process.env, FAKE_CODEX_RECORD: record, FAKE_CODEX_THREAD_JSON: JSON.stringify(validThread), ...env }, timeoutMs: 1_000, ...options });
6363
if (code) await assert.rejects(promise, { code }); else await promise;
64-
for (let index = 0; index < 50 && !(await readFile(record, 'utf8')).includes('lifecycle'); index += 1) await new Promise((resolve) => setTimeout(resolve, 5));
65-
assert.match(await readFile(record, 'utf8'), /"lifecycle":"SIGTERM"/);
64+
if (process.platform !== 'win32') {
65+
for (let index = 0; index < 50 && !(await readFile(record, 'utf8')).includes('lifecycle'); index += 1) await new Promise((resolve) => setTimeout(resolve, 5));
66+
assert.match(await readFile(record, 'utf8'), /"lifecycle":"SIGTERM"/);
67+
}
6668
});
6769
});
6870

@@ -76,8 +78,10 @@ test('bounds and redacts stderr diagnostics without blocking', async () => {
7678
test('deep expected responses fail as controlled protocol errors and terminate the real child', async () => {
7779
const directory = await mkdtemp(join(tmpdir(), 'codex-app-deep-response-')); const record = join(directory, 'record.jsonl'); await writeFile(record, '');
7880
await assert.rejects(readCodexThread('thread-1', { executable: process.execPath, args: [fake], env: { ...process.env, FAKE_CODEX_RECORD: record, FAKE_CODEX_DEEP_RESPONSE_DEPTH: '10000' }, timeoutMs: 1_000 }), { code: 'CODEX_APP_SERVER_MALFORMED' });
79-
for (let index = 0; index < 50 && !(await readFile(record, 'utf8')).includes('lifecycle'); index += 1) await new Promise((resolve) => setTimeout(resolve, 5));
80-
assert.match(await readFile(record, 'utf8'), /"lifecycle":"SIGTERM"/);
81+
if (process.platform !== 'win32') {
82+
for (let index = 0; index < 50 && !(await readFile(record, 'utf8')).includes('lifecycle'); index += 1) await new Promise((resolve) => setTimeout(resolve, 5));
83+
assert.match(await readFile(record, 'utf8'), /"lifecycle":"SIGTERM"/);
84+
}
8185
});
8286

8387
test('deep unrelated notifications are ignored without preventing a valid response', async () => {

tests/hooks.test.mjs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
import assert from 'node:assert/strict';
33
import { mkdtemp, readFile, writeFile, mkdir, readdir, symlink, unlink } from 'node:fs/promises';
44
import { tmpdir } from 'node:os';
5-
import { join } from 'node:path';
5+
import { join, sep } from 'node:path';
66
import { spawn } from 'node:child_process';
7+
import { fileURLToPath } from 'node:url';
78
import test from 'node:test';
89
import { resolveWorkspaceStorage } from '../scripts/lib/workspace.mjs';
910
import { createStateStore } from '../scripts/lib/state.mjs';
@@ -12,11 +13,13 @@ import { createManagedZCodeClient, createZCodeClient, releaseManagedZCodeOwner }
1213
import { ownerIdForSession } from '../scripts/lib/job-control.mjs';
1314
import { brokerEndpointFor, ensureZCodeBroker, prioritizeBrokerOwnership, probeBrokerHealth, reconcileBrokerOwnership, writeBrokerIdentity } from '../scripts/zcode-broker.mjs';
1415

15-
const root = new URL('../', import.meta.url).pathname;
16+
const root = fileURLToPath(new URL('../', import.meta.url));
1617
const fakeZCode = join(root, 'tests/fixtures/fake-zcode-cli.mjs');
1718
const legacyBroker = join(root, 'tests/fixtures/legacy-zcode-broker-v1.mjs');
1819
const ownerStoreLockHolder = join(root, 'tests/fixtures/owner-store-lock-holder.mjs');
1920

21+
function isGateRunPath(path) { return path.split(sep).includes('gate-runs'); }
22+
2023
async function jsonFiles(directory) {
2124
const found = []; let entries;
2225
try { entries = await readdir(directory, { withFileTypes: true }); } catch { return found; }
@@ -121,7 +124,7 @@ test('unborn repositories get baselines and full untracked contents affect finge
121124
const prompt = await runHook('user-prompt-hook.mjs', { session_id: 'unborn', turn_id: 'turn', cwd, hook_event_name: 'UserPromptSubmit', transcript_path: null, model: 'gpt', permission_mode: 'default', prompt: 'work' }, env); assert.equal(prompt.code, 0); assert.deepEqual(prompt.json, {}); assert.equal((await createIdentityStore({ dataRoot: data }).resolveActiveTurn({ sessionId: 'unborn', workspace: cwd })).turnId, 'turn');
122125
bytes.fill(66, 160 * 1024, 224 * 1024); await writeFile(join(cwd, 'large.bin'), bytes);
123126
const stop = await runHook('stop-review-gate-hook.mjs', { session_id: 'unborn', turn_id: 'turn', cwd, hook_event_name: 'Stop', transcript_path: null, model: 'gpt', permission_mode: 'default', stop_hook_active: false, last_assistant_message: 'done' }, env); assert.equal(stop.code, 0); assert.deepEqual(stop.json, {});
124-
assert.equal((await jsonFiles(join(data, 'workspaces'))).filter((path) => path.includes('/gate-runs/')).length, 1, 'same-size middle-only untracked edits must change the fingerprint');
127+
assert.equal((await jsonFiles(join(data, 'workspaces'))).filter(isGateRunPath).length, 1, 'same-size middle-only untracked edits must change the fingerprint');
125128
});
126129

127130
test('changing only an untracked symlink target changes the fingerprint without following it', async () => {
@@ -131,7 +134,7 @@ test('changing only an untracked symlink target changes the fingerprint without
131134
const prompt = await runHook('user-prompt-hook.mjs', { session_id: 'symlink', turn_id: 'turn', cwd, hook_event_name: 'UserPromptSubmit', transcript_path: null, model: 'gpt', permission_mode: 'default', prompt: 'work' }, env); assert.equal(prompt.code, 0);
132135
await unlink(link); await symlink('missing-target-b', link);
133136
const stop = await runHook('stop-review-gate-hook.mjs', { session_id: 'symlink', turn_id: 'turn', cwd, hook_event_name: 'Stop', transcript_path: null, model: 'gpt', permission_mode: 'default', stop_hook_active: false, last_assistant_message: 'done' }, env); assert.equal(stop.code, 0); assert.deepEqual(stop.json, {});
134-
assert.equal((await jsonFiles(join(data, 'workspaces'))).filter((path) => path.includes('/gate-runs/')).length, 1, 'same-path symlink target changes must reach the gate path');
137+
assert.equal((await jsonFiles(join(data, 'workspaces'))).filter(isGateRunPath).length, 1, 'same-path symlink target changes must reach the gate path');
135138
});
136139

137140
test('SubagentStart marks forwarding suppression without changing parent permission snapshot', async () => {
@@ -332,7 +335,7 @@ test('Stop gate skips unchanged and atomically consumes exact changed baseline',
332335
const input = { session_id: 'parent', turn_id: 'turn-2', cwd, hook_event_name: 'Stop', transcript_path: null, model: 'gpt', permission_mode: 'default', stop_hook_active: false, last_assistant_message: 'done' };
333336
const [one, two] = await Promise.all([runHook('stop-review-gate-hook.mjs', input, { ...env, ZCODE_PATH: fakeZCode, FAKE_ZCODE_GATE_RESULT: 'ALLOW: clean' }), runHook('stop-review-gate-hook.mjs', input, { ...env, ZCODE_PATH: fakeZCode, FAKE_ZCODE_GATE_RESULT: 'ALLOW: clean' })]);
334337
assert.equal([one, two].filter((result) => result.json?.decision === 'block').length, 0);
335-
const snapshots = (await jsonFiles(join(data, 'workspaces'))).filter((path) => path.includes('/gate-runs/'));
338+
const snapshots = (await jsonFiles(join(data, 'workspaces'))).filter(isGateRunPath);
336339
assert.equal(snapshots.length, 1);
337340
});
338341

@@ -369,7 +372,7 @@ test('Stop rechecks stale setup readiness before session creation and fails open
369372
]) await t.test(scenario.name, async () => {
370373
const { cwd, data, env } = await workspace(); const record = join(data, 'zcode-calls.jsonl'); await writeFile(record, ''); await writeGateConfig(data, cwd, { enabled: true, setupReady: true, status: 'ready' }); await runHook('session-lifecycle-hook.mjs', { session_id: 'owner', cwd, hook_event_name: 'SessionStart', transcript_path: null, model: 'gpt', permission_mode: 'default', source: 'startup' }, env);
371374
const prompt = { session_id: 'owner', turn_id: 'turn', cwd, hook_event_name: 'UserPromptSubmit', transcript_path: null, model: 'gpt', permission_mode: 'default', prompt: 'edit' }; await runHook('user-prompt-hook.mjs', prompt, env); await writeFile(join(cwd, 'tracked.txt'), `${scenario.name}\n`); const script = scenario.fixture ? join(root, 'tests/fixtures/stop-gate-with-timeout.mjs') : 'stop-review-gate-hook.mjs'; const result = await runHook(script, { ...stopFields(prompt), hook_event_name: 'Stop', stop_hook_active: false, last_assistant_message: 'done' }, { ...env, ZCODE_PATH: fakeZCode, FAKE_ZCODE_RECORD: record, ...scenario.extra }, { absolute: scenario.fixture });
372-
assert.equal(result.code, 0); assert.notEqual(result.json?.decision, 'block'); assert.match(result.json.systemMessage, /\$zcode:setup/); const runs = (await jsonFiles(join(data, 'workspaces'))).filter((path) => path.includes('/gate-runs/')); assert.equal(runs.length, 1); const snapshot = JSON.parse(await readFile(runs[0], 'utf8')); assert.equal(snapshot.status, 'skipped_setup_not_ready'); assert.equal(snapshot.reason, scenario.reason);
375+
assert.equal(result.code, 0); assert.notEqual(result.json?.decision, 'block'); assert.match(result.json.systemMessage, /\$zcode:setup/); const runs = (await jsonFiles(join(data, 'workspaces'))).filter(isGateRunPath); assert.equal(runs.length, 1); const snapshot = JSON.parse(await readFile(runs[0], 'utf8')); assert.equal(snapshot.status, 'skipped_setup_not_ready'); assert.equal(snapshot.reason, scenario.reason);
373376
const calls = (await readFile(record, 'utf8')).trim().split('\n').filter(Boolean).map(JSON.parse); assert.ok(!calls.some((call) => call.method === 'session/send'));
374377
});
375378
});

tests/integration/companion.test.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import assert from 'node:assert/strict';
22
import { spawn } from 'node:child_process';
33
import { mkdir, mkdtemp, readFile, readdir, rename, rm, stat, symlink, unlink, writeFile } from 'node:fs/promises';
44
import { tmpdir } from 'node:os';
5-
import { join } from 'node:path';
5+
import { basename, join } from 'node:path';
66
import { fileURLToPath } from 'node:url';
77
import test from 'node:test';
88

@@ -171,7 +171,7 @@ test('artifact read and write reject intermediate directory symlinks', async ()
171171

172172
const readContext = await fixture(); const completed = await companion(readContext, ['review']); const readStorage = await resolveWorkspaceStorage(readContext);
173173
const resultsRoot = join(readStorage.directory, 'results'); const readEscape = join(readContext.directory, 'read-escape'); await mkdir(readEscape);
174-
const name = completed.json.job.resultArtifact.split('/').at(-1); await rename(join(resultsRoot, name), join(readEscape, name)); await rm(resultsRoot, { recursive: true }); await symlink(readEscape, resultsRoot);
174+
const name = basename(completed.json.job.resultArtifact); await rename(join(resultsRoot, name), join(readEscape, name)); await rm(resultsRoot, { recursive: true }); await symlink(readEscape, resultsRoot);
175175
const readResult = await companion(readContext, ['result', completed.json.job.id]);
176176
assert.notEqual(readResult.code, 0); assert.equal(readResult.json.error.code, 'RESULT_READ_FAILED');
177177
});

tests/setup.test.mjs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import assert from 'node:assert/strict';
33
import { mkdtemp, readFile, writeFile, mkdir, realpath, stat } from 'node:fs/promises';
44
import { tmpdir } from 'node:os';
55
import { join } from 'node:path';
6-
import { pathToFileURL } from 'node:url';
6+
import { fileURLToPath, pathToFileURL } from 'node:url';
77
import test from 'node:test';
88
import { spawn } from 'node:child_process';
99

@@ -12,7 +12,7 @@ import { diagnoseZCodeAuth, pluginRootFromModuleUrl, runSetup } from '../scripts
1212
import { resolveWorkspaceStorage } from '../scripts/lib/workspace.mjs';
1313
import { runCompanion } from '../scripts/zcode-companion.mjs';
1414

15-
const root = new URL('../', import.meta.url).pathname;
15+
const root = fileURLToPath(new URL('../', import.meta.url));
1616
const fakeCodex = join(root, 'tests/fixtures/fake-codex-app-server.mjs');
1717
const fakeZCode = join(root, 'tests/fixtures/fake-zcode-cli.mjs');
1818

@@ -120,7 +120,9 @@ test('plugin root derivation decodes file URLs with spaces and percent character
120120

121121
test('app-server failure cannot persist a ready gate and enable/disable touches only workspace gate state', async () => {
122122
const failed = await context({ codexEnv: { FAKE_CODEX_ERROR: 'hooks/list' } }); await assert.rejects(runSetup({ ...failed.options, reviewGate: true }), { code: 'CODEX_CONFIG_REQUEST_FAILED' });
123-
for (let index = 0; index < 50 && !(await readFile(failed.record, 'utf8')).includes('lifecycle'); index += 1) await new Promise((resolvePromise) => setTimeout(resolvePromise, 10)); assert.match(await readFile(failed.record, 'utf8'), /"lifecycle":"SIGTERM"/);
123+
if (process.platform !== 'win32') {
124+
for (let index = 0; index < 50 && !(await readFile(failed.record, 'utf8')).includes('lifecycle'); index += 1) await new Promise((resolvePromise) => setTimeout(resolvePromise, 10)); assert.match(await readFile(failed.record, 'utf8'), /"lifecycle":"SIGTERM"/);
125+
}
124126
const failedStorage = await resolveWorkspaceStorage({ dataRoot: failed.dataRoot, workspace: failed.cwd }); await assert.rejects(readFile(join(failedStorage.directory, 'config/review-gate.json'), 'utf8'), { code: 'ENOENT' });
125127
const disabled = await context({ hooks: hookMetadata(root, 'trusted'), features: { hooks: true } }); const report = await runSetup({ ...disabled.options, reviewGate: false }); assert.equal(report.reviewGate.enabled, false);
126128
const disabledStorage = await resolveWorkspaceStorage({ dataRoot: disabled.dataRoot, workspace: disabled.cwd }); const gate = JSON.parse(await readFile(join(disabledStorage.directory, 'config/review-gate.json'), 'utf8')); assert.equal(gate.enabled, false); assert.equal(gate.setupReady, true);

0 commit comments

Comments
 (0)