Skip to content

feat(tui): add side conversation switching - #3760

Merged
Astro-Han merged 2 commits into
apache:mainfrom
me2seeks:feat/3746-tui-side-toggle
Aug 26, 2026
Merged

feat(tui): add side conversation switching#3760
Astro-Han merged 2 commits into
apache:mainfrom
me2seeks:feat/3746-tui-side-toggle

Conversation

@me2seeks

@me2seeksme2seeks commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Completes TUI side conversations with retained switching and Host-backed parent status.

  • Adds terminal-compatible Ctrl+/ switching for legacy and Kitty keyboard protocols.
  • Preserves independent unsent drafts for the parent and side views.
  • Keeps running Turns alive when switching views.
  • Projects the parent Session’s authoritative Host state while the side is visible.
  • Uses English-only status copy, including:
    • Side from main thread · Ctrl+/ to switch · Ctrl+C to close
    • Ctrl+/ for side
    • actionable and terminal main … variants
  • Closes and recoverably removes the retained side when navigating outside the pair.
  • Stops an active side Turn before removal when the pair is discarded.
  • Keeps model, permission, thinking, and orchestration settings as creation-time copies that evolve independently afterward.

Fixes#3746

Verification

  • npm run lint
  • npm run format:check
  • npm run typecheck
  • npm --workspace maka-agent run build
  • npm --workspace maka-agent run test:dist — 466 passed
  • npm run check:asf-headers
  • npx knip --workspace packages/cli — reports only the existing dev-cli.ts and five existing exported-type findings; no changed file is reported

Relevant behavior evidence:

✔ Ctrl+/ toggles side views, preserves drafts, and projects parent status in English
✔ Ctrl+/ detaches from a running side Turn without stopping it
✔ /session outside the pair discards its retained side conversation
✔ observes actionable and terminal parent status from the Host projection
✔ renders side conversation status in English for every UI locale

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 finished only describes a parent Turn observed running while the side was open.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

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

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@me2seeks
me2seeksforce-pushed the feat/3746-tui-side-toggle branch from bb79332 to 1870897CompareAugust 25, 2026 06:07
@me2seeks
me2seeksforce-pushed the feat/3746-tui-side-toggle branch from 1870897 to 36d4e4bCompareAugust 25, 2026 16:06
@me2seeks
me2seeks marked this pull request as ready for review August 25, 2026 16:17

@jackwenerjackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NO-GO at 36d4e4b8a8a1e102588fb94d799c8522043a298e: 1 P1 and 1 P3.

