From db6f775dfff405e1514ec02fedb0450d42c7dad2 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 22 Sep 2026 10:26:37 -0500 Subject: [PATCH] fix: keep quarantined chat retries disabled after continuation Co-Authored-By: Paperclip --- tests/runner-e2e/README.md | 5 +++-- tests/runner-e2e/catalog.ts | 2 +- tests/runner-e2e/chat-qualification.ts | 4 +++- ui/src/components/TaskChatThread.test.tsx | 14 ++++++++++++++ ui/src/components/TaskChatThread.tsx | 1 + ui/src/components/task-chat/TaskChatThreadView.tsx | 2 +- ui/src/components/task-chat/task-chat-model.ts | 2 ++ 7 files changed, 25 insertions(+), 5 deletions(-) diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index fb1500d600d..15b642759e3 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -910,7 +910,8 @@ The fixture then releases the read-only brief wait and sends a new chat message that records the known saved-plan and interrupted-command outcomes. The server must verify that the recorded worker and provider process groups stopped before admitting exactly one successful fresh session. The answer must contain the -reference supplied only after the crash, and the saved plan must remain unchanged. +reference supplied only after the crash, and the saved plan must remain unchanged. The old quarantined run must not regain +a misleading Try again control after the fresh turn succeeds. This qualifies **explicit conversation continuation after local worker loss**. It does not qualify replay of uncertain actions, automatic recovery, remote worker loss, or exact-session resumption. It budgets two provider runs. Unexpected @@ -939,6 +940,6 @@ it does not certify a native option in the wizard, which is not offered yet. Current proof and remaining decisions are recorded in [the 21 September qualification report](QUALIFICATION-2026-09-21.md). In particular, -the original worker-loss attempts quarantined both providers. The version 8 crash +the original worker-loss attempts quarantined both providers. The version 9 crash eval requires a usable fresh conversation after verified cleanup. A passing quarantine guard alone is not a recovered workflow. diff --git a/tests/runner-e2e/catalog.ts b/tests/runner-e2e/catalog.ts index b93386a10ce..28fa16c2d36 100644 --- a/tests/runner-e2e/catalog.ts +++ b/tests/runner-e2e/catalog.ts @@ -975,7 +975,7 @@ export const runnerSuites: readonly RunnerSuiteFixture[] = [ profiles: runnerProfiles.filter(profile => ["runner-codex", "runner-acpx-claude"].includes(profile.id)) .map(profile => productionStoryProfile(defaultPermissionProfile(profile))), environments: [localEnvironment], tasks: chatQualificationTasks, expectedMatrixSize: 6, - definitionMetadata: { version: 8, permissions: "production-defaults", instructions: "production", crashBoundary: "verified-native-worker-pid-at-file-wait", recovery: "new-user-message-after-verified-cleanup", answerGrading: "exact-grounded-propositions-plus-separate-semantic-review", scheduling: "explicit-only" }, + definitionMetadata: { version: 9, permissions: "production-defaults", instructions: "production", crashBoundary: "verified-native-worker-pid-at-file-wait", recovery: "new-user-message-after-verified-cleanup", answerGrading: "exact-grounded-propositions-plus-separate-semantic-review", scheduling: "explicit-only" }, }, ...(process.env.PAPERCLIP_RUNNER_E2E_CONNECTION_REVIEWS === "1" ? [connectionReviewSuite] : []), { diff --git a/tests/runner-e2e/chat-qualification.ts b/tests/runner-e2e/chat-qualification.ts index 484b4c36b0c..a57cb170d3b 100644 --- a/tests/runner-e2e/chat-qualification.ts +++ b/tests/runner-e2e/chat-qualification.ts @@ -171,7 +171,7 @@ export async function runWorkerCrash(context: Context) { await input.page.reload({ waitUntil: "domcontentloaded" }); await expect(input.page.getByTestId("task-chat-composer-input")).toBeVisible(); await expect(input.page.getByRole("status", { name: "Task recovery" }).getByRole("link", { name: "Inspect run" })).toBeVisible(); - await expect(input.page.getByRole("button", { name: "Retry", exact: true })).toHaveCount(0); + await expect(input.page.getByRole("button", { name: /^(Retry|Try again)$/ })).toHaveCount(0); const refused = await input.api.request.post(`/api/agents/${input.fixtures.agent.id}/wakeup`, { data: { failedRunId: failed.id, reason: "retry_failed_run" }, }); @@ -195,6 +195,8 @@ export async function runWorkerCrash(context: Context) { planAfter: await input.api.get(`/api/issues/${context.issue().id}/documents/plan`) }; await input.evidence("chat-worker-recovery.json", e); assertCrashRecovered(e); + // A fresh success must not re-enable Retry on the quarantined historical run. + await expect(input.page.getByTestId("task-chat-run-failed-try-again")).toHaveCount(0); expect(await input.api.get(`/api/companies/${input.fixtures.company.id}/issues`)).toEqual([]); } finally { await writeFile(wait.gate, reference); diff --git a/ui/src/components/TaskChatThread.test.tsx b/ui/src/components/TaskChatThread.test.tsx index d8c060cfa7e..1df30e6039c 100644 --- a/ui/src/components/TaskChatThread.test.tsx +++ b/ui/src/components/TaskChatThread.test.tsx @@ -1170,6 +1170,20 @@ describe("TaskChatThread runtime transcript selection", () => { expect(onRetryFailedRun).toHaveBeenCalledExactlyOnceWith("failed-bootstrap"); }); + it.each([false, true])("never offers the old quarantined run as Try again after continuation: %s", continued => { + render( {}} issueStatus="in_progress" + onRetryFailedRun={vi.fn()} linkedRuns={[{ + runId: "quarantined", runtimeMode: "native", status: "failed", errorCode: "native_session_cleanup_quarantined", + agentId: "agent-1", agentName: "Runner", adapterType: "paperclip_runner", + createdAt: "2026-08-25T18:00:00.000Z", startedAt: "2026-08-25T18:00:00.000Z", finishedAt: "2026-08-25T18:00:02.000Z", + ...(continued ? { execution: { phase: "completed" as const, label: "Continued in another run", cause: "native_session_cleanup_quarantined", + lastConfirmedActivityAt: null, retryAt: null, attempt: 2, maxAttempts: 3, recoveryOwner: null, nextAction: null, + permittedActions: ["inspect_run" as const], predecessorRunId: null, successorRunId: "fresh-run" } } : {}), + }]} />); + expect(container.textContent).toContain("Run failed"); + expect(container.querySelector('[data-testid="task-chat-run-failed-try-again"]')).toBeNull(); + }); + it("explains a native provider usage limit without exposing its error code", async () => { const onRetryFailedRun = vi.fn(); render( diff --git a/ui/src/components/TaskChatThread.tsx b/ui/src/components/TaskChatThread.tsx index 7047ab14184..6c468c25625 100644 --- a/ui/src/components/TaskChatThread.tsx +++ b/ui/src/components/TaskChatThread.tsx @@ -1614,6 +1614,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { tone: source.status === "cancelled" ? "neutral" : "error", label, detail, + retryable: code !== "native_session_cleanup_quarantined", collapsible: true, runId: source.id, createdAtIso: finishedAt diff --git a/ui/src/components/task-chat/TaskChatThreadView.tsx b/ui/src/components/task-chat/TaskChatThreadView.tsx index 52d900ecbfc..0046380e0e4 100644 --- a/ui/src/components/task-chat/TaskChatThreadView.tsx +++ b/ui/src/components/task-chat/TaskChatThreadView.tsx @@ -172,7 +172,7 @@ function renderItem( onRetryFailedRun(item.runId!) : onTryAgainNoLiveExecutionPath diff --git a/ui/src/components/task-chat/task-chat-model.ts b/ui/src/components/task-chat/task-chat-model.ts index 9dbab8ada14..717b9b36b56 100644 --- a/ui/src/components/task-chat/task-chat-model.ts +++ b/ui/src/components/task-chat/task-chat-model.ts @@ -246,6 +246,8 @@ export interface TaskChatMarkerItem { variant: "session_start" | "interrupted" | "turn_boundary"; label: string; detail?: string; + /** False when the recorded run cannot be retried, even after the chat continues. */ + retryable?: boolean; /** Renders the marker as a quiet disclosure row with detail beneath it. */ collapsible?: boolean; /** Expected cancellation is neutral; unexpected failures remain destructive. */