diff --git a/desktop/src/features/agents/ui/AgentSessionToolItem/ToolItem.tsx b/desktop/src/features/agents/ui/AgentSessionToolItem/ToolItem.tsx index e0f2ce9fbc3..85a484deb52 100644 --- a/desktop/src/features/agents/ui/AgentSessionToolItem/ToolItem.tsx +++ b/desktop/src/features/agents/ui/AgentSessionToolItem/ToolItem.tsx @@ -10,6 +10,7 @@ import type { TranscriptItem } from "../agentSessionTypes"; import { getBuzzToolInfo } from "../agentSessionToolCatalog"; import { buildCompactToolSummary } from "../agentSessionToolSummary"; import type { AgentTranscriptIdentityProps } from "../activityRenderClasses/types"; +import { useIsInsideWorkBlockRail } from "../agentSessionTranscriptContext"; import { formatTranscriptTimestampTitle, getToolDurationDisplay, @@ -34,6 +35,7 @@ export function ToolItem({ profiles?: UserProfileLookup; }) { const [isExpanded, setIsExpanded] = React.useState(false); + const insideWorkBlockRail = useIsInsideWorkBlockRail(); const hasArgs = Object.keys(item.args).length > 0; const hasResult = item.result.trim().length > 0; const canonicalToolName = item.buzzToolName ?? item.toolName; @@ -57,7 +59,10 @@ export function ToolItem({ [], ); - if (compactSummary.presentation === "message") { + // On a work block's rail a posted message is a step the agent took, so it + // takes the same muted, expandable row as every other step. Everywhere else + // it keeps the bubble: see `useIsInsideWorkBlockRail`. + if (compactSummary.presentation === "message" && !insideWorkBlockRail) { return (
{ @@ -147,160 +147,6 @@ test("conversation renders agent messages as unboxed prose at full fidelity", as assert.ok(message.querySelector("code"), "markdown/code fidelity preserved"); }); -test("conversation collapses a finished thought into a Thought for Ns disclosure", async () => { - const { container } = await renderTranscript("conversation"); - const disclosure = container.querySelector( - '[data-testid="transcript-thought-item"]', - ); - assert.ok(disclosure, "thought should render"); - assert.equal(disclosure.tagName, "DETAILS"); - assert.equal(disclosure.open, false, "finished thoughts start collapsed"); - // thought at :02, next turn item (plan) at :07 → 5s. - assert.equal( - disclosure - .querySelector('[data-testid="transcript-thought-disclosure"]') - .textContent.trim(), - "Thought for 5s", - ); -}); - -test("conversation auto-opens the thought disclosure while it is streaming", async () => { - // A live turn on this channel makes the trailing item the streaming one, which - // is exactly the condition the disclosure auto-opens for. - syncAgentTurnsFromEvents(AGENT.agentPubkey, [ - { - seq: 1, - timestamp: "2026-06-14T19:00:02.000Z", - kind: "turn_started", - agentIndex: 0, - channelId: "chan-1", - sessionId: "sess-1", - turnId: "turn-1", - payload: null, - }, - ]); - const streamingThought = items().slice(0, 2); - const { container } = await renderTranscript("conversation", { - channelId: "chan-1", - items: streamingThought, - }); - - const disclosure = container.querySelector( - '[data-testid="transcript-thought-item"]', - ); - assert.equal(disclosure.open, true, "streaming thought should be open"); - const summary = disclosure.querySelector( - '[data-testid="transcript-thought-disclosure"]', - ); - // Shimmer paints a visual-only aria-hidden duplicate of the label, so match a - // prefix rather than the whole text node. - assert.match(summary.textContent, /^Thinking…/); - assert.doesNotMatch(summary.textContent, /Thought for/); -}); - -test("conversation folds the thought when the turn moves on, even after the browser echoes the auto-open toggle", async () => { - // Regression guard for the programmatic-toggle echo trap: a real browser fires - // `toggle` when React flips `open`, so the auto-open for a streaming thought - // arrives back at the component as if the reader had clicked. JSDOM does not - // fire that event itself, so the test injects it. If the handler records it as - // a reader choice, the disclosure stays pinned open forever and reasoning - // never recedes once the agent acts. - syncAgentTurnsFromEvents(AGENT.agentPubkey, [ - { - seq: 1, - timestamp: "2026-06-14T19:00:02.000Z", - kind: "turn_started", - agentIndex: 0, - channelId: "chan-1", - sessionId: "sess-1", - turnId: "turn-1", - payload: null, - }, - ]); - const streaming = { channelId: "chan-1", items: items().slice(0, 2) }; - const { container, setOverrides } = await renderRerenderableTranscript( - "conversation", - streaming, - ); - - const disclosure = container.querySelector( - '[data-testid="transcript-thought-item"]', - ); - assert.equal(disclosure.open, true, "streaming thought should auto-open"); - - // The browser echo: `open` already agrees with what React rendered, and the - // event follows rather than causes that state. - await act(async () => { - disclosure.open = true; - disclosure.dispatchEvent(new domWindow.Event("toggle")); - }); - assert.equal( - disclosure.open, - true, - "the echo must not disturb the open tail", - ); - - // The turn produces its next item, so the thought is no longer the streaming - // tail and should recede. - await setOverrides({ ...streaming, items: items().slice(0, 3) }); - - const settled = container.querySelector( - '[data-testid="transcript-thought-item"]', - ); - assert.equal( - settled.open, - false, - "a thought the agent has acted on should fold again", - ); - assert.match( - settled - .querySelector('[data-testid="transcript-thought-disclosure"]') - .textContent.trim(), - /^Thought for 5s/, - ); -}); - -test("conversation keeps a reader-opened thought open after the turn moves on", async () => { - // The other half of the guard: a toggle that DISAGREES with the rendered state - // is a genuine reader choice and must win over the stream transition. - syncAgentTurnsFromEvents(AGENT.agentPubkey, [ - { - seq: 1, - timestamp: "2026-06-14T19:00:02.000Z", - kind: "turn_started", - agentIndex: 0, - channelId: "chan-1", - sessionId: "sess-1", - turnId: "turn-1", - payload: null, - }, - ]); - // Three items so the thought is not the streaming tail: it renders collapsed. - const settledItems = { channelId: "chan-1", items: items().slice(0, 3) }; - const { container, setOverrides } = await renderRerenderableTranscript( - "conversation", - settledItems, - ); - - const disclosure = container.querySelector( - '[data-testid="transcript-thought-item"]', - ); - assert.equal(disclosure.open, false, "a settled thought starts collapsed"); - - await act(async () => { - disclosure.open = true; - disclosure.dispatchEvent(new domWindow.Event("toggle")); - }); - - await setOverrides({ ...settledItems, items: items() }); - - assert.equal( - container.querySelector('[data-testid="transcript-thought-item"]').open, - true, - "the reader's choice should survive later transcript items", - ); -}); - test("conversation renders the plan as a checklist card with in-place progress", async () => { const { container } = await renderTranscript("conversation"); const card = container.querySelector('[data-testid="transcript-plan-item"]'); @@ -519,3 +365,142 @@ test("the byte-for-byte baseline actually exercises every renderable item kind", assert.match(lifecycleText, /Context compacted/); assert.match(lifecycleText, /Turn failed/); }); + +/** + * A reader's expansion survives the work blocks being regrouped underneath it. + * + * Work blocks are derived, not stored: `groupConversationWorkBlocks` rebuilds + * them every render and ids each one after its first step + * (`work-block:${items[0].id}`). `findFinalAnswerId` exempts only the LAST + * assistant message from the block, so a second assistant message demotes the + * first — the earlier answer becomes work, and the runs on either side of it + * merge into one block: + * + * frame 2 work-block:thought:1[...] msg:1 work-block:thought:2[...] + * frame 3 work-block:thought:1[thought:1,tool:1,msg:1,thought:2,tool:2] msg:2 + * + * `work-block:thought:2` stops existing, so React unmounts it. With the fold + * state held in that component, a reader who had opened the second block to read + * its steps was folded shut by the agent posting a second message — an event + * they did not cause and cannot predict. This drives the real list so the actual + * regrouping runs, rather than asserting against hand-built segments. + */ +test("conversation keeps a reader's expanded work block open when a later answer regroups it", async () => { + const shared = { channelId: "chan-1", sessionId: "sess-1", turnId: "turn-1" }; + const at = (seconds) => + `2026-06-14T19:00:${String(seconds).padStart(2, "0")}.000Z`; + const thought = (id, seconds) => ({ + ...shared, + id, + type: "thought", + renderClass: "thought", + title: "Thinking", + text: `reasoning ${id}`, + timestamp: at(seconds), + }); + const tool = (id, seconds) => ({ + ...shared, + id, + type: "tool", + renderClass: "shell", + title: id, + toolName: "shell", + buzzToolName: null, + status: "completed", + args: {}, + result: "ok", + isError: false, + timestamp: at(seconds), + startedAt: at(seconds), + completedAt: at(seconds), + descriptor: { + renderClass: "shell", + label: "Ran command", + preview: id, + source: "shell", + groupKey: "shell:command", + }, + }); + const answer = (id, seconds) => ({ + ...shared, + id, + type: "message", + renderClass: "message", + role: "assistant", + title: "Test Agent", + text: `answer ${id}`, + timestamp: at(seconds), + }); + + const firstRun = [thought("thought:1", 1), tool("tool:1", 2)]; + const frame1 = [...firstRun, answer("msg:1", 3)]; + const frame2 = [...frame1, thought("thought:2", 4), tool("tool:2", 5)]; + const frame3 = [...frame2, answer("msg:2", 6)]; + + const { container, setOverrides } = await renderRerenderableTranscript( + "conversation", + { items: frame1 }, + ); + const blocks = () => [ + ...container.querySelectorAll('[data-testid="transcript-work-block"]'), + ]; + const summaries = () => [ + ...container.querySelectorAll( + '[data-testid="transcript-work-block-summary"]', + ), + ]; + const openCount = () => + summaries().filter((node) => node.getAttribute("aria-expanded") === "true") + .length; + + await setOverrides({ items: frame2 }); + assert.equal( + blocks().length, + 2, + "the demoted-answer frame should show two separate work blocks", + ); + assert.equal(openCount(), 0, "both blocks start folded once work finished"); + + // The reader opens the SECOND block — the one the merge destroys. + await act(async () => { + summaries()[1].click(); + }); + assert.equal(openCount(), 1, "the reader's click should open that block"); + + await setOverrides({ items: frame3 }); + assert.equal( + blocks().length, + 1, + "the second answer should merge the runs into one block", + ); + assert.equal( + openCount(), + 1, + "the merged block must stay open — the reader asked to see those steps, and an agent posting again is not a reason to fold them away", + ); + // The steps they were reading are actually on screen, not merely a block + // reporting itself open. + const stepText = [ + ...container.querySelectorAll('[data-testid="transcript-work-block-step"]'), + ] + .map((node) => node.textContent) + .join("\n"); + assert.match( + stepText, + /tool:2/, + "the reader's steps should still be visible", + ); + + // ...and the merged block is still theirs to fold. Recording the choice + // against only the block's first step would leave the absorbed block's stale + // `open` entry behind, and since an open choice wins the read, the merged + // block could never be folded again — the reader's click would do nothing. + await act(async () => { + summaries()[0].click(); + }); + assert.equal( + openCount(), + 0, + "a reader who folds the merged block must actually fold it", + ); +}); diff --git a/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversationHarness.mjs b/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversationHarness.mjs index 6dda85385d8..11c6c89bf56 100644 --- a/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversationHarness.mjs +++ b/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversationHarness.mjs @@ -191,7 +191,7 @@ export const AUTHOR_TRUNCATED = `${AUTHOR.slice(0, 8)}…${AUTHOR.slice(-4)}`; * (`agentSessionTranscriptHelpers.ts` `parsePromptText`). The author row must * never display this as a name. */ -export const TRIGGER_TITLE = "@Mention"; +const TRIGGER_TITLE = "@Mention"; export const AUTHOR_PROFILES = { [AUTHOR]: { displayName: "Ada Lovelace", @@ -569,8 +569,3 @@ afterEach(() => { resetActiveAgentTurnsStore?.(); }); after(() => dom.window.close()); -/** - * The jsdom window itself. Exported for the few tests that must construct a - * real DOM event (`new domWindow.Event("toggle")`) to simulate a browser echo. - */ -export const domWindow = dom.window; diff --git a/desktop/src/features/agents/ui/AgentSessionTranscriptList.tsx b/desktop/src/features/agents/ui/AgentSessionTranscriptList.tsx index 9a5135c6f0c..11c4246479e 100644 --- a/desktop/src/features/agents/ui/AgentSessionTranscriptList.tsx +++ b/desktop/src/features/agents/ui/AgentSessionTranscriptList.tsx @@ -34,6 +34,12 @@ import { getTranscriptMessageLink, } from "./AgentSessionTranscriptChrome"; import { buildConversationTurnMeta } from "./agentSessionConversationMeta"; +import { AgentSessionWorkBlockSegment } from "./AgentSessionWorkBlock"; +import { AgentSessionWorkBlockDisclosureProvider } from "./agentSessionWorkBlockDisclosure"; +import { + conversationSegmentsForBlock, + type TranscriptConversationSegment, +} from "./agentSessionWorkBlockGrouping"; import { useTranscriptAnimationEnabled } from "./transcriptAnimationPreference"; import { useTranscriptTimestampsEnabled } from "./transcriptTimestampPreference"; import { TranscriptActivityItem } from "./activityRenderClasses/TranscriptActivityItem"; @@ -171,8 +177,9 @@ export function AgentSessionTranscriptList({ // Returns a shared constant for the other variants, so their render output is // untouched. const turnMeta = React.useMemo( - () => buildConversationTurnMeta(displayBlocks, { isTurnLive, variant }), - [displayBlocks, isTurnLive, variant], + () => + buildConversationTurnMeta(displayBlocks, { isTurnLive, items, variant }), + [displayBlocks, isTurnLive, items, variant], ); const scrollContainerClassNames = cn( @@ -240,40 +247,42 @@ export function AgentSessionTranscriptList({ > - {displayBlocks.map((block) => { - const blockKey = getDisplayBlockKey(block); - return ( - - {/* content-visibility stays on a non-animated child: motion + + {displayBlocks.map((block) => { + const blockKey = getDisplayBlockKey(block); + return ( + + {/* content-visibility stays on a non-animated child: motion measures the outer wrapper for layout animations, which would otherwise force skipped offscreen rows to render. */} -
- -
-
- ); - })} - {isTurnLive && !isCompactPreview ? ( - - ) : null} +
+ +
+
+ ); + })} + {isTurnLive && !isCompactPreview ? ( + + ) : null} +
@@ -378,6 +387,15 @@ function TranscriptDisplayBlockView({ ); } + // Variant in, presentation out: the conversation variant folds a turn's + // thinking/tools/notes into work blocks, and every other variant renders the + // shared segments untouched. Keeping the choice here — rather than inside the + // segment components — is what keeps `default`/`compactPreview` byte-identical + // to their pre-work-block markup. + const segments: TranscriptConversationSegment[] = isConversation + ? conversationSegmentsForBlock(block) + : block.segments; + return (
- {block.segments.map((segment) => ( + {segments.map((segment) => ( + ); + } + if (segment.kind === "prompt") { return ( { + const view = await renderBlock( + [step("a"), step("b", { status: "executing", completedAt: null })], + { liveTurnId: null, streamingItemId: null }, + ); + + const summary = view.summary(); + assert.ok( + summary, + "history gets its folded summary line — the orphaned status must not suppress it", + ); + assert.match( + summary.textContent, + /2 steps$/, + "an abandoned step is not known to have failed, so the count stays neutral", + ); + assert.equal(await view.settleToStepCount(0), 0, "the rail folds away"); + // Then OPEN it and look. Asserting "nothing pulses" on the folded block would + // be vacuous: the fold unmounts every row, so no glyph exists to carry a + // pulse class and the assertion would hold against a build where abandoned + // steps pulse forever — the exact Codex finding. The reader's complaint is + // about what they see when they scroll back and expand, so that is where the + // assertion belongs. + await view.expand(); + assert.equal(view.stepCount(), 2, "the expanded rail has rows to inspect"); + assert.deepEqual( + view.glyphStates(), + ["settled", "settled"], + "the abandoned step reads as settled, not running", + ); + assert.deepEqual( + view.pulseStates(), + [], + "nothing pulses when nothing is running", + ); +}); + +test("the same items still hold the block open while the turn is live", async () => { + // The paired half: this is what makes the gate meaningful rather than a + // blanket "never trust executing". Identical items, live turn. + const view = await renderBlock( + [step("a"), step("b", { status: "executing", completedAt: null })], + { liveTurnId: "turn-1", streamingItemId: null }, + ); + + assert.equal(view.summary(), null, "a live block has no folded line"); + assert.equal(view.stepCount(), 2, "the rail stays open"); + assert.deepEqual(view.glyphStates(), ["settled", "running"]); + assert.deepEqual(view.pulseStates(), ["running"], "live work pulses"); +}); + +test("an agent live on a later turn does not resurrect an earlier turn's abandoned step", async () => { + // The reason the signal is a turn id and not a boolean: a restarted agent is + // live, but not on this turn, and a global flag would keep this step spinning. + const view = await renderBlock( + [step("a"), step("b", { status: "executing", completedAt: null })], + { liveTurnId: "turn-2", streamingItemId: null }, + ); + + assert.ok( + view.summary(), + "this turn is history even though the agent is busy", + ); + assert.equal(await view.settleToStepCount(0), 0); + // Expanded, for the same reason as above: a folded rail has no glyph to pulse, + // so the negative has to be taken on rows that exist. + await view.expand(); + assert.deepEqual(view.glyphStates(), ["settled", "settled"]); + assert.deepEqual(view.pulseStates(), []); +}); + +test("a block live when the session ends folds instead of spinning forever", async () => { + const { act } = await import("@testing-library/react"); + // The bug as the reader meets it live: the agent dies mid-step, so the item + // never reaches a terminal status and the only thing that changes is that no + // turn is live any more. + const live = [ + step("a"), + step("b", { status: "executing", completedAt: null }), + ]; + const view = await renderBlock(live, { + liveTurnId: "turn-1", + streamingItemId: "b", + }); + assert.equal(view.stepCount(), 2, "live: the rail is open"); + assert.equal(view.summary(), null); + + // Session gone. The items are BYTE-IDENTICAL — only liveness changed. + await act(async () => { + view.stream(live, null, null); + }); + + assert.ok(view.summary(), "the block settles when its session goes away"); + assert.match(view.summary().textContent, /2 steps$/); + // Taken HERE, before the fold finishes: the rail is still mounted for the + // settle frame, so the glyph that was pulsing a moment ago still exists and + // can be asked whether it stopped. Once `settleToStepCount(0)` has run there + // are no rows left and the same assertion proves nothing. + assert.equal(view.stepCount(), 2, "the rail is still mounted to inspect"); + assert.deepEqual(view.glyphStates(), ["settled", "settled"]); + assert.deepEqual( + view.pulseStates(), + [], + "the pulse stops the moment the session goes away, not when the fold finishes", + ); + assert.equal(await view.settleToStepCount(0), 0); +}); + +// ── The gap between two turns ───────────────────────────────────────────────── + +/** + * A finished block must stay folded through the gap before the next turn shows + * anything. + * + * The rendered half of the `buildConversationTurnMeta` gap contract (see + * `agentSessionConversationMeta.test.mjs`). The meta test proves the hints are + * right; this proves the reader sees the consequence, because the symptom was + * never a wrong id — it was a settled 6-step block re-opening, dropping to its + * last three steps behind a "previous steps" disclosure, and then folding back, + * on every single turn. + * + * Six steps rather than two on purpose: the live window only applies above + * three, so a smaller block would hide the loudest part of the regression. + */ +test("a finished block stays folded while the next turn has started but shown nothing", async () => { + const { act } = await import("@testing-library/react"); + const items = ["a", "b", "c", "d", "e", "f"].map((id) => step(id)); + + // Finished: nothing live, so the block is folded to its summary line. + const view = await renderBlock(items, { + liveTurnId: null, + streamingItemId: null, + }); + assert.match(view.summary().textContent, /6 steps$/); + assert.equal(await view.settleToStepCount(0), 0, "it starts folded"); + + // The next turn starts. It owns liveness (turn-2) and, having emitted nothing + // renderable, contributes no streaming item — which is exactly what the fixed + // `latestTurnId`/`streamingIdForTail` pair reports for this frame. + await act(async () => { + view.stream(items, null, "turn-2"); + }); + + assert.ok( + view.summary(), + "the folded summary line survives the next turn starting", + ); + assert.match(view.summary().textContent, /6 steps$/); + assert.equal(view.stepCount(), 0, "the rail does not re-open"); + assert.equal( + view.previousSteps(), + null, + "and the live window's previous-steps disclosure never appears", + ); + assert.deepEqual(view.pulseStates(), []); +}); + +test("an orphaned step keeps its own row detail rather than gaining an interrupted marker", async () => { + // Deliberate: we do not know what happened to the step, so it renders as the + // neutral step it is with whatever it recorded. A visible "interrupted" + // treatment would be a design addition, not part of this fix. + const view = await renderBlock( + [step("a"), step("b", { status: "executing", completedAt: null })], + { liveTurnId: null, streamingItemId: null }, + ); + await view.expand(); + await view.settleToStepCount(2); + + assert.deepEqual( + view.glyphStates(), + ["settled", "settled"], + "no third state was invented for an abandoned step", + ); + assert.equal( + view.qa('[data-testid="transcript-tool-item"]').length, + 2, + "both steps still render through the normal tool presenter", + ); +}); + +test("the rail bullet masks the spine with the drawer surface colour", async () => { + // The bullet has to mask the spine passing behind it, and the mask must match + // the surface the transcript is drawn on. A mask in any other colour shows as + // a disc of the wrong shade around every bullet (berd's BOT-1599). + const view = await renderBlock([step("a"), step("b")]); + await view.expand(); + const bullet = view.q("[data-step-state]"); + assert.match(bullet.className, /\bbg-background\b/); + assert.match(bullet.className, /\bring-background\b/); + assert.match(bullet.className, /\brounded-full\b/); +}); + +test("the spine is drawn for every step except the last", async () => { + const view = await renderBlock([step("a"), step("b"), step("c")]); + await view.expand(); + const spines = view.qa(".w-px"); + assert.equal( + spines.length, + 2, + "three steps means two connecting segments; a trailing spine would dangle", + ); +}); + +test("thinking renders as a rail row with its own glyph, not a nested disclosure", async () => { + const view = await renderBlock([thoughtStep("thought:1"), step("a")]); + await view.expand(); + const thought = view.q('[data-testid="transcript-work-block-thought"]'); + assert.ok(thought, "reasoning renders on the rail"); + assert.match(thought.textContent, /weighing the options/); + assert.equal( + view.q('[data-testid="transcript-thought-disclosure"]'), + null, + "the block is already one disclosure — a thought must not add a second", + ); +}); + +/** + * An interim note is progress, not a second reply. + * + * #6720 gives every conversation-variant assistant message a 20px avatar + name + * identity row, which is right for the turn's answer. A rail note is the same + * item type, so routing it through that presenter would render a fully + * attributed agent turn nested inside a muted step row — the reader would see + * the agent apparently reply twice, once inside the work it was doing. berd + * draws the same line: its `progress` entry is a plain rail row. + * + * The suppression is done on this side (a dedicated prose body) rather than by + * reaching into the message presenter, so #6720 keeps one rule for what a + * message looks like. + */ +test("an interim note renders as rail prose with no identity row", async () => { + const view = await renderBlock([ + step("a"), + noteStep("msg:interim", "checked the three call sites"), + ]); + await view.expand(); + + const note = view.q('[data-testid="transcript-work-block-note"]'); + assert.ok(note, "the note renders on the rail"); + assert.match(note.textContent, /checked the three call sites/); + + assert.ok( + view.q('[data-testid="transcript-assistant-identity"]') === null, + "an avatar + name row inside a muted step reads as a second reply", + ); + assert.ok( + view.q('[data-testid="transcript-assistant-message"]') === null, + "the note must not go through the message presenter at all", + ); +}); + +/** + * A relay post is a step, not a reply — the same rule as an interim note, + * reached by a different route. + * + * A note is an assistant *message* the block re-presents as prose. A relay post + * is a *tool call* that classifies as `renderClass: "message"`, so it renders + * through `CompactMessageSummary`: 28px avatar, bordered speech bubble, + * timestamp, delivery-receipt button. That is right in the activity feed, where + * a posted message is a destination to open; on the rail it makes the agent + * appear to reply in the middle of its own work — and it did, in the seeded + * browser preview, which is where this was caught. + * + * Suppressing it needs the presentation signal rather than the transcript + * variant: the same relay step OUTSIDE a block in this variant keeps its + * bubble, which the next test pins. + */ +test("a relay post on the rail is a plain step, with no bubble or avatar", async () => { + const view = await renderBlock([step("a"), relayStep("relay:1")]); + await view.expand(); + + assert.equal( + view.stepCount(), + 2, + "the relay post takes its own rail row, like any other step", + ); + assert.equal( + view.qa('[data-work-block-entry="tool"]').length, + 2, + "a relay post is a tool step — it is something the agent did", + ); + assert.equal( + view.q('[data-testid="transcript-tool-message-preview"]'), + null, + "a speech bubble inside a muted step reads as the agent replying mid-work", + ); + assert.equal( + view.q('[data-testid="transcript-agent-sent-avatar"]'), + null, + "no identity avatar on the rail", + ); + assert.equal( + view.q('[data-testid="transcript-sent-message-context-button"]'), + null, + "no delivery receipt on the rail", + ); + + // It is still a real, expandable tool row carrying its command. + const rows = view.qa('[data-testid="transcript-tool-item"]'); + assert.equal(rows.length, 2, "both steps render as tool rows"); + assert.ok( + rows[1].querySelector("details"), + "the relay step keeps the ordinary step disclosure so its args stay reachable", + ); + assert.match( + rows[1].textContent, + /Sent|posted the findings/, + "the row still says what the step was", + ); +}); + +/** + * The other half of the branch: outside a block the bubble is correct and must + * survive. Without this, suppressing the bubble everywhere in the conversation + * variant would pass the test above. + */ +test("the same relay post outside a work block keeps its message bubble", async () => { + const { createElement } = await import("react"); + const { render } = await import("@testing-library/react"); + const { QueryClient, QueryClientProvider } = await import( + "@tanstack/react-query" + ); + const { createMemoryHistory, createRootRoute, createRouter, RouterProvider } = + await import("@tanstack/react-router"); + const { AgentSessionTranscriptVariantProvider } = await import( + "./agentSessionTranscriptContext.ts" + ); + const { TranscriptActivityItem } = await import( + "./activityRenderClasses/TranscriptActivityItem.tsx" + ); + + // `gcTime: 0`: React Query's default is 300000ms, and this is the one test + // that actually drives the bubble presenter's `useQuery`, so its query arms a + // five-minute gc timer at teardown. node:test waits that timer out before + // exiting — this file's tests sum to ~2s but the wall was ~303s, all passing, + // with no failing assertion to point at the cause. + const queryClient = new QueryClient({ + defaultOptions: { queries: { gcTime: 0, retry: false } }, + }); + const rootRoute = createRootRoute({ + component: () => + createElement( + QueryClientProvider, + { client: queryClient }, + createElement( + AgentSessionTranscriptVariantProvider, + { value: "conversation" }, + createElement(TranscriptActivityItem, { + agentAvatarUrl: null, + agentName: "Agent", + agentPubkey: "pk", + item: relayStep("relay:1"), + }), + ), + ), + }); + const router = createRouter({ + history: createMemoryHistory({ initialEntries: ["/"] }), + routeTree: rootRoute, + }); + await router.load(); + const view = render(createElement(RouterProvider, { router })); + + assert.ok( + view.container.querySelector( + '[data-testid="transcript-tool-message-preview"]', + ), + "outside a block a posted message is a destination the reader can open — the bubble stays", + ); +}); + +test("the rail glyph is chosen by kind, and prose kinds share the speech bubble", async () => { + // The exhaustive switch is the point: a note that fell through to the tool + // branch would wear a wrench and read as something the agent ran. + const view = await renderBlock([ + thoughtStep("thought:1"), + noteStep("msg:interim"), + step("a"), + step("b", { isError: true, status: "failed" }), + ]); + await view.expand(); + + const glyphClass = (kind, index = 0) => { + const rows = view.qa(`[data-work-block-entry="${kind}"]`); + const icon = rows[index].querySelector("svg"); + return icon.getAttribute("class") ?? ""; + }; + + // lucide stamps each icon with a `lucide-` class, so the glyph + // identity is readable from the DOM without reaching into the icon modules. + assert.match(glyphClass("thought"), /lucide-message-circle/); + assert.match( + glyphClass("note"), + /lucide-message-circle/, + "prose is the agent talking, whether it is reasoning or a note", + ); + assert.match(glyphClass("tool", 0), /lucide-wrench/); + assert.match( + glyphClass("tool", 1), + /lucide-circle/, + "a failed step is a filled dot, not a wrench", + ); +}); + +test("the rail bullet is never red, whatever the step's outcome", async () => { + // A failure is carried by glyph shape and by the folded line's count. Tinting + // the bullet would make one bad step read as an alarm across the whole run. + // + // The running step keeps this block live, so the rail is already open — which + // is also the only state in which a running bullet can be observed at all. + const view = await renderBlock( + [ + step("a"), + step("b", { isError: true, status: "failed" }), + step("c", { status: "executing", completedAt: null }), + ], + { streamingItemId: "c" }, + ); + + assert.deepEqual( + view.glyphStates(), + ["settled", "failed", "running"], + "all three outcomes are on screen", + ); + for (const bullet of view.qa("[data-step-state]")) { + assert.ok( + !/\b(text|bg|ring)-(destructive|red)/.test(bullet.className), + `rail bullet for ${bullet.getAttribute("data-step-state")} must stay muted`, + ); + assert.match(bullet.className, /\btext-muted-foreground\b/); + } +}); + +/** + * berd brightens rail prose with `usePrimaryText={open}`. Here the brightening + * is unconditional, and this test records why that is not a divergence: a closed + * block unmounts its rows rather than dimming them, so there is no state in + * which rail prose is on screen and NOT in an open block. A `primaryText` prop + * would have an unreachable false branch. + */ +test("rail prose is primary text, and a closed block has no prose on screen at all", async () => { + const { act } = await import("@testing-library/react"); + const live = [ + thoughtStep("thought:1"), + noteStep("msg:interim"), + step("b", { status: "executing", completedAt: null }), + ]; + const view = await renderBlock(live, { streamingItemId: "b" }); + + const prose = () => + view.qa( + '[data-testid="transcript-work-block-thought"],[data-testid="transcript-work-block-note"]', + ); + + assert.equal(prose().length, 2, "both prose rows are on the live rail"); + for (const node of prose()) { + assert.match( + node.className, + /\btext-foreground\b/, + "prose the reader can see is primary, not muted", + ); + assert.ok( + !/\btext-muted-foreground\b/.test(node.className), + "the row must not carry both colours", + ); + } + + // Finish the turn: the block folds and takes its prose with it. + await act(async () => { + view.stream([thoughtStep("thought:1"), noteStep("msg:interim"), step("b")]); + }); + assert.equal(await view.settleToStepCount(0), 0, "it folded"); + assert.equal( + prose().length, + 0, + "a folded block renders no prose, so there is no dimmed state to test", + ); + + // And the reader reopening it brings the same primary prose back. + await act(async () => { + view.summary().click(); + }); + assert.equal(await view.settleToStepCount(3), 3, "the reader reopened it"); + assert.equal(prose().length, 2); + for (const node of prose()) { + assert.match(node.className, /\btext-foreground\b/); + } +}); + +// ── Streaming cost ─────────────────────────────────────────────────────────── + +/** + * A block re-renders on every append while work streams. Unchanged steps must + * not re-render with it: each step's presenter rebuilds compact tool summaries, + * parses diffs and renders markdown/images, so an unmemoized step row makes a + * long block cost O(n) of that work per appended step. + * + * Counted at the presenter boundary — `TranscriptActivityItem` looks its + * presenter up in `ACTIVITY_RENDER_CLASS_PRESENTERS` on every render, so + * swapping in a counting presenter observes exactly the work a step row + * triggers, without reaching into React internals. + */ +async function countStepRenders(initialItems, nextItems) { + const { createElement } = await import("react"); + const { render } = await import("@testing-library/react"); + const { ACTIVITY_RENDER_CLASS_PRESENTERS } = await import( + "./activityRenderClasses/TranscriptActivityItem.tsx" + ); + const { AgentSessionTranscriptTurnMetaProvider } = await import( + "./agentSessionTranscriptContext.ts" + ); + const { AgentSessionWorkBlockSegment } = await import( + "./AgentSessionWorkBlock.tsx" + ); + + const renders = []; + const original = ACTIVITY_RENDER_CLASS_PRESENTERS.shell; + ACTIVITY_RENDER_CLASS_PRESENTERS.shell = function CountingPresenter(props) { + renders.push(props.item.id); + return createElement("div", null, props.item.id); + }; + + try { + const element = (items) => + createElement( + AgentSessionTranscriptTurnMetaProvider, + { + value: { + liveTurnId: items[items.length - 1].turnId, + streamingItemId: items[items.length - 1].id, + }, + }, + createElement(AgentSessionWorkBlockSegment, { + agentAvatarUrl: null, + agentName: "Agent", + agentPubkey: "pk", + block: { + id: "work-block:a", + items, + timestamp: items[0].timestamp, + }, + }), + ); + + const view = render(element(initialItems)); + renders.length = 0; + view.rerender(element(nextItems)); + return renders; + } finally { + ACTIVITY_RENDER_CLASS_PRESENTERS.shell = original; + } +} + +test("appending a step does not re-render the steps already on the rail", async () => { + // The block is expanded (a live block with ≤3 steps shows them all), and the + // prior steps are the SAME objects across both renders, as the transcript + // store replaces items rather than mutating them. + const settled = [step("a"), step("b")]; + const appended = [ + ...settled, + step("c", { status: "executing", completedAt: null }), + ]; + + const rendered = await countStepRenders(settled, appended); + + assert.deepEqual(rendered, ["c"]); +}); + +test("a step that actually changed does re-render", async () => { + // Guards the memo from being too aggressive: an executing step settling is a + // new object for that id, and it must re-render to drop its running glyph. + const a = step("a"); + const executing = step("b", { status: "executing", completedAt: null }); + const settled = step("b"); + + const rendered = await countStepRenders([a, executing], [a, settled]); + + assert.deepEqual(rendered, ["b"]); +}); diff --git a/desktop/src/features/agents/ui/AgentSessionWorkBlock.test.mjs b/desktop/src/features/agents/ui/AgentSessionWorkBlock.test.mjs new file mode 100644 index 00000000000..1b4f185c626 --- /dev/null +++ b/desktop/src/features/agents/ui/AgentSessionWorkBlock.test.mjs @@ -0,0 +1,402 @@ +/** + * Work-block rendering while a turn is live and when it finishes: the rail, the + * live window, the fold animation, and the reader's disclosure choice. + * Orphaned work and streaming cost live in + * `AgentSessionWorkBlock.orphaned.test.mjs`; the shared rig is + * `AgentSessionWorkBlockTestRig.mjs`. + */ + +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { + renderBlock, + setPrefersReducedMotion, + step, +} from "./AgentSessionWorkBlockTestRig.mjs"; + +// ── Live ───────────────────────────────────────────────────────────────────── + +test("a live block shows no header line — the rail is the status", async () => { + const view = await renderBlock([ + step("a"), + step("b", { status: "executing", completedAt: null }), + ]); + assert.equal( + view.summary(), + null, + "a header while live would only restate what the arriving steps show", + ); + assert.equal(view.stepCount(), 2); +}); + +test("a live block windows to the last three steps with the rest behind a disclosure", async () => { + const items = ["a", "b", "c", "d", "e"].map((id) => step(id)); + items[4] = step("e", { status: "executing", completedAt: null }); + const view = await renderBlock(items, { streamingItemId: "e" }); + + assert.equal(view.stepCount(), 3, "only the live window renders on the rail"); + const disclosure = view.previousSteps(); + assert.ok(disclosure, "older steps sit behind a disclosure"); + assert.match(disclosure.textContent, /2 previous steps/); +}); + +test("expanding previous steps reveals the older steps in place", async () => { + const { act } = await import("@testing-library/react"); + const items = ["a", "b", "c", "d", "e"].map((id) => step(id)); + items[4] = step("e", { status: "executing", completedAt: null }); + const view = await renderBlock(items, { streamingItemId: "e" }); + + assert.equal(view.stepCount(), 3); + await act(async () => { + view.previousSteps().click(); + }); + assert.equal( + await view.settleToStepCount(5), + 5, + "all five steps are now on the rail", + ); +}); + +// ── Finished ───────────────────────────────────────────────────────────────── + +test("a block that was already finished on mount folds to an N steps line", async () => { + const view = await renderBlock([step("a"), step("b"), step("c")]); + const summary = view.summary(); + assert.ok(summary, "a finished block gets its summary line"); + assert.match(summary.textContent, /3 steps/); + assert.equal(summary.getAttribute("aria-expanded"), "false"); + assert.equal(view.stepCount(), 0, "the rail is collapsed away"); +}); + +test("a finished block containing a failure names the failure in its folded line", async () => { + const view = await renderBlock([ + step("a"), + step("b", { isError: true, status: "failed" }), + step("c"), + ]); + assert.match( + view.summary().textContent, + /3 steps · 1 failed/, + "a failure must never hide behind a neutral count", + ); +}); + +test("clicking the folded line expands the whole rail", async () => { + const { act } = await import("@testing-library/react"); + const view = await renderBlock([step("a"), step("b"), step("c")]); + assert.equal(view.stepCount(), 0); + + await act(async () => { + view.summary().click(); + }); + + assert.equal(await view.settleToStepCount(3), 3); + assert.equal(view.summary().getAttribute("aria-expanded"), "true"); +}); + +test("a block expanded by the reader while live shows every step, not just the window", async () => { + const { act } = await import("@testing-library/react"); + const items = ["a", "b", "c", "d", "e"].map((id) => step(id)); + items[4] = step("e", { status: "executing", completedAt: null }); + const view = await renderBlock(items, { streamingItemId: "e" }); + + assert.equal(view.stepCount(), 3); + await act(async () => { + view.previousSteps().click(); + }); + assert.equal( + await view.settleToStepCount(5), + 5, + "a reader who asked to see the work sees all of it", + ); + + // And the window does NOT come back as more work streams in: the reader's + // choice is not re-decided on every append. + await act(async () => { + view.stream( + [...items, step("f", { status: "executing", completedAt: null })], + "f", + ); + }); + assert.equal( + await view.settleToStepCount(6), + 6, + "a reader-expanded live block keeps showing everything as it grows", + ); +}); + +// ── Fold animation ─────────────────────────────────────────────────────────── + +test("a block that finishes while mounted stays open for a paint so the collapse is visible", async () => { + const { act } = await import("@testing-library/react"); + const live = [ + step("a"), + step("b", { status: "executing", completedAt: null }), + ]; + const view = await renderBlock(live); + assert.equal(view.summary(), null, "live: no header"); + assert.equal(view.stepCount(), 2); + + // The turn finishes: the same block id re-renders with settled steps. + await act(async () => { + view.stream([step("a"), step("b")]); + }); + + // Still open on the commit right after finishing — that open state is what + // gives the height animation something to collapse FROM. A block that jumped + // straight to closed would swap a rail for a one-line summary between frames. + assert.equal( + view.stepCount(), + 2, + "the rail is still mounted for the settle frame", + ); + assert.ok( + view.summary(), + "the summary line appears as soon as work finishes", + ); + + // After the settle frames it closes — once the collapse animation has run. + assert.equal(await view.settleToStepCount(0), 0, "the block settles closed"); +}); + +test("under reduced motion a finishing block folds immediately, with no settle frames", async () => { + const { act } = await import("@testing-library/react"); + setPrefersReducedMotion(true); + + const view = await renderBlock([ + step("a"), + step("b", { status: "executing", completedAt: null }), + ]); + assert.equal(view.stepCount(), 2); + + await act(async () => { + view.stream([step("a"), step("b")]); + }); + + assert.equal( + view.stepCount(), + 0, + "reduced motion skips the animation, so there is nothing to hold open for", + ); + assert.ok( + view.summary(), + "it still folds to a summary line — only the animation is skipped", + ); +}); + +// ── Reader choice ──────────────────────────────────────────────────────────── + +/** + * The echo trap, and why this block is structurally immune to it. + * + * `
` fires `toggle` for programmatic `open` changes as well as for + * clicks, indistinguishably — so a policy-driven open echoes back looking like + * a reader choice and pins the row to its first policy state forever. That trap + * cost time on the tool-run card. + * + * This block cannot hit it, because its disclosure is a ` + ); +} + +function PreviousStepsDisclosure({ + agentAvatarUrl, + agentName, + agentPubkey, + entries, + motionEnabled, + profiles, +}: AgentTranscriptIdentityProps & { + entries: WorkBlockEntry[]; + motionEnabled: boolean; + profiles?: UserProfileLookup; +}) { + const [open, setOpen] = React.useState(false); + + return ( + + + + + + + ); +} + +function WorkBlockRail({ + agentAvatarUrl, + agentName, + agentPubkey, + animateEnter, + entries, + profiles, +}: AgentTranscriptIdentityProps & { + animateEnter: boolean; + entries: WorkBlockEntry[]; + profiles?: UserProfileLookup; +}) { + return ( +
+ + {entries.map((entry, index) => ( + + + + ))} + +
+ ); +} + +function WorkBlockStepRow({ + agentAvatarUrl, + agentName, + agentPubkey, + entry, + isLast, + profiles, +}: AgentTranscriptIdentityProps & { + entry: WorkBlockEntry; + isLast: boolean; + profiles?: UserProfileLookup; +}) { + return ( +
+ + +
+ ); +} + +/** + * A step's content: one exhaustive switch over the entry kind, so a new kind + * cannot silently inherit another kind's presentation. + * + * The entry arrives SPREAD into props rather than as an `entry` object, because + * this component is memoized and the projection is rebuilt whenever the block's + * item array changes — i.e. on every append — so entry objects are fresh each + * time and a memo keyed on one would never hit. Spread, the compared props are + * `item` (reference-stable: the transcript store replaces items rather than + * mutating them) plus two strings, so shallow comparison is a sound "did not + * change" test where comparison on the entry wrapper is not. + * + * Spreading keeps the discriminated union intact — `kind` and `item` stay paired + * in the props type — so the switch below narrows `item` to the kind's own item + * type. That is what removes the previous `item.type === "thought" ? ... : ""` + * re-checks: a mismatch is no longer representable, so there is no mismatch + * branch to render an empty body for. + * + * The memo boundary is the body rather than the whole row because the glyph + * depends on the row's *position* (`isLast` decides whether the spine + * continues), and that changes for the previous last row on every append. + * Passing position into the memoized part would invalidate that row's body on + * each append for a one-pixel spine segment; splitting keeps the cheap, + * position-dependent half outside and the expensive, item-dependent half in. + */ +const WorkBlockStepBody = React.memo(function WorkBlockStepBody( + props: WorkBlockEntryBodyProps, +) { + return ( +
+ +
+ ); +}); + +type WorkBlockEntryBodyProps = AgentTranscriptIdentityProps & { + profiles?: UserProfileLookup; +} & WorkBlockEntry; + +/** + * One exhaustive switch over the entry kind, so a new kind cannot silently + * inherit another kind's presentation. + * + * Because the props carry the whole discriminated entry, narrowing on `kind` + * also narrows `item` to that kind's item type. The previous version took + * `kind` and `item` as independent fields and so had to re-check + * `item.type === "thought" ? item.text : ""` — a mismatch branch that rendered + * an empty body and could only ever be reached by a projection bug. + */ +function WorkBlockEntryBody(props: WorkBlockEntryBodyProps) { + switch (props.kind) { + case "thought": + return ( + + ); + case "note": + return ( + + ); + case "tool": + return ( + + ); + } +} + +/** + * Prose on the rail: reasoning, or an interim note the agent addressed to the + * reader. No disclosure of its own — the whole block is already one, and + * nesting a second would mean two clicks to read something the reader has just + * chosen to reveal. + * + * A note deliberately does NOT go through the message presenter. That presenter + * gives every conversation-variant assistant message an avatar + name identity + * row, which is right for the turn's answer and wrong here: a fully attributed + * agent turn nested inside a muted rail step reads as a second reply rather + * than as progress. berd draws the same distinction — its `progress` entry is a + * plain rail row, not a message bubble. The focus code-block recipe is kept by + * providing the same `CodeBlockVariantContext` value the presenter would. + * + * berd brightens rail prose with `usePrimaryText={open}`. Here it is + * unconditional, because the rail only ever exists inside the open disclosure + * panel — a closed block unmounts its rows entirely rather than rendering them + * dimmed. Threading an `open` flag down to this component would be a prop whose + * false branch is unreachable, which is the same shape of dead-code-that-looks- + * load-bearing as the disclosure echo guard this block also dropped. If the + * block ever renders a peek of its rows while closed, the flag comes back with + * a test that can actually reach both branches. + */ +function WorkBlockProseBody({ + testId, + text, +}: { + testId: string; + text: string; +}) { + return ( +
+ + + +
+ ); +} + +/** + * The spine and this row's bullet. + * + * The bullet masks the spine passing behind it, which is what makes the rail + * read as a series of stops rather than a line with icons floating over it. + * The mask has to match the surface the transcript is actually drawn on — the + * cover drawer's `bg-background` — because a mask in any other colour shows up + * as a visible disc of the wrong shade around every bullet. + * + * (berd's equivalent uses `bg-card` and warns against `bg-background`; that is + * the same rule, not a different one. In berd the transcript sits on a card, so + * `bg-card` is its surface. Buzz's drawer surface is `bg-background`, and the + * two tokens are NOT interchangeable here: they share a value in the base + * themes, but in Buzz Dark the drawer sits inside `[data-buzz-content-surface]`, + * which locally overrides `--background` to `--buzz-content-dark` while + * `--card` keeps the theme value. Measured in a seeded browser: + * + * | theme | bullet | drawer surface | spine | + * | ------------ | --------------- | --------------- | --------------- | + * | github-light | rgb(255,255,255)| rgb(255,255,255)| rgb(229,229,230)| + * | buzz-dark | rgb(26,26,26) | rgb(26,26,26) | rgb(64,69,74) | + * + * `bg-card` paints the bullet `rgb(36,41,46)` over that `rgb(26,26,26)` drawer + * — a visible disc of the wrong shade, which is exactly the BOT-1599 failure. + * Light mode alone matches under either class, so light-mode evidence is not + * sufficient here. Following berd's class literally would be following the + * letter of its note against its point.) + */ +function WorkBlockRailGlyph({ + entry, + isLast, +}: { + entry: WorkBlockEntry; + isLast: boolean; +}) { + return ( +