diff --git a/coworker/engine.py b/coworker/engine.py index 3c3e7d8d..78a88316 100644 --- a/coworker/engine.py +++ b/coworker/engine.py @@ -214,6 +214,17 @@ class TurnEngine: for p in msg["content"] ) + def _tail_is_retriable_error(self) -> bool: + """True when the history tail is an error notice, looking through any model_switch + notices appended after it (a switch must not consume the retry).""" + for message in reversed(self.messages): + if message.get("role") != "notice": + return False + if message.get("kind") == "model_switch": + continue + return message.get("kind") == "error" + return False + def _append_notice(self, kind: str, text: Optional[str] = None) -> None: """Persist a turn-ending marker (error/interrupted) as a display-only `notice` message: it survives reload like the transcript does, but `_outbound_messages` @@ -226,13 +237,10 @@ class TurnEngine: async def retry(self) -> AsyncIterator[Event]: """Re-run the model loop after a provider error — no new user message; the failed turn's input is already the tail of history. Guarded on the tail being an error - notice so a stray retry frame can't re-answer a completed turn.""" - last = self.messages[-1] if self.messages else None - if not ( - isinstance(last, dict) - and last.get("role") == "notice" - and last.get("kind") == "error" - ): + notice so a stray retry frame can't re-answer a completed turn. Trailing + model_switch notices don't break the guard — switching models and THEN retrying + is the intended recovery path (owner-hit 2026-07-23).""" + if not self._tail_is_retriable_error(): return self._cancel.clear() yield Event(EventType.TURN_START, {"input": ""}) diff --git a/surfaces/gui/e2e/error-retry.spec.ts b/surfaces/gui/e2e/error-retry.spec.ts index 12fd3395..5dbe17bb 100644 --- a/surfaces/gui/e2e/error-retry.spec.ts +++ b/surfaces/gui/e2e/error-retry.spec.ts @@ -25,3 +25,25 @@ test("provider error shows a retriable notice; Retry re-runs without a new user // …and the button is gone now that the error notice is no longer the transcript tail. await expect(page.getByTestId("notice-retry")).toHaveCount(0); }); + +test("Retry survives a model switch — the intended recovery path", async ({ page }) => { + await page.goto("/"); + await page.getByText("Draft the launch note").first().click(); + const box = page.getByPlaceholder(/Ask the coworker/); + await box.fill("please fail the turn"); + await box.press("Enter"); + await expect(page.getByTestId("notice-retry")).toBeVisible({ timeout: 10_000 }); + + // Switch models: the info marker lands AFTER the error — Retry must stay offered + // (owner-hit 2026-07-23: the switch notices consumed it). + const picker = page.locator(".dd").filter({ hasText: "Claude Opus 4.8" }); + await picker.locator(".pill").click(); + await page.locator(".dd-item").filter({ hasText: "GPT-5.5" }).click(); + await expect(page.getByText(/Model switched to gpt-5.5/).first()).toBeVisible(); + const retry = page.getByTestId("notice-retry"); + await expect(retry).toBeVisible(); + + await retry.click(); + await expect(page.getByText("Recovered after retry.").first()).toBeVisible({ timeout: 10_000 }); + await expect(page.getByTestId("notice-retry")).toHaveCount(0); +}); diff --git a/surfaces/gui/src/components/Transcript.tsx b/surfaces/gui/src/components/Transcript.tsx index b8927faa..c6c95be1 100644 --- a/surfaces/gui/src/components/Transcript.tsx +++ b/surfaces/gui/src/components/Transcript.tsx @@ -313,6 +313,19 @@ interface Props { onRetry?: () => void; } +// The transcript index whose notice gets the Retry button: the tail error notice, looking +// through info notices after it (model switches must not consume the retry — switching +// models and THEN retrying is the intended recovery path). -1 when the tail is anything else. +export function retryAnchor(items: Item[]): number { + for (let i = items.length - 1; i >= 0; i--) { + const it = items[i]; + if (it.kind !== "notice") return -1; + if (it.retriable) return i; + if (it.tone !== "info") return -1; + } + return -1; +} + export function Transcript({ items, running, streamingText, onRetry }: Props) { // §33 grouping: a turn = the maximal run of assistant/tool/resolved-approval items between // breakers (user, connector, notices, plan/dir requests…). Trailing assistant texts are the @@ -435,7 +448,7 @@ export function Transcript({ items, running, streamingText, onRetry }: Props) { return (
{item.text} - {item.retriable && !running && onRetry && block.i === items.length - 1 && ( + {item.retriable && !running && onRetry && block.i === retryAnchor(items) && ( diff --git a/tests/test_engine_stop.py b/tests/test_engine_stop.py index cee2c62a..7c52eb7b 100644 --- a/tests/test_engine_stop.py +++ b/tests/test_engine_stop.py @@ -353,3 +353,35 @@ def test_stop_during_thinking_keeps_partial_reasoning(tmp_path): assert events[-1].type == EventType.INTERRUPTED partial = engine.messages[-2] # [-1] is the interrupted notice assert partial["role"] == "assistant" and partial["reasoning"].startswith("t0 ") + + +def test_retry_survives_model_switches(tmp_path): + """Error → switch models (one or more times) → Retry must still re-run, on the NEW + model (owner-hit 2026-07-23: the switch notices consumed the retry guard).""" + provider = FlakyProvider(failures=1) + engine = TurnEngine( + provider=provider, + registry=ToolRegistry(), + permissions=PermissionEngine(workspace_root=tmp_path), + model="gemini:gemini-3.6-flash", + ) + + async def scenario(): + first = [ev async for ev in engine.run("hello")] + assert engine.switch_model("gemini:gemini-3.1-pro-preview") is not None + assert engine.switch_model("gpt-5.6-sol") is not None + second = [ev async for ev in engine.retry()] + return first, second + + first, second = asyncio.run(scenario()) + assert first[-1].type == EventType.ERROR + assert second[-1].type == EventType.TURN_END + assert engine.model == "gpt-5.6-sol" + assert engine.messages[-1]["content"] == "recovered" + # Still exactly one user message; and a completed session stays retry-proof. + assert sum(1 for m in engine.messages if m.get("role") == "user") == 1 + assert asyncio.run(_drain_retry(engine)) == [] + + +async def _drain_retry(engine): + return [ev async for ev in engine.retry()]