From 2ee01cb85a2a78fe6903c2719113dc02096aaff4 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 8 Jul 2026 11:18:37 -0400 Subject: [PATCH] fix(desktop): defer pre-resolution null-session items in splitIntoSessionRuns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first-turn wire sequence emitted by the harness is: turn_started(sessionId=null) → session/new(sessionId=null) → session_resolved(sessionId=X) → session/prompt(sessionId=X) The previous splitIntoSessionRuns opened a synthetic 'unknown' run for the leading null-session items, then opened a second run when session_resolved arrived. This caused two bugs: 1. Leaked system prompt: the session/new metadata had no following user prompt in its 'unknown' run so pendingSystemPrompt was never consumed and emitted as a standalone 'System prompt' row. Caught by Desktop Smoke E2E (3), observer-feed-screenshots.spec.ts:726 (expected count 0, got 1). 2. Spurious session boundary: two runs from one session triggered buildTranscriptDisplayBlocks to inject a session-boundary block on a feed that has exactly one session — the opposite of the signal this PR adds. Fix: buffer pre-resolution null-session items and prepend them to the first run that has a non-null sessionId. Only if the entire stream has no resolved session do they form a fallback 'unknown' run. Mid-stream null-session items (after resolution) continue to attribute to the current run as before. Added two regression tests: - firstTurnSequence_noStandaloneSystemPrompt: asserts zero standalone session/new blocks, zero session-boundary blocks, system prompt present inside the turn block (prompt bundle). - genuineSecondSession_boundaryPreserved: asserts the deferral fix does not collapse two genuinely distinct sessions — boundary is still injected. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../agentSessionTranscriptGrouping.test.mjs | 159 ++++++++++++++++++ .../ui/agentSessionTranscriptGrouping.ts | 54 +++++- 2 files changed, 205 insertions(+), 8 deletions(-) diff --git a/desktop/src/features/agents/ui/agentSessionTranscriptGrouping.test.mjs b/desktop/src/features/agents/ui/agentSessionTranscriptGrouping.test.mjs index 569895556..2c29710c8 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscriptGrouping.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionTranscriptGrouping.test.mjs @@ -860,3 +860,162 @@ test("isObserverEventAfter returns false for same timestamp, lower seq", () => { const candidate = { timestamp: "2026-07-08T00:00:01.000Z", seq: 3 }; assert.ok(!isObserverEventAfter(candidate, stored)); }); + +// ── Pre-resolution null-session deferral (regression for first-turn bundle) ─── + +/** + * Builds the exact wire sequence the harness emits on the first turn: + * turn_started(null) → session/new(null) → session_resolved(sess) → session/prompt(sess) + * + * After processTranscriptEvent the TranscriptItems produced are: + * - lifecycle turn_started: sessionId=null, turnId="turn-001" + * - metadata session/new (system prompt): sessionId=null, turnId=null + * - lifecycle session_resolved: sessionId="session-001", turnId="turn-001" + * - message session/prompt:user: sessionId="session-001", turnId="turn-001" + */ +function firstTurnSequence() { + const ts = "2026-07-08T10:00:00.000Z"; + return [ + // turn_started: sessionId null, turnId present + { + id: "turn-started", + type: "lifecycle", + renderClass: "lifecycle", + title: "Turn started", + text: "", + timestamp: ts, + acpSource: "turn_started", + turnId: "turn-001", + sessionId: null, + channelId: "chan-1", + }, + // session/new system prompt: sessionId null, turnId null (processTranscriptEvent forces turnId null) + { + id: "system-prompt:chan-1", + type: "metadata", + renderClass: "raw-rail", + title: "System prompt", + sections: [ + { title: "Base", body: "You are a helpful AI assistant." }, + { title: "System", body: "You are Observer Agent." }, + ], + timestamp: ts, + acpSource: "session/new", + turnId: null, + sessionId: null, + channelId: "chan-1", + }, + // session_resolved: first item with a non-null sessionId + { + id: "session-resolved", + type: "lifecycle", + renderClass: "lifecycle", + title: "Session ready", + text: "", + timestamp: ts, + acpSource: "session_resolved", + turnId: "turn-001", + sessionId: "session-001", + channelId: "chan-1", + }, + // session/prompt:user — the user message bubble + { + id: "user-prompt", + type: "message", + role: "user", + title: "Buzz event", + text: "@Observer Agent help me debug this", + timestamp: ts, + acpSource: "session/prompt:user", + turnId: "turn-001", + sessionId: "session-001", + channelId: "chan-1", + }, + ]; +} + +test("buildTranscriptDisplayBlocks_firstTurnSequence_noStandaloneSystemPrompt", () => { + // Regression for: pre-resolution null-session items (turn_started + session/new) + // were assigned to a synthetic "unknown" run, causing the system prompt to emit + // as a standalone "System prompt" row instead of staying in the prompt bundle. + const blocks = buildTranscriptDisplayBlocks(firstTurnSequence()); + + // (a) No standalone "System prompt" single block. + const systemPromptSingles = blocks.filter( + (b) => b.kind === "single" && b.item.acpSource === "session/new", + ); + assert.equal( + systemPromptSingles.length, + 0, + "system prompt must not appear as a standalone single block", + ); + + // (b) No session-boundary blocks — this is a single-session transcript. + const boundaryBlocks = blocks.filter((b) => b.kind === "session-boundary"); + assert.equal( + boundaryBlocks.length, + 0, + "must be zero session-boundary blocks for a first-turn single-session sequence", + ); + + // (c) System prompt is inside the turn group (consumed by the prompt bundle). + const turnBlocks = blocks.filter((b) => b.kind === "turn"); + assert.ok(turnBlocks.length > 0, "at least one turn block must exist"); + const allTurnItems = flattenDisplayBlocks(turnBlocks); + const systemPromptInTurn = allTurnItems.some( + (item) => item.acpSource === "session/new", + ); + assert.ok( + systemPromptInTurn, + "system prompt item must be present inside a turn block (prompt bundle)", + ); +}); + +test("buildTranscriptDisplayBlocks_genuineSecondSession_boundaryPreserved", () => { + // Regression guard: the deferral fix must not over-collapse two genuinely + // distinct sessions. A second session_resolved with a different sessionId + // still gets its own run and a session-boundary block. + const ts2 = "2026-07-08T11:00:00.000Z"; + const items = [ + // First session (may have pre-resolution preamble) + ...firstTurnSequence(), + // Second session — starts fresh with its own turn_started (no pre-null preamble here) + { + id: "turn-started-2", + type: "lifecycle", + renderClass: "lifecycle", + title: "Turn started", + text: "", + timestamp: ts2, + acpSource: "turn_started", + turnId: "turn-002", + sessionId: "session-002", + channelId: "chan-1", + }, + { + id: "user-prompt-2", + type: "message", + role: "user", + title: "Buzz event", + text: "second session message", + timestamp: ts2, + acpSource: "session/prompt:user", + turnId: "turn-002", + sessionId: "session-002", + channelId: "chan-1", + }, + ]; + + const blocks = buildTranscriptDisplayBlocks(items); + const boundaryBlocks = blocks.filter((b) => b.kind === "session-boundary"); + assert.equal( + boundaryBlocks.length, + 1, + "exactly one session-boundary block between two distinct sessions", + ); + assert.equal( + boundaryBlocks[0].sessionId, + "session-002", + "boundary is labeled with the newer session id", + ); +}); diff --git a/desktop/src/features/agents/ui/agentSessionTranscriptGrouping.ts b/desktop/src/features/agents/ui/agentSessionTranscriptGrouping.ts index ced54d19c..f7798f4f2 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscriptGrouping.ts +++ b/desktop/src/features/agents/ui/agentSessionTranscriptGrouping.ts @@ -377,8 +377,22 @@ function getRenderClass(item: TranscriptItem) { /** * Split a flat, time-ordered array of TranscriptItems into contiguous session - * runs. Items with a null sessionId are attributed to the most recently seen - * session (or a synthetic "unknown" run if no session has appeared yet). + * runs keyed by `sessionId`. + * + * **Null-session handling**: the real first-turn wire sequence is + * `turn_started(null) → session/new(null) → session_resolved(sess-X) → …`. + * Pre-resolution items arrive with `sessionId: null` before any session has + * been assigned. We defer those leading null-session items and **prepend them + * to the first run that has a non-null sessionId**, so they stay in the same + * session run as the turn they belong to and the `pendingSystemPrompt` slot in + * `buildBlocksForRun` can consume them correctly. + * + * Mid-stream null-session items (after at least one session has resolved) are + * attributed to the most recently seen session run — same as before, this + * handles gap frames that arrive after resolution. + * + * Only if the entire stream is null-session (no session ever resolves) do the + * deferred items form a single fallback run keyed `"unknown"`. * * A new run begins whenever the sessionId changes to a distinct non-null value. */ @@ -387,19 +401,43 @@ function splitIntoSessionRuns( ): Array<{ sessionId: string; items: TranscriptItem[] }> { const runs: Array<{ sessionId: string; items: TranscriptItem[] }> = []; let currentRun: { sessionId: string; items: TranscriptItem[] } | null = null; + // Buffer for items that arrive before any session has resolved. + const preSessionBuffer: TranscriptItem[] = []; for (const item of items) { - const sid: string = item.sessionId ?? currentRun?.sessionId ?? "unknown"; - if ( - !currentRun || - (item.sessionId && item.sessionId !== currentRun.sessionId) - ) { - currentRun = { sessionId: sid, items: [] }; + if (item.sessionId === null || item.sessionId === undefined) { + if (currentRun === null) { + // No session resolved yet — defer into the pre-session buffer. + preSessionBuffer.push(item); + } else { + // Session already resolved — attribute to current run. + currentRun.items.push(item); + } + continue; + } + + // item.sessionId is non-null from here. + if (!currentRun || item.sessionId !== currentRun.sessionId) { + const newRun: { sessionId: string; items: TranscriptItem[] } = { + sessionId: item.sessionId, + items: [], + }; + if (currentRun === null && preSessionBuffer.length > 0) { + // First resolved session: prepend buffered pre-resolution items. + newRun.items.push(...preSessionBuffer); + preSessionBuffer.length = 0; + } + currentRun = newRun; runs.push(currentRun); } currentRun.items.push(item); } + // Entire stream was null-session (no session ever resolved): emit as one run. + if (currentRun === null && preSessionBuffer.length > 0) { + runs.push({ sessionId: "unknown", items: preSessionBuffer }); + } + return runs; }