Skip to content

Keep closed browser, editor, and proven terminal tabs out of the host list - #27565

Closed
brennanb2025 wants to merge 4 commits into
mainfrom
brennanb2025/tab-close-host-commit
Closed

brennanb2025 wants to merge 4 commits into
mainfrom
brennanb2025/tab-close-host-commit

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

A tab closed from a phone or paired desktop could come back because the background process still included it in the next tab list. After an accepted browser/editor close, or a terminal close with enough recorded evidence, that process now removes the tab before answering. Other clients receive the updated list.

What Changed

Previously, a filter hid completed closes for clients while the underlying list could still name the tab. The filter applied to every already registered client with state for that workspace, including bystanders.

The host, the background process publishing workspace tabs, now updates its own list after the window accepts a browser or editor close. For a whole terminal tab, it first records the exact identity of every pane. Those records let paired desktops recognize intentional retirement without treating a missing row as evidence that a remote process died. The host removes the terminal only when every listed pane has matching evidence; a declined close removes nothing.

The existing removal functions repair groups, split layouts, active selection, and recent-tab history while preserving survivor order. Terminal removal now recognizes both supported browser identifiers, preventing an unrelated browser group from collapsing. Existing chat placement is preserved while a close is in progress.

Review also corrected a refusal response: when rejecting an absent-surface close republishes its live parent, the existing optional republication marker is now truthful. A refusal that sends no replacement list leaves the marker absent.

Why

Updating the published source list makes a completed close consistent for readers that do not use the client filter. Reusing the existing terminal evidence preserves execution ownership and mixed-version recovery without adding a durable closing flag or a new protocol.

Remaining difference Classification Reason / follow-up
Keep the existing filter when an accepted terminal close has incomplete evidence Temporary The underlying rows must remain until the window publishes or every pane has matching evidence. Follow-up: make the window acknowledgement establish published omission or complete exact evidence; no ticket exists yet.
Keep filtering individual panes Intended Pane closes remain unacknowledged; removal follows the window's next publication.

The filter still covers all already registered workspace clients and clears on omission, activation, or client/workspace cleanup. This PR does not guarantee immediate underlying-list omission for incomplete terminal closes, individual panes, new client identities, or restarts.

Linked Issue

Follow-up to #27205 and the tab-close lifecycle work.

Visual Proof

Final-commit native-chat check: the owner desktop shows a real Claude chat running sleep 120, second of four strip tabs. Eight seconds after the paired desktop closed it, the chat was absent on both; three survivors kept their order through reconnect. These sampled states do not establish every transition frame. The native test used the supported Opus alias; the exact 1M structured-native variant was not verified.

Mac native busy chat close

chat-busy2.png chat-after-close2-8s.png
chat-busy2.png chat-after-close2-8s.png

Final-commit Mac paired-desktop check: the second desktop confirmed stopping busy Terminal 2. Both clients retained Terminal 1 and omitted Terminal 2 after eight seconds and after reconnect/refetch.

Mac paired desktop busy-terminal close

busy-before-close.png after-close-8s.png
busy-before-close.png after-close-8s.png

Final-commit Linux SSH host before/after: the editor close from the second hub-paired desktop removed README.md, and the subsequent busy-terminal close removed one terminal while the remaining strip order stayed intact. The selected browser shows an existing unavailable-server state; these images prove strip membership, not browser functionality. Saved runtime results separately prove the remote job stopped.

Final SHA SSH close evidence

before-host.png after-host.png
before-host.png after-host.png
t1-after.png
t1-after.png

The final image is the owner desktop after a second desktop connected to the same SSH runtime closed the busy terminal. Both clients’ saved tab snapshots omitted that terminal; a second-client screenshot was unavailable. The shared-runtime setup needed the pairing-identity workaround described under Testing.

Testing

  • Manual checks completed on remote Mac and Linux SSH rigs; limitations below
  • Automated tests added/updated

Final commit: 3c6fb11.

  • 66 focused tests across eight explicit files passed on the final reviewed source.
  • Final Node typecheck and staged checks passed. The layout-writer gate from current main also passed for every file changed by this PR.
  • All 23 static source checks passed. The installed Bun 1.3.14 cannot run the aggregate bun check command; the Node compiler check and CI typechecks passed.
  • GitHub CI passed all 13 non-skipped checks, including static analysis/typechecking, five Node test shards, relay integration, packaging, and final verification. End-to-end and cross-version jobs were skipped.
  • Mac paired desktops: closing a running Terminal 2 through the real Stop and Close action removed it from both clients through reconnect/refetch and preserved Terminal 1 and its group. Closing a real native Claude chat running sleep 120 removed it from both at the sampled three/eight-second checks and through reconnect; three survivors kept their order.
  • Linux Docker SSH: the second-client editor close stayed absent on both through republishing/reconnect. Two desktops connected to the same SSH runtime closed a busy terminal; both omitted it, survivor order stayed intact, and the remote job stopped. That setup needed an isolated-profile workaround for an existing pairing-identity bookkeeping issue. A desktop-window acknowledgement was not separately captured for this headless SSH-runtime route.
  • Deferred, pre-existing: a hub-paired desktop retained a closed SSH terminal mirror after reconnect on both exact merge base e969881 and final 3c6fb11. This A/B finding is outside this PR.
  • Coverage limits: busy SSH chat has no signed-in agent in the container; the client-hosted SSH browser close and live Windows checks are unverified. The live structured-native test used the supported Opus alias; the exact 1M variant was not verified. Intermediate frames were not measured; the host characterization test covers chat position, while live checks cover sampled removal and survivor order.

Review

Completed independent correctness and architecture reviews, an architecture re-challenge, fresh review of the final correction, and the readiness checklist. The architecture verdict was to keep the design. Review fixed registered-bystander return with incomplete evidence, hidden sibling panes after reopening, surviving browser group identities, and the refusal republication marker.

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer implementation is copied.

Notes

No new required wire field, stream opcode, persisted schema, dependency, platform branch, or Git command. SSH and folder workspaces were considered in source review; live execution evidence is listed separately above. This PR stays a draft.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Before/after screenshots attached; coverage limitations explained above
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered
  • Required checks pass, or remaining failures are explained with evidence

QA ran in top-level qa-pr-27565 worktrees on the remote machines, with evidence kept outside Git. Owned rig apps were stopped and both remote QA worktrees were removed through Orca. The Linux coordination server is preserved because another PR has a live terminal on it.

… list

Before: when a phone or paired desktop closed a terminal, browser or editor
tab that the desktop window owns, the host asked the window to close it and
answered "closed", but its own tab list kept the tab until the window's next
publish. A per-client "closed tab" filter hid it from the client meanwhile.

Now the host drops the tab from its list when the close commits:
- terminal tabs closed through the acknowledged window relay are retired with
  retirement proofs captured before the relay (the window kills the PTYs during
  it); a tab with any surface lacking a proof is left for the window's own
  republish, and stale-terminal refusals retire nothing;
- relayed browser and editor tabs are retired once the window answers.

The per-client filter now covers only panes closed from a client: the window
drops a pane on an unacknowledged notice, so the host still lists it until the
window republishes. Selection cleanup and its revision bump are unchanged.
…ves the host list

Review round 1 follow-ups for the host-side close commit:
- A chat, browser or editor tab leaving the host list now repairs groups the
  way a terminal retirement does: a group it leaves empty is dropped with its
  layout leaf, and the active tab and group move to ones that still exist.
- When the window's own frame already dropped a closed terminal tab before it
  acknowledged the close, the proofs captured before the relay still ship, so
  paired desktops can retire the tab instead of treating it as an orphan.
- The relayed browser/editor close and the terminal list retirement live in
  files named for them.
- Tests: a split terminal with one unproven pane retires nothing; the terminal
  path keeps the publication epoch; a bystander client's projected version
  stays monotonic across a close.
@brennanb2025 brennanb2025 changed the title A closed tab leaves the host's own tab list when its close commits Keep closed browser, editor, and proven terminal tabs out of the host list Oct 12, 2026
@brennanb2025

brennanb2025 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Closing a tab from a phone or paired desktop could leave it in the host’s published list, allowing a later update to bring it back. This PR removes accepted browser/editor closes and terminal closes with complete pane evidence from that source list, while repairing selection, groups, layouts, and recent-tab history.

Source review is complete at 3c6fb11eb63. No finished review rounds were repeated after restart. The existing correctness/architecture reviews and readiness check remain applicable: all 20 reviewed source hashes still match.

Fixed during the loop: incomplete-proof terminal closes returning for already registered bystander clients; sibling panes remaining hidden after reopening; surviving browser groups losing their supported identifiers; and a live-close refusal omitting its existing republication marker. The description now states the filter’s actual scope and the incomplete-proof/pane exceptions.

Verified: 66 final focused tests, Node typecheck, 23 static source checks, the layout-writer gate for PR-changed files, and all 13 non-skipped CI checks. Final-commit Mac checks closed a busy terminal and a real native Claude chat from the second desktop; both stayed absent at the sampled eight-second check and after reconnect, with survivors preserved. Linux SSH checks covered second-client editor removal and busy-terminal removal on the shared execution runtime, including the remote job stopping. Before/after images are in the PR body.

Deferred: the hub-paired desktop’s stale SSH terminal mirror reproduced identically on the merge base and PR head. The shared-SSH setup also needed a workaround for existing pairing-identity bookkeeping.

Unverified: busy SSH chat without a signed-in agent, the client-hosted SSH browser close, live Windows behavior, the exact 1M structured-native variant (the live test used the supported Opus alias), and every transition frame. The sampled chat close passed; transcript-scroll checks are not evidence for tab-strip position. Chat position is covered by the host characterization test. End-to-end and cross-version CI jobs were skipped; the local aggregate Bun command was unsupported, while Node and CI typechecks passed.

Owned rig apps stopped; both remote qa-pr-27565 worktrees were removed, with evidence preserved outside Git. The Linux server remains because another PR has a live terminal on it. The PR remains a draft and was not merged.

@coderabbitai

coderabbitai Bot commented Oct 12, 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: 2e054898-20d7-4451-ac92-ad34106f6ab6
📥 Commits

Reviewing files that changed from the base of the PR and between 37c2b43 and 3c6fb11.

📒 Files selected for processing (20)
  • src/main/runtime/client-session-tab-closed-selection.ts
  • src/main/runtime/client-session-tab-selection.test.ts
  • src/main/runtime/client-session-tab-selection.ts
  • src/main/runtime/host-session-tab-close-commit.test.ts
  • src/main/runtime/host-session-tab-close-incomplete-proof.test.ts
  • src/main/runtime/mobile-session-absent-close-republication.test.ts
  • src/main/runtime/mobile-session-lifecycle-close-adjudication.ts
  • src/main/runtime/mobile-session-relayed-tab-close.ts
  • src/main/runtime/mobile-session-tab-close-outcome.ts
  • src/main/runtime/mobile-session-tab-group-order.ts
  • src/main/runtime/mobile-session-tab-retirement.test.ts
  • src/main/runtime/mobile-session-tab-retirement.ts
  • src/main/runtime/mobile-session-terminal-retirement.ts
  • src/main/runtime/mobile-session-terminal-tab-group-retirement.test.ts
  • src/main/runtime/mobile-session-terminal-tab-retirement.ts
  • src/main/runtime/orca-runtime-close-headless-mobile-terminal-tab.ts
  • src/main/runtime/orca-runtime-close-mobile-session-tab.ts
  • src/main/runtime/orca-runtime-tests/mobile-session-tabs-part-02.spec.ts
  • src/main/runtime/preserved-chat-tab-position.test.ts
  • src/main/runtime/structured-agent-session-tab-retirement.ts

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

📝 Walkthrough

Walkthrough

The change adds shared snapshot-retirement logic and proof-based terminal-tab retirement. It updates close handling for terminal, browser, editor, and pane tabs. Client selection cleanup now distinguishes IDs removed from selection from IDs suppressed in projections. Tests cover retirement, close acknowledgements, incomplete proofs, stale snapshots, and client projections.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 3c6fb

No concrete issue remains that blocks merging. The unverified environments still warrant the planned validation.

Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 45.16% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 20 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly summarizes the main change: accepted browser, editor, and proven terminal closes remove tabs from the host list.
Description check Passed The description is complete and matches the template. It explains the user impact, implementation, rationale, linked issue, visual proof, testing, limitations, review status, and checklist results.
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.
  • Fix all pre-merge checks with AI
✨ 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.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Closing. A host with no window is moving to list session tabs from its saved layout (#28354 and the series replacing #28421), so the host's list is correct at commit by construction and this separate commit-time drop isn't needed. #27583 is coming off this branch onto main.

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