fix: pre-landing review round — 8 auto-fixes + 8 accepted findings hardened

The ship review army (4 specialists + red-team + checklist, 29 findings)
produced 8 mechanical auto-fixes and 11 decisions; the accepted set:

- win32 slug parity completed: lib/bin-context.ts gains the remote-first
  outermost walk + degraded-cache self-heal the bash side got this wave —
  the two implementations now agree on the stray-marker live-bug shape,
  pinned by shared fixtures (multi-specialist 9/10 finding).
- probe honors the plan's bounded-read decision: 256KB prefix, extraction
  semantics mirrored from parseTranscriptJsonl so probe/prepare can never
  diverge on the same file (>1MB transcript test).
- policy normalize parity: bash normalize() now matches canonicalizeRemote
  on .git/-trailing and uppercase-.GIT shapes (7-shape corpus pinned two
  ways) — a deny for those shapes could previously slip the transcript gate.
- session-update reclaim is TOCTOU-safe (atomic mv-aside on both branches).
- settings-hook: unparseable settings.json errors instead of being replaced
  with {}; ensure-event keys on (event, source) so matcher changes update
  in place — never zero or two registrations.
- dot-only slug guard at both parse sites (hostile 'url = ..' can't escape
  projects/); enqueue tmp-file janitor (1h TTL, inside the drain lock);
  brain-sync .migrating never clobbered; drop-queue/status count .migrating;
  snapshot -o warning correct + surfaced in diff mode; version-bump test
  order-dependence removed; uninstall clears the advance stamp.

Deferred with record: slug heal-probe cost sentinel (P3 TODO), FF_OK
conflation (noted, misdiagnosis-only).

270 pass / 0 fail across the 10 touched suites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-08-17 13:20:20 -07:00
co-authored by Claude Fable 5
parent 9fecf0f16f
commit b7d44c45b4
18 changed files with 648 additions and 76 deletions
+65
View File
@@ -343,6 +343,71 @@ describe('timeline-stop-hook wiring', () => {
}
});
test('corrupt settings.json: ensure-event refuses (exit 1) and never rewrites the file', () => {
// The old catch{} folded an unparseable EXISTING settings.json into {}
// and the atomic write replaced the user's permissions/env/other hooks
// with just ours. Now: loud stderr error, exit 1, file byte-identical.
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-ensure-corrupt-'));
try {
const settingsFile = path.join(dir, 'settings.json');
const corrupt = '{ "permissions": { "allow": ["Bash(npm:*)"] }, INVALID';
fs.writeFileSync(settingsFile, corrupt);
const r = spawnSync('bash', [
SETTINGS_HOOK, 'ensure-event',
'--event', 'Stop',
'--command', HOOK,
'--source', 'gstack-timeline-stop',
'--timeout', '5',
], { env: { ...process.env, GSTACK_SETTINGS_FILE: settingsFile }, encoding: 'utf-8', timeout: 15_000 });
expect(r.status).toBe(1);
expect(r.stderr).toContain('not valid JSON');
// Never rewritten — the corrupt bytes (and whatever the user can still
// salvage from them) survive verbatim.
expect(fs.readFileSync(settingsFile, 'utf-8')).toBe(corrupt);
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
});
test('a matcher change updates the tagged entry in place — still exactly one registration', () => {
// Identity key is (event, source): an existing gstack entry with a STALE
// matcher must be updated, never joined by a second entry (the old key
// included the matcher, so any future matcher change would duplicate).
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-ensure-matcher-'));
try {
const settingsFile = path.join(dir, 'settings.json');
fs.writeFileSync(settingsFile, JSON.stringify({
hooks: {
PreToolUse: [{
_gstack_source: 'gstack-plan-tune',
matcher: 'OldMatcher',
hooks: [{ type: 'command', command: '/old/path/hook', timeout: 5 }],
}],
},
}, null, 2) + '\n');
const r = spawnSync('bash', [
SETTINGS_HOOK, 'ensure-event',
'--event', 'PreToolUse',
'--command', '/new/path/hook',
'--source', 'gstack-plan-tune',
'--matcher', 'NewMatcher',
'--timeout', '5',
], { env: { ...process.env, GSTACK_SETTINGS_FILE: settingsFile }, encoding: 'utf-8', timeout: 15_000 });
expect(r.status).toBe(0);
const s = JSON.parse(fs.readFileSync(settingsFile, 'utf-8'));
expect(s.hooks.PreToolUse).toHaveLength(1); // updated in place — never two
expect(s.hooks.PreToolUse[0].matcher).toBe('NewMatcher');
expect(s.hooks.PreToolUse[0].hooks[0].command).toBe('/new/path/hook');
expect(s.hooks.PreToolUse[0]._gstack_source).toBe('gstack-plan-tune');
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
});
test('a failed update leaves exactly one registration — never zero, never two', () => {
// Root can write through 0o555 directories, so the failure injection
// (read-only dir) does not bind there; the invariant is still covered by