fix(errors): address #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 #247 failure-renderer copy contract
can't rot.

Full cdk (2345) + agent (1251) gates green.

Author: isadeks

agent/tests/test_hooks.py

@@ -9,11 +9,14 @@
 
 from hooks import (
     _reset_blocker_reason_for_tests,
+    _stuck_guard_between_turns_hook,
     build_hook_matchers,
     detect_egress_denial,
     last_blocker_reason,
+    last_stuck_summary,
     post_tool_use_hook,
     pre_tool_use_hook,
+    reset_stuck_summary,
 )
 from policy import PolicyEngine
 
@@ -1721,6 +1724,53 @@ def test_post_tool_use_records_failures_into_the_guard(self):
         # the guard now has enough failures to steer
         assert guard.evaluate().kind == "steer"
 
+    def test_between_turns_hook_latches_stuck_summary_from_the_guard(self):
+        # #599 N2: pin the PRODUCTION write path for the _LAST_STUCK_SUMMARY latch
+        # (hooks.py:1464-1465). The enrichment tests monkeypatch the getter, so
+        # without this test deleting those two lines would leave the latch
+        # permanently None and every test would still pass. Drive the real hook
+        # with a failure-dominated guard and assert the module latch is populated.
+        from stuck_guard import WINDOW, StuckGuard
+
+        reset_stuck_summary()
+        assert last_stuck_summary() is None
+        guard = StuckGuard()
+        cmd = {"command": "mise //cdk:test"}
+        # recent_failure_summary needs a FULL window (>= WINDOW) of byte-identical
+        # failures (WINDOW_FAIL_THRESHOLD of them) — fill it past WINDOW.
+        for _ in range(WINDOW + 2):
+            guard.record_tool_result("Bash", cmd, self._oom())
+        # Precondition: the guard itself considers the window failure-dominated.
+        assert guard.recent_failure_summary() is not None
+
+        result = _stuck_guard_between_turns_hook({"stuck_guard": guard})
+        # advisory steer text returned…
+        assert isinstance(result, list)
+        # …and, crucially, the terminal-reason latch was written by the hook.
+        summary = last_stuck_summary()
+        assert summary is not None
+        assert "last tool calls repeated" in summary
+        reset_stuck_summary()
+
+    def test_between_turns_hook_clears_stuck_summary_when_not_failure_dominated(self):
+        # Symmetric guard: a recovered task (clean window) must CLEAR the latch,
+        # not leave a stale "stuck" summary that a later max_turns cap would echo.
+        # Pre-seed a stale latch via a failure-dominated guard.
+        from stuck_guard import WINDOW, StuckGuard
+
+        stuck = StuckGuard()
+        for _ in range(WINDOW + 2):
+            stuck.record_tool_result("Bash", {"command": "mise //cdk:test"}, self._oom())
+        _stuck_guard_between_turns_hook({"stuck_guard": stuck})
+        assert last_stuck_summary() is not None
+
+        # A fresh, healthy guard on the next turn clears it (recent_failure_summary None).
+        healthy = StuckGuard()
+        healthy.record_tool_result("Read", {"file": "a.py"}, "def hello(): return 1")
+        _stuck_guard_between_turns_hook({"stuck_guard": healthy})
+        assert last_stuck_summary() is None
+        reset_stuck_summary()
+
     def test_post_tool_use_record_error_never_blocks_screening(self):
         # A guard that raises on record must not break the PASS_THROUGH path.
         from stuck_guard import StuckGuard

cdk/src/handlers/orchestrate-task.ts

@@ -37,7 +37,7 @@ import {
   type PollState,
 } from './shared/orchestrator';
 import { runPreflightChecks } from './shared/preflight';
-import { startSessionWithRetry } from './shared/session-start-retry';
+import { isAutoRetried, startSessionWithRetry } from './shared/session-start-retry';
 import { deleteEcsPayload } from './shared/strategies/ecs-strategy';
 import type { TaskRecord } from './shared/types';
 import { workflowIsReadOnly, workflowRequiresRepo } from './shared/workflows';
