[nwparker]

ELI5

Orca watches the programs running in your terminals so it can tell you when an agent
finishes. To avoid hammering the app it only allows itself 8 of those checks per second,
across every pane you have open — and it spent them one pane at a time.

That works fine with a handful of panes. With three hundred, 8 checks a second means each
individual pane gets looked at about once every forty seconds, even though the code believes
it is checking that pane every three quarters of a second. So the more agents you run, the
longer Orca takes to notice one has finished — exactly backwards from what you want.

But those checks were never eight separate things. Every local pane's answer is read out of
one snapshot of the machine's process list, which Orca already captures at most twice a
second no matter how many panes ask. Eight panes asking is one look at the process list; three
hundred panes asking is also one look at the process list.

So the allowance was counting the wrong thing. It now counts looks at the process list instead
of panes. The number stays at 8 per second; it just buys three hundred panes now instead of
eight.

Why it matters

The budget is a hard global cap:

• MAX_INSPECTION_STARTS_PER_SECOND = 8 in
  src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts, shared by
  every pane in the app.
• The tiers it is supposed to serve are active 750ms / idle 2000 / hidden 3000 /
  no-evidence 15000 (agent-completion-poll-cadence.ts).

A pane re-enqueues once per tier interval, and the queue was FIFO one-at-a-time, so the
effective per-pane period was max(tier, N/8 seconds). Every pane past the eighth was served
at the cap's rate, not its tier's. The reporting session had 905 terminal leaves; at a few
hundred panes holding agent evidence the effective period is tens of seconds for a pane the
code polls at 750ms–3000ms. That is user-visible as "Orca didn't tell me my agent finished",
and it gets worse monotonically as panes are added.

Note on what this is not: it is not a subprocess-churn fix. getProcessTableSnapshot()
already sits behind a 500ms TTL plus in-flight dedupe (src/shared/process-table-snapshot-reader.ts),
on Windows too (readWindowsProcessTable), so the ps/process-table capture rate is ~2/s
app-wide regardless of pane count. That TTL is exactly what makes a whole round of local reads
one host observation, and it is what the limiter should have been counting all along.

What changed

Three renderer files. No IPC, preload, wire, or main-process change — each pane keeps its
existing per-pane pty:inspectProcess invoke.

• agent-process-inspection-queue.ts — canStartInspection becomes availableInspectionStarts,
  the pump drains every sharesHostObservation task as one round on one start, and starts the
  round's tasks synchronously so they all land in the same tick (and therefore the same
  process-table capture window). enqueueAgentProcessInspection now pumps on a deduplicated
  microtask instead of inline, so a synchronous burst of enqueues forms one round — pumping
  inline spent a start per pane until the concurrency slots filled and then parked the rest of
  the burst on the 100ms retry. Unshared tasks keep the old one-start-each admission and the
  old pending-title priority. Disposed tasks are compacted out in one pass instead of a
  splice per drop, so the per-round predicate cost is linear rather than quadratic at pane scale.
• agent-completion-process-monitor.ts — sets sharesHostObservation from isRemotePtyId.
• agent-process-inspection-round.test.ts — new guard.

An earlier revision of this PR also added a pty:inspectProcessBatch IPC channel, a preload
method, a renderer coalescer, a batch entry type and a 256-entry chunk limit. That is dropped.
It carried all of the merge risk (new wire surface, per-entry guard threading, positional
short-reply semantics, capability fallback) and a failure mode that does not exist today — one
hung entry inside a Promise.all chunk withholding up to 255 sibling results — while main's
batch handler was itself only Promise.all over the identical per-entry inspection. It saved
invoke framing and nothing else. The measurement section below prices that framing.

Measurement

Deterministic counter guard, no wall-clock assertion. 300 panes enqueue a cadence
inspection; advance fake timers 200ms (well inside one rate-limiter window) and count how
many panes were actually inspected.

Before (queue file reverted to origin/main, everything else this branch):

 FAIL  src/renderer/.../agent-process-inspection-round.test.ts
   × inspects every pane of a 300-pane round inside the same admission budget
   × launches the round in one tick instead of one pane per admission
   × drops disposed panes out of the round instead of inspecting them

AssertionError: expected 8 to be 300 // Object.is equality
     47|     expect(inspected.size).toBe(PANES)
AssertionError: expected [ 4, 1, 1, 1, 1 ] to deeply equal [ 300 ]
     83|     expect([...launchesPerTick.values()]).toEqual([PANES])

 Tests  3 failed | 2 passed (5)

After: Test Files 1 passed (1) / Tests 5 passed (5)

Starts spent, same 200ms window, same 300 panes:

	starts	panes inspected	per-pane period at N=300
before	8	8	~37.5s (N/8)
after	1	300	tier + one admission slot

