Skip to content

Commit eb76193

Browse files
CasualDeveloperCasualDeveloper
committed
fix: preserve revert consistency
Park execution while a revert is staged and serialize revert mutations with prompt and synthetic admission. Commit replacement prompts, changed selection, and optional context atomically with the revert, retaining pending input before its chronological boundary. Apply selection intent only to winning steer admissions. Keep queued requests and same-ID retries from reapplying selection, share the short admission lock with explicit switches, and expose intent to prompt hooks without claiming it is already committed. Keep projected reads in SessionStore and scope plugin setup and hook admission bypasses to their active invocation. Serialize client mutations without delaying optimistic display or releasing same-ID retries early. Do not hold the input FIFO through shell completion or certify agent selection from reconciled admission receipts. Keep pending start-snapshot readiness interruptible when a provider fails before first assistant publication, so Stop can release execution and subsequent revert staging. Retain uninterruptible final step/tool cleanup. Bind delayed undo to its captured Session and paginated chronological history, and preserve queue rollback and full-clear redo. Regenerate the public clients and API documents from the prompt schema. Closes anomalyco#37751, anomalyco#44357
1 parent e9def88 commit eb76193

61 files changed

Lines changed: 6609 additions & 630 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎packages/app/e2e/regression/session-header.spec.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { SessionMessageInfo } from "@opencode/client/promise"
22
import { expect, test, type Page } from "@playwright/test"
3-
import { seed, sessionHref } from "../utils/app"
3+
import { pageMessagesFrom, seed, sessionHref } from "../utils/app"
44
import { fixture, mockStressTimeline } from "../utils/session-fixture"
55
import { expectSessionTitle } from "../utils/waits"
66
import { mockWorkspace, openSession } from "../utils/workspace"
@@ -327,6 +327,35 @@ test.describe("revert", () => {
327327
expect(staged).toEqual([])
328328
})
329329

