Skip to content

feat(tui): add side conversation lifecycle - #3759

Merged
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:feat/3746-tui-side-conversations
Aug 25, 2026
Merged

feat(tui): add side conversation lifecycle#3759
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:feat/3746-tui-side-conversations

Conversation

@me2seeks

@me2seeks me2seeks commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds the Runtime Host-backed lifecycle for temporary TUI side conversations.

  • Adds the hidden /side [prompt] command.
  • Branches from the latest completed Host Turn and hides inherited parent history.
  • Keeps temporary side Sessions out of ordinary Session discovery.
  • Moves reusable Session-copy cleanup authority into storage, with startup and exit recovery.
  • Returns to the parent and removes the side Session when an empty Ctrl+C closes it.
  • Preserves Host ownership of Session, Turn, configuration, and interaction state.

This is the first of two stacked PRs. The follow-up adds retained Ctrl+/ switching and parent status projection.

Refs #3746

Verification

  • npm run lint
  • npm run format:check
  • npm run build
  • npm run typecheck
  • npm --workspace maka-agent run test:dist — 448 passed
  • npm --workspace @maka/storage test — 946 passed, 16 platform skips
  • npm --workspace @maka/desktop run build
  • npm run check:asf-headers

Relevant behavior evidence:

✔ /side submits in a temporary Session and one empty Ctrl-C closes it
✔ /side detaches from a running parent Turn without stopping it
✔ one empty Ctrl-C interrupts and closes a running side conversation

Review focus

The Runtime Host remains the lifecycle authority. The TUI retains only the current parent/side pair, while the shared storage cleanup lease provides recoverable removal after interruption or process exit.

AI use

Select exactly one:

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

Tool(s) and scope: Codex researched the existing Runtime Host and TUI seams, implemented the lifecycle and cleanup changes, added tests, and performed simplification and adversarial 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

@Astro-Han Astro-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 blocking issues.

[P2] Cleanup lease lacks Host identity — switching Hosts can delete the recovery lease

tui-session-copies is shared across all Hosts; switching from Host A to B causes B's recovery to remove A's side Session lease (lease has no Host ID), leaving the hidden side Session orphaned without a lease.

[P2] Side pair does not block identity-changing navigation

Normal /session//new//rewind can change the driver active Session while a side pair is open, but cleanup checks only the local pair. This can mistakenly stop the parent Session or hide descendants without an owning lease.

Fix: namespace leases by Host identity and make side open block identity-changing navigation (or atomically transfer ownership).

Checks on 572a5ca23a are test/audit/package: success.

简体中文存在清理租约与导航隔离两项阻断。

@me2seeks

Copy link
Copy Markdown
Contributor Author

Thanks for catching these. Addressed both blockers in b9e5b54ee.

  • Cleanup leases are now namespaced by the Runtime Host's stable rootId, so another Host sharing the same Client Data Root cannot consume or erase them. hostEpoch is intentionally not used because cleanup must survive an ordinary Host restart.
  • This lifecycle PR now blocks /session, /new, /rewind, and foreign-session import while a side conversation is open. The stacked switching PR (feat(tui): add side conversation switching #3760) replaces that temporary restriction with explicit pair-aware stop/remove when leaving the pair.

Validation:

  • npm run lint
  • npm run format:check
  • npm run typecheck
  • npm --workspace maka-agent run test:dist — 448 passed
  • npm run check:asf-headers

@me2seeks
me2seeks force-pushed the feat/3746-tui-side-conversations branch from b9e5b54 to 5cd23d6 Compare August 25, 2026 06:44
@me2seeks me2seeks closed this Aug 25, 2026
@me2seeks me2seeks reopened this Aug 25, 2026

@Astro-Han Astro-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.

Update on 5cd23d6880:

The two prior P2s are now closed. The cleanup lease is now namespaced by stable rootId (shared DB at <parent>/<rootId>/runtime.sqlite with strict hex check), and blockIdentityChangeWhileSideOpen now covers all identity-changing navigations.

Remaining P3 (non-blocking): tui:${process.pid} incarnation — PID reuse after crash can keep a stale lease alive indefinitely; consider a lock or start-token.

Checks on 5cd23d6880 are test/audit/package: success.

简体中文该头两项阻断已闭合,剩余 P3 为进程代次。

@me2seeks

Copy link
Copy Markdown
Contributor Author

Thanks for rechecking. Agreed on the remaining P3: a PID is a liveness hint, not a complete process-incarnation identity.

I’m keeping that hardening outside #3759 because a correct solution must be cross-platform and distinguish PID reuse without weakening concurrent-TUI protection. A random suffix alone would not be sufficient because another TUI could not validate its liveness. This PR remains scoped to the two blocking ownership and navigation fixes.

Preserve the side-conversation cleanup root identity while adopting the Runtime Host typed request API from main.

Generated-by: Codex
@me2seeks
me2seeks requested a review from Astro-Han August 25, 2026 14:15

@Astro-Han Astro-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.

Update on 0444059d:

Freshness from 5cd23d6: pure merge main (parent + 475d54), P2 fixes intact; host-boot import relocation preserved, queue-test wrapper removal aligns with main typed-request refactor; no new P0-P3, prior P1/P2 remain closed.

Checks on 0444059d242c are SUCCESS and current for this head.

简体中文该头为纯合并漂移,无新增阻断。

@me2seeks
me2seeks requested a review from Astro-Han August 25, 2026 14:47

@Astro-Han Astro-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 — 0444059, no P0-P2, hosted checks SUCCESS per human decision.

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.

2 participants