feat(agent): stuck/runaway guard + surface the failure behind a max_turns cap (ABCA-662) by isadeks · Pull Request #600 · aws-samples/sample-autonomous-cloud-coding-agents

isadeks · 2026-07-14T01:07:03Z

Stacked on #599 (base pr/error-classifier-hardening). The classifier changes reference ErrorClass introduced in #599, so review/merge #599 first; I'll rebase onto main once #599 lands.

What

Backports the agent-side stuck-guard subsystem from linear-vercel, plus a modest max-turns diagnostic.

feat(agent): stuck/runaway guard — new StuckGuard (agent/src/stuck_guard.py) that records tool results via the PostToolUse hook and detects a repeating-failing-command loop (the ABCA-483 spin).
fix: stuck-guard advisory-only — drop the hard bail; keep a one-time steer (preserves the K10 "no false kill" stance).
fix(agent): catch loop-of-variations early (ABCA-662) — a signature-agnostic trailing window (last 6 tool outcomes): when ≥5 share a byte-identical failure fingerprint across different commands, steer the agent once, mid-run.
max-turns terminal copy — when a max_turns cap coincides with a failure-dominated trailing window, the pipeline appends a neutral observation to the reason (last tool calls repeated: \git push` → invalid credentials`). The failure stays classified as the plain "Exceeded max turns": it surfaces what was observed and lets the reader decide — it does not re-title the run or claim more turns wouldn't help (the last 6 calls can't distinguish a hard blocker from a long task that hit a recoverable snag at the tail). The mid-run steer to the agent still coaches assertively — that's advice to the agent, not a classification of the outcome.

Conflict resolution (keep-both / partial)

hooks.py — 4 add-vs-add interleaves with main's feat(agent): observable blocker signal + bounded self-remediation for environmental faults #251 egress-denial detection: kept both the #251 progress/egress path and the new stuck_guard recording in post_tool_use_hook.
pipeline.py — the incoming hunk carried context from a not-included commit (8035b71, ENOSPC/OOM build_infra_failed, which depends on the unmerged feat(linear): parent/sub-issue orchestration with dependency-aware child tasks #247 build subsystem). Dropped that context; kept only the ABCA-662 stuck-summary enrichment. Verified build_infra_failed is referenced nowhere.

Test

agent: ty clean, ruff check + ruff format --check clean, 1212 tests pass (coverage 80.83%; stuck_guard.py 89%).
cdk: compile clean, 124 suites / 2274 tests pass, eslint --fix no mutations.

…loop Live-caught ABCA-483: a one-line README task burned ALL 100 turns (~22 min, $1.53) because the agent re-ran the SAME failing command (mise //cdk:test → JS-heap OOM, exit 134) over and over, yak-shaving the build env instead of finishing. Nothing noticed until the hard max_turns cap killed it. New `stuck_guard.py` (pure, dep-free): PostToolUse records each tool result; a coarse (tool_name, command) signature tracks consecutive failures. A new between-turns hook (registered after cancel, before nudge — same short-circuit rationale) then: - STEER once at 3 repeats: inject 'stop retrying X — work around it or finish with what you have'; - BAIL at 6 repeats: end the turn loop early (continue_=False) so the task fails fast instead of grinding to the cap. Conservative by design: keys on a REPEATING FAILURE (not a raw turn count), so a large task making varied progress never trips it; unknown tool output counts as success. Fail-open everywhere — a guard bug can't wedge the agent. Platform-agnostic, MAIN-BOUND: lives entirely in agent/src (stuck_guard.py + hooks.py); surfaces via the channel-neutral error_message. The honest reason is then rendered per-channel by the classifier / failure-reply (several fixes). 1158 agent tests pass (ruff + ty clean).

Reviewing the guard for false-positive risk (user: 'make sure the turn limit isn't too aggressive') showed the bail path was unsafe: distinguishing a true spin from a legitimately-iterating agent (re-running the same test command as it fixes failures one by one) from raw output is genuinely fragile — a digit- normalizing fingerprint that ignores volatile GC timings ALSO collapses 'test file_0' vs 'file_1' (the progress signal), so it would kill a working agent. A false-positive KILL is far worse than a false-positive nudge. So: removed BAIL entirely. The guard now only ever injects a ONE-TIME advisory steer when the same command fails with IDENTICAL output N times; the max_turns cap (with a fix's honest 'Exceeded max turns' reason) is the real runaway backstop. A false positive costs exactly one extra advisory comment. Platform-agnostic, main-bound.

…ns early (ABCA-662) ABCA-662 capped at max_turns, but the CAUSE was masked: it had finished the code and then thrashed ~15 turns retrying a failing 'git push' (remote: invalid credentials) every which way — a DIFFERENT command each turn, so the existing per-signature stuck-guard (same command × identical output ≥3) never tripped, and the terminal reason read only 'Exceeded max turns'. max_turns is a CORRECT classification, but the user can't tell 'genuinely needed more turns' from 'spun on an error' — and raising the cap just burns more tokens. Two changes, both advisory (no hard kill — keeps the K10 no-false-kill stance): - stuck_guard: add a signature-agnostic TRAILING WINDOW (last 6 tool outcomes). When ≥5 share the SAME byte-identical failure fingerprint across DIFFERENT commands, steer once ('you're spinning on one failing op — stop, it won't resolve by retrying'). EXACT fingerprint (no digit-blur) so a healthy iterate-and-fix loop (same cmd, a different test failing each run) never trips it — the K10 case, still covered by its test. Also exposes recent_failure_ summary(). - pipeline/hooks: the between-turns hook latches that summary; the terminal path appends it to a max_turns reason → 'error_max_turns … — spinning on failing tool calls (last: git push → invalid credentials)'. Only enriches max_turns; a productive run adds nothing. - error-classifier: a new bucket for 'error_max_turns … spinning on failing tool calls' → 'Ran out of turns while stuck on a failing step' with the honest remedy (raising --max-turns won't help; fix the failing op / check the env), ordered before the generic max_turns bucket. Tests: window-spin + K10 no-trip + summary + pipeline enrichment + classifier bucket.

…es (ABCA-662 review) The prior 'spinning on failing tool calls' classifier bucket asserted 'raising --max-turns won't help'. That over-claims: the trailing window can't distinguish (a) a HARD blocker no turn count fixes from (b) a LONG task that hit a transient/recoverable snag near the end and just ran out — which is 662 itself (siblings 661/663 pushed fine with the same token → its 'invalid credentials' was a transient race that more turns / a retry WOULD have cleared). Reframe the bucket to SURFACE what it was stuck on and present BOTH paths (environment blocker → fix + retry; transient/recoverable or a shorter sibling got past it → just retry / raise --max-turns), letting the failure KIND in the detail tell the reader which applies. Title 'Ran out of turns retrying a failing step'. The early window-steer (agent told to STOP and report while it still has turns) is the real mechanism that lets a long task escape the thrash; this copy is just the honest post-hoc explanation. Test updated to assert both remedies, not the over-claim.

…ted failure, don't diagnose it Reviewer concern on the ABCA-662 work: re-titling a max_turns failure as "Ran out of turns retrying a failing step" over-claims. The stuck-guard's trailing window is only the last ~6 tool calls, which genuinely CANNOT distinguish a hard blocker (bad creds, no permission — more turns won't help) from a long task that made real progress and hit a recoverable snag only at the tail (more turns / a retry WOULD help — 662 itself: siblings pushed fine with the same token). Framing the whole run around its last 6 calls misrepresents the latter. - error-classifier: drop the separate "retrying a failing step" bucket. A max_turns failure stays the plain "Exceeded max turns"; the description/remedy point the reader at the observed detail and still surface the environment-blocker path, but assert NO cause. - stuck_guard.recent_failure_summary: emit a NEUTRAL observation ("last tool calls repeated: `<cmd>` → <err>") instead of "spinning on failing tool calls". The mid-run steer TO the agent (an advisory nudge, not a user surface) still says "spinning" — that's fine, it's coaching the agent, not classifying the outcome. - tests updated to pin the neutral wording + assert the title is NOT re-framed.

scottschreckengaust

Review — Principal Architect (Stack B: agent error handling, PR 2/2 · stacked on #599)

Base verified against fresh origin PR head (diffed vs #599's branch pr/error-classifier-hardening). All mandatory agents run (table below).

Verdict: Approve (with nits)

Excellent stuck/runaway-guard work. The StuckGuard catches two distinct spin shapes — a per-signature identical-failure streak (ABCA-483) and the signature-agnostic trailing-window loop-of-variations (ABCA-662: the agent retries a failing git push every which way, each getting "invalid credentials", so no single command streak grows but the window is failure-dominated). It's advisory-only by deliberate design — a false nudge costs one comment; the max_turns cap is the real backstop — and the K10 reasoning (do NOT digit-blur fingerprints, so a healthy iterate-and-fix loop with a different failing test each run reads as progress and never trips) is a subtle, correct call. Cleanly stacked on #599's ErrorClass.

Verified myself: _LAST_STUCK_SUMMARY module-global is correctly reset per-task (reset_stuck_summary() at run_task top, before hooks run — mirrors the existing blocker-latch pattern), the deferred from hooks import … is intentional (avoids a circular import, matches the pre-existing convention), and the max_turns copy stays neutral (appends an observation, makes no causal claim about whether more turns would help) — a mature UX decision.

Non-blocking suggestions / nits

N3 (crit-8 test seam) — The max_turns reason-enrichment chain is tested only at its two pure endpoints: stuck_guard.recent_failure_summary() (unit) and the classifier consuming a hand-built string. The wiring between them is unverified — neither the pipeline.py:477 append (if err and "error_max_turns" in err: … last_stuck_summary()) nor the hooks.py:1465 latch-write is exercised end-to-end. Add a TestResolveOverallTaskStatus case monkeypatching hooks.last_stuck_summary (assert append on max_turns, no-append on a non-max_turns error, no double-append), and one driving _stuck_guard_between_turns_hook/stop_hook then asserting last_stuck_summary() — including the load-bearing "recovered → clears to None" behavior the comment claims.
N4 (crit-6 boundary) — The window-steer boundary is untested at exactly WINDOW_FAIL_THRESHOLD (5/6): the tests straddle it at 6 (steers) and 4 (doesn't) but skip the ==5 >= edge where an off-by-one would hide. Also add "window not full" (5 identical failures in a length-5 window → no steer, since _dominant_window_failure requires a full window). Two ~3-line tests; WINDOW_FAIL_THRESHOLD isn't even imported into the test file, so nothing currently pins the constant's boundary semantics.
N5 (cosmetic) — record_tool_result calls _looks_failed(tool_response) twice (window bookkeeping + streak); results are identical, could hoist to one call. And the window-steer's double-nudge guard keys on _last_failing_sig, so a per-signature steer + a window steer can both fire for one broad spin — benign (advisory; _window_steered still caps window steers at one).

Tests & CI

CI: all green (title, CodeQL ×3, dead-code advisory, secrets/deps/actions, build (agentcore)); MERGEABLE.
Coverage: stuck-guard core detection and hook integration are genuinely well-covered — the test_post_tool_use_record_error_never_blocks_screening (a guard raise must not break the fail-open screening path), test_stop_hook_steers_not_bails (advisory-only contract), test_iterating_agent_same_command_DIFFERENT_failures_never_steers (K10 false-positive guard), and the neutral-wording recent_failure_summary assertions are all high-value, behavior-not-implementation tests. Gaps are the integration seams → N3/N4.
Bootstrap synth-coverage: N/A (Python agent code + a small classifier copy tweak; no CDK construct/stack/resource change).

Review agents run (Stage 3 — mandatory)

Agent	Ran?	Outcome
`code-reviewer`	✅	No blocking defects; stuck-guard streak/window logic, per-task reset, and deferred imports all verified correct.
`silent-failure-hunter`	✅	The three advisory swallow-paths are all appropriate fail-open: PostToolUse `record_tool_result` swallow can't mask screening (runs before/independent of the scanner); the between-turns swallow only disables an advisory nudge, never a safety behavior; `_looks_failed` unknown→success is the correct bias for an additive guard (a miss costs nothing — `max_turns` is the floor). No silent-failure defects.
`pr-test-analyzer`	✅	Core detection well-covered; integration-seam gaps → N3, boundary → N4.
`type-design-analyzer`	⏭️ omitted	`StuckAction`/`_SigState` are simple dataclasses; no complex invariant to analyze.
`comment-analyzer`	⏭️ omitted	Comment accuracy spot-checked by hand + the other agents; the neutral-observation wording is correct and well-documented.
`/security-review`	⏭️ omitted	No IAM/Cedar/network/secrets change; the guard previews untrusted repo output but truncates (`_CMD_PREVIEW_LEN`) and never executes it.

Human heuristics

Proportionality — ✅ Deliberately minimal (advisory, no kill); matches the ABCA-483/662 incidents.
Coherence — ✅ Mirrors existing conventions (dedup set, per-task reset, between-turns hook ordering with the cancel<stuck<nudge invariant).
Clarity — ✅ Exemplary "why" comments, especially the K10 don't-blur-digits rationale and the neutral-observation design.
Appropriateness — ✅ The "surface what was observed, make no causal claim" max_turns copy is a notably mature UX call.

Governance

Branch pr/agent-stuck-guard carries no issue number (ADR-003) — same item as #599; attach an approved issue + conforming branch before merge. Code is approve-ready.

Bottom line: approve-ready. N3/N4 are worthwhile follow-up tests (the integration seams and the exact window boundary), not blockers. The advisory-only design and K10 false-positive avoidance are high quality. Merge after #599 lands (stacked).

_{🤖 Generated with Claude Code}

…ary (#600 N3/N4) Follow-up coverage from the #600 review (feature approved as-is; tests only): - N3 (crit-8 wiring seam): the max_turns reason-enrichment was tested only at its two pure endpoints (stuck_guard.recent_failure_summary + the classifier on a hand-built string) — the append in _resolve_overall_task_status that joins them was unverified. Add TestMaxTurnsStuckEnrichment (monkeypatching hooks.last_stuck_summary): appends on error_max_turns, does NOT append on a non-max_turns error, leaves the reason unchanged when there's no summary, and does not double-append when the summary is already present. - N4 (crit-6 boundary): the window-steer was tested at 6/6 (steers) and 4/6 (doesn't) but not at the exact WINDOW_FAIL_THRESHOLD `>=` edge. Add test_window_steers_at_exactly_the_threshold (5/6 same-fingerprint fails in a full window → steers) and test_no_steer_when_window_not_yet_full (5 fails in a length-5 history → no steer, since _dominant_window_failure requires a full window). Imports WINDOW/WINDOW_FAIL_THRESHOLD so the constants' boundary semantics are now pinned. Full agent suite green (1218); ruff format + check clean.

isadeks · 2026-07-14T20:58:25Z

Follow-up on the N3/N4 nits — since this merged into the Stack B base branch (pr/error-classifier-hardening, #599) before I added the tests, I put them on #599 (commit 6b7cd3f) so they ride to main when #599 merges:

N3 (crit-8 wiring seam): TestMaxTurnsStuckEnrichment in test_pipeline_outcomes.py — drives the _resolve_overall_task_status max_turns append itself (monkeypatching hooks.last_stuck_summary): appends on error_max_turns, does NOT append on a non-max_turns error, unchanged when the summary is None, and no double-append.
N4 (crit-6 boundary): two tests in test_stuck_guard.py — steers at exactly WINDOW_FAIL_THRESHOLD (5/6 in a full window) and does NOT steer when the window isn't full yet (5 fails / length-5 history). Imports WINDOW/WINDOW_FAIL_THRESHOLD so the boundary semantics are pinned.
N5 (cosmetic): left as-is — the double _looks_failed call and per-sig+window double-nudge are benign/advisory as you noted; not worth source churn on merged code.

All green (50 tests across the two files, ruff clean).

…sient/service/user axis with auto-retry-once (aws-samples#599) * fix(agent-classifier): recognize 'Agent session error (subtype=…)' wrapper The error classifier keyed on `agent_status=error_max_turns` (pipeline.py's wrapper) but missed runner.py:515's `Agent session error (subtype='error_max_turns')`. A real max-turns failure fell through to UNKNOWN → 'Unexpected error' (live-caught ABCA-483: a task hit the 100-turn cap but the Linear reply read 'Unexpected error'). The max_turns / max_budget / error_during_execution patterns now match BOTH the `agent_status=` and `subtype=` wrappers. Platform-agnostic, MAIN-BOUND (cherry-pick clean: single file + colocated test, no Linear/orchestration coupling). * feat(errors): 3-way transient/service/user classification + auto-retry-once on transient session-start Error handling couldn't distinguish a transient hiccup from a real service error — the retryable boolean conflated 'self-heals on retry' with 'you must change something first', so a customer couldn't tell whether to retry or call their admin. - error-classifier.ts: new ErrorClass axis (transient / service / user) on every classification. transient = infra/service hiccup a retry clears (ECS deploy-race, ENI/capacity delay, network blip, Bedrock throttle, concurrency cap); service = real platform/config fault an admin owns (bad token/scopes, model-not-enabled, blueprint misconfig); user = the request/code is the thing to change (build/test failed, guardrail, wrong PR, max-turns). retryGuidance() keys on errorClass and takes an autoRetried flag. New isTransientError() helper. - orchestrate-task.ts: AUTO-RETRY-ONCE at session-start for transient failures — the one place a retry is idempotent by construction (no clone/commits/PR yet). Emits session_start_retry; stamps '[auto-retried]' so the surface says 'I tried again, still failed'. Mid-run crashes are NOT retried. Backport note: the failure-reply.ts consumer hunk from the origin commit is omitted — that module is part of the not-yet-merged aws-samples#247 orchestration UX and does not exist on main. The classifier axis + auto-retry stand alone. (cherry picked from commit 63a12dc) * fix(errors): classify the claude Exec-format/broken-shim failure with retry guidance (ABCA-659) When the agent image's claude CLI can't be exec'd (OSError Exec format error, or the shim's own 'claude native binary not installed'), the classifier had no matching pattern and fell through to the bare 'Unexpected error' with a generic 'report it' remedy — the exact no-guidance anti-pattern the error-feedback arc (K5 buckets, remedy+retryable+errorClass) was built to eliminate. Live-caught on ABCA-659's retry: the ❌ Linear comment just said 'Unexpected error', telling the user nothing about whether to retry or escalate. Add a COMPUTE + transient pattern so the failure reads honestly and actionably: 'Couldn't start the coding agent (environment issue) … not a problem with your request … reply here to try again; if every attempt fails the same way, the agent image needs a rebuild — contact your ABCA admin with the task id.' The transient class also lets the platform's session-start auto-retry take a shot. Matched before the AGENT/UNKNOWN fallthrough; two tests pin the copy + axis. * feat(agent): stuck/runaway guard + surface the failure behind a max_turns cap (ABCA-662) (aws-samples#600) * feat(agent): stuck/runaway guard — break a repeating-failing-command loop Live-caught ABCA-483: a one-line README task burned ALL 100 turns (~22 min, $1.53) because the agent re-ran the SAME failing command (mise //cdk:test → JS-heap OOM, exit 134) over and over, yak-shaving the build env instead of finishing. Nothing noticed until the hard max_turns cap killed it. New `stuck_guard.py` (pure, dep-free): PostToolUse records each tool result; a coarse (tool_name, command) signature tracks consecutive failures. A new between-turns hook (registered after cancel, before nudge — same short-circuit rationale) then: - STEER once at 3 repeats: inject 'stop retrying X — work around it or finish with what you have'; - BAIL at 6 repeats: end the turn loop early (continue_=False) so the task fails fast instead of grinding to the cap. Conservative by design: keys on a REPEATING FAILURE (not a raw turn count), so a large task making varied progress never trips it; unknown tool output counts as success. Fail-open everywhere — a guard bug can't wedge the agent. Platform-agnostic, MAIN-BOUND: lives entirely in agent/src (stuck_guard.py + hooks.py); surfaces via the channel-neutral error_message. The honest reason is then rendered per-channel by the classifier / failure-reply (several fixes). 1158 agent tests pass (ruff + ty clean). * fix: stuck-guard is advisory-only — drop the bail, keep a one-time steer Reviewing the guard for false-positive risk (user: 'make sure the turn limit isn't too aggressive') showed the bail path was unsafe: distinguishing a true spin from a legitimately-iterating agent (re-running the same test command as it fixes failures one by one) from raw output is genuinely fragile — a digit- normalizing fingerprint that ignores volatile GC timings ALSO collapses 'test file_0' vs 'file_1' (the progress signal), so it would kill a working agent. A false-positive KILL is far worse than a false-positive nudge. So: removed BAIL entirely. The guard now only ever injects a ONE-TIME advisory steer when the same command fails with IDENTICAL output N times; the max_turns cap (with a fix's honest 'Exceeded max turns' reason) is the real runaway backstop. A false positive costs exactly one extra advisory comment. Platform-agnostic, main-bound. * fix(agent): explain WHY a task hit max_turns + catch loop-of-variations early (ABCA-662) ABCA-662 capped at max_turns, but the CAUSE was masked: it had finished the code and then thrashed ~15 turns retrying a failing 'git push' (remote: invalid credentials) every which way — a DIFFERENT command each turn, so the existing per-signature stuck-guard (same command × identical output ≥3) never tripped, and the terminal reason read only 'Exceeded max turns'. max_turns is a CORRECT classification, but the user can't tell 'genuinely needed more turns' from 'spun on an error' — and raising the cap just burns more tokens. Two changes, both advisory (no hard kill — keeps the K10 no-false-kill stance): - stuck_guard: add a signature-agnostic TRAILING WINDOW (last 6 tool outcomes). When ≥5 share the SAME byte-identical failure fingerprint across DIFFERENT commands, steer once ('you're spinning on one failing op — stop, it won't resolve by retrying'). EXACT fingerprint (no digit-blur) so a healthy iterate-and-fix loop (same cmd, a different test failing each run) never trips it — the K10 case, still covered by its test. Also exposes recent_failure_ summary(). - pipeline/hooks: the between-turns hook latches that summary; the terminal path appends it to a max_turns reason → 'error_max_turns … — spinning on failing tool calls (last: git push → invalid credentials)'. Only enriches max_turns; a productive run adds nothing. - error-classifier: a new bucket for 'error_max_turns … spinning on failing tool calls' → 'Ran out of turns while stuck on a failing step' with the honest remedy (raising --max-turns won't help; fix the failing op / check the env), ordered before the generic max_turns bucket. Tests: window-spin + K10 no-trip + summary + pipeline enrichment + classifier bucket. * fix(errors): don't over-claim on a max_turns spin — offer BOTH remedies (ABCA-662 review) The prior 'spinning on failing tool calls' classifier bucket asserted 'raising --max-turns won't help'. That over-claims: the trailing window can't distinguish (a) a HARD blocker no turn count fixes from (b) a LONG task that hit a transient/recoverable snag near the end and just ran out — which is 662 itself (siblings 661/663 pushed fine with the same token → its 'invalid credentials' was a transient race that more turns / a retry WOULD have cleared). Reframe the bucket to SURFACE what it was stuck on and present BOTH paths (environment blocker → fix + retry; transient/recoverable or a shorter sibling got past it → just retry / raise --max-turns), letting the failure KIND in the detail tell the reader which applies. Title 'Ran out of turns retrying a failing step'. The early window-steer (agent told to STOP and report while it still has turns) is the real mechanism that lets a long task escape the thrash; this copy is just the honest post-hoc explanation. Test updated to assert both remedies, not the over-claim. * fix(errors): max_turns stays "Exceeded max turns" — observe the repeated failure, don't diagnose it Reviewer concern on the ABCA-662 work: re-titling a max_turns failure as "Ran out of turns retrying a failing step" over-claims. The stuck-guard's trailing window is only the last ~6 tool calls, which genuinely CANNOT distinguish a hard blocker (bad creds, no permission — more turns won't help) from a long task that made real progress and hit a recoverable snag only at the tail (more turns / a retry WOULD help — 662 itself: siblings pushed fine with the same token). Framing the whole run around its last 6 calls misrepresents the latter. - error-classifier: drop the separate "retrying a failing step" bucket. A max_turns failure stays the plain "Exceeded max turns"; the description/remedy point the reader at the observed detail and still surface the environment-blocker path, but assert NO cause. - stuck_guard.recent_failure_summary: emit a NEUTRAL observation ("last tool calls repeated: `<cmd>` → <err>") instead of "spinning on failing tool calls". The mid-run steer TO the agent (an advisory nudge, not a user surface) still says "spinning" — that's fine, it's coaching the agent, not classifying the outcome. - tests updated to pin the neutral wording + assert the title is NOT re-framed. * fix+test: guard session-start retry emit, extract + test it, classify raw error Addresses the aws-samples#599 review (Stack B): - B1 (MEDIUM): the `session_start_retry` telemetry emit was unguarded — a TaskEvents PutItem fault (throttle/timeout, co-occurring with the transient session-start failures this path handles) threw AFTER `autoRetried=true` but BEFORE the retry ran, so the user was told a second attempt failed when none had, and the real retriable cause was discarded. The emit is now best-effort (try/catch → WARN-and-continue) so telemetry never aborts/mis-attributes the retry. The `correlation` envelope is threaded so the retry event carries the same trace context as `session_started` (aws-samples#245). - B2 (crit-9): the auto-retry had ZERO coverage (it lived inline in the durable handler's start-session step the suite never invokes). Extracted to an exported `startSessionWithRetry(strategy, input, deps)` — matching the repo's "test the exported helper" pattern — and unit-tested all four branches (success / non-transient no-retry / transient→retry→success / transient→transient→throw) plus the B1 best-effort-emit guard. - N2: classify the RAW error, not a ``Session start failed: …`` wrapper. The wrapper string itself matched a TRANSIENT pattern, so EVERY session-start failure classified transient and the "non-transient throws immediately" branch was effectively dead (a config/auth fault ate a pointless ~1-min retry). Classifying the raw string restores that branch (verified: TaskDefinition- inactive → retry; ECS_CLUSTER_ARN-not-configured / AccessDenied → surface). - N1: soften the `[auto-retried]` comment — it forward-referenced renderFailureReply/AUTO_RETRIED_MARKER that don't exist yet. Now states the marker is persisted verbatim for a forthcoming failure renderer (retryGuidance ships ahead of its consumer); no behaviour claim about rendering today. Full cdk build green (compile + jest + eslint + synth); 5 new helper tests pass. * test: cover the max_turns-enrichment wiring seam + window-steer boundary (aws-samples#600 N3/N4) Follow-up coverage from the aws-samples#600 review (feature approved as-is; tests only): - N3 (crit-8 wiring seam): the max_turns reason-enrichment was tested only at its two pure endpoints (stuck_guard.recent_failure_summary + the classifier on a hand-built string) — the append in _resolve_overall_task_status that joins them was unverified. Add TestMaxTurnsStuckEnrichment (monkeypatching hooks.last_stuck_summary): appends on error_max_turns, does NOT append on a non-max_turns error, leaves the reason unchanged when there's no summary, and does not double-append when the summary is already present. - N4 (crit-6 boundary): the window-steer was tested at 6/6 (steers) and 4/6 (doesn't) but not at the exact WINDOW_FAIL_THRESHOLD `>=` edge. Add test_window_steers_at_exactly_the_threshold (5/6 same-fingerprint fails in a full window → steers) and test_no_steer_when_window_not_yet_full (5 fails in a length-5 history → no steer, since _dominant_window_failure requires a full window). Imports WINDOW/WINDOW_FAIL_THRESHOLD so the constants' boundary semantics are now pinned. Full agent suite green (1218); ruff format + check clean. * fix(cli): mirror errorClass onto ErrorClassification (CDK↔CLI type sync) aws-samples#599 added `errorClass?: ErrorClassType` to the CDK ErrorClassification, which serializes into the GET /tasks/{id} `error_classification` response — but the hand-maintained CLI mirror (cli/src/types.ts) wasn't updated, so the CLI's structured error display couldn't surface the transient/service/user axis (real drift per the CLAUDE.md shared-API-shape routing rule). Add the optional field to the CLI's ErrorClassification, inlined as a union literal rather than a named `ErrorClassType` export — the types-sync drift check (scripts/check-types-sync.ts) requires CDK to be the source of truth for any exported type name, and CDK exports ErrorClassType from error-classifier.ts, not shared/types.ts. Inlining keeps the field synced without a CLI-only export. Verified: `//:check:types-sync` OK (57 CLI exports validated), cli build green (600 tests, compile + eslint clean). * fix(errors): address aws-samples#599 nits N1-N3 — [auto-retried] breadcrumb + test seams N1 (Medium): the [auto-retried] marker was never stamped on branch 4 (transient- then-transient), because startSessionWithRetry THROWS there and the caller's autoRetried flag is only set on the success paths. A double-transient failure was told 'reply to retry' instead of 'I already retried' — the exact confusion the marker exists to prevent. Fix: tag the thrown error with a Symbol (AUTO_RETRIED) + export isAutoRetried(); orchestrate-task reads it in the catch and stamps the marker on both paths a retry ran. Corrected the docstring (was claiming the fact was 'observable via the thrown-from state' — it wasn't). N2 (Low): pin the two untested wiring seams. CDK: branch-4 test now asserts isAutoRetried(err) is true (and false for a first-attempt/non-object failure). Agent: new tests drive _stuck_guard_between_turns_hook end-to-end and assert the _LAST_STUCK_SUMMARY latch is written (and cleared on a healthy window) — the production path the enrichment test's monkeypatch bypasses. Deleting hooks.py's latch write now fails a test. N3 (Low): pin retryGuidance's two USER fall-through branches (retryable-user non-guardrail, not-retryable-user) so the aws-samples#247 failure-renderer copy contract can't rot. Full cdk (2345) + agent (1251) gates green. --------- Co-authored-by: Alain Krok <alkrok@amazon.com>

isadeks added 4 commits July 13, 2026 21:00

isadeks requested review from a team as code owners July 14, 2026 01:07

isadeks changed the title ~~feat(agent): stuck/runaway guard + explain WHY a task hit max_turns (ABCA-662)~~ feat(agent): stuck/runaway guard + surface (not diagnose) the failure behind a max_turns cap (ABCA-662) Jul 14, 2026

isadeks added enhancement New feature or request agent-runtime Python agent container: pipeline, runner, hooks, prompts, tools, Dockerfile validation-loop Tasks related to improve the validation loop for ABCA's codebase P2 lowest priority labels Jul 14, 2026

isadeks changed the title ~~feat(agent): stuck/runaway guard + surface (not diagnose) the failure behind a max_turns cap (ABCA-662)~~ feat(agent): stuck/runaway guard + surface the failure behind a max_turns cap (ABCA-662) Jul 14, 2026

scottschreckengaust mentioned this pull request Jul 14, 2026

fix(errors): classify agent-session/Exec-format failures + 3-way transient/service/user axis with auto-retry-once #599

Merged

scottschreckengaust approved these changes Jul 14, 2026

View reviewed changes

krokoko merged commit 99f9d11 into pr/error-classifier-hardening Jul 14, 2026
15 checks passed

krokoko deleted the pr/agent-stuck-guard branch July 14, 2026 19:54

isadeks mentioned this pull request Jul 14, 2026

fix(errors): honest failure classification + transient auto-retry + stuck/runaway guard #610

Open

5 tasks

scottschreckengaust mentioned this pull request Jul 15, 2026

feat(ecs): make the ECS Fargate compute backend usable (context-gated wiring + ECS-parity fixes) #596

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(agent): stuck/runaway guard + surface the failure behind a max_turns cap (ABCA-662)#600

feat(agent): stuck/runaway guard + surface the failure behind a max_turns cap (ABCA-662)#600
krokoko merged 5 commits into
pr/error-classifier-hardeningfrom
pr/agent-stuck-guard

isadeks commented Jul 14, 2026 •

edited

Loading

Uh oh!

scottschreckengaust left a comment

Uh oh!

Uh oh!

isadeks commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

isadeks commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Conflict resolution (keep-both / partial)

Test

Uh oh!

scottschreckengaust left a comment

Choose a reason for hiding this comment

Review — Principal Architect (Stack B: agent error handling, PR 2/2 · stacked on #599)

Verdict: Approve (with nits)

Non-blocking suggestions / nits

Tests & CI

Review agents run (Stage 3 — mandatory)

Human heuristics

Governance

Uh oh!

Uh oh!

isadeks commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

isadeks commented Jul 14, 2026 •

edited

Loading