Skip to content

feat(desktop): resume a thread from its items, not just its messages - #229

Merged
oratis merged 1 commit into
mainfrom
feat/desktop-resume-items
Aug 3, 2026
Merged

feat(desktop): resume a thread from its items, not just its messages#229
oratis merged 1 commit into
mainfrom
feat/desktop-resume-items

Conversation

@oratis

@oratis oratis commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Finding F8 in docs/THREE_WAY_REVIEW.md — and much more visible now that #220 shipped the Changes panel: resume a thread and its review findings are gone, even though they are on disk.

What was happening

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:

function messageFromItem(item: CompletedItem): StoredMessage | null {
  if (item.type === 'user_message' && ) return ;
  const message = item.payload.message;
  if (!isStoredMessage(message)) return null;   // ← approval, ask_user, review_finding, error
  return message;
}

So the app-server persisted those items faithfully and the UI never showed them again.

What changes

Resume projects the snapshot. Messages and tool cards render as before; approvals, ask-user exchanges, review findings/actions and errors become transcript entries; and 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 on first resume — keep falling back to the message projection. The existing resume e2e case covers that path and is unchanged.

storedToMsgs is split into a per-message fold (appendStoredLine). The projection has to interleave non-message items, and a tool_result block must match an assistant turn already in the list — something a fresh storedToMsgs([line]) call cannot see. Without the split, resumed tool cards would have silently lost their results.

Verification

7 new unit tests over the pure projection (ordering, tool-result attachment, each restored item type, unknown item types, empty threads). The preview fixture gains a thread whose snapshot carries real items — user message, assistant + tool_use, tool_result, approval, ask_user, review_finding — and a Playwright case resumes it and asserts both the transcript entries and the finding showing up live in the Changes panel. The other fixture threads stay empty so the legacy path keeps its coverage.

pnpm typecheck · lint · format:check clean; desktop 97 · core 750/16 skipped · cli 208 · server 41 · protocol 24 · vscode 12 · lsp 13 · scripts 21; playwright test → 7 passed.

Not in this PR: reasoning is still dropped in the desktop. It isn't a projection bug — the protocol doesn't carry reasoning at all (only reasoningTokens in usage), so it needs a transient event and a capability flag. That's the next one.

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
@oratis
oratis merged commit 3d27582 into main Aug 3, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant