From 48d4e52143db366896dab9eaaa8f90274d1cc0e7 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Thu, 27 Aug 2026 12:44:18 +1000 Subject: [PATCH] fix(app): keep pending steers after assistant work (#45435) --- .../app/e2e/regression/session-queue.spec.ts | 143 +++++++++++++++++- .../timeline/controller-projection.test.ts | 101 +++++++++++++ .../session/timeline/controller-projection.ts | 15 +- 3 files changed, 252 insertions(+), 7 deletions(-) diff --git a/packages/app/e2e/regression/session-queue.spec.ts b/packages/app/e2e/regression/session-queue.spec.ts index 21f06eac2bf..93bc42ce092 100644 --- a/packages/app/e2e/regression/session-queue.spec.ts +++ b/packages/app/e2e/regression/session-queue.spec.ts @@ -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: Type, + data: Extract["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 }) => { prompts.push(input.body) @@ -126,10 +131,11 @@ async function openSession(page: Page, mock: ReturnType, 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/, + ]) + }) +} diff --git a/packages/app/src/session/timeline/controller-projection.test.ts b/packages/app/src/session/timeline/controller-projection.test.ts index 8783a78633b..dade7fdf141 100644 --- a/packages/app/src/session/timeline/controller-projection.test.ts +++ b/packages/app/src/session/timeline/controller-projection.test.ts @@ -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 = [ { diff --git a/packages/app/src/session/timeline/controller-projection.ts b/packages/app/src/session/timeline/controller-projection.ts index ec0c0c259ac..29d79aae12e 100644 --- a/packages/app/src/session/timeline/controller-projection.ts +++ b/packages/app/src/session/timeline/controller-projection.ts @@ -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: {