diff --git a/src/components/session/ChatPanel.tsx b/src/components/session/ChatPanel.tsx index 087b9cd2..335c7339 100644 --- a/src/components/session/ChatPanel.tsx +++ b/src/components/session/ChatPanel.tsx @@ -52,8 +52,10 @@ import { import { getReasoningTraceExpansionMode, getMessageScrollFingerprint, + resolvePlanMessagePresentation, shouldShowConversationLoadingState, } from "@/components/session/chat-panel.utils"; +import { ConversationPlanCard } from "@/components/session/ConversationPlanCard"; import { useScopedTaskId } from "@/components/session/task-scope-context"; import { getTurnModelInfoLabel } from "@/lib/providers/turn-model-info"; import { cn } from "@/lib/utils"; @@ -165,6 +167,8 @@ interface MessageRowProps { completedAt?: string; parts: MessagePart[]; displayParts?: MessagePart[]; + isPlanResponse?: boolean; + planText?: string; isStreaming?: boolean; steerDeliveryState?: ChatMessage["steerDeliveryState"]; providerBoundary?: ChatMessage["providerBoundary"]; @@ -217,6 +221,10 @@ const MessageRow = memo(function MessageRow(args: MessageRowProps) { () => getMessageElapsedLabel({ message, nowMs: elapsedAnchorMs }), [elapsedAnchorMs, message], ); + const planPresentation = useMemo( + () => resolvePlanMessagePresentation(message), + [message], + ); const userMessageSourceText = message.displayContent ?? message.content; const turnModelInfoLabel = getTurnModelInfoLabel(message); const steerDeliveryLabel = @@ -319,14 +327,19 @@ const MessageRow = memo(function MessageRow(args: MessageRowProps) { className={message.role === "assistant" ? "pb-1" : undefined} onCopy={handleUserMessageCopy} > - + {planPresentation.showPlanCard ? ( + + ) : null} + {planPresentation.showAssistantBody ? ( + + ) : null} {message.role === "user" && steerDeliveryLabel ? ( @@ -653,10 +666,12 @@ function ChatPanelMessageList(props: { const [turnCompletionScrollTick, setTurnCompletionScrollTick] = useState(0); const previousActiveTurnIdRef = useRef(activeTurnId); - const visibleMessages = useMemo( - () => messages.filter((message) => !message.isPlanResponse), - [messages], - ); + // Plan responses stay in the transcript and render as a dedicated plan card + // (see `resolvePlanMessagePresentation`). They used to be filtered out here, + // which left the floating `PlanViewer` as their only renderer — so the plan + // vanished as soon as the task moved past plan review, and any follow-up + // content sharing the message was dropped with it. + const visibleMessages = messages; const threadActionStateByMessageId = useMemo( () => buildConversationTurnActionStateByMessageId({ diff --git a/src/components/session/ConversationPlanCard.tsx b/src/components/session/ConversationPlanCard.tsx new file mode 100644 index 00000000..38ac484f --- /dev/null +++ b/src/components/session/ConversationPlanCard.tsx @@ -0,0 +1,26 @@ +import { ClipboardCheck } from "lucide-react"; +import { MessageResponse } from "@/components/ai-elements"; + +/** + * Renders a plan response inline in the conversation. + * + * The floating `PlanViewer` only stays open while the task is under plan + * review, so the transcript needs its own copy — otherwise an approved or + * revised plan becomes unreadable the moment the conversation moves on. + */ +export function ConversationPlanCard(props: { planText: string }) { + return ( +
+
+ +

Plan

+
+
+ {props.planText} +
+
+ ); +} diff --git a/src/components/session/chat-panel.utils.ts b/src/components/session/chat-panel.utils.ts index a58fc3d0..7d1a348c 100644 --- a/src/components/session/chat-panel.utils.ts +++ b/src/components/session/chat-panel.utils.ts @@ -1,5 +1,6 @@ import type { ChatMessage, CodeDiffPart, FileContextPart, ImageContextPart, MessagePart, ToolUsePart } from "@/types/chat"; import { detectTruncationNotice } from "@/lib/truncation-visibility"; +import { hasMeaningfulPlanText, normalizePlanText } from "@/lib/plan-text"; export function isPendingDiffStatus(status: CodeDiffPart["status"]) { return status === "pending"; @@ -560,3 +561,49 @@ export function getMessageBodyFallbackState(args: { return "content"; } + +export interface PlanMessagePresentation { + /** Normalized plan body to render in the transcript plan card. */ + planText: string; + showPlanCard: boolean; + /** Whether the regular assistant trace should render alongside the card. */ + showAssistantBody: boolean; +} + +/** + * Plan responses render as a dedicated card in the transcript so the reviewed + * plan stays readable after the floating plan viewer closes. + * + * Older records folded the rest of the turn (follow-up text, tool calls, + * pending approvals) into the same message. Those parts are still rendered + * below the card so nothing stays hidden; freshly captured plan messages carry + * no parts, so they render as the card alone. + */ +export function resolvePlanMessagePresentation( + message: Pick< + ChatMessage, + "isPlanResponse" | "planText" | "content" | "parts" | "displayParts" + >, +): PlanMessagePresentation { + if (message.isPlanResponse !== true) { + return { planText: "", showPlanCard: false, showAssistantBody: true }; + } + + const planText = normalizePlanText(message.planText ?? message.content ?? ""); + if (!hasMeaningfulPlanText(planText)) { + return { planText: "", showPlanCard: false, showAssistantBody: true }; + } + + // Deliberately scoped to `parts` — `getRenderableMessageParts` would fall + // back to `content`, which on a plan message is the plan text itself and + // would render underneath the card a second time. + const planParts = message.displayParts ?? message.parts; + const showAssistantBody = + planParts.length > 0 + && getMessageBodyFallbackState({ + isActivelyStreaming: false, + renderableParts: planParts, + }) === "content"; + + return { planText, showPlanCard: true, showAssistantBody }; +} diff --git a/src/lib/session/provider-event-replay.ts b/src/lib/session/provider-event-replay.ts index b2047766..d290f26d 100644 --- a/src/lib/session/provider-event-replay.ts +++ b/src/lib/session/provider-event-replay.ts @@ -386,11 +386,110 @@ function normalizeEventToPart(args: { } } +/** + * True when an event belongs to a message of its own because the current target + * is already a plan response. + * + * `plan_ready` is excluded on purpose: re-presenting an updated plan replaces + * the existing plan message rather than starting a new one. `provider_turn` + * only qualifies when it announces a *different* native turn — the plan's own + * turn still belongs to the plan row. + */ +function startsMessageAfterPlan(args: { + target: ChatMessage; + event: NormalizedProviderEvent; +}): boolean { + const { target, event } = args; + if (event.type === "plan_ready") { + return false; + } + if (event.type === "provider_turn") { + return ( + target.nativeProviderTurnId != null && + target.nativeProviderTurnId !== event.nativeTurnId + ); + } + return normalizeEventToPart({ event }) !== null; +} + +function providerBoundariesEqual( + left: ChatMessage["providerBoundary"], + right: ChatMessage["providerBoundary"], +): boolean { + return ( + left?.providerId === right?.providerId && + left?.kind === right?.kind && + left?.nativeId === right?.nativeId + ); +} + +/** + * Copy the native turn identity of `from` onto `message`. + * + * Splitting one provider turn across several rows must not strand a row without + * that identity: `buildConversationTurnActionStateByMessageId` disables + * fork/rollback on any assistant row missing `nativeProviderTurnId` ("this + * response predates native turn tracking"). + */ +function inheritNativeTurnIdentity(args: { + message: ChatMessage; + from: ChatMessage; +}): ChatMessage { + const { from } = args; + return { + ...args.message, + ...(from.nativeProviderSessionId + ? { nativeProviderSessionId: from.nativeProviderSessionId } + : {}), + ...(from.nativeProviderTurnId + ? { nativeProviderTurnId: from.nativeProviderTurnId } + : {}), + ...(from.providerBoundary + ? { providerBoundary: from.providerBoundary } + : {}), + }; +} + +/** + * Seal the trailing plan row and open the assistant message that carries the + * rest of the turn. The new row inherits the plan's native turn identity; a + * later `provider_turn`/`history_boundary` for a genuinely new turn overwrites + * it in place. + */ +function openMessageAfterPlan(args: { + messages: ChatMessage[]; + plan: ChatMessage; + taskId: string; + messageIndexOffset: number; + provider: ProviderId; + model: string; +}): { messages: ChatMessage[]; target: ChatMessage } { + const target = inheritNativeTurnIdentity({ + message: createStreamingAssistantMessage({ + taskId: args.taskId, + count: args.messages.length + args.messageIndexOffset, + provider: args.provider, + model: args.model, + ...(args.plan.modelInfo ? { modelInfo: args.plan.modelInfo } : {}), + }), + from: args.plan, + }); + return { + messages: [ + ...args.messages.slice(0, -1), + finalizeAssistantMessage({ message: args.plan }), + target, + ], + target, + }; +} + function createStreamingAssistantMessage(args: { taskId: string; count: number; provider: ProviderId; model: string; + modelInfo?: TurnModelInfo; }): ChatMessage { const startedAt = buildRecentTimestamp(); return { @@ -398,6 +497,7 @@ function createStreamingAssistantMessage(args: { role: "assistant", model: args.model, providerId: args.provider, + ...(args.modelInfo ? { modelInfo: args.modelInfo } : {}), content: "", startedAt, isStreaming: true, @@ -962,18 +1062,43 @@ export function replayProviderEventsToTaskState(args: { current = [...current, assistant]; targetIndex = current.length - 1; } - const boundaryTarget = current[targetIndex]; + let boundaryTarget = current[targetIndex]; if (boundaryTarget) { const nextBoundary = { providerId: event.providerId, kind: event.boundaryKind, nativeId: event.nativeId, } as const; + // A boundary for a different native turn cannot belong to a sealed plan + // row — it belongs to the response that follows the plan. Claude emits + // this ahead of `provider_turn`, so the split has to start here too. if ( - boundaryTarget.providerBoundary?.providerId !== - nextBoundary.providerId || - boundaryTarget.providerBoundary.kind !== nextBoundary.kind || - boundaryTarget.providerBoundary.nativeId !== nextBoundary.nativeId + boundaryTarget.isPlanResponse === true && + targetIndex === current.length - 1 && + boundaryTarget.providerBoundary != null && + !providerBoundariesEqual( + boundaryTarget.providerBoundary, + nextBoundary, + ) + ) { + const opened = openMessageAfterPlan({ + messages: current, + plan: boundaryTarget, + taskId: args.taskId, + messageIndexOffset, + provider: args.provider, + model: args.model, + }); + current = opened.messages; + targetIndex = current.length - 1; + boundaryTarget = opened.target; + changed = true; + } + if ( + !providerBoundariesEqual( + boundaryTarget.providerBoundary, + nextBoundary, + ) ) { current = current.map((message, index) => index === targetIndex @@ -1019,13 +1144,16 @@ export function replayProviderEventsToTaskState(args: { const finalizedTarget = finalizeAssistantMessage({ message: cleanedTarget, }); - const planMessage = createPlanAssistantMessage({ - taskId: args.taskId, - count: current.length + messageIndexOffset, - provider: args.provider, - model: args.model, - modelInfo: target.modelInfo, - planText: event.planText, + const planMessage = inheritNativeTurnIdentity({ + message: createPlanAssistantMessage({ + taskId: args.taskId, + count: current.length + messageIndexOffset, + provider: args.provider, + model: args.model, + modelInfo: target.modelInfo, + planText: event.planText, + }), + from: finalizedTarget, }); current = [...current.slice(0, -1), finalizedTarget, planMessage]; @@ -1039,6 +1167,28 @@ export function replayProviderEventsToTaskState(args: { current = [...current.slice(0, -1), cleanedTarget]; } + // A plan response renders as a dedicated plan card whose body is the plan + // text alone, so anything the agent produces afterwards has no place in it. + // Appending it here used to hide the rest of the turn — the "shall I + // proceed?" question, follow-up tool calls, even pending approvals — behind + // the card. Start a fresh assistant message instead. + if ( + target.isPlanResponse === true && + startsMessageAfterPlan({ target, event }) + ) { + const opened = openMessageAfterPlan({ + messages: current, + plan: target, + taskId: args.taskId, + messageIndexOffset, + provider: args.provider, + model: args.model, + }); + current = opened.messages; + target = opened.target; + changed = true; + } + const updated = appendProviderEventToAssistant({ message: target, event, diff --git a/tests/chat-panel-utils.test.ts b/tests/chat-panel-utils.test.ts index 65bbb685..f0ef4014 100644 --- a/tests/chat-panel-utils.test.ts +++ b/tests/chat-panel-utils.test.ts @@ -14,6 +14,7 @@ import { isCodeDiffSummarySystemEvent, parseFileChangeToolInput, isPendingDiffStatus, + resolvePlanMessagePresentation, isSubagentProgressSystemEvent, shouldRenderInlineToolPart, shouldRenderInlineSystemEvent, @@ -449,3 +450,58 @@ describe("tool visibility", () => { }); }); }); + +describe("resolvePlanMessagePresentation", () => { + test("renders a freshly captured plan as the card alone", () => { + expect(resolvePlanMessagePresentation({ + isPlanResponse: true, + planText: "## Plan\n- Inspect\n- Patch", + content: "## Plan\n- Inspect\n- Patch", + parts: [], + })).toEqual({ + planText: "## Plan\n- Inspect\n- Patch", + showPlanCard: true, + showAssistantBody: false, + }); + }); + + test("keeps legacy post-plan parts visible below the card", () => { + // Older records folded the rest of the turn into the plan message; the + // whole message used to be dropped from the transcript. + expect(resolvePlanMessagePresentation({ + isPlanResponse: true, + planText: "## Plan\n- Inspect", + content: "## Plan\n- Inspect", + parts: [{ type: "text", text: "Shall I proceed with the plan above?" }], + })).toEqual({ + planText: "## Plan\n- Inspect", + showPlanCard: true, + showAssistantBody: true, + }); + }); + + test("leaves non-plan messages untouched", () => { + expect(resolvePlanMessagePresentation({ + isPlanResponse: false, + content: "Regular answer.", + parts: [], + })).toEqual({ + planText: "", + showPlanCard: false, + showAssistantBody: true, + }); + }); + + test("falls back to the regular body when the plan text is empty", () => { + expect(resolvePlanMessagePresentation({ + isPlanResponse: true, + planText: " ", + content: " ", + parts: [{ type: "text", text: "Something else." }], + })).toEqual({ + planText: "", + showPlanCard: false, + showAssistantBody: true, + }); + }); +}); diff --git a/tests/provider-event-replay.test.ts b/tests/provider-event-replay.test.ts index adfded21..0a7f682a 100644 --- a/tests/provider-event-replay.test.ts +++ b/tests/provider-event-replay.test.ts @@ -814,6 +814,287 @@ describe("plan response replay", () => { }); }); + test("moves post-plan assistant text into its own message", () => { + // Regression: a plan message renders as a dedicated plan card, so anything + // appended to it after the plan (the agent's "shall I proceed?" question, + // follow-up tool work) used to be swallowed with the card and never + // reached the transcript. + const replayed = replayProviderEventsToTaskState({ + taskId: "task-1", + messages: [], + events: [ + { type: "plan_ready", planText: "1. Inspect\n2. Patch" }, + { type: "text", text: "Shall I proceed with the plan above?" }, + { type: "done" }, + ], + provider: "claude-code", + model: "claude-sonnet-4-6", + }); + + expect(replayed.messages).toHaveLength(2); + expect(replayed.messages[0]).toMatchObject({ + content: "1. Inspect\n2. Patch", + isPlanResponse: true, + planText: "1. Inspect\n2. Patch", + isStreaming: false, + }); + expect(replayed.messages[0]?.parts).toEqual([]); + expect(typeof replayed.messages[0]?.completedAt).toBe("string"); + expect(replayed.messages[1]?.isPlanResponse).not.toBe(true); + expect(replayed.messages[1]).toMatchObject({ + content: "Shall I proceed with the plan above?", + isStreaming: false, + }); + }); + + test("keeps post-plan tool work out of the plan message", () => { + const replayed = replayProviderEventsToTaskState({ + taskId: "task-1", + messages: [], + events: [ + { type: "plan_ready", planText: "1. Inspect\n2. Patch" }, + { + type: "tool", + toolUseId: "write-1", + toolName: "Write", + input: '{"file_path":"a.ts"}', + state: "input-available", + }, + { type: "done" }, + ], + provider: "claude-code", + model: "claude-sonnet-4-6", + }); + + expect(replayed.messages).toHaveLength(2); + expect(replayed.messages[0]?.parts).toEqual([]); + expect(replayed.messages[1]?.isPlanResponse).not.toBe(true); + expect( + replayed.messages[1]?.parts.some((part) => part.type === "tool_use"), + ).toBe(true); + }); + + test("keeps the plan row on its own native turn when a follow-up turn starts", () => { + // Regression: `provider_turn` for the post-approval turn used to land on the + // sealed plan row, so the plan row advertised the follow-up turn while the + // follow-up row carried no native turn at all — which disables its + // fork/rollback actions ("predates native turn tracking"). + const replayed = replayProviderEventsToTaskState({ + taskId: "task-1", + messages: [], + events: [ + { + type: "provider_turn", + providerId: "claude-code", + nativeSessionId: "sess-1", + nativeTurnId: "turn-a", + }, + { type: "plan_ready", planText: "1. Inspect\n2. Patch" }, + { + type: "provider_turn", + providerId: "claude-code", + nativeSessionId: "sess-1", + nativeTurnId: "turn-b", + }, + { type: "text", text: "Plan approved. Implementing…" }, + { type: "done" }, + ], + provider: "claude-code", + model: "claude-sonnet-4-6", + }); + + expect(replayed.messages).toHaveLength(2); + expect(replayed.messages[0]).toMatchObject({ + isPlanResponse: true, + nativeProviderSessionId: "sess-1", + nativeProviderTurnId: "turn-a", + }); + expect(replayed.messages[1]).toMatchObject({ + content: "Plan approved. Implementing…", + nativeProviderSessionId: "sess-1", + nativeProviderTurnId: "turn-b", + }); + }); + + test("routes a follow-up history boundary to the follow-up response", () => { + // Claude emits `history_boundary` ahead of `provider_turn`; Codex emits it + // after. Either order must leave the plan row on its own boundary. + const claudeOrder = replayProviderEventsToTaskState({ + taskId: "task-1", + messages: [], + events: [ + { + type: "history_boundary", + providerId: "claude-code", + boundaryKind: "message", + nativeId: "turn-a", + targetRole: "assistant", + }, + { + type: "provider_turn", + providerId: "claude-code", + nativeSessionId: "sess-1", + nativeTurnId: "turn-a", + }, + { type: "plan_ready", planText: "1. Inspect\n2. Patch" }, + { + type: "history_boundary", + providerId: "claude-code", + boundaryKind: "message", + nativeId: "turn-b", + targetRole: "assistant", + }, + { + type: "provider_turn", + providerId: "claude-code", + nativeSessionId: "sess-1", + nativeTurnId: "turn-b", + }, + { type: "text", text: "Plan approved. Implementing…" }, + { type: "done" }, + ], + provider: "claude-code", + model: "claude-sonnet-4-6", + }); + + expect(claudeOrder.messages).toHaveLength(2); + expect(claudeOrder.messages[0]?.providerBoundary).toMatchObject({ + nativeId: "turn-a", + }); + expect(claudeOrder.messages[0]?.nativeProviderTurnId).toBe("turn-a"); + expect(claudeOrder.messages[1]?.providerBoundary).toMatchObject({ + nativeId: "turn-b", + }); + expect(claudeOrder.messages[1]?.nativeProviderTurnId).toBe("turn-b"); + + const codexOrder = replayProviderEventsToTaskState({ + taskId: "task-2", + messages: [], + events: [ + { + type: "provider_turn", + providerId: "codex", + nativeSessionId: "thread-1", + nativeTurnId: "turn-1", + }, + { + type: "history_boundary", + providerId: "codex", + boundaryKind: "turn", + nativeId: "turn-1", + targetRole: "assistant", + }, + { type: "plan_ready", planText: "1. Inspect\n2. Patch" }, + { + type: "provider_turn", + providerId: "codex", + nativeSessionId: "thread-1", + nativeTurnId: "turn-2", + }, + { + type: "history_boundary", + providerId: "codex", + boundaryKind: "turn", + nativeId: "turn-2", + targetRole: "assistant", + }, + { type: "text", text: "Plan approved. Implementing…" }, + { type: "done" }, + ], + provider: "codex", + model: "gpt-5.4", + }); + + expect(codexOrder.messages).toHaveLength(2); + expect(codexOrder.messages[0]?.providerBoundary).toMatchObject({ + nativeId: "turn-1", + }); + expect(codexOrder.messages[0]?.nativeProviderTurnId).toBe("turn-1"); + expect(codexOrder.messages[1]?.providerBoundary).toMatchObject({ + nativeId: "turn-2", + }); + expect(codexOrder.messages[1]?.nativeProviderTurnId).toBe("turn-2"); + }); + + test("carries the plan row's native turn onto same-turn follow-up content", () => { + // No new `provider_turn` arrived, so the text belongs to the very turn that + // produced the plan. The split row must inherit that turn instead of + // reporting itself as untracked. + const replayed = replayProviderEventsToTaskState({ + taskId: "task-1", + messages: [], + events: [ + { + type: "provider_turn", + providerId: "claude-code", + nativeSessionId: "sess-1", + nativeTurnId: "turn-a", + }, + { type: "plan_ready", planText: "1. Inspect\n2. Patch" }, + { type: "text", text: "Shall I proceed?" }, + { type: "done" }, + ], + provider: "claude-code", + model: "claude-sonnet-4-6", + }); + + expect(replayed.messages).toHaveLength(2); + expect(replayed.messages[0]?.nativeProviderTurnId).toBe("turn-a"); + expect(replayed.messages[1]).toMatchObject({ + content: "Shall I proceed?", + nativeProviderSessionId: "sess-1", + nativeProviderTurnId: "turn-a", + }); + }); + + test("carries the native turn onto a plan split off from streamed commentary", () => { + const replayed = replayProviderEventsToTaskState({ + taskId: "task-1", + messages: [], + events: [ + { + type: "provider_turn", + providerId: "codex", + nativeSessionId: "thread-1", + nativeTurnId: "turn-1", + }, + { type: "text", text: "Analyzing the codebase.\n\n" }, + { type: "plan_ready", planText: "## Plan\n- Step 1" }, + { type: "done" }, + ], + provider: "codex", + model: "gpt-5.4", + }); + + expect(replayed.messages).toHaveLength(2); + expect(replayed.messages[0]?.nativeProviderTurnId).toBe("turn-1"); + expect(replayed.messages[1]).toMatchObject({ + isPlanResponse: true, + nativeProviderSessionId: "thread-1", + nativeProviderTurnId: "turn-1", + }); + }); + + test("updates a re-presented plan in place instead of forking a message", () => { + const replayed = replayProviderEventsToTaskState({ + taskId: "task-1", + messages: [], + events: [ + { type: "plan_ready", planText: "1. Inspect" }, + { type: "plan_ready", planText: "1. Inspect\n2. Patch" }, + { type: "done" }, + ], + provider: "claude-code", + model: "claude-sonnet-4-6", + }); + + expect(replayed.messages).toHaveLength(1); + expect(replayed.messages[0]).toMatchObject({ + isPlanResponse: true, + planText: "1. Inspect\n2. Patch", + }); + }); + test("normalizes commentary out of plan_ready content", () => { const replayed = replayProviderEventsToTaskState({ taskId: "task-1",