Repository navigation
Conversation
…eport-only The window now says where each fresh terminal goes: the first pane of a tab sends new-tab with the tab's creation fields, a split sends the parent leaf, direction, ratio and the tree after the split, background launches send the same, and a Codex restart that rebuilds a rootless layout sends root. A reattach sends nothing. Main still only records whether placement names the tab its binding write picks; its 'absent' value now counts only new leaves without placement, so it reads the fallback mint and graft directly. Extends the shared placement type with optional row, ratio and proposedRoot, each dropped alone if malformed, so older and newer peers keep working.
…he tab lacks The adopted tab is created without launchAgent, so a row naming one would diverge from the renderer's tab once main applies placement.
…order wrapInSplit now places the new pane first itself, so the pane-created handler's proposedRoot sees the real post-split tree instead of the order before the subtree split moved it.
Core step C1 of 7. When a new terminal's binding names a tab main does not have yet, main builds the row from the creation fields the window sent; when it names a split whose parent is in the tab, main takes the window's tree (holding exactly today's leaves plus the new one) or splits at the parent with the given direction and ratio. Spawns without placement, or whose placement doesn't fit the tab's current shape, keep today's mint and root graft. The headless split reuses the same split-at-leaf op, and the agreement span now reports exactly when placement was applied.
…to topo-c1-placement-applies
This was referenced Oct 7, 2026
Closed
…'s row Main stamped the spawn's cwd on every tab it built from placement. Once main's copy is authoritative that would add startupCwd to rows the window never gave one.
5 tasks done
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 15 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (7)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughPTY binding persistence now applies placement details when they agree with the current session shape. New-tab row fields and split layouts are applied through placement helpers. Headless terminal layout updates also use the shared split insertion helper. The agreement check no longer converts thrown errors into a Priority: ⬇️ Low Merge Risk: 🔵 Low · up to A split started during another layout change can restore an older pane arrangement. The pane sessions remain intact, but the layout may need to be arranged again. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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. Comment |
…lies # Conflicts: # src/main/runtime/headless-terminal-split-layout.ts
10 of 12 tasks
Contributor
Author
|
Closing: the layout rework is being redesigned so the runtime holds the only copy of the layout and every window/client is a view. This branch is kept for reference. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
When the app starts a new terminal, the window already tells the main process where it belongs: a new tab (with its title, color and so on), or a split next to a given pane (with direction, size and the full tree after the split). Until now main only logged that and kept guessing: it made a tab called "Terminal N" and stuck every new split pane to the right of the whole tab. Now main uses what the window told it.
This is core step C1 of 7 in the terminal layout refactor. The core steps are reviewed as separate stacked PRs and merged together as one unit. Base is #26078 (B1-5, the window side); it is retargeted to
mainwhen that merges.What Changed
What users see: nothing. The window's own save still overwrites main's copy a moment later with the same tab and the same tree, so what you see and what gets saved don't change. (The parity check below confirms this.) The only time main's copy shows is when the app is killed between the terminal starting and the window's next save. In that case a restart now brings back the real tab title and the real split shape, not "Terminal N" and a split on the right. That's the "nested before/after splits restore the wrong shape" case.
How it works, in
applyPtyBinding, for a pane main doesn't know yet:pendingActivationSpawn: true, sort order and ids are unchanged.The decision is the existing
terminalPanePlacementAgreementcheck: placement is applied only when it returnsagrees, so thebinding.placementspan attribute now reads as "applied" vs "fell back". Since the write now depends on that check, I removed the report-onlytry/catcharound it (and itscheck_threwvalue). Main's own headless split reuses the same split-at-pane function (splitLayoutLeaf) instead of its private copy.Why
Later core steps need main's tree to match the window's. C4 applies geometry from main, and C6 needs a local split to cause no rebuild after the reply. A parent pane plus a direction can't describe a "before" split or a split around a subtree, so main takes the window's tree, but only after checking it holds exactly the expected panes. Otherwise it falls back to splitting the parent. I chose not to add a new resolver: the existing agreement check already sorted placements into "fits" and "doesn't fit".
Wire compatibility: every placement field stays optional. Old clients send none and keep today's behavior. A malformed field is dropped on parse, and a malformed placement as a whole is ignored.
Linked Issue
Part of the terminal layout refactor (STA-9417 core). No separate issue.
Visual Proof
N/A: nothing changes on screen. The window's save makes the saved state identical to main's.
Testing
ORCA_BACKGROUND_LAUNCH=1 pnpm run test:e2e:terminal-layout-parity -- --base origin/main --head HEADpassed with no differences (base385fe8ab4ee, all 9 scenarios on both builds).terminal-pane-placement-apply.test.ts(21 cases):pty-binding-placement-inertness.test.ts:ssh:copypnpm tcandpnpm run check:code-quality:changedpass. Allsrc/mainandsrc/sharedunit tests pass, apart from 45 browser, daemon and fish-shell tests that fail the same way on the base. The test(terminal): pin layout invariants the mirror refactor must keep #26073 gate tests also pass on this branch.Review
Agent skill upstream boundary
Notes
ssh:for SSH spawns). Tested.Checklist