fix(app): keep pending steers after assistant work (#45435)

This commit is contained in:
Luke Parker
2026-08-27 12:44:18 +10:00
committed by GitHub
parent 40cbea3c19
commit 48d4e52143
3 changed files with 252 additions and 7 deletions
@@ -1,5 +1,5 @@
import { expect, test, type Page } from "@playwright/test"
import type { OpenCodeEvent } from "@opencode-ai/client/promise"
import type { OpenCodeEvent, SessionMessageInfo } from "@opencode-ai/client/promise"
import { base64Encode } from "@opencode-ai/util/encode"
import { mockOpenCodeServer } from "../utils/mock-server"
import { expectAppVisible } from "../utils/waits"
@@ -18,7 +18,7 @@ type InboxRow = {
delivery: "steer" | "queue"
}
function createQueueMock(seed: string[]) {
function createQueueMock(seed: string[], messages: SessionMessageInfo[] = []) {
const rows: InboxRow[] = seed.map((text, index) => ({
id: `inb_seed_${index + 1}`,
sessionID,
@@ -32,13 +32,16 @@ function createQueueMock(seed: string[]) {
const changes: { inboxID: string; action: "cancel" | "steer" }[] = []
const log: string[] = []
let sequence = 0
const emit = (type: OpenCodeEvent["type"], data: OpenCodeEvent["data"]) => {
const emit = <Type extends OpenCodeEvent["type"]>(
type: Type,
data: Extract<OpenCodeEvent, { type: Type }>["data"],
) => {
sequence += 1
events.push({
id: `evt_queue_${sequence}`,
type,
created: Date.now(),
durable: { aggregateID: sessionID, seq: sequence, version: 1 },
durable: { aggregateID: sessionID, seq: sequence, version: type === "session.tool.success" ? 2 : 1 },
data,
} as OpenCodeEvent)
}
@@ -47,6 +50,8 @@ function createQueueMock(seed: string[]) {
prompts,
changes,
log,
messages,
emit,
events: () => events.splice(0),
onPrompt: (input: { sessionID: string; body: Record<string, unknown> }) => {
prompts.push(input.body)
@@ -126,10 +131,11 @@ async function openSession(page: Page, mock: ReturnType<typeof createQueueMock>,
directory,
title: "Session queue regression",
version: "dev",
model: { id: "queue-model", providerID: "opencode" },
time: { created: 1700000000000, updated: 1700000000000 },
},
],
pageMessages: () => ({ items: [] }),
pageMessages: () => ({ items: mock.messages }),
sessionStatus: () => ({ [sessionID]: { type: "running" } }),
inbox: () => mock.rows.map((row) => ({ ...row, payload: { ...row.payload } })),
onPrompt: mock.onPrompt,
@@ -227,3 +233,130 @@ test("editing restores the existing draft and replaces only the original queue p
expect(mock.changes.map((change) => change.action)).toEqual(["cancel", "cancel", "cancel"])
expect(mock.log[0]).toBe("prompt:queue")
})
for (const delivery of ["steer", "queue"] as const) {
test(`keeps finished tools above a pending ${delivery === "queue" ? "queue-to-steer" : "steer"} follow-up`, async ({
page,
}, testInfo) => {
const model = { id: "queue-model", providerID: "opencode" }
const userID = "msg_queue_initial_user"
const assistantID = "msg_queue_continued_assistant"
const followUp = "U2: Also check the retry path."
const mock = createQueueMock(
[],
[
{ id: userID, type: "user", text: "U1: Inspect the queue ordering.", time: { created: 1700000000000 } },
{
id: "msg_queue_initial_assistant",
type: "assistant",
agent: "build",
model,
content: [{ type: "text", text: "A1: I will inspect the current implementation." }],
finish: "tool-calls",
time: { created: 1700000000001, completed: 1700000000002 },
},
],
)
const view = await openSession(page, mock, delivery)
const transcript = page.locator("[data-timeline-virtual-content]")
const thinking = transcript.locator('[data-timeline-row="Thinking"]')
await expect(transcript.getByText("A1: I will inspect the current implementation.", { exact: true })).toBeVisible()
await expect(thinking).toBeVisible()
await expect(view.input).toBeEditable()
await view.input.fill(followUp)
await view.input.press("Enter")
await expect.poll(() => mock.rows.map((row) => row.delivery)).toEqual([delivery])
await expect(view.input).toHaveText("")
const inboxID = mock.rows[0].id
const pending = transcript.locator(`[data-timeline-row="UserMessage"][data-message-id="${inboxID}"]`)
if (delivery === "queue") {
const queued = view.rows.filter({ hasText: followUp })
await expect(queued).toBeVisible()
await expect(pending).toHaveCount(0)
await queued.hover()
await queued.getByRole("button", { name: "Steer", exact: true }).click()
await expect.poll(() => mock.changes).toEqual([{ inboxID, action: "steer" }])
}
await expect(view.rows).toHaveCount(0)
await expect(pending).toContainText(followUp)
// The next assistant step still belongs to U1: U2 has been admitted, not delivered.
mock.emit("session.step.started", { sessionID, assistantMessageID: assistantID, agent: "build", model })
for (const tool of [
{ id: "tool_queue_read", name: "read", input: { path: "src/queue.ts" } },
{ id: "tool_queue_grep", name: "grep", input: { pattern: "retry", path: "src" } },
]) {
const ref = { sessionID, assistantMessageID: assistantID, id: tool.id }
mock.emit("session.tool.input.started", { ...ref, name: tool.name })
mock.emit("session.tool.input.ended", { ...ref, text: JSON.stringify(tool.input) })
mock.emit("session.tool.called", { ...ref, input: tool.input, executed: true })
mock.emit("session.tool.success", {
...ref,
content: [{ type: "text", text: "Inspection complete." }],
executed: true,
})
}
mock.emit("session.step.ended", {
sessionID,
assistantMessageID: assistantID,
finish: "tool-calls",
cost: 0,
tokens: { input: 100, output: 20, reasoning: 0, cache: { read: 0, write: 0 } },
})
const tools = page.locator('[data-timeline-part-ids="tool_queue_read,tool_queue_grep"]')
await expect(tools).toBeVisible()
await expect(tools).toContainText(/Used\s*Read, Grep/)
await expect(tools.locator('[data-component="tag"]')).toHaveText("2")
await expect(thinking).toBeVisible()
await expect(pending).toBeVisible()
expect(mock.rows.map((row) => ({ id: row.id, delivery: row.delivery }))).toEqual([
{ id: inboxID, delivery: "steer" },
])
await transcript.screenshot({ path: testInfo.outputPath("pending-steer.png") })
// Soft assertions let delivery run too, even when the pending ordering regresses.
await expect
.soft(tools.or(thinking).or(pending))
.toHaveText([/Used\s*Read, Grep/, /Thinking/, /U2: Also check the retry path\./])
await expect
.soft(transcript.locator('[data-timeline-row="AssistantPart"]').filter({ has: tools }))
.toHaveAttribute("data-message-id", userID)
await expect
.configure({ soft: true })
.poll(async () => {
const boxes = await Promise.all([tools.boundingBox(), thinking.boundingBox(), pending.boundingBox()])
return (
boxes.every((box) => box !== null) &&
boxes[0]!.y + boxes[0]!.height <= boxes[1]!.y &&
boxes[1]!.y + boxes[1]!.height <= boxes[2]!.y
)
})
.toBe(true)
mock.rows.splice(0, 1)
mock.emit("session.inbox.delivered", { sessionID, inboxID })
await expect(thinking).toHaveAttribute("data-message-id", inboxID)
await expect(pending).toHaveCount(1)
await expect(transcript.locator('[data-timeline-row="UserMessage"]')).toHaveCount(2)
await expect(transcript.locator('[data-timeline-row="AssistantPart"]').filter({ has: tools })).toHaveAttribute(
"data-message-id",
userID,
)
const later = { sessionID, assistantMessageID: "msg_queue_follow_up_assistant" }
mock.emit("session.step.started", { ...later, agent: "build", model })
mock.emit("session.text.started", { ...later, ordinal: 0 })
mock.emit("session.text.ended", { ...later, ordinal: 0, text: "A3: Now checking the retry path for U2." })
const response = transcript
.locator('[data-timeline-row="AssistantPart"]')
.filter({ hasText: "A3: Now checking the retry path for U2." })
await expect(response).toHaveAttribute("data-message-id", inboxID)
await expect(tools.or(pending).or(response).or(thinking)).toHaveText([
/Used\s*Read, Grep/,
/U2: Also check the retry path\./,
/A3: Now checking the retry path for U2\./,
/Thinking/,
])
})
}
@@ -1,6 +1,8 @@
import { describe, expect, test } from "bun:test"
import type { SessionInboxInfo, SessionMessageInfo } from "@opencode-ai/client/promise"
import { createRoot } from "solid-js"
import { applyTimelineMessageHandoff, visibleTimelineMessages } from "./controller-projection"
import { createTimelineProjection } from "./projection"
const messages = [
{ id: "msg_1", type: "user", text: "first", time: { created: 1 } },
@@ -17,6 +19,105 @@ const messages = [
] satisfies SessionMessageInfo[]
describe("visibleTimelineMessages", () => {
const steer = {
id: "msg_3",
sessionID: "ses_1",
timeCreated: 3,
type: "user",
delivery: "steer",
payload: { text: "queued" },
} satisfies SessionInboxInfo
const work = {
id: "msg_5",
type: "assistant",
agent: "build",
model: { id: "model", providerID: "provider" },
content: [
{
type: "tool",
id: "tool_read",
name: "read",
state: {
status: "completed",
input: { filePath: "src/example.ts" },
content: [{ type: "text", text: "export const example = true" }],
metadata: {},
},
time: { created: 5, completed: 6 },
},
],
time: { created: 5, completed: 6 },
} satisfies SessionMessageInfo
test("keeps work and thinking above an undelivered steer", () => {
const source = [...messages.slice(0, 3), work]
const visible = visibleTimelineMessages(source, [steer])
expect(visible.map((message) => message.id)).toEqual(["msg_1", "msg_2", "msg_5", "msg_3"])
expect(source.map((message) => message.id)).toEqual(["msg_1", "msg_2", "msg_3", "msg_5"])
expect(visible[2]).toBe(work)
createRoot((dispose) => {
const projection = createTimelineProjection({
sessionMessages: () => visible,
status: () => ({ type: "busy" }),
showReasoningSummaries: () => false,
shellToolDefaultOpen: () => false,
editToolDefaultOpen: () => false,
pendingUserMessageIDs: () => new Set([steer.id]),
})
expect(projection.activeMessageID()).toBe("msg_1")
expect(projection.rows().map((row) => [row._tag, row.userMessageID])).toEqual([
["UserMessage", "msg_1"],
["AssistantPart", "msg_1"],
["Thinking", "msg_1"],
["TurnGap", "msg_3"],
["UserMessage", "msg_3"],
])
expect(
projection
.assistantMessagesByParent()
.get("msg_1")
?.map((message) => message.id),
).toEqual(["msg_2", "msg_5"])
expect(projection.assistantMessagesByParent().has(steer.id)).toBe(false)
dispose()
})
})
test("moves a queued input after existing work when changed to steer", () => {
const source = [...messages.slice(0, 3), work]
expect(visibleTimelineMessages(source, [{ ...steer, delivery: "queue" }]).map((message) => message.id)).toEqual([
"msg_1",
"msg_2",
"msg_5",
])
expect(visibleTimelineMessages(source, [steer]).map((message) => message.id)).toEqual([
"msg_1",
"msg_2",
"msg_5",
"msg_3",
])
const delivered = [messages[0], messages[1], work, messages[2]]
expect(visibleTimelineMessages(delivered, [])).toBe(delivered)
})
test("preserves steer order and excludes reverted steers", () => {
const source = [...messages, work]
const pending = [steer, { ...steer, id: "msg_4" }]
expect(visibleTimelineMessages(source, pending).map((message) => message.id)).toEqual([
"msg_1",
"msg_2",
"msg_5",
"msg_3",
"msg_4",
])
expect(visibleTimelineMessages(source, pending, "msg_4").map((message) => message.id)).toEqual([
"msg_1",
"msg_2",
"msg_3",
])
})
test("hides queued inputs until delivery", () => {
const pending = [
{
@@ -17,8 +17,19 @@ export function visibleTimelineMessages(
const queued = new Set(
pending.flatMap((item) => (item.type === "user" && item.delivery === "queue" ? [item.id] : [])),
)
if (queued.size === 0 && !revertMessageID) return messages
return messages.filter((message) => !queued.has(message.id) && (!revertMessageID || message.id < revertMessageID))
const steers = new Set(
pending.flatMap((item) => (item.type === "user" && item.delivery === "steer" ? [item.id] : [])),
)
if (queued.size === 0 && steers.size === 0 && !revertMessageID) return messages
const visible = messages.filter(
(message) => !queued.has(message.id) && (!revertMessageID || message.id < revertMessageID),
)
if (steers.size === 0) return visible
// Pending steers do not own assistant work until they are delivered.
return [
...visible.filter((message) => !steers.has(message.id)),
...visible.filter((message) => steers.has(message.id)),
]
}
export function timelineChildTitle(input: {