Repository navigation
Keep SSH chat providers alive after the desktop disconnects - #26473
brennanb2025 wants to merge 10 commits into
Conversation
brennanb2025
changed the title
…-owed-work-keeps-server
It builds on the native chat rest test rig, which opens a real SQLite database.
Review summary (final head
|
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to A narrow timing window can interrupt a provider starting background work. Track in-flight attaches in the idle probe before merging, or accept this bounded risk. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 9 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Description check The description is complete and directly explains the user impact, implementation, rationale, linked issues, testing, compatibility, and review status. Required template sections are present, and the … Title check The title clearly and concisely describes the main change: keeping SSH chat providers alive after the desktop disconnects. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check 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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
8cb67f08-61ba-4fba-a3ce-a55e6c0fa3b6
📒 Files selected for processing (9)
config/scripts/vitest-sqlite-runtime-files.mjssrc/main/native-chat/agent-session-wire/structured-agent-session-host.tssrc/main/orcad/orcad-idle-exit-monitor.test.tssrc/main/orcad/orcad-idle-exit-monitor.tssrc/main/orcad/orcad-managed-idle-exit-chat-provider.test.tssrc/main/orcad/orcad-managed-idle-exit-host.tssrc/main/orcad/orcad-managed-idle-exit-provider-race.test.tssrc/main/orcad/orcad-managed-idle-exit.test.tssrc/main/orcad/orcad-managed-idle-exit.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
| hasLoadedProviders = (): boolean => | ||
| [...this.sessions.values()].some(({ child }) => child !== null) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '215,290p' src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts
sed -n '60,110p' src/main/orcad/orcad-managed-idle-exit-host.ts
rg -n 'ensureProviderChild|requestIdleShutdown|indexProviderChild|onSpawned' src/main/native-chat/agent-session-wire src/main/orcad/orcad-managed-idle-exit-host.tsRepository: stablyai/orca
Length of output: 8902
🏁 Script executed:
set -eu
printf '%s\n' '--- idle exit implementation ---'
rg -n -F --glob '*.ts' -- 'installOrcadManagedIdleExit' src/main/orcad src/main
rg -n -F --glob '*.ts' -- 'stopForIdle' src/main/orcad
printf '%s\n' '--- acquisition and attach entrypoints ---'
sed -n '1,190p' src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts
sed -n '1,230p' src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts
sed -n '1,245p' src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts
printf '%s\n' '--- idle monitor and stop coordination ---'
rg -n -F --glob '*.ts' -- 'quietSince' src/main/orcad
rg -n -F --glob '*.ts' -- 'requestShutdown' src/main/orcadRepository: stablyai/orca
Length of output: 30637
🏁 Script executed:
set -eu
printf '%s\n' '--- idle monitor ---'
sed -n '1,125p' src/main/orcad/orcad-idle-exit-monitor.ts
printf '%s\n' '--- managed idle host stop path ---'
sed -n '1,190p' src/main/orcad/orcad-managed-idle-exit-host.ts
printf '%s\n' '--- provider race test ---'
sed -n '1,125p' src/main/orcad/orcad-managed-idle-exit-provider-race.test.ts
printf '%s\n' '--- structured host lifetime and shutdown references ---'
rg -n -F --glob '*.ts' -- 'trackStart' src/main/native-chat/agent-session-wire
rg -n -F --glob '*.ts' -- 'holdClosedStructuredAgentSessionSends' src/main/native-chat/agent-session-wire
rg -n -F --glob '*.ts' -- 'dispose()' src/main/native-chat/agent-session-wire/structured-agent-session-host*.ts src/main/native-chat/agent-session-wire/structured-agent-session-host-*.ts
rg -n -F --glob '*.ts' -- 'closeStructuredAgentSession' src/main/native-chat/agent-session-wireRepository: stablyai/orca
Length of output: 16489
🏁 Script executed:
set -eu
printf '%s\n' '--- shutdown binding and startup stop ---'
rg -n -F --glob '*.ts' -- 'installOrcadShutdownSignals' src/main
rg -n -F --glob '*.ts' -- 'bindOrcadIdleShutdown' src/main
rg -n -F --glob '*.ts' -- 'stop(): Promise' src/main
printf '%s\n' '--- lifecycle and host teardown ---'
rg -n -F --glob '*.ts' -- 'class StructuredAgentSessionHost' src/main/native-chat/agent-session-wire
sed -n '1,150p' src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts
sed -n '225,285p' src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
printf '%s\n' '--- task tracking and quit paths ---'
rg -n -F --glob '*.ts' -- 'trackAttach' src/main/native-chat/agent-session-wire
rg -n -F --glob '*.ts' -- 'trackAttach' src/main
rg -n -F --glob '*.ts' -- 'stopFor' src/main/orcad/orcad-entry.ts src/main/orcad src/main | head -80Repository: stablyai/orca
Length of output: 15820
🏁 Script executed:
set -eu
printf '%s\n' '--- task queue ---'
sed -n '1,180p' src/main/native-chat/agent-session-wire/structured-agent-session-task-queue.ts
printf '%s\n' '--- host construction and acquisition wiring ---'
sed -n '1,180p' src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
printf '%s\n' '--- entry stop binding ---'
sed -n '385,425p' src/main/orcad/orcad-entry.ts
printf '%s\n' '--- lifecycle shutdown trigger ---'
sed -n '1,105p' src/main/orcad/orcad-lifecycle.tsRepository: stablyai/orca
Length of output: 16531
Keep idle exit busy while provider acquisition is in flight.
hasLoadedProviders() is false after onSpawned commits the provider process and before indexProviderChild() records the child. During this interval, stopForIdle() can yield for daemon retirement, then the final check can request shutdown without newer RPC activity. Shutdown aborts acquisitions before draining attaches, so it can interrupt the provider start and its queued delivery.
Include active attach or delivery-start tracking in the idle probe. The existing task queue already tracks these operations.
Suggested fix
diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-task-queue.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-task-queue.ts
@@
trackAttach<T>(operation: Promise<T>): Promise<T> {
this.attaching.add(operation)
@@
return operation
}
+ hasActiveAttaches(): boolean {
+ return this.attaching.size > 0
+ }
+
diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
@@
hasSession = (sessionId: string): boolean => this.sessions.has(sessionId)
hasLoadedProviders = (): boolean =>
- [...this.sessions.values()].some(({ child }) => child !== null)
+ this.tasks.hasActiveAttaches() ||
+ [...this.sessions.values()].some(({ child }) => child !== null)This branch has not been deployed
1 participant
ELI5
An SSH chat waiting for permission could be killed after the desktop disconnected. The remote server counted only agents marked as working, so a waiting chat looked idle even though its provider was still loaded. Returning to the chat then showed the approval as Cancelled and the reply as failed.
The managed server now stays up while it holds a chat provider. A running reply or an unanswered approval or question keeps that provider loaded until it is answered, stopped, or exits.
What Changed
The structured chat host exposes whether its existing in-memory sessions hold any provider children. Managed server idle exit reads that observation alongside clients, terminals, jobs, and the activation fence, and rechecks it and client activity after asynchronous terminal-daemon retirement before requesting shutdown. This also protects a returning desktop while its provider is still starting.
The existing provider idle sweep remains the only policy deciding when an idle provider should be released. It already protects running turns, queued sends, pending prompts, and background work. With no work pending, it releases the provider after 30 minutes of inactivity; the managed server can then complete its existing 15-minute quiet period and exit.
Updates and rollbacks still restart the server immediately, including while a reply runs; existing recovery marks that reply Interrupted, and #26460 preserves its recorded working time. Explicit Stop server still works.
Why
Server lifetime should follow the provider it owns, rather than reproduce the provider's work policy in the server, update planner, and stop handlers. This adds no persisted obligation, journal scan, process probe, wire field, update deferral, or recovery change.
Compared with the common pattern:
Lifetime decision: running turns and unanswered prompts have no separate expiry. Their lifetime ends through an answer, Stop/Close, provider exit, or explicit Stop server. An idle conversation with no pending work releases its provider after the existing idle window.
Linked Issue
SSH managed-server rollout: #24863. Companion interruption recovery: #26460.
Visual Proof
No rendered UI changes. The live SSH check below shows the behaviour this PR protects.
Setup. A local desktop app was connected over SSH to a managed Orca server in a Linux container, with a stand-in Claude (the real CLI never ran). The server's idle timeout was shortened to 60 seconds. The app was an integration build with every SSH chat fix merged together. This PR was at
da06daa534b, and its own diff is unchanged by the later main sync.1. A Claude chat in "Ask for approval" asks to run a command. The user then quits the desktop app with the request still unanswered.
2. The server stays up while the user is away. More than 126 seconds later, more than twice the idle timeout, the server and the waiting Claude process were both still running, and the command had not run.
3. Reopening the app shows the same request still waiting. Clicking Allow runs the command and the reply completes.
The earlier "Interrupted" turn and the cancelled request in that screenshot are from a first attempt in the same run. The test driver pressed Escape to close an onboarding tip, and Escape also cancelled the pending request. That first attempt has nothing to do with this PR.
Testing
50 targeted tests pass across the managed idle probes, idle monitor, real structured-host provider lifecycle, and existing provider idle sweep. They cover a disconnected chat with a pending approval, idle provider release followed by server exit, a quiet running turn surviving repeated idle windows, a provider loading while daemon retirement awaits a reply, and a connection or request returning during the same wait.
The approval and running-turn regressions were run against the original production code and both failed because the server incorrectly exited. The same tests pass with this change.
After the final merge with
main(head6d3c504df69):orca-ci-checks --no-typecheckpasses 23/23.mainstarted requiring every test that uses the chat "rest test rig" (a helper that opens a real SQLite database) to be listed for the Node test runtime. The new provider test was added to that list.Live: a live SSH check covered the pending approval (see Visual Proof). WSL and Windows hosts were not exercised live. The change uses platform-independent host state and doesn't touch the remote protocol.
Review
Reviewed in three rounds; the last full review found nothing blocking. Only managed idle exit changes; update, rollback, stop-request, reservation, and recovery code matches the branch base.
Agent skill upstream boundary
Notes
New servers apply the idle protection with either old or new desktops because the decision is entirely on the execution host. Old servers keep their previous idle behavior; this patch introduces no client capability or mixed-version wire change. Folder workspaces and non-SSH managed hosts use the same structured host observation.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred) — local targeted validation and final CI are passing