fix(agent): build/verify failure diagnosis — timeout classify, claude-binary Exec-format, failing-line surfacing

[isadeks]

Stacked on #596 (pr/ecs-substrate-hardening) → #503. Merge those first; this PR then retargets. Diff shown is against #596's branch.

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 — a TimeoutExpired from a long build previously fell through to a bare "Unexpected error"; now classified as a user-actionable timeout (retry / raise BUILD_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 at bin/claude.exe that its postinstall replaces with the platform-native ELF; the postinstall silently fell back at build time, leaving the shim on PATH → every agent died with Exec format error. Now re-run install.cjs + hard-verify claude --version so a broken shim fails the image build, not every task.
• fix(agent): log stdout on run_cmd failure — build-gate failures were undebuggable without the command output.
• fix(agent): surface the failing line from a parallel build DAG — a fan-out mise run build failure showed only a tail; now surfaces the line that actually names the failure.

Testing

• Full mise run build green (CDK + agent + CLI); new/updated tests in error-classifier.test.ts, test_shell.py.
• Live-verified: the arm64-native-claude fix confirmed in the deployed image (claude --version → real 2.1.191, not the error shim).

Notes for reviewers

• No compute-backend coupling — pure agent-runtime + error-classifier changes. Stacked here only because they were validated together with the ECS substrate work.

[scottschreckengaust]

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. The return out or tail fallback guarantees a failure never logs nothing. Verified run_cmd's check=True path 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 earlier poll timeout exceeded rule.
• Dockerfile claude install.cjs re-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_MARKERS includes broad substrings ("error:", "failed") that could surface benign advisory lines. It's logging-only, capped, redacted, and the _FAILURE_LINE_NOISE allowlist 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}

[isadeks]

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.cjs to the same RUN … npm --prefix … update … line in agent/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 fails docker build rather 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 error into 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.)

The base branch was changed.

[scottschreckengaust]

Review — Principal Architect (re-review vs. clean main)

My earlier review here is dismissed — it was diffed against #596's branch. Both parents are now merged (#503, #596), so this re-review is against a clean main base; the diff is exactly the 5 files described in the body. #599 (the complementary Exec format error classifier) also merged since — I re-verified there's no overlap/shadowing. Mandatory agents re-run: code-reviewer, silent-failure-hunter, pr-test-analyzer, comment-analyzer. All findings independently verified against real code (I did not take agent output at face value — the loudest agent finding turned out wrong; see below).

Verdict: Request changes — one narrow blocker; everything else is approve-ready

The core work is genuinely good and each change is tied to a live incident. The blocker is not a crash — it's that this PR's own feature (accurate, actionable error legibility) ships a user-facing remedy that points an operator at a lever that does nothing, under a comment whose causal narrative the code contradicts. On a repo whose backing issue is literally titled "honest failure classification" (#610, approved), that's a contract defect, not a nit.

---

Blocking

B1 — The new TIMEOUT remedy misdirects the operator to a non-functional knob, and its anchoring comment is contradicted by the code. cdk/src/handlers/shared/error-classifier.ts:440-450

Two linked, verified problems in the new classification block:

1. BUILD_VERIFY_TIMEOUT_S is consumed nowhere in agent/. grep -rn BUILD_VERIFY_TIMEOUT_S agent/ returns nothing. verify_build/verify_lint (agent/src/post_hooks.py:19,39) call run_cmd(["mise","run","build"], check=False) with no timeout=, so they always use run_cmd's hardcoded timeout: int = 600 (agent/src/shell.py:244). The env var is only set (CDK ecs-agent-cluster.ts, value 3600) and asserted in a test — nothing reads it. So the remedy line "an admin can raise BUILD_VERIFY_TIMEOUT_S" tells an operator to turn a knob that isn't wired to anything.
2. The comment's live-caught narrative is contradicted by the code. The comment (lines 438-444) says this was "live-caught on ABCA-667: the full mise run build exceeded the 600s default and surfaced as a bare 'Unexpected error'." But post_hooks.py:25,45 catch subprocess.TimeoutExpired, log "… timed out — treating as failed", and return False — i.e. the verify path converts a build timeout into a returncode failure and never emits a raw TimeoutExpired … timed out after N seconds string. So the regex cannot fire for the mise run build verify path it names; it only fires for some other, unwrapped run_cmd timeout (clone/setup/etc.).

The classification itself (TIMEOUT, retryable: true, not "Unexpected error") is a real improvement and worth keeping — the defect is the operator-facing copy + the comment. Two acceptable fixes: (a) wire verify_build/verify_lint to read BUILD_VERIFY_TIMEOUT_S and pass it as timeout= (makes the remedy true), or (b) reword the comment to describe the pattern generically ("any uncaught run_cmd TimeoutExpired") and drop/soften the BUILD_VERIFY_TIMEOUT_S promise in the remedy. Either closes it; (b) is the smaller change if the wiring is out of scope. Please also reconcile the "600s default" here vs. the "1800s default" comment at ecs-agent-cluster.ts while you're in there.

---

Non-blocking (nits)

• N1 — errorClass omitted on the new pattern. error-classifier.ts:446-452 is the only entry in PATTERNS that omits errorClass. This is functionally fine (absent ⇒ defaults to USER, isTransientError() false ⇒ no auto-retry — which is arguably correct: a slow build re-run unchanged just times out again). But it's implicit against the file's own "New PATTERNS should always set it" convention (line 80-82). Add errorClass: ErrorClass.USER with a one-line comment stating the no-auto-retry intent, so a future maintainer doesn't "fix" it to TRANSIENT and start auto-retrying doomed builds. (I disagree with the reviewer suggestion to make it TRANSIENT — auto-retrying an unchanged slow build is wrong.)

• N2 — Regex misses float-formatted, non-"build" timeouts. error-classifier.ts:445: timed out after \d+ ?s won't match … after 600.0 seconds. In practice this is theoretical — run_cmd's timeout is int-typed so Python always renders N seconds, and the 600.0/build case still matches via the second alternative. Cheap hardening: \d+(?:\.\d+)? ?s(econds)?. (Flagging because an agent called this a blocker — it isn't.)

• N3 — One misleading test gives false confidence. agent/tests/test_shell.py:227 test_benign_zero_errors_line_not_surfaced_as_failure uses eslint: 0 errors, 0 warnings, which matches no entry in _FAILURE_LINE_MARKERS, so the _FAILURE_LINE_NOISE allowlist guard (shell.py:88-90) is never exercised — the test passes identically if the allowlist is deleted. The genuinely uncovered branch: a line that hits a marker and a noise term (e.g. error: produced 0 errors after retry) positioned outside the tail window. Not blocking, but worth a real assertion since the allowlist is load-bearing.

• N4 — Truncation cap uncovered. shell.py _MAX_SURFACED_FAILURE_LINES (40) truncation branch is never reached by the tests (the coverage-table test surfaces ~16 lines). Defensive-only; low priority. A >40-marker-line test would pin it.

• N5 (carried, waived by author) — Broad _FAILURE_LINE_MARKERS substrings (error:, failed). Logging-only, capped, redacted, allowlisted; author's "add to allowlist rather than narrow markers" reasoning is sound. Awareness only.

---

Merge-coordination note (not a #597 defect)

#613 is still OPEN and edits the same agent/Dockerfile RUN line — the two will textually conflict. #597's hunk is the superset (it adds && claude --version to hard-gate the image build). Whoever merges first wins the line; the other rebases. Suggest closing #613 in favor of this, or dropping #597's Dockerfile hunk on rebase if #613 lands first. (The author already flagged this in a PR comment — captured here for the record.)

---

What's solid (verified, no action)

• shell.py _surface_failure_lines + stdout-on-failure logging — silent-failure-hunter + my own trace confirm the change is purely additive: the if result.stdout: block sits before if check:, and raise RuntimeError(...) (line 282) is byte-for-byte unchanged — no raising path became logged-only. return out or tail guarantees a failure never logs nothing (empty-stdout branch is unreachable behind if result.stdout:). stdout is redact_secrets-wrapped (double-redacted via log()). Correct fix for the ABCA-662 interleaved-DAG tail problem.
• Dockerfile install.cjs re-run + claude --version gate — accurate comment, correct RUN; a broken postinstall shim now fails docker build instead of every task. Local exec check, no network/auth. Strictly better than restore-and-hope.
• #599 overlap — its Exec format error classifier is complementary (classification) to this PR's Dockerfile prevention; no shadowing. The new TIMEOUT pattern is the first timed out matcher in the array and sits before poll timeout exceeded — no ordering conflict.

Governance

Approved backing issue #610 (bug+approved) covers this error-legibility work — governance satisfied. Branch pr/agent-build-robustness doesn't encode the issue number (ADR-003 branch-name convention) — a de-facto-waived nit, not a blocker.

CI

All green (title, CodeQL ×3, dead-code advisory, secrets/deps/actions, build (agentcore)); MERGEABLE but BLOCKED on required review. No CDK construct/stack change → bootstrap synth-coverage N/A.

Review agents run

code-reviewer ✓ · silent-failure-hunter ✓ · pr-test-analyzer ✓ · comment-analyzer ✓. Omitted: type-design-analyzer (no new types — one regex pattern object + one module-level helper), security-review/IAM (no IAM/Cedar/network/secrets surface — the only security-adjacent line, secret redaction of newly-surfaced stdout, was checked by silent-failure-hunter and holds; note redact_secrets doesn't mask AWS keys, a pre-existing gap this PR widens slightly — worth a follow-up, out of scope here).

Human heuristics

• Proportionality — pass. A regex pattern + a ~30-line helper for a real, recurring diagnosis gap; no over-abstraction.
• Coherence — pass. Changes land in the right packages (agent/ runtime, cdk/ classifier) per the routing table; consistent with existing PATTERNS/run_cmd idioms.
• Clarity — concern (B1): the TIMEOUT remedy surfaces a plausible-but-misleading action (a dead knob) — exactly the AI004 "hides failure behind a plausible default" failure mode, here inverted into a plausible-but-wrong remedy.
• Appropriateness — pass. Tests assert on observed log output (contract), not internal calls; the MIDDLE-line test genuinely reproduces the ABCA-662 root cause. (One misleading test — N3.)

_{🤖 Generated with Claude Code}

[isadeks]

Review — build/verify failure diagnosis (#597)

Re-reviewed against clean main (both stack parents #503/#596 merged; #599 also merged). Full diff is the 5 files described. I verified the code paths by reading them at the current head; agent suite 29/29 green, build (agentcore) green.

Overview. Four independent, incident-tied robustness fixes to the agent's build/verify path: (1) classify a build/verify TimeoutExpired as a user-actionable TIMEOUT instead of "Unexpected error"; (2) re-run install.cjs + claude --version in the Dockerfile so a broken claude-code shim fails the image build not every task; (3) log stdout on run_cmd failure; (4) surface the failing line from the middle of an interleaved parallel-DAG log. Each maps to a live incident (ABCA-662/667). Scope is tight and proportional.

Confirming @scottschreckengaust's blocker — B1 is correct (verified independently)

I traced both halves against the code at head; both hold:

1. BUILD_VERIFY_TIMEOUT_S is read nowhere in agent/. It's only set in CDK (ecs-agent-cluster.ts:191 = 3600) and asserted in a test. verify_build/verify_lint (post_hooks.py:19,39) call run_cmd(["mise","run","build"], check=False) with no timeout=, so they use run_cmd's hardcoded timeout: int = 600 (shell.py:244). So the new remedy — "an admin can raise BUILD_VERIFY_TIMEOUT_S" (error-classifier.ts:450) — points the operator at a lever wired to nothing.
2. The comment's causal story is contradicted by the code. verify_build/verify_lint catch subprocess.TimeoutExpired (post_hooks.py:25,45), log "timed out — treating as failed", and return False — they convert a build timeout into a returncode failure and never emit the raw TimeoutExpired … timed out after N seconds string. So the new regex cannot fire for the mise run build verify path the comment names as its ABCA-667 origin; it only matches some other unwrapped run_cmd timeout (clone/setup).

On a repo whose backing issue (#610) is "honest failure classification," shipping a remedy that misdirects and a comment the code contradicts is a genuine contract defect, not a nit — agreed it blocks. The classification itself (TIMEOUT / retryable: true / not "Unexpected error") is a real improvement worth keeping.

Fix — I'll take Scott's option (b) plus reconciliation (smaller than wiring the env var, which is arguably out of scope for this PR):

• Reword the comment to describe the pattern generically ("any uncaught run_cmd TimeoutExpired", e.g. clone/setup), dropping the ABCA-667-mise-build narrative.
• Soften the remedy: drop the BUILD_VERIFY_TIMEOUT_S promise (or make it conditional/"if wired") since nothing reads it; keep "retry / move to the larger ECS build compute."
• Reconcile the "600s default" here vs the "1800s" comment at ecs-agent-cluster.ts while in there.
• (If wiring verify_build/verify_lint to read BUILD_VERIFY_TIMEOUT_S and pass timeout= is preferred, that makes the remedy true and is the better long-term fix — but it's a behavior change beyond this PR's stated scope. I'll do (b) unless a maintainer wants (a).)

Nits — all verified accurate, will address

• N1 — the new pattern is the only PATTERNS entry omitting errorClass (confirmed: 28 classification blocks, 27 with errorClass), against the file's own convention. Absent ⇒ USER + no auto-retry, which is correct here (auto-retrying an unchanged slow build just times out again). Will add errorClass: ErrorClass.USER with a comment stating the deliberate no-auto-retry intent, so nobody "fixes" it to TRANSIENT.
• N3 — test_benign_zero_errors_line_not_surfaced_as_failure uses eslint: 0 errors, 0 warnings, which matches no _FAILURE_LINE_MARKERS entry (error: ≠ errors), so the _FAILURE_LINE_NOISE allowlist guard is never exercised — the test passes even if the allowlist is deleted. Will add a real case: a line hitting both a marker and a noise term, outside the tail window, asserting it's dropped.
• N2 (float-format regex), N4 (untested 40-line truncation cap) — low-priority hardening; will fold N2's \d+(?:\.\d+)? in with B1 and add an N4 test.

Solid (verified, no action)

• _surface_failure_lines + stdout-on-failure logging is purely additive: the if result.stdout: block precedes if check: and the raise RuntimeError(...) is unchanged — no raising path became logged-only; return out or tail guarantees a failure never logs nothing; stdout is redact_secrets-wrapped. Correct fix for the interleaved-DAG tail problem.
• Dockerfile install.cjs + claude --version gate — accurate comment, correct RUN, local-exec only. Strictly better than restore-and-hope.
• #599 overlap is complementary (its Exec format error classifier vs this PR's Dockerfile prevention); the new TIMEOUT pattern sits before poll timeout exceeded, no ordering conflict.

Coordination

• #613 still edits the same Dockerfile RUN line — this PR's hunk is the superset (adds && claude --version). Whoever merges first wins; the other rebases. I'll close #613 in favor of this or drop this hunk if #613 lands first.
• Follow-up (out of scope): redact_secrets doesn't mask AWS access keys — a pre-existing gap this PR's stdout-surfacing widens slightly. Worth a separate issue.

Plan: pushing a fix commit for B1 (option b + reconcile the 600/1800 comment) + N1 + N3, with N2/N4 folded in. Thanks @scottschreckengaust — the B1 catch is right, and I appreciate that you re-verified against real code rather than trusting the agent pass.

[isadeks]

Addressed all of it in 7e54b338.

B1 (blocker) — took your option (b). You're right: BUILD_VERIFY_TIMEOUT_S is read by nothing in agent/ on this branch (verify_build/verify_lint call run_cmd with no timeout= → the hardcoded 600s), and the comment's "live-caught via mise run build" narrative is contradicted by the verify path catching TimeoutExpired itself. So:

• Reworded the comment to describe the real trigger — an uncaught run_cmd TimeoutExpired (clone/setup/etc.), explicitly noting the verify path catches its own timeout and doesn't reach here.
• Softened the remedy to a generic "retry — it may just be slow; if a repo's build is legitimately long its environment needs more time/capacity, contact your ABCA admin" — no dead-knob promise. The real BUILD_VERIFY_TIMEOUT_S wiring (verify passing the env as timeout=) lives on the ECS-substrate track, not this main-bound branch, so I kept it out of scope rather than balloon this PR.
• Also reconciled the ecs-agent-cluster.ts comment (your "while you're in there"): it now says the env is provisioned ahead of its consuming wiring rather than currently effective, and drops the "see post_hooks.py BUILD_VERIFY_TIMEOUT_S" cross-ref (that symbol isn't on this branch).

N1 — added errorClass: ErrorClass.USER explicitly, with a comment stating the no-auto-retry intent (agree with you, not the earlier suggestion: auto-retrying an unchanged slow build just times out again — must not be TRANSIENT).

N2 — hardened the regex: timed out after \d+(?:\.\d+)? ?s(econds)?.

N3 — replaced the false-confidence test. The old one used eslint: 0 errors which matches no marker, so the allowlist never ran. The new test uses error: eslint produced 0 errors after autofix retry — hits marker error: and noise 0 errors — placed outside the tail window so it's only reachable via the marker scan. Deleting _FAILURE_LINE_NOISE now fails it (verified).

N4 — added a > _MAX_SURFACED_FAILURE_LINES test that asserts the … (more failure lines truncated) breadcrumb fires and the matched-scan (pre-tail) is capped.

#613 Dockerfile coordination — noted; #597's hunk is the superset (adds && claude --version). Whoever merges first wins the line; I'll rebase the other.

Gates: cdk 2439 tests + synth + eslint green; agent 1278 tests + ruff + ty green. Pushing re-set the review to REVIEW_REQUIRED — ready for your re-look.

[scottschreckengaust]

Review — Principal Architect (re-review of the B1 fix, 7e54b338)

My prior CHANGES_REQUESTED raised one blocker (B1) + nits N1–N4. I re-verified the fix commit 7e54b338 against real code (worktree at PR head, clean main base) rather than trusting the commit message — read error-classifier.ts, shell.py, post_hooks.py, and the two test files at head, re-ran the suites, and re-ran comment-analyzer on the reworded comments. B1 is genuinely resolved and every nit is addressed.

Verdict: Approve

The error-legibility feature now tells the truth — which, on a branch backed by #610 ("honest failure classification"), is exactly the bar.

B1 — resolved (option b, verified)

error-classifier.ts:436-460: the anchoring comment no longer claims the mise run build verify path as its ABCA-667 origin. It now correctly states the regex matches only uncaught run_cmd timeouts (clone/setup) and that the verify path (post_hooks.py:25,45) catches TimeoutExpired and returns a plain failure, so the pattern never fires for it. The remedy dropped the dead-knob promise (BUILD_VERIFY_TIMEOUT_S is read nowhere in agent/ — re-confirmed by grep) and now reads "retry / contact admin." ecs-agent-cluster.ts:186-194 reconciled: the misleading "1800s default" is gone, replaced with an honest "provisioned ahead of the consuming wiring" note (grep confirms nothing reads the env; verify still uses run_cmd's 600s default). comment-analyzer independently confirmed all reworded comments are accurate against the code.

Nits — all addressed

• N1 — error-classifier.ts:456-459: errorClass: ErrorClass.USER now set explicitly, with a comment stating the deliberate no-auto-retry intent (re-running an unchanged slow build just times out again — must not be "fixed" to TRANSIENT). Matches the file's own convention.
• N2 — error-classifier.ts:449: regex hardened to \d+(?:\.\d+)? ?s(econds)? — float-formatted seconds now match.
• N3 — test_shell.py:271: test_marker_line_with_noise_term_is_filtered_by_allowlist is now a real test of the load-bearing _FAILURE_LINE_NOISE allowlist — a line hitting both a marker (error:) and a noise term (0 errors), placed outside the tail window so it's only reachable via the marker scan. Deleting the allowlist now fails the test (verified the guard at shell.py:217-219 is what it exercises).
• N4 — test_shell.py:290: test_surfaced_failure_lines_are_capped_and_truncation_marked pins the _MAX_SURFACED_FAILURE_LINES cap (40) + the … (more failure lines truncated) breadcrumb — the branch (shell.py:221-223) no prior test reached.

Tests & CI

• Ran locally at PR head: agent/tests/test_shell.py → 31 passed; cdk/test/handlers/shared/error-classifier.test.ts → 77 passed (the isolated-run coverage-threshold warning is a single-file artifact, not a failure).
• CI all green: CodeQL ×3, dead-code advisory, secrets/deps/actions, title, build (agentcore). MERGEABLE.
• No CDK construct/stack logic change (the ecs-agent-cluster.ts hunk is comment-only) → bootstrap synth-coverage N/A.

Review agents run

comment-analyzer ✓ (re-run on the fix — the delta is almost entirely comment rewording, its exact scope; all claims verified accurate). The full panel (code-reviewer, silent-failure-hunter, pr-test-analyzer) ran on my prior review of the same diff; the fix commit's executable delta is one regex char-class + one errorClass line, both re-verified by hand here. Omitted: type-design-analyzer (no new types), security-review/IAM (no IAM/Cedar/network/secrets surface).

Human heuristics

• Proportionality — pass. Comment/copy fix + two real tests; no over-engineering. Correctly declined to wire BUILD_VERIFY_TIMEOUT_S (that behavior change belongs on the ECS-substrate track, not this main-bound branch).
• Coherence — pass. Changes stay in the right packages; consistent with existing PATTERNS/run_cmd idioms.
• Clarity — pass (was the B1 concern). The remedy no longer misdirects to a dead knob; comments now match the code.
• Appropriateness — pass. N3/N4 tests assert the load-bearing behavior (allowlist filtering, truncation cap), not just what the code happens to do.

Governance

Backing issue #610 (bug+approved) covers this work — satisfied. Branch pr/agent-build-robustness doesn't encode the issue number (ADR-003) — a de-facto-waived nit, not a blocker.

Coordination (not a #597 defect)

#613 is still OPEN and edits the same agent/Dockerfile RUN line; #597's hunk is the superset (adds && claude --version). Whoever merges first wins the line; the other rebases — author already committed to closing #613 in favor of this or dropping the hunk on rebase.

Nice work — the fix does exactly what "honest failure classification" asks. Approving.

_{🤖 Generated with Claude Code}