fix(agent): build/verify failure diagnosis — timeout classify, claude-binary Exec-format, failing-line surfacing#597
Conversation
…A-667, was 'Unexpected error') Replication finding: a live fork coding child (ABCA-667) failed with 'TimeoutExpired: Command [mise run build] timed out after 600 seconds' — the full ~2800-test fork build exceeded the 600s cap. That's the pre-agent verify-build subprocess timeout, which lands raw in error_message and fell through to 'Unexpected error' + generic guidance — exactly the transient-vs-real confusion this rework targets. It's not a crash and not broken code: the build ran too long. Add a TIMEOUT classification for / → 'Build/tests didn't finish in time', user class, retryable, remedy names the real fix (retry / raise BUILD_VERIFY_TIMEOUT_S or move to the larger ECS build box). Ordered before the generic poll-timeout pattern. Test uses the exact ABCA-667 shape. 60 classifier tests green. (cherry picked from commit 6394a59) (cherry picked from commit b9c1a94)
… (Exec format error) claude-code@2.1.191 uses a shim (bin/claude.exe) + platform-optional-dependency install model: its postinstall (install.cjs) is supposed to replace the shim with the platform-native ELF binary. When that postinstall silently falls back (e.g. it hits EACCES overwriting the shim under npm's root de-privileging), the error-shim stays on PATH and every agent run dies at the run_agent step with 'OSError: [Errno 8] Exec format error: \'claude\'' — even though the native arm64 binary is present in the image as an optional dep, just never wired up. Live-caught on ABCA-659's retry: all 3 ECS runs failed this way on a freshly rebuilt image (the prior working image ran the older single-binary 2.1.142). Re-run install.cjs explicitly after the npm install so the native binary is placed at build time, and hard-verify with 'claude --version' so a still-broken shim fails the image build instead of every task at runtime. (cherry picked from commit 7cb4a87) (cherry picked from commit 7de778b)
…debuggable The post-agent build gate logged only stderr on a non-zero exit. But build/test tooling (jest, tsc, the mise task DAG) writes the ACTUAL failing-task error to STDOUT, not stderr — stderr often carries only the runner's plan echo. So a red `mise run build` surfaced every task STARTING in CloudWatch but never WHICH one failed or why (ABCA-662: a 7s exit-1 with zero captured cause, indistinguishable from a phantom, and the reason Linear showed a build-fail GitHub CI passed). run_cmd now also tails the last 20 lines of stdout on failure (redacted — repo build output is untrusted). 4 tests pin: stdout surfaced, tailed-not-headed, redacted, and NOT dumped on success. (cherry picked from commit 67d84db) (cherry picked from commit bb578fd)
…ust a tail The stdout-on-failure logging (67d84db) tailed the last 20 lines — enough for a serial command, but NOT for `mise run build`, which runs 4 packages in PARALLEL and interleaves their output. The failing task's error lands in the MIDDLE while the tail captures whatever finished LAST (e.g. a PASSING package's coverage table) — so the log showed a green coverage table on a red build and named nothing (ABCA-662: three verification runs where the actual failing sub-task was never visible in CloudWatch). run_cmd now scans the WHOLE stdout for failure-signature lines (FAIL/✕/●/error TS/ELIFECYCLE/coverage-threshold/mise 'no task'/'task failed'/traceback/…), filters benign noise ('0 errors', '--no-error'), and logs those FIRST, then a trailing tail for context — deduped, capped at 40 lines. Falls back to the tail when nothing matches (unknown tool). This is the tooling gap that made the whole ABCA-662 build investigation slow: every failure was diagnosable only by luck or local repro. 5 tests incl. the mid-DAG-failure + coverage-threshold cases. (cherry picked from commit 3fe99c9) (cherry picked from commit aaa4f18)
isadeks
added
bug
scottschreckengaust
left a comment
scottschreckengaust
left a comment
There was a problem hiding this comment.
Review — Principal Architect (Stack A: ECS substrate, PR 3/4)
Base verified against fresh origin PR head (diffed vs #596). Mandatory agents run stack-wide (full table on the #596 review).
Verdict: Approve (with a nit)
Solid build/verify-diagnosis work, all three changes tied to live incidents:
_surface_failure_lines(agent/src/shell.py) — scans the WHOLE stdout for failure-signature lines, not just the tail. Correct for a parallel mise DAG that interleaves output so the failing task's line lands in the middle while the tail is a passing package's coverage table (ABCA-662). Capped at 40 + a 15-line tail, deduped, and redacted (redact_secrets) since repo build output is untrusted. Thereturn out or tailfallback guarantees a failure never logs nothing. Verifiedrun_cmd'scheck=Truepath still raises — the added stdout logging is purely additive, not a swallow.- Error-classifier TIMEOUT pattern (
cdk/src/handlers/shared/error-classifier.ts) — classifies a build/verify wall-clock kill distinctly from a crash (retryable: true, user-actionable remedy). Verified it's ordered before the generic catch-all and isn't shadowed by the earlierpoll timeout exceededrule. - Dockerfile claude
install.cjsre-run +claude --version— a real fix: claude-code@2.1.x ships an error-shim that its postinstall can silently leave in place, dying at runtime with "Exec format error". Re-running the installer and hard-verifying it exec's fails the build instead of the task.
Tests (test_shell.py::TestRunCmdFailureLogging, error-classifier.test.ts) cover the key cases well: stdout-logged-on-failure, mid-DAG line surfaced, coverage-threshold surfaced, benign "0 errors" not falsely elevated, redaction, success-doesn't-dump, no-marker→tail fallback, and the timeout classified as retryable-not-Unexpected.
Non-blocking nit
- N5 — Broad failure markers.
_FAILURE_LINE_MARKERSincludes broad substrings ("error:","failed") that could surface benign advisory lines. It's logging-only, capped, redacted, and the_FAILURE_LINE_NOISEallowlist covers common false positives — so worst case is slightly noisier CloudWatch, no functional impact. Flagging for awareness only.
Governance note
Branch pr/agent-build-robustness carries no issue number and the body references only stacked PRs (ADR-003). Same governance item raised on #596 — please attach an approved issue and a conforming branch name before merge. Code is approve-ready.
CI
All green (title, CodeQL ×3, dead-code advisory, secrets/deps/actions, build (agentcore)); MERGEABLE. No CDK construct/stack change → bootstrap synth-coverage N/A.
🤖 Generated with Claude Code
|
Thanks for the approve. On N5 (broad |
isadeks
left a comment
isadeks
left a comment
There was a problem hiding this comment.
Cross-reference: Dockerfile fix overlaps #613 (and complements #599)
Flagging a coordination point surfaced while reviewing #613 — the claude-binary Exec format error is being addressed across three open PRs:
- This PR (#597) and #613 both append
node install.cjsto the sameRUN … npm --prefix … update …line inagent/Dockerfile, so they will textually conflict — both can't merge as-is. #597's version is the superset: it also adds&& claude --version, hard-gating the image build so a silent postinstall fallback failsdocker buildrather than every task at runtime (strictly better than restore-and-hope), plus a comment explaining the shim mechanism. - #599 is complementary, not conflicting — it classifies the same
Exec format errorinto a user-facing message (error-classifier.ts, disjoint file). Prevention (here/#613) + legibility (#599) are the two halves of ABCA-659.
Suggested resolution: since #597's Dockerfile hunk supersedes #613's, either close #613 in favor of this PR, or — if this PR stays blocked behind its stack (#596 → #503; #503 merged, #596 still open) and the exec-format break is hurting people now — let #613 merge against main and drop the Dockerfile hunk here on rebase. Whoever lands first wins the line; the other should rebase around it.
(I verified install.cjs is idempotent and exists in @anthropic-ai/claude-code@2.1.191; couldn't reproduce the npm update clobber on linux/amd64, so it looks arm64/AgentCore-specific — consistent with the live-caught reports. The && claude --version gate here is the right belt-and-braces regardless.)
2 participants
Summary
Platform-agnostic robustness fixes for the agent's build/verify path, surfaced dogfooding ABCA-on-ABCA on the ECS build box but not ECS-specific — they improve failure diagnosis for any repo/substrate.
What changed
fix(errors): classify a build/verify command TIMEOUT — aTimeoutExpiredfrom a long build previously fell through to a bare "Unexpected error"; now classified as a user-actionable timeout (retry / raiseBUILD_VERIFY_TIMEOUT_S),retryable: true. Live-caught (ABCA-667).fix(agent): place the claude-code native binary at image build — claude-code@2.1.x ships an error-shim atbin/claude.exethat its postinstall replaces with the platform-native ELF; the postinstall silently fell back at build time, leaving the shim on PATH → every agent died withExec format error. Now re-runinstall.cjs+ hard-verifyclaude --versionso a broken shim fails the image build, not every task.fix(agent): log stdout onrun_cmdfailure — build-gate failures were undebuggable without the command output.fix(agent): surface the failing line from a parallel build DAG — a fan-outmise run buildfailure showed only a tail; now surfaces the line that actually names the failure.Testing
mise run buildgreen (CDK + agent + CLI); new/updated tests inerror-classifier.test.ts,test_shell.py.claude --version→ real 2.1.191, not the error shim).Notes for reviewers