`/gsd-review --agy` hung indefinitely on large prompts. agy's print mode runs the full tool-enabled agent, and on a big, file-path-rich prompt its agentic Cascade loops on the code_search/grep tool and never converges; the transcript fallback only runs after agy exits, so it can't recover a run that never exits. The agy CLI exposes no per-tool deny (that lives in the Antigravity SDK), but it does expose --print-timeout — agy's native print-mode cap. Pass it explicitly so a stalled run self-terminates through the tool's own mechanism; a non-zero exit discards any partial output so the existing transcript fallback / "review failed" stub take over. Adds a regression test. Closes #687 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
5
.changeset/687-agy-print-timeout.md
Normal file
5
.changeset/687-agy-print-timeout.md
Normal file
@@ -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.
|
||||
@@ -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,
|
||||
|
||||
45
tests/bug-687-agy-timeout.test.cjs
Normal file
45
tests/bug-687-agy-timeout.test.cjs
Normal file
@@ -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 <N> -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, /(?<!print-)\bg?timeout[ \t]+[^\n]*\bagy\b/,
|
||||
'must not wrap agy in an external timeout binary — use its --print-timeout flag');
|
||||
assert.doesNotMatch(c, /kill -9 "\$_AGY/, 'must not use a kill -9 watchdog on agy');
|
||||
});
|
||||
|
||||
test('no unguarded bare "agy -p" invocation remains at line start', () => {
|
||||
// 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');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user