mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix: preserve file identities and join Windows test processes
This commit is contained in:
1 parent
0c11ff3535
commit
1fd0ea5d68
3 files changed
+81
-17
No files matched your search
@@ -352,12 +352,12 @@ export function amendImplementation(phase: string, activePlan: string, snapshotP
|
||||
throw new Error('Assembled accepted obligations do not match; no overwrite');
|
||||
}
|
||||
if (next !== original) {
|
||||
const before = statSync(source);
|
||||
const before = statSync(source, { bigint: true });
|
||||
const directory = mkdtempSync(join(dirname(source), '.autoplan-amend-'));
|
||||
try {
|
||||
const stage = join(directory, 'plan');
|
||||
writeFileSync(stage, next, { flag: 'wx', mode: before.mode & 0o777 });
|
||||
const current = statSync(source);
|
||||
writeFileSync(stage, next, { flag: 'wx', mode: Number(before.mode & 0o777n) });
|
||||
const current = statSync(source, { bigint: true });
|
||||
if (before.dev !== current.dev || before.ino !== current.ino || readFileSync(source, 'utf8') !== original) {
|
||||
throw new Error('Active plan changed during amendment; no overwrite');
|
||||
}
|
||||
@@ -378,13 +378,15 @@ export function initializePlan(sourcePlan: string, activePlan: string, restorePa
|
||||
missing.unshift(basename(parent)); parent = dirname(parent);
|
||||
}
|
||||
const canonical = join(realpathSync(parent), ...missing, basename(file));
|
||||
const state = lstatSync(canonical, { throwIfNoEntry: false });
|
||||
const state = lstatSync(canonical, { throwIfNoEntry: false, bigint: true });
|
||||
if (state && !state.isFile()) throw new Error('Initialization destinations must be regular files, not links or directories');
|
||||
return { file: canonical, state, bytes: state ? readFileSync(canonical) : undefined };
|
||||
};
|
||||
const active = destination(activePlan);
|
||||
const restore = destination(restorePath);
|
||||
const sourceState = statSync(source);
|
||||
// Windows file IDs can exceed Number's exact range. Preserve their full
|
||||
// identity for alias, concurrent-change and rollback ownership checks.
|
||||
const sourceState = statSync(source, { bigint: true });
|
||||
if (!sourceState.isFile()) throw new Error('Initialization source must be a regular file');
|
||||
const sourceBytes = readFileSync(source);
|
||||
const sameFile = (a: typeof sourceState, b: typeof sourceState) => a.dev === b.dev && a.ino === b.ino;
|
||||
@@ -423,8 +425,8 @@ export function initializePlan(sourcePlan: string, activePlan: string, restorePa
|
||||
const next = normalized(sourceBytes);
|
||||
const expectedScopeHash = sha256(extractImplementationPlan(next.toString('utf8')));
|
||||
const unchanged = () => {
|
||||
const now = statSync(source);
|
||||
const current = lstatSync(active.file, { throwIfNoEntry: false });
|
||||
const now = statSync(source, { bigint: true });
|
||||
const current = lstatSync(active.file, { throwIfNoEntry: false, bigint: true });
|
||||
if (!sameFile(now, sourceState) || !readFileSync(source).equals(sourceBytes) ||
|
||||
(active.state ? !current?.isFile() || !sameFile(current, active.state) || !readFileSync(active.file).equals(active.bytes!) : current !== undefined)) {
|
||||
throw new Error('Initialization input or destination changed; refusing to overwrite it');
|
||||
@@ -454,7 +456,7 @@ export function initializePlan(sourcePlan: string, activePlan: string, restorePa
|
||||
restoreStage = mkdtempSync(join(dirname(restore.file), '.gstack-autoplan-restore-'));
|
||||
const stagedActive = join(activeStage, 'active.md');
|
||||
const stagedRestore = join(restoreStage, 'original.md');
|
||||
writeFileSync(stagedActive, next, { flag: 'wx', mode: active.state ? active.state.mode & 0o777 : 0o600 });
|
||||
writeFileSync(stagedActive, next, { flag: 'wx', mode: active.state ? Number(active.state.mode & 0o777n) : 0o600 });
|
||||
writeFileSync(stagedRestore, sourceBytes, { flag: 'wx', mode: 0o400 });
|
||||
unchanged();
|
||||
// Link publishes complete restore bytes exclusively; an existing backup is never replaced.
|
||||
@@ -472,8 +474,8 @@ export function initializePlan(sourcePlan: string, activePlan: string, restorePa
|
||||
} finally {
|
||||
// On pre-publication failure remove only the restore inode this invocation published.
|
||||
if (backupPublished && !activePublished && restoreStage) {
|
||||
const current = lstatSync(restore.file, { throwIfNoEntry: false });
|
||||
if (current?.isFile() && sameFile(current, statSync(join(restoreStage, 'original.md')))) unlinkSync(restore.file);
|
||||
const current = lstatSync(restore.file, { throwIfNoEntry: false, bigint: true });
|
||||
if (current?.isFile() && sameFile(current, statSync(join(restoreStage, 'original.md'), { bigint: true }))) unlinkSync(restore.file);
|
||||
}
|
||||
if (activeStage) rmSync(activeStage, { recursive: true, force: true });
|
||||
if (restoreStage) rmSync(restoreStage, { recursive: true, force: true });
|
||||
|
||||
@@ -171,6 +171,50 @@ test('line endings and Unicode survive normalization; only a missing final separ
|
||||
}
|
||||
});
|
||||
|
||||
test('large file identities remain distinct on reuse while real hardlink aliases are rejected', () => {
|
||||
const f = fixture();
|
||||
const worker = join(f.dir, 'large-file-ids.ts');
|
||||
writeFileSync(worker, `import { mock } from 'bun:test';
|
||||
const real = { ...await import('node:fs') };
|
||||
const ids = new Map();
|
||||
function observed(kind, file, options) {
|
||||
const exact = real[kind](file, { ...options, bigint: true });
|
||||
if (!exact) return exact;
|
||||
const key = exact.dev + ':' + exact.ino;
|
||||
if (!ids.has(key)) ids.set(key, 2n ** 60n + BigInt(ids.size));
|
||||
const ino = ids.get(key);
|
||||
const state = options?.bigint ? exact : real[kind](file, options);
|
||||
return new Proxy(state, { get(target, key, receiver) {
|
||||
return key === 'ino' ? (options?.bigint ? ino : Number(ino)) : Reflect.get(target, key, receiver);
|
||||
} });
|
||||
}
|
||||
mock.module('node:fs', () => ({ ...real,
|
||||
statSync: (file, options) => observed('statSync', file, options),
|
||||
lstatSync: (file, options) => observed('lstatSync', file, options),
|
||||
}));
|
||||
const { initializePlan } = await import(${JSON.stringify(TOOL)});
|
||||
const [source, active, restore, alias] = process.argv.slice(2);
|
||||
const initial = initializePlan(source, active, restore);
|
||||
const reused = initializePlan(source, active, restore);
|
||||
real.linkSync(source, alias);
|
||||
let rejected = false;
|
||||
try { initializePlan(source, alias, restore + '.other'); }
|
||||
catch (error) { rejected = error.message.includes('ambiguous alias'); }
|
||||
console.log(JSON.stringify({ initial: initial.reused, reused: reused.reused, rejected,
|
||||
roundedIds: new Set([...ids.values()].map(Number)).size, exactIds: ids.size }));
|
||||
`);
|
||||
const result = spawnSync(process.execPath, [worker, f.source, f.active, f.restore, join(f.dir, 'hardlink.md')], {
|
||||
encoding: 'utf8', timeout: 10_000,
|
||||
});
|
||||
expect(result.status, result.stderr).toBe(0);
|
||||
const report = JSON.parse(result.stdout);
|
||||
expect(report).toMatchObject({ initial: false, reused: true, rejected: true, roundedIds: 1 });
|
||||
expect(report.exactIds).toBeGreaterThan(2);
|
||||
expect(readFileSync(f.source)).toEqual(original);
|
||||
expect(readFileSync(f.restore)).toEqual(original);
|
||||
expect(existsSync(f.restore + '.other')).toBe(false);
|
||||
});
|
||||
|
||||
test('staging failure cleans owned temporary files without changing source or active bytes', () => {
|
||||
const f = fixture();
|
||||
const active = join(f.dir, 'harness', 'plans', 'assigned.md');
|
||||
|
||||
@@ -9,6 +9,7 @@ const DIR = mkdtempSync(path.join(tmpdir(), 'claude-windows-job-'));
|
||||
const FAKE = path.join(DIR, 'fake claude.ts');
|
||||
const DESCENDANT = path.join(DIR, 'pipe holder.ts');
|
||||
const PID_FILE = path.join(DIR, 'descendant.pid');
|
||||
const PROVIDER_PID_FILE = path.join(DIR, 'provider.pid');
|
||||
const CLI = path.join(ROOT, 'bin/gstack-claude-code');
|
||||
const FAILED_JOB = path.join(DIR, 'failed job.ts');
|
||||
|
||||
@@ -35,6 +36,7 @@ writeFileSync(FAKE, `
|
||||
import { spawn } from 'node:child_process';
|
||||
import { existsSync, rmSync, writeFileSync } from 'node:fs';
|
||||
await Bun.stdin.text();
|
||||
writeFileSync(process.env.PROVIDER_PID_FILE!, String(process.pid));
|
||||
rmSync(process.env.PID_FILE!, { force: true });
|
||||
const child = spawn(process.execPath, [process.env.DESCENDANT!], {
|
||||
stdio: ['ignore', 'inherit', 'inherit'],
|
||||
@@ -62,6 +64,7 @@ function environment(mode: string): NodeJS.ProcessEnv {
|
||||
GSTACK_CLAUDE_BIN_ARGS: JSON.stringify([FAKE]),
|
||||
FAKE_MODE: mode,
|
||||
PID_FILE,
|
||||
PROVIDER_PID_FILE,
|
||||
DESCENDANT,
|
||||
};
|
||||
}
|
||||
@@ -71,15 +74,18 @@ function alive(pid: number): boolean {
|
||||
}
|
||||
|
||||
async function expectDead(pid: number) {
|
||||
expect(Number.isSafeInteger(pid) && pid > 0).toBe(true);
|
||||
for (let i = 0; i < 40 && alive(pid); i++) await Bun.sleep(25);
|
||||
expect(alive(pid)).toBe(false);
|
||||
}
|
||||
|
||||
function cleanupDescendant() {
|
||||
try {
|
||||
const pid = Number(readFileSync(PID_FILE, 'utf8'));
|
||||
if (alive(pid)) process.kill(pid, 'SIGKILL');
|
||||
} catch { /* No descendant was created. */ }
|
||||
function cleanupOwnedProcesses() {
|
||||
for (const file of [PID_FILE, PROVIDER_PID_FILE]) {
|
||||
try {
|
||||
const pid = Number(readFileSync(file, 'utf8'));
|
||||
if (Number.isSafeInteger(pid) && pid > 0 && alive(pid)) process.kill(pid, 'SIGKILL');
|
||||
} catch { /* No owned process remains. */ }
|
||||
}
|
||||
}
|
||||
|
||||
// These exercise the actual standalone CLI boundary. A job must never be
|
||||
@@ -101,7 +107,10 @@ describe('Windows Claude CLI job containment', () => {
|
||||
|
||||
for (const [mode, expected] of [['descendant', 'output-drain'], ['timeout', 'timeout']]) {
|
||||
test.skipIf(process.platform !== 'win32')(`${mode} kills owned descendants and preserves a sibling process`, async () => {
|
||||
rmSync(PID_FILE, { force: true });
|
||||
rmSync(PROVIDER_PID_FILE, { force: true });
|
||||
const sibling = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' });
|
||||
const siblingClosed = new Promise<void>(resolve => sibling.once('close', () => resolve()));
|
||||
const started = Date.now();
|
||||
try {
|
||||
const result = spawnSync(process.execPath, [CLI, '--cwd', DIR, '--access', 'none', '--timeout-ms', '2000'], {
|
||||
@@ -114,19 +123,23 @@ describe('Windows Claude CLI job containment', () => {
|
||||
expect(parsed.error.code).toBe(expected);
|
||||
expect(Date.now() - started).toBeLessThan(6000);
|
||||
await expectDead(Number(readFileSync(PID_FILE, 'utf8')));
|
||||
await expectDead(Number(readFileSync(PROVIDER_PID_FILE, 'utf8')));
|
||||
expect(alive(sibling.pid!)).toBe(true);
|
||||
} finally {
|
||||
cleanupDescendant();
|
||||
cleanupOwnedProcesses();
|
||||
sibling.kill('SIGKILL');
|
||||
await siblingClosed;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
test.skipIf(process.platform !== 'win32')('abrupt runner exit closes the job and reaps its descendants', async () => {
|
||||
rmSync(PID_FILE, { force: true });
|
||||
rmSync(PROVIDER_PID_FILE, { force: true });
|
||||
const runner = spawn(process.execPath, [CLI, '--cwd', DIR, '--access', 'none', '--timeout-ms', '10000'], {
|
||||
env: environment('timeout'), stdio: ['pipe', 'ignore', 'ignore'],
|
||||
});
|
||||
const runnerClosed = new Promise<void>(resolve => runner.once('close', () => resolve()));
|
||||
runner.stdin!.end('review');
|
||||
try {
|
||||
let descendant = 0;
|
||||
@@ -136,9 +149,14 @@ describe('Windows Claude CLI job containment', () => {
|
||||
expect(descendant).toBeGreaterThan(0);
|
||||
runner.kill('SIGKILL');
|
||||
await expectDead(descendant);
|
||||
// The fake provider also owns the fixture cwd. Job termination starts
|
||||
// every member's shutdown; leaf death alone does not prove it is done.
|
||||
await expectDead(Number(readFileSync(PROVIDER_PID_FILE, 'utf8')));
|
||||
} finally {
|
||||
runner.kill('SIGKILL');
|
||||
cleanupDescendant();
|
||||
cleanupOwnedProcesses();
|
||||
// Join the direct child and close its streams before fixture teardown.
|
||||
await runnerClosed;
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user