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

[isadeks]

What

Backports the standalone error-classifier hardening from linear-vercel to main: user-facing failures that previously rendered as a bare "Unexpected error" now get a precise title + actionable remedy, and a new transient/service/user axis drives a safe auto-retry.

Three commits:

1. fix(agent-classifier): recognize Agent session error (subtype=…) wrapper — the classifier keyed only on agent_status=…, so the subtype=… wrapper emitted by the terminal-error path fell through to "Unexpected error" (live-caught on ABCA-483: a real max-turns cap surfaced as "Unexpected error").

2. feat(errors): 3-way transient/service/user classification + auto-retry-once on transient session-start — a new ErrorClass axis (transient / service / user) on every classification, because the retryable boolean conflated "self-heals on retry" with "you must change something first". retryGuidance() now keys on errorClass. orchestrate-task.ts auto-retries once on a transient session-start failure — the one place a retry is idempotent by construction (no clone/commits/PR yet) — and stamps [auto-retried] so the surface can say "I tried again, still failed". Mid-run crashes are not retried.

3. fix: classify the claude Exec-format/broken-shim failure with retry guidance (ABCA-659) — when the agent image can't exec the claude CLI (Exec format error / native binary not installed), classify it as a transient COMPUTE/image fault with a retry-then-rebuild remedy instead of "Unexpected error".

Scope / backport note

This is one slice of a larger error-feedback arc on linear-vercel. The rest of that arc modifies failure-reply.ts / orchestration-reconciler.ts / orchestration-rollup.ts, which are part of the not-yet-merged #247 orchestration subsystem and don't exist on main — so they're deliberately left out.

Commit 2 is a partial cherry-pick of its origin commit: the origin also touched failure-reply.ts (#247-only, absent from main), so that hunk is dropped. The kept portion (error-classifier.ts + orchestrate-task.ts) only depends on classifyError/isTransientError and stands alone. The orchestrate-task.ts merge preserves main's failTask(…, task.repo) signature and omits an unrelated #299 readOnly field that main's startSession input doesn't define.

Test

mise run build green in cdk/: compile clean, 124 suites / 2273 tests pass, global coverage gate met, eslint --fix produces no mutations, synth clean (only pre-existing cdk-nag INFO/WARN annotations).

[scottschreckengaust]

Review — Principal Architect (Stack B: agent error handling, PR 1/2)

Base verified against fresh origin/main. Stacked: #600 builds on this branch. All mandatory agents run (table below). Dead-code ratchet status verified by actually running knip.

Verdict: Request changes

Strong error-classifier work — the subtype= wrapper fix (ABCA-483: real max-turns failures no longer read as "Unexpected error"), the transient/service/user ErrorClass axis with fail-safe defaults (absent ⇒ USER, never auto-retry an unknown), and the TaskDefinition is inactive / claude Exec format error classifications. Pattern ordering verified correct (specific patterns precede the catch-alls). Two issues hold it: one MEDIUM correctness bug and a crit-9 test gap on the riskiest new behavior.

Blocking issues

B1 — MEDIUM: the unguarded session_start_retry emit can falsely stamp [auto-retried] and replace the real failure cause. orchestrate-task.ts:189-197:

autoRetried = true;                                        // set first
await emitTaskEvent(taskId, 'session_start_retry', {...}); // UNGUARDED
handle = await strategy.startSession(startInput);          // retry — never runs if the emit threw

emitTaskEvent (orchestrator.ts:205) is a bare await ddb.send(new PutCommand(...)) with no internal try/catch — it throws on any DynamoDB fault. If the TaskEventsTable PutItem throttles/times out (exactly the conditions co-occurring with the transient session-start failures this path handles), it throws after autoRetried=true but before the retry executes. The outer catch then produces Session start failed: <DDB error> [auto-retried]: the user is told a second attempt was made and failed when none ran, and the real retriable cause is discarded for an observability-layer error they can't act on. This breaks the file's own best-effort-emit convention (the concurrency-cap path at :89 deliberately guards its emits).

• Fix: wrap the emit in try/catch (WARN-and-continue) so a telemetry failure never aborts or mis-attributes the retry. Also pass the correlation envelope — it's dropped here vs. the sibling session_started emit (minor #245 trace gap; fold into the same fix).

B2 — crit-9 test gap: the session-start auto-retry has ZERO coverage. orchestrate-task.test.ts has no test for retry / session_start_retry / [auto-retried]. The retry lives inline in the durable handler's start-session step, which the suite never invokes (it tests exported shared/orchestrator helpers in isolation). All four branches are unverified — most importantly "non-transient failure must NOT retry" (where a mis-classification wastes a retry or doubles a non-idempotent op) and the transient→retry→succeeds happy path the feature exists for.

• Fix: extract the retry into an exported startSessionWithRetry(strategy, input) (matches the existing "test the exported helper" pattern) and unit-test the 4 branches; or add a durable-handler test with a mock strategy that rejects-then-resolves.

Non-blocking suggestions / nits

• N1 (LOW — confirmed by two agents) — retryGuidance(), AUTO_RETRIED_MARKER, and renderFailureReply have no production consumer on main (the latter two don't exist in the repo; retryGuidance is called only by its own test). The orchestrate-task.ts:226 comment forward-references renderFailureReply/AUTO_RETRIED_MARKER as if they exist. Not a runtime bug (the [auto-retried] string is persisted verbatim and doesn't affect classification), but the "I already tried again" copy the feature advertises is never rendered. Either wire retryGuidance into the failure renderer, or soften the comments to "will be consumed by the forthcoming renderFailureReply" and note retryGuidance ships ahead of its consumer.
• N2 (LOW) — The retry classifies the re-wrapped string `Session start failed: ${firstErr}`, and since /Session start failed/i is itself TRANSIENT and sits after only a few specific patterns, a genuinely non-transient fault (missing-ECS-env config error, raw AccessDenied) classifies TRANSIENT and eats one ~1-min retry. Safe (never masked as success), but the inline comment's claim "a NON-transient failure throws immediately" is only true for faults matching an earlier SERVICE pattern. Classify String(firstErr) directly, or soften the comment.

Tests & CI

• CI: all green (title, CodeQL ×3, dead-code advisory, secrets/deps/actions, build (agentcore)); MERGEABLE.
• Dead-code ratchet — ran it for ground truth. It reports count 81 > baseline 78 on this head, but it's advisory-only, not blocking: not part of mise run build, and the CI job is continue-on-error (mise.toml: "Advisory in CI for now; flips to blocking once the baseline is driven to zero"); origin/main itself already sits at 80 > 78. The +1 knip attributes here is ErrorClassType, which is used (error-classifier.ts:82, the errorClass? field type) — a knip false-positive on a (typeof X)[keyof typeof X] alias, not real dead code. No action required.
• Coverage: classifier axes are genuinely well-tested (the every-classification-has-errorClass invariant loop is high-value). The gap is the auto-retry → B2.
• Bootstrap synth-coverage: N/A (classifier + handler logic; no CDK construct/stack/resource change).

Review agents run (Stage 3 — mandatory)

Agent	Ran?	Outcome
code-reviewer	✅	No blocking logic defects; flagged the over-broad-but-safe retry gate (N2) + unwired retryGuidance (N1).
silent-failure-hunter	✅	1 MEDIUM → B1 (unguarded retry-event emit). Confirmed the classifier's fail-safe defaults (absent ⇒ USER, isTransientError false for absent) are correct, non-masking.
pr-test-analyzer	✅	crit-9 → B2 (auto-retry untested); classifier axes well-covered.
type-design-analyzer	⏭️ omitted	ErrorClass/ErrorClassType are simple const-object enums; no complex new type.
comment-analyzer	⏭️ omitted	Comment accuracy covered by hand + code-reviewer/silent-failure (N1/N2 are the comment findings).
/security-review	⏭️ omitted	No IAM/Cedar/network/secrets/input-gateway change.

Human heuristics

• Proportionality — ✅ Matches documented live incidents (ABCA-483/659); minimal surface.
• Coherence — ✅ ErrorClass unifies retry semantics repo-wide; #600 stacks cleanly on it.
• Clarity — ⚠️ Excellent "why" comments, but two forward-reference code that doesn't exist yet (N1) or overstate a guarantee (N2).
• Appropriateness — ✅ Sound; the one gap (B1) is that the retry-event emit should follow the file's own best-effort pattern.

Governance

Branch pr/error-classifier-hardening carries no issue number and the body references only linear-vercel — no approved GitHub issue (ADR-003). Please attach an approved issue + conforming branch before merge (a natural sibling of the ABCA-483/659 incidents). Not gating the code verdict, but required to merge.

Bottom line: fix B1 (guard the retry-event emit — a real MEDIUM that can misreport failures) and B2 (test the untested auto-retry, ideally by extracting it); N1/N2 are comment/wiring cleanups. The classifier work is high quality.

_{🤖 Generated with Claude Code}

[isadeks]

Thanks — addressed both blockers + both nits in e0efe30.

B1 (unguarded retry emit) — extracted the retry into startSessionWithRetry where the session_start_retry emit is now best-effort (try/catch → WARN-and-continue), so a TaskEvents PutItem fault can't abort the retry or falsely stamp [auto-retried]. Also threaded the correlation envelope so the retry event carries the same trace context as session_started (the #245 gap you noted).

B2 (untested auto-retry) — took the extract-and-test path: startSessionWithRetry(strategy, input, deps) (deps = injected emitRetryEvent/logger, so it's pure and unit-testable). New session-start-retry.test.ts covers all four branches — success / non-transient-no-retry / transient→retry→success / transient→transient→throw — plus a dedicated B1 test (emit throws → retry still runs and succeeds, fault logged not propagated).

N2 (over-broad retry gate) — this was sharper than LOW: because the code classified the re-wrapped `Session start failed: …` string and /Session start failed/i is itself a TRANSIENT pattern, every session-start failure classified transient — so a genuine config/auth fault (ECS_CLUSTER_ARN is not configured, AccessDenied) ate a pointless ~1-min retry and the "non-transient throws immediately" branch was effectively dead. Now classifies String(firstErr) directly; verified TaskDefinition is inactive → retry while the config/auth faults → surface immediately. (Widening the classifier's transient patterns — e.g. ThrottlingException — is a separate classifier-completeness concern, not this gate.)

N1 (forward-ref comments) — softened the [auto-retried] comment: it no longer references renderFailureReply/AUTO_RETRIED_MARKER (which don't exist on main). It now states the marker is persisted verbatim for a forthcoming renderer, and that retryGuidance() ships ahead of its consumer.

Dead-code ratchet — noted, agreed it's a knip false-positive on ErrorClassType ((typeof X)[keyof typeof X] alias, used at error-classifier.ts:82); no action. Separately I filed #607 for the real pre-existing ratchet failure on main (81 > baseline 78), which is independent of this PR.

B2/governance — will file an approved GitHub issue (sibling of ABCA-483/659) + rename the branch to fix/<n>-… before merge, same as the rest of the batch (#604/#606 pattern).

Full cdk build green (compile + jest + eslint + synth). Re-requesting review.

[scottschreckengaust]

Follow-up review — prior blockers addressed; B2 governance still stands

Re-reviewed the reworked PR (now +1442, new session-start-retry.ts + test_pipeline_outcomes.py since my CHANGES_REQUESTED). The code is in good shape and my two prior technical blockers are resolved; the remaining blocker is governance (B2).

✅ Resolved since my prior review

• B1 (retry logic untestable inline in the durable handler) — extracted to cdk/src/handlers/shared/session-start-retry.ts with all four branches unit-tested (success / non-transient-throw / transient-then-success / transient-then-transient) plus a best-effort-emit-guard test. The emit is now wrapped so a TaskEvents PutItem fault can't abort or mis-attribute the retry. ✅
• N2 (classify the raw error, not the Session start failed: wrapper) — fixed. startSessionWithRetry classifies String(firstErr); classifying the wrapper would have matched the /Session start failed/i transient pattern and made every session-start failure retry (turning the non-transient-throw branch into dead code). Good catch. ✅
• Verified independently: the stuck-guard is genuinely advisory-only (can only return none/steer, never kills); cancel-wins is preserved (cancel index 0 short-circuits before stuck-guard/nudge); the fail-open swallows in hooks.py/stuck_guard.py don't alter the screening decision or terminal-status resolution; the regex additions sit before the generic catch-all. No blocking code defects.

🔴 Blocking — B2: governance (ADR-003) — unchanged

Still no approved issue on a conforming branch:

• No linked closing issue; the PR is labeled (bug/validation-loop/orchestration/P1) but that is not the approved gate. I searched — there's no open issue with approved covering the error-classifier/stuck-guard work (the ABCA-483/659/662 references are Linear ids, not GitHub issues).
• Branch pr/error-classifier-hardening doesn't follow (feat|fix)/<issue-number>-short-description.

Per ADR-003: file (or link) an issue with acceptance criteria, get the approved label, self-assign, comment "Starting implementation," and re-cut on a conforming branch (e.g. fix/<n>-error-classifier-hardening). This is the merge gate; the code itself is mergeable-quality.

🟠 New non-blocking finding — CLI type drift

#599 adds errorClass?: ErrorClassType to ErrorClassification in cdk/src/handlers/shared/error-classifier.ts, which serializes into the GET /tasks/{id} response (types.ts:252 error_classification → :607 classifyError(...)). The mirror in cli/src/types.ts:72-78 was not updated — it still lacks errorClass?. Per the CLAUDE.md routing rule (shared API shapes must stay in sync), add:

readonly errorClass?: 'transient' | 'service' | 'user';

Not a build-breaker (additive optional; extra JSON is ignored, CLI compiles), but it's real drift and the CLI's structured error display can't surface the new axis until it's mirrored.

🟡 Nits (non-blocking)

1. retryGuidance() + [auto-retried] are dead on main (no consumer until the #247 failure-renderer lands). The PR is explicit that they "ship ahead of the consumer," and the dead-code ratchet is advisory here, so acceptable — but keep the producer/consumer contract pinned by test (retryGuidance(c, true) copy is already asserted) so it can't rot before #247.
2. Retry is narrower than the prose implies — the comment cites "ENI/capacity delay, Bedrock throttle" but only TaskDefinition is inactive classifies TRANSIENT today; raw ThrottlingException/TimeoutError fall through to UNKNOWN→USER and are not retried (safe, but the named cases are latent until those patterns are added).
3. pipeline.py max_turns enrichment does from hooks import last_stuck_summary and appends unguarded. It can't realistically raise (trivial global read) and the whole body is inside run_task's crash handler, so terminal status is always written — but a one-line local try/except around just the append would ensure an enrichment bug can never degrade the (already-correct) terminal reason.
4. orchestrate-task.ts wiring seam untested — the extracted startSessionWithRetry is well covered, but no test drives the durable handler to assert it's actually called and that [auto-retried] is stamped on the double-failure path. A refactor reverting to a direct startSession would pass every test. Also 3 hook-glue branches (stuck-hook cancel short-circuit, evaluate() fail-open, the real producer→latch→consumer chain for the stuck summary) are untested.

Tests & CI

Coverage is strong and behavior-first: the 4-branch retry matrix, the errorClass invariant ("every pattern carries one"), the ABCA-662 neutrality assertions (title NOT re-framed, remedy points at the detail), and the stuck-guard boundary trio (exact >=/full-window edges, K10 false-positive avoidance). CI green, MERGEABLE.

_{🤖 Reviewed with Claude Code}

[isadeks]

Resolved the conflict + addressed the new CLI-drift finding (0e3a76b merge, 4fea6c5 CLI fix).

Conflict (from #503 landing on main — both PRs touch orchestrate-task.ts): a one-line import collision — #503 adds deleteEcsPayload, this PR adds startSessionWithRetry. Kept both (both are used: startSessionWithRetry in start-session, deleteEcsPayload at finalize). Verified by full mise //cdk:build (2340 tests + synth green) — semantically clean, not just marker removal.

New finding — CLI type drift: added errorClass? to the CLI's ErrorClassification mirror. Inlined it as a 'transient' | 'service' | 'user' union rather than a named ErrorClassType export — the //:check:types-sync gate requires CDK to be the source of truth for exported type names, and CDK exports ErrorClassType from error-classifier.ts (not shared/types.ts), so a named CLI export tripped the drift check. Inlining keeps the field synced without a CLI-only export. //:check:types-sync now OK; cli build green (600 tests).

Nits: N3-guard (try/except around the max_turns append) + the wiring-seam tests you flagged (nit 4) I'd fold into the existing #608 follow-up rather than widen this PR — they're the same "assert the durable handler actually calls the extracted helper" class. Nits 1/2 (retryGuidance ships-ahead-of-consumer, retry narrower than prose) are documented-as-intended. LMK if you'd rather any of those land here.

B2 governance: filing the approved governing issue + branch rename to fix/<n>-error-classifier-hardening as the last step before merge (same as the Stack A pattern; #600 already merged into this branch, so this PR carries all of Stack B to main).

[isadeks]

Filed #610 as the governing issue for this Stack B error-handling work (covers #599 + the merged #600), now approved. That clears the B2-issue side; branch rename to fix/610-error-classifier-hardening remains as the last step before merge.

[isadeks]

@scottschreckengaust — re-requesting review. B2 is cleared: governing issue #610 is now approved (covers this PR + the merged #600). B1/N2 you already confirmed resolved; the new CLI type-drift finding is fixed (4fea6c5), and the #503 conflict is resolved (full mise //cdk:build green — MERGEABLE). Nits 3/4 folded into #608.

Same branch-name note as #596: renaming closes the open PR, so I've left it and linked the approved #610 — happy to carry fix(errors): … (#610) in the squash-merge title instead of re-cutting. Could you clear the CHANGES_REQUESTED?

[scottschreckengaust]

Review — Principal Architect (Stack B: error-classifier + stuck guard, re-review)

Re-reviewed against fresh origin/main (merge-base = main tip 22705e8; fully current). This PR is stacked — it carries the merged #600 stuck-guard work to main alongside the classifier hardening. All mandatory agents re-run on head 4e38a45. This supersedes my earlier CHANGES_REQUESTED.

Verdict: Approve (with nits)

Both of my prior technical blockers are resolved and governance is now cleared, so I'm approving. Summary of what closed since my last pass:

• B1 (unguarded retry-event emit could falsely stamp [auto-retried] / discard the real cause) — fixed: the retry is extracted into session-start-retry.ts::startSessionWithRetry, the emit is wrapped best-effort (WARN-and-continue), and a telemetry fault can no longer abort or mis-attribute the retry. Verified by session-start-retry.test.ts:94.
• B2 (untested auto-retry) — fixed: all four branches (succeed / non-transient-throw / transient-then-success / transient-then-transient) plus the emit-guard are unit-tested at the function boundary.
• N2 (classify the raw error, not the Session start failed: wrapper) — fixed at session-start-retry.ts:98 (classifyError(String(firstErr))), which restores the non-transient-throw branch.
• CLI drift — fixed: cli/src/types.ts now mirrors errorClass?: 'transient' | 'service' | 'user' exactly (same field name, optionality, literals, order) — CLAUDE.md types-sync contract satisfied.
• Governance (ADR-003) — cleared: #610 carries the approved label and covers this PR + the merged #600. (The pr/ branch name is a de-facto-waived nit — five PRs merged on pr/* branches last week.)

I independently re-verified the load-bearing invariants: classifier pattern ordering (specific before catch-all; the (?:agent_status|subtype)= widening matches both wrappers), fail-safe defaults (absent errorClass ⇒ user, isTransientError false ⇒ never auto-retry an unknown), the stuck-guard advisory-only contract (only ever none/steer, never kills), and cancel-wins (triple-guarded: registration order + early-return + dispatcher break). All correct.

One non-blocking finding (below) is worth fixing before the #247 failure-renderer lands, but it changes no live behavior today, so it doesn't gate the merge.

Non-blocking findings

N1 (Medium, but inert today) — the [auto-retried] breadcrumb is never stamped on the one path a retry actually ran-and-failed (branch 4), and the new module's docstring claims the opposite. Independently confirmed by control-flow reading + two agents.

Trace: on branch 4 (transient → retry also fails), startSessionWithRetry throws at session-start-retry.ts:116 and never returns, so orchestrate-task.ts:189 (autoRetried = retried) is unreachable. autoRetried stays false in the outer catch → retriedNote = '' (:222) → the failure persists as Session start failed: <err> with no [auto-retried]. So the marker fires in zero cases where a retry genuinely ran-and-failed — precisely the case it exists to record.

The session-start-retry.ts:64-67 docstring asserts the reverse — "the caller stamps its own [auto-retried] marker … because this function throws rather than returns on the double-failure" — but the function attaches nothing to the thrown error, so the caller cannot observe autoRetried across the throw. The docstring is misleading for the next maintainer.

Why it's non-blocking: the classifier ignores the suffix (no classification impact), and no consumer renders it on main yet — retryGuidance()/[auto-retried] ship ahead of the #247 failure-renderer. So it's a dormant diagnostic-breadcrumb bug, not a live regression. But when #247 lands, a user who hit a double-transient session-start failure would be told "reply to retry" instead of "I already tried again" — the exact confusion the marker was built to prevent. Worth fixing now while the contract is fresh.

• Fix: make the retry fact observable across the throw (attach autoRetried: true to the thrown error, or flip the caller's flag via a callback before the second attempt), and add an orchestrate-task test asserting the persisted message ends with [auto-retried] on the double-transient path (see inline suggestion). Or, if branch-4-no-stamp is genuinely intended, delete the false docstring paragraph.

N2 (Low) — untested wiring seams (both sides). pr-test-analyzer confirms orchestrate-task.ts:179-224 (does the handler actually call startSessionWithRetry and stamp the marker?) has no test — durableHandler isn't exported and the suite tests only the exported helpers. Commit 6b7cd3f0 covers the agent-side max_turns-enrichment seam, not this CDK retry seam. Agent-side twin: the between-turns hook that writes _LAST_STUCK_SUMMARY (hooks.py:110-111) — the production input to the enrichment — is bypassed by the enrichment test's monkeypatch, so deleting those two lines would leave the latch permanently None and every test stays green. Both are low-severity (the extracted units are well-covered; blast radius is ~5 lines of glue), but they're the seams a future refactor would silently break. Fixing N1 naturally closes the CDK half.

N3 (Low) — retryGuidance() fall-through branches unpinned. The retryable-user (non-guardrail) and not-retryable-user copy (error-classifier.ts:1498-1503) aren't asserted; the high-signal transient/service/guardrail copy is. Two more assertions keep the #247 contract from rotting.

Documentation

No doc/guide/ADR changes required — this is internal classifier/handler/agent logic with no new env var, command, or contract surface. Roadmap not touched (acceptable; error-classifier hardening isn't a roadmap line item). No Starlight mirror impact.

Tests & CI

• CI: all green (CodeQL ×3, dead-code advisory, secrets/deps/actions, build (agentcore), title); MERGEABLE.
• Dead-code ratchet: advisory-only (not in mise run build, continue-on-error in CI); the ErrorClassType alias it flags is genuinely used. No action.
• Coverage: strong and behavior-first — the four-branch retry matrix with .rejects.toBe() identity checks (distinguishing "re-throws the original" from "throws the retry's"), the every-classification-carries-an-errorClass invariant loop, the subtype= + Exec-format cases, the stuck-guard boundary trio (exact >=/full-window edges, K10 false-positive avoidance), and the max_turns-enrichment truth table. Gaps are the two seams in N2 + N3 copy — all non-blocking.
• Bootstrap (ADR-002): N/A — no CDK construct/stack/resource-type change (classifier + handler logic + agent Python only).
• #366 perf: clean — the changed CDK test files are pure unit tests (no new App()/Template.fromStack()/bundling).

Review agents run (Stage 3 — mandatory)

Agent	Ran?	Outcome
code-reviewer	✅	1 non-blocking → N1 (branch-4 [auto-retried] never stamped + misleading docstring). Pattern ordering, fail-safe defaults, CLI mirror exactness, advisory-only/cancel-wins all verified correct.
silent-failure-hunter	✅	No blocking silent failures. Retry-emit swallow justified+logged; non-transient re-thrown (original error); double-transient throws the retry's error; stuck-guard fail-open swallows don't alter screening/terminal resolution; max_turns enrichment is inside the crash-handled try (degrades gracefully). Independently reached the same N1.
pr-test-analyzer	✅	Prior B2 closed (4 branches + emit guard); the orchestrate-task retry seam + hooks.py latch-refresh seam still untested (N2); retryGuidance fall-through copy unpinned (N3). #366 clean.
type-design-analyzer	⏭️ omitted	ErrorClass/ErrorClassType are simple const-object enums; no complex new type.
comment-analyzer	⏭️ omitted	Comment accuracy covered by hand + code-reviewer (N1 is the comment/docstring finding).
/security-review	⏭️ omitted	No IAM/Cedar/network/secrets/input-gateway change.

Human heuristics

• Proportionality — ✅ Matches documented live incidents (ABCA-483/659/662); minimal surface; the stuck-guard is advisory-only, no kill authority.
• Coherence — ✅ ErrorClass unifies retry semantics repo-wide; the CLI mirror stays in lockstep; #600 stacks cleanly.
• Clarity — ⚠️ One drift: the session-start-retry.ts:64-67 docstring describes a caller-stamps-the-marker flow the code doesn't implement (N1).
• Appropriateness — ✅ Sound; the retry is confined to the one idempotent-by-construction step (no clone/commits/PR yet), and mid-run crashes are deliberately not retried.

Bottom line: approving. Both prior blockers resolved, governance cleared (#610 approved), invariants verified. Please address N1 (make branch 4 stamp [auto-retried] + fix the docstring, ideally with the seam test in N2) before the #247 renderer lands — it's the one finding with a future user-facing edge, though it's inert today.

_{🤖 Generated with Claude Code}

[scottschreckengaust]

Non-blocking (N1) — branch 4 throws here, so the caller never learns a retry happened. This second startSession() throws on the double-transient path, so orchestrate-task.ts:189 (autoRetried = retried) is never reached and the outer catch stamps no [auto-retried] marker — the one case a retry genuinely ran-and-failed produces no breadcrumb. The docstring at :64-67 claims "the caller stamps its own marker … because this function throws", but nothing is attached to the thrown error, so the caller can't observe it. Inert today (no renderer consumes the marker on main), but it silently breaks the contract the #247 failure-renderer will depend on.

Make the fact observable across the throw:

Suggested change

	const handle = await strategy.startSession(input);
	try {
	const handle = await strategy.startSession(input);
	return { handle, autoRetried: true };
	} catch (retryErr) {
	// Let the caller stamp `[auto-retried]` even though we throw: the retry
	// DID run and fail — that's exactly the case the marker records.
	if (retryErr instanceof Error) {
	(retryErr as Error & { autoRetried?: boolean }).autoRetried = true;
	}
	throw retryErr;
	}

Then in orchestrate-task.ts catch: const retried = autoRetried || (err as { autoRetried?: boolean })?.autoRetried === true; and add a handler test asserting the persisted error_message ends with [auto-retried] on the transient→transient path (currently untested — N2).

[isadeks]

Addressed all three nits in `2837130` (you approved, but N1 was worth fixing while the contract is fresh — thanks for the catch):

N1 (the medium one) — `[auto-retried]` now stamped on the double-transient path. You were exactly right: branch 4 throws from `startSessionWithRetry`, so the caller's `autoRetried` local was still `false` in the catch and the marker was never applied — a double-transient failure got the first-attempt copy ("reply to retry") instead of "I already retried". Fix: tag the thrown error with a `Symbol(autoRetried)` and export `isAutoRetried(err)`; `orchestrate-task.ts` reads it in the catch (`autoRetried || isAutoRetried(err)`). Corrected the docstring that claimed the fact was already "observable via the thrown-from state".

N2 — both wiring seams pinned. CDK: the branch-4 test now asserts `isAutoRetried(secondErr) === true` (+ a negative test: a first-attempt/non-object failure is NOT tagged). Agent: two new tests drive `_stuck_guard_between_turns_hook` end-to-end and assert the `_LAST_STUCK_SUMMARY` latch is written from `guard.recent_failure_summary()` (and cleared on a healthy window) — the production path the enrichment test's monkeypatch bypasses. Deleting `hooks.py:1464-1465` now fails a test.

N3 — `retryGuidance` fall-throughs pinned. Added assertions for the retryable-USER (non-guardrail) → "reply with extra guidance" and not-retryable-USER → "a retry may not resolve this" branches, built as explicit classifications so a future reclassification of a sample string can't silently un-cover them.

Full cdk (2345) + agent (1251) gates green. (One flaky `Synthesis finished with errors` in a build run did not reproduce across three clean synths — my changes are Lambda-code-only, no synth-structural change.)