test: review-army + adversarial test hardening

- Tripwire scans execFileSync too (ceiling 8: two more grep-needle string
  exemptions); merge-introduced timeout-less spawnSync in
  question-preference-hook fixed — the tripwire caught a site that landed
  on main AFTER the sweep, on its first day.
- gstack-detach gains TWO watchdog kill regression tests: TERM-immune
  grandchild (the killpg-after-grace escalation) and the leader-dies
  variant (the pgid-at-spawn fix — the case the first test cannot see).
- eval-flake-rank gets its unit suite (final-attempt accounting, artifact
  exclusion, shard recursion, recency bound).
- Groupkill/startup-grace shim markers are per-run unique (pid-suffixed
  sleep durations): sibling Conductor worktrees run free suites with no
  machine lock, and fixed markers let one run pgrep/pkill the other's
  shims — a cross-run flake inside the anti-flake tests.
- flake-ledger test pins the project-scoped local default; stale empty
  section headers in touchfiles-data deleted (they invited entries under
  deliberately retired categories).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-08-31 05:38:26 +00:00
co-authored by Claude Fable 5
parent e4954aaebb
commit 5b04f05ba4
8 changed files with 148 additions and 25 deletions
+70
View File
@@ -0,0 +1,70 @@
/**
* Unit tests for the flake-rank aggregator (WS1's dial). The CLI ranks tests
* by retried passes (the flake signature) across finalized eval-store runs —
* these pin the accounting: N attempt records = 1 run of that test, the
* FINAL attempt decides pass/fail, retried passes count separately, partials
* and runner artifacts are excluded, shard dirs recurse, and the recency
* bound drops stale files.
*/
import { describe, expect, test } from 'bun:test';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { aggregate, collectEvalFiles } from '../scripts/eval-flake-rank';
const entry = (name: string, passed: boolean, attempt: number) => ({
name, suite: 's', tier: 'e2e', passed, attempt, duration_ms: 1000, cost_usd: 0.1,
});
const run = (tests: object[], extra: object = {}) => JSON.stringify({
schema_version: 2, version: '1.0.0', branch: 'b', git_sha: 'x', hostname: 'h',
timestamp: '2026-08-31T00:00:00Z', tier: 'e2e',
total_tests: tests.length, passed: 0, failed: 0, total_cost_usd: 0, total_duration_ms: 0,
tests, ...extra,
});
describe('eval-flake-rank aggregate', () => {
test('final attempt decides; retried pass counts as retriedPass, not a fail', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'flakerank-'));
fs.writeFileSync(path.join(dir, 'run1.json'), run([
entry('flaky', false, 1), entry('flaky', true, 2), // pass on retry
entry('steady', true, 1),
entry('broken', false, 1), entry('broken', false, 2), // fails even retried
]));
fs.writeFileSync(path.join(dir, 'run2.json'), run([
entry('flaky', true, 1), entry('steady', true, 1),
]));
const series = aggregate(collectEvalFiles(dir));
expect(series.get('flaky')).toMatchObject({ runs: 2, passes: 2, fails: 0, retriedPasses: 1, totalAttempts: 3 });
expect(series.get('steady')).toMatchObject({ runs: 2, passes: 2, fails: 0, retriedPasses: 0 });
expect(series.get('broken')).toMatchObject({ runs: 1, passes: 0, fails: 1, retriedPasses: 0, totalAttempts: 2 });
fs.rmSync(dir, { recursive: true, force: true });
});
test('partials and runner artifacts are excluded; shard dirs recurse', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'flakerank-'));
fs.mkdirSync(path.join(dir, 'shards', 'slug-a'), { recursive: true });
fs.writeFileSync(path.join(dir, 'shards', 'slug-a', 'run.json'), run([entry('sharded', true, 1)]));
fs.writeFileSync(path.join(dir, '_partial-e2e.json'), run([entry('inflight', false, 1)], { _partial: true }));
fs.writeFileSync(path.join(dir, 'manifest.json'), '{"version":1}');
fs.writeFileSync(path.join(dir, 'slice-3.json'), '{"version":1}');
const files = collectEvalFiles(dir);
expect(files).toHaveLength(1);
const series = aggregate(files);
expect(series.has('sharded')).toBe(true);
expect(series.has('inflight')).toBe(false);
fs.rmSync(dir, { recursive: true, force: true });
});
test('recency bound drops files older than sinceDays', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'flakerank-'));
const stale = path.join(dir, 'old.json');
fs.writeFileSync(stale, run([entry('ancient', true, 1)]));
const old = new Date(Date.now() - 90 * 86_400_000);
fs.utimesSync(stale, old, old);
fs.writeFileSync(path.join(dir, 'new.json'), run([entry('recent', true, 1)]));
const files = collectEvalFiles(dir, 60);
expect(files.map((f) => path.basename(f))).toEqual(['new.json']);
fs.rmSync(dir, { recursive: true, force: true });
});
});
+7 -2
View File
@@ -52,8 +52,13 @@ describe('flake ledger', () => {
fs.rmSync(dir, { recursive: true, force: true });
});
test('env override wins over the tmpdir default', () => {
test('env override wins; local default is project-scoped, never machine-global', () => {
expect(flakeLedgerPath({ GSTACK_FLAKE_LEDGER: '/x/y.jsonl' } as NodeJS.ProcessEnv)).toBe('/x/y.jsonl');
expect(flakeLedgerPath({} as NodeJS.ProcessEnv)).toContain('gstack-flake-ledger.jsonl');
// Without the env override, the default lives under the PROJECT dir
// (sibling worktrees of different repos must not interleave one series);
// tmpdir is only the last-resort fallback when slug detection fails.
const local = flakeLedgerPath({} as NodeJS.ProcessEnv);
expect(local).toMatch(/flake-ledger\.jsonl$/);
expect(local.includes(path.join('.gstack', 'projects')) || local.startsWith(os.tmpdir())).toBe(true);
});
});
+53
View File
@@ -69,6 +69,59 @@ describe('gstack-detach', () => {
} finally { fs.rmSync(dir, { recursive: true, force: true }); }
}, 16000);
test('watchdog group-SIGKILLs TERM-immune grandchildren (no orphan survives)', () => {
// Regression pin for the 2026-08 escalation change: the watchdog used to
// follow its killpg(SIGTERM) + 5s grace with a DIRECT proc.kill() — a
// grandchild that ignores TERM survived and burned cores/API for hours
// (the observed 15-hour-orphan class). Now the grace escalates to
// killpg(SIGKILL). The child here traps TERM and spawns a TERM-immune
// grandchild; only a GROUP SIGKILL clears both. Markers are per-run
// unique (pid) so concurrent worktree suites can't cross-kill.
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gd-'));
const log = path.join(dir, 'run.log');
const g1 = `6091.${process.pid}`;
const g2 = `6092.${process.pid}`;
const alive = (m: string) => spawnSync('pgrep', ['-f', `sleep ${m.replace('.', '\\.')}`], { stdio: 'pipe', timeout: 5_000 }).status === 0;
try {
spawnSync(DETACH, ['--log', log, '--timeout', '1', '--', 'bash', '-c',
`trap '' TERM; (trap '' TERM; sleep ${g1}) & exec sleep ${g2}`],
{ encoding: 'utf-8', timeout: 10000 });
expect(waitFor(() => logHas(log, '### gstack-detach EXIT=timeout ###'), 15000)).toBe(true);
// Grace is 5s after the TERM that both processes ignore — the SIGKILL
// escalation must clear the whole group shortly after the sentinel.
expect(waitFor(() => !alive(g1) && !alive(g2), 10000),
'TERM-immune child/grandchild survived the watchdog — killpg(SIGKILL) escalation regressed').toBe(true);
} finally {
spawnSync('pkill', ['-9', '-f', `sleep 609[12]\\.${process.pid}`], { stdio: 'ignore', timeout: 5_000 });
fs.rmSync(dir, { recursive: true, force: true });
}
}, 30000);
test('watchdog kills the grandchild even when the LEADER dies on the SIGTERM', () => {
// The pgid-after-grace bug: killpg(getpgid(proc.pid), SIGKILL) raised
// ESRCH once the leader had honored the TERM, and the except fell back
// to proc.kill() on a corpse — the TERM-immune grandchild lived forever.
// The fix captures the pgid AT SPAWN. This variant is the one the
// TERM-immune-leader test above cannot see.
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gd-'));
const log = path.join(dir, 'run.log');
const g = `6093.${process.pid}`;
const alive = () => spawnSync('pgrep', ['-f', `sleep ${g.replace('.', '\\.')}`], { stdio: 'pipe', timeout: 5_000 }).status === 0;
try {
// Leader: no trap — dies on the watchdog's SIGTERM. Grandchild:
// TERM-immune, same group — only a saved-pgid SIGKILL reaches it.
spawnSync(DETACH, ['--log', log, '--timeout', '1', '--', 'bash', '-c',
`(trap '' TERM; sleep ${g}) & sleep 60`],
{ encoding: 'utf-8', timeout: 10000 });
expect(waitFor(() => logHas(log, '### gstack-detach EXIT=timeout ###'), 15000)).toBe(true);
expect(waitFor(() => !alive(), 10000),
'grandchild survived a dead leader — the pgid must be captured at spawn, not resolved after the grace').toBe(true);
} finally {
spawnSync('pkill', ['-9', '-f', `sleep 6093\\.${process.pid}`], { stdio: 'ignore', timeout: 5_000 });
fs.rmSync(dir, { recursive: true, force: true });
}
}, 30000);
test('machine --lock serializes concurrent runs (second WAITS for the first)', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gd-'));
const lock = `gstack-detach-test-${process.pid}`;
-10
View File
@@ -229,8 +229,6 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
'retro': ['bin/gstack-retro-metrics', 'retro/**', 'test/skill-e2e-retro.test.ts'],
'retro-base-branch': ['bin/gstack-retro-metrics', 'retro/**', 'test/skill-e2e-retro.test.ts'],
// Global discover
// CSO
'cso-full-audit': ['cso/**', 'test/skill-e2e-cso.test.ts'],
'cso-diff-mode': ['cso/**', 'test/skill-e2e-cso.test.ts'],
@@ -297,8 +295,6 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
'test/skill-e2e-docsync-spawned.test.ts',
],
// Plan completion audit + verification
// Design
'design-consultation-core': ['design-consultation/**', 'scripts/gen-skill-docs.ts', 'test/helpers/llm-judge.ts', 'test/skill-e2e-design.test.ts'],
'design-consultation-existing': ['design-consultation/**', 'scripts/gen-skill-docs.ts', 'test/skill-e2e-design.test.ts'],
@@ -307,8 +303,6 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
'plan-design-review-no-ui-scope': ['plan-design-review/**', 'scripts/gen-skill-docs.ts', 'test/skill-e2e-design.test.ts'],
'design-review-fix': ['design-review/**', 'browse/src/**', 'scripts/gen-skill-docs.ts', 'test/skill-e2e-design.test.ts'],
// Design Shotgun
// /diagram (diagram-render bundle consumers). Triplet = deterministic
// functional (gate); authoring quality = LLM-judged benchmark (periodic).
'diagram-triplet': ['diagram/**', 'lib/diagram-render/**', 'browse/src/write-commands.ts', 'browse/src/read-commands.ts', 'test/skill-e2e-diagram.test.ts'],
@@ -696,8 +690,6 @@ export const E2E_TIERS: Record<string, 'gate' | 'periodic'> = {
'retro': 'periodic',
'retro-base-branch': 'gate',
// Global discover
// CSO — gate for security guardrails, periodic for quality
'cso-full-audit': 'periodic', // D2a demotion 2026-08: 250s/$0.57 full audit; cso targeted tests stay gate
'cso-diff-mode': 'gate',
@@ -827,8 +819,6 @@ export const LLM_JUDGE_TOUCHFILES: Record<string, string[]> = {
'design-review/SKILL.md fix loop': ['design-review/SKILL.md', 'design-review/SKILL.md.tmpl', 'test/skill-llm-eval.test.ts'],
'design-consultation/SKILL.md research': ['design-consultation/SKILL.md', 'design-consultation/SKILL.md.tmpl', 'test/skill-llm-eval.test.ts'],
// Office Hours
// Deploy skills
'land-and-deploy/SKILL.md workflow': ['land-and-deploy/SKILL.md', 'land-and-deploy/SKILL.md.tmpl', 'land-and-deploy/sections/**', 'test/skill-llm-eval.test.ts'],
'canary/SKILL.md monitoring loop': ['canary/SKILL.md', 'canary/SKILL.md.tmpl', 'test/skill-llm-eval.test.ts'],
+1
View File
@@ -730,6 +730,7 @@ describe('Conductor spawned deny (#2733)', () => {
const scriptKind = spawnSync(BIN, [], {
env: { PATH: process.env.PATH ?? '/usr/bin:/bin', ...env },
encoding: 'utf-8',
timeout: 30_000,
}).stdout.trim();
expect(
spawnedByEnv(env),
+11 -7
View File
@@ -36,12 +36,16 @@ describe('session-runner timeout kills the whole process group', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'groupkill-'));
const shimDir = path.join(dir, 'bin');
fs.mkdirSync(shimDir);
// Unique-ish sleep durations double as pgrep markers: they appear only
// in the shim's children's argv, never in this test process's cmdline.
// Markers are unique PER RUN (fractional seconds carry this process's
// pid): sibling Conductor worktrees run free suites concurrently with no
// machine lock, and fixed markers let one run pgrep/pkill the OTHER
// run's shims (review finding — a cross-run flake inside the anti-flake
// tests). GNU sleep accepts decimals, argv stays greppable.
const mark = (n: number) => `${n}.${process.pid}`;
const shim = [
'#!/bin/bash',
'sleep 6041 &', // the orphan-candidate grandchild
'exec sleep 6042', // the shim itself, wedged forever, no NDJSON
`sleep ${mark(6041)} &`, // the orphan-candidate grandchild
`exec sleep ${mark(6042)}`, // the shim itself, wedged forever, no NDJSON
].join('\n');
fs.writeFileSync(path.join(shimDir, 'claude'), `${shim}\n`, { mode: 0o755 });
@@ -71,13 +75,13 @@ describe('session-runner timeout kills the whole process group', () => {
// The kill is SIGKILL on the GROUP: give the OS a beat to reap, then
// require both the wedged shim and its grandchild gone.
await new Promise((r) => setTimeout(r, 1_000));
expect(aliveWithArg('sleep 6042'), 'the fake claude itself survived the timeout kill').toBe(false);
expect(aliveWithArg('sleep 6041'), 'the grandchild ORPHANED — group kill regressed to a direct-child kill').toBe(false);
expect(aliveWithArg(`sleep ${mark(6042)}`), 'the fake claude itself survived the timeout kill').toBe(false);
expect(aliveWithArg(`sleep ${mark(6041)}`), 'the grandchild ORPHANED — group kill regressed to a direct-child kill').toBe(false);
} finally {
process.env.PATH = realPath;
// Belt and braces: never leak the markers into later tests even on
// assertion failure.
spawnSync('pkill', ['-f', 'sleep 604[12]'], { stdio: 'ignore', timeout: 5_000 });
spawnSync('pkill', ['-f', `sleep 604[12]\\.${process.pid}`], { stdio: 'ignore', timeout: 5_000 });
fs.rmSync(dir, { recursive: true, force: true });
}
}, 60_000);
+4 -4
View File
@@ -42,7 +42,7 @@ describe('session-runner two-phase timeout', () => {
'#!/bin/bash',
'sleep 2',
'echo \'{"type":"system","subtype":"init"}\'',
'exec sleep 6071',
`exec sleep 6071.${process.pid}`,
].join('\n') + '\n', { mode: 0o755 });
const realPath = process.env.PATH;
@@ -65,7 +65,7 @@ describe('session-runner two-phase timeout', () => {
expect(wall).toBeLessThan(20_000);
} finally {
process.env.PATH = realPath;
Bun.spawnSync(['pkill', '-f', 'sleep 6071'], { timeout: 5_000 });
Bun.spawnSync(['pkill', '-f', `sleep 6071\\.${process.pid}`], { timeout: 5_000 });
fs.rmSync(dir, { recursive: true, force: true });
}
}, 60_000);
@@ -74,7 +74,7 @@ describe('session-runner two-phase timeout', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'grace-'));
const shimDir = path.join(dir, 'bin');
fs.mkdirSync(shimDir);
fs.writeFileSync(path.join(shimDir, 'claude'), '#!/bin/bash\nexec sleep 6072\n', { mode: 0o755 });
fs.writeFileSync(path.join(shimDir, 'claude'), `#!/bin/bash\nexec sleep 6072.${process.pid}\n`, { mode: 0o755 });
const realPath = process.env.PATH;
process.env.PATH = `${shimDir}:${realPath}`;
@@ -95,7 +95,7 @@ describe('session-runner two-phase timeout', () => {
expect(result.costEstimate.turnsUsed).toBe(0);
} finally {
process.env.PATH = realPath;
Bun.spawnSync(['pkill', '-f', 'sleep 6072'], { timeout: 5_000 });
Bun.spawnSync(['pkill', '-f', `sleep 6072\\.${process.pid}`], { timeout: 5_000 });
fs.rmSync(dir, { recursive: true, force: true });
}
}, 60_000);
+2 -2
View File
@@ -37,7 +37,7 @@ const SCAN_ROOTS = [
'browser-skills',
];
const SYNC_SPAWN = /\b(?:spawnSync|execSync|Bun\.spawnSync)\s*\(/;
const SYNC_SPAWN = /\b(?:spawnSync|execSync|execFileSync|Bun\.spawnSync)\s*\(/;
/** Generous on purpose: multi-line arg arrays push the options object far
* below the call line (observed: +13 lines in codex-model-probe). A wide
* window trades a sliver of false-negative risk for zero rename churn. */
@@ -48,7 +48,7 @@ const EXEMPT_MARKER = /tripwire-exempt:/;
const COMMENT_LINE = /^\s*(?:\/\/|\*|\/\*)/;
/** Shrink-only: lower it when exemptions burn down; never raise it. */
const EXEMPT_CEILING = 6;
const EXEMPT_CEILING = 8;
const SELF = path.join('test', 'spawnsync-timeout-tripwire.test.ts');