From a475280ed328713c54478851a4a8338a2d1d0c1d Mon Sep 17 00:00:00 2001 From: t Date: Mon, 3 Aug 2026 13:40:43 +0800 Subject: [PATCH] feat(desktop): resume a thread from its items, not just its messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding F8 in docs/THREE_WAY_REVIEW.md, and more visible now that the Changes panel exists: resume a thread and its review findings were gone, even though they were on disk. The desktop called resumeProtocolThread(), threw the returned snapshot away, and rebuilt the transcript from the canonical session projection instead. That projection is historyFromThread(), which keeps only items carrying a StoredMessage — so approvals, ask-user exchanges, errors, review findings and review actions were persisted by the app-server and then never shown again. Resume now projects the snapshot. Messages and tool cards render as before; the other items become transcript entries, and review findings/actions are adopted into the Changes panel so a resumed conversation can still apply or revert them. Threads with no items — legacy sessions imported lazily — keep falling back to the message projection, which the existing e2e case covers. storedToMsgs is split into a per-message fold so the projection can interleave non-message items without breaking tool-result attachment: a tool_result block has to match an assistant turn already in the list, which a fresh storedToMsgs([line]) call cannot see. Co-Authored-By: Claude Opus 5 --- apps/desktop/e2e/desktop-preview.spec.ts | 21 ++++ apps/desktop/src/App.tsx | 21 +++- apps/desktop/src/lib/repl-stream.test.ts | 111 +++++++++++++++++++++ apps/desktop/src/lib/repl-stream.ts | 118 ++++++++++++++++++++++- apps/desktop/src/lib/use-changes.ts | 29 +++++- apps/desktop/src/lib/window-shim.ts | 8 +- apps/desktop/src/preview-app.tsx | 83 +++++++++++++++- apps/desktop/src/types/global.d.ts | 8 +- 8 files changed, 389 insertions(+), 10 deletions(-) diff --git a/apps/desktop/e2e/desktop-preview.spec.ts b/apps/desktop/e2e/desktop-preview.spec.ts index 1cdde36..491c671 100644 --- a/apps/desktop/e2e/desktop-preview.spec.ts +++ b/apps/desktop/e2e/desktop-preview.spec.ts @@ -167,3 +167,24 @@ test('reviews findings and the working tree from the Changes panel', async ({ pa await expect(panel.getByText('applied', { exact: true })).toBeVisible(); await expect(panel.getByRole('button', { name: 'Revert', exact: true })).toBeVisible(); }); + +test('resumes a thread from its protocol items, not just its messages', async ({ page }) => { + await page.locator('[title*="2026-06-02-bbb222"]').click(); + const main = page.getByRole('main'); + + // Messages and tool cards, as before. + await expect(main.getByText('Harden the loader', { exact: true })).toBeVisible(); + await expect(main.getByText(/Reading the loader\./)).toBeVisible(); + await expect(main.locator('.tool-card').filter({ hasText: 'Read' })).toBeVisible(); + + // The items the message projection dropped on the floor. + await expect(main.getByText('⏸ Edit — allow', { exact: true })).toBeVisible(); + await expect(main.getByText('❯ Which loader? → the config one', { exact: true })).toBeVisible(); + await expect(main.getByText(/Loader swallows parse errors/)).toBeVisible(); + + // And the finding is live in the Changes panel, not just narrated. + await page.getByRole('button', { name: /^Changes\b/ }).click(); + const panel = page.getByTestId('changes-panel'); + await expect(panel.getByText('Loader swallows parse errors')).toBeVisible(); + await expect(panel.getByRole('button', { name: 'src/loader.ts:42' })).toBeVisible(); +}); diff --git a/apps/desktop/src/App.tsx b/apps/desktop/src/App.tsx index 495384b..a648138 100644 --- a/apps/desktop/src/App.tsx +++ b/apps/desktop/src/App.tsx @@ -15,7 +15,13 @@ import { UpdateBanner } from './components/UpdateBanner.js'; import { registerShortcut } from './lib/keyboard.js'; import { clearProtocolThread as clearAgentHistory } from './lib/protocol-agent.js'; import { loadProjectPath, saveProjectPath } from './lib/project.js'; -import { storedToMsgs, type Msg } from './lib/repl-stream.js'; +import { + storedToMsgs, + threadReviewItems, + threadToMsgs, + type Msg, + type ThreadLike, +} from './lib/repl-stream.js'; import { onUpdateDownloaded, startUpdaterPolling } from './lib/updater.js'; import { changesBadge } from './lib/changes-reducer.js'; import { useChanges } from './lib/use-changes.js'; @@ -237,8 +243,17 @@ export function App(): JSX.Element { // Load the session's stored messages, adopt them into the agent, and // remount ReplScreen seeded with the reconstructed conversation. try { - const { history } = await window.deepcode.sessions.resume({ id }); - setResumedMessages(storedToMsgs(history as Parameters[0])); + const { history, thread } = await window.deepcode.sessions.resume({ id }); + const snapshot = thread as ThreadLike | undefined; + // Prefer the protocol snapshot; fall back to the message projection + // for legacy threads that have no items yet. + const hasItems = (snapshot?.turns ?? []).some((t) => t.items.length > 0); + setResumedMessages( + hasItems + ? threadToMsgs(snapshot!) + : storedToMsgs(history as Parameters[0]), + ); + if (snapshot) changes.adopt(threadReviewItems(snapshot)); } catch { setResumedMessages(undefined); // fall back to a fresh view } diff --git a/apps/desktop/src/lib/repl-stream.test.ts b/apps/desktop/src/lib/repl-stream.test.ts index 2240b85..6ca8d01 100644 --- a/apps/desktop/src/lib/repl-stream.test.ts +++ b/apps/desktop/src/lib/repl-stream.test.ts @@ -7,6 +7,9 @@ import { lastAssistantIndex, pickTarget, storedToMsgs, + threadReviewItems, + threadToMsgs, + type AssistantMsg, type Msg, type ToolInvocation, } from './repl-stream.js'; @@ -170,3 +173,111 @@ describe('repl-stream mutators', () => { expect(pickTarget({ irrelevant: 1 })).toBeUndefined(); }); }); + +describe('threadToMsgs', () => { + const turn = (items: Array<{ type: string; payload: Record }>) => ({ + turns: [{ items }], + }); + + it('projects a user message', () => { + expect(threadToMsgs(turn([{ type: 'user_message', payload: { text: 'hi' } }]))).toEqual([ + { role: 'user', text: 'hi' }, + ]); + }); + + it('attaches a tool result to the assistant turn that issued the call', () => { + const msgs = threadToMsgs( + turn([ + { + type: 'assistant_message', + payload: { + message: { + role: 'assistant', + content: [ + { type: 'text', text: 'Reading.' }, + { type: 'tool_use', id: 't1', name: 'Read', input: { file_path: 'a.ts' } }, + ], + }, + }, + }, + { + type: 'tool_result', + payload: { + message: { + role: 'user', + content: [{ type: 'tool_result', tool_use_id: 't1', content: 'file body' }], + }, + }, + }, + ]), + ); + const assistant = msgs.find((m) => m.role === 'assistant') as AssistantMsg; + expect(assistant.turn.tools).toHaveLength(1); + // The result must land on the existing card, not create a second turn. + expect(assistant.turn.tools[0]!.status).toBe('ok'); + expect(assistant.turn.tools[0]!.resultText).toBe('file body'); + expect(msgs.filter((m) => m.role === 'assistant')).toHaveLength(1); + }); + + it('restores the items the message projection dropped', () => { + const msgs = threadToMsgs( + turn([ + { type: 'approval', payload: { toolName: 'Edit', decision: 'allow' } }, + { type: 'ask_user', payload: { question: 'Which one?', answer: 'the first' } }, + { + type: 'review_finding', + payload: { path: 'src/a.ts', startLine: 4, title: 'Null crash' }, + }, + { type: 'review_action', payload: { kind: 'apply', findingIds: ['f1'] } }, + { type: 'error', payload: { message: 'provider timed out' } }, + ]), + ); + const text = msgs.map((m) => (m.role === 'system' ? m.text : '')).join('\n'); + expect(text).toContain('Edit'); + expect(text).toContain('allow'); + expect(text).toContain('Which one?'); + expect(text).toContain('the first'); + expect(text).toContain('src/a.ts:4'); + expect(text).toContain('Null crash'); + expect(text).toContain('review apply'); + expect(text).toContain('provider timed out'); + expect(msgs.at(-1)).toMatchObject({ level: 'error' }); + }); + + it('keeps items in the order they completed', () => { + const msgs = threadToMsgs( + turn([ + { type: 'user_message', payload: { text: 'first' } }, + { type: 'ask_user', payload: { question: 'q', answer: 'a' } }, + { type: 'user_message', payload: { text: 'second' } }, + ]), + ); + expect(msgs.map((m) => m.role)).toEqual(['user', 'system', 'user']); + }); + + it('ignores item types it does not know', () => { + expect(threadToMsgs(turn([{ type: 'something_new', payload: {} }]))).toEqual([]); + }); + + it('returns nothing for a thread with no turns', () => { + expect(threadToMsgs({ turns: [] })).toEqual([]); + }); +}); + +describe('threadReviewItems', () => { + it('collects findings and actions across turns', () => { + const { findings, actions } = threadReviewItems({ + turns: [ + { items: [{ type: 'review_finding', payload: { findingId: 'f1' } }] }, + { + items: [ + { type: 'review_action', payload: { actionId: 'a1', kind: 'apply' } }, + { type: 'user_message', payload: { text: 'ignored' } }, + ], + }, + ], + }); + expect(findings).toEqual([{ findingId: 'f1' }]); + expect(actions).toEqual([{ actionId: 'a1', kind: 'apply' }]); + }); +}); diff --git a/apps/desktop/src/lib/repl-stream.ts b/apps/desktop/src/lib/repl-stream.ts index bb08641..f8cfbb5 100644 --- a/apps/desktop/src/lib/repl-stream.ts +++ b/apps/desktop/src/lib/repl-stream.ts @@ -157,8 +157,20 @@ export interface StoredLine { * dropped (they were streaming-only). All turns are non-streaming (finalized). */ export function storedToMsgs(stored: StoredLine[]): Msg[] { - let msgs: Msg[] = []; - for (const m of stored) { + return stored.reduce(appendStoredLine, [] as Msg[]); +} + +/** + * Fold one stored message into the transcript. + * + * Split out of storedToMsgs so a thread projection can interleave non-message + * items without losing tool-result attachment: a `tool_result` block has to be + * matched against the assistant turn already in `msgs`, which a fresh + * storedToMsgs([line]) call cannot see. + */ +export function appendStoredLine(input: Msg[], m: StoredLine): Msg[] { + let msgs = [...input]; + { if (m.role === 'assistant') { const texts: string[] = []; const tools: ToolInvocation[] = []; @@ -202,3 +214,105 @@ export function pickTarget(input: Record): string | undefined { } return undefined; } + +// ── Resuming from a protocol thread ────────────────────────────────────── + +/** The subset of a protocol CompletedItem this projection needs. */ +export interface ThreadItem { + type: string; + payload: Record; +} +export interface ThreadTurn { + items: ThreadItem[]; +} +export interface ThreadLike { + turns: ThreadTurn[]; +} + +/** + * Rebuild the transcript from a protocol thread snapshot. + * + * The session projection the desktop used to resume from keeps only the items + * that carry a StoredMessage, so approvals, ask-user exchanges, errors and + * review findings were persisted in the snapshot and then never shown again. + * This reads the snapshot itself, so a resumed conversation looks like the one + * that was interrupted. + */ +export function threadToMsgs(thread: ThreadLike): Msg[] { + let msgs: Msg[] = []; + const str = (value: unknown): string => (typeof value === 'string' ? value : ''); + + for (const turn of thread.turns) { + for (const item of turn.items) { + switch (item.type) { + case 'user_message': + if (str(item.payload.text)) msgs.push({ role: 'user', text: str(item.payload.text) }); + break; + + case 'assistant_message': + case 'tool_result': { + const message = item.payload.message as StoredLine | undefined; + if (Array.isArray(message?.content)) msgs = appendStoredLine(msgs, message); + break; + } + + case 'approval': { + const tool = str(item.payload.toolName) || 'tool'; + const decision = str(item.payload.decision) || 'answered'; + msgs.push({ role: 'system', text: `⏸ ${tool} — ${decision}` }); + break; + } + + case 'ask_user': { + const question = str(item.payload.question); + const answer = str(item.payload.answer); + msgs.push({ role: 'system', text: `❯ ${question}${answer ? ` → ${answer}` : ''}` }); + break; + } + + case 'review_finding': + msgs.push({ + role: 'system', + text: `⚑ ${str(item.payload.path)}${ + typeof item.payload.startLine === 'number' ? `:${item.payload.startLine}` : '' + } — ${str(item.payload.title)}`, + }); + break; + + case 'review_action': + msgs.push({ + role: 'system', + text: `${str(item.payload.kind) === 'revert' ? '↩' : '✎'} review ${str( + item.payload.kind, + )} · ${(item.payload.findingIds as string[] | undefined)?.length ?? 0} finding(s)`, + }); + break; + + case 'error': + msgs.push({ + role: 'system', + text: str(item.payload.message) || 'Turn failed.', + level: 'error', + }); + break; + } + } + } + return msgs; +} + +/** Review findings and actions carried by a resumed thread, for the Changes panel. */ +export function threadReviewItems(thread: ThreadLike): { + findings: Record[]; + actions: Record[]; +} { + const findings: Record[] = []; + const actions: Record[] = []; + for (const turn of thread.turns) { + for (const item of turn.items) { + if (item.type === 'review_finding') findings.push(item.payload); + else if (item.type === 'review_action') actions.push(item.payload); + } + } + return { findings, actions }; +} diff --git a/apps/desktop/src/lib/use-changes.ts b/apps/desktop/src/lib/use-changes.ts index e2139c0..fc2cf92 100644 --- a/apps/desktop/src/lib/use-changes.ts +++ b/apps/desktop/src/lib/use-changes.ts @@ -22,6 +22,11 @@ export interface UseChanges { apply: (findings: ReviewFinding[]) => Promise; revert: (actionId: string) => Promise; clear: () => void; + /** Seed findings/actions carried by a resumed thread. */ + adopt: (items: { + findings: Record[]; + actions: Record[]; + }) => void; } interface BusEvent { @@ -103,5 +108,27 @@ export function useChanges(): UseChanges { const toggleFile = useCallback((path: string) => dispatch({ type: 'toggle-file', path }), []); const clear = useCallback(() => dispatch({ type: 'cleared' }), []); - return { state, refresh, toggleFile, apply, revert, clear }; + // Resuming replays the thread's review items so the panel shows what the + // conversation already found, not an empty list over a repo full of changes. + const adopt = useCallback( + (items: { findings: Record[]; actions: Record[] }) => { + dispatch({ type: 'cleared' }); + for (const finding of items.findings) { + dispatch({ type: 'finding', finding: finding as unknown as ReviewFinding }); + } + for (const action of items.actions) { + dispatch({ + type: 'action', + action: { + actionId: String(action.actionId ?? ''), + findingIds: Array.isArray(action.findingIds) ? action.findingIds.map(String) : [], + kind: action.kind === 'revert' ? 'revert' : 'apply', + }, + }); + } + }, + [], + ); + + return { state, refresh, toggleFile, apply, revert, clear, adopt }; } diff --git a/apps/desktop/src/lib/window-shim.ts b/apps/desktop/src/lib/window-shim.ts index 64e92df..f4d755a 100644 --- a/apps/desktop/src/lib/window-shim.ts +++ b/apps/desktop/src/lib/window-shim.ts @@ -75,14 +75,18 @@ export function installTauriShim(): void { })); }, async resume({ id }) { - await resumeProtocolThread(id); + // The snapshot carries every completed item — approvals, ask-user + // exchanges, errors, review findings. The session projection keeps only + // the message-bearing ones, so resuming from it silently dropped the + // rest even though they were on disk. + const thread = await resumeProtocolThread(id); const lines = await sessionRead(id); const history = lines.map((l) => ({ role: l.role, content: l.content, timestamp: l.timestamp ?? '', })) as unknown as import('@deepcode/core/dist/types.js').StoredMessage[]; - return { history, sessionId: id }; + return { history, sessionId: id, thread }; }, }, plugins: { diff --git a/apps/desktop/src/preview-app.tsx b/apps/desktop/src/preview-app.tsx index a79c551..785f5c1 100644 --- a/apps/desktop/src/preview-app.tsx +++ b/apps/desktop/src/preview-app.tsx @@ -154,13 +154,94 @@ let activeThreadId = MOCK_SESSIONS[0]!.id; let activeTurn: TurnSnapshot | null = null; const protocolRequests: ProtocolRequest[] = []; +// One fixture thread carries completed items so resume can be exercised against +// the protocol snapshot; the others stay empty so the legacy message-projection +// fallback keeps being covered too. +const THREAD_WITH_ITEMS = MOCK_SESSIONS[1]!.id; + function threadSnapshot(id: string): ThreadSnapshot { return { id, cwd: '/Users/oratis/Projects/DeepCode/test', createdAt: '2026-08-01T00:00:00.000Z', updatedAt: '2026-08-01T00:00:00.000Z', - turns: [], + turns: + id === THREAD_WITH_ITEMS + ? [ + { + id: 'resumed-turn-1', + threadId: id, + status: 'completed', + startedAt: '2026-08-01T00:00:00.000Z', + completedAt: '2026-08-01T00:00:04.000Z', + items: [ + { + id: 'i1', + type: 'user_message', + completedAt: '2026-08-01T00:00:00.000Z', + payload: { text: 'Harden the loader' }, + }, + { + id: 'i2', + type: 'assistant_message', + completedAt: '2026-08-01T00:00:01.000Z', + payload: { + message: { + role: 'assistant', + content: [ + { type: 'text', text: 'Reading the loader.' }, + { + type: 'tool_use', + id: 't1', + name: 'Read', + input: { file_path: 'src/loader.ts' }, + }, + ], + }, + }, + }, + { + id: 'i3', + type: 'tool_result', + completedAt: '2026-08-01T00:00:02.000Z', + payload: { + message: { + role: 'user', + content: [{ type: 'tool_result', tool_use_id: 't1', content: 'ok' }], + }, + }, + }, + { + id: 'i4', + type: 'approval', + completedAt: '2026-08-01T00:00:02.500Z', + payload: { toolName: 'Edit', decision: 'allow' }, + }, + { + id: 'i5', + type: 'ask_user', + completedAt: '2026-08-01T00:00:03.000Z', + payload: { question: 'Which loader?', answer: 'the config one' }, + }, + { + id: 'i6', + type: 'review_finding', + completedAt: '2026-08-01T00:00:03.500Z', + payload: { + findingId: 'resumed-finding-1', + title: 'Loader swallows parse errors', + body: 'The catch returns undefined.', + path: 'src/loader.ts', + startLine: 42, + endLine: 42, + priority: 1, + replacement: 'throw err;', + }, + }, + ], + }, + ] + : [], }; } diff --git a/apps/desktop/src/types/global.d.ts b/apps/desktop/src/types/global.d.ts index 4b15ad5..1a908da 100644 --- a/apps/desktop/src/types/global.d.ts +++ b/apps/desktop/src/types/global.d.ts @@ -51,7 +51,13 @@ export interface DeepCodeAPI { }; sessions: { list: (args?: { limit?: number }) => Promise; - resume: (args: { id: string }) => Promise<{ history: unknown[]; sessionId: string }>; + /** + * Resumes the protocol thread and returns its snapshot. `history` remains + * the canonical message projection for legacy threads that have no items. + */ + resume: (args: { + id: string; + }) => Promise<{ history: unknown[]; sessionId: string; thread?: unknown }>; }; plugins: { list: () => Promise;