From 94cce9dda6a89fc116e05b1b4a01976f8f067fb3 Mon Sep 17 00:00:00 2001 From: Yash Date: Mon, 14 Sep 2026 23:17:34 +0000 Subject: [PATCH] fix(harness): preserve operation authority across Resume retries --- .changeset/assistant-ui-operation-retries.md | 5 + packages/harness/docs/assistant-history.md | 6 + .../harness/web/e2e/assistant-end.spec.ts | 122 ++++++++++++++++++ .../harness/web/e2e/assistant-history.spec.ts | 75 ++++++++++- packages/harness/web/src/App.tsx | 4 +- .../components/AssistantHistoryActions.tsx | 70 +++++++--- .../web/src/lib/assistant-resume-client.ts | 28 +++- .../harness/web/src/lib/use-harness-state.ts | 28 ++-- 8 files changed, 303 insertions(+), 35 deletions(-) create mode 100644 .changeset/assistant-ui-operation-retries.md diff --git a/.changeset/assistant-ui-operation-retries.md b/.changeset/assistant-ui-operation-retries.md new file mode 100644 index 000000000..69ca8c0ec --- /dev/null +++ b/.changeset/assistant-ui-operation-retries.md @@ -0,0 +1,5 @@ +--- +"@sapiom/harness": patch +--- + +Preserve uncertain Assistant Resume requests and fence stale Terminal Resume and End acknowledgements. diff --git a/packages/harness/docs/assistant-history.md b/packages/harness/docs/assistant-history.md index 18660e13f..81c7a81a7 100644 --- a/packages/harness/docs/assistant-history.md +++ b/packages/harness/docs/assistant-history.md @@ -189,3 +189,9 @@ or confirmed removal, and other sessions keep their own process and binding. Brief part selection keeps the latest useful Assistant text, tool results and attachments even when blank or omitted control parts follow them. Omission counts remain explicit without incorrectly labeling every removed part as earlier. + +End and ordinary Terminal Resume acknowledgements yield to newer operations on +the same Studio. An uncertain Assistant Resume keeps its original revision and +operation ID across its own lifecycle event and Close/Back navigation; Retry +Resume reconciles that request. Only a verified lifecycle conflict permits a new +availability check. diff --git a/packages/harness/web/e2e/assistant-end.spec.ts b/packages/harness/web/e2e/assistant-end.spec.ts index c95e4bf4f..79ff7d123 100644 --- a/packages/harness/web/e2e/assistant-end.spec.ts +++ b/packages/harness/web/e2e/assistant-end.spec.ts @@ -98,6 +98,16 @@ async function openStudio(page: Page) { ? { ok: true } : { failure: { code: "cleanup_unconfirmed" } }, }); + if (status === 200) + await expect + .poll(() => + page.evaluate( + () => (window as any).__HARNESS_TEST__.killSessionCalls ?? [], + ), + ) + .toContain( + new URL(requests[0]!.request().url()).pathname.split("/").at(-1), + ); }, }; } @@ -308,3 +318,115 @@ test("late End cannot remove or select a row after an auth barrier", async ({ await expect(page.locator(".harness-terminal")).toBeVisible(); expect(fixture.unexpected).toEqual([]); }); + +async function controlledTerminalResume(page: Page) { + const requests: Route[] = []; + await page.route("**/api/sessions/*/resume", (route) => { + requests.push(route); + }); + await page.evaluate(async () => { + const url = performance + .getEntriesByType("resource") + .find( + (entry) => new URL(entry.name).pathname === "/src/lib/api.ts", + )!.name; + const { MockApi } = await import(url); + MockApi.prototype.resumeSession = async function (id: string) { + const response = await fetch( + `/api/sessions/${encodeURIComponent(id)}/resume`, + { method: "POST" }, + ); + const session = await response.json(); + (window as any).__HARNESS_TEST__.terminalResumeResponse = session.title; + return session; + }; + }); + return requests; +} +const resumable = (title = "Resumed Terminal") => + session("sess-boot", { + title, + agentSessionId: "8f2b1c6a-4d3e-4a11-9c2f-1a2b3c4d5e6f", + }); +const terminalExited = (page: Page) => + publish(page, { + type: "session.status", + session: { ...resumable(), status: "exited", ready: false }, + }); + +test("late End ACK cannot overwrite a newer ordinary Terminal Resume without an Assistant bus transition", async ({ + page, +}) => { + const fixture = await openStudio(page); + const resumes = await controlledTerminalResume(page); + await project(page, 1, [lifecycle("sess-boot")]); + await openEnd(page); + await page.getByTestId("end-session-confirm-btn").click(); + await expect.poll(() => fixture.requests.length).toBe(1); + await terminalExited(page); + await page.getByTestId("dead-session-resume").click(); + await expect.poll(() => resumes.length).toBe(1); + await resumes[0]!.fulfill({ json: resumable() }); + await expect(page.locator(".harness-terminal")).toBeVisible(); + await fixture.settle(200); + await expect(selected(page)).toHaveAttribute("data-session-id", "sess-boot"); + await expect(page.getByTestId("session-tab-sess-boot")).toContainText( + "Resumed Terminal", + ); + await expect(page.locator(".harness-terminal")).toBeVisible(); +}); + +for (const barrier of ["End", "account", "newer bus"] as const) { + test(`an ordinary Terminal Resume response yields to ${barrier}`, async ({ + page, + }) => { + const fixture = await openStudio(page); + const resumes = await controlledTerminalResume(page); + await project(page, 1, [lifecycle("sess-boot")]); + await terminalExited(page); + await page.getByTestId("dead-session-resume").click(); + await expect.poll(() => resumes.length).toBe(1); + if (barrier === "End") { + await openEnd(page); + await page.getByTestId("end-session-confirm-btn").click(); + await fixture.settle(200); + await expect(selected(page)).not.toHaveAttribute( + "data-session-id", + "sess-boot", + ); + } else { + if (barrier === "account") + await publish(page, { + type: "auth.changed", + authenticated: true, + organizationName: "Account B", + }); + await publish(page, { + type: "session.status", + session: resumable("Newer current Terminal"), + }); + } + await resumes[0]!.fulfill({ json: resumable("Stale Terminal response") }); + await expect + .poll(() => + page.evaluate( + () => (window as any).__HARNESS_TEST__.terminalResumeResponse, + ), + ) + .toBe("Stale Terminal response"); + await expect( + page.getByText("Stale Terminal response", { exact: true }), + ).toHaveCount(0); + if (barrier === "End") { + await expect(selected(page)).not.toHaveAttribute( + "data-session-id", + "sess-boot", + ); + await expect(page.getByTestId("session-tab-sess-boot")).toHaveCount(0); + } else { + await expect(page.getByTestId("session-tab-sess-boot")).toContainText( + "Newer current Terminal", + ); + } + }); +} diff --git a/packages/harness/web/e2e/assistant-history.spec.ts b/packages/harness/web/e2e/assistant-history.spec.ts index 822457bdb..3dff8074c 100644 --- a/packages/harness/web/e2e/assistant-history.spec.ts +++ b/packages/harness/web/e2e/assistant-history.spec.ts @@ -413,7 +413,7 @@ const review = async (page: Page, id = "assistant-only") => { const checkResume = (page: Page) => page.getByRole("button", { name: "Check Resume availability", exact: true }); const resumeButton = (page: Page) => - page.getByRole("button", { name: "Resume Assistant", exact: true }); + page.getByRole("button", { name: /^(Resume Assistant|Retry Resume)$/ }); test("Resume verifies availability, preserves identity and opens paused without submitting work", async ({ page, @@ -516,7 +516,9 @@ test("uncertain Resume retries the same operation and accepts reconciled later r .getByRole("button", { name: "Close", exact: true }) .click(); await page.getByRole("button", { name: "Go back" }).click(); - await checkResume(page).click(); + await expect( + page.getByRole("button", { name: "Retry Resume", exact: true }), + ).toBeEnabled(); probe.failResume = false; probe.resumedRevision = 4; await resumeButton(page).click(); @@ -553,7 +555,7 @@ for (const barrier of ["navigation", "account", "lifecycle"] as const) { }, }); await probe.resumes[0]!.fulfill({ json: probe.resumed("assistant-only") }); - if (barrier === "lifecycle") await expect(checkResume(page)).toBeEnabled(); + if (barrier === "lifecycle") await expect(resumeButton(page)).toBeEnabled(); else await expect(page.getByTestId("assistant-history-pane")).toHaveCount(0); await expect( @@ -652,3 +654,70 @@ test("resumed Assistant yields to newer foreground Terminal work", async ({ ).toHaveAttribute("aria-pressed", "true"); await expect(page.locator(".harness-terminal")).toBeVisible(); }); + +for (const revisit of [false, true]) { + test(`Resume keeps the original revision after its bus commit and lost ACK${revisit ? " across Close and Back" : ""}`, async ({ + page, + }) => { + const probe = await setup(page); + probe.holdResume = true; + await review(page); + await checkResume(page).click(); + await resumeButton(page).click(); + await expect.poll(() => probe.resumes.length).toBe(1); + const original = probe.resumes[0]!.request().postDataJSON(); + await publish(page, { + type: "assistant.state", + snapshot: { + hostInstanceId: "history-host", + authorityRevision: "account-a", + revision: 2, + enabled: true, + sessions: [], + lifecycles: [probe.resumed("assistant-only").attachment.lifecycle], + }, + }); + await probe.resumes[0]!.fulfill({ status: 503, json: {} }); + await expect(page.getByRole("alert")).toContainText( + "Retry to check the same operation", + ); + if (revisit) { + await page + .getByTestId("assistant-history-pane") + .getByRole("button", { name: "Close", exact: true }) + .click(); + await page.getByRole("button", { name: "Go back" }).click(); + } + await page + .getByRole("button", { name: "Retry Resume", exact: true }) + .click(); + await expect.poll(() => probe.resumes.length).toBe(2); + expect(probe.resumes[1]!.request().postDataJSON()).toEqual(original); + expect(original.expectedRevision).toBe(2); + expect(probe.inspections).toHaveLength(1); + probe.resumedRevision = 4; + await probe.resumes[1]!.fulfill({ json: probe.resumed("assistant-only") }); + await expect(page.getByText("Paused", { exact: true })).toBeVisible(); + }); +} + +test("a definite Resume conflict permits a fresh availability check without auto dispatch", async ({ + page, +}) => { + const probe = await setup(page); + probe.holdResume = true; + await review(page); + await checkResume(page).click(); + await resumeButton(page).click(); + await expect.poll(() => probe.resumes.length).toBe(1); + await probe.resumes[0]!.fulfill({ + status: 409, + json: { failure: openCodeTransportFailure("lifecycle_changed") }, + }); + await expect(checkResume(page)).toBeEnabled(); + await expect(resumeButton(page)).toHaveCount(0); + expect(probe.resumes).toHaveLength(1); + await expect(page.getByTestId("assistant-transcript")).toContainText( + "Saved answer assistant-only", + ); +}); diff --git a/packages/harness/web/src/App.tsx b/packages/harness/web/src/App.tsx index 848934d9b..b55500fac 100644 --- a/packages/harness/web/src/App.tsx +++ b/packages/harness/web/src/App.tsx @@ -88,7 +88,7 @@ import { Terminal } from "./components/Terminal"; import { AssistantPane } from "./components/AssistantPane"; import { AssistantHistoryPane } from "./components/AssistantHistoryPane"; import type { AssistantHistoryEntry } from "../../src/shared/assistant-history"; -import { AssistantHistoryActions } from "./components/AssistantHistoryActions"; +import { AssistantHistoryActions, type AssistantResumeOperation } from "./components/AssistantHistoryActions"; import { AssistantContinueAction } from "./components/AssistantContinueAction"; import { DormantTerminalPane } from "./components/DormantTerminalPane"; import type { ChatDraftStore } from "./components/OpenCodeChat"; @@ -742,7 +742,7 @@ export const App = (): JSX.Element => { const [assistantReview, setAssistantReview] = useState<{ entry: AssistantHistoryEntry; authority: string; navigation: number; } | null>(null); - const assistantResumeOperations = useMemo(() => new Map(), [harness.assistantHistoryAuthority, harness.bootToken]); + const assistantResumeOperations = useMemo(() => new Map(), [harness.assistantHistoryAuthority, harness.bootToken]); const setReviewSummary = useCallback((summary: SessionSummary | null) => { setAssistantReview(null); setTerminalReviewSummary(summary); diff --git a/packages/harness/web/src/components/AssistantHistoryActions.tsx b/packages/harness/web/src/components/AssistantHistoryActions.tsx index 235ab2d79..bdf4b743e 100644 --- a/packages/harness/web/src/components/AssistantHistoryActions.tsx +++ b/packages/harness/web/src/components/AssistantHistoryActions.tsx @@ -1,7 +1,15 @@ import { useEffect, useRef, useState } from "react"; import type { AssistantHistoryEntry } from "../../../src/shared/assistant-history"; import type { AssistantLifecycle } from "../../../src/shared/assistant-session"; -import { inspectAssistant } from "../lib/assistant-resume-client"; +import { + inspectAssistant, + isAssistantLifecycleConflict, +} from "../lib/assistant-resume-client"; + +export interface AssistantResumeOperation { + entry: AssistantHistoryEntry; + operationId: string; +} export function AssistantHistoryActions({ entry, @@ -13,7 +21,7 @@ export function AssistantHistoryActions({ entry: AssistantHistoryEntry; lifecycle?: AssistantLifecycle; bootToken: string; - operations: Map; + operations: Map; onResume: ( entry: AssistantHistoryEntry, operationId: string, @@ -48,12 +56,11 @@ export function AssistantHistoryActions({ busy === "resume" ) return; - operations.delete(`${entry.harnessSessionId}:${observedRevision.current}`); observedRevision.current = current.lifecycle.revision; request.current?.abort(); request.current = null; setInspected(null); - setFailure(null); + if (!operations.has(entry.harnessSessionId)) setFailure(null); setBusy(null); }, [entry.harnessSessionId, current.lifecycle.revision, busy, operations]); const run = async (action: "inspect" | "resume") => { @@ -66,22 +73,45 @@ export function AssistantHistoryActions({ if (action === "inspect") { const next = await inspectAssistant(current, bootToken, abort.signal); if (!abort.signal.aborted) setInspected(next); - } else if (inspected?.nativeResume === "available") { - const key = `${entry.harnessSessionId}:${inspected.lifecycle.revision}`; - const operationId = operations.get(key) ?? crypto.randomUUID(); - operations.set(key, operationId); - const opened = await onResume(inspected, operationId, abort.signal); - if (!abort.signal.aborted && opened) operations.delete(key); + } else { + const key = entry.harnessSessionId; + const operation = + operations.get(key) ?? + (inspected?.nativeResume === "available" + ? { entry: inspected, operationId: crypto.randomUUID() } + : null); + if (!operation) return; + // Keep the original revision even when our own commit arrives over the + // bus before an HTTP failure. A retry reconciles that exact operation. + operations.set(key, operation); + const opened = await onResume( + operation.entry, + operation.operationId, + abort.signal, + ); + if ( + !abort.signal.aborted && + opened && + operations.get(key) === operation + ) + operations.delete(key); else if (!abort.signal.aborted) - setFailure("This session changed. Check Resume availability again."); + setFailure( + "Your selection or session changed. Return here to retry the original Resume request.", + ); } } catch (error) { - if (!abort.signal.aborted) + if (!abort.signal.aborted) { + if (isAssistantLifecycleConflict(error)) { + operations.delete(entry.harnessSessionId); + setInspected(null); + } setFailure( error instanceof Error ? error.message : "Assistant is unavailable. Check again.", ); + } } finally { if (request.current === abort) { request.current = null; @@ -89,7 +119,8 @@ export function AssistantHistoryActions({ } } }; - const available = inspected?.nativeResume === "available"; + const retry = operations.has(entry.harnessSessionId); + const available = retry || inspected?.nativeResume === "available"; return (
{current.lifecycle.lifecycle === "ending" ? ( @@ -107,14 +138,17 @@ export function AssistantHistoryActions({ ? "Checking Resume availability…" : busy === "resume" ? "Resuming Assistant…" - : available - ? "Resume Assistant" - : "Check Resume availability"} + : retry + ? "Retry Resume" + : available + ? "Resume Assistant" + : "Check Resume availability"} {available && (

- Restores the same conversation. Restarted Assistant stays paused - until you send a message. + {retry + ? "Retry checks the original Resume request, including its saved revision." + : "Restores the same conversation. Restarted Assistant stays paused until you send a message."}

)} {inspected && !available && ( diff --git a/packages/harness/web/src/lib/assistant-resume-client.ts b/packages/harness/web/src/lib/assistant-resume-client.ts index 7401b111f..a2862c779 100644 --- a/packages/harness/web/src/lib/assistant-resume-client.ts +++ b/packages/harness/web/src/lib/assistant-resume-client.ts @@ -2,7 +2,10 @@ import { z } from "zod"; import type { HarnessSession } from "@shared/types"; import type { AssistantHistoryEntry } from "../../../src/shared/assistant-history"; import { parseAssistantLifecycle } from "../../../src/shared/assistant-session"; -import { parseOpenCodeTransportFailure } from "../../../src/shared/opencode-errors"; +import { + parseOpenCodeTransportFailure, + type OpenCodeTransportFailure, +} from "../../../src/shared/opencode-errors"; import { parseAssistantHistoryEntry } from "./assistant-history-client"; const id = z.string().regex(/^[A-Za-z0-9][A-Za-z0-9._-]{0,255}$/); @@ -33,6 +36,23 @@ const object = (value: unknown): Record => ? (value as Record) : {}; +export class AssistantActionError extends Error { + constructor( + readonly status: number, + readonly failure: OpenCodeTransportFailure | null, + ) { + super( + failure?.message ?? + "Assistant could not be restored. Your saved history is still available. Retry to check the same operation.", + ); + } +} + +export const isAssistantLifecycleConflict = (error: unknown): boolean => + error instanceof AssistantActionError && + error.status === 409 && + error.failure?.code === "lifecycle_changed"; + export async function assistantActionRequest( entry: AssistantHistoryEntry, action: "inspect" | "resume" | "continue", @@ -56,9 +76,9 @@ export async function assistantActionRequest( ); const value = object(await response.json().catch(() => null)); if (!response.ok) - throw new Error( - parseOpenCodeTransportFailure(value.failure ?? value.error)?.message ?? - "Assistant could not be restored. Your saved history is still available. Retry to check the same operation.", + throw new AssistantActionError( + response.status, + parseOpenCodeTransportFailure(value.failure ?? value.error), ); return value; } diff --git a/packages/harness/web/src/lib/use-harness-state.ts b/packages/harness/web/src/lib/use-harness-state.ts index e08ca3d06..713af6168 100644 --- a/packages/harness/web/src/lib/use-harness-state.ts +++ b/packages/harness/web/src/lib/use-harness-state.ts @@ -1645,26 +1645,37 @@ export function useHarnessState(): HarnessStateHook { const resumeSession = useCallback( async (harnessSessionId: string): Promise => { const seqAtStart = switchSeqRef.current; + const authority = assistantAuthorityKey(), auth = authoritySequence.current; + const source = sessionsRef.current.find((row) => row.id === harnessSessionId); + const operation = (sessionOperationGenerations.current.get(harnessSessionId) ?? 0) + 1; + sessionOperationGenerations.current.set(harnessSessionId, operation); + const status = sessionStatusRevisions.current.get(harnessSessionId) ?? 0; + const current = () => auth === authoritySequence.current && authority === assistantAuthorityKey() && + operation === sessionOperationGenerations.current.get(harnessSessionId); try { const session = await api.resumeSession(harnessSessionId); + const row = sessionsRef.current.find((candidate) => candidate.id === harnessSessionId); + if (!current() || session.id !== harnessSessionId || !row || (source && (!sameStudioAuthority(source, row) || !sameStudioAuthority(source, session)))) return session; + const changed = status !== (sessionStatusRevisions.current.get(harnessSessionId) ?? 0); + const latest = changed ? sessionStatusSnapshots.current.get(harnessSessionId) : session; setState((prev) => prev ? { ...prev, sessions: prev.sessions.map((s) => - s.id === session.id ? session : s, + s.id === session.id && !changed ? session : s, ), } : prev, ); // Only claim focus if the user hasn't explicitly switched sessions // while the resume was in flight — their pick outranks this resolve. - if (switchSeqRef.current === seqAtStart) selectSession(session.id); - return session; + if (switchSeqRef.current === seqAtStart && latest?.status !== "exited") selectSession(session.id); + return latest ?? session; } catch (err) { // Surface resume failures as a toast so a failed resume is never silent // (the caller fires this with void and swallows the rejection). - setToast( + if (current()) setToast( createToastMessage( err instanceof ApiError && err.reason ? err.reason @@ -1674,7 +1685,7 @@ export function useHarnessState(): HarnessStateHook { throw err; } }, - [selectSession], + [selectSession, assistantAuthorityKey], ); const restartMcpSession = useCallback( @@ -1902,7 +1913,8 @@ export function useHarnessState(): HarnessStateHook { const closeSession = useCallback( async (id: string): Promise => { if (endingSessions.current.has(id)) return; - sessionOperationGenerations.current.set(id, (sessionOperationGenerations.current.get(id) ?? 0) + 1); + const operation = (sessionOperationGenerations.current.get(id) ?? 0) + 1; + sessionOperationGenerations.current.set(id, operation); endingSessions.current.add(id); setEndingSessionIds(new Set(endingSessions.current)); const selectionAtStart = switchSeqRef.current; @@ -1915,7 +1927,7 @@ export function useHarnessState(): HarnessStateHook { await api.killSession(id); } catch (err) { // Neither a rejected End nor unconfirmed cleanup dismisses the row. - if (authAtStart === authoritySequence.current && authorityAtStart === assistantAuthorityKey()) setToast( + if (authAtStart === authoritySequence.current && authorityAtStart === assistantAuthorityKey() && operation === sessionOperationGenerations.current.get(id)) setToast( createToastMessage( "Session cleanup is incomplete. The session is still available; retry End session to finish stopping it.", ), @@ -1925,7 +1937,7 @@ export function useHarnessState(): HarnessStateHook { endingSessions.current.delete(id); setEndingSessionIds(new Set(endingSessions.current)); } - if (authAtStart !== authoritySequence.current || authorityAtStart !== assistantAuthorityKey()) return; + if (authAtStart !== authoritySequence.current || authorityAtStart !== assistantAuthorityKey() || operation !== sessionOperationGenerations.current.get(id)) return; const lifecycles = assistantOrder.current().snapshot?.lifecycles ?? []; const lifecycle = lifecycles.find((row) => row.harnessSessionId === id); // A newer Resume can finish before this older End response arrives.