[OrcaWin]

	Files	Added	Deleted	Net
Test	10	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​840	$\color{#cf222e}{\Huge{\mathbf{−}}}$​23	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​817
Prod	18	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​463	$\color{#cf222e}{\Huge{\mathbf{−}}}$​85	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​378

ELI5

When Orca's remote relay shuts down, it used to send "please stop" to its file-watcher and AI-agent helper processes and then move on, without checking that they actually stopped. This change makes the relay wait until those helpers are really gone. If one won't stop, the relay keeps running and tries the shutdown again later, instead of exiting and leaving the helper behind on the host.

This is the T2 P1 slice of porting #16741 (owned child disposal: the watcher pool and the relay's agent and watcher children).

What Changed

Before: on relay shutdown, file-watcher child processes and agent processes (for example, commit-message generation) were signalled and then forgotten. An agent request that timed out was treated as finished even while its process was still running. A watcher child could outlive the relay and keep a folder handle open, which matters on Windows, where open handles block deleting that folder.

After: shutdown resolves only after every child the relay owns has physically exited.

• Watcher processes: WatcherProcessSupervisor records each child it launches. Its new disposeAndWait() waits for each child's exit or close event. RuntimeWatcherProcessPool hands retired supervisors to RuntimeWatcherDisposalOwners, which waits for them and retries any that failed on the next attempt.
• Relay watch registry:
  • Refuses new fs.watch requests once shutdown starts.
  • Waits for in-flight native unsubscribes and pending setups to finish.
  • Stops the pool's children, through FsHandler.disposeWatchers().
• Agent processes: AgentExecHandler tracks every spawned agent child until it emits close. On shutdown it refuses new runs and ends each process tree.
• Relay shutdown: RelayRuntimeServices.disposeOwnedProcesses() now waits on the agents and the watchers, next to the existing file and git stream drains from feat(relay): fence and drain file and git response streams on shutdown #24185. Any failure is reported as relay_owned_process_shutdown_incomplete. The existing deferred-shutdown path in relay-grace-lifecycle.ts then retries. Every dispose step is safe to repeat, so retries are fine.
• Watcher child messages: the watcher child's process.send now passes a callback. A send that fails because the host is gone is ignored, instead of surfacing as an error event.

Why

A relay that exits while its children are still running leaves orphaned processes on the user's SSH host. Those processes keep file handles open, and the next relay can't verify or clean them up. Waiting for the physical exit or close event is the only reliable evidence: a sent signal, or a timed-out request, says nothing about whether the process is gone.

Why defer and retry instead of exiting anyway: the relay already has a deferred-shutdown retry, so I reused it instead of adding a separate mechanism.

Adapted from #16741 (a68b6f3), by hunk:

• Skipped the ownership-transfer and capture hunks in relay-runtime-services.ts (T7).
• Skipped the search and fs-handler-utils changes in fs-handler.ts. Main's fs.search abort signal and bundled ripgrep stay as they are.
• Skill-upload and AI-vault cleanup failures stay log-and-continue, as on main. They are left for a later T2 slice.
• Promise.withResolvers was replaced in relay-bundled code, which still has to run on Node 18 hosts.

Linked Issue

Part of the Node runtime plan (Phase 3), porting #16741.

Visual Proof

N/A. There is no UI change. The difference is in how the relay shuts down on the remote host.

Testing

• New:

  • owned-children
  • supervisor-disposal
  • disposal-owners
  • pool-shutdown
  • agent-exec-disposal
  • relay-watcher-shutdown
  • file-stream shutdown, metadata-level cases only (main's fs-handler-stream tests already cover the terminal-frame case)

• Extended:

  • relay-runtime-owned-process-shutdown: agents and watchers, failure plus retry.
  • parcel-watcher-process-entry: the send callback.

• Updated, two existing expectations:

  • A timed-out agent child stays tracked until it closes.
  • Production pool disposal may signal a watcher child more than once while it waits for the exit.

• Results:

  • All relay unit tests: 204 files, 1943 passed.
  • All watcher, fs, agent and relay-runtime tests: 79 files, 683 passed.

• macOS only locally. The Windows handle-release behavior is covered by CI.

• I manually tested these changes locally

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

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (1)

AGENTS.md — auto-discovered

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: 6a21a434-6b40-48c5-bb3c-40baa97e360a

📥 Commits

Reviewing files that changed from the base of the PR and between ebd50b0 and 609019d.

📒 Files selected for processing (1)

• src/main/ipc/parcel-watcher-owned-children.ts

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

---

📝 Walkthrough

Walkthrough

Watcher and relay agent child processes are tracked through physical exit, and their owners expose awaited disposal. Runtime watcher pools retain failed disposal attempts for retry. Relay watcher shutdown fences new watches and waits for setup, teardown, and pool disposal. Runtime shutdown now awaits agent, watcher, response-stream, and file-stream cleanup. IPC sends now provide a callback, and tests cover the updated shutdown and cleanup behavior.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 60901

The change makes shutdown wait for owned watcher and agent processes and report incomplete cleanup. No actionable merge risk remains in the reviewed evidence; it is ready for normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 60901

Awaited shutdown improves cleanup and failure reporting. However, on Windows, the new agent shutdown path can attempt to terminate a process using an old process ID after the original process has exited. Under process-ID reuse, this could affect an unrelated process running with permissions accessible to the relay.

Retained concerns

• Medium · security · inferred: The new agent shutdown path retains children until close and then invokes Windows taskkill using child.pid, without checking exitCode or signalCode. A child can exit while inherited streams remain open; if its PID is reused before close, the first shutdown attempt can target an unrelated process. The disposal-local retry guard does not establish process identity or record earlier request-level termination calls. This weakens shutdown containment to owned processes, although actual impact depends on Windows timing and relay permissions.

Security review details

Security Blast Radius

• inferred — The stale-PID concern is confined to the Windows termination branch and the host processes that the relay account is permitted to terminate. It can cross the intended owned-child boundary, but no cross-tenant reachability or elevated privilege gain is established.

Security Findings and Attack Paths

• inferred — A launched agent could exit while a descendant keeps inherited streams open, leaving its handle retained until close. If Windows reuses the parent PID before shutdown, the new disposal call can issue taskkill against that reused identity. Request timeout already invokes this helper, but finishing the request does not remove the new lifetime tracking or mark its disposal-local signalled set. This is a conditional ownership failure, not a verified exploit or evidence of additional execution authority.

Trust Boundaries and Controls

• observed — The inspected watcher production path tracks handles produced by its local launch operation, not request-provided process IDs. Supervisor shutdown prevents further launches, and termination treats an existing exit or signal code as completed termination.

Resilience and Maintainability Implications

• observed — Watcher termination escalates to a hard kill and reports deadline failure instead of acknowledging success. The owner registry keeps failed supervisors available for retry, preserving cleanup accountability across partial failure.

Hardening Proposals

• proposed — Separate stream-close waiting from process identity authority. Avoid Windows PID-based tree termination after confirmed parent exit, and use identity-preserving tree ownership if descendants must remain terminable after that exit. Coordinate request-level and shutdown termination bookkeeping rather than relying only on a disposal-local retry guard.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 28 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: awaiting owned watcher and agent child processes during relay shutdown.
Description check	✅ Passed	The description is detailed and mostly complete. It includes the ELI5 summary, changes, rationale, linked issue reference, visual-proof explanation, testing results, platform coverage, and relevant ch…
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

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[pullfrog]

✅ No new issues found.

Reviewed changes

• Watcher child ownership and awaited disposal — WatcherProcessSupervisor now records every launched child (parcel-watcher-owned-children.ts) and exposes disposeAndWait(); the runtime pool retires slots through RuntimeWatcherDisposalOwners, which waits for physical exit and retries failed attempts on the next shutdown call.
• Relay watch registry shutdown — new relay-watcher-shutdown.ts fences new fs.watch once disposed, closes every known/failed watch state, joins in-flight native unsubscribes, and drains pending setups; FsHandler.disposeWatchers() exposes it. The subscribeState late-resolution branch and the relay-watcher-teardown-tracker.ts:50 change keep a late subscription retryable rather than silently forgotten.
• Agent child lifetime — new RelayAgentProcessLifetime tracks each spawned agent child until it emits close; AgentExecHandler.dispose() fences admission, kills each tree at most once, and rejects after RELAY_AGENT_CLOSE_DEADLINE_MS (10s) so a timed-out request is no longer taken as proof the child is gone.
• Shutdown integration — RelayRuntimeServices.disposeOwnedProcesses() awaits agents and watchers alongside the existing stream drains and reports relay_owned_process_shutdown_incomplete so the grace lifecycle defers and retries.
• Watcher signal and messaging — signalWatcherChild marks the graceful signal so an awaited termination only escalates to SIGKILL; the watcher child's process.send now passes a callback so a dead host no longer surfaces an error event.

I traced the full drain tree for liveness (agent 10s close deadline, watcher 10s exit deadline installed before the signalled early-return, 60s unsubscribe/subscribe bounds), checked that every supervisor-launched child reaches ownedChildren.track before disposeAndWait snapshots and is removed only on definitive exit/close, and confirmed a failed disposal attempt is retained for explicit retry rather than dropped. No blocking issue found.

ℹ️ Nitpicks

• The PR description's Testing section still says pool disposal "may signal a watcher child more than once while it waits for the exit"; commit e274d9f changed this to a single graceful signal with SIGKILL escalation and updated the expectation to toHaveBeenCalledTimes(1). Worth updating the description so it matches the shipped behavior.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[greptile-apps]

Confidence Score: 4/5

[High risk] Adds graceful shutdown sequencing to file watcher process lifecycle.

The PR does not appear safe to merge while an agent child can remain alive after a failed termination attempt without being signalled again.

Findings

1.  Failed termination cannot retry ▶
2.  Agent termination cannot recover ▶

Summary

The PR makes relay shutdown wait for owned watcher and agent children to physically exit, retaining failed cleanup for later shutdown attempts.

• Watcher supervisors and the pool now join child disposal, including retired supervisors.
• Relay watch admission is fenced during shutdown, and agent processes remain tracked until close.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Relay shutdown] --> B[Dispose agents]
  A --> C[Close watches and pending setups]
  C --> D[Dispose watcher pool and retired supervisors]
  B --> E{All owned children closed?}
  D --> E
  E -->|Yes| F[Complete shutdown]
  E -->|No| G[Defer and retry shutdown]

Loading

_{Reviews (5) · Last reviewed commit: "fix(watcher): take the child process typ..."}

[greptile-apps]

Failed termination cannot retry

If taskkill fails to start on a Windows host while an agent child is still alive, this code still marks the child as signalled. The close deadline then rejects, but later shutdown attempts skip termination and only wait for that child, leaving the relay unable to finish shutdown even if the failure was temporary.

[greptile-apps]

Agent termination cannot recover

If an agent child remains alive after the first termination attempt fails—for example, if Windows taskkill fails—shutdown reaches its close deadline and is deferred. The child stays marked as signalled, so every later shutdown attempt only waits for it and never tries to stop it again. The relay can remain unable to complete shutdown while that child is alive.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: db330497-86a1-4a9c-9076-6ee8007df17a

📥 Commits

Reviewing files that changed from the base of the PR and between c991893 and e274d9f.

📒 Files selected for processing (28)

• src/main/ipc/parcel-watcher-child-termination.ts
• src/main/ipc/parcel-watcher-entry-path.ts
• src/main/ipc/parcel-watcher-owned-children.test.ts
• src/main/ipc/parcel-watcher-owned-children.ts
• src/main/ipc/parcel-watcher-process-entry.test.ts
• src/main/ipc/parcel-watcher-process-entry.ts
• src/main/ipc/parcel-watcher-process-supervisor.ts
• src/main/ipc/parcel-watcher-supervisor-disposal.test.ts
• src/main/ipc/parcel-watcher-supervisor-disposal.ts
• src/main/ipc/runtime-watcher-disposal-owners.test.ts
• src/main/ipc/runtime-watcher-disposal-owners.ts
• src/main/ipc/runtime-watcher-pool-shutdown.test.ts
• src/main/ipc/runtime-watcher-pool-state.ts
• src/main/ipc/runtime-watcher-process-pool.ts
• src/relay/agent-exec-disposal.test.ts
• src/relay/agent-exec-handler.test.ts
• src/relay/agent-exec-handler.ts
• src/relay/fs-file-stream-shutdown.test.ts
• src/relay/fs-handler.ts
• src/relay/relay-agent-process-lifetime.ts
• src/relay/relay-filesystem-watch-registry.ts
• src/relay/relay-runtime-owned-process-shutdown.test.ts
• src/relay/relay-runtime-services.ts
• src/relay/relay-watcher-process-pool.ts
• src/relay/relay-watcher-setup-wait.ts
• src/relay/relay-watcher-shutdown.test.ts
• src/relay/relay-watcher-shutdown.ts
• src/relay/relay-watcher-teardown-tracker.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.