Skip to content

Commit 6155122

Browse files
nexxelnCasualDeveloper
authored andcommitted
fix(tui): undo prompts the server already accepted
/undo restored a prompt whenever session.inbox.cancel returned 204. That cancel is a no-op when the prompt was already delivered, and it could reach the server before the prompt's own POST. Undo now runs in the per-session send chain, cancels, and reads the message back. When the message exists, it interrupts, waits, and stages the revert. Fixes anomalyco#54150
1 parent 3bc37de commit 6155122

2 files changed

Lines changed: 97 additions & 66 deletions

File tree

‎packages/client/src/solid/data.ts‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import { SessionMessage } from "@opencode/schema/session-message"
4545
import {
4646
isFormAlreadySettledError,
4747
isFormNotFoundError,
48+
isMessageNotFoundError,
4849
isPermissionNotFoundError,
4950
type SessionPromptInput,
5051
} from "../promise"
@@ -364,9 +365,10 @@ export function createData(config: CreateDataInput) {
364365
// to exist server-side instead of failing with "not found".
365366
const creating = new Map<string, Promise<unknown>>()
366367

367-
// Per-session send chain: prompts and compactions must be admitted in
368-
// submission order. Each waits for the previous POST to settle, so one
369-
// failure does not block the next.
368+
// Per-session send chain. Admissions, withdrawals, and undos run in
369+
// submission order, so a cancel cannot overtake the prompt it targets. Each
370+
// waits for the previous request to settle, so one failure does not block
371+
// the next.
370372
const sending = new Map<string, Promise<unknown>>()
371373
const messageLoads = new Map<string, Promise<unknown>>()
372374
const compacting = new Map<string, { id: string; observed: Set<string>; request: Promise<SessionInboxCompaction> }>()
@@ -383,7 +385,7 @@ export function createData(config: CreateDataInput) {
383385
}
384386

385387
// Capture creation before settlement clears its entry, so dependent RPCs still see a failed create.
386-
function sendAdmission<Value>(sessionID: string, send: () => Promise<Value>, gate?: Promise<unknown>) {
388+
function sendInOrder<Value>(sessionID: string, send: () => Promise<Value>, gate?: Promise<unknown>) {
387389
const created = creating.get(sessionID)
388390
const previous = sending.get(sessionID)
389391
const request = Promise.resolve()
@@ -397,6 +399,20 @@ export function createData(config: CreateDataInput) {
397399
return request
398400
}
399401

402+
// Cancellation is idempotent, so only the projected message shows whether delivery won.
403+
async function withdrawInput(sessionID: string, inboxID: string) {
404+
await api().session.inbox.cancel({ sessionID, inboxID })
405+
return api()
406+
.session.message.get({ sessionID, messageID: inboxID })
407+
.then(
408+
() => "delivered" as const,
409+
(error: unknown) => {
410+
if (isMessageNotFoundError(error)) return "withdrawn" as const
411+
throw error
412+
},
413+
)
414+
}
415+
400416
// Upsert an admitted inbox item into pending and (for user and synthetic
401417
// items) the visible transcript. Used by the inbox.enqueued
402418
// handler and by optimistic admission; the upsert is what reconciles
@@ -1477,6 +1493,9 @@ export function createData(config: CreateDataInput) {
14771493
store.session.pending[sessionID]?.some((item) => item.id === inboxID && item.type !== "compaction") ?? false
14781494
)
14791495
},
1496+
withdraw(input: { sessionID: string; inboxID: string }) {
1497+
return sendInOrder(input.sessionID, () => withdrawInput(input.sessionID, input.inboxID))
1498+
},
14801499
},
14811500
pending: {
14821501
list(sessionID: string) {
@@ -1619,7 +1638,7 @@ export function createData(config: CreateDataInput) {
16191638
// speculative row on an echo, and remember consumed IDs until the POST
16201639
// settles so its older response cannot resurrect a queued row.
16211640
const observed = new Set<string>()
1622-
const request = sendAdmission(input.sessionID, async () => {
1641+
const request = sendInOrder(input.sessionID, async () => {
16231642
if (input.model) await api().session.switchModel({ sessionID: input.sessionID, model: input.model })
16241643
return api().session.compact({ sessionID: input.sessionID, id })
16251644
})
@@ -1682,7 +1701,7 @@ export function createData(config: CreateDataInput) {
16821701
payload,
16831702
})
16841703
}
1685-
return sendAdmission(
1704+
return sendInOrder(
16861705
request.sessionID,
16871706
async () => {
16881707
const prepared = await prepare?.()
@@ -1703,6 +1722,19 @@ export function createData(config: CreateDataInput) {
17031722
throw error
17041723
})
17051724
},
1725+
// A loaded message that is no longer pending has left the inbox, so only a pending or unloaded one
1726+
// needs the server to decide.
1727+
undo(input: { sessionID: string; messageID: string }) {
1728+
return sendInOrder(input.sessionID, async () => {
1729+
const delivered =
1730+
!result.session.input.has(input.sessionID, input.messageID) &&
1731+
result.session.message.get(input.sessionID, input.messageID) !== undefined
1732+
if (!delivered && (await withdrawInput(input.sessionID, input.messageID)) === "withdrawn") return
1733+
await api().session.interrupt({ sessionID: input.sessionID })
1734+
await api().session.wait({ sessionID: input.sessionID })
1735+
await api().session.revert.stage({ sessionID: input.sessionID, messageID: input.messageID })
1736+
})
1737+
},
17061738
sync(sessionID: string, options?: { children?: boolean }) {
17071739
return sync.run(options?.children ? `session.family:${sessionID}` : `session:${sessionID}`, async () => {
17081740
const [info, children] = await Promise.all([

‎packages/tui/src/routes/session/index.tsx‎

Lines changed: 59 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -649,11 +649,11 @@ export function Session(props: {
649649
const current = prompt()
650650
const wasEmpty = Boolean(
651651
current &&
652-
!current.current.text.trim() &&
653-
!current.current.files?.length &&
654-
!current.current.agents?.length &&
655-
!current.current.skills?.length &&
656-
!current.current.pasted.length,
652+
!current.current.text.trim() &&
653+
!current.current.files?.length &&
654+
!current.current.agents?.length &&
655+
!current.current.skills?.length &&
656+
!current.current.pasted.length,
657657
)
658658
if (!wasEmpty || !current) return
659659
const list = messages()
@@ -680,13 +680,13 @@ export function Session(props: {
680680
const active = prompt()
681681
return Boolean(
682682
!sessionDisposed &&
683-
route.sessionID === event.data.sessionID &&
684-
active &&
685-
active.current.text === expectedText &&
686-
(active.current.files?.length ?? 0) === expectedFiles &&
687-
(active.current.agents?.length ?? 0) === expectedAgents &&
688-
(active.current.skills?.length ?? 0) === expectedSkills &&
689-
active.current.pasted.length === expectedPasted,
683+
route.sessionID === event.data.sessionID &&
684+
active &&
685+
active.current.text === expectedText &&
686+
(active.current.files?.length ?? 0) === expectedFiles &&
687+
(active.current.agents?.length ?? 0) === expectedAgents &&
688+
(active.current.skills?.length ?? 0) === expectedSkills &&
689+
active.current.pasted.length === expectedPasted,
690690
)
691691
}
692692
const canOpen = () =>
@@ -745,7 +745,7 @@ export function Session(props: {
745745
}),
746746
)
747747
const runPendingAction = createSingleFlight<string>()
748-
const mutatePending = async (action: PendingAction, inboxID: string, failureLabel?: string) => {
748+
const mutatePending = async (action: PendingAction, inboxID: string) => {
749749
const result = await runPendingAction(inboxID, async () => {
750750
const request =
751751
action === "steer"
@@ -758,7 +758,7 @@ export function Session(props: {
758758
(error) => error,
759759
)
760760
if (!error) return true
761-
const label = failureLabel ?? (action === "cancel" ? "delete" : action)
761+
const label = action === "cancel" ? "delete" : action
762762
toast.show({ title: `Failed to ${label} pending prompt`, message: errorMessage(error), variant: "error" })
763763
return false
764764
})
@@ -802,19 +802,37 @@ export function Session(props: {
802802
toast.show({ message: "Leave shell mode before undoing a queued prompt", variant: "error" })
803803
return
804804
}
805-
void mutatePending("cancel", queued.id, "undo").then((undone) => {
806-
if (!undone) return
807-
target.setMode("normal")
808-
target.set(
809-
appendPrompt(
810-
target.current,
811-
{ ...projectedPromptInput(queued.payload), pasted: [] },
812-
renderer.widthMethod,
813-
),
814-
)
815-
dialog.clear()
816-
target.focus()
817-
})
805+
void runPendingAction(queued.id, () =>
806+
data.session.input.withdraw({ sessionID: route.sessionID, inboxID: queued.id }),
807+
).then(
808+
(outcome) => {
809+
if (!outcome) return
810+
if (outcome === "delivered") {
811+
toast.show({
812+
message: "Prompt was already sent. Use /undo to take it back",
813+
variant: "warning",
814+
})
815+
return
816+
}
817+
target.setMode("normal")
818+
target.set(
819+
appendPrompt(
820+
target.current,
821+
{ ...projectedPromptInput(queued.payload), pasted: [] },
822+
renderer.widthMethod,
823+
),
824+
)
825+
dialog.clear()
826+
target.focus()
827+
},
828+
(error) => {
829+
toast.show({
830+
title: "Failed to undo pending prompt",
831+
message: errorMessage(error),
832+
variant: "error",
833+
})
834+
},
835+
)
818836
},
819837
},
820838
]}
@@ -1115,21 +1133,11 @@ export function Session(props: {
11151133
dialog.clear()
11161134
return
11171135
}
1118-
const sessionID = route.sessionID
11191136
const target = prompt()
1120-
void (async () => {
1121-
if (pendingDeliveries().has(message.id)) {
1122-
if (!(await mutatePending("cancel", message.id))) return
1123-
} else {
1124-
await client.api.session.interrupt({ sessionID })
1125-
await client.api.session.wait({ sessionID })
1126-
await client.api.session.revert.stage({ sessionID, messageID: message.id })
1127-
}
1128-
target?.set({
1129-
...projectedPromptInput(message),
1130-
pasted: [],
1131-
})
1132-
})().catch((error) => toast.show({ message: errorMessage(error), variant: "error", duration: 5000 }))
1137+
void data.session
1138+
.undo({ sessionID: route.sessionID, messageID: message.id })
1139+
.then(() => target?.set({ ...projectedPromptInput(message), pasted: [] }))
1140+
.catch((error) => toast.show({ message: errorMessage(error), variant: "error", duration: 5000 }))
11331141
dialog.clear()
11341142
},
11351143
},
@@ -1268,7 +1276,9 @@ export function Session(props: {
12681276
},
12691277
run: () => {
12701278
const lastAssistantMessage = messagesBeforeRevert().findLast(
1271-
(msg) => msg.type === "assistant" || (msg.type === "compaction" && msg.status === "completed" && msg.summary.trim() !== ""),
1279+
(msg) =>
1280+
msg.type === "assistant" ||
1281+
(msg.type === "compaction" && msg.status === "completed" && msg.summary.trim() !== ""),
12721282
)
12731283
if (!lastAssistantMessage) {
12741284
toast.show({ message: "No assistant messages found", variant: "error" })
@@ -1578,9 +1588,7 @@ export function Session(props: {
15781588
onMouseOut={() => setLatestHovered(false)}
15791589
onMouseUp={toBottom}
15801590
>
1581-
<text
1582-
fg={latestHovered() ? theme.text.action.secondary.hovered : theme.text.action.secondary.base}
1583-
>
1591+
<text fg={latestHovered() ? theme.text.action.secondary.hovered : theme.text.action.secondary.base}>
15841592
Jump to latest ↓
15851593
</text>
15861594
</box>
@@ -1626,12 +1634,7 @@ export function Session(props: {
16261634
}}
16271635
</Show>
16281636
</Match>
1629-
<Match
1630-
when={
1631-
session() &&
1632-
currentLocation.error?.location.directory === session()!.location.directory
1633-
}
1634-
>
1637+
<Match when={session() && currentLocation.error?.location.directory === session()!.location.directory}>
16351638
<SessionLocationMissing
16361639
directory={session()!.location.directory}
16371640
projectID={session()!.projectID}
@@ -2927,9 +2930,7 @@ function BlockTool(props: BlockToolProps) {
29272930
<Show
29282931
when={props.spinner}
29292932
fallback={
2930-
<text
2931-
fg={permission() ? theme.text.feedback.warning.base : (props.headerColor ?? theme.text.muted)}
2932-
>
2933+
<text fg={permission() ? theme.text.feedback.warning.base : (props.headerColor ?? theme.text.muted)}>
29332934
{title()}
29342935
</text>
29352936
}
@@ -3117,11 +3118,7 @@ function ShellDisplay(props: {
31173118
<Show
31183119
when={isRunning()}
31193120
fallback={
3120-
<text
3121-
fg={theme.text.base}
3122-
wrapMode={expanded() ? "word" : "char"}
3123-
maxHeight={expanded() ? undefined : 2}
3124-
>
3121+
<text fg={theme.text.base} wrapMode={expanded() ? "word" : "char"} maxHeight={expanded() ? undefined : 2}>
31253122
{limitedInput()}
31263123
</text>
31273124
}
@@ -3222,7 +3219,9 @@ function Read(props: ToolProps) {
32223219
Read {pathFormatter.format(stringValue(props.input.path))}
32233220
<Show when={props.input.offset !== undefined || props.input.limit !== undefined}>
32243221
:{finiteNumber(props.input.offset) || 1}-
3225-
{props.input.limit ? (finiteNumber(props.input.offset) || 1) + (finiteNumber(props.input.limit) || 0) - 1 : ""}
3222+
{props.input.limit
3223+
? (finiteNumber(props.input.offset) || 1) + (finiteNumber(props.input.limit) || 0) - 1
3224+
: ""}
32263225
</Show>
32273226
</InlineTool>
32283227
<For each={loaded()}>

0 commit comments

Comments
 (0)