diff --git a/apps/server/test/public/public-thread-timeline-manager-response-regressions.test.ts b/apps/server/test/public/public-thread-timeline-manager-response-regressions.test.ts new file mode 100644 index 0000000000..a37a53e15e --- /dev/null +++ b/apps/server/test/public/public-thread-timeline-manager-response-regressions.test.ts @@ -0,0 +1,322 @@ +import { + encodeClientTurnRequestIdNumber, + threadScope, + turnScope, +} from "@bb/domain"; +import { + threadTimelineResponseSchema, + type ThreadTimelineResponse, + type TimelineRow, +} from "@bb/server-contract"; +import { describe, expect, it } from "vitest"; +import { pruneThreadEventHistory } from "../../src/services/system/event-pruning.js"; +import { readJson } from "../helpers/json.js"; +import { seedEvent, seedThreadFixture } from "../helpers/seed.js"; +import { withTestHarness } from "../helpers/test-app.js"; +import type { TestAppHarness } from "../helpers/test-app.js"; + +const EXECUTION = { + model: "gpt-5", + serviceTier: "default", + reasoningLevel: "medium", + permissionMode: "full", + source: "client/turn/requested", +} as const; + +async function getTimeline( + harness: TestAppHarness, + threadId: string, +): Promise { + const response = await harness.app.request( + `/api/v1/threads/${threadId}/timeline`, + ); + expect(response.status).toBe(200); + return threadTimelineResponseSchema.parse(await readJson(response)); +} + +function topLevelConversations(rows: readonly TimelineRow[]) { + return rows.filter( + (row): row is Extract => + row.kind === "conversation", + ); +} + +describe("GET /threads/:id/timeline manager response boundaries", () => { + it("keeps the cproxy response, child completion, and final response top-level", async () => { + await withTestHarness(async (harness) => { + const { environment, thread } = seedThreadFixture(harness); + const turn = { + threadId: thread.id, + environmentId: environment.id, + providerThreadId: "cproxy-session-1", + scope: turnScope("turn-cproxy"), + } as const; + const childTellRequestId = encodeClientTurnRequestIdNumber({ value: 1 }); + const lifecycleRequestId = encodeClientTurnRequestIdNumber({ value: 2 }); + + seedEvent(harness.deps, { + threadId: thread.id, + environmentId: environment.id, + sequence: 1, + type: "client/turn/requested", + scope: threadScope(), + data: { + direction: "outbound", + requestId: childTellRequestId, + source: "tell", + initiator: "agent", + senderThreadId: "thr_cproxy_child", + input: [ + { type: "text", text: "Child report: implementation ready." }, + ], + target: { kind: "new-turn" }, + request: { method: "turn/start", params: {} }, + execution: EXECUTION, + }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 2, + type: "turn/started", + data: {}, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 3, + type: "turn/input/accepted", + data: { clientRequestId: childTellRequestId }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 4, + type: "item/completed", + data: { + item: { + type: "agentMessage", + id: "cproxy-assistant-detailed", + text: "I reviewed the full implementation and confirmed each important invariant.", + }, + }, + }); + seedEvent(harness.deps, { + threadId: thread.id, + environmentId: environment.id, + sequence: 5, + type: "client/turn/requested", + scope: threadScope(), + data: { + direction: "outbound", + requestId: lifecycleRequestId, + source: "tell", + initiator: "system", + senderThreadId: null, + systemMessageKind: "child-completed", + systemMessageSubject: { + kind: "thread", + threadId: "thr_cproxy_child", + threadName: "cproxy worker", + }, + input: [ + { + type: "text", + text: "cproxy worker completed: implementation ready.", + }, + ], + target: { kind: "auto", expectedTurnId: "turn-cproxy" }, + request: { method: "turn/start", params: {} }, + execution: EXECUTION, + }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 6, + type: "turn/input/accepted", + data: { clientRequestId: lifecycleRequestId }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 7, + type: "item/completed", + data: { + item: { + type: "agentMessage", + id: "cproxy-assistant-final", + text: "Done.", + }, + }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 8, + type: "turn/completed", + data: { status: "completed" }, + }); + + const timeline = await getTimeline(harness, thread.id); + const conversations = topLevelConversations(timeline.rows); + expect(conversations.map((row) => row.text)).toEqual([ + "Child report: implementation ready.", + "I reviewed the full implementation and confirmed each important invariant.", + "cproxy worker completed: implementation ready.", + "Done.", + ]); + const assistantRows = conversations.filter( + (row) => row.role === "assistant", + ); + expect(assistantRows).toHaveLength(2); + expect(new Set(assistantRows.map((row) => row.id)).size).toBe(2); + }); + }); + + it("keeps Terminal-Bench assistant texts recoverable after resolved-delta pruning", async () => { + await withTestHarness(async (harness) => { + const { environment, thread } = seedThreadFixture(harness); + const turn = { + threadId: thread.id, + environmentId: environment.id, + providerThreadId: "terminal-bench-session-1", + scope: turnScope("turn-terminal-bench"), + } as const; + const lifecycleRequestId = encodeClientTurnRequestIdNumber({ value: 1 }); + const firstText = + "The complete Terminal-Bench analysis remains visible across the lifecycle steer."; + + seedEvent(harness.deps, { + ...turn, + sequence: 1, + type: "turn/started", + data: {}, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 2, + type: "item/agentMessage/delta", + data: { + itemId: "terminal-assistant-detailed", + delta: "The complete Terminal-Bench analysis remains visible ", + }, + }); + seedEvent(harness.deps, { + threadId: thread.id, + environmentId: environment.id, + sequence: 3, + type: "client/turn/requested", + scope: threadScope(), + data: { + direction: "outbound", + requestId: lifecycleRequestId, + source: "tell", + initiator: "system", + senderThreadId: null, + systemMessageKind: "child-completed", + systemMessageSubject: { + kind: "thread", + threadId: "thr_terminal_bench", + threadName: "Terminal-Bench worker", + }, + input: [ + { + type: "text", + text: "Terminal-Bench worker completed: checks passed.", + }, + ], + target: { + kind: "auto", + expectedTurnId: "turn-terminal-bench", + }, + request: { method: "turn/start", params: {} }, + execution: EXECUTION, + }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 4, + type: "turn/input/accepted", + data: { clientRequestId: lifecycleRequestId }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 5, + type: "item/agentMessage/delta", + data: { + itemId: "terminal-assistant-detailed", + delta: "across the lifecycle steer.", + }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 6, + type: "item/completed", + data: { + item: { + type: "agentMessage", + id: "terminal-assistant-detailed", + text: firstText, + }, + }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 7, + type: "item/completed", + data: { + item: { + type: "commandExecution", + id: "terminal-tool-boundary", + command: "terminal-bench run --task regression", + cwd: "/repo", + status: "completed", + approvalStatus: null, + exitCode: 0, + }, + }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 8, + type: "item/agentMessage/delta", + data: { + itemId: "terminal-assistant-final", + delta: "Verified.", + }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 9, + type: "item/completed", + data: { + item: { + type: "agentMessage", + id: "terminal-assistant-final", + text: "Verified.", + }, + }, + }); + seedEvent(harness.deps, { + ...turn, + sequence: 10, + type: "turn/completed", + data: { status: "completed" }, + }); + + const pruning = pruneThreadEventHistory(harness.deps, { + mode: "idle", + threadId: thread.id, + }); + expect(pruning.removedResolvedItemDeltas).toBe(1); + + const timeline = await getTimeline(harness, thread.id); + const conversations = topLevelConversations(timeline.rows); + expect(conversations.map((row) => row.text)).toEqual([ + firstText, + "Terminal-Bench worker completed: checks passed.", + "Verified.", + ]); + const assistantRows = conversations.filter( + (row) => row.role === "assistant", + ); + expect(assistantRows).toHaveLength(2); + expect(new Set(assistantRows.map((row) => row.id)).size).toBe(2); + }); + }); +}); diff --git a/packages/agent-runtime/src/__fixtures__/pi/message-start-assistant.json b/packages/agent-runtime/src/__fixtures__/pi/message-start-assistant.json new file mode 100644 index 0000000000..8a4a79f7dd --- /dev/null +++ b/packages/agent-runtime/src/__fixtures__/pi/message-start-assistant.json @@ -0,0 +1,26 @@ +{ + "type": "message_start", + "message": { + "role": "assistant", + "content": [], + "api": "anthropic-messages", + "provider": "anthropic", + "model": "claude-haiku-4-5", + "usage": { + "input": 0, + "output": 0, + "cacheRead": 0, + "cacheWrite": 0, + "totalTokens": 0, + "cost": { + "input": 0, + "output": 0, + "cacheRead": 0, + "cacheWrite": 0, + "total": 0 + } + }, + "stopReason": "pending", + "timestamp": 1777995780000 + } +} diff --git a/packages/agent-runtime/src/pi/delta-translation.test.ts b/packages/agent-runtime/src/pi/delta-translation.test.ts index 83514bd7c2..9f6b6ee48a 100644 --- a/packages/agent-runtime/src/pi/delta-translation.test.ts +++ b/packages/agent-runtime/src/pi/delta-translation.test.ts @@ -604,6 +604,236 @@ describe("pi delta translation equivalence", () => { ); }); + it("assigns distinct ids to consecutive assistant messages without a tool boundary", () => { + const harness = createHarness(); + harness.translate(loadFixture("agent-start.json")); + + harness.translate(loadFixture("message-start-assistant.json")); + const firstDeltaEvents = harness.translate({ + type: "message_update", + assistantMessageEvent: { + type: "text_delta", + contentIndex: 0, + delta: + "The detailed response stays visible after the child lifecycle update.", + }, + } as AgentSessionEvent); + const firstItemId = agentMessageDeltaId(firstDeltaEvents); + + const firstCompletedEvents = harness.translate( + loadFixture("message-start-assistant.json"), + ); + const secondDeltaEvents = harness.translate({ + type: "message_update", + assistantMessageEvent: { + type: "text_delta", + contentIndex: 0, + delta: "Done.", + }, + } as AgentSessionEvent); + const secondItemId = agentMessageDeltaId(secondDeltaEvents); + const finalEvents = harness.translate({ + type: "agent_end", + messages: [ + { + role: "assistant", + content: [{ type: "text", text: "Done." }], + stopReason: "stop", + }, + ], + willRetry: false, + } as AgentSessionEvent); + + expect(firstItemId).toMatch(ITEM_ID_PATTERN); + expect(secondItemId).toMatch(ITEM_ID_PATTERN); + expect(secondItemId).not.toBe(firstItemId); + expect(firstCompletedEvents).toContainEqual( + expect.objectContaining({ + type: "item/completed", + item: expect.objectContaining({ + id: firstItemId, + type: "agentMessage", + text: "The detailed response stays visible after the child lifecycle update.", + }), + }), + ); + expect(finalEvents).toContainEqual( + expect.objectContaining({ + type: "item/completed", + item: expect.objectContaining({ + id: secondItemId, + type: "agentMessage", + text: "Done.", + }), + }), + ); + }); + + it("settles retry partial output before assigning the retried assistant a new id", () => { + const harness = createHarness(); + harness.translate(loadFixture("agent-start.json")); + harness.translate(loadFixture("message-start-assistant.json")); + + const partialEvents = harness.translate({ + type: "message_update", + assistantMessageEvent: { + type: "text_delta", + contentIndex: 0, + delta: "Partial output before retry.", + }, + } as AgentSessionEvent); + const partialItemId = agentMessageDeltaId(partialEvents); + const retryEvents = harness.translate( + createPiAgentErrorEvent("temporary provider failure", true), + ); + const retryBoundaryEvents = harness.translate( + loadFixture("message-start-assistant.json"), + ); + const retriedEvents = harness.translate({ + type: "message_update", + assistantMessageEvent: { + type: "text_delta", + contentIndex: 0, + delta: "Full output after retry.", + }, + } as AgentSessionEvent); + const retriedItemId = agentMessageDeltaId(retriedEvents); + const finalEvents = harness.translate({ + type: "agent_end", + messages: [ + { + role: "assistant", + content: [{ type: "text", text: "Full output after retry." }], + stopReason: "stop", + }, + ], + willRetry: false, + } as AgentSessionEvent); + + expect(partialItemId).toMatch(ITEM_ID_PATTERN); + expect(retryEvents.some((event) => event.type === "item/completed")).toBe( + false, + ); + expect(retryBoundaryEvents).toContainEqual( + expect.objectContaining({ + type: "item/completed", + item: expect.objectContaining({ + id: partialItemId, + type: "agentMessage", + text: "Partial output before retry.", + }), + }), + ); + expect(retriedItemId).toMatch(ITEM_ID_PATTERN); + expect(retriedItemId).not.toBe(partialItemId); + expect(finalEvents).toContainEqual( + expect.objectContaining({ + type: "item/completed", + item: expect.objectContaining({ + id: retriedItemId, + type: "agentMessage", + text: "Full output after retry.", + }), + }), + ); + }); + + it("does not duplicate a completed assistant at an empty aborted boundary", () => { + const harness = createHarness(); + harness.translate(loadFixture("agent-start.json")); + harness.translate(loadFixture("message-start-assistant.json")); + + const textEvents = harness.translate({ + type: "message_update", + assistantMessageEvent: { + type: "text_delta", + contentIndex: 0, + delta: "Completed before the abort.", + }, + } as AgentSessionEvent); + const completedItemId = agentMessageDeltaId(textEvents); + const boundaryEvents = harness.translate( + loadFixture("message-start-assistant.json"), + ); + const emptyBoundaryEvents = harness.translate( + loadFixture("message-start-assistant.json"), + ); + const abortEvents = harness.translate({ + type: "agent_end", + messages: [ + { + role: "assistant", + content: [{ type: "text", text: "Completed before the abort." }], + api: "anthropic-messages", + provider: "anthropic", + model: "claude-haiku-4-5", + usage: { + input: 10, + output: 5, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 15, + cost: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + total: 0, + }, + }, + stopReason: "stop", + timestamp: 1777995781000, + }, + { + role: "assistant", + content: [], + api: "anthropic-messages", + provider: "anthropic", + model: "claude-haiku-4-5", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + total: 0, + }, + }, + stopReason: "aborted", + errorMessage: "Request aborted", + timestamp: 1777995782000, + }, + ], + willRetry: false, + } satisfies AgentSessionEvent); + const completedAssistantEvents = [ + ...boundaryEvents, + ...emptyBoundaryEvents, + ...abortEvents, + ].filter( + (event) => + event.type === "item/completed" && event.item.type === "agentMessage", + ); + + expect(completedItemId).toMatch(ITEM_ID_PATTERN); + expect(boundaryEvents).toContainEqual( + expect.objectContaining({ + type: "item/completed", + item: expect.objectContaining({ + id: completedItemId, + text: "Completed before the abort.", + }), + }), + ); + expect(emptyBoundaryEvents).toEqual([]); + expect(completedAssistantEvents).toHaveLength(1); + }); + it("assigns a new assistant id after a tool call interrupts streaming", () => { const harness = createHarness(); harness.translate(loadFixture("agent-start.json")); @@ -755,8 +985,7 @@ describe("pi delta translation equivalence", () => { const started = events.find( (event) => - event.type === "item/started" && - event.item.type === "commandExecution", + event.type === "item/started" && event.item.type === "commandExecution", ); if (started?.type !== "item/started") { throw new Error("expected a commandExecution item/started"); diff --git a/packages/agent-runtime/src/pi/delta-translation.ts b/packages/agent-runtime/src/pi/delta-translation.ts index 6e5dc7e95e..377be6d965 100644 --- a/packages/agent-runtime/src/pi/delta-translation.ts +++ b/packages/agent-runtime/src/pi/delta-translation.ts @@ -60,6 +60,7 @@ const piEventTypeSchema = z "agent_start", "compaction_end", "compaction_start", + "message_start", "message_update", "tool_execution_end", "tool_execution_start", @@ -181,6 +182,17 @@ function isPiCompactionNoop(errorMessage: string): boolean { return piCompactionNoopMessages.has(errorMessage.trim()); } +const piAssistantMessageStartEventSchema = z + .object({ + type: z.literal("message_start"), + message: z + .object({ + role: z.literal("assistant"), + }) + .passthrough(), + }) + .passthrough(); + const piMessageUpdateEventSchema = z .object({ type: z.literal("message_update"), @@ -784,6 +796,23 @@ export function createPiDeltaTranslator( return deltas; } + case "message_start": { + const piEvent = piAssistantMessageStartEventSchema.safeParse(event); + if (!piEvent.success) { + return []; + } + // The next assistant start finalizes accumulated prior text; the first + // start has no open stream and is a no-op in the shared assembler. + return [ + { + kind: "message.close", + channel: "assistant", + streamKey: ASSISTANT_STREAM_KEY, + ...parentRefField, + }, + ]; + } + case "message_update": { const piEvent = piMessageUpdateEventSchema.safeParse(event); if (!piEvent.success) { diff --git a/packages/host-daemon-contract/src/protocol.ts b/packages/host-daemon-contract/src/protocol.ts index d4e52b3223..eff9b09207 100644 --- a/packages/host-daemon-contract/src/protocol.ts +++ b/packages/host-daemon-contract/src/protocol.ts @@ -1,3 +1,7 @@ +// Version 144 closes Pi's prior assistant stream at each assistant +// `message_start`, so later assistant output uses a new canonical item id. +// Older daemons can merge separate assistant outputs and prune visible text. +// // Version 143 lets daemons from before session-open's `localApiPort` field // reach the protocol-version check by defaulting that field at the server // boundary. Without it, those daemons receive `invalid_request` instead of @@ -90,7 +94,7 @@ // // The version mismatch is what triggers the enrolled daemon's automatic update // instead of an `invalid-message` reconnect loop. -export const HOST_DAEMON_PROTOCOL_VERSION = 143 as const; +export const HOST_DAEMON_PROTOCOL_VERSION = 144 as const; /** * Absolute ceiling for any executable artifact delivered to a host daemon — diff --git a/packages/host-daemon-contract/test/contract.test.ts b/packages/host-daemon-contract/test/contract.test.ts index a6c232256e..a89ad6857d 100644 --- a/packages/host-daemon-contract/test/contract.test.ts +++ b/packages/host-daemon-contract/test/contract.test.ts @@ -1138,7 +1138,7 @@ describe("host-daemon command schemas", () => { // mixed version. Version 113 carried the Devin Desktop open target rename // and remains part of the protocol lineage. it("uses the current host-daemon protocol version", () => { - expect(HOST_DAEMON_PROTOCOL_VERSION).toBe(143); + expect(HOST_DAEMON_PROTOCOL_VERSION).toBe(144); expect(HOST_ARTIFACT_MAX_BYTES).toBe(256 * 1024 * 1024); }); diff --git a/packages/thread-view/src/completed-turn-grouping.ts b/packages/thread-view/src/completed-turn-grouping.ts index df6a8e6a2d..ee86eecf9a 100644 --- a/packages/thread-view/src/completed-turn-grouping.ts +++ b/packages/thread-view/src/completed-turn-grouping.ts @@ -5,8 +5,9 @@ import type { import { getProjectionSummaryCount } from "./apply-turn-message-detail.js"; import { getMessageStartedAt } from "./format-helpers.js"; import { - findLastTerminalTimelineMessage, + isChildLifecycleSystemSteerMessage, isSingletonContextManagementOperation, + isTimelineTerminalMessage, isTimelineUngroupableMessage, } from "./timeline-message-helpers.js"; @@ -190,14 +191,17 @@ function groupCompletedTurnSummaryMessages( return; } - // Human follow-ups split one provider turn into multiple visible exchange - // segments. Keep each segment's last assistant/error message beside the - // user row instead of burying it inside that segment's collapsed summary. + // Human and child-lifecycle boundaries split one provider turn into visible + // exchanges. Preserve only an assistant/error directly beside the boundary. const sourceMessages = groupedMessages; groupedMessages = []; - const terminalMessage = preserveLastTerminalMessage - ? findLastTerminalTimelineMessage(sourceMessages) - : undefined; + const lastMessage = sourceMessages.at(-1); + const terminalMessage = + preserveLastTerminalMessage && + lastMessage !== undefined && + isTimelineTerminalMessage(lastMessage) + ? lastMessage + : undefined; if (!terminalMessage) { appendSummaryGroup(sourceMessages); return; @@ -229,7 +233,8 @@ function groupCompletedTurnSummaryMessages( flushExternalBoundariesBefore(message); if (isTimelineUngroupableMessage(message)) { flushGroupedMessages( - message.kind === "user" && message.initiator === "user", + (message.kind === "user" && message.initiator === "user") || + isChildLifecycleSystemSteerMessage(message), ); items.push({ kind: "ungrouped-message", diff --git a/packages/thread-view/src/timeline-message-helpers.ts b/packages/thread-view/src/timeline-message-helpers.ts index 8e3d2c989a..0696bf4a41 100644 --- a/packages/thread-view/src/timeline-message-helpers.ts +++ b/packages/thread-view/src/timeline-message-helpers.ts @@ -1,5 +1,26 @@ +import type { SystemMessageKind } from "@bb/domain"; import type { EventProjectionMessage } from "./event-projection-types.js"; +const CHILD_LIFECYCLE_SYSTEM_MESSAGE_KINDS = new Set([ + "child-needs-attention", + "child-completed", + "child-failed", + "child-interrupted", + "child-outcome-batch", +]); + +export function isChildLifecycleSystemSteerMessage( + message: EventProjectionMessage, +): boolean { + return ( + message.kind === "user" && + message.initiator === "system" && + message.turnRequest.kind === "steer" && + message.turnRequest.status === "accepted" && + CHILD_LIFECYCLE_SYSTEM_MESSAGE_KINDS.has(message.systemMessageKind) + ); +} + export function isTimelineTerminalMessage( message: EventProjectionMessage, ): boolean { @@ -12,7 +33,9 @@ export function isTimelineSummaryGroupableSteerMessage( return ( message.kind === "user" && message.turnRequest.kind === "steer" && - (message.initiator === "agent" || message.initiator === "system") + (message.initiator === "agent" || + (message.initiator === "system" && + !isChildLifecycleSystemSteerMessage(message))) ); } diff --git a/packages/thread-view/test/completed-turn-grouping.test.ts b/packages/thread-view/test/completed-turn-grouping.test.ts index aa24884073..fdf14e03d9 100644 --- a/packages/thread-view/test/completed-turn-grouping.test.ts +++ b/packages/thread-view/test/completed-turn-grouping.test.ts @@ -1,9 +1,11 @@ -import { turnScope } from "@bb/domain"; +import { turnScope, type SystemMessageKind } from "@bb/domain"; import { describe, expect, it } from "vitest"; import { groupCompletedTurnMessages } from "../src/completed-turn-grouping.js"; import type { CompletedTurnMessageGroups } from "../src/completed-turn-grouping.js"; import type { EventProjectionAssistantTextMessage, + EventProjectionCommandMessage, + EventProjectionErrorMessage, EventProjectionMessage, EventProjectionOperationMessage, EventProjectionTurnRequest, @@ -39,8 +41,36 @@ function assistantMessage( }; } +function commandMessage(args: MessageBaseArgs): EventProjectionCommandMessage { + return { + ...messageBase(args), + kind: "command", + callId: args.id, + command: "pnpm test", + cwd: "/repo", + parsedIntents: [], + source: null, + output: "", + exitCode: 0, + completedAt: args.seq, + approvalStatus: null, + status: "completed", + }; +} + +function errorMessage(args: MessageBaseArgs): EventProjectionErrorMessage { + return { + ...messageBase(args), + kind: "error", + message: args.id, + detail: null, + rawType: "provider/error", + }; +} + interface UserMessageArgs extends MessageBaseArgs { initiator?: EventProjectionUserMessage["initiator"]; + systemMessageKind?: SystemMessageKind; turnRequest?: EventProjectionTurnRequest; } @@ -50,7 +80,7 @@ function userMessage(args: UserMessageArgs): EventProjectionUserMessage { kind: "user", initiator: args.initiator ?? "user", senderThreadId: null, - systemMessageKind: "unlabeled", + systemMessageKind: args.systemMessageKind ?? "unlabeled", systemMessageSubject: null, turnRequest: args.turnRequest ?? { isGrouped: false, @@ -251,6 +281,138 @@ describe("groupCompletedTurnMessages", () => { ]); }); + it.each([ + "child-needs-attention", + "child-completed", + "child-failed", + "child-interrupted", + "child-outcome-batch", + ] satisfies SystemMessageKind[])( + "preserves the preceding terminal message at a %s system steer", + (systemMessageKind) => { + const assistantBefore = assistantMessage({ + id: "assistant-before", + seq: 1, + }); + const lifecycle = userMessage({ + id: "child-lifecycle", + initiator: "system", + seq: 2, + systemMessageKind, + turnRequest: { isGrouped: false, kind: "steer", status: "accepted" }, + }); + const assistantAfter = assistantMessage({ + id: "assistant-after", + seq: 3, + }); + const groups = groupCompletedTurnMessages( + completedTurn( + [assistantBefore, lifecycle, assistantAfter], + assistantAfter, + ), + ); + + expect(groups.summaryItems).toEqual([ + { kind: "ungrouped-message", message: assistantBefore }, + { kind: "ungrouped-message", message: lifecycle }, + ]); + expect(groups.terminalMessages).toEqual([assistantAfter]); + }, + ); + + it("does not move an assistant across work before a lifecycle boundary", () => { + const assistantBefore = assistantMessage({ + id: "assistant-before", + seq: 1, + }); + const command = commandMessage({ id: "command", seq: 2 }); + const lifecycle = userMessage({ + id: "child-lifecycle", + initiator: "system", + seq: 3, + systemMessageKind: "child-completed", + turnRequest: { isGrouped: false, kind: "steer", status: "accepted" }, + }); + const assistantAfter = assistantMessage({ + id: "assistant-after", + seq: 4, + }); + const groups = groupCompletedTurnMessages( + completedTurn( + [assistantBefore, command, lifecycle, assistantAfter], + assistantAfter, + ), + ); + + expect(groups.summaryItems).toEqual([ + expect.objectContaining({ + kind: "summary", + sourceMessages: [assistantBefore, command], + }), + { kind: "ungrouped-message", message: lifecycle }, + ]); + expect(groups.terminalMessages).toEqual([assistantAfter]); + }); + + it("preserves an error directly before a lifecycle boundary", () => { + const errorBefore = errorMessage({ id: "provider-error", seq: 1 }); + const lifecycle = userMessage({ + id: "child-lifecycle", + initiator: "system", + seq: 2, + systemMessageKind: "child-failed", + turnRequest: { isGrouped: false, kind: "steer", status: "accepted" }, + }); + const assistantAfter = assistantMessage({ + id: "assistant-after", + seq: 3, + }); + const groups = groupCompletedTurnMessages( + completedTurn([errorBefore, lifecycle, assistantAfter], assistantAfter), + ); + + expect(groups.summaryItems).toEqual([ + { kind: "ungrouped-message", message: errorBefore }, + { kind: "ungrouped-message", message: lifecycle }, + ]); + expect(groups.terminalMessages).toEqual([assistantAfter]); + }); + + it.each(["pending", "rejected"] as const)( + "keeps a %s lifecycle request folded", + (status) => { + const assistantBefore = assistantMessage({ + id: "assistant-before", + seq: 1, + }); + const lifecycle = userMessage({ + id: "child-lifecycle", + initiator: "system", + seq: 2, + systemMessageKind: "child-completed", + turnRequest: { isGrouped: false, kind: "steer", status }, + }); + const assistantAfter = assistantMessage({ + id: "assistant-after", + seq: 3, + }); + const groups = groupCompletedTurnMessages( + completedTurn( + [assistantBefore, lifecycle, assistantAfter], + assistantAfter, + ), + ); + + expect(groups.summaryItems).toEqual([ + expect.objectContaining({ + kind: "summary", + sourceMessages: [assistantBefore, lifecycle], + }), + ]); + expect(groups.terminalMessages).toEqual([assistantAfter]); + }, + ); + it("segments summary groups around converted legacy user messages", () => { const turn = completedTurn( [ diff --git a/packages/thread-view/test/completed-turn-summary-rendering.test.ts b/packages/thread-view/test/completed-turn-summary-rendering.test.ts index efadcb69ca..285f549d96 100644 --- a/packages/thread-view/test/completed-turn-summary-rendering.test.ts +++ b/packages/thread-view/test/completed-turn-summary-rendering.test.ts @@ -586,6 +586,227 @@ describe("completed turn summary rendering", () => { ]); }); + it("keeps cproxy parent responses visible across a child completion steer", () => { + const event = createTimelineEventFactory({ threadId: "thread-1" }); + const childTell = event.clientTurnRequested({ + initiator: "agent", + senderThreadId: "thr_child", + target: { kind: "new-turn" }, + text: "Child report: the implementation is ready.", + }); + const lifecycle = event.clientTurnRequested({ + initiator: "system", + senderThreadId: null, + systemMessageKind: "child-completed", + systemMessageSubject: { + kind: "thread", + threadId: "thr_child", + threadName: "Implementation worker", + }, + target: { kind: "auto", expectedTurnId: "turn-1" }, + text: "Implementation worker completed: the fix is ready.", + }); + + const timeline = renderCompletedTimeline({ + events: [ + childTell, + event.turnStarted(), + event.inputAccepted({ clientRequestId: childTell.data.requestId }), + event.assistantCompleted({ + itemId: "assistant-detailed", + text: "I reviewed the implementation in detail and confirmed the important invariants.", + }), + lifecycle, + event.inputAccepted({ clientRequestId: lifecycle.data.requestId }), + event.assistantCompleted({ + itemId: "assistant-final", + text: "Done.", + }), + event.turnCompleted(), + ], + }); + + const conversations = timeline.rows.filter( + (row): row is Extract => + row.kind === "conversation", + ); + expect(conversations.map((row) => row.text)).toEqual([ + "Child report: the implementation is ready.", + "I reviewed the implementation in detail and confirmed the important invariants.", + "Implementation worker completed: the fix is ready.", + "Done.", + ]); + expect(conversations.map((row) => row.id)).toEqual([ + expect.any(String), + expect.stringContaining("assistant-detailed"), + expect.any(String), + expect.stringContaining("assistant-final"), + ]); + }); + + it("keeps a Terminal-Bench streamed response visible across a child completion steer", () => { + const event = createTimelineEventFactory({ threadId: "thread-1" }); + const lifecycle = event.clientTurnRequested({ + initiator: "system", + senderThreadId: null, + systemMessageKind: "child-completed", + systemMessageSubject: { + kind: "thread", + threadId: "thr_terminal_bench", + threadName: "Terminal-Bench worker", + }, + target: { kind: "auto", expectedTurnId: "turn-1" }, + text: "Terminal-Bench worker completed: all checks passed.", + }); + + const timeline = renderCompletedTimeline({ + events: [ + event.turnStarted(), + event.assistantDelta({ + itemId: "assistant-before-lifecycle", + delta: "The full benchmark analysis remains intact.", + }), + lifecycle, + event.inputAccepted({ clientRequestId: lifecycle.data.requestId }), + event.assistantCompleted({ + itemId: "assistant-before-lifecycle", + text: "The full benchmark analysis remains intact.", + }), + event.commandCompleted({ + itemId: "tool-after-lifecycle", + command: "terminal-bench run --task regression", + }), + event.assistantCompleted({ + itemId: "assistant-final", + text: "Verified.", + }), + event.turnCompleted(), + ], + }); + + expect(rowSignatures(timeline.rows)).toEqual([ + "conversation:assistant", + "conversation:user", + "turn:6-6", + "conversation:assistant", + ]); + expect( + timeline.rows + .filter( + (row): row is Extract => + row.kind === "conversation", + ) + .map((row) => row.text), + ).toEqual([ + "The full benchmark analysis remains intact.", + "Terminal-Bench worker completed: all checks passed.", + "Verified.", + ]); + }); + + it("does not move an assistant across a command at a lifecycle boundary", () => { + const event = createTimelineEventFactory({ threadId: "thread-1" }); + const lifecycle = event.clientTurnRequested({ + initiator: "system", + senderThreadId: null, + systemMessageKind: "child-completed", + systemMessageSubject: { + kind: "thread", + threadId: "thr_child", + threadName: "Implementation worker", + }, + target: { kind: "auto", expectedTurnId: "turn-1" }, + text: "Implementation worker completed: source order preserved.", + }); + + const timeline = renderCompletedTimeline({ + events: [ + event.turnStarted(), + event.assistantCompleted({ + itemId: "assistant-before-command", + text: "This response precedes the validation command.", + }), + event.commandCompleted({ + itemId: "command-before-lifecycle", + command: "pnpm test", + }), + lifecycle, + event.inputAccepted({ clientRequestId: lifecycle.data.requestId }), + event.assistantCompleted({ + itemId: "assistant-final", + text: "Done.", + }), + event.turnCompleted(), + ], + }); + + expect(timeline.rows.map((row) => row.kind)).toEqual([ + "turn", + "conversation", + "conversation", + ]); + const turnRow = requireOnlyTurnRow(timeline.rows); + expect(rowSignatures(turnRow.children ?? [])).toEqual([ + "conversation:assistant", + "work:command", + ]); + expect( + timeline.rows + .filter( + (row): row is Extract => + row.kind === "conversation", + ) + .map((row) => row.text), + ).toEqual([ + "Implementation worker completed: source order preserved.", + "Done.", + ]); + }); + + it("keeps an unlabeled system steer folded into the completed-turn summary", () => { + const event = createTimelineEventFactory({ threadId: "thread-1" }); + const housekeeping = event.clientTurnRequested({ + initiator: "system", + senderThreadId: null, + systemMessageKind: "unlabeled", + systemMessageSubject: null, + target: { kind: "auto", expectedTurnId: "turn-1" }, + text: "[bb system] Continue after reconnect.", + }); + + const timeline = renderCompletedTimeline({ + events: [ + event.turnStarted(), + event.assistantCompleted({ + itemId: "assistant-before-housekeeping", + text: "The response before reconnect is housekeeping context.", + }), + housekeeping, + event.inputAccepted({ clientRequestId: housekeeping.data.requestId }), + event.commandCompleted({ + itemId: "tool-after-housekeeping", + command: "git status --short", + }), + event.assistantCompleted({ + itemId: "assistant-final", + text: "Done.", + }), + event.turnCompleted(), + ], + }); + + expect(rowSignatures(timeline.rows)).toEqual([ + "turn:1-7", + "conversation:assistant", + ]); + const turnRow = requireOnlyTurnRow(timeline.rows); + expect(rowSignatures(turnRow.children ?? [])).toEqual([ + "conversation:assistant", + "conversation:user", + "work:command", + ]); + }); + it("splits completed turn summaries around converted legacy user messages", () => { const event = createTimelineEventFactory({ threadId: "thread-1" }); diff --git a/packages/thread-view/test/timeline-test-harness.ts b/packages/thread-view/test/timeline-test-harness.ts index b06789126f..895b7e82bf 100644 --- a/packages/thread-view/test/timeline-test-harness.ts +++ b/packages/thread-view/test/timeline-test-harness.ts @@ -15,6 +15,8 @@ import type { ThreadEventRow, ThreadEventRowOfType, ThreadEventUserContent, + SystemMessageKind, + SystemMessageSubject, SystemThreadInterruptedReason, ThreadEventWarningCategory, ThreadTurnInitiator, @@ -88,6 +90,8 @@ type ClientTurnRequestedArgs = EventFactoryRowOptions & { requestMethod?: "thread/start" | "turn/start"; senderThreadId?: string | null; source?: "spawn" | "tell"; + systemMessageKind?: SystemMessageKind; + systemMessageSubject?: SystemMessageSubject | null; target?: TurnRequestTarget; text: string; }; @@ -591,6 +595,12 @@ export function createTimelineEventFactory( source: args.source ?? "tell", initiator, senderThreadId, + ...(args.systemMessageKind !== undefined + ? { systemMessageKind: args.systemMessageKind } + : {}), + ...(args.systemMessageSubject !== undefined + ? { systemMessageSubject: args.systemMessageSubject } + : {}), input: args.input ?? [ { type: "text", text: args.text, mentions: [] }, ], diff --git a/plans/fix-disappearing-manager-responses.md b/plans/fix-disappearing-manager-responses.md new file mode 100644 index 0000000000..a12368d67d --- /dev/null +++ b/plans/fix-disappearing-manager-responses.md @@ -0,0 +1,98 @@ +# Keep manager responses visible across child lifecycle steers + +## Goal + +Prevent a child lifecycle notification from removing a parent assistant response that the user already saw. Preserve each Pi assistant output as a separate canonical timeline item when one steered turn contains several assistant messages. + +## Scope + +### Completed-turn projection + +- Treat these accepted system-steer message kinds as visible exchange boundaries: + - `child-needs-attention` + - `child-completed` + - `child-failed` + - `child-interrupted` + - `child-outcome-batch` +- Keep the assistant or error message immediately before the boundary visible when the turn changes from active to completed. +- Keep the child lifecycle message visible in source order. +- Continue to fold `unlabeled` system steers and other housekeeping or reconnect messages. +- Do not change ordinary tool/activity collapsing or broaden this change to every system or agent steer. + +### Pi assistant item identity + +- Use Pi's assistant `message_start` boundary to close any open assistant stream before the new assistant message starts. +- Let the shared delta assembler mint a new item id for the new assistant stream. +- Keep `agent_end` as the terminal close for the last assistant message. +- Do not add provider-specific item ids or weaken resolved-delta pruning. + +This boundary rule must support two assistant messages in one steered turn with no tool call between them. The first completion must not prune the second message or replace the first message's text. + +## Implementation seams + +- `packages/thread-view/src/timeline-message-helpers.ts` + - Add a narrow helper for the five child lifecycle system-message kinds. + - Use the helper when deciding whether a system steer is groupable and whether a boundary preserves the preceding terminal message. +- `packages/thread-view/src/completed-turn-grouping.ts` + - Preserve the preceding terminal assistant/error at a child lifecycle boundary. +- `packages/agent-runtime/src/pi/delta-translation.ts` + - Parse assistant `message_start` events. + - Emit an assistant `message.close` without final text. The assembler will close an existing stream from its accumulated text and do nothing when no stream is open. +- Tests should use existing helpers and public routes rather than test-only production hooks. + +## Regression tests + +1. Pi translator regression + - Start one Pi turn. + - Stream a detailed assistant response. + - Emit a second assistant `message_start` without a tool boundary. + - Stream a short second response and finish the turn. + - Assert the first and second assistant events use different item ids and both complete with their own text. + +2. Thread-view cproxy regression + - Child agent tell starts the parent turn. + - Parent emits a detailed assistant response. + - Accepted `child-completed` system auto-steer enters the same turn. + - Parent emits a short final response and the turn completes. + - Assert the detailed response, lifecycle message, and short response remain top-level and in order. + +3. Thread-view Terminal-Bench regression + - Stream the first assistant item across the accepted child lifecycle request. + - Add a tool boundary and a later assistant item. + - Complete the turn. + - Assert the full first response remains top-level rather than moving under the completed-turn summary. + +4. Control regression + - Repeat the completed-turn shape with an accepted `unlabeled` system steer. + - Assert the housekeeping message remains folded under the completed-turn summary. + +5. Public timeline regression + - Persist provider-shaped cproxy and Terminal-Bench event sequences through the real database and timeline route. + - Assert the completed public timeline keeps the previously visible assistant response and uses distinct row ids. + - Where the test runs resolved-delta pruning, assert both assistant texts remain recoverable after pruning. + +## Validation + +Run with Turbo: + +```bash +pnpm exec turbo run test --filter=@bb/agent-runtime --force -- --run src/pi/delta-translation.test.ts +pnpm exec turbo run test --filter=@bb/thread-view --force -- --run test/completed-turn-grouping.test.ts test/completed-turn-summary-rendering.test.ts +pnpm exec turbo run test --filter=@bb/server --force -- --run +pnpm exec turbo run typecheck --filter=@bb/agent-runtime --filter=@bb/thread-view --filter=@bb/server +pnpm exec turbo run lint --filter=@bb/agent-runtime --filter=@bb/thread-view --filter=@bb/server +git diff --check +``` + +If Turbo does not forward focused Vitest arguments for a package, run the package's full Turbo test task instead of bypassing Turbo. + +## Acceptance checks + +- A child lifecycle auto-steer cannot remove a parent assistant response from the completed top-level timeline. +- All five child lifecycle kinds use the same boundary rule. +- `unlabeled` and reconnect/housekeeping system steers keep their current folded behavior. +- Two Pi assistant messages in one steered turn without a tool have different item and timeline row ids. +- Resolved-delta pruning cannot replace the first assistant text with the second assistant completion. +- Public timeline output agrees with thread-view unit projection. +- No running BB process is restarted or reloaded. +- No commit, push, issue, PR, merge, deployment, or release is created.