fix: orchestration messaging does not survive IDE/app restart (#10702) - #10751
fix: orchestration messaging does not survive IDE/app restart (#10702)#10751jibin7jose wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughTerminal handle resolution now preserves pane mappings across runtime graph teardown and canonicalizes orchestration send, dispatch, and ask targets to live terminal handles. The orchestration check command validates 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/shared/agent-process-recognition.ts (1)
290-296: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for environment-prefixed commands.
Please add or confirm tests covering
FOO=bar claude, multiple leading assignments, and assignment-only input returningnull. These cases define the new recognition contract and protect downstream consumers from future tokenization regressions.src/cli/handlers/orchestration.ts (1)
142-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for stale terminal handles.
Please cover live env handles, successful pane reminting, failed reminting for
--terminal(no_active_terminal), and the preserved--fromerror path insrc/cli/handlers/orchestration.test.ts.Also applies to: 401-403
src/main/runtime/orca-runtime.ts (1)
23945-23950: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated pane-key capture loop.
This exact loop is repeated verbatim in
markGraphUnavailable(lines 23982-23987). Extract into a shared private helper (e.g.capturePaneKeyHistoryBeforeTeardown()) so a future change to the capture logic can't be applied to only one of the two teardown paths.♻️ Proposed refactor
+ private capturePaneKeyHistoryBeforeTeardown(): void { + for (const [handle, record] of this.handles) { + if (isTerminalLeafId(record.leafId)) { + this.historicalPaneKeys.set(handle, makePaneKey(record.tabId, record.leafId)) + } + } + }And in both
markRendererReloadingandmarkGraphUnavailable:- for (const [handle, record] of this.handles) { - if (isTerminalLeafId(record.leafId)) { - this.historicalPaneKeys.set(handle, makePaneKey(record.tabId, record.leafId)) - } - } + this.capturePaneKeyHistoryBeforeTeardown()src/main/runtime/rpc/methods/orchestration.ts (1)
207-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for stale-handle canonicalization.
This helper now controls routing for
send,dispatch, andask, but the shown test only exercises already-live handles. Add cases asserting stale handles resolve to the current live handle for persisted messages, notifications, dispatch context/preambles, and decision-gate delivery.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d76410a3-73c1-4011-92d3-900aac1221f9
📒 Files selected for processing (4)
src/cli/handlers/orchestration.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/methods/orchestration.tssrc/shared/agent-process-recognition.ts
…tarts # Conflicts: # src/main/runtime/rpc/methods/orchestration.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/handlers/orchestration.ts (1)
179-195: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for stale environment handles.
The supplied adjacent test covers the
--fromrewrite, but not this new--terminalbehavior. Add cases for successful pane reminting,no_active_terminalwhen reminting fails, and explicit--terminalbypassing environment validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 189444df-05ab-4cb9-9fd1-22952fca8353
📒 Files selected for processing (1)
src/cli/handlers/orchestration.ts
2 participants
Fixes #10702, #10406, #8808, #9163
Summary
This PR resolves the overarching issue of orchestration messaging failing after IDE/app restarts (#10702) by implementing the following robust fixes:
Implicit Handle Validation ([Bug]: Implicit orchestration identity resolves from launch-time ORCA_TERMINAL_HANDLE — after app restart,
checksilently listens on a dead mailbox while worker_done queues on the live handle #10406)Agents running
orchestration checkno longer silently listen on a dead mailbox whileworker_donequeues on the live handle. Theorchestration checkhandler now properly validates and remintsORCA_TERMINAL_HANDLE.Environment Variable Stripping in Agent Recognition ([Bug]: agent recognition never recovers after runtime restart — relaunch via 'terminal send' stays "no recognized agent detected" (macOS, 1.4.141) #8808)
Agent process recognition failed if the CLI was launched with environment variables. We now strip these prefixes before attempting to recognize the active process command.
Delivery Follows Pane-Key ([Bug]: Terminal/session handle instability breaks orchestration message addressing and injection #9163)
To handle terminal/session handle instability across restarts:
orca-runtime, with bounded map sizes to prevent infinite accumulation.Screenshots
No visual change. (Backend routing and state management only).
Testing
orchestration checkautomatically remints handles whenORCA_TERMINAL_HANDLEgoes stale.recognizeAgentProcessFromCommandLinecorrectly parses environments starting with variable assignments (e.g.,CLAUDE_CONFIG_DIR=...).OrcaRuntimeteardown accurately persists historical pane keys into the bounded cache map (HISTORICAL_PANE_KEY_MAX = 512).orchestration.send,orchestration.dispatch, andorchestration.ask.node_modulesdependencies in the development environment. We are relying on the GitHub Actions CI/CD pipeline to execute the full test suite and verify no regressions were introduced.AI Review Report
No outstanding issues. CodeRabbit nitpicks regarding bounded map growth have been resolved in the latest commit.
Cross-Platform Considerations:
This PR has been evaluated for macOS, Linux, Windows, and Electron-specific differences. The changes to terminal handle validation, environment variable stripping in agent recognition, and pane-key resolution are entirely platform-agnostic string parsing and map lookups within the Node.js runtime layer. No native OS modules or platform-specific Electron APIs are used, guaranteeing identical behavior across macOS, Linux, and Windows.
Security Audit
No security implications. No new dependencies or network boundaries were introduced. State caches are bounded to prevent memory leaks.
Notes
This fully closes the epic product requirement for multi-agent orchestration surviving app restarts without needing manual broadcasts.
ELI5
After restarting the app, agent orchestration messaging (check/dispatch/worker signals) often broke because handles and env vars went stale. This makes handles remint correctly and restores messaging so multi-agent workflows survive restarts.