Skip to content

Save runtime-started tabs in the tab bar before their terminal starts - #27083

Open
Jinwoo-H wants to merge 4 commits into
mainfrom
layout-pr4-start-queue
Open

Jinwoo-H wants to merge 4 commits into
mainfrom
layout-pr4-start-queue

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 22 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​358 $\color{#cf222e}{\Huge{\mathbf{−}}}$​169 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​189
Prod 27 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​410 $\color{#cf222e}{\Huge{\mathbf{−}}}$​123 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​287

ELI5

When something other than the desktop window opens a terminal (the orca CLI, an orchestration worker, a phone with no desktop window open, or worktree setup), Orca now saves the new tab in the tab bar and its tab group before the terminal process starts. It used to save only a bare terminal record after the process started, and never the tab-bar entry or group, so on a headless orca serve the saved tab bar was simply missing.

What Changed

Before: a runtime-started terminal spawned first. When it reported back, the binding step "minted" a minimal terminal row (or grafted a new pane as a vertical split) into the saved session. It never wrote a tab-bar entry or group. Headless servers therefore saved terminal rows with no tab bar, and later headless closes and group moves saved partial or mismatched tab bars.

After: one runtime function, spawnInAdmittedPane (src/main/runtime/runtime-pane-admission.ts), handles every runtime-originated create and split:

  1. It writes the pane, its tab-bar entry and its group in one store commit through the merged layout module: Loader → createTerminalTab / splitPane → Serializer (terminal-pane-admission.ts, committed by the terminal-topology commit module).
  2. It then spawns. The binding uses mayCreate: false, so it never mints a tab or grafts a pane. If the pane was closed or moved while the terminal started, the bind is refused and that process is discarded.
  3. If the start fails, or an agent-session resume attaches to a session already running in another pane, it removes the pane it wrote. The caller sees today's outcome: an error, and no extra tab.

Pane state in this PR is starting only. failed and waiting_for_host arrive with layout.startPane and published pane status in the switch (PR 6), where a view can draw them. Old RPC methods keep their contracts: terminal.create still replies with the ptyId once the process runs.

Council item J (start-attempt identity) is not in this PR. No flow in PR 4 starts the same pane twice; close and move while starting are covered by the refused bind and tested. J lands with layout.startPane in PR 6.

Tab group. A tab the phone opens is written into the group the phone asked for (targetGroupId). Before, the write put it in the first group, so on a headless server with two groups the phone's tab moved to the first group after a server restart. A tab opened with no group named (CLI, orchestration) still goes to the first group, as on main and in what clients see live. A split stays in its parent's group.

The headless group writer now also keeps the tab bar consistent: each entry's group comes from the group that lists it, and a reorder writes the group order. It is marked as deleted by the switch.

Deleted: hostAdmittedMembership, persistHeadlessTerminalSplit, persistHeadlessTerminalTabOrder, the phone create's after-spawn view-mode write, and main's three pendingActivationSpawn stamps (renderer hydration always re-sets that flag, so the saved one was never read). The mint and graft in applyPtyBinding stay: window-originated spawns still rely on them until PR 6. The Loader keeps its row.pendingActivationSpawn rule, because profiles saved by older builds still carry the flag.

Why

This is design row 4, and the first rework PR that changes behaviour. Writing the pane first through the same model and Serializer the runtime will own after the switch removes the "minimal tab mint" class of half-saved tabs, without a second write path. The alternative, teaching the binding step to also invent tab-bar entries and groups, would add another hand-written copy of tab order. The rework exists to remove those copies.

Linked Issue

Workspace tab layout rework, PR 4 (design row 4).

Visual Proof

N/A. A new tab from the CLI, a phone or setup looks exactly as before. The change is in what is saved, and the layout oracle measures that.

Testing

Layout oracle in record mode, local macOS; the SSH lane ran against a Docker host. Specs were run on the base commit and on this branch.

Spec base branch
workspace-layout-oracle-headless (orcad and Electron; 16 runs on base, 18 on the branch with the new group scenario) 0 unexpected 0 unexpected
workspace-layout-oracle (window, 20) 0 unexpected 0 unexpected
workspace-layout-oracle-ssh (2) 0 unexpected 0 unexpected

Known-on-main entries:

  • Removed (now fixed):
    • headless tab_bar_missing
    • headless tab_lists_disagree (close saved a partial tab bar)
    • group_lists_missing_tab for split and move, on orcad and Electron
  • Added: headless move-tab-between-groups tab_order_disagrees. A headless reorder saves rows through the session merge door, which keeps the stored row order. This already happened before this PR, and the order a client sees after the reorder and across a restart is the same; the oracle's group-order and restart checks pass on both. It was hidden because the rules stopped at "no tab bar". It is fixed in PR 6 together with client-reorder-tabs.
  • Unchanged: the orcad rename-after-restart entry. That bug is not addressed here.

Unit tests:

  • New and rewritten tests: runtime-admitted tabs survive a stale window save; they are written with their tab-bar entry and group; a pane closed, or moved to another tab, while its terminal starts is not bound; a projected-only split source is refused before spawning; a failed split takes its pane back.

  • A phone create into the second group stays there, and a CLI create stays in the first group, across a cold restart and in the desktop window (unit test, a new headless oracle scenario on orcad and Electron, and a before/after run against the real app).

  • The seeded topology model test passes at 200 seeds, with the new write racing stale window saves.

  • pnpm tc, pnpm lint and check:code-quality:changed pass.

  • The full src/main and src/shared run fails only the live-shell tests (zsh, fish, [Bug]: An exec in user rc files skips the wrapper's ZDOTDIR restore — shell-ready marker never emitted, ~15s added to every spawn #13767), which fail the same way on the base commit.

  • I manually tested these changes locally

  • Automated tests added/updated

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • SSH: a relay-fallback host's pane is written in its ssh:<target> partition, the same partition its binding lands in.
  • Folder workspaces: they use the same path; the workspace key is the session key.
  • Windows: nothing platform-specific.
  • Mixed versions: the saved format is unchanged, so older builds read the same documents.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Jinwoo-H and others added 2 commits October 9, 2026 21:29
… they spawn

Co-Authored-By: Claude <noreply@anthropic.com>
…roup writer for the switch

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cdbf42b2-7a58-43d2-a9c4-1a8bff3ada91


📥 Commits

Reviewing files that changed from the base of the PR and between 13e6a9e and 0c48d14.



📒 Files selected for processing (6)
  • src/main/runtime/orca-runtime-create-terminal.ts
  • src/main/runtime/orca-runtime-tests/mobile-session-tabs-cold-serve-hydrate.spec.ts
  • src/main/runtime/runtime-terminal-contracts.ts
  • src/main/runtime/runtime-terminal-spawn-placement.ts
  • tests/e2e/workspace-layout-oracle-headless.spec.ts
  • tests/e2e/workspace-layout-oracle-known-on-main.ts


🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/runtime/runtime-terminal-contracts.ts


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




📝 Walkthrough
📝 Walkthrough

Walkthrough

The change adds durable terminal-pane admission and withdrawal operations and routes runtime terminal creation and splits through pane admission before PTY spawn. PTY binding no longer grants membership authority or creates a missing pane. The change also updates headless tab ordering and removes several separate persistence paths and pending-activation fields. Tests cover admission, stale snapshots, spawn cleanup, and pane changes during startup.



Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 0c48d

Runtime-created terminals now join the requested tab group, or the first group when none is given. Tests cover persistence across restart. No merge-blocking risk was found in the supplied changes.

Pre-merge checks | Passed 3 | Failed 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 34.21% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 47 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check Warning The description thoroughly explains the change, rationale, testing, compatibility considerations, and checklist items. However, the Linked Issue section does not provide an actual issue link or a comp… Add the actual linked issue URL or complete the Fixes # reference with the issue number. Keep the existing explanation and testing details unchanged unless further corrections are needed.
✅ Passed checks (3 passed)
Check name Status Explanation
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.
Title check Passed The title clearly and concisely describes the primary change: saving runtime-started tabs before the terminal process starts.

Full details: Description check

Explanation

The description thoroughly explains the change, rationale, testing, compatibility considerations, and checklist items. However, the Linked Issue section does not provide an actual issue link or a completed Fixes # reference, despite the template requiring one.


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR





🛠️ Fix failing CI checks 💡
  • 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.

Jinwoo-H and others added 2 commits October 10, 2026 19:40
A phone, CLI or orchestration create on a headless host with two tab groups was written into the
first group, so after a restart the tab moved groups. The phone's targetGroupId, else the host's
focused group, now reaches the pane admission.

Co-Authored-By: Claude <noreply@anthropic.com>
Defaulting to the host's focused group moved a CLI-created tab to the right group after a restart
and in the desktop window, while main and the live view keep it in the first group. Only the
phone's explicit targetGroupId now picks the group.

Co-Authored-By: Claude <noreply@anthropic.com>

This branch has not been deployed

No deployments
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