Skip to content

Commit 90b23ac

Browse files
CasualDeveloperCasualDeveloper
committed
fix: preserve revert consistency
Keep execution parked while a revert is staged and serialize revert mutations with prompt and synthetic admission. Commit replacement prompts and their optional context atomically with the revert, retaining queued input before its chronological boundary. Keep projected reads in SessionStore and scope plugin setup and hook admission bypasses to their active invocation. Serialize client mutations while preserving optimistic display and same-ID retries; resolve undo boundaries by transcript position rather than message ID. Closes anomalyco#37751, anomalyco#44357
1 parent 5556ad5 commit 90b23ac

56 files changed

Lines changed: 6840 additions & 491 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-message-revert.spec.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,44 @@ test("hides revert actions in a child session", async ({ page }) => {
9595
await message.hover()
9696
await expect(message.getByRole("button", { name: "Revert message" })).toHaveCount(0)
9797
})
98+
99+
test("keeps pending input when staging and replacing a revert", async ({ page }) => {
100+
const changes: { inboxID: string; action: "cancel" | "steer" }[] = []
101+
const prompts: Record<string, unknown>[] = []
102+
await mockOpenCodeServer(page, {
103+
...fixture,
104+
sessions: [session],
105+
inbox: [
106+
{
107+
id: "msg_pending_before_boundary",
108+
sessionID,
109+
time: { created: 2 },
110+
type: "user",
111+
delivery: "queue",
112+
payload: { text: "Keep this queued input" },
113+
},
114+
],
115+
onInboxChange: (input) => changes.push({ inboxID: input.inboxID, action: input.action }),
116+
onPrompt: (input) => prompts.push(input.body),
117+
})
118+
await page.goto(`/server/${base64Encode(server)}/session/${sessionID}`)
119+
await expectSessionTitle(page, "Session message revert")
120+
const pending = page.locator('[data-component="session-queue-row"]').filter({ hasText: "Keep this queued input" })
121+
await expect(pending).toBeVisible()
122+
const message = page.locator('[data-message-id="msg_second"]')
123+
await message.hover()
124+
await message.getByRole("button", { name: "Revert message" }).click()
125+
const prompt = page.getByRole("textbox", { name: "Prompt" })
126+
await expect(prompt).toHaveText("Second prompt")
127+
await prompt.fill("Replacement prompt")
128+
const admitted = page.waitForResponse(
129+
(response) =>
130+
response.request().method() === "POST" && new URL(response.url()).pathname === `/api/session/${sessionID}/prompt`,
131+
)
132+
await prompt.press("Enter")
133+
expect((await admitted).ok()).toBe(true)
134+
await expect(prompt).toHaveText("")
135+
await expect(pending).toBeVisible()
136+
expect(prompts).toMatchObject([{ text: "Replacement prompt" }])
137+
expect(changes).toEqual([])
138+
})

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ export type ComposerSession = {
8080
data: {
8181
location: { command: Pick<Data["location"]["command"], "list"> }
8282
session: {
83+
mutate: <T>(
84+
sessionID: string,
85+
operation: (mutation: {
86+
prompt: (input: Parameters<Data["session"]["prompt"]>[0]) => Promise<unknown>
87+
}) => Promise<T>,
88+
) => Promise<T>
8389
prompt: (input: Parameters<Data["session"]["prompt"]>[0]) => Promise<unknown>
8490
setStatus: Data["session"]["setStatus"]
8591
}

‎packages/app/src/composer/submit.test.ts‎

Lines changed: 129 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
1-
import { describe, expect, test } from "bun:test"
1+
import { afterEach, describe, expect, test } from "bun:test"
2+
import { createRoot } from "solid-js"
3+
import { OpenCode } from "@opencode/client/promise"
4+
import { createData } from "@opencode/client/solid"
25
import type { ModelSelection } from "@/providers/models/selection"
36
import type { SessionMessageUser } from "@opencode/client/promise"
47
import { Skill } from "@opencode/schema/skill"
58
import type { ActiveComposerAdapter, ComposerControls, ComposerSession, NewSessionComposerAdapter } from "./adapter"
69
import { createMemoryComposerState } from "./state"
710
import { createComposerSubmit } from "./submit"
811

12+
const disposers: Array<() => void> = []
13+
afterEach(() => disposers.splice(0).forEach((dispose) => dispose()))
14+
915
const selectedModel = {
1016
id: "model-1",
1117
name: "Model 1",
@@ -87,6 +93,33 @@ function session(input: {
8793
switchAgent?: ComposerSession["api"]["switchAgent"]
8894
switchModel?: ComposerSession["api"]["switchModel"]
8995
}): ComposerSession {
96+
const api = OpenCode.make({ baseUrl: "http://opencode.test" })
97+
const data = createRoot((dispose) => {
98+
disposers.push(dispose)
99+
return createData({
100+
directory: "C:/repo",
101+
event: { on: () => () => {}, listen: () => () => {} },
102+
api: () => ({
103+
...api,
104+
session: {
105+
...api.session,
106+
prompt: async (value) => {
107+
if (!value.id) throw new Error("Client admission must supply a message ID")
108+
input.calls.push("prompt")
109+
await input.prompt(value)
110+
return {
111+
id: value.id,
112+
sessionID: value.sessionID,
113+
time: { created: Date.now() },
114+
type: "user",
115+
delivery: value.delivery ?? "steer",
116+
payload: { text: value.text },
117+
}
118+
},
119+
},
120+
}),
121+
})
122+
})
90123
return {
91124
id: "session-1",
92125
directory: "C:/repo",
@@ -111,10 +144,8 @@ function session(input: {
111144
location: { command: { list: () => [] } },
112145
session: {
113146
setStatus: (_sessionID, status) => input.statuses?.push(status),
114-
prompt: async (value) => {
115-
input.calls.push("prompt")
116-
await input.prompt(value)
117-
},
147+
mutate: data.session.mutate,
148+
prompt: data.session.prompt,
118149
},
119150
},
120151
}
@@ -352,6 +383,48 @@ describe("Composer submission", () => {
352383
expect(state.current()).toEqual([{ type: "text", content: "", start: 0, end: 0 }])
353384
})
354385

386+
test.each([false, true])("reserves prompt admission before later session mutations (image: %s)", async (image) => {
387+
const state = createMemoryComposerState({ prompt: "replace history" }).capture()
388+
if (image)
389+
state.set([
390+
...state.current(),
391+
{
392+
type: "image",
393+
id: "attachment",
394+
filename: "image.png",
395+
mime: "image/png",
396+
blob: { id: "attachment", url: "data:image/png;base64,YQ==" },
397+
},
398+
])
399+
const calls: string[] = []
400+
const target = session({
401+
calls,
402+
current: () => ({ agent: "build", model: { id: "model-1", providerID: "provider-1", variant: "balanced" } }),
403+
prompt: async (value) => {
404+
expect(value.files?.length ?? 0).toBe(image ? 1 : 0)
405+
},
406+
})
407+
const adapter: ActiveComposerAdapter = {
408+
kind: "active-session",
409+
state,
410+
ready: () => true,
411+
controls,
412+
working: () => false,
413+
session: () => target,
414+
interrupt: async () => undefined,
415+
submitted() {},
416+
setEditor() {},
417+
}
418+
419+
const submitted = submitInput(adapter).submit(new Event("submit"))
420+
const redo = target.data.session.mutate(target.id, async () => {
421+
calls.push("redo")
422+
})
423+
await Promise.all([submitted, redo])
424+
425+
expect(calls).toEqual(["switch-model", "prompt", "redo"])
426+
})
427+
355428
test("starts and promotes a New Session once before admitting its first prompt", async () => {
356429
const draft = createMemoryComposerState({ prompt: "first prompt" }).capture()
357430
const promoted = createMemoryComposerState({ prompt: "restored draft" }).capture()
@@ -610,6 +683,57 @@ describe("Composer submission", () => {
610683
expect(history).toEqual([`add:${text}`, `remove:${text}`, `add:${text}`, `remove:${text}`])
611684
})
612685

686+
test("preserves browser comments through failed admission and same-ID retry", async () => {
687+
const state = createMemoryComposerState({ prompt: "Update this button" }).capture()
688+
const comment = {
689+
type: "browser" as const,
690+
tabID: "tab_00000000-0000-4000-8000-000000000000",
691+
url: "http://localhost:5173/",
692+
element: { selector: "#save", label: "button#save" },
693+
comment: "Rename this",
694+
}
695+
state.context.add({ ...comment, commentID: "browser-comment" })
696+
const requests: Parameters<ComposerSession["data"]["session"]["prompt"]>[0][] = []
697+
const failed = Promise.withResolvers<void>()
698+
const accepted = Promise.withResolvers<void>()
699+
const target = session({
700+
calls: [],
701+
prompt: async (value) => {
702+
requests.push(value)
703+
if (requests.length <= 2) throw new Error("network unavailable")
704+
accepted.resolve()
705+
},
706+
})
707+
const adapter: ActiveComposerAdapter = {
708+
kind: "active-session",
709+
state,
710+
ready: () => true,
711+
controls,
712+
working: () => false,
713+
session: () => target,
714+
interrupt: async () => undefined,
715+
submitted() {},
716+
setEditor() {},
717+
}
718+
const submission = submitInput(adapter, { missingSelection() {}, failed: () => failed.resolve() })
719+
720+
await submission.submit(new Event("submit"))
721+
await failed.promise
722+
expect(state.current()).toMatchObject([{ type: "text", content: "Update this button" }])
723+
expect(state.context.items()).toMatchObject([{ ...comment, commentID: "browser-comment" }])
724+
725+
await submission.submit(new Event("submit"))
726+
await accepted.promise
727+
expect(requests).toHaveLength(3)
728+
expect(new Set(requests.map((request) => request.id)).size).toBe(1)
729+
requests.forEach((request) => {
730+
expect(request.metadata?.comments).toEqual([comment])
731+
expect(request.text).toContain('selector "#save"')
732+
expect(request.text).toContain("Rename this")
733+
})
734+
expect(state.context.items()).toEqual([])
735+
})
736+
613737
test("forwards structured mentions to custom commands", async () => {
614738
const state = createMemoryComposerState().capture()
615739
state.set([

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

Lines changed: 49 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -88,22 +88,26 @@ export function createComposerSubmit(input: ComposerSubmitInput) {
8888
return
8989
}
9090
if (submitting.has(input.adapter.state)) return
91-
// Images restored from a draft or history carry ids only; the optimistic message shows their URLs.
92-
const value = {
93-
...read,
94-
images: await Promise.all(
95-
read.images.map(async (image) => ({
96-
...image,
97-
blob: { ...image.blob, url: (await resolveBlobUrl(image.blob)) ?? image.blob.url },
98-
})),
99-
),
100-
}
10191
submitting.add(input.adapter.state)
10292
const comments = input.comments.capture()
10393
// Capture command intent before starting a session in a worktree whose catalog has not loaded.
104-
const command = value.mode === "normal" ? findCommand(input.commands(), value.text) : undefined
94+
const command = read.mode === "normal" ? findCommand(input.commands(), read.text) : undefined
10595

10696
try {
97+
// Only new sessions need image URLs for the handoff preview. Existing sessions
98+
// load image bytes inside sendPrompt's reservation so a revert cannot overtake them.
99+
const value =
100+
input.adapter.kind === "new-session" && read.images.length
101+
? {
102+
...read,
103+
images: await Promise.all(
104+
read.images.map(async (image) => ({
105+
...image,
106+
blob: { ...image.blob, url: (await resolveBlobUrl(image.blob)) ?? image.blob.url },
107+
})),
108+
),
109+
}
110+
: read
107111
const started =
108112
input.adapter.kind === "active-session"
109113
? { session: input.adapter.session(), cleanupReady: Promise.resolve() }
@@ -120,6 +124,7 @@ export function createComposerSubmit(input: ComposerSubmitInput) {
120124
const optimisticBusy = !input.adapter.working()
121125
if (optimisticBusy && input.adapter.kind === "new-session")
122126
session.data.session.setStatus(session.id, "running")
127+
// Reserve before any await so a later revert or prompt cannot overtake this admission.
123128
const sending = sendPrompt(session, value, input.adapter.controls().model.selection.trackSessionCommit, () => {
124129
if (optimisticBusy && input.adapter.kind === "active-session")
125130
session.data.session.setStatus(session.id, "running")
@@ -401,44 +406,46 @@ async function applySelection(
401406
}
402407
}
403408

404-
async function sendPrompt(
409+
function sendPrompt(
405410
session: ComposerSession,
406411
value: ComposerSubmission,
407412
track: ModelSelection["trackSessionCommit"] | undefined,
408413
onAdmit: () => void,
409414
) {
410-
const request = await buildSubmissionRequest(session, value)
411-
// Switching agent or model reconfigures the session immediately, and with it
412-
// the remainder of a running turn. A steer targets that turn, so its
413-
// selection applies now; a queued follow-up must not reconfigure the turn it
414-
// waits behind, so it runs with the session selection at delivery time (the
415-
// intended selection stays recorded in its metadata).
416-
if (value.delivery === "steer") {
417-
await applySelection(session, value.selection, track)
418-
}
415+
return session.data.session.mutate(session.id, async (mutation) => {
416+
const request = await buildSubmissionRequest(session, value)
417+
// Switching agent or model reconfigures the session immediately, and with it
418+
// the remainder of a running turn. A steer targets that turn, so its
419+
// selection applies now; a queued follow-up must not reconfigure the turn it
420+
// waits behind, so it runs with the session selection at delivery time (the
421+
// intended selection stays recorded in its metadata).
422+
if (value.delivery === "steer") {
423+
await applySelection(session, value.selection, track)
424+
}
419425

420-
const admission = {
421-
id: value.id,
422-
sessionID: session.id,
423-
delivery: value.delivery,
424-
text: request.text,
425-
files: request.files.map((file) => ({ uri: file.uri, name: file.name, mention: file.mention })),
426-
agents: request.agents,
427-
skills: request.skills,
428-
metadata: {
429-
displayText: request.displayText,
430-
comments: request.comments,
431-
attachments: request.attachments,
432-
agent: value.selection.agent,
433-
model: {
434-
...value.selection.model,
435-
...(value.selection.variant ? { variant: value.selection.variant } : {}),
426+
const admission = {
427+
id: value.id,
428+
sessionID: session.id,
429+
delivery: value.delivery,
430+
text: request.text,
431+
files: request.files.map((file) => ({ uri: file.uri, name: file.name, mention: file.mention })),
432+
agents: request.agents,
433+
skills: request.skills,
434+
metadata: {
435+
displayText: request.displayText,
436+
comments: request.comments,
437+
attachments: request.attachments,
438+
agent: value.selection.agent,
439+
model: {
440+
...value.selection.model,
441+
...(value.selection.variant ? { variant: value.selection.variant } : {}),
442+
},
436443
},
437-
},
438-
}
439-
const sending = session.data.session.prompt(admission).catch(() => session.data.session.prompt(admission))
440-
onAdmit()
441-
await sending
444+
}
445+
const sending = mutation.prompt(admission)
446+
onAdmit()
447+
await sending
448+
})
442449
}
443450

444451
async function buildSubmissionRequest(session: ComposerSession, value: ComposerSubmission) {

‎packages/app/src/new-session/composer-adapter.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,16 @@ export function createNewSessionComposerAdapter(props: {
152152
location: data.location,
153153
session: {
154154
setStatus: data.session.setStatus,
155+
mutate: (sessionID, operation) =>
156+
data.session.mutate(sessionID, (mutation) =>
157+
operation({
158+
prompt: (input) =>
159+
mutation.prompt({
160+
...input,
161+
gate: Promise.all([input.gate, afterCreation(async () => undefined)]),
162+
}),
163+
}),
164+
),
155165
prompt: (input) =>
156166
data.session.prompt({
157167
...input,

0 commit comments

Comments
 (0)