Skip to content

fix(runtime): the host stops a removed project's shells and agents itself - #29964

Merged
nwparker merged 10 commits into
mainfrom
nwparker/host-stops-removed-project-terminals
Oct 17, 2026
Merged

nwparker merged 10 commits into
mainfrom
nwparker/host-stops-removed-project-terminals

Conversation

@nwparker

@nwparker nwparker commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​212 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​212
Prod 6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​174 $\color{#cf222e}{\Huge{\mathbf{−}}}$​17 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​157

ELI5

When you remove a project from a remote Orca server, the server deletes the project, but its shells and agents can keep running with nothing in Orca pointing at them. Now the server stops them itself before it lets go of the project.

What Changed

Before: repo.rm on a runtime host (a managed SSH server, orcad, or a paired desktop) removed the project and forgot its terminal topology, but never stopped its terminals. The only stopping came from the client: before calling repo.rm, it sent terminal.stop for each workspace it knew about. Workspaces the client hadn't loaded yet (before hydration) or that were hidden kept their shells and agents running after removal. Older clients stop nothing at all.

After: the host stops the project's terminals itself, whichever client asked.

Mechanism: a new stopTerminalsForProject lives in src/main/runtime/terminal/stop/project-terminal-stop.ts. It finds the project's terminals in the host's own PTY and pane table, not in a client's workspace list. It first refreshes that table from the provider, which matters after a restart. It fences the stop to the project's execution host: native, or the row's SSH connection, which includes a runtime:-stamped row with a nested SSH connection. Each worktree is stopped under the existing exclusive terminal-mutation lease, so a spawn already in flight finishes and is stopped too. RuntimeRepositorySettingsController.remove runs that sweep, removes the project, then sweeps again to catch a terminal that registered meanwhile. After removal, acquireWorktreeTerminalSpawn refuses new spawns into the removed project.

Edge cases:

  • Deadline: everything runs inside a 10-second budget, so repo.rm still answers within the client's 15-second timeout.
  • Unconfirmed stops: a stop that can't be confirmed doesn't block removal. The PTY stays in the host's table with its own liveness verdict; it is never marked exited.
  • Same-id siblings: if a sibling row with the same id would claim the same PTYs, the sweep stops nothing rather than risk the sibling's shells.

Why

The execution host owns execution (AGENTS.md), and only the host can see every terminal a project has. Teaching the client to find more workspaces would still miss whatever it hasn't loaded, and would not help older clients. Keeping this inside repo.rm needs no wire change: an older client gets the fix from a newer host for free. A newer client talking to an older host keeps its existing terminal.stop pre-pass, which this PR leaves in place.

Linked Issue

Second external Codex SSH review, finding 5 (P2). No GitHub issue.

Visual Proof

N/A: there is no UI change. This is host-side terminal cleanup on project removal.

Testing

  • New src/main/runtime/project-removal-terminal-stop.test.ts covers:

    • stopping a workspace no client listed
    • leaving other projects and other-connection PTYs alone
    • runtime:-stamped rows, native and over a nested SSH connection
    • a spawn already in flight, and one that registers during the first sweep
    • refusing a spawn after removal
    • skipping the stop when a same-id sibling would share it
    • completing removal when a stop can't be confirmed
  • Ran src/main/runtime/terminal, src/main/ipc/pty, src/main/ipc/repos and the related runtime tests single-worker: all pass. The only failure is orca-runtime.test.ts "preserves existing badgeColor on runtime cloneRepo…", which also fails on unmodified main. Ran pnpm tc and the static-analysis job's lint, code-quality and ratchet steps locally.

  • Live on omarchy (Linux), isolated under /tmp with its own HOME, XDG and user-data dirs on a private port:

    • Setup: a headless orcad, a git project with two worktrees, and a shell in each (PIDs captured).
    • Called repo.rm directly, as a client that had listed no workspaces would, so no terminal.stop was sent.
    • On origin/main's orcad bundle, both shells were still alive after the project was removed. That reproduces the finding.
    • With this branch's bundle, both shells had exited by the time repo.rm returned, and the project was gone.
  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

Review

Ran Codex review (gpt-6.1-sol, high) and fixed every P0/P1. Remaining P2s, dismissed:

  • A spawn into a worktree that neither the resolved list nor the PTY table knows yet can still slip through. It is narrow, and the post-removal spawn refusal covers it once its lease is taken.
  • The post-removal spawn refusal is keyed by repo id, not by host, for a same-id sibling that survives on this server. The spawn carries no host at lease time.
  • The stop refuses while the renderer graph reloads. That matches the existing terminal.stop contract, which never acts on an unstable graph.

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

  • Wire compatibility: no wire change. An old client with a new host gets the host-side stop; a new client with an old host keeps the client pre-stop.
  • Platforms: cross-platform (uses the existing PTY controller).
  • Folder workspaces: project-kind folder repos use the same repoId::path worktree ids.

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)

