mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-12 16:08:59 +02:00
fix(review,ship): run the codex diff passes under the timeout wrapper (#1036)
The `_gstack_codex_timeout_wrapper` added in #1056 was wired into codex/SKILL.md but never into the /review and /ship diff passes, which kept running under a bare 5-minute Bash gate. An unwrapped stall returns no exit code and no output, which downstream reads as "Codex reviewed and found nothing" — a truncated pass silently became a clean bill. Measured on codex-cli 0.145.0: a pass was killed at 287s of a 300s budget mid-tool-call, and the same prompt completed in 336s. Both passes in scripts/resolvers/review.ts (adversarial `codex exec` and the structured `codex review --base` pass) now re-source gstack-codex-probe and run under `_gstack_codex_timeout_wrapper 540`, with the Bash tool gate raised to 600000 ms so the wrapper fires FIRST and a stall surfaces as a diagnosable exit 124. The timeout guidance now says a timed-out pass is MISSING COVERAGE, not a clean result, and points at the run's rollout log under ~/.codex/sessions/ for partial output. The stale "timeout doesn't exist on macOS" claim is gone — the wrapper resolves gtimeout, then timeout, then runs unwrapped, so it is safe without coreutils. Static guards in test/codex-hardening.test.ts pin all three sites (resolver, review/SKILL.md, ship/sections/adversarial.md): both calls wrapped, wrapper budget strictly under the Bash gate, and no reappearance of the macOS claim that steered these call sites away from the wrapper in the first place. The Claude-output path guard in test/gen-skill-docs.test.ts now scrubs ~/.codex/sessions/ (a user-facing Codex CLI path, same class as the ~/.codex/logs/ exemption) before banning Codex host paths. Generated files regenerated via gen:skill-docs; factory golden refreshed. Contributed by @aegixx (PR #2379). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
8c5bb4545b
commit
fad28d81ce
@@ -1944,16 +1944,21 @@ describe('Codex generation (--host codex)', () => {
|
||||
const content = fs.readFileSync(path.join(ROOT, 'review', 'SKILL.md'), 'utf-8');
|
||||
expect(content).toContain('.claude/skills/review/checklist.md');
|
||||
expect(content).toContain('~/.claude/skills/gstack');
|
||||
// Must NOT contain Codex paths
|
||||
// Must NOT contain Codex HOST paths. `~/.codex/sessions/` is exempt: the
|
||||
// timeout-wrapper guidance documents the Codex CLI's own rollout-log
|
||||
// location (a user-facing CLI path, same class as ~/.codex/logs/ in the
|
||||
// codex skill), not the gstack Codex host install path.
|
||||
expect(content).not.toContain('.agents/skills');
|
||||
expect(content).not.toContain('~/.codex/');
|
||||
expect(content.replaceAll('~/.codex/sessions/', '')).not.toContain('~/.codex/');
|
||||
});
|
||||
|
||||
test('Claude output unchanged: ship skill still uses .claude/skills/ paths', () => {
|
||||
const content = readShipUnion();
|
||||
expect(content).toContain('~/.claude/skills/gstack');
|
||||
expect(content).not.toContain('.agents/skills');
|
||||
expect(content).not.toContain('~/.codex/');
|
||||
// ~/.codex/sessions/ is the Codex CLI's rollout-log path (user-facing),
|
||||
// documented by the adversarial-pass timeout guidance — see review test above.
|
||||
expect(content.replaceAll('~/.codex/sessions/', '')).not.toContain('~/.codex/');
|
||||
});
|
||||
|
||||
test('Claude output unchanged: all Claude skills have zero Codex paths', () => {
|
||||
@@ -1962,9 +1967,11 @@ describe('Codex generation (--host codex)', () => {
|
||||
// pair-agent legitimately documents how Codex agents store credentials.
|
||||
// codex + autoplan document the Codex CLI auth file (~/.codex/auth.json)
|
||||
// and log path (~/.codex/logs/) — those are user-facing Codex CLI paths,
|
||||
// not the gstack Codex host install path.
|
||||
// not the gstack Codex host install path. ~/.codex/sessions/ (rollout
|
||||
// logs, referenced by the review/ship timeout guidance) is the same
|
||||
// user-facing class, so it is scrubbed before the ban.
|
||||
if (skill.dir !== 'pair-agent' && skill.dir !== 'codex' && skill.dir !== 'autoplan') {
|
||||
expect(content).not.toContain('~/.codex/');
|
||||
expect(content.replaceAll('~/.codex/sessions/', '')).not.toContain('~/.codex/');
|
||||
}
|
||||
// gstack-upgrade legitimately references .agents/skills for cross-platform detection
|
||||
if (skill.dir !== 'gstack-upgrade') {
|
||||
|
||||
Reference in New Issue
Block a user