Findings

  1. [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 in pair.parentDraft and clears the editor (packages/cli/src/pi-tui-runner.ts:1741-1742). Ctrl+C deliberately takes the side-close path (:3535-3546), but closeSideConversation() adopts the parent and then discards the pair without restoring pair.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.

  2. [P3][reasonable failure/reconnect path] Do not render observer transport exhaustion as the factual main closed state.

    RuntimeHostSessionChannel.onFailed means bounded subscription recovery was exhausted, not that the parent Session was archived. Nevertheless, runtime-host-session-driver.ts:729-733 maps it to closed, and pi-tui-runner.ts:1689-1699 does 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 test check completed successfully.
  • Current main was 19b47632be61b9116ae25d67e286f6a7878d6645; the synthetic merge was clean (tree 6cda61a10e030463a1b886c660bfaadc54b013e0), completed the full build:test, and passed 288/288 focused transcript/TUI/Host-driver/conversation-copy tests. This includes the new main conversation-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.

@me2seeks
me2seeksforce-pushed the feat/3746-tui-side-toggle branch from 36d4e4b to 69a941aCompareAugust 26, 2026 04:26
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Addressed both review findings in 69a941a9b:

  • Restores the retained parent draft after the parent switch succeeds and before the side pair is discarded. The regression covers /sideCtrl+/ → type parent draft → Ctrl+/ → empty-side Ctrl+C.
  • Treats parent observer open failure and exhausted transport recovery as unknown status instead of the factual closed state. A genuinely archived parent still renders main closed; regressions cover both initial-open failure and retry exhaustion.

Validation: root build; CLI tests 466/466; lint; format; ASF headers; git diff --check.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found no blocking issues.

Implements side conversation switching with preserved drafts, isolated model/permission state, and independent Host observer channel; hosted test: SUCCESS (32930257469).

No P0-P3.

简体中文该头无阻断。

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@M4n5ter
M4n5terforce-pushed the feat/3746-tui-side-toggle branch from 69a941a to 62bb17cCompareAugust 26, 2026 10:01

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — 62bb17c, rebase-only, no P0-P3 carries over, per me2seeks pre-auth.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@Astro-Han
Astro-Han merged commit f2ddc83 into apache:mainAug 26, 2026
2 checks passed
@M4n5ter

Copy link
Copy Markdown
Member

[P2] Preserve the editor's undo shortcut when no side conversation exists.

The raw input listener currently consumes every byte matched by matchesSideConversationToggle() before the editor receives it (packages/cli/src/pi-tui-runner.ts:3595-3598). On legacy terminals, Ctrl+/, Ctrl+_, and the editor's default undo binding Ctrl+- all arrive as 0x1f. The helper therefore treats the editor's undo key as a side-view toggle even when sideConversation is undefined; toggleSideConversation() returns immediately, but the event has already been consumed.

A probe against the shipped @earendil-works/pi-tui 0.84.2 editor confirmed the path: after typing draft, delivering 0x1f to the editor restores the previous empty snapshot, while the new listener classifies and consumes that same byte first. This affects ordinary TUI editing for every legacy-terminal user, not only users who open /side. The current side-switching tests send 0x1f only after a pair exists, so they do not cover the regression.

Please intercept the ambiguous legacy byte only while a side pair exists. Outside that state it must fall through to the editor. Add a regression that types a normal draft without opening /side, sends 0x1f, and verifies that undo still works. While a side pair is active, the legacy encoding cannot distinguish the two shortcuts, so retaining the documented toggle there is a reasonable explicit tradeoff.

简体中文

[P2] 没有 side conversation 时,应保留编辑器的撤销快捷键。

当前原始输入监听器会在编辑器收到按键前,无条件吞掉 matchesSideConversationToggle() 匹配的字节(packages/cli/src/pi-tui-runner.ts:3595-3598)。在传统终端中,Ctrl+/Ctrl+_ 和编辑器默认的撤销键 Ctrl+- 都编码成 0x1f。因此,即使 sideConversation 不存在,撤销键也会被当成 side view 切换键;toggleSideConversation() 随后直接返回,但按键已经丢失。

使用项目当前的 @earendil-works/pi-tui 0.84.2 做探针验证:输入 draft 后,把 0x1f 交给编辑器会恢复之前的空快照;新增监听器却会先识别并吞掉同一字节。该回归影响所有使用传统终端的普通 TUI 编辑场景,并不只影响打开 /side 的用户。现有测试只在 side pair 已存在时发送 0x1f,因此无法发现此问题。

请仅在 side pair 存在时拦截这个含义有歧义的传统终端字节;其他状态应让它继续传给编辑器。请补一个回归测试:不打开 /side,输入普通草稿后发送 0x1f,确认撤销仍生效。side pair 已打开时,传统编码无法区分两个快捷键,继续保留已公开的切换行为是合理且明确的取舍。


Posted by an automated review agent operated by @M4n5ter. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.

简体中文

本条评论由 @M4n5ter 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。

@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Fixed in follow-up #3929. The ambiguous legacy byte now falls through to the editor until a side pair exists; the new regression proves Ctrl+- undo without /side, while the existing active-side toggle coverage remains.

Posted by Codex on behalf of @me2seeks.

Sign up for freeto 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.

feat(tui): add Codex-style side conversations

4 participants

@me2seeks@M4n5ter@jackwener@Astro-Han