diff --git a/.changeset/687-agy-print-timeout.md b/.changeset/687-agy-print-timeout.md new file mode 100644 index 000000000..ea9f4eb48 --- /dev/null +++ b/.changeset/687-agy-print-timeout.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 689 +--- +**`/gsd-review --agy` no longer hangs the whole review on large prompts.** On a big, file-path-rich prompt Antigravity's `agy -p` agentic Cascade can loop on its `code_search`/grep steps and never converge. The invocation now passes agy's own `--print-timeout` flag (its native print-mode cap) so a stalled run self-terminates through the tool's own mechanism; on a non-zero exit any partial output is discarded so the existing transcript fallback / "review failed" stub take over. diff --git a/gsd-core/workflows/review.md b/gsd-core/workflows/review.md index 58edb986c..123da933c 100644 --- a/gsd-core/workflows/review.md +++ b/gsd-core/workflows/review.md @@ -350,8 +350,19 @@ if [ -f "$_AGY_CACHE" ]; then fi fi -# Step 1 — primary invocation: stdout works on macOS, Linux, and WSL -agy -p "$(cat /tmp/gsd-review-prompt-{phase}.md)" 2>/dev/null > /tmp/gsd-review-antigravity-{phase}.md +# Step 1 — primary invocation: stdout works on macOS, Linux, and WSL. +# Bound the run with agy's OWN `--print-timeout` (issue #687). On a large, +# file-path-rich prompt agy's agentic Cascade can loop on its code_search/grep +# steps and never converge; `--print-timeout` is agy's native cap for print mode +# (defaults to 5m — see maintainer note above), so we pass it explicitly to let a +# stalled run self-terminate through the tool's own mechanism. A non-zero exit +# (timeout or crash) discards any partial output so the Step 2 transcript fallback +# / Step 3 stub take over. +agy --print-timeout 300s -p "$(cat /tmp/gsd-review-prompt-{phase}.md)" 2>/dev/null > /tmp/gsd-review-antigravity-{phase}.md +_AGY_RC=$? +if [ "$_AGY_RC" -ne 0 ]; then + : > /tmp/gsd-review-antigravity-{phase}.md +fi # Step 2 — transcript fallback: catches Windows agy -p stdout bug (and any future stdout-silent edge cases). # Reads only lines appended AFTER the pre-flight watermark. If agy failed before writing a new response, diff --git a/tests/bug-687-agy-timeout.test.cjs b/tests/bug-687-agy-timeout.test.cjs new file mode 100644 index 000000000..e75711d67 --- /dev/null +++ b/tests/bug-687-agy-timeout.test.cjs @@ -0,0 +1,45 @@ +// allow-test-rule: source-text-is-the-product +// review.md is a workflow file whose deployed text IS the runtime contract; the +// agy -p invocation cannot be run in CI, so we assert on its content (issue #687). +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const reviewPath = path.resolve(__dirname, '..', 'gsd-core', 'workflows', 'review.md'); +const read = () => fs.readFileSync(reviewPath, 'utf-8'); + +describe('bug #687: agy print mode must be bounded via its native --print-timeout', () => { + test('invokes agy with its own --print-timeout flag (not an external killer)', () => { + assert.match(read(), /agy --print-timeout \d+s? -p "\$\(cat/, + 'review.md must cap agy through `agy --print-timeout -p …` (the tool\'s own mechanism)'); + }); + + test('discards partial output on non-zero exit so the fallback fires', () => { + const c = read(); + assert.match(c, /_AGY_RC.*-ne 0/, 'review.md must check the agy exit code'); + assert.match(c, /: > \/tmp\/gsd-review-antigravity-/, + 'review.md must truncate the output file when agy timed out / failed'); + }); + + test('agy is bounded only by its own --print-timeout, not an external process killer', () => { + const c = read(); + // Print-mode reviewers invoke the tool directly; agy must self-terminate via + // --print-timeout, never via an external SIGKILL/timeout binary wrapped around it. + assert.doesNotMatch(c, /-s KILL/, 'must not SIGKILL agy from the outside'); + // Any external timeout binary wrapping agy — `timeout 300s agy …`, + // `gtimeout 300 agy …`, `timeout -s KILL 300 agy …`. The lookbehind keeps + // agy's own `--print-timeout` flag from tripping it. + assert.doesNotMatch(c, /(? { + // A bare `agy -p "$(cat …)"` with no cap was the original hang. + assert.doesNotMatch(read(), /^agy -p "\$\(cat/m, + 'review.md must not invoke agy -p without --print-timeout'); + }); +});