Skip to content

Extract conversation switch foundations without activating them - #34384

Merged
brennanb2025 merged 1 commit into
mainfrom
brennanb2025/flip-switch-foundation
Oct 27, 2026
Merged

brennanb2025 merged 1 commit into
mainfrom
brennanb2025/flip-switch-foundation

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

This separates the existing transcript reader and record publication code, plus inactive switch definitions, from the chat/terminal switch tracked in #31595. No switch caller, handler or capability is installed by this PR, so users keep the existing chat and terminal behavior. The active feature stays together in #31595.

The cut contains 22 files and 1,777 production / 31 test-support lines added or removed. Independent review verified the exact cut, unchanged active reader/publication behavior and complete-tree conservation against the reviewed implementation and newer main changes. The normal commit hook, 144 targeted tests and hosted CI passed; the four root compiler projects and full unit shards actually executed.

Targeted ordinary flows passed with real Claude and Codex: two native-chat sends and replies; the existing copy/fork into a new CLI terminal with a reply while the source stays open; normal CLI exit followed by Session History adoption and another reply. Both isolated app stages stopped gracefully and all recorded owned process identities exited. These results cover the inactive foundation; the new in-place switch remains subject to #31595’s separate review and QA.

Reference comparison is complete for this cut: committed publication and one reader owner follow the common pattern; the inactive delivery cut is intentional.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

The changes add session-switch execution contracts, command lifecycle management, store admission hooks, and deadline-bound daemon preparations. The renderer gains a native chat read owner for history hydration, pagination, and transport lifecycle. Shared helpers and RPC schemas add lease adjudication, mutation and switch validation, terminal-surface identity capture, and captured session-tab placement.

Priority: Low

Merge Risk: Minimal · up to 04b6c

This change extracts switch command definitions and supporting helpers without activating any switch behavior, so existing chat and terminal behavior should be unchanged. The one open review note is a speculative edge case in unused tab-placement code. Merge once the author's pending CI and baseline runs pass.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv… Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check Warning The description explains the scope, rationale, behavior impact, testing, and relationship to issue #31595. However, it does not use the required template sections, does not provide an actual Linked Is… Rewrite the description using all required template headings. Add the actual linked issue reference in the Fixes # section, include N/A with a reason for Visual Proof if there is no visual change, complete the Testing checkboxes, and ad…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title "Extract conversation switch foundations without activating them" matches the changeset. The PR adds inactive switch command definitions, record-publication code, and transcript-reader code,…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "Extract conversation switch foundations without activating them" matches the changeset. The PR adds inactive switch command definitions, record-publication code, and transcript-reader code, with no switch caller, handler registration, or capability advertisement. The title is short, specific, and states the main point: the foundations are extracted but not activated.

Full details: Docstring Coverage

Explanation

Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 17 functions across 22 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Description check

Explanation

The description explains the scope, rationale, behavior impact, testing, and relationship to issue #31595. However, it does not use the required template sections, does not provide an actual Linked Issue entry such as Fixes #..., and does not include the required Visual Proof, AI Disclosure, Review, Notes, Agent skill upstream boundary, or Checklist sections.

Resolution

Rewrite the description using all required template headings. Add the actual linked issue reference in the Fixes # section, include N/A with a reason for Visual Proof if there is no visual change, complete the Testing checkboxes, and address AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist items.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

🔇 Additional comments: 1
src/shared/session-tab-placement.ts-68-73: 1

68-73: Minor · Functional Correctness · Quick win

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Preserve parent-group placement when the anchor retires.

If afterParentGroup is true and a split sibling survives, this branch ignores that option. For a capture of [left, right, trailing] after left, removing left makes this branch insert created before right. The existing parent-group rule places created after right. Capture the group boundary or retain the anchor’s parent identity so the missing-anchor path can apply the same rule. (raw.githubusercontent.com)


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1ba82a86-fdf1-4e3a-afb1-f7d0fab8b540
Commits

Reviewing files that changed from the base of the PR and between c2b4819 and 04b6cb2.

Files selected for processing: 22
  • src/main/daemon/daemon-pty-spawn-preparations.test.ts
  • src/main/daemon/daemon-pty-spawn-preparations.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-switch-command-selection.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-switch-commands.ts
  • src/main/runtime/agent-session-record-publication.ts
  • src/main/runtime/agent-session-record-store.ts
  • src/main/runtime/agent-session-store-transactions.ts
  • src/renderer/src/components/native-chat/structured-agent-session-read-owner-contract.ts
  • src/renderer/src/components/native-chat/structured-agent-session-read-owner-create.ts
  • src/renderer/src/components/native-chat/structured-agent-session-read-owner.ts
  • src/shared/agent-session-lease-adjudication.ts
  • src/shared/agent-session-native-content.ts
  • src/shared/agent-session-switch-execution.ts
  • src/shared/pty-fenced-termination.ts
  • src/shared/rpc-contract/structured-agent-session-mutation-envelope.ts
  • src/shared/rpc-contract/structured-agent-session-params.ts
  • src/shared/rpc-contract/structured-agent-session-question-params.ts
  • src/shared/rpc-contract/terminal-unary-params.ts
  • src/shared/runtime-rpc-envelope.ts
  • src/shared/session-tab-placement.ts
  • src/shared/terminal-tab-close.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

The larger switch change needs its existing reader and record publication code separated so it can be reviewed and validated on its own. This PR extracts those bodies and adds inactive definitions. It installs no new switch action and preserves the existing chat and terminal behavior; the active feature remains tracked in #31595.

Independent review found no open defects for this cut. The 22 selected file blobs match the reviewed implementation, and complete-tree reconstruction preserves the remaining change and newer main code. The common ownership/publication pattern was compared first-hand; the inactive delivery cut is intentional. No temporary behavior exclusions remain in this cut.

Validation on 04b6cb24e71619b59164e209b0f3548e1bce8746: the normal hook and all 144 tests in 11 named files passed. Hosted CI executed all four root compiler projects and the full unit shards, including the actual reader-owner and committed-publication tests. Real Claude and Codex each completed the three ordinary flows below. Captured identities prove normal CLI exit before adoption; both isolated app stages stopped gracefully and all recorded owned processes exited. The final source, evidence hashes and six original screenshots were independently audited.

For Brennan's review

These show the existing copy into a new terminal and History adoption into a new chat. The in-place switch in #31595 has its separate review and QA.

Verified flow Claude Codex
Native chat: two sends and real replies Claude native Codex native
New CLI terminal: inherited exchanges, real reply, original chat kept Claude cli Codex cli
After normal CLI exit: exact History adoption and another real reply Claude adopted Codex adopted

Three source boundaries: commit before publication, preserved reader owner, and uninstantiated switch factory.

@brennanb2025
brennanb2025 merged commit 4a39d18 into main Oct 27, 2026
54 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