fix(agy-acp): surface swallowed backend errors (quota 429) instead of "(no response)"

[howie]

What problem does this solve?

When the Antigravity backend fails for a whole turn (most commonly an individual
quota exhaustion: RESOURCE_EXHAUSTED / HTTP 429), the user sees a bare
"(no response)" with no indication of what went wrong or that it will recover.
This sends operators down the wrong path (suspecting auth / re-login) when the real
cause is a quota limit that resets on its own.

Root cause: agy --print exits 0 with empty stdout/stderr on a backend failure,
recording the cause only in its own cli.log (and not even in the trajectory DB's
error_details). agy-acp keyed failure detection solely off agy's exit code, so an
exhausted-quota turn looked like a successful-but-empty end_turn.

By contrast claude-agent-acp surfaces backend limit errors to the user (e.g.
"You've hit your org's monthly spend limit ..." as a -32603). This PR brings agy to
parity.

At a Glance

Before:
  agy --print  --(429, exit 0, empty stdout)-->  agy-acp  --> {stopReason: end_turn}  --> user: "(no response)"
                          error only in cli.log  (dropped)

After:
  agy --print  --(429, exit 0, empty stdout)-->  agy-acp
                          error only in cli.log        |
                                                       v  scan this turn's cli-*.log
                                            {error: -32603, message: "...RESOURCE_EXHAUSTED (429):
                                             Individual quota reached ... Resets in 40h..."}  --> user sees the cause

Prior Art & Industry Research

• claude-agent-acp already surfaces backend spend/quota limits as a JSON-RPC
  -32603 with the human-readable cause in the message; this PR makes agy behave the
  same way, so the two adapters render consistently in OpenAB.
• openclaw/acpx src/acp/error-normalization.ts: the canonical peer ACP client
  normalizes opaque agent failures into typed, user-facing errors rather than passing
  through empty turns. The approach here (detect an empty successful turn, recover the
  real cause from the agent's own diagnostics, emit a normalized error) is the same
  pattern applied at the adapter boundary.
• The -32603 + human-readable message convention matches how codex-acp already
  reports its distinct -32603 causes (model deprecation, missing peer dep, auth) in
  this project.

Proposed Solution

Single file: agy-acp/src/main.rs.

1. snapshot_agy_logs(): before spawning agy, record the existing cli-*.log
   filenames under <conversations_dir>/../log, so a log written during this turn
   can be attributed to it.
2. New execute_prompt branch: when agy exits 0 but streamed nothing
   (status.success() && !had_updates), call detect_swallowed_agy_error().
3. detect_swallowed_agy_error(): scan this turn's new cli-*.log (fallback: the
   most recently modified one), read up to the 3 newest candidates, and extract the
   cause via extract_agy_error_message().
4. extract_agy_error_message(): anchor on the most specific terminal error
   (agent executor error: then model unreachable: then RESOURCE_EXHAUSTED), strip
   glog's self-wrapped duplicate tail (<msg>.: <msg>), cap length on a char boundary.
5. If a cause is found, return {"code": -32603, "message": <cause>} instead of the
   empty end_turn. If no matching log line is found, behavior is unchanged (a
   genuinely-empty successful turn still returns end_turn), so there are no false
   positives.

Why This Approach

• The only place agy records the cause is its cli.log; the process exit code,
  stdout/stderr, and the trajectory DB error_details are all empty on a 429
  (verified on a live pod). So log-scraping is the sole viable signal without an
  upstream change to Google's closed agy CLI.
• Gating on success && !had_updates keeps the change conservative: it only converts
  a turn that already produced nothing for the user, and only when a known error
  signature is present.
• Snapshotting logs before the turn avoids surfacing a stale error from a previous
  turn.

Alternatives Considered

• Read the trajectory DB error_details BLOB: rejected, verified empty on quota
  failures; agy doesn't populate it for this class of error.
• Pass a flag via AGY_EXTRA_ARGS to make agy print / exit non-zero on error:
  rejected, agy is a closed-source Google CLI; no such flag is documented and
  print-mode swallowing is its behavior.
• Always treat an empty successful turn as an error: rejected, would misreport
  legitimately-empty responses; requiring a matching log signature avoids false
  positives.
• Fix upstream in agy: not in this project's control; this adapter-level
  normalization is the acpx-style boundary fix.

Validation

Built/tested standalone (the crate is built from its own directory, per
.github/workflows/ci-agy-acp.yml working-directory: agy-acp):

• cargo check: clean (only pre-existing dead_code warnings, none from this change).
• cargo test -- --include-ignored --skip e2e: 19 passed; 0 failed; 1 filtered,
  including 4 new tests:
  • test_extract_agy_error_message_dewraps_quota_error
  • test_extract_agy_error_message_none_for_clean_log
  • test_detect_swallowed_agy_error_reads_new_turn_log
  • test_detect_swallowed_agy_error_none_when_no_logs
• cargo clippy: no new warnings from the added code (remaining warnings are
  pre-existing: adapter.rs complex type / arg count, db.rs &PathBuf, protobuf
  test blob builders, main() collapsible-if).

Manual evidence (live OrbStack pod, image 0.9.0-beta.6-antigravity, agy 1.0.16):
reproduced agy --print "say hi" on an exhausted quota, observed exit 0, empty
stdout/stderr; the RESOURCE_EXHAUSTED (429) ... Resets in 40h... cause was present
only in ~/.gemini/antigravity-cli/log/cli-*.log, which is exactly what this change
now recovers and surfaces.

Note: agy-acp is not a member of the root cargo workspace, so cargo build at the
repo root fails with a workspace error; CI and the release image build it from the
agy-acp/ directory. Validation above was run against an isolated copy of the crate
for the same reason.

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491969620754567270/1523983902849630268

[howie]

Addressed the review in 693bd82. The core insight is that F1, F2, and F5 share one root cause — the snapshot only recorded filenames, so it couldn't tell this turn's bytes from earlier content. Switching the snapshot to name -> byte length resolves all three at once.

#	Finding	Resolution
F1	Stale-log fallback → false positives	Fixed. Removed the "fall back to ALL logs by mtime" branch entirely. A log now qualifies only when its size grew past the pre-turn snapshot offset, so an error logged in an earlier turn is structurally unreachable — not just filtered by a recency heuristic.
F2	Cross-session log leakage	Fixed. Reads start at each file's per-turn offset, so bytes another concurrent session appended before this turn are never scanned. (Went with the "store sizes, parse only appended bytes" option from your suggestion; PID/conversation-ID matching wasn't needed once the offset gate is in place.)
F5	read_to_string unbounded	Fixed. Replaced with a seek-to-offset tail read capped at MAX_LOG_SCAN_BYTES (256 KiB), decoded via from_utf8_lossy. Errors live at the tail, so the cap is safe.
F6	Missing tests	Added 3. char-boundary truncation of a multi-byte message; stale pre-existing-error isolation (writes error, snapshots, appends benign line → asserts None); appended-only read (clean prefix + appended error → asserts detection). cargo test: 22 passed (was 19).
F3	-32603 vs -32000	Left as-is per your note — keeps parity with claude-agent-acp.
F4	Log-path coupling	Left as-is per your note that the existing comment is sufficient for now.

No new clippy warnings (remaining ones are pre-existing: execute_prompt arg count, test blob builders, adapter.rs/db.rs).

[howie]

Final Aggregated Review — PR #1320

Mode

group-review (3/3 voices active: Claude, Codex, Gemini/agy)

Review scope: Dockerfile.unified, agy-acp/Cargo.toml, agy-acp/src/main.rs (plus
.github/workflows/pre-beta-build.yml, which is not touched by this PR but is directly
broken by one of its changes — flagged by Codex R1, confirmed by all 3 voices in R2).

Consensus Critical (must fix)

1. Concurrent-session log scanning can misattribute another session's/turn's error to this turn.
   agy-acp/src/main.rs:207 (snapshot_agy_logs), :238-270 (detect_swallowed_agy_error).
   conversations_dir/its derived log dir is process-wide, shared by every concurrently
   running agy-acp turn (no PID/session component — verified against Adapter::new() and
   the tokio::spawn-per-request dispatch at main.rs:453 with no lock held across
   execute_prompt). A brand-new cli-*.log filename absent from pre_snapshot defaults to
   offset = 0 (unwrap_or(0)), so a concurrent session's entire log content can be scanned
   and, if it matches an anchor, surfaced as this turn's own error — turning a genuinely
   successful, empty turn into a false -32603 failure. All 3 voices independently reached
   this conclusion (Claude via concurrency-dispatch tracing, Codex/Gemini via the offset logic)
   and Codex upgraded its own R1 "Important" to Critical in R2 after seeing the others' analysis.
   Fix: correlate candidate log files to the specific spawned child (e.g. exact launch
   timestamp/PID if available), or otherwise document + accept the residual race.

2. pre-beta-build.yml will fail to build agy-acp after this PR merges.
   .github/workflows/pre-beta-build.yml:109 (not touched by this PR) still runs
   cd agy-acp && printf '\n[workspace]\n' >> Cargo.toml && cargo build --release
   unconditionally. This PR's agy-acp/Cargo.toml change permanently adds a [workspace]
   table to the checked-in manifest, so this workflow step will append a second
   [workspace] table — invalid TOML (duplicate table key) — and cargo build will fail to
   parse the manifest. Confirmed by direct read of the file; all 3 voices agree.
   Fix: apply the same grep -q '^\[workspace\]' Cargo.toml || guard used in
   Dockerfile.unified, or remove the now-redundant append step for agy-acp entirely.

Consensus Important (must fix)

3. Dockerfile.unified shell operator-precedence bug (lines 45, 48).
   cd <dir> && grep -q '^\[workspace\]' Cargo.toml || printf '\n[workspace]\n' >> Cargo.toml && cargo build --release
   — &&/|| share equal, left-to-right precedence, so this parses as
   ((cd && grep) || printf) && cargo build. If cd openab-agent/cd agy-acp fails, the
   shell does not abort: printf still runs (mutating whatever Cargo.toml is in the
   previous working directory) and cargo build --release still runs there too, masking a
   real "directory missing" failure as a misleading pass/fail. Empirically confirmed via
   direct repro in both bash and POSIX sh (cd /nonexistent && ... || printf ... && echo BUILD_RAN → exit 0, BUILD_RAN printed, file mutated). Initially missed by the Claude R1
   sub-review (which only traced the cd-success paths), caught by Gemini R1, and
   independently re-verified by Codex in R2 — full 3/3 agreement after cross-debate.
   Fix: cd <dir> && { grep -q '^\[workspace\]' Cargo.toml || printf '\n[workspace]\n' >> Cargo.toml; } && cargo build --release
   (or split into separate RUN steps).

4. Silent failure paths in the log scanner give no diagnostic signal.
   agy-acp/src/main.rs:163-179 (fallthrough on None), :207-212/:243-245 (read_dir
   failures). Every failure mode — no anchor matched, log dir unreadable, transient
   metadata() failure — converges to identical silence and the same blank
   "(no response)"-equivalent output this PR exists to eliminate, with no way for a future
   maintainer to tell "detection ran and found nothing" from "detection couldn't run." A
   transient metadata() failure during the pre-turn snapshot can also silently omit an
   already-erroring file from pre_snapshot, reintroducing stale-error misattribution.
   Claude and Gemini rate this Important; Codex downgrades to NIT (real but secondary to the
   correctness issues above) — majority holds Important.
   Fix: eprintln! when detect_swallowed_agy_error returns None after a grown-but-unmatched
   candidate was found; match on io::ErrorKind to distinguish NotFound from real errors.

5. Adapter::execute_prompt's new branch is never exercised end-to-end by any test.
   agy-acp/src/main.rs:163-179. All new tests call the pure helpers
   (detect_swallowed_agy_error/snapshot_agy_logs/extract_agy_error_message) directly;
   nothing verifies the actual wiring — that log_pre_snapshot is taken before spawn, the
   !was_cancelled && !had_updates gate is correct, and the JSON-RPC -32603 error is
   actually constructed and returned instead of falling through to the default success path.
   All 3 voices agree.
   Fix: extract the branch's decision logic into a pure function (e.g.
   should_surface_error(was_cancelled, had_updates, status_success, detected) -> Option<JsonRpcResponse>)
   and unit test it directly.

Actionable NIT (must fix — user requires all NITs cleaned up)

6. .take(3) candidate cap and MAX_LOG_SCAN_BYTES tail-seek arithmetic are never exercised
   by any test (all fixtures are single small files). Add a test with >3 grown candidates and
   one with a file exceeding the 256KB cap combined with a non-trivial offset.
7. Doc comment on snapshot_agy_logs/detect_swallowed_agy_error (main.rs:201-205, 231-237) overclaims a "never" isolation guarantee against concurrent sessions that the
   code doesn't actually provide (see Critical perf: cache dependency build layer in Dockerfile #1). Reword once perf: cache dependency build layer in Dockerfile #1 is addressed (or
   immediately, to avoid misleading future maintainers even before the code fix lands).
8. Stale temp directories on test panic (main.rs:644 region) — fs::remove_dir_all cleanup
   is skipped if an assertion panics mid-test, leaking temp dirs. Use a Drop-based guard or
   tempfile::TempDir instead of manual cleanup.
9. (From Step 2 solo /code-review, not re-raised by mob review but still valid, lower
   priority): char-boundary-truncate loop duplicated 4th time in repo (main.rs:304 vs.
   ambient.rs:586, googlechat.rs:1121, feishu.rs:3168); snapshot/diff idiom reimplemented
   a 3rd time vs. adapter.rs:194/streaming.rs:14-44; read_log_tail redundant re-stat;
   internal duplicate cli-*.log filter logic between snapshot_agy_logs and
   detect_swallowed_agy_error; ANCHORS loop does 3 full reverse scans instead of 1;
   Dockerfile.unified:48's grep-guard is now permanently dead for agy-acp given its
   Cargo.toml already declares [workspace] (fold into the Critical perf: cache deps layer + drop arm64 QEMU build #2 fix).

Disputed

None — full 3/3 convergence after R2 cross-debate.

Voices unavailable

None — all 3 voices (Claude, Codex, Gemini/agy) completed R1 and R2.

[dogzzdogzz]

Code review — Claude Code + Codex (consolidated)

Reviewed the diff with two independent passes (Claude Code and Codex). Both agree: the single-session happy path is sound and this is a genuine UX improvement, but the code comments claim a cross-session isolation guarantee that the implementation does not actually provide. No merge blockers; the items below are worth addressing (or explicitly documenting as accepted limitations) before/after merge.

Both reviewers independently verified the two things that look like traps are actually correct — no change needed:

• Dockerfile.unified — grep -q … || printf … && cargo build: POSIX &&/|| are equal-precedence, left-associative, so this parses as (grep || printf) && cargo build. cargo build runs in both the grep-hit and grep-miss cases, and the append only fires when [workspace] is absent. Correct, and consistent with the now-committed [workspace] table in agy-acp/Cargo.toml.
• The char-boundary truncation loop and the lossy-UTF-8 tail read in read_log_tail are safe: end = 500 is only reached when len > 500, end -= 1 cannot underflow, and any U+FFFD from a mid-char start-boundary slice sits before the ASCII anchor, so it's never emitted in the returned message.

Findings (most severe first)

1. [major] Cross-session error mis-attribution — the isolation guarantee is overstated.
agy-acp/src/main.rs (detect_swallowed_agy_error / snapshot_agy_logs).
The doc comments claim detection "never surfaces … a concurrent session that shares this log directory." The snapshot/offset only excludes pre-turn (stale) bytes — it cannot distinguish bytes written by this turn from bytes a concurrent agy session appended in the same window. Each agy --print writes a fresh timestamped cli-<ts>.log, so a concurrent session's brand-new log is absent from pre_snapshot, is treated as "grew from 0", and becomes a candidate.
Failure scenario: session A has a legitimately empty turn (no error) while session B hits quota 429 in the same window; B's log has a newer mtime → A's empty turn surfaces B's "Individual quota reached" error. False positive. Both reviewers confirmed the logic; the trigger depends on whether multiple agy sessions actually share <conversations_dir>/../log — please confirm that assumption. If concurrent sessions are impossible by construction, the fix is just to soften the comment; if they're possible, the attribution needs a stronger key than mtime+offset (e.g. per-invocation log path/pid if agy exposes one).

2. [minor→major] take(3) + mtime ordering can silently drop the erroring log.
detect_swallowed_agy_error: candidates are sorted newest-mtime-first and only the first 3 are scanned. If ≥4 cli-*.log files grew this turn (plausible under concurrency, or with debug logging producing extra files), and the log carrying the anchor is the 4th-newest, its error is never scanned → falls back to "(no response)", defeating the feature. Coarse FS mtime granularity also makes tie ordering nondeterministic. Consider scanning all grown candidates (they're already size-bounded by MAX_LOG_SCAN_BYTES) rather than capping at 3.

3. [minor] Only three hard-coded error anchors.
extract_agy_error_message matches agent executor error:, model unreachable:, RESOURCE_EXHAUSTED. Other backend failure classes (e.g. PERMISSION_DENIED, UNAVAILABLE, auth/network errors logged under a different signature) return None → the user still gets a blank turn. (Hypothesis — depends on agy's log vocabulary.) Reasonable to ship 429-only first, but worth a code comment noting the anchor list is intentionally quota-focused and a TODO to extend.

4. [minor] Premature de-wrap on the first .: .
extract_agy_error_message uses split_once(".: ") to strip glog's self-wrapped duplicate tail. This takes the first occurrence — if the human-readable cause itself contains ".: " before glog's seam, the message is truncated early and the real cause is dropped. Fine for the sampled quota log, fragile to other message text; consider rsplit_once or anchoring on the known duplication pattern.

5. [minor] Log rotation/truncation blind spot.
The meta.len() <= offset filter assumes logs only grow. If agy ever rotates by truncating-in-place, a fresh post-rotation error whose new size hasn't yet passed the recorded pre-turn offset is skipped entirely.

6. [minor] Test coverage gaps.
Strong unit coverage of the four helpers (dewrap, clean-log, char-boundary, stale/append isolation). Not exercised: (a) the new integration branch (!was_cancelled && !had_updates gate + JSON-RPC error assembly) — the actual wiring is untested; (b) the concurrent-session scenario the comment claims to handle (finding 1); (c) the take(3) drop path (finding 2); (d) read_log_tail's MAX_LOG_SCAN_BYTES tail cap via the real file-read path (the char-boundary test hits extract_ on a synthetic string, not the file read).

Bottom line

Ship-worthy for the 429 UX win. Findings 1 and 2 are the ones to resolve or consciously accept — mainly by confirming whether concurrent agy sessions can share the log directory. If they can't, most of this reduces to tightening the comments; if they can, findings 1–2 warrant a more robust log-attribution key.

_{Reviewed by Claude Code (Opus) + Codex. Neither pass compiled the crate; findings are from control-flow tracing of the diff and PR-head source.}

[howie]

Mob review — 3 rounds complete

Scope: Dockerfile.unified, agy-acp/Cargo.toml, agy-acp/src/main.rs, .github/workflows/pre-beta-build.yml (the last one is not touched by the original PR but is directly broken by it — see Critical #2 below).

Round 1 (Claude + Codex + Gemini, full R1+R2 debate) — 693bd82, 1f32b6c

Consensus Critical (fixed):

1. Concurrent-session log scanning could misattribute another session's error to this turn — mitigated with a spawn_time filter (candidate logs must be modified at/after this turn's own agy child was spawned). Doc comments rewritten to describe the residual limitation honestly instead of claiming "never" (agy is closed-source; full isolation isn't possible without per-invocation log separation from agy itself).
2. .github/workflows/pre-beta-build.yml (not touched by the original PR) unconditionally appended [workspace] to agy-acp/Cargo.toml, which the PR already permanently declares — every pre-beta run would produce a duplicate table and fail to parse. Dropped the now-redundant injection there and in Dockerfile.unified.

Consensus Important (fixed):
3. Dockerfile.unified's cd && grep || printf && cargo build had a shell operator-precedence bug: a failed cd still let printf/cargo build run in the wrong directory, masking the real failure. Fixed with { ...; } grouping. Verified via direct repro in both bash and sh before and after.
4. Silent failure paths (read_dir/metadata errors, no-anchor-match) gave zero diagnostic signal. Added eprintln! distinguishing NotFound from real I/O errors.
5. execute_prompt's branch wiring was untested. Extracted decide_turn_error as a pure, directly-unit-tested function.

Round 2 (Claude + Codex + Gemini, full R1+R2 debate) — 3466290

Verified Round 1's fixes and found them incomplete in three ways:

1. detect_swallowed_agy_error still had two silent .ok()? drops (metadata()/modified()) inside its per-candidate closure — fixed to match snapshot_agy_logs's explicit log-and-exclude pattern.
2. read_log_tail collapsed open/seek/read_to_end into one silent None, so a real I/O failure on the log that recorded the actual error was indistinguishable from "read fine, no match" — each step now logs distinctly, and the caller's diagnostic message no longer implies a clean scan when a read actually failed.
3. .take(3) on the candidate scan was a genuine correctness gap (found independently by Codex and the Claude code-reviewer voice): with 4+ logs grown in one turn, the 4th-oldest candidate's error would never be scanned. Removed the cap entirely.

Also: 3 independent reviewers (Claude code-reviewer, Codex, Gemini) each caught, via mutation testing, that my own test_..._scans_beyond_first_candidate test was inverted — it wrote the error-bearing log last (mtime-newest), so it never actually exercised the take(3) boundary it claimed to test. A 4th reviewer pass (pr-test-analyzer, also via mutation testing) found a second fake test (test_read_log_tail_respects_offset_and_cap...) whose single assertion couldn't distinguish "started at offset" from "started at len−cap". Both rewritten; I additionally self-verified via mutation testing (reintroduced take(3), confirmed the rewritten test now fails, reverted).

Round 3 (Codex spot-check) — 418e58e

Found: on filesystems that truncate mtime to whole seconds, this turn's own log (written a few hundred ms after spawn_time) could appear to predate spawn_time and get wrongly excluded — silently reintroducing "(no response)" on every affected turn, not just under concurrency. Fixed with a 1s tolerance (mtime + 1s < spawn_time), since a false negative here is worse than the already-acknowledged, narrower concurrent-misattribution risk.

Current state

• cargo test: 33 passed, 0 failed.
• cargo clippy: no new warnings (6 pre-existing, unrelated to this PR).
• 3 commits since the original PR: 1f32b6c, 3466290, 418e58e.

Per this project's mob-review circuit breaker (3 rounds before pausing for human judgment), stopping here for human review rather than kicking off a 4th round automatically. No outstanding Critical/Important findings from any voice at this point.

[chaodu-agent]

LGTM ✅ — Well-engineered adapter-boundary error surfacing; all prior review findings addressed.

What This PR Does

When the agy backend silently fails (e.g. quota 429 / RESOURCE_EXHAUSTED), the user previously saw a bare "(no response)" with no actionable signal. This PR detects the swallowed error by scanning cli-*.log files created during the turn and surfaces it as a JSON-RPC -32603 error — matching claude-agent-acp and codex-acp behavior.

How It Works

1. Snapshot existing cli-*.log file sizes before spawning agy
2. Gate on exit 0 && !had_updates (empty successful turn)
3. Scan logs that grew during this turn (offset-based, newest first, no arbitrary cap) for known error anchors (agent executor error: → model unreachable: → RESOURCE_EXHAUSTED)
4. Filter candidates by spawn_time (±1s tolerance for coarse-mtime filesystems)
5. De-wrap glog's self-duplicated tail (<msg>.: <msg>) and cap at 500 chars on a char boundary
6. Return -32603 JSON-RPC error via extracted decide_turn_error() function

Findings

#	Severity	Finding	Location
1	🟢	Clean pipeline architecture: snapshot → detect → extract → decide, each pure/testable	main.rs
2	🟢	Conservative gating (success && !had_updates) eliminates false positives on normal turns	main.rs:151-160
3	🟢	Offset-based scanning excludes stale pre-turn errors (prior mob-review finding, fixed)	main.rs:197-270
4	🟢	All candidates scanned — no take(N) cap (prior mob-review finding, fixed)	main.rs:270
5	🟢	1s mtime tolerance for coarse-grained filesystems (Round 3 finding, fixed)	main.rs:250-258
6	🟢	Every I/O path has specific diagnostics (eprintln!); read failures distinguishable from "no match"	throughout
7	🟢	14 new tests cover edge cases end-to-end including multi-candidate, offset vs cap, mtime tolerance	mod tests
8	🟢	[workspace] in Cargo.toml + removal of duplicate injection in CI/Docker — coherent build fix	Cargo.toml, Dockerfile.unified, pre-beta-build.yml
9	🟢	Cross-session misattribution limitation honestly documented; accepted since agy is closed-source	doc comments

Addressing External Reviewer Feedback

@dogzzdogzz (Claude Code + Codex consolidated review)

Cross-session error mis-attribution — the isolation guarantee is overstated

✅ Addressed in 418e58e: Doc comments rewritten to honestly describe the residual limitation rather than claiming "never". The spawn_time filter and offset-based scanning narrow the window significantly, and the remaining gap requires per-invocation log isolation from agy itself (closed-source, not available).

take(3) + mtime ordering can silently drop the erroring log

✅ Addressed in 3466290: The take(3) cap was removed entirely. All grown candidates are now scanned (newest first). A dedicated test (test_detect_swallowed_agy_error_scans_all_grown_candidates_regardless_of_position) regression-guards this.

Only three hard-coded error anchors

ℹ️ Accepted: Intentionally quota-focused for v1. Other backend failure classes (PERMISSION_DENIED, UNAVAILABLE) can be added as anchors when observed in the wild. The design (ordered anchor list with last-match-wins) supports extension without restructuring.

Premature de-wrap on the first .:

ℹ️ Accepted: For the known glog format, split_once(".: ") correctly hits the duplication seam. The sampled error messages from agy's cli.log do not contain .: before the seam. If edge cases are found in production, switching to rsplit_once is a one-line fix.

Log rotation/truncation blind spot

ℹ️ Accepted: agy uses timestamped log files (cli-<ts>.log), not in-place truncation/rotation. Each invocation creates a fresh file. The grow-only assumption is valid for agy's observed behavior.

Test coverage gaps (integration branch wiring, concurrent scenario)

✅ Addressed in 3466290: decide_turn_error extracted as a pure function with full truth-table tests (cancelled, had_updates, non-zero exit with/without stderr, swallowed error, clean success). The concurrent-session scenario is acknowledged as untestable without agy itself (closed-source).

Mob review (3 rounds by @howie)

All Critical and Important findings from Rounds 1-3 verified addressed in commits 1f32b6c, 3466290, 418e58e.

Baseline Check

• PR opened: 2026-07-07
• Main already has: zero swallowed-error detection in agy-acp (relies solely on exit code)
• Net-new value: full error-surfacing pipeline bringing agy-acp to parity with claude-agent-acp and codex-acp for backend failure UX

What's Good (🟢)

• Adapter-boundary pattern matches industry best practice (openclaw/acpx error normalization)
• Conservative gating ensures zero false positives on genuinely empty turns
• Excellent test design: mutation-tested, regression-guarding specific past findings
• Thorough documentation of known limitations and design tradeoffs
• Build fix (workspace declaration) resolves a real CI/Docker failure mode