feat(tui): add side conversation switching - #3760
Conversation
bb79332 to
1870897
Compare
Generated-by: Codex
1870897 to
36d4e4b
Compare
jackwener
left a comment
There was a problem hiding this comment.
NO-GO at 36d4e4b8a8a1e102588fb94d799c8522043a298e: 1 P1 and 1 P3.
Findings
-
[P1][normal user path] Restore the retained parent draft when Ctrl+C closes an empty side Composer.
Reproduction: open
/side, press Ctrl+/ to return to the parent, type an unsent parent draft, press Ctrl+/ back to the still-empty side, then press Ctrl+C. The second toggle stores the text inpair.parentDraftand clears the editor (packages/cli/src/pi-tui-runner.ts:1741-1742). Ctrl+C deliberately takes the side-close path (:3535-3546), butcloseSideConversation()adopts the parent and then discards the pair without restoringpair.parentDraft(:1842-1851). The parent therefore reappears with the side's empty editor state, permanently losing the user's only copy of the unsent text. Restore the parent draft after the parent switch succeeds and before clearing the pair, and add this exact close-path regression. The current draft test stops after toggling back into the side, so it cannot catch the loss. -
[P3][reasonable failure/reconnect path] Do not render observer transport exhaustion as the factual
main closedstate.RuntimeHostSessionChannel.onFailedmeans bounded subscription recovery was exhausted, not that the parent Session was archived. Nevertheless,runtime-host-session-driver.ts:729-733maps it toclosed, andpi-tui-runner.ts:1689-1699does the same when opening the observer fails. A live but unreachable parent is therefore indistinguishable from a genuinely archived parent. Preserve the previous/unknown state or introduce an explicit unavailable state, with tests for both initial-open failure and retry exhaustion.
Evidence and gates
- Reviewed head/base:
36d4e4b8a8a1e102588fb94d799c8522043a298e/e28ce6f515471d59b9063dd8a951db82e58f8fea. - The Host-backed observer, legacy and Kitty Ctrl+/ normalization, independent running Turns, stop-before-remove ordering, recoverable cleanup, navigation outside the pair, and creation-time settings copies otherwise follow the intended production paths.
- The exact-head hosted
testcheck completed successfully. - Current
mainwas19b47632be61b9116ae25d67e286f6a7878d6645; the synthetic merge was clean (tree6cda61a10e030463a1b886c660bfaadc54b013e0), completed the fullbuild:test, and passed 288/288 focused transcript/TUI/Host-driver/conversation-copy tests. This includes the newmainconversation-copy changes from #3806.
The P1 must be fixed before approval. Because this is a feature, the merge decision remains with maintainers after the blocker is closed and the new exact head is re-reviewed.
Summary
Completes TUI side conversations with retained switching and Host-backed parent status.
Ctrl+/switching for legacy and Kitty keyboard protocols.Side from main thread · Ctrl+/ to switch · Ctrl+C to closeCtrl+/ for sidemain …variantsFixes #3746
Verification
npm run lintnpm run format:checknpm run typechecknpm --workspace maka-agent run buildnpm --workspace maka-agent run test:dist— 463 passednpm run check:asf-headersnpx knip --workspace packages/cli— reports only the existingdev-cli.tsand five existing exported-type findings; no changed file is reportedRelevant behavior evidence:
Review focus
#3759 has merged, and this branch is rebased onto current main. The Files changed view now contains only the switching slice.
The background observer reuses the existing Runtime Host continuity channel. It does not introduce a second lifecycle authority or persist TUI projection state. Historical terminal Turns are suppressed so
main finishedonly describes a parent Turn observed running while the side was open.AI use
Select exactly one:
Tool(s) and scope: Codex implemented terminal key normalization, switching, draft isolation, Host status projection, cleanup integration, tests, and first-principles, adversarial, and simplification reviews.
Checklist
Does this PR entail a change in behavior?