diff --git a/.changeset/assistant-workspace-action-preflight.md b/.changeset/assistant-workspace-action-preflight.md new file mode 100644 index 000000000..b5080cbd4 --- /dev/null +++ b/.changeset/assistant-workspace-action-preflight.md @@ -0,0 +1,5 @@ +--- +"@sapiom/harness": patch +--- + +Check the selected workspace before Assistant history actions start, and preserve saved operation retries when a workspace selection changes. diff --git a/packages/harness/docs/assistant-history.md b/packages/harness/docs/assistant-history.md index bae2910d9..e43268ffb 100644 --- a/packages/harness/docs/assistant-history.md +++ b/packages/harness/docs/assistant-history.md @@ -224,3 +224,11 @@ canonical record path. History, inspection, Resume and mixed Terminal records keep both identities, and reject a changed or retargeted path. Refreshes through aliases share canonical publication ordering, so an empty response clears stale rows without hiding failures in a different workspace. + +Inspection, Resume and Continue send the selected workspace pair as an optional +transport precondition, checked before native work. A `workspace_changed` 409 +requires closing the record and selecting it again after Past sessions refreshes; +it does not prove that an earlier unacknowledged operation was never prepared. +Legacy requests remain accepted. The original saved operation UUID/revisions and +receipt fingerprints are unchanged. Explicit re-selection can refresh Resume's +launch spelling only with the same verified canonical binding and retry scope. diff --git a/packages/harness/src/server/assistant-lifecycle.test.ts b/packages/harness/src/server/assistant-lifecycle.test.ts index 942715b9d..03b5b0c3d 100644 --- a/packages/harness/src/server/assistant-lifecycle.test.ts +++ b/packages/harness/src/server/assistant-lifecycle.test.ts @@ -259,6 +259,97 @@ it("returns the current raw/canonical proof on same-ID alias Resume", async () = }); }); +it.each(["inspect", "resume", "continue"])( + "rejects a stale selected workspace before %s can start", + async (action) => { + const current = { cwd: "/alias-b", canonicalCwd: savedEntry.cwd }; + session.cwd = current.cwd; + entry.mockResolvedValue({ ...savedEntry, workspace: current }); + for (const expectedWorkspace of [ + { ...current, cwd: "/alias-a" }, + { ...current, canonicalCwd: "/previous-target" }, + ]) { + const response = await post(action, { + expectedRevision: 4, + expectedWorkspace, + ...(action !== "inspect" ? { operationId: op } : {}), + ...(action === "continue" ? { expectedRecordRevision: 7 } : {}), + }); + expect(response.status).toBe(409); + expect((await response.json()).error.code).toBe("workspace_changed"); + } + expect(inspect).not.toHaveBeenCalled(); + expect(resume).not.toHaveBeenCalled(); + expect(continuation).not.toHaveBeenCalled(); + }, +); + +it("rejects a raw session rebind between history verification and action admission", async () => { + const expectedWorkspace = { cwd: session.cwd, canonicalCwd: savedEntry.cwd }; + entry.mockImplementationOnce(async () => { + session.cwd = "/rebound"; + return { ...savedEntry, workspace: expectedWorkspace }; + }); + expect( + (await post("inspect", { expectedRevision: 4, expectedWorkspace })).status, + ).toBe(409); + expect(inspect).not.toHaveBeenCalled(); +}); + +it.each([ + null, + {}, + { cwd: 1, canonicalCwd: "/workspace" }, + { cwd: "/workspace", canonicalCwd: "relative" }, + { cwd: "/workspace", canonicalCwd: "/workspace", extra: true }, +])( + "rejects malformed selected workspace %j before action admission", + async (expectedWorkspace) => { + expect( + (await post("inspect", { expectedRevision: 4, expectedWorkspace })) + .status, + ).toBe(400); + expect(entry).not.toHaveBeenCalled(); + expect(inspect).not.toHaveBeenCalled(); + }, +); + +it("keeps prepared Continue retries on the original durable tuple, with or without a workspace precondition", async () => { + const request = { + expectedRevision: 4, + operationId: op, + expectedRecordRevision: 7, + }; + const expectedWorkspace = { cwd: "/alias-a", canonicalCwd: savedEntry.cwd }; + session.cwd = expectedWorkspace.cwd; + const current = { + ...savedEntry, + workspace: expectedWorkspace, + lifecycle: { ...lifecycle, revision: 9 }, + }; + entry.mockResolvedValue(current); + const first = await post("continue", { ...request, expectedWorkspace }); + expect(first.status).toBe(200); + // The first ACK may have been lost: stale intent blocks only this retry, + // never claims the durable prepared child does not exist or erases its tuple. + entry.mockResolvedValue({ + ...current, + workspace: { ...expectedWorkspace, cwd: "/alias-b" }, + }); + session.cwd = "/alias-b"; + expect( + (await post("continue", { ...request, expectedWorkspace })).status, + ).toBe(409); + expect(continuation).toHaveBeenCalledTimes(1); + entry.mockResolvedValue(current); + session.cwd = expectedWorkspace.cwd; + for (const body of [request, { ...request, expectedWorkspace }]) + expect((await post("continue", body)).status).toBe(200); + expect(continuation.mock.calls).toEqual( + Array.from({ length: 3 }, () => ["studio-a", request]), + ); +}); + it.each(["inspect", "resume"])( "rejects %s when a raw alias rebind leaves the canonical binding unchanged", async (action) => { diff --git a/packages/harness/src/server/assistant-lifecycle.ts b/packages/harness/src/server/assistant-lifecycle.ts index cc976f4a0..de16276be 100644 --- a/packages/harness/src/server/assistant-lifecycle.ts +++ b/packages/harness/src/server/assistant-lifecycle.ts @@ -19,11 +19,16 @@ import { openCodeTransportFailure } from "../shared/opencode-errors.js"; import { createBootTokenMiddleware } from "./auth.js"; import { assistantHistoryMatches, + assistantHistoryWorkspaceSchema, + assistantWorkspace, sameAssistantWorkspace, } from "../shared/assistant-history.js"; const inspectionRequest = z - .object({ expectedRevision: z.number().int().nonnegative().safe() }) + .object({ + expectedRevision: z.number().int().nonnegative().safe(), + expectedWorkspace: assistantHistoryWorkspaceSchema.optional(), + }) .strict(); const resumeRequest = inspectionRequest .extend({ operationId: z.string().uuid() }) @@ -79,6 +84,7 @@ function failure(res: Response, error: unknown) { ? 403 : [ "lifecycle_changed", + "workspace_changed", "session_ended", "cleanup_unconfirmed", "continuation_unconfirmed", @@ -139,6 +145,23 @@ export function createAssistantLifecycleRouter(options: { res.sendStatus(404); return; } + // Compare selected intent before any native work. This transport + // precondition is deliberately not part of a durable operation tuple. + const expectedWorkspace = parsed.data.expectedWorkspace; + if (expectedWorkspace) { + const current = assistantWorkspace(entry); + const session = options.getSession(id); + if ( + expectedWorkspace.cwd !== current.cwd || + expectedWorkspace.canonicalCwd !== entry.cwd || + !session || + session.id !== id || + session.cwd !== expectedWorkspace.cwd + ) + throw new OpenCodeTransportError( + openCodeTransportFailure("workspace_changed"), + ); + } if (action === "inspect") { if (entry.lifecycle.revision !== parsed.data.expectedRevision) throw changed(); @@ -192,10 +215,13 @@ export function createAssistantLifecycleRouter(options: { throw new OpenCodeTransportError( openCodeTransportFailure("context_unavailable"), ); - const prepared = await options.continue( - id, - continueRequest.parse(parsed.data), - ); + const { expectedRevision, operationId, expectedRecordRevision } = + continueRequest.parse(parsed.data); + const prepared = await options.continue(id, { + expectedRevision, + operationId, + expectedRecordRevision, + }); res.json({ session: sessionView(prepared.session), attachment: prepared.attachment, diff --git a/packages/harness/src/shared/opencode-errors.ts b/packages/harness/src/shared/opencode-errors.ts index ded445453..880c4edbc 100644 --- a/packages/harness/src/shared/opencode-errors.ts +++ b/packages/harness/src/shared/opencode-errors.ts @@ -8,6 +8,7 @@ export const openCodeTransportErrorCodes = [ "context_unavailable", "transport_unavailable", "lifecycle_changed", + "workspace_changed", "session_ended", "execution_paused", "cleanup_unconfirmed", @@ -50,6 +51,13 @@ export interface OpenCodeStudioErrorEvent { } const fixedFailures = { + workspace_changed: { + code: "workspace_changed", + message: + "This session's workspace changed. Close this record, reopen Past sessions, and select it again after the list refreshes.", + retryable: true, + action: "reconnect", + }, continuation_unconfirmed: { code: "continuation_unconfirmed", message: diff --git a/packages/harness/web/e2e/assistant-continuation.spec.ts b/packages/harness/web/e2e/assistant-continuation.spec.ts index a66aa853c..ec16d54fd 100644 --- a/packages/harness/web/e2e/assistant-continuation.spec.ts +++ b/packages/harness/web/e2e/assistant-continuation.spec.ts @@ -383,6 +383,7 @@ test("Continue opens a distinct paused child from the displayed record and prese expect(body).toEqual({ expectedRevision: 2, expectedRecordRevision: 1, + expectedWorkspace: { cwd, canonicalCwd: cwd }, operationId: expect.stringMatching(/^[a-f0-9-]{36}$/), }); await project(page, 2, "account-a", [lifecycle(childId)]); @@ -469,7 +470,51 @@ test("lost Continue response survives refresh, host restart and a newer record w ), ); expect(persisted).toHaveLength(1); - expect(JSON.parse(persisted[0]![1])).toEqual(request); + const { expectedWorkspace, ...tuple } = request; + expect(expectedWorkspace).toEqual({ cwd, canonicalCwd: cwd }); + expect(JSON.parse(persisted[0]![1])).toEqual(tuple); +}); + +test("workspace conflict preserves uncertain Continue without offering a new operation", async ({ + page, +}, info) => { + const probe = await setup(page); + probe.fail = true; + await review(page); + await button(page).click(); + await expect(page.getByRole("alert")).toContainText("Retry this operation"); + const original = probe.requests[0]!.request().postDataJSON(); + probe.fail = false; + probe.hold = true; + await button(page).click(); + await expect.poll(() => probe.requests.length).toBe(2); + await probe.requests[1]!.fulfill({ + status: 409, + json: { error: openCodeTransportFailure("workspace_changed") }, + }); + await expect(page.getByRole("alert")).toContainText( + "select it again after the list refreshes", + ); + await expect(button(page)).toHaveText("Retry Continue"); + await expect( + page.getByRole("button", { name: "Review latest record", exact: true }), + ).toHaveCount(0); + await expect( + page.getByRole("button", { name: "Start a new continuation", exact: true }), + ).toHaveCount(0); + expect(probe.requests[1]!.request().postDataJSON()).toEqual(original); + const persisted = await page.evaluate(() => + Object.entries(localStorage).filter(([key]) => + key.startsWith("studio.assistant-continue."), + ), + ); + const { expectedWorkspace, ...tuple } = original; + expect(expectedWorkspace).toEqual({ cwd, canonicalCwd: cwd }); + expect(persisted).toHaveLength(1); + expect(JSON.parse(persisted[0]![1])).toEqual(tuple); + await page.screenshot({ + path: info.outputPath("continue-workspace-conflict.png"), + }); }); for (const invalid of ["source", "record", "hash", "child", "lease"]) { diff --git a/packages/harness/web/e2e/assistant-history.spec.ts b/packages/harness/web/e2e/assistant-history.spec.ts index 7f31d4409..2f05a9ee4 100644 --- a/packages/harness/web/e2e/assistant-history.spec.ts +++ b/packages/harness/web/e2e/assistant-history.spec.ts @@ -121,7 +121,12 @@ async function setup(page: Page, mode = "valid") { mode === "alias" ? { cwd, canonicalCwd: `/private${cwd}` } : undefined; const savedEntry = (id: string) => workspace - ? { ...entry(id), cwd: workspace.canonicalCwd, workspace } + ? { + ...entry(id), + cwd: workspace.canonicalCwd, + workspace, + continuationScope: "a".repeat(64), + } : entry(id); const streams = new Set(); const events = createServer((req, res) => { @@ -1174,3 +1179,106 @@ test("a slow sibling alias cannot replace a newer canonical history bucket", asy page.getByTestId("assistant-history-ordered-alias"), ).toContainText("Fresh alias history"); }); + +test("workspace conflict retains lost-ACK Resume and refreshes only its proof after explicit re-selection", async ({ + page, +}, info) => { + const probe = await setup(page, "alias"); + 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", + ); + const workspace = { + cwd: "/Users/demo/refreshed-alias", + canonicalCwd: `/private${cwd}`, + }; + const retries: Route[] = []; + probe.resumedRevision = 4; + await page.route("**/api/sessions/*/assistant/resume", (route) => { + retries.push(route); + if (route.request().postDataJSON().expectedWorkspace.cwd !== workspace.cwd) + return route.fulfill({ + status: 409, + json: { error: openCodeTransportFailure("workspace_changed") }, + }); + const result = probe.resumed("assistant-only"); + return route.fulfill({ + json: { + ...result, + workspace, + session: { ...result.session, cwd: workspace.cwd }, + }, + }); + }); + await resumeButton(page).click(); + await expect(page.getByRole("alert")).toContainText( + "select it again after the list refreshes", + ); + await expect( + page.getByRole("button", { name: "Retry Resume", exact: true }), + ).toBeVisible(); + expect(retries[0]!.request().postDataJSON()).toEqual(original); + await page.screenshot({ + path: info.outputPath("workspace-selection-conflict.png"), + }); + await page + .getByTestId("assistant-history-pane") + .getByRole("button", { name: "Close", exact: true }) + .click(); + await page.route("**/api/sessions/assistant-history?**", (route) => { + const query = new URL(route.request().url()).searchParams.get("cwd")!; + return route.fulfill({ + json: + query === cwd + ? { + workspace: { cwd, canonicalCwd: workspace.canonicalCwd }, + entries: [ + { + ...entry("assistant-only"), + cwd: workspace.canonicalCwd, + workspace, + continuationScope: "a".repeat(64), + lifecycle: { + ...entry("assistant-only").lifecycle, + lifecycle: "open", + revision: 3, + }, + }, + ], + } + : { entries: [] }, + }); + }); + await rows(page); + await expect(page.getByText("Loading…", { exact: true })).toHaveCount(0); + await page.getByTestId("assistant-history-assistant-only").click(); + await resumeButton(page).click(); + await expect(page.getByText("Paused", { exact: true })).toBeVisible(); + expect(retries[1]!.request().postDataJSON()).toEqual({ + ...original, + expectedWorkspace: workspace, + }); + await page.screenshot({ + path: info.outputPath("workspace-selection-retry.png"), + }); + expect( + probe.calls.filter((url) => /prompt_async|final-response/.test(url)), + ).toEqual([]); +}); diff --git a/packages/harness/web/src/components/AssistantHistoryActions.tsx b/packages/harness/web/src/components/AssistantHistoryActions.tsx index bdf4b743e..de1fa43a8 100644 --- a/packages/harness/web/src/components/AssistantHistoryActions.tsx +++ b/packages/harness/web/src/components/AssistantHistoryActions.tsx @@ -4,6 +4,7 @@ import type { AssistantLifecycle } from "../../../src/shared/assistant-session"; import { inspectAssistant, isAssistantLifecycleConflict, + resumeEntryForSelection, } from "../lib/assistant-resume-client"; export interface AssistantResumeOperation { @@ -84,8 +85,11 @@ export function AssistantHistoryActions({ // 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); + // Explicit re-selection may refresh only the launch spelling. The + // original operation, revision and canonical binding remain fixed. + const selected = resumeEntryForSelection(operation.entry, entry); const opened = await onResume( - operation.entry, + selected, operation.operationId, abort.signal, ); diff --git a/packages/harness/web/src/lib/assistant-history-workspace.test.ts b/packages/harness/web/src/lib/assistant-history-workspace.test.ts index 1102dfc64..0510fe700 100644 --- a/packages/harness/web/src/lib/assistant-history-workspace.test.ts +++ b/packages/harness/web/src/lib/assistant-history-workspace.test.ts @@ -1,5 +1,6 @@ import { afterEach, expect, it, vi } from "vitest"; import { createHash } from "node:crypto"; +import { openCodeTransportFailure } from "../../../src/shared/opencode-errors"; import { assistantHistoryMatches, type AssistantHistoryEntry, @@ -10,9 +11,15 @@ import { } from "./assistant-history-client"; import { inspectAssistant, + isAssistantLifecycleConflict, + resumeEntryForSelection, resumeAssistantRequest, } from "./assistant-resume-client"; -import { continueAssistantRequest } from "./assistant-continuation-client"; +import { + continueAssistantRequest, + prepareContinueRequest, + savedContinueRequest, +} from "./assistant-continuation-client"; const operationId = "11111111-1111-4111-8111-111111111111"; const workspace = { cwd: "/launch-alias", canonicalCwd: "/canonical" }; @@ -142,9 +149,49 @@ it("resumes through the exact verified pair without changing the requested opera expect(JSON.parse(request.body as string)).toEqual({ operationId, expectedRevision: 2, + expectedWorkspace: workspace, + }); +}); + +it("sends inspected intent even for an older entry with only exact cwd", async () => { + const legacy = { ...entry, workspace: undefined }; + respond({ entry: legacy }); + await inspectAssistant(legacy, "boot", signal()); + expect( + JSON.parse(vi.mocked(fetch).mock.calls[0]![1]!.body as string), + ).toEqual({ + expectedRevision: 2, + expectedWorkspace: { cwd: entry.cwd, canonicalCwd: entry.cwd }, }); }); +it("refreshes only Resume transport spelling after re-selection of the same proven binding", () => { + const selected = { + ...entry, + workspace: { ...workspace, cwd: "/alias-b" }, + lifecycle: { ...entry.lifecycle, revision: 8 }, + }; + const retried = resumeEntryForSelection(entry, selected); + expect(retried).toEqual({ ...entry, workspace: selected.workspace }); + expect(entry.workspace).toEqual(workspace); + expect(retried.lifecycle).toBe(entry.lifecycle); + for (const unsafe of [ + { ...selected, continuationScope: undefined }, + { ...selected, continuationScope: "b".repeat(64) }, + { ...selected, cwd: "/different-project" }, + { ...selected, harnessSessionId: "different-studio" }, + { ...selected, workspace: undefined }, + ]) + expect(resumeEntryForSelection(entry, unsafe)).toBe(entry); + const legacy = { ...entry, continuationScope: undefined }; + expect( + resumeEntryForSelection(legacy, { + ...selected, + continuationScope: undefined, + }), + ).toBe(legacy); +}); + it.each(["missing", "malformed", "canonical", "raw", "session"])( "rejects a %s Resume workspace proof", async (change) => { @@ -208,9 +255,68 @@ it("verifies a Continue child against the source launch spelling without substit (await continueAssistantRequest(entry, request, "boot", signal())).session .cwd, ).toBe(workspace.cwd); + expect( + JSON.parse(vi.mocked(fetch).mock.calls[0]![1]!.body as string), + ).toEqual({ ...request, expectedWorkspace: workspace }); + expect(request).toEqual({ + operationId, + expectedRevision: 2, + expectedRecordRevision: 1, + }); value.session.cwd = entry.cwd; respond(value); await expect( continueAssistantRequest(entry, request, "boot", signal()), ).rejects.toThrow("could not be verified"); }); + +it("retains a persisted uncertain Continue tuple after a workspace preflight conflict", async () => { + const key = `studio.assistant-continue.v1:${entry.continuationScope}:${entry.harnessSessionId}`; + const request = { + operationId, + expectedRevision: 2, + expectedRecordRevision: 1, + }; + const storage = new Map([[key, JSON.stringify(request)]]); + vi.stubGlobal("localStorage", { + getItem: (key: string) => storage.get(key) ?? null, + setItem: (key: string, value: string) => storage.set(key, value), + }); + vi.stubGlobal("navigator", { + locks: { + request: async (_name: string, callback: () => unknown) => callback(), + }, + }); + vi.stubGlobal( + "fetch", + vi.fn( + async () => + new Response( + JSON.stringify({ + error: openCodeTransportFailure("workspace_changed"), + }), + { status: 409 }, + ), + ), + ); + const pending = await prepareContinueRequest(entry, 99); + const error = await continueAssistantRequest( + entry, + pending, + "boot", + signal(), + ).catch((error: unknown) => error); + expect(error).toMatchObject({ + status: 409, + failure: { code: "workspace_changed" }, + }); + expect(isAssistantLifecycleConflict(error)).toBe(false); + expect(savedContinueRequest(entry)).toEqual(request); + expect(storage.get(key)).toBe(JSON.stringify(request)); + expect( + await prepareContinueRequest( + { ...entry, lifecycle: { ...entry.lifecycle, revision: 10 } }, + 99, + ), + ).toEqual(request); +}); diff --git a/packages/harness/web/src/lib/assistant-resume-client.ts b/packages/harness/web/src/lib/assistant-resume-client.ts index 66077d396..7461c357f 100644 --- a/packages/harness/web/src/lib/assistant-resume-client.ts +++ b/packages/harness/web/src/lib/assistant-resume-client.ts @@ -59,6 +59,21 @@ export const isAssistantLifecycleConflict = (error: unknown): boolean => error.status === 409 && error.failure?.code === "lifecycle_changed"; +/** A refreshed selection may change only transport spelling for the same proven binding. */ +export function resumeEntryForSelection( + original: AssistantHistoryEntry, + selected: AssistantHistoryEntry, +): AssistantHistoryEntry { + return selected.harnessSessionId === original.harnessSessionId && + selected.cwd === original.cwd && + selected.workspace?.canonicalCwd === original.cwd && + !!selected.continuationScope && + /^[a-f\d]{64}$/.test(selected.continuationScope) && + selected.continuationScope === original.continuationScope + ? { ...original, workspace: selected.workspace } + : original; +} + export async function assistantActionRequest( entry: AssistantHistoryEntry, action: "inspect" | "resume" | "continue", @@ -77,7 +92,12 @@ export async function assistantActionRequest( credentials: "omit", cache: "no-store", signal, - body: JSON.stringify(body), + body: JSON.stringify({ + ...body, + expectedWorkspace: assistantHistoryWorkspaceSchema.parse( + assistantWorkspace(entry), + ), + }), }, ); const value = object(await response.json().catch(() => null));