nwparker and others added 10 commits October 16, 2026 20:47
Co-Authored-By: Claude <noreply@anthropic.com>
…un here

Co-Authored-By: Claude <noreply@anthropic.com>
…ct's removal

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

Co-Authored-By: Claude <noreply@anthropic.com>
…nd wait out in-flight spawns

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

Co-Authored-By: Claude <noreply@anthropic.com>
…same-id sibling shares

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…aiter is dequeued

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

coderabbitai Bot commented Oct 17, 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: 06d8a66d-ce26-4fe4-99a8-04e46f42e253
📥 Commits

Reviewing files that changed from the base of the PR and between 07d3e33 and af6f22a.

📒 Files selected for processing: 7
  • src/main/runtime/construction/runtime-construction-services.ts
  • src/main/runtime/construction/runtime-host-service-calls.ts
  • src/main/runtime/construction/runtime-host-services.ts
  • src/main/runtime/project-removal-terminal-stop.test.ts
  • src/main/runtime/runtime-repository-settings-controller.ts
  • src/main/runtime/session-graph/session-graph-reload-lifecycle.ts
  • src/main/runtime/terminal/stop/project-terminal-stop.ts

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

📝 Walkthrough

The runtime adds a project-wide terminal-stop service and connects it to project removal. Removal attempts terminal stops before and after project removal, with a deadline. The stop implementation refreshes and gathers project worktrees, then stops terminals under worktree mutation locks. It also rejects qualifying terminal-spawn requests when the project row is absent. Tests cover worktree and host ownership, concurrent and late spawns, and unverifiable stops. Repository-catalog dependencies are consolidated into a shared object.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to af6f2

Project removal remains mergeable under its bounded, best-effort terminal-stop behavior.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Description check Warning The description is detailed and covers the user impact, implementation, rationale, testing, compatibility, and checklist. However, the required Linked Issue section does not contain an actual issue re… Add a valid linked issue under "Fixes #" or provide the repository-approved issue reference. Update the final validation checklist after running or confirming coverage for lint, typecheck, test, and build commands; document any known failur…
Docstring Coverage Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: the runtime host stops terminals for a removed project.
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: Description check

Explanation

The description is detailed and covers the user impact, implementation, rationale, testing, compatibility, and checklist. However, the required Linked Issue section does not contain an actual issue reference, and the full lint, typecheck, test, and build checklist remains unchecked.

Resolution

Add a valid linked issue under "Fixes #" or provide the repository-approved issue reference. Update the final validation checklist after running or confirming coverage for lint, typecheck, test, and build commands; document any known failure and its baseline status if CI will cover it.

  • 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.

@nwparker
nwparker merged commit a82298e into main Oct 17, 2026
31 checks passed
nwparker added a commit that referenced this pull request Oct 20, 2026
…hells

repo.rm now stops the project's terminals on the host (#29964), so the
'orphan left by older builds' setup can no longer be built; use a shell
only the host knows of instead. Resolve the project path so macOS
/private/var matches what the host reports.
nwparker added a commit that referenced this pull request Oct 20, 2026
…k remove that stays removed (#31082)

* feat(settings): one-list Remote Orca Servers page and bulk-manageable SSH Hosts

Remote Orca Servers: replace the three mode cards, Advanced fold, duplicate
managed/SSH-access lists and troubleshooting accordion with a single list
(one row per server: status, one action, overflow menu), an always-visible
'New projects open on' row, and Share / Cloud VM cards that open dialogs.

SSH Hosts: group by source, filter, checkbox + shift-range selection, one
confirm for bulk remove (skipping hosts with a managed server or
workspaces), bulk Test. 'Sync ~/.ssh/config' no longer clears removal
tombstones; 'Restore removed hosts (n)' is a separate confirmed action.

* fix(settings): address review findings on servers and SSH hosts redesign

- SSH filter stays clearable when the list shrinks under the threshold
- Cmd/Ctrl-A only selects hosts when pressed in the list itself
- group legacy no-source hosts with main's import predicate (now shared)
- capture the shift-range anchor before queuing the selection update
- blank server names de-duplicate instead of colliding
- Share dialog Done refreshes link count and resets name editing
- Install / SSH-route dialogs reload SSH hosts when opened
- default-host row shows the selector when the default is a WSL host
- managed 'Not running' no longer masks connecting/revoked/update states
- servers without a dropped transport can be re-checked

* test(e2e): WSL host spec matches repo.rm ending a removed project's shells

repo.rm now stops the project's terminals on the host (#29964), so the
'orphan left by older builds' setup can no longer be built; use a shell
only the host knows of instead. Resolve the project path so macOS
/private/var matches what the host reports.

* fix(settings): carry 'Orca on <host>' naming and paired-twice warning into the new server list

Ports #31099 and #31100 into the redesigned rows, install dialog and SSH
host dialogs.

* test(e2e): keep the WSL spec's project path as created
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