330+
test("undo loads the staged boundary from an older page", async ({ page }) => {
331+
const sessionID = "ses_paginated_revert"
332+
const previous = { id: "msg_fff_previous", type: "user" as const, text: "Previous prompt", time: { created: 1 } }
333+
const history = [
334+
previous,
335+
{ id: "msg_000_boundary", type: "user" as const, text: "Boundary prompt", time: { created: 2 } },
336+
...Array.from({ length: 200 }, (_, index) => ({
337+
id: `msg_after_${index}`,
338+
type: "user" as const,
339+
text: "Reverted prompt",
340+
time: { created: index + 3 },
341+
})),
342+
]
343+
const staged: { sessionID: string; messageID: string }[] = []
344+
const { editor } = await openSession(page, {
345+
name: "PaginatedRevert",
346+
sessions: [{ id: sessionID, title: "Paginated revert", revert: { messageID: "msg_000_boundary" } }],
347+
pageMessages: pageMessagesFrom({ [sessionID]: history }),
348+
onRevertStage: (input) => staged.push(input),
349+
})
350+
await editor.pressSequentially("/undo")
351+
await expect(
352+
page.locator('[data-component="composer-suggestions"] [data-suggestion-id][data-active]'),
353+
).toContainText("/undo")
354+
await editor.press("Enter")
355+
await expect(editor).toHaveText("Previous prompt")
356+
expect(staged).toEqual([{ sessionID, messageID: previous.id }])
357+
})
358+
330359
test("hides revert actions in a child session", async ({ page }) => {
331360
await mockWorkspace(page, {
332361
...workspace,

‎packages/app/e2e/regression/session-queue.spec.ts‎

Lines changed: 138 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { PromptInput } from "@opencode/schema/prompt-input"
44
import { SessionInbox } from "@opencode/schema/session-inbox"
55
import { SessionMessage } from "@opencode/schema/session-message"
66
import { Schema } from "effect"
7-
import { holdRoute, provider } from "../utils/app"
7+
import { holdRoute, provider, sessionHref } from "../utils/app"
88
import type { MockServerConfig } from "../utils/mock-server"
99
import { openSession } from "../utils/workspace"
1010

@@ -14,6 +14,7 @@ const sessionID = "ses_session_queue_regression"
1414
const PromptBody = Schema.Struct({
1515
id: SessionMessage.ID.pipe(Schema.optional),
1616
...PromptInput.Prompt.fields,
17+
selection: PromptInput.Selection.pipe(Schema.optional),
1718
metadata: SessionInbox.UserPayload.fields.metadata,
1819
delivery: SessionInbox.Delivery.pipe(Schema.optional),
1920
resume: Schema.Boolean.pipe(Schema.optional),
@@ -95,6 +96,8 @@ function createQueueMock(seed: string[], messages: SessionMessageInfo[] = []) {
9596
const body = decodePromptBody(input.body)
9697
prompts.push(body)
9798
log.push(`prompt:${body.delivery ?? "steer"}`)
99+
// The real server is first-admission-wins for a same-Session retry with the same ID.
100+
if (rows.some((row) => row.id === body.id && row.sessionID === input.sessionID)) return
98101

99102
const row: InboxRow = {
100103
id: body.id ?? `inb_mock_${sequence}`,
@@ -161,6 +164,7 @@ async function openQueue(
161164
followUpBehavior?: "queue" | "steer",
162165
revert?: string,
163166
status: "idle" | "running" = "running",
167+
keybinds?: Record<string, string>,
164168
) {
165169
const model = { id: "queue-model", name: "Queue Model" }
166170

@@ -185,6 +189,7 @@ async function openQueue(
185189
}
186190

187191
if (followUpBehavior) options.seed = { settings: { general: { followUpBehavior } } }
192+
if (keybinds) options.seed = { settings: { ...options.seed?.settings, keybinds } }
188193
await openSession(page, options)
189194
const composer = page.locator('[data-component="composer"]')
190195

@@ -221,13 +226,15 @@ test("follow-up preference controls Enter while Mod+Enter uses the alternate del
221226
await view.input.fill("steer this correction")
222227
await view.input.press("ControlOrMeta+Enter")
223228
await expect.poll(() => mock.prompts.map((prompt) => prompt.delivery)).toEqual(["queue", "steer"])
229+
expect(mock.prompts[0].selection).toBeUndefined()
230+
expect(mock.prompts[1].selection).toEqual({ agent: "build", model: { providerID: "opencode", id: "queue-model" } })
224231
await expect(view.input).toHaveText("")
225232
})
226233

227-
test("a steer shows in the timeline as the composer clears, before its selection commits", async ({ page }) => {
234+
test("a steer shows in the timeline as the composer clears, before atomic admission commits", async ({ page }) => {
228235
const mock = createQueueMock([])
229236
const view = await openQueue(page, mock, "steer")
230-
const commit = await holdRoute(page, (url) => url.pathname === `/api/session/${sessionID}/model`, { method: "POST" })
237+
const commit = await holdRoute(page, (url) => url.pathname === `/api/session/${sessionID}/prompt`, { method: "POST" })
231238

232239
const prompt = page.locator('[data-timeline-virtual-content] [data-timeline-row="UserMessage"]', {
233240
hasText: "steer while the model commits",
@@ -279,6 +286,132 @@ test("a steer into an idle session leaves the finished turn finished until deliv
279286
await expect(finished).toHaveCount(1)
280287
})
281288

289+
for (const action of ["Undo", "Delete", "Move to queue"] as const) {
290+
test(`${action} waits for optimistic admission before changing pending input`, async ({ page }) => {
291+
const mock = createQueueMock([])
292+
const view = await openQueue(page, mock, action === "Undo" ? "queue" : "steer")
293+
const admission = await holdRoute(page, (url) => url.pathname === `/api/session/${sessionID}/prompt`, {
294+
method: "POST",
295+
})
296+
await view.input.fill("Withdraw this input")
297+
await view.input.press("Enter")
298+
const request = await admission.arrived
299+
const row =
300+
action === "Undo"
301+
? view.rows.filter({ hasText: "Withdraw this input" })
302+
: userRow(page, request.postDataJSON().id)
303+
await row.hover()
304+
const control = row.getByRole("button", { name: action })
305+
const changed = page.waitForResponse(
306+
(response) =>
307+
response.request().method() !== "OPTIONS" &&
308+
new URL(response.url()).pathname === `/api/session/${sessionID}/inbox/${request.postDataJSON().id}`,
309+
)
310+
await control.click()
311+
if (action === "Undo") await expect(control).toBeDisabled()
312+
expect(mock.changes).toEqual([])
313+
admission.release()
314+
expect((await changed).ok()).toBe(true)
315+
await expect(view.rows).toHaveCount(action === "Move to queue" ? 1 : 0)
316+
await expect(view.input).toHaveText(action === "Undo" ? "Withdraw this input" : "")
317+
const change = action === "Move to queue" ? "queue" : "cancel"
318+
expect(mock.changes).toEqual([{ inboxID: request.postDataJSON().id, action: change }])
319+
expect(mock.log).toEqual([
320+
action === "Undo" ? "prompt:queue" : "prompt:steer",
321+
`${change}:${request.postDataJSON().id}`,
322+
])
323+
})
324+
}
325+
326+
test("Undo does not withdraw a follow-up submitted after the undo gesture", async ({ page }) => {
327+
const mock = createQueueMock(
328+
[],
329+
[{ id: "msg_existing", type: "user", text: "Existing history", time: { created: 1 } }],
330+
)
331+
const view = await openQueue(page, mock, "steer", undefined, "running", { "session.undo": "ctrl+alt+shift+u" })
332+
const admission = await holdRoute(page, (url) => url.pathname === `/api/session/${sessionID}/prompt`, {
333+
method: "POST",
334+
})
335+
await view.input.fill("First input")
336+
await view.input.press("Enter")
337+
const first = await admission.arrived
338+
await expect(view.input).toHaveText("")
339+
await expect(userRow(page, first.postDataJSON().id)).toContainText("First input")
340+
await page.keyboard.press("Control+Alt+Shift+u")
341+
await view.input.fill("Later input")
342+
await view.input.press("Enter")
343+
await expect(userRow(page, first.postDataJSON().id)).toContainText("First input")
344+
admission.release()
345+
await expect.poll(() => mock.prompts.map((prompt) => prompt.text)).toEqual(["First input", "Later input"])
346+
await expect.poll(() => mock.changes).toEqual([{ inboxID: first.postDataJSON().id, action: "cancel" }])
347+
await expect(view.input).toHaveText("First input")
348+
await expect.poll(() => mock.rows.map((row) => row.payload.text)).toEqual(["Later input"])
349+
})
350+
351+
test("a revert waiting behind admission restores only its captured session after navigation", async ({ page }) => {
352+
const otherID = "ses_revert_other"
353+
const message = { id: "msg_revert_owned", type: "user" as const, text: "Restore A", time: { created: 1 } }
354+
const mock = createQueueMock([], [message])
355+
const workspace = await openSession(page, {
356+
name: "SessionQueueRegression",
357+
sessions: [
358+
{ id: sessionID, title: "Session A" },
359+
{ id: otherID, title: "Session B" },
360+
],
361+
pageMessages: (id) => ({ items: id === sessionID ? mock.messages : [] }),
362+
inbox: () => mock.rows,
363+
sessionStatus: () => ({ [sessionID]: { type: "running" } }),
364+
onPrompt: mock.onPrompt,
365+
onInboxChange: mock.onInboxChange,
366+
events: mock.events,
367+
seed: { settings: { general: { followUpBehavior: "queue" } } },
368+
})
369+
const admission = await holdRoute(page, (url) => url.pathname === `/api/session/${sessionID}/prompt`, {
370+
method: "POST",
371+
})
372+
await workspace.editor.fill("Admit before revert")
373+
await workspace.editor.press("Enter")
374+
await admission.arrived
375+
await expect(workspace.editor).toHaveText("")
376+
const delivered = userRow(page, message.id)
377+
await delivered.hover()
378+
await delivered.getByRole("button", { name: "Revert message" }).click()
379+
await page.locator(`a[data-titlebar-tab-link][href="${sessionHref(otherID)}"]`).click()
380+
await expect(workspace.editor).toBeEditable()
381+
await workspace.editor.fill("Keep B's draft")
382+
const staged = page.waitForResponse(
383+
(response) => new URL(response.url()).pathname === `/api/session/${sessionID}/revert/stage`,
384+
)
385+
admission.release()
386+
expect((await staged).ok()).toBe(true)
387+
await expect(workspace.editor).toHaveText("Keep B's draft")
388+
await page.locator(`a[data-titlebar-tab-link][href="${sessionHref(sessionID)}"]`).click()
389+
await expect(workspace.editor).toHaveText("Restore A")
390+
})
391+
392+
test("an edit refuses a revert the server has staged before its event reaches the client", async ({ page }) => {
393+
const mock = createQueueMock(["Edit this input"])
394+
const view = await openQueue(page, mock, "queue")
395+
await view.rows.getByText("Edit this input", { exact: true }).click()
396+
await expect(view.input).toHaveText("Edit this input")
397+
await expect(view.input).toBeFocused()
398+
await view.input.press("End")
399+
await view.input.pressSequentially(", changed")
400+
await expect(view.input).toHaveText("Edit this input, changed")
401+
await page.route(
402+
(url) => url.pathname === `/api/session/${sessionID}`,
403+
(route) => {
404+
if (route.request().method() !== "GET") return route.fallback()
405+
return route.fulfill({ json: { data: { revert: { messageID: "msg_boundary" } } } })
406+
},
407+
)
408+
await view.input.press("Enter")
409+
await expect(page.getByText("Redo the revert before you reorder or edit queued prompts")).toBeVisible()
410+
await expect(view.input).toHaveText("Edit this input, changed")
411+
expect(mock.prompts).toEqual([])
412+
expect(mock.changes).toEqual([])
413+
})
414+
282415
test("dragging reorders queued prompts", async ({ page }) => {
283416
const mock = createQueueMock(["first queued prompt", "second queued prompt", "third queued prompt"])
284417
mock.rows[0].payload.files = [
@@ -354,7 +487,7 @@ for (const change of ["reorder", "edit"] as const) {
354487

355488
// The replacements admitted before the failure are withdrawn; the originals stay in place.
356489
await expect.poll(() => mock.log.includes(`cancel:${mock.prompts.at(-1)?.id}`)).toBe(true)
357-
await expect.poll(() => mock.changes.length).toBe(mock.prompts.length)
490+
await expect.poll(() => mock.changes.length).toBe(new Set(mock.prompts.map((prompt) => prompt.id)).size)
358491
expect(mock.changes.every((entry) => entry.action === "cancel" && !entry.inboxID.startsWith("inb_seed_"))).toBe(
359492
true,
360493
)
@@ -501,6 +634,7 @@ test("editing a comment-only prompt keeps its notes once", async ({ page }) => {
501634
// With no display text, the editor shows the note itself, so the edit owns it as text.
502635
await view.rows.getByText(note, { exact: true }).click()
503636
await expect(view.input).toHaveText(note)
637+
await expect(view.input).toBeFocused()
504638
await view.input.press("End")
505639
await view.input.pressSequentially(", please")
506640
await expect(view.input).toHaveText(`${note}, please`)

‎packages/app/src/composer/adapter.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,11 @@ export type ComposerSession = {
6969
switchModel: (
7070
input: Parameters<ServerSDK["api"]["session"]["switchModel"]>[0],
7171
) => ReturnType<ServerSDK["api"]["session"]["switchModel"]>
72-
revert: {
73-
commit: (
74-
input: Parameters<ServerSDK["api"]["session"]["revert"]["commit"]>[0],
75-
) => ReturnType<ServerSDK["api"]["session"]["revert"]["commit"]>
76-
}
7772
}
7873
data: {
7974
location: { command: Pick<Data["location"]["command"], "list"> }
8075
session: {
76+
mutate: Data["session"]["mutate"]
8177
prompt: (input: Parameters<Data["session"]["prompt"]>[0]) => ReturnType<Data["session"]["prompt"]>
8278
setStatus: Data["session"]["setStatus"]
8379
}

0 commit comments

Comments
 (0)