fix(desktop): open side conversations without a settled turn - #4508
fix(desktop): open side conversations without a settled turn#4508liuxiaocs7 wants to merge 2 commits into
Conversation
4dd22a0 to
d1f5b17
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Read d1f5b17f against main 72eb982d (+1135/−504, 32 files: production +405/−147, tests +704/−338, docs +26/−19). The problem is real and the history makes it stronger than the body says: #1375 shipped side conversations with an empty-context api.create branch, and #2398 removed it because the new idempotent copy lease needed a boundary string, not to protect anything. The comment in quote-companion-core.ts ("if no completed turn exists, the side conversation starts empty") kept describing #1375 while the code hard-failed, which is the evidence. The invariant that does matter, never copy from a mid-flight Turn, still lives in latestSettledTurnId, and {kind:'empty'} copies nothing, so nothing is weakened. Routing the empty fork through the existing copy authority is the right seam: SessionCreateInput has no parentSessionId, so a create-based path would also need an epoch and would reopen the second creation path #2398 closed. The epoch is needed. Fingerprint sentinel, lineage invariant (empty only with intent: side_conversation and no branchOfTurnId), forward migration of headers and SQLite leases, and lease replay all check out, and the permission path in use-quote-companion.ts:743-769 fails closed. Core 766/766, the four runtime-host files, the five desktop suites, typecheck and Biome all green locally.
Before writing this up I tried the shape on a scratch branch, because the numbers looked wrong for what the PR does: +1135/−504 for "open a side chat before the first Turn settles". Three things account for it, in order of size.
1. The copyPoint discriminated union is a second authority, and everything built for it goes with it. The PR already decides "empty" by intent === 'side_conversation' in two places: the decoder at session-revision.ts:185 and the lineage invariant at session-store.ts:1515. The kind discriminator is a third representation of the same fact, and it is the reason for two copies of SessionConversationCopyPoint (core and protocol), the throughTurn / isSessionConversationCopyPoint / sameConversationCopyPoint helpers, decodeSessionConversationCopyPoint, the forward migration of persisted headers (session-store.ts:1354), the SQLite lease migration (session-copy-cleanup.ts:488), the '\0empty' fingerprint sentinel, two now-dead "requires a turn boundary" throws in the coordinator, and a mechanical rename across seven test files. Keeping sourceTurnId?: string optional instead, with absence meaning empty, needs none of that: old headers and old leases are valid as they are, no rewrite on disk, through-turn and revision fingerprints stay byte-identical to what is shipped (JSON.stringify already renders the missing element as null, so empty gets its own fingerprint for free), and the three tests that only exist to prove the migration and the malformed-shape rejection disappear with the shape. On the scratch branch that is −350 lines with every suite green. The epoch is still needed: a new client sends the request without sourceTurnId and an old Host's required-field check rejects it, so the handshake boundary stands; but the epoch note's sentence about "conversation-copy identity an older client cannot decode" is not true either way, conversationCopy never crosses the protocol.
2. The preparing pipeline is dead after this change and was left in the tree. ensureFork has one caller left (use-quote-companion.ts:737) and it passes showPreparing: false, so preparing is always false, yet use-workbar-controller.ts:298 still creates the tab with preparingOnCreate = true, which paints one busy frame (no close key, drag disabled) before the panel's passive effect clears it, and contradicts the new sentence at docs/side-conversation.md:118. The whole chain rides on that one value: quoteCompanion.preparing copy (zero consumers already), .maka-quote-companion-preparing css, showPreparing / setPreparing, the preparing field through use-side-conversation-workspace, use-workbar-controller, workbar-host and workbar-surface, and the entire busy-tab mechanism in workbar-surface.tsx (busyTabIds, useSortable({disabled}), aria-busy, the conditional close button). The body's own doc change retires the eager-loading contract, so nothing is left for it to serve; removing it is part of this change, −101 lines, and it makes the −504 honest. Same family: BranchFromTurnInput keeps sourceTurnId? beside copyPoint? so {} is well-typed, with one producer left at app-shell-turn-actions.ts:103; the CLI at runtime-host-session-driver.ts:848 still throws "requires at least one completed Turn", so desktop and TUI now disagree, and passing empty there needs no Host change; and protocol.test.ts:422 adds a 34th epoch > N assertion the largest one already implies.
3. Lazy fork and empty fork are two independent changes bundled together. Their code does not overlap: lazy alone is −67 net with zero protocol change and no epoch; empty alone is +167 net, all in packages plus quote-companion-core.ts. All of the −504 is lazy (the eager mount effect, the two-second forkRetryPending poll, the submitLocked mirror, the Spinner overlay are compensations for "must have a settled Turn at mount", which lazy removes and empty does not), and lazy is also what brings the staged-permission machinery (~55 production, ~60 test). They can ship in one PR, but as two commits, and the bundling is why the net reads worse than either half.
Tests: no fixture matrix, but by mutation two guards have no test (copyCurrent's empty condition, the todo.query assertion in the UDS test is empty because the fixture never has an in-progress Todo; and the core lineage branch, which is the second copy of the decoder rule and can go), and the empty-fork obligation is proven twice (quote-companion-disposal and quote-companion-retry), so one of those can go. Nothing in e2e opens a side chat during the first Turn; the hook and Host layers cover it, which is enough, but the body's "tests fail without it" should not imply e2e.
With 1 and 2 the PR lands around +700/−540, net +160 instead of +630, with one authority for the copy boundary, no on-disk rewrite, and no dead UI state machine. That is the shape I would like to see before merging.
Merge-order note: #4386, #4308, #4439 and #4500 also claim 95; whoever lands later renumbers. And the screenshots are a stated mock-up; this changes what users see (composer enabled at once, no overlay, tab behaviour), so real before/after captures from the same stories and viewport, light and dark, or a short recording, once the preparing frame is gone.
Evidence boundary: static read of d1f5b17f; core, runtime-host and desktop suites, typecheck and Biome run locally; Playwright, Storybook and a live window not exercised.
AI-assisted review: drafted with Maka; I verified the #1375/#2398 history, the upsertPanel(panel, true) path, the CLI throw and the lease replay myself.
简体中文
问题成立且历史更有力:#1375 本来支持空上下文,#2398 为幂等租约删掉了那条分支。走 copy 权威是对的接缝,epoch 必要。但体量不对,我在实验分支上试过:一,copyPoint 判别联合是第二份权威,intent === 'side_conversation' 在 decoder 和血缘不变量里已经判别了 empty,kind 派生出两份类型、三个 helper、decoder、header 与租约两条迁移、指纹哨兵和七个测试文件的改名,改成可选 sourceTurnId 全部不需要,旧数据原样有效,−350 行全绿,epoch 仍要 bump 但注释里那句 client 解不了 conversationCopy 不成立;二,preparing 管道在本 PR 之后恒为 false 却整条留着,use-workbar-controller.ts:298 仍以 preparing 建 tab,与文档矛盾,连同 busy-tab 机制一起删 −101 行;CLI 仍要求已完成 Turn,与 desktop 不一致。三,lazy 和 empty 代码零重叠,−504 全是 lazy 的,至少拆成两个 commit。这两步之后 PR 约 +700/−540,净 +160。
5be48e3 to
e3ebd40
Compare
Opening the Desktop side conversation while the main session's first turn was still running failed with "无法创建侧边对话,请稍后重试。": the panel forked eagerly at mount and only from the latest completed turn, of which there is none during the first turn. The composer is now usable immediately (no eager fork, no preparing block), and the fork is created lazily on the first send: through the latest settled turn if one exists, else with an empty context. An empty copy inherits the source model/cwd/permission and records parentSessionId, but copies no messages, no runtime runs/events, no in-progress Todo, and fabricates no branchOfTurnId lineage. The conversation-copy boundary keeps its `sourceTurnId`, now optional: absent means an empty copy, accepted only for the side_conversation intent (enforced at the protocol decoder and the persistence lineage invariant). Threaded through core, storage, the Runtime Host protocol/coordinator, the desktop IPC/renderer, and the CLI (which now also opens an empty side conversation instead of erroring). RUNTIME_HOST_COMPATIBILITY_EPOCH is bumped for the optional-field wire shape; session.revision.create still requires it, so its shape and fingerprint are unchanged. The now-dead "preparing"/busy-tab pipeline (loading frame, aria-busy, drag/close disabling) is removed since the composer is live at once. Fixes apache#4507 Generated-by: Claude Code
e3ebd40 to
b661209
Compare
…copy Follow-up hygiene on the lazy empty side-conversation fork: - Add a direct normalizeSessionHeader test for the empty-copy storage lineage invariant: an empty side-conversation copy (absent sourceTurnId and branchOfTurnId) is accepted, while a fabricated branchOfTurnId, a missing side_conversation intent, and an unanchored through-turn copy are all rejected. This uses the previously-unused normalizeSessionHeader / SessionConversationCopy / SessionHeader imports. - Add a CLI test that opening a side conversation while the parent's first turn is still running forks an empty copy: session.branch.create omits sourceTurnId and keeps intent side_conversation, matching the desktop. - Remove the now-dead quoteCompanion.defaultName / preparing locale keys (and their stale eager-fork JSDoc) left after the preparing pipeline was removed; the empty tab title already falls back to sideChat/sideChatNumbered. Generated-by: Claude Code
Summary
Opening the Desktop side conversation (quote-companion side chat) while the main session's first turn was still running failed with the banner "无法创建侧边对话,请稍后重试。" and stayed unusable until that turn settled. The panel forked the main session eagerly at mount and only from the latest completed turn (
latestSettledTurnId); during the first turn there is no completed turn, soensureCompanionForkreturnedfork_setup_failed.This makes the side chat behave like comparable tools (Codex
/side, Cursor branch): it snapshots whatever context is settled and is never blocked by the main session's state.parentSessionId, but copies no messages, no runtime runs/events, no in-progress Todo, and fabricates nobranchOfTurnIdlineage — so it never leaks a mid-flight turn's partial state.The conversation-copy boundary keeps its
sourceTurnId, now optional: an absentsourceTurnIdmeans an empty copy, accepted only for theside_conversationintent (enforced at the Runtime Host protocol decoder and the persistence lineage invariant). This is threaded through core, storage (the lineage invariant + the recoverable cleanup lease), the Runtime Host protocol/coordinator, the desktop IPC/renderer, and the CLI — which now also opens an empty side conversation instead of erroring, matching the desktop.RUNTIME_HOST_COMPATIBILITY_EPOCHis bumped for the optional-field wire shape;session.revision.createstill requiressourceTurnId, so its wire shape and fingerprint are unchanged.The now-dead "preparing" / busy-tab pipeline (the loading frame,
aria-busy, drag/close disabling on a not-yet-ready tab) is removed, since the composer is live from the first frame.Fixes #4507
Screenshots
Opening the side chat while the main session's first turn is still running (simulated mockup, not a live capture):
Verification
Ran locally (all green):
@maka/core,@maka/storage,@maka/runtime-host(protocol epoch floor, coordinator two-client UDS incl. an empty side conversation end-to-end with an in-progress source Todo the empty copy must not inherit, revision),maka-agent(CLI), and the desktop main suites (quote-companion-retry,quote-companion-disposal,permission-response-ipc-boundary,runtime-host-session-execution-ipc-main,workbar-controller,workbar-services-adapter); desktoptscfor main / renderer / preload; the renderer-architecture ratchet against main; the Astryx surface inventory check; and Biome.Review focus
Runtime Host protocol/persistence change (hence the epoch bump). Backward compatibility and safety:
sourceTurnId, which is the current shape — no on-disk migration or rewrite is needed; existing sessions load and in-flight copies recover unchanged. Through-turn and revision fingerprints stay byte-identical to a requiredsourceTurnId(an absent one serializes tonull, giving an empty copy its own identity).sourceTurnId) is accepted only for theside_conversationintent, at both the protocol decoder and the persistence invariant.isValidConversationCopyLineageaccepts an empty branch (parentSessionIdset,branchOfTurnIdabsent) while still rejecting a through-turn branch whosebranchOfTurnIdis missing/mismatched; revisions remain through-turn-only.AI use
Select exactly one:
Tool(s) and scope: Claude Code (Anthropic) — implemented the change across the protocol/core/storage/desktop/CLI layers and the tests under human direction and review. The commit carries a
Generated-by: Claude Codetrailer.Checklist
Does this PR entail a change in behavior?