Repository navigation
fix(pending): bind a queued message to the agent process as command outcomes do, and read its birth without blocking - #1986
Merged
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 13 minutes. View limit details
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 |
Owner
Author
|
Merge pass note: this PR conflicts with #1980 (macOS process metadata read from the kernel). Both are open maintainer PRs, and Whichever is approved and green first merges first, and the other one then needs a rebase. Both touch |
Owner
Author
|
CodeRabbit's review limit was reached on this PR, so no CodeRabbit review ran. This PR changes queued-input identity, so here are the code-review agent reviews that stand in, with how each finding was resolved. omo-native-code-reviewer (rebased commit
Codex (gpt-6.1-sol, xhigh), two rounds on the rebased branch, each with reproductions it ran:
Codex also confirmed that the rebase's conflict resolutions in No HIGH or MEDIUM finding is open. |
Owner
Author
|
Merge pass review of ac58e89. The Astra gate (omo-native-gate-reviewer, gpt-6-astra) rejects it on a HIGH blocker in the queued-input path. What holds: the launcher regression test and 57 targeted unit tests pass. My local run was 262/262, and typecheck is clean. BLOCKER G1 (HIGH): a queued message can reach a replacement agent while the async identity read is running (
The full reproduction code and outputs are in section G1 of the gate report. Native macOS, native Windows and full CI were not verified separately. This PR touches |
added 6 commits
October 26, 2026 18:43
…utcomes do, and read its birth without blocking Pending sends and command outcomes each decided which agent process a send went to, by different rules. Pending took the first process in front of the pane, so an agent behind a launcher that does not exec it (an npm claude wrapper, a shell script) was never the bound process, and a restart behind the launcher went unnoticed. Both now pick through agentProcessInFront (server/agent-process.ts): the bound or launch-reported process while herdr lists it, else the agent's own program before an interpreter or launcher running it. With a bound pid, outcomes also require it to still be one of the agent's processes, as Windows hands a dead agent's pid on at once. currentIdentity read the process's birth with processStartedAt, which on macOS runs ps with Bun.spawnSync on the event loop, up to three times per pane event while a first message waits. It now awaits processStartedAtAsync, which runs ps as an awaited child, bounded to a second, and counts the birth from the clock taken as ps started, so a late answer does not move it. The client snapshot currentIdentity reads also ran a synchronous ps for each foreground process in OmO and GJC discovery; both now await the same read. Fixes #1545
…ng its session An argv-silent custom OmO is known only by its session holder, so agentProcesses does not list it, while the bundled Claude SDK child it runs each turn in matches omo by its omo-ai path. The rebased pick therefore bound a queued follow-up to that child; when the turn ended and the child exited, the unchanged OmO was taken for another agent and the message was held as pending_target_changed. currentIdentity now prefers the OmO process that holds a session (heldOmoProcess, the rule typing already uses in paneProcessRoots), reading the starts it compares without holding the event loop. Found by the Codex review of the rebase onto main.
…acOS claudePaneSession read the Claude process's start with processStartedAt, which runs ps with Bun.spawnSync on macOS. Every client snapshot starts the Claude subagent refresh that calls it, including the snapshot a pending message is checked against, so the server still stopped while ps answered. It now awaits processStartedAtAsync.
…ives it no cwd heldOmoProcess asked omoProcessIdentity with no fallback directory, so where herdr reports no cwd for a foreground process (macOS has no /proc to read it from) a custom OmO's holder could not be verified and its follow-up was again bound to the SDK child. OmO discovery already places such a process in the pane's own directory (paneFallbackCwd); the holder lookup now does the same. Found by the Codex re-review.
currentIdentity took the foreground listing before it awaited the process's birth, and did not look again. An agent that replaced it while the read was out was then checked against the old process's birth: the queued paste, or the Enter after it, reached the replacement. The listing is now read again after the awaited reads. A process no longer in front by then is not the one that runs, and the reads are made again from what is; while the foreground keeps changing, the latest process is named with no birth, so the message is held rather than sent. Found by the merge pass's Astra gate (G1).
… and Enter The check before a write awaited the screen read, and Pi's receipt anchor, after it named the process. An agent that replaced it meanwhile still received the paste or Enter. pendingContext now confirms its process is still in front after those reads, and the Pi path does so again after the anchor, so the window left is the write's own call: herdr cannot compare and send in one step. Follows the merge pass's Astra gate (G1).
devswha
force-pushed
the
fix/agent-process-identity
branch
from
October 26, 2026 09:43
ac58e89 to
7108808
Compare
Owner
Author
|
G1 is fixed in Changes:
Results for your two interleavings, plus a third one:
Test:
Runs on this head: 622 unit tests across 30 files ( Each check now makes one or two more |
Owner
Author
|
Merge pass re-review of 7108808 (Astra gate). The two original interleavings are fixed, but one awaited step still sits between the check and the paste. Fixed:
G1-R1 (HIGH): an await between the identity check and the plain paste (
This window is in the application's own code. It isn't a request for an atomic herdr compare-and-send. Before Enter, the added Pi recheck already covers the awaited receipt preparation. This PR touches |
…epared The check before the paste was followed by awaited steps of its own: the Claude and Hermes input-box checks and the OpenCode catalog. An agent that replaced the checked one meanwhile still received the plain paste. dispatchPendingText now confirms the process is still in front after them, before the message is marked written; typing skills already checks before every key. Found by the merge pass's Astra re-review (G1-R1).
Owner
Author
|
G1-R1 is fixed in Change: Test: a fourth case in The other writes in
Runs on this head: 651 unit tests across 30 files (adding the OpenCode skill and catalog suites to the earlier set) and the conventions tests pass. Under |
Owner
Author
|
Merge pass review report. Overall: PASSED. This PR changes queued input (pending sends), which AGENTS.md lists as needing a real review. Each round was reviewed by an Astra gate (omo-native-gate-reviewer, gpt-6-astra), which approved 07e8aa0 after three rounds. What it does: pending sends and command outcomes now pick the agent process with the same logic ( Findings and how each was resolved:
Tests: the focused suite passes 46/46, including the birth, screen and catalog races. They are deterministic regressions with explicit entry and release promises. Locally 265/265 pass, and typecheck is clean. Conflicts: this PR conflicts with #1980 ( |
devswha
pushed a commit
that referenced
this pull request
Oct 26, 2026
… however many reads it makes The case where a launch's agent leaves before any event scripted exactly one process read for the queueing. An identity read that reads the foreground again before it answers (#1986) then saw the agent gone at its queueing and the case failed, though nothing it checks changed. The agent now leaves from the second session snapshot on, which every identity read starts with, so the queueing's reads see it and every later read, whichever drain makes it, finds it gone.
devswha
added a commit
that referenced
this pull request
Oct 26, 2026
… and await name lookups' git (#1980) ## Change #1404 sampled the server's main thread on macOS waiting inside `Bun.spawnSync` while herdr answered in under a millisecond. #1748 bounded those waits at 1 s, but each one still held every connection while it ran. This change removes the synchronous subprocess calls from the paths that run per request or per status poll: - **macOS process metadata no longer runs `ps`.** `process-start.ts` (`processStartedAt`, `processStartLowerBound`, `processStartUpperBound[Async]`), `gjc-runtime.ts` `gjcTerminal` and `hermes.ts` `hermesTerminalId` read the process's start and controlling terminal from the kernel's `kinfo_proc` (`sysctl KERN_PROC_PID`, plus `devname(3)` for the tty), through `bun:ffi` on libSystem. `claude-store.ts` already made the same read for Claude on macOS; the helper moves to `server/darwin-process.ts`. The read takes microseconds and starts no process, so it **stays synchronous**: no caller changes, and no new "unknown" state reaches OmO resolution or pending input (the reason #1407 was closed). The start is now exact rather than `ps`'s whole seconds, and every comparison keeps its tolerance. - **`files.ts` `filesNamed`** (opening a file an agent named without its folder) awaits the streaming git listing `paneFiles` already used, which has the same 3 s deadline, `SIGKILL` and 16 MiB cap, instead of `Bun.spawnSync`. `file-view.ts` `locateFile`/`locateFileInFolder` become async; `findFile`, their only production caller, already was. - Lookups for one folder that arrive while its git listing runs share that listing, instead of each starting a git (the old synchronous lookup ran them one after another). - **`usage.ts`**: the keychain FIFO's `mkfifo` is awaited and killed after 5 s. - **CI**: the macOS job also runs `server/darwin-process.test.ts`, the only check that the kernel record's terminal field matches `ps`. This is a workflow change. Still synchronous, on purpose, because each runs once at startup or inside the owner guard, not per request: `process-owner.ts` (owner guard; its persisted `lstart` birth format is compared across restarts), `managed.ts` (`git rev-parse` before the supervisor starts), `remote-entry.ts` (Windows marker at bridge start), `pty/host.ts` and `pty/sidecar.ts` (PTY host detection). Refs #1404 (it also asks about the reporter's remaining stalls, which #1407's comments tie to a `Background` LaunchAgent `ProcessType`; this PR does not claim to close that). Covers the macOS half of #1545's "no spawnSync reachable from currentIdentity". ## Validation - **eamac** (macOS 27.0.1 arm64, Bun 1.4.3), a fresh clone of origin/main `7ab4bde0b` and then the same clone with this patch. A script runs the four metadata reads a status poll makes (`processStartedAt`, `processStartLowerBound`, `hermesTerminalId`, `gjcTerminal`) in a loop for 4 s. Meanwhile a 2 ms timer measures event-loop gaps and a separate process sends HTTP requests to the same server every 20 ms: | | one round of reads, p50 | longest timer gap | HTTP p50 / max | |---|---|---|---| | main | 6.6–6.8 ms | 14.6–20.7 ms | 2.7–3.0 / 14.5–14.8 ms | | this PR | 0.06 ms | 2.6 ms | 0.2 / 0.7 ms | | main, `taskpolicy -b` (background QoS, as the reporter's LaunchAgent ran) | 27.6–29.6 ms | 52.5–53.2 ms | 9.9–11.3 / 48–52 ms | | this PR, `taskpolicy -b` | 0.47 ms | 11.5 ms (69 ms once) | 0.6–0.7 / 10.6 ms (112 ms once) | The two background-QoS outliers come from the scheduler throttling a busy loop; no subprocess is started. - New `server/darwin-process.test.ts` checks the `e_tdev` offset and the NODEV/short-answer cases. On macOS it also checks that the kernel's start and tty match what `ps -o tty= -o lstart=` prints for a live child, and that a missing pid gives null. The kinfo_proc layout was checked with clang on eamac: `sizeof(struct kinfo_proc)` = 648, `offsetof(kp_eproc.e_tdev)` = 572. - New `files.test.ts` case: while a fake git blocks on a FIFO, `filesNamed` lets a timer run before it answers, and a second lookup of the same folder shares that one git (one `Bun.spawn`; without the sharing it makes two, checked). Rewiring `filesNamed` to `Bun.spawnSync` makes that test fail (checked, then restored). The existing "both inventories" test now asserts that `Bun.spawnSync` is never called. - macOS focused suites on eamac (darwin-process, process-start, claude-store, gjc-runtime, files, file-view, hermes, grok, muse-store, usage): 342 pass, 7 skip, 1 fail. The failure is #1971, which fails the same way on unchanged main (3 of 3 runs). - Linux focused suites: 347 pass, 3 skip, 0 fail. - `bun run check fast` on the final head (5f8b825, rebased on main after #1986): all pass, unit 7261 pass, 0 fail, 25 skip. On eamac with a real tty, the macOS process tests give 299 pass, 3 skip, 0 fail. - Reviews: omo-native-code-reviewer: APPROVE, no blockers. Its findings and how each was resolved: - MEDIUM: `darwin-process.test.ts` did not run in the macOS CI job. Fixed: `ci.yml` runs it. - LOW: comments still gave `ps`'s whole seconds as the reason for the macOS start tolerances. Fixed: reworded; the tolerances are unchanged. - LOW: concurrent lookups of one folder each started their own git. Fixed: one shared listing per folder. - LOW: the offset unit test writes at the offset the code reads, so it cannot catch a wrong offset. Covered by the macOS CI test, which compares with `ps`. - NIT: `sysctlCall()` makes a closure per call. Kept: negligible. - Codex Sol review did not run (host stall); the merge pass reviews. - Astra gate (merge pass) B1, PID reuse could pair one process's terminal with another's start: fixed in 4d5ac0d. `darwinTerminal` returns the tty and the start from one `kinfo_proc`, and `gjcTerminal` and Hermes's breadcrumb lookup take both from it. A new test feeds two records to one lookup and checks that only the first is read; going back to two reads fails it (checked). On eamac with a real terminal (`ssh -tt`, `/dev/ttys001`), the tty and start match `ps`; darwin-process, gjc-runtime, hermes and process-start tests: 56 pass, 3 skip, 0 fail. --------- Co-authored-by: mp <mp@local>
This was referenced Oct 26, 2026
devswha
added a commit
that referenced
this pull request
Oct 28, 2026
…sion, not its SDK child (#1985) (#2186) Fixes #1985 #1985 had two halves. The first was the synchronous macOS `ps` behind `paneProcessRoots`, `codexTranscriptPath` and `hermesTranscriptForPane`, and #1980 already removed it on main. The second was that command outcomes picked a custom OmO without the session-holder rule. This PR fixes the second. ## What changes for the user - **A command sent to a custom OmO keeps its row in Chat.** A custom OmO is one whose launcher does not name omo, so it is known only by the session-holder record it writes. Command outcomes (`server/prompt.ts` `outcomeProcess`) found it through `agentProcesses` alone. A `/command` sent to it was therefore bound to its bundled Claude SDK child, which matches omo by its `omo-ai` path and exits at the end of each turn. When the child exited, the row was never adopted into the session herdr then reported for the pane, and stayed out of it. Queued messages already preferred the holder (#1986). ## How - `bindCommandProcess(paneId, command, seen, pane)`: for an OmO send it reads `heldOmoProcess` once, the rule pending sends and typing use, and passes it to `agentProcessInFront`. The three callers (`core/composer.ts` twice, `core/pending.ts`) pass the pane. - A process bound as the holder is marked `held`, together with its start time, on macOS too. On macOS that start is the kernel's `kinfo_proc` record from #1980, not `ps`. The bound check (`checkBoundProcess`) accepts it while herdr lists it, even though `agentProcesses` does not name it, and reads no holders again on a poll. - A holder only counts while its process's start can be read. That is never the case on Windows, which hands a finished process's pid to the next one at once, so nothing changes there. Where the start is known, a later start under the same pid is another process, and the existing start comparison marks the send replaced. - Other agents are unchanged: `held` is only read for `omo`. ## The `ps` half (already on main) On eamac (macOS, Bun 1.4.3), in a `/tmp` clone of main `dbbaad5`, `processStartedAt` and `processStartedAtAsync` for a live child return the same exact start in 2.2 ms with no `Bun.spawnSync` call, and a missing pid gives null. `server/process-start.ts` has no `spawnSync`. `codex.ts` and `hermes.ts` still call the synchronous `processStartedAt`, which on macOS is now that kernel read, not a subprocess. ## How it was verified - New `server/outcome-omo-holder.test.ts` drives the real `GET /api/pane/prompt` (`handlePromptRequest`) with a custom OmO, its holder record and its SDK child, with no per-process cwd (as on macOS). It has four cases: 1. After the SDK child exits, the row is kept and adopted into the session herdr reports. This fails on main: the send is bound to the SDK child (41002, not 41001). 2. A second holder of the same session moves before the bound one. The row is kept, and `heldOmoProcess` is not called on any poll. This fails on the first commit. 3. and 4. After the OmO exits, an unrelated Claude Code gets its pid, once with Linux and once with macOS mocked. The row is not carried into Claude's session. The macOS case fails on the second commit. - With only the bound-check half reverted, case 1 fails at the adoption step. - The outcome, pending, composer, omo and agent-process suites pass (259 tests across 18 files), as does typecheck. - `bun run check fast` passes on the final change before the last rebase (7937 unit tests, 0 fail). The rebase onto main brought in only plugin, Codex model and sidebar changes, and the 18 suites above pass again on it. - Reviews: - omo-native-code-reviewer approved the first commit with no HIGH or MEDIUM. Its LOWs were the per-poll cost, fixed below, and the fixed fixture pids, the same pattern as `core/pending-omo-holder.test.ts`. - Codex (gpt-6.1-sol, fast, xhigh) reproduced three MEDIUMs over two rounds, each now a test case above: - With two holders, a reorder dropped the bound one. - `heldOmoProcess` ran on every poll, 194 to 815 ms of synchronous reads with thousands of holder folders. - The recorded holder accepted a reused pid on macOS, where no start was recorded. - Codex's last round, on the final delta, approved with no findings.
This was referenced Oct 28, 2026
devswha
added a commit
that referenced
this pull request
Oct 28, 2026
…2207, #2190, #2092) (#2237) Closes #2207 Closes #2190 Closes #2092 Three follow-ups to the question and outcome ownership reviews. All three fail closed: when the bridge cannot tell which send or call a question belongs to, it gives no card or ownership rather than a wrong one. ## What was wrong - **#2207** (`server/outcome.ts`): an earlier same-command send could lend its reserved echo count to a later one even if its echoes had been counted on screens read the other way (cut at Claude's fullscreen diff panel, or uncut). Once the oldest reservation expired, the later send skipped its own echo and could claim the question of the same command typed in the terminal. - **#2190** (`server/omo.ts`, `server/prompt.ts`, `server/core/pending.ts`): binding a queued OmO send's outcome read the session holders again and took the first one herdr listed, so two holders listed in a different order moved the outcome to the other OmO (and that OmO's exit removed the row). The holder check also dropped the start time it had verified, so a pid reused before the next start read was recorded as the holder. - **#2092** (`server/dialogs/omo.ts`): a clipped opened OmO form that fit two questions of one call gave no match for that call, so a decoy call that fit once was taken as the unique match. The answer route then saved the answer into the wrong call. ## What changes - `reserveResultEchoes` only uses an earlier send's count, or its history-row proof (`printedPastEarlier`), when that send was counted the same way as this one (`countedCut`). The existing reset after a predecessor whose result was observed and whose anchor scrolled off still applies. - `heldOmoProcess` returns the holder with the start its record was checked against, and prefers the pid a send is already bound to. `currentIdentity` returns the holder only when its pid settled in front and its birth equals that verified start. A queued send passes it to `bindCommandProcess(…, held)`, which then reads no holders, and `sentToProcess` keys a held OmO by the verified start. - `askedQuestion` reports `AMBIGUOUS` when more than one question of a call passes the full checks for a clipped form (choice mode, option count, question text, full labels). The form readers then give no card for any open call, so the answer route refuses with 409 `prompt_changed` after Alt+Up and sends no answer key. ## Tests (each fails on main) - `outcome.test.ts` "(#2207)": `ownsCommandDialog` is false for the third send (main: true). "…scrolled off": a send after a resolved predecessor counted the other way still owns its own question. This was a regression found in review round 1 and is kept as a guard. - `core/pending-omo-holder.test.ts` "(#2190)": the queued `/context` binds to holder A (main: B, 41003) with no extra holder read. `outcome-omo-holder.test.ts` "(#2190)": the binding keeps the verified start (main: the replacement's start 9000). - `prompt.test.ts` "(#2092)": the parser gives no card for the clipped form while the actual and decoy calls are open (main: the decoy card), and still gives the decoy card when the other call's questions cannot be on screen. The answer route returns 409 `prompt_changed` with only `alt+up` sent (main: 200, answer saved). Local: `bun run typecheck` green; outcome (114), pending (63), omo (42) and prompt/dialogs (740) test files pass; `bun run check fast` green. ## Reviews omo-native-code-reviewer and Codex Sol 6.1 Fast xhigh, adversarial and read-only. - Round 1: two MEDIUMs, both fixed in this PR. #2092 counted ambiguity on the rough first-line match, so a call whose questions could not be on screen hid the valid card (found by both reviewers). #2207's guard ran before the resolved-predecessor reset (found by Codex). - Round 2 (Codex, the two fixes only, last round): PASS, no CRITICAL/HIGH/MEDIUM. Its sandbox could not open the fake socket for the #2092 route test, so the 409 route was verified locally (`prompt.test.ts`, 740 pass across prompt and dialogs). LOW notes left as is: - If the foreground never settles over three reads, a queued OmO send's holder is not passed on, so the binding falls back to the agent's own program. - After the diff panel opens or closes, same-command sends own nothing until a send is counted the same way again. This is the intended fail-closed behaviour. - Direct (unqueued) OmO sends still read the holder records at binding; tracked in the follow-up issue. ## Left on #2190 - "Two OmO processes in one pane that both hold sessions bind to the first one herdr lists (same rule as #1986)" is unchanged for a fresh send: nothing tells the two apart before a send is bound. Once bound, the send now keeps its holder.
devswha
added a commit
that referenced
this pull request
Nov 1, 2026
…aude Code runs as it starts (#2526) <!-- See CONTRIBUTING.md for checks, translations, contract tests and the changelog (a file in changelog.d/, never CHANGELOG.md). --> ## Change Fixes #2525. A first message from the New tab or New workspace dialog now goes to Claude Code once it is ready. Until now it was often held with "the pane's terminal or agent session changed" (`pending_target_changed`), or sometimes "the agent started for this message has ended" (`first_message_launch_ended`). Nothing sent it after that until the user pressed Send now. The create route queues the first message right after herdr's `agent.start`, before herdr names the agent. `queueFirstMessage` then bound it to the first process in front. While Claude Code starts, that is often a child it runs (`git`, `gh auth token`, `security`, `sh -c ps`). When the child exited, the next pane event found another process and held the message. - `server/core/pending.ts` (`queueFirstMessage`): with no launch pid, the message binds at queue time only to a front process that runs the launched kind's program (`runsAgentProgram`, the test `currentIdentity` already uses for `recognized`). In every other case the process stays unbound. `learn()` binds the agent once herdr names it. A launch with the shell still in front already takes this path. - `server/core/pending.test.ts`: one `it.each` with three cases (see Validation). - `changelog.d/first-message-startup-child.md`: a `### Fixed` entry. The change keeps the guards from #1545 and #2222: - #1545 (fixed in #1986, 1f8bb08): a queued message binds to the agent's own process, not to a launcher that herdr lists before it. That rule lives in `currentIdentity`'s pick (`agentProcessInFront`), which this change does not touch. When herdr names the agent at queue time, the pick returns the agent's process, `runsAgentProgram` accepts it, and the message binds at queue time as before. An npm-style launcher (`node …/claude`) also counts as the kind's program (`agentProcesses`), so a launcher binding is unchanged too. - #2222 (9b17565): a first message stays bound to the agent it was queued under, and a same-kind replacement gets `pending_target_changed`. When the agent itself is in front at queue time, nothing changes: the bind happens at queue time, as before. The third unit case checks this with herdr naming no agent yet. The contract test from #2222 passes with this change. - What changes: only a front process that is not the launched kind's program, and not the shell, is no longer bound at queue time. That process is never the agent the message is for. ## Validation - `server/core/pending.test.ts`, new `it.each` (codex stands in for the agent, `git` for the child): - A child in front at queue time, then herdr names the agent: the message stays `first_message_unattended` (no page, so held from the start), not `pending_target_changed`. - A child in front at queue time, then the child ends before herdr names the agent: the message stays `first_message_unattended`, not `first_message_launch_ended`. - The agent in front at queue time, then a same-kind replacement: `pending_target_changed`. This is the #2222 guard. It passes on `main` and with the fix. - On `main` (8d28829 source, new test kept): 40 pass, 2 fail. The first case gets `pending_target_changed`, the second gets `first_message_launch_ended`. - With the fix: 42 pass, 0 fail. - `bun test server/core/pending scripts/changelog-fold.test.ts`: 104 pass, 0 fail (5 files). - `bun test server/conventions.test.ts src/conventions.test.ts`: 20 pass. - `bun run typecheck`: exit 0. - Contract tests through `bun run check run`, `main` (8d28829) and this branch side by side, each on its own isolated herdr session. Both ran on the Node PTY host. - `server/pending-input.contract.test.ts`, 3 runs each. `main`: 105, 103 and 104 pass (7, 9 and 8 fail). This branch: 105, 104 and 104 pass (7, 8 and 8 fail). - The same 7 fail in every run on both, and also fail on `main` on this Mac (herdr 0.9.0): the Pi maximum-length receipt, the #1545 test (its control fails here: herdr lists the agent before the launcher), promotes once, the model list hint, malformed framing (2), and the first message behind a wrapper. - 2 more fail now and then on both: "arbitrates completion racing with a click" (`main` 2 of 3, this branch 0 of 3) and "sends over Claude's grey suggestion while the viewport is scrolled" (`main` 1 of 3, this branch 2 of 3). - The #2222 test ("is bound to the agent process it was queued under") passes in all 6 runs. - "refuses Send now of a held agent-only message while Codex's collapsed queue blocks it" failed once in my test runs's run with this change. It ran 6 times on each side here (3 full runs, 3 runs of it alone). `main`: 5 pass, 1 fail, with the same 15 s pending-frame timeout. This branch: 6 pass. The test uses a connection's own `submit` and never calls `queueFirstMessage`. The flake is on `main` too. - `server/conversation-push.contract.test.ts`: 7 pass, 0 fail on both. - Live A/B, from my test runs at feb3ae0 (same `pending.ts` code as `main`): - A real Claude Code (haiku) on an isolated herdr session. Two Ovis servers, `main` and the fix, in ABBA order. Headless Chromium at 1280×800. New tab, Claude, a first message, Start. - `main`: held in 11 of 17 launches (9 `pending_target_changed`, 2 `first_message_launch_ended`). One batch held 1 of 7, the next 10 of 10. Earlier batches held 23 of 32. - With the fix: held in 0 of 18 launches. Sent 4377 ms after Start (median, p90 4903). The answer showed at 6533 ms (p90 7781). - `main` launches that were not held: sent at 4461 ms (p90 4616, n=5), answer at 6417 ms (p90 6778). The path that already worked is not slower. - Fisher exact test on the held count, 11/17 against 0/18: p ≈ 3e-5. - The lag harness scenario without a first message (S4) is unchanged: frame 1085 ms against 1066 ms on `main`. - Not run: `bun run check fast`. On this Mac it hangs on `server/usage-login-private`. These unit files also fail on `main` here: `scripts/check`, `scripts/self-hosted-runner`, `scripts/install-star`, `scripts/ci-browser`, `scripts/ci-browser-parallel`, `server/codex`, `server/codex-processes`, `server/session-history`, `server/letta`, `server/remote-entry`, `server/core/composer-framing`, `server/usage-login-private`, and sometimes `server/bridge`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01HabHntUPvy5yA9iU3G1m5y --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: devswha <devswha@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1545
Pending sends and command outcomes each decided which agent process a send went to, by different rules. Pending sends also read that process's birth with a synchronous
pson macOS, on the event loop. This makes both use one pick, and takes the synchronouspsoff the path a pending message is checked on.What changes for the user
claudewrapper, a shell script) is now held withpending_target_changedwhen the agent behind the launcher restarts, as it is for an agent started directly. Before, pending took the first process in front, which is the launcher. Its pid never changed, so the message went to the new agent.psanswers. Before, a first message's automatic drain ran up to three synchronouspscalls per pane event, and every client snapshot ran one more for each foreground process in OmO and GJC panes, and one for each Claude pane.How
server/agent-process.tsagentProcessInFront(info, agent, bound, held): the bound (or launch-reported) pid while herdr lists it, elseheld, else the agent's own program before an interpreter or launcher running it (agentProcesses), else none.currentIdentity(server/core/pending.ts) uses it, and falls back to the first process in front when herdr names no agent or none of its programs runs. That fallback is the old rule, solaunchGoneandbindLaunchedbehave as before.currentIdentitypassesheld: the OmO process that holds a session (heldOmoProcess,server/omo.ts), the rule typing already uses inpaneProcessRoots. An argv-silent custom OmO is known only by its holder, and its bundled Claude SDK child, which matches omo by itsomo-aipath, exits at the end of each turn; withouthelda follow-up would be bound to that child and held when the turn ended.outcomeProcess(server/prompt.ts) uses it too; with a bound pid, the process must still be one of the agent's, because Windows hands a dead agent's pid on at once.processStartedAtAsync(server/process-start.ts): on Linux the same/procbirth as before. On macOS,psruns as an awaited child, bounded to a second with SIGKILL like the synchronous reader, and the birth is counted from the clock taken aspsstarted. A late answer therefore cannot push the birth pastsamePendingIdentity's one-second tolerance.processStartUpperBoundAsyncshares the read.currentIdentityreads the foreground again after its awaited reads; if its process is no longer in front, the reads are made again from what is (at most three times), and while the foreground keeps changing it names the newest process with no birth, so the check fails closed.pendingContext, the check before the paste and before Enter, confirms the process is still in front after its own screen and card reads, and the Pi path does so again after its receipt anchor. What is left is the write's own call; herdr cannot compare and send in one step.currentIdentityandqueueFirstMessage; OmO discovery (omoPanes,omoSessionForPane, which pre-read the starts 8 at a time) andheldOmoProcess; GJC discovery (gjcPanesand its transcript lookup); andclaudePaneSession, which the Claude subagent refresh calls. All of these run inside, or are started by, every client snapshot.How it was verified
pending-input.contract.test.ts, "holds a queued message when the agent behind a launcher listed before it is replaced (One agent-process identity for pending sends and command outcomes, with no synchronous ps on macOS #1545)": a node launcher stays in front and starts the agent; the agent is killed and restarted behind it.core/pending.tsput back, it is delivered to the new agent and the test fails.core/pending-omo-holder.test.ts: a custom OmO known only by its holder, with its SDK child listed beside it. The follow-up is bound to the OmO and delivered after the child exits. The processes carry no cwd of their own, as on macOS. Without theheldpick, or without the pane-directory fallback, it is bound to the child (the test fails).core/pending-identity-race.test.ts: the agent in front is replaced while the check before the paste awaits its birth read, while the check before Enter does, and while the check before the paste reads the screen. A deferred promise holds each read open until the replacement is in place, with no sleeps. Nothing reaches the replacement: the message staysheld, oruncertainwhen the paste had already gone to the original. The first two fail on the previous head, and the third fails without the last check.agent-process.test.ts: launcher before native, the bound pid kept, the held OmO over its SDK child, the fallback.process-start.test.ts: on macOS nospawnSync, thepsargv and bounds, a killedpssays nothing, and a late answer does not move the birth.omo.test.ts,gjc-status.test.ts/gjc-runtime.test.tsandconversation.test.ts(claudePaneSession): on macOS, starts are read throughpschildren and never throughspawnSync. Putting the synchronous read back fails each of these tests.omo.test.tsalso pinsheldOmoProcess.pending-inputandoutcomepass on the final change before the rebase (99 pass), as doesclaude-sessionon the review fixes.claudePaneSessionpass (622 before the rebase onto the latest main, 606 of them rerun after it), as do typecheck and the conventions tests.bun run check fastpasses on the final change before the rebase (7206 unit tests). The rebase onto the latest main brought in onlysrc/terminal changes and one markdown test, which this PR does not touch.killSignalpinned, narrower changelog wording).pson to OmO and then GJC discovery, inside the client snapshot thatcurrentIdentityreads. Both are now async.core/pending.ts,omo.tsand their tests, where main added pi delivery receipts and held OmO holders), both reviewed the rebased commit again. omo-native-code-reviewer found no HIGH or MEDIUM issues. Codex found that the pick bound a custom OmO's follow-up to its SDK child (fixed by theheldpick above) and traced the snapshot-startedclaudePaneSessionto a synchronousps(now async). Its re-review of those two fixes confirmed both and traced no synchronous spawn left incurrentIdentity's call graph, including the background discovery a snapshot starts. It found one remaining case: where herdr gives a foreground process no cwd (macOS), the holder lookup could not place the custom OmO.heldOmoProcessnow falls back to the pane's own directory, as OmO discovery does. Codex's own reproduction of that case passes on this branch (bound to the OmO, delivered once, no synchronous spawn); there was no further Codex round, as Codex sessions were stalling on the host.paneProcessRoots(typing into an OmO or OpenCode pane, including a first message with an OpenCode skill mention) and the Codex and Hermes transcript lookups still read a process start synchronously on macOS, and command outcomes still pick an OmO without theheldrule. They are outside the check a pending message goes through; see Read the remaining process starts without a synchronous ps on macOS, and pick a custom OmO by its holder in command outcomes (#1545 follow-ups) #1985.Merge pass gate G1 (a replacement while a check's read was awaited reached the paste or Enter) is fixed by the two latest commits; see the reply comment.
This changes queued-input identity, so it needs a real review.