@@ -217,9 +217,15 @@ const durableHandler: DurableExecutionHandler<OrchestrateTaskEvent, void> = asyn
       // classification). It is a breadcrumb for a FORTHCOMING failure renderer to
       // detect and surface "I already tried again" to the channel — no consumer
       // renders it yet on this branch (retryGuidance() in error-classifier.ts is
-      // the intended copy source; it ships ahead of its consumer). Only stamped
-      // when the single transient retry above also failed.
-      const retriedNote = autoRetried ? ' [auto-retried]' : '';
+      // the intended copy source; it ships ahead of its consumer).
+      //
+      // Stamp on BOTH paths a retry ran (#599 N1): branch 3 sets `autoRetried`
+      // above then a LATER step throws; branch 4 (transient-then-transient) throws
+      // FROM startSessionWithRetry before `autoRetried` is assigned, so the local
+      // is still false — read the fact off the thrown error via isAutoRetried().
+      // Without this, a double-transient failure was told "reply to retry" instead
+      // of "I already retried" — the exact confusion the marker exists to prevent.
+      const retriedNote = (autoRetried || isAutoRetried(err)) ? ' [auto-retried]' : '';
       await failTask(taskId, TaskStatus.HYDRATING, `Session start failed: ${String(err)}${retriedNote}`, task.user_id, true, task.repo);
       throw err;
     }

cdk/src/handlers/shared/session-start-retry.ts

@@ -54,6 +54,20 @@ export interface StartSessionWithRetryResult {
   readonly autoRetried: boolean;
 }
 
+/**
+ * Symbol tag pinned onto the error thrown from branch 4 (transient-then-transient)
+ * so the retry fact survives the throw — the ``autoRetried`` result field only
+ * exists on the success paths. Kept as a Symbol (not an own string prop) so it
+ * never collides with, or leaks into, the error's serialized shape. Read via
+ * {@link isAutoRetried}.
+ */
+const AUTO_RETRIED = Symbol('autoRetried');
+
+/** True iff ``err`` was thrown after an auto-retry already ran (branch 4). */
+export function isAutoRetried(err: unknown): boolean {
+  return typeof err === 'object' && err !== null && (err as Record<symbol, unknown>)[AUTO_RETRIED] === true;
+}
+
 /**
  * Start a compute session, auto-retrying ONCE on a transient failure.
  *
@@ -62,9 +76,13 @@ export interface StartSessionWithRetryResult {
  *   2. first attempt fails NON-transient → re-throw the original error (no retry).
  *   3. first attempt fails transient, retry succeeds → return it, ``autoRetried: true``.
  *   4. first attempt fails transient, retry also fails → throw the retry's error
- *      (with ``autoRetried`` observable via the thrown-from state; the caller
- *      stamps its own ``[auto-retried]`` marker — it owns ``autoRetried`` because
- *      this function throws rather than returns on the double-failure).
+ *      TAGGED with ``autoRetried = true`` (see {@link isAutoRetried}). The result
+ *      object carries ``autoRetried`` only on the success paths (1/3); on the
+ *      double-failure this function throws, so the fact is instead pinned onto the
+ *      thrown error itself — the caller reads it via {@link isAutoRetried} to stamp
+ *      the ``[auto-retried]`` marker. Without the tag the caller could not tell a
+ *      double-transient failure (retry ran) from a first-attempt failure (it did
+ *      not), and would wrongly tell the user "reply to retry" (N1).
  *
  * The ``emitRetryEvent`` call is BEST-EFFORT and internally guarded (B1): a
  * TaskEvents PutItem fault (throttle/timeout — exactly the conditions that
@@ -113,7 +131,18 @@ export async function startSessionWithRetry(
         error: emitErr instanceof Error ? emitErr.message : String(emitErr),
       });
     }
-    const handle = await strategy.startSession(input);
-    return { handle, autoRetried: true };
+    try {
+      const handle = await strategy.startSession(input);
+      return { handle, autoRetried: true };
+    } catch (retryErr) {
+      // Branch 4: the retry ALSO failed. Pin the retry fact onto the thrown error
+      // so the caller can stamp ``[auto-retried]`` (N1) — otherwise a double-
+      // transient failure is indistinguishable from a first-attempt failure and
+      // the user is wrongly told "reply to retry" instead of "I already retried".
+      if (typeof retryErr === 'object' && retryErr !== null) {
+        (retryErr as Record<symbol, unknown>)[AUTO_RETRIED] = true;
+      }
+      throw retryErr;
+    }
   }
 }

cdk/test/handlers/shared/error-classifier.test.ts

@@ -600,6 +600,39 @@ describe('classifyError', () => {
       expect(g).toMatch(/edit the request/i);
     });
 
+    // #599 N3: pin the two USER fall-through branches so the #247 failure-renderer
+    // contract can't rot silently. Built as explicit classifications (the exact
+    // category/errorClass/retryable each branch keys on) rather than relying on a
+    // sample string that might reclassify later.
+    test('retryGuidance: retryable USER (non-guardrail) → "reply here with any extra guidance"', () => {
+      const cls: ErrorClassification = {
+        category: ErrorCategory.AGENT,
+        title: 'build failed',
+        description: 'the build/test step failed',
+        remedy: 'fix the failing step',
+        retryable: true,
+        errorClass: ErrorClass.USER,
+      };
+      const g = retryGuidance(cls);
+      expect(g).toMatch(/extra guidance/i);
+      expect(g).toMatch(/try again/i);
+      expect(g).not.toMatch(/edit the request/i); // not the guardrail branch
+    });
+
+    test('retryGuidance: not-retryable USER/unknown → "a retry may not resolve this"', () => {
+      const cls: ErrorClassification = {
+        category: ErrorCategory.UNKNOWN,
+        title: 'agent reported non-success',
+        description: 'the agent finished without success',
+        remedy: 'review the task output',
+        retryable: false,
+        errorClass: ErrorClass.USER,
+      };
+      const g = retryGuidance(cls);
+      expect(g).toMatch(/may not resolve this/i);
+      expect(g).toMatch(/contact your ABCA admin/i);
+    });
+
     test('isTransientError is false for null / absent classification', () => {
       expect(isTransientError(null)).toBe(false);
       expect(isTransientError(undefined)).toBe(false);

cdk/test/handlers/shared/session-start-retry.test.ts

@@ -18,7 +18,7 @@
  */
 
 import type { SessionHandle } from '../../../src/handlers/shared/compute-strategy';
-import { startSessionWithRetry } from '../../../src/handlers/shared/session-start-retry';
+import { isAutoRetried, startSessionWithRetry } from '../../../src/handlers/shared/session-start-retry';
 
 const HANDLE: SessionHandle = {
   sessionId: 'sess-1',
@@ -79,15 +79,29 @@ describe('startSessionWithRetry — the 4 branches (#599 B2)', () => {
     expect(emitReasons).toHaveLength(1); // the session_start_retry event fired once
   });
 
-  it('4. transient then transient → throws the retry error (second failure surfaces)', async () => {
+  it('4. transient then transient → throws the retry error TAGGED autoRetried (#599 N1)', async () => {
     const secondErr = new Error('TaskDefinition is inactive (again)');
     const startSession = jest
       .fn()
       .mockRejectedValueOnce(TRANSIENT)
       .mockRejectedValueOnce(secondErr);
     const { d } = deps();
+    // The double-transient error must carry the retry fact across the throw so the
+    // caller stamps `[auto-retried]` (a first-attempt failure must NOT be tagged).
     await expect(startSessionWithRetry({ startSession }, {} as never, d)).rejects.toBe(secondErr);
     expect(startSession).toHaveBeenCalledTimes(2);
+    expect(isAutoRetried(secondErr)).toBe(true);
+  });
+
+  it('isAutoRetried is false for a first-attempt (non-transient) failure and non-objects', async () => {
+    // Branch 2's re-thrown error ran no retry → must NOT be tagged, else the caller
+    // would wrongly tell the user "I already retried".
+    const startSession = jest.fn().mockRejectedValueOnce(NON_TRANSIENT);
+    const { d } = deps();
+    await expect(startSessionWithRetry({ startSession }, {} as never, d)).rejects.toBe(NON_TRANSIENT);
+    expect(isAutoRetried(NON_TRANSIENT)).toBe(false);
+    expect(isAutoRetried(undefined)).toBe(false);
+    expect(isAutoRetried('a string error')).toBe(false);
   });
 });