Verified at 50 / 300 / 800 panes: inspected=50 bursts=[50], inspected=300 bursts=[300],
inspected=800 bursts=[800] — one start, one tick, full coverage, at every scale.

What dropping the batch IPC costs

The one thing the batch bought was invoke framing, so it was priced directly rather than
argued about. Real Electron 43 IPC, hidden BrowserWindow, process.cpuUsage() deltas
sampled in both the renderer and the main process (this machine is loaded, so wall clock
is reported but not relied on), 60 rounds per measurement × 2 interleaved passes, 15 warmup
rounds. Both shapes run the identical per-entry host work behind a shared TTL'd snapshot, so
the only variable is framing. Harness: /tmp/orcaperf/ipc-bench-18438.

panes	shape	invokes/round	renderer µs CPU/round	main µs CPU/round	total µs CPU/round	µs CPU/pane	sync launch burst p95	round latency median
50	per-pane (shipped)	50	827	1088	1915	38.3	0.15 ms	0.60 ms
50	batch	1	126	123	250	5.0	0.10 ms	0.20 ms
300	per-pane (shipped)	300	3957	5504	9462	31.5	0.40 ms	3.00 ms
300	batch	2	476	466	942	3.1	0.10 ms	0.60 ms
800	per-pane (shipped)	800	10255	13741	23996	30.0	0.85 ms	7.70 ms
800	batch	4	1122	1462	2584	3.2	0.20 ms	1.00 ms

So the batch was worth ~10x on framing: ~30µs of aggregate CPU per pane-inspection versus
~3µs. Converted to steady state — inspections/s is N / tier, identical under both shapes,
because both inspect the whole round inside one window:

panes	tier	inspections/s	per-pane CPU/s	batch CPU/s	delta
300	active 750ms	400	12.6 ms/s (1.3% of one core)	1.3 ms/s	+11.4 ms/s
300	hidden 3000ms	100	3.2 ms/s (0.3%)	0.3 ms/s	+2.8 ms/s
800	active 750ms	1067	32.0 ms/s (3.2% of one core)	3.5 ms/s	+28.6 ms/s
800	hidden 3000ms	267	8.0 ms/s (0.8%)	0.9 ms/s	+7.1 ms/s

Three things put that in scale:

1. It does not touch the frame budget. The queue starts a round in one synchronous burst,
   so the jank-relevant number is how long that burst holds the renderer main thread. It is
   0.85 ms at p95 even when all 800 panes ride a single round — 5% of a 16.7 ms frame. The
   batch's is 0.20 ms. No frame is dropped at any tested pane count; the rest of the cost is
   completion handling spread across later ticks.
2. It is small against the capture the feature already pays for. One
   ps -axo pid=,ppid=,pgid=,tpgid=,stat=,tty=,lstart=,command= on this machine (1814
   processes, 453 KB) costs 254 ms CPU (30 ms user + 224 ms sys) in the forked ps, plus
   2.3 ms in the main process to drain the pipe. The 500 ms TTL caps that at 2/s app-wide, so
   the feature already spends ~510 ms CPU/s there at any pane count, on main today. The
   framing delta is 2.2% of that at 300 active panes and 5.6% at 800. (Both the numerator and
   that denominator are inflated on this box — 1814 processes is ~3x a normal machine — and
   they inflate together.)
3. Neither shape changes the win. Coverage and latency are identical: 300/300 and 800/800
   panes inspected inside one rate-limiter window either way. The batch only ever made the
   mechanism cheaper, never faster.

Against that, the batch cost a new IPC channel, a preload method with a capability fallback,
per-entry incarnation-guard threading, positional short-reply semantics, and a Promise.all
chunk in which one hung entry withholds up to 255 sibling results — a head-of-line stall that
does not exist when each pane owns its own invoke. Trading a few percent of one core for that
surface is the wrong side of the trade, so it is dropped. If the CPU ever becomes the binding
constraint, the batch is a self-contained follow-up that needs none of this PR undone.

For reference, main spends only ~0.24 ms CPU/s on inspection round trips at any N — because
it only ever performs 8 of them per second, which is the bug.

Correctness

No wire, IPC, preload or main-process surface changes. git diff origin/main --stat is
three files, all under src/renderer/src/components/terminal-pane/. Every pane still calls
the same pty:inspectProcess with the same arguments and gets the same reply; nothing about
what is computed, by whom, or how it is transported changes. Only when the renderer is
allowed to issue the call changes.

Verdict integrity (docs/reference/ssh-execution-boundary.md). Untouched. Each pane's
inspection is its own promise, its own rejection, and its own
classifyTerminalProcessInspectionFailure. There is no shared reply, no positional decoding,
and no path by which one pane's outcome is derived from a sibling's. The vocabulary stays
live / unverifiable / exited.

Per-pane incarnation guard. expectedIncarnationId is captured per request and rechecked
after the await (getExpectedIncarnationId() === expectedIncarnationIdAtRequest in
agent-completion-process-monitor.ts), exactly as before. The queue never inspects, reorders
or merges a task's arguments — it only decides which tick a task's run() is invoked on, and
run() closes over its own pane's state.

Remote wire compatibility (docs/reference/remote-wire-compatibility.md). No wire change
and no behavior change for remote panes. shouldRunCadenceInspection() already returns false
for remote PTY ids, so SSH and paired-runtime panes never cadence-poll; their only inspections
are pending-title ones, they are flagged sharesHostObservation: false, and they keep the
existing one-start-each admission and the existing pending-title priority. Old and new
clients are indistinguishable on the wire. The remote plan stays the one already recorded in
agent-process-inspection-cost.ts — consume the batched foreground evidence hosts already
publish on the pty.listProcesses inventory (#17525) — which this PR does not pre-empt.
src/main/providers/ssh-pty-provider-rpc-operations.ts is untouched.

Does the 8/s cap still need to exist? Yes, and it is numerically unchanged. What was wrong
was its unit: it counted panes when it should have counted host observations. A start is now
one observation — a shared round costs one however many panes ride it, an unshared (remote)
task costs one each. MAX_CONCURRENT_INSPECTIONS = 4 is likewise unchanged and now bounds
concurrent rounds. A round holds its concurrency slot until its last task settles, so a slow
pane cannot let the next round overlap it.

Priority ordering. The shared round drains before the unshared pending-title scan within
the same pump. This is not starvation: a local pending-title read is itself
sharesHostObservation: true and rides that very round, and a remote pending-title read is
served in the same pump from the 7 remaining starts. Raised (non-blocking) by pullfrog; the
split does not change the ordering it reviewed, and the reasoning still holds.

Edge cases. SSH and paired-runtime panes: unchanged path, unchanged admission, still
hidden/no-cadence. Folder workspaces: this path keys on PTY id only and never touches
worktree state. Windows: isAgentProcessInspectionCostly and the no-evidence 15s tier are
untouched, readWindowsProcessTable has the same TTL+dedupe that makes the round shared, and
the existing agent-completion-no-evidence-cadence.test.ts guard still passes. Empty queue,
all-disposed queue, and clock-jump-backwards (now < inspectionStarts[0]) all keep their
existing handling.

Negative user-facing trade-offs

None user-facing. Every pane still gets the same inspection, computed by the same code,
transported the same way, with the same guard; both the admission budget and the cadence tiers
are numerically unchanged, and no pane waits on a window its own cadence did not already
contain.

One measured non-user-facing cost, stated plainly: because each pane keeps its own invoke,
this spends ~10x the IPC framing CPU that the dropped batch revision would have — ~30µs of
aggregate renderer+main CPU per pane-inspection instead of ~3µs. In absolute terms that is
+11.4 ms CPU/s at 300 panes all holding a live agent, and +28.6 ms/s at an adversarial 800
(1.3% and 3.2% of one core, split across two processes). It does not cost a frame — the
synchronous launch burst is 0.85 ms at p95 with all 800 panes in one round — and it is a few
percent of the ~510 ms CPU/s the process-table captures behind this feature already cost on
main at any pane count. It is a real number and it is the price of not adding the batch's
wire surface and head-of-line-stall failure mode. Table and method above.

Linked Issue

N/A — no tracking issue. This came out of a performance session on a report with 905 terminal
leaves, where the per-pane cadence tiers were silently degrading to max(tier, N/8 seconds).

Visual Proof

N/A — nothing rendered changes. There is no new UI, no layout, color, typography or component
change, and no new interaction: the same inspection runs for the same panes with the same tiers
and the same global budget. The only observable difference is when an existing completion
signal fires at high pane counts, which is not screenshottable at 300 panes; it is pinned instead
by the deterministic counters in agent-process-inspection-round.test.ts
(inspected=300, bursts=[300], red on origin/main with expected 8 to be 300) and by the
CPU-delta table under Measurement.

Test plan

• nice -n 10 node_modules/.bin/vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane src/renderer/src/runtime — 596 files / 5474 tests pass (and 667 files / 6141 with src/main/ipc/pty added).
• ... src/main/ipc/pty-session-liveness-and-ownership.test.ts src/main/providers — 85 files / 868 tests pass.
• New agent-process-inspection-round.test.ts: 300-pane coverage guard (red proof above),
  single-tick round launch on a single start (with 7 unshared panes still fitting in the same
  window, which is what proves the round cost one start), remote panes still admitted one start
  at a time at N=300,
  pending-title still prioritized, disposed panes dropped out of the round.
• tsc --noEmit -p config/tsconfig.node.json and -p config/tsconfig.tc.web.json both clean.
• pnpm run check:code-quality:changed — 0 new findings across 3 changed files.
• oxfmt --write + oxlint clean on all changed files.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: ef0eacda-098a-4e7d-99d6-19017cee455c

📥 Commits

Reviewing files that changed from the base of the PR and between 265f6a7b4caf5a7c69a8ace0bac7082691114f0c and ca019fb.

📒 Files selected for processing (2)

• src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts
• src/renderer/src/components/terminal-pane/agent-process-inspection-round.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The inspection queue batches tasks that share a local host observation into one round. It limits remote inspections individually. The queue computes concurrent and per-second admission capacity, removes disposed tasks, preserves pending-title priority, and defers pumping through a deduplicated microtask. Tests cover shared-task bursts, remote limits, priority handling, and disposed-task filtering.

Merge Risk: ⚪ Minimal · up to ca019

Local terminal inspections now share a single observation round while remote panes retain individual admission. The change preserves rate limiting and priority behavior with coverage for the updated scheduling paths, leaving no actionable current-head merge risk.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The description explains the problem, implementation, testing, performance impact, and visual proof in detail. However, it omits several required template sections, including Testing with the required…	Add all required template sections and complete them. Provide a valid linked issue instead of "N/A", or obtain an explicit repository exception. Add the Testing checkboxes, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Ch…

✅ Passed checks (3 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly summarizes the primary change: one inspection start now covers a complete terminal cadence round.

Full details: Description check

Explanation

The description explains the problem, implementation, testing, performance impact, and visual proof in detail. However, it omits several required template sections, including Testing with the required checkboxes, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist. It also provides "N/A" for Linked Issue even though the template requires an issue link.

Resolution

Add all required template sections and complete them. Provide a valid linked issue instead of "N/A", or obtain an explicit repository exception. Add the Testing checkboxes, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist sections.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

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

• Batched inspection IPC — pty:inspectProcessBatch fans the per-pane inspectOnePtyProcess (verbatim extraction of the old pty:inspectProcess body) out over a batch, settling each entry independently as { inspection } | { error } with no shared state between entries.
• Queue round coalescing — the inspection queue now drains all batched: true tasks as one round (one "start"), keeps unbatched/remote panes admitted one-at-a-time, and drops disposed tasks in a single compacting pass instead of splice-per-drop.
• Renderer coalescer — inspectLocalTerminalProcessCoalesced merges same-tick local reads into one microtask-flushed IPC call, chunked at TERMINAL_PROCESS_INSPECTION_BATCH_LIMIT (256), with a per-pane fallback when inspectProcessBatch is absent (web/older preload).
• Routing — inspectRuntimeTerminalProcess keeps remote ids (remote:/SSH) on the per-pane path and only routes local ids through the coalescer; the monitor sets batched from isRemotePtyId.

I traced the verdict-integrity, per-entry incarnation, and capability-fallback paths end-to-end and found them sound:

• inspectPtyProviderProcessForRenderer classifies known failures into clientOnlyUnverifiableInspection before throwing, so the batch's { error: message } carry is equivalent to the single-pane IPC rejection when the renderer re-classifies — code is never the deciding factor for errors that reach the catch.
• The 300-pane regression test is a real guard: it asserts an exact count (toBe(300)) plus a round-trip budget, not a truthiness stand-in.
• availableInspectionStarts accounting is consistent — a batched round and each unbatched task each consume exactly one start, so activeInspections ≤ MAX_CONCURRENT_INSPECTIONS and inspectionStarts.length ≤ MAX_INSPECTION_STARTS_PER_SECOND both hold after a full pump.

One thing I verified but chose not to flag as blocking: remote panes only ever enqueue pending-title (their cadence polls are gated off), and batching reduces local start consumption, so the priority-order change (batched rounds drain before unbatched pending-title) has no meaningful starvation effect — the queue re-pumps on the next tick rather than 100ms when a start is freed mid-round.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[nwparker]

Design changed since the bot pass — this PR is now renderer-only

@pullfrog reviewed 4381a9a, which is no longer the head. That revision bundled two
independent changes; the batched IPC half has been dropped entirely and the branch is now
three renderer files with no IPC, preload, wire or main-process surface at all. git diff origin/main --stat:

 .../terminal-pane/agent-completion-process-monitor.ts     |   3 +
 .../terminal-pane/agent-process-inspection-queue.ts       | 122 ++++++++++++++-----
 .../terminal-pane/agent-process-inspection-round.test.ts  | 135 +++++++++++++++++++++

Gone: pty:inspectProcessBatch, inspectProcessBatch on PreloadApi,
local-terminal-inspection-batch.ts, TerminalProcessInspectionBatchEntry,
TERMINAL_PROCESS_INSPECTION_BATCH_LIMIT, the inspectOnePtyProcess extraction in
src/main/ipc/pty/ipc/inspect.ts, and the routing change in runtime-terminal-inspection.ts.
Each pane keeps its existing per-pane pty:inspectProcess invoke. So the three findings
pullfrog traced end-to-end and cleared — batch verdict integrity, per-entry incarnation
threading, capability fallback — no longer have any code to apply to.

On the one point it raised but did not block (batched rounds draining before unbatched
pending-title): the ordering is unchanged by the split and the reasoning still holds, with
one addition. A local pending-title read is itself sharesHostObservation: true, so it rides
the very round that drains first rather than queueing behind it; a remote pending-title read
is unshared and is served in the same pump from the remaining 7 starts. There is a new
regression test for the remote case (still serves a pending-title read ahead of the queued cadence backlog). Kept as-is.

The framing cost the batch was buying has been measured rather than assumed — real
Electron 43 IPC, process.cpuUsage() deltas in both processes, at 50/300/800 panes. The batch
was worth ~10x on framing (~3µs vs ~30µs of aggregate CPU per pane-inspection), which is
+11.4 ms CPU/s at 300 active panes and +28.6 ms/s at 800 — 1.3% and 3.2% of one core, against
the ~510 ms CPU/s the ps captures behind this feature already cost on main at any pane
count. Coverage and latency are identical under both shapes. Full table and method in the PR
body; that trade is now stated in the negative-trade-offs section rather than the section
claiming "None".

@coderabbitai was rate-limited on the previous head and produced no findings; it is welcome to
review the current one.

On the e2e / ssh docker watcher isolation failure

It failed on 4381a9a in
ssh-cold-hydration-gap-tab-seeding.spec.ts:180
— "adds no tab when the host workspace snapshot stalls across a relaunch", a 120s poll
timeout waiting for isTargetHydrated after the blocked remote workspace snapshot is released.
That is the SSH cold-hydration/tab-seeding path; this branch never touched it, and it does not
touch src/main or src/preload at all any more. The same check is green on the neighbouring
merges (#18389, #18378). Re-running on the split head.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 5bd011e5-cca4-45c5-a200-bae94b6336da

📥 Commits

Reviewing files that changed from the base of the PR and between 7106101 and 265f6a7b4caf5a7c69a8ace0bac7082691114f0c.

📒 Files selected for processing (3)

• src/renderer/src/components/terminal-pane/agent-completion-process-monitor.ts
• src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts
• src/renderer/src/components/terminal-pane/agent-process-inspection-round.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Changed files ==='
git diff --name-status origin/main...HEAD

echo '=== Diff ==='
git diff --find-renames --find-copies --unified=80 origin/main...HEAD -- \
  src/renderer/src/components/terminal-pane/agent-completion-process-monitor.ts \
  src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts \
  src/renderer/src/components/terminal-pane/agent-process-inspection-round.test.ts

echo '=== Queue outline ==='
ast-grep outline src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts --items all

echo '=== Monitor outline ==='
ast-grep outline src/renderer/src/components/terminal-pane/agent-completion-process-monitor.ts --items all

echo '=== Test outline ==='
ast-grep outline src/renderer/src/components/terminal-pane/agent-process-inspection-round.test.ts --items all

echo '=== Existing queue test references ==='
rg -n -C 3 --glob '*agent-process-inspection*.test.ts' \
  '(enqueueAgentProcessInspection|requestInspection|pending-title|sharesHostObservation|dispose)' \
  src/renderer/src/components/terminal-pane

Length of output: 34426

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== All enqueue call sites ==='
rg -n -C 5 '\benqueueAgentProcessInspection\s*\(' src

echo '=== Inspection option and callback definitions ==='
rg -n -C 8 '\binspectProcess\s*[:=]|\binspectProcess\s*\(' \
  src/renderer/src/components/terminal-pane \
  -g '*.ts' -g '*.tsx'

echo '=== Process inspection/cache implementation candidates ==='
rg -n -C 8 \
  '(inspectProcess|process-table|process table|inFlight|in-flight|TTL|ttl|dedup)' \
  src/renderer/src \
  -g '*.ts' -g '*.tsx'

echo '=== Queue rejection-containment test ==='
cat -n src/renderer/src/components/terminal-pane/agent-process-inspection-queue-rejection-containment.test.ts

echo '=== Poll scheduler request behavior ==='
ast-grep outline src/renderer/src/components/terminal-pane/agent-completion-poll-scheduler.ts --items all
sed -n '1,260p' src/renderer/src/components/terminal-pane/agent-completion-poll-scheduler.ts

Length of output: 201406

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Files mentioning pty:inspectProcess ==='
rg -l -g '*.ts' -g '*.tsx' -g '!**/*.test.*' "'?pty:inspectProcess'?|inspectProcess" \
  src/main src/preload src/renderer/src/components/terminal-pane 2>/dev/null || true

echo '=== Exact inspectProcess registrations and invokes ==='
rg -n -C 12 -g '*.ts' -g '*.tsx' \
  "(handle|invoke|register).*['\"]pty:inspectProcess['\"]|['\"]pty:inspectProcess['\"]" \
  src/main src/preload src/renderer/src/components/terminal-pane 2>/dev/null || true

echo '=== Candidate local inspection cache files ==='
fd -t f -e ts -e tsx . src/main src/renderer/src/components/terminal-pane 2>/dev/null \
  | rg '(inspect|inspection|process).*\.(ts|tsx)$' \
  | rg -v '\.test\.' \
  | head -80

echo '=== Monitor factory construction and inspectProcess binding ==='
rg -n -C 15 -g '*.ts' -g '*.tsx' \
  "createAgentCompletionProcessMonitor\s*\(|inspectProcess:\s*" \
  src/renderer/src/components/terminal-pane

echo '=== Current changed-file line numbers ==='
git diff --unified=3 origin/main...HEAD -- \
  src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts \
  src/renderer/src/components/terminal-pane/agent-process-inspection-round.test.ts

Length of output: 201745

---

@nwparker: I confirmed that this revision is renderer-only. The previous IPC, preload, and main-process findings do not apply.

I found one queue-admission issue.

enqueueAgentProcessInspection() starts the pump synchronously. Therefore, the first four local tasks each create a separate startInspectionRound() before activeInspections reaches MAX_CONCURRENT_INSPECTIONS. This consumes four slots and four rate-limiter starts. A remote pending-title task that arrives after these local tasks cannot use one of the stated seven remaining starts. It waits until a local task settles.

The new test permits this behavior with PANES - 4, so it does not verify one shared local round or the mixed local/remote priority case. Please defer the initial pump and coalesce queued local tasks before admission. Add a regression test with local tasks that remain in flight and a remote pending-title task. The test should verify that the local round uses one slot and the remote read starts in the same admission cycle.

You are interacting with an AI system.

[pullfrog]

✅ No new issues found.

Reviewed changes

• Dropped the batch IPC entirely — pty:inspectProcessBatch, the preload method, and the renderer coalescer are gone. The current head is renderer-only: three files, no wire/preload/main surface. The prior review's batch-shaped concerns (positional short-reply semantics, capability fallback, Promise.all head-of-line stall) no longer apply.
• Coalesced local inspections in the queue — InspectionTask gained sharesHostObservation, set from isRemotePtyId, and pumpInspectionQueue now drains every shared-observation task as one round on one start, then serves unshared (remote) tasks one-per-start with pending-title priority. Rate and concurrency caps still bound rounds, and MAX_INSPECTION_STARTS_PER_SECOND / MAX_CONCURRENT_INSPECTIONS are numerically unchanged.
• Added a regression guard — agent-process-inspection-round.test.ts covers the 300-pane round, single-tick launch, remote one-at-a-time admission, pending-title priority, and disposed-pane drop. It fails pre-fix (8 panes inspected instead of 300).

I traced the load-bearing invariant end-to-end and it holds: every non-remote pane's inspection resolves out of one TTL-and-in-flight-deduped capture (getProcessTableSnapshot on POSIX, readWindowsProcessTable on Windows via snapshotReader), while remote:/SSH panes each cost their own host round trip and stay sharesHostObservation: false. The round's synchronous launch, the outstanding-tracked concurrency slot (held until the last task settles), and the availableInspectionStarts count accounting are all consistent — no over-subscription of the 8/s or 4-concurrent budgets. The rejection-containment guard still passes against the rewritten queue.

The one rough edge — the first MAX_CONCURRENT_INSPECTIONS panes of a synchronous burst each start eagerly rather than riding the coalesced round — is explicitly priced in the PR body (5 starts for 300 panes, still within the 8/s budget) and was already raised by another reviewer; I don't consider it a must-fix.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[nwparker]

Note for @pullfrog: the incremental review above was written against 265f6a7, which is no longer the head. The one rough edge it named and declined to block on — "the first MAX_CONCURRENT_INSPECTIONS panes of a synchronous burst each start eagerly rather than riding the coalesced round" — is fixed in ca019fbf. @coderabbitai raised the same thing and was right to; the eager starts were harmless on their own (5 of 8), but they pushed the rest of the burst onto scheduleInspectionPump(100), which cannot be preempted while a timer is pending, so 296 of 300 panes waited the full 100ms on a 750ms tier.

enqueueAgentProcessInspection now pumps on a deduplicated microtask instead of inline, so a synchronous burst forms exactly one round on exactly one start: N=50 bursts=[50], N=300 bursts=[300], N=800 bursts=[800], previously [4,46] / [4,296] / [4,796]. The PR body’s "5 starts" line is updated to 1. The regression test now pins the cost rather than the shape — it enqueues 7 unshared panes alongside the round and requires all 7 to still run in the same window.

Details in the reply on the original thread.

[pullfrog]

No new review warranted. The head (ca019fbf) is unchanged since pullfrog's last review (#5108721133), which already traced the deduplicated-microtask pump and the tightened 7-unshared-pane regression guard end-to-end and found no new issues — exactly the fix this note describes (N=50/300/800 → bursts=[50]/[300]/[800]). No commits have landed since, so there is no incremental delta to re-review.

Task list (5/5 completed)

• Checkout PR 18438 and identify incremental scope
• Read prior reviews and retire addressed threads
• Cover incremental + full diff, triage changes
• Dispatch specialists if unresolved questions remain
• Draft and submit review

  ^{｜ View workflow run ｜ via Pullfrog ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

queue_file="src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts"
test_file="src/renderer/src/components/terminal-pane/agent-process-inspection-round.test.ts"

echo '=== Current head ==='
git rev-parse --short HEAD
git show -s --format='%h %s' HEAD

echo '=== Changed files ==='
git diff --name-status origin/main...HEAD

echo '=== Queue structure ==='
ast-grep outline "$queue_file" --items all

echo '=== Queue implementation ==='
rg -n -C 18 \
  '\benqueueAgentProcessInspection\s*\(|\bscheduleInspectionPump\s*\(|\bpumpInspectionQueue\s*\(|\bstartInspectionRound\s*\(|\bMAX_(CONCURRENT_INSPECTIONS|INSPECTION_STARTS_PER_SECOND)\b|sharesHostObservation' \
  "$queue_file"

echo '=== Round test structure ==='
ast-grep outline "$test_file" --items all

echo '=== Relevant regression tests ==='
rg -n -C 20 \
  '(same.tick|same tick|unshared|remote|pending-title|pending title|300|800|microtask|round|start)' \
  "$test_file"

echo '=== Current diff ==='
git diff --unified=40 origin/main...HEAD -- "$queue_file" "$test_file"

Length of output: 27139

---

@nwparker Verified on ca019fbf.

enqueueAgentProcessInspection() now queues a deduplicated microtask. The first pump sees the complete synchronous burst.

pumpInspectionQueue() takes all shared local tasks into one startInspectionRound(). That round consumes one concurrency slot and one rate-limiter start. The queue then admits unshared tasks from the remaining budget.

The regression test enqueues 300 shared tasks plus seven unshared tasks. It verifies one shared launch tick and that all seven unshared tasks run in the test window. This covers the admission-cost regression that caused the original finding.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

[pullfrog]

✅ No new issues found.

Reviewed changes

• Coalesced the enqueue-time burst — enqueueAgentProcessInspection now pumps through a deduplicated microtask (queueInspectionPump + inspectionPumpQueued) instead of inline, so a synchronous 300-pane burst forms one round rather than spending a start per pane until the concurrency slots fill and parking the rest on the 100 ms retry. This addresses the earlier "coalesce the initial burst" feedback.
• Tightened the round-launch guard — the test now asserts exactly one tick carrying PANES (toEqual([PANES]), no longer the loose >= PANES - 4) and adds seven unshared panes that must still fit in the same window, which is what proves the round cost one of the eight starts.

I traced the microtask pump against the existing timer pump (scheduleInspectionPump) and the settle path: both re-read availableInspectionStarts and re-account activeInspections/inspectionStarts fresh, so a redundant pump can't over-spend the budget, and the 100 ms timer remains a backstop when starts are exhausted. pending-title priority and the disposed-drop are unchanged, and the whole-round synchronous launch still lands every read in one tick. The round, rejection-containment, and no-evidence cadence guards all pass against the new code.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Since the prior pullfrog review of 4381a9a, the batched IPC half has been dropped entirely and the branch is now renderer-only (git diff origin/main is three files, no IPC/preload/wire/main surface):

• Removed the batch IPC surface — pty:inspectProcessBatch, the PreloadApi method, local-terminal-inspection-batch.ts, TerminalProcessInspectionBatchEntry, TERMINAL_PROCESS_INSPECTION_BATCH_LIMIT, the inspectOnePtyProcess extraction, and the runtime-terminal-inspection.ts routing change are all gone. Every pane keeps its per-pane pty:inspectProcess invoke.
• Renamed batched → sharesHostObservation — the monitor now sets the flag from isRemotePtyId (options.isRemotePtyId?.(ptyId) !== true), so local reads coalesce into a shared round and remote reads stay on the one-start-each path.
• Reworked admission to a count — canStartInspection became availableInspectionStarts (min(concurrency headroom, rate headroom)), and startInspectionRound launches a whole shared round synchronously on a single start, holding its slot until the last task settles.
• Added agent-process-inspection-round.test.ts — five guards, including the new remote pending-title regression test.

The three findings traced end-to-end in the prior review (batch verdict integrity, per-entry incarnation threading, capability fallback) have no code to apply to after the split. The one non-blocking point about shared-round-vs-pending-title ordering still holds, and its reasoning is now backed by the new remote regression test. Queue accounting (concurrency slot bounded by MAX_CONCURRENT_INSPECTIONS, rate window bounded by MAX_INSPECTION_STARTS_PER_SECOND) stays consistent after a full pump, and the disposed-task compaction is correctly linear. The measured framing tradeoff is now stated plainly in the negative-trade-offs section rather than claimed as "None".

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Since the prior pullfrog review (265f6a7), one commit (ca019fb) landed:

• Coalesced the initial burst via a deduplicated microtask — enqueueAgentProcessInspection no longer pumps inline; it defers to queueInspectionPump, a queueMicrotask guarded by inspectionPumpQueued, so a synchronous burst of enqueues forms one round instead of spending a start per pane until the concurrency slots fill and parking the remainder on the 100ms retry. resetAgentProcessInspectionQueueForTests clears the new flag.
• Strengthened the single-start guard — the "launches the whole round" test now enqueues 7 unshared panes alongside the 300 shared ones and asserts [...launchesPerTick.values()] === [PANES] plus unshared === 7, which is what proves the shared round consumed exactly one of the eight starts (the 7 unshared still fit in the remaining budget) rather than the looser >= PANES - 4.

The microtask path is correct: a synchronous burst queues once, pumpInspectionQueue drains the shared round and up to the concurrency headroom of unshared tasks, then re-pumps via the existing 0ms/100ms timer path with the inspectionPumpTimer !== null guard still preventing double scheduling. The settle path and rate/concurrency accounting are unchanged, tasks are removed from the queue before run() so nothing double-runs, and the flag reset is safe because a stale pending microtask only re-enters an empty queue. queueMicrotask is already idiomatic across the renderer. I ran agent-process-inspection-round.test.ts, agent-process-inspection-queue-rejection-containment.test.ts, and agent-completion-no-evidence-cadence.test.ts — all pass.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[nwparker]

Cleared the two @coderabbitai pre-merge warnings:

• Description check — fixed. The body was missing the template's ## Linked Issue and
  ## Visual Proof sections; both are added. Linked Issue is N/A (no tracking issue — this came
  out of a perf session on a 905-terminal-leaf report), and Visual Proof is N/A because nothing
  rendered changes: no new UI, no layout/color/type/component change, no new interaction. The only
  observable difference is when an existing completion signal fires at high pane counts, which is
  pinned by the deterministic counters in agent-process-inspection-round.test.ts rather than by a
  screenshot.
• Docstring coverage (36% vs an 80% threshold) — declining, as a threshold that conflicts with
  this repo's stated convention. AGENTS.md asks for "Concise/Brief Non-obvious Comments ONLY —
  DO NOT explain the obvious... BE CONCISE. 1 LINE if possible". The functions the check counts as
  undocumented are the ones where a docstring would only restate the name (queueInspectionPump
  has one, scheduleInspectionPump/pumpInspectionQueue are pre-existing and unchanged in intent).
  The three genuinely non-obvious things here — why a start is a host observation rather than a
  pane, why the round launches synchronously in one tick, and why the pump moved to a microtask —
  all carry comments already, plus the catch-before-finally rationale that was there before.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@nwparker Thanks for the clarification.

The PR description now satisfies the template. N/A is appropriate for both sections in this renderer-only change. The deterministic queue tests are suitable evidence for the changed completion-signal timing.

I withdraw the docstring-coverage concern. The stated AGENTS.md convention correctly favors concise comments for non-obvious behavior. The existing comments cover the queue behavior that requires explanation.

🐇

---

✏️ Learnings added

Learnt from: nwparker
URL: https://github.com/stablyai/orca/pull/18438

Timestamp: 2026-09-04T03:37:52.926Z
Learning: In this repository, `AGENTS.md` requires concise comments only for non-obvious behavior and discourages docstrings that restate obvious function names. When reviewing TypeScript code, do not require docstrings for self-explanatory functions such as `scheduleInspectionPump` or `pumpInspectionQueue` when the non-obvious queue semantics already have focused comments.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.