[brennanb2025]

	Files	Added	Deleted	Net
Test	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​511	$\color{#cf222e}{\Huge{\mathbf{−}}}$​146	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​365
Prod	3	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​31	$\color{#cf222e}{\Huge{\mathbf{−}}}$​10	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​21

Problem

On Windows, users running many orchestration workers saw Orca keep about 100 hidden codex app-server processes alive and start more than one per second, pinning the CPU at 100% for hours (tracked in #34141). Releasing the settled workers made it stop.

Each structured worker kept a full chat-stream subscription solely to retry parked mail when its chat changed. Building those frames and refreshing resting slash lists read the / command list for unreleased workers, which could start background Codex discovery even when no composer displayed the list.

Scope: native chat orchestration workers only

This only affects orchestration workers that run as native chat (structured) sessions. Terminal orchestration workers never held this subscription or a resting command list, so they never caused the storm and are untouched here.

Each structured worker kept a hidden subscription so orchestration could re-deliver parked mail when the worker's chat changed. That subscription used the same path a visible chat tab uses, so every settled-but-unreleased worker counted as a resting chat someone was looking at and kept asking for its / and $ command list. Any agent's worker could create that demand, but only Codex turns it into processes: each Codex check started an app-server that stays up about 70 seconds for plugin sync, while Claude checks exit as soon as they answer and were already capped at two at a time.

After this PR, the worker's subscription listens only for new chat-log entries and never asks for a command list. Chat tabs on desktop and phone are unchanged.

Change

Worker redrive now observes the host's existing journal publications directly. Commits, tombstones, epoch changes, reattach snapshots and row-free work changes still wake the existing coalescer. Release, stop and abandon cancel pending wakes and remove the observer. Display-only catalog/roster/activity frames no longer create worker retry demand. Visible desktop and phone command-list behavior is unchanged.

This PR removes internal-worker discovery demand. Catalog scheduling and the shared probe process bound ship separately. It overlaps #28567 in subscribeForRedrive; only its subscription-call block changes here.

Validation

Full review path: orchestration runtime. QA classification: CLI/orchestration/performance tests row.

• Real CLI orchestration send, actual socket/RPC/runtime, production worker-topology callback and real journal publications reach fixture-provider pointer delivery and durable mail receipts. The same inputs pass with the former stream carrier and the new observer: held mail, a throwing send that retains unread/undelivered mail, later successful delivery, and teardown cancellation.
• A production-trigger contract with 16 and 160 real at-rest worker sessions proves one localized publication visits one observer, all-worker bursts coalesce once per worker, and client/catalog/frame/history/queue-publication/roster reads stay at zero. Disposal prevents further visits and cancels pending callbacks.
• Real-host regressions cover tombstoned settlement, epoch rollover, close/reopen/reattach, row-free generation end and release/stop/abandon. All 13 redrive tests pass on the test commit through tc-gate; the builder's seven-file run passed 73 tests, with additional consumer and mail-delivery tests checked during review.

The CLI integration uses a fixture provider and exercises CLI send through this changed flow; it does not claim a real-agent or app run. CI supplies static analysis, typechecking and affected tests on the final head.

[coderabbitai]

Walkthrough

The client delivery now supports per-session journal-publication observers and invokes them after journal publication. The session host exposes observer registration. Structured worker sessions use the observer to schedule coalesced redrives. Updated tests cover redrive behavior, observer registration, and cleanup.

Priority: Normal

Merge Risk: Low · up to 68b9c

Unexpected worker exits can leave parked mail waiting longer than intended because the post-release state change does not wake its redrive observer.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv…	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check		The description is mostly complete and well-reasoned. It has a clear Problem section, a precise Scope section, a Change section that explains the mechanism, and a detailed Validation section. It also …	Add an ELI5 section in plain language. Add a Why section that compares this approach with the alternatives considered, including why the existing chat-stream subscription was not kept. Replace the Linked Issue placeholder with a real issue …

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title is concise, specific, and accurately describes the main change: preventing settled orchestration workers from continuing to read resting slash lists.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 6 functions across 5 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Description check

Explanation

The description is mostly complete and well-reasoned. It has a clear Problem section, a precise Scope section, a Change section that explains the mechanism, and a detailed Validation section. It also includes the LoC summary block. Several template sections are missing or incomplete. The ELI5 section is absent. The Why section does not explicitly compare the chosen approach against alternatives. The Linked Issue section has no 'Fixes #' issue reference, only a mention of #34141 in the body. The Visual Proof section is missing; the change affects backend orchestration only, so it should read N/A with a reason. The Testing checkboxes are not ticked, and the manual-testing item is not addressed. The AI Disclosure section is absent. The Review and Agent skill upstream boundary sections are absent. The Checklist is absent.

Resolution

Add an ELI5 section in plain language. Add a Why section that compares this approach with the alternatives considered, including why the existing chat-stream subscription was not kept. Replace the Linked Issue placeholder with a real issue reference such as 'Fixes #34141' if that is the tracked issue. Add a Visual Proof entry. Write N/A and a short reason, because the change has no UI change. Mark the Testing checkboxes that apply, and state the platforms actually tested, such as Windows, if manual testing was done. Complete or remove the AI Disclosure section as the template requires. Complete the Agent skill upstream boundary checkbox. Address the Notes items for Security, cross-platform, Remote SSH, Mobile, backwards compatibility, and performance. Tick or confirm the Checklist items.

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Minor · Notify journal observers from the restate… · structured-agent-session-client-delivery.ts:186-192

src/main/native-chat/agent-session-wire/structured-agent-session-client-delivery.ts:186-192
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Notify journal observers from the restate branch.

An unexpected child exit can publish the ordinary wake before settlement releases the worker. Release can then change the runtime fence without a nonempty journal commit. The later { restate: true } branch snapshots the new fence and returns without notifying journal observers. Parked mail can remain unrevaluated until another publication.

Notify the observers once from the restate branch. Keep the ordinary publishJournal(...) notification unchanged.

Suggested fix

     if (options.restate) {
       const fence = structuredAgentSessionConversationFence(this.deps().store, sessionId)
       this.subscribers.snapshot(sessionId, journal, fence)
+      this.notifyJournalObservers(sessionId)
       return
     }
@@
-    for (const observer of this.journalObservers.get(sessionId) ?? []) {
+    this.notifyJournalObservers(sessionId)
+  }
+
+  private notifyJournalObservers(sessionId: string): void {
+    for (const observer of this.journalObservers.get(sessionId) ?? []) {
       try {
         observer()
       } catch (error) {
         this.deps().logger.warn('observing a journal publication failed', {
           scope: 'journal-delivery',
@@
         })
       }
     }
   }

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 23302493-c84b-4088-bfe3-d7536e3bd67d

Commits

Reviewing files that changed from the base of the PR and between c69e9d0 and 68b9c21.

Files selected for processing: 1

• src/main/runtime/rpc/methods/orchestration-structured-worker-redrive.test.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.

[brennanb2025]

Settled orchestration workers were keeping invisible chat streams alive, causing resting command discovery for chats nobody was viewing. Workers now observe journal publications directly, so parked mail still retries while those workers create no command-list demand. Visible desktop and phone menus keep their existing behavior. Catalog/process bounds are separate work.

Full-path review completed with three validation passes; the fresh decider ruled KEEP on this head. Correctness, architecture, consumer, reference and readiness checks found no PR-caused production regression. The fixes removed a prohibited test assertion and added the missing CLI, moved-path and scale evidence; production code stayed unchanged during the loop.

Verified: real CLI send through actual socket/RPC/runtime and production worker-topology wiring to fixture-provider pointer delivery and durable receipts. The former stream and new observer both retain mail after a throwing send, deliver after a later journal publication, and cancel pending retries on release. Thirteen redrive tests pass after the final test-loading correction (68b9c21144e); the 16/160-worker contract shows one observer visit for one localized publication, one coalesced wake per worker for bursts, zero client/catalog reads, and zero post-disposal callbacks. Existing tests cover tombstone/reset/reattach/row-free changes and release/stop/abandon. This is the orchestration/performance tests row, with a fixture provider. It does not claim a real-agent or app run.

The conflict-free main merge c69e9d025f9 changed no PR production file. The later test-only commit 68b9c21144e uses the existing runtime CLI-entry loading pattern to preserve TypeScript project boundaries and explicitly types the population fixture array; every outcome assertion is preserved. Final-head CI passed after one failed-jobs-only retry of the unchanged WSL stop-settle test. The first attempt’s only unit failure was that test; this PR’s 13 redrive tests passed in the same shard. The retry establishes intermittence, not the WSL gate’s root cause. A pre-existing pending coalescer may fire after whole-host shutdown on both carriers; no user-visible failure was established, and this PR does not claim to fix it. No open blocking finding or product decision remains.

For Brennan’s review: observer and disposal, worker binding, CLI outcome comparison and scale contract. #28567 overlaps subscribeForRedrive; this PR changes only its subscription-call block. No rendered flow changed, so screenshots were not required.