Repository navigation
fix(chat): find a herdr-core Claude behind a launcher or an hc attach tab - #3023
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 33 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. |
|
Review (omo + Codex Terra xhigh) on 436c21d: CLEAR. Remaining findings, none blocking:
Earlier round (9e8cb69): the reproduced HIGH (an |
|
Local merge gate (owner-approved): |
… PID record (#3047) ## Change On Windows, herdr and herdr-core name only a pane's shell in front, so `windowsShellClaude` (#2903) takes Claude from the process table: the nearest one below the shell, and any other not started by it. Claude Code's native build can run the session in a child `claude.exe` of the `claude.exe` the shell started, and only that child writes the PID record (`~/.claude/sessions/<pid>.json`). Seen on a real Windows 11 PC with herdr-core and Claude Code 2.1.304: ```text powershell.exe -> claude.exe (no options, no record) -> claude.exe --permission-mode ... (the record) ``` The lookup takes the launcher, which names no session and no store: - Chat: `GET /api/pane/conversation` answers `{"source":"scrollback","turns":[]}`. - Earlier sessions: 409 `claude_store_unknown`. - Resume: a Claude that comes up on such a chain has its new tab closed, 409 `launch_store_unknown`. Now, of two or more Claudes below the shell, #2903's rule runs over those holding a live PID record in a store Claude may use, once one does. Both callers share the lookup, so Chat and Earlier sessions get the same process. | Claudes below the shell | Before | Now | | --- | --- | --- | | None, or the table says nothing | `[]`, `null` | Same | | One, with or without a record | That one | Same, and no store is read | | Launcher and child, no record yet | The launcher: no session, store unknown | Same | | Launcher, and a child holding the record | The launcher | **The child** | | A holder, and a Claude beside it with no record | Both: refused | **The holder** | | Two holders, neither started by the other | Both: refused | Same | | A holder, and a Claude it started that holds one too | The nearest one | The outer holder | A Claude with no record names no session, so the one beside it with a record is the only session there is to show; if the other is still starting, the answer turns to refused once it writes its own. A record that cannot be read, or that two stores hold, may be the live one, so its process counts as a holder, and the callers still refuse that process's store (`AmbiguousClaudeStore`, #586). This is Windows only, as #2903 is: both callers reach `windowsShellClaude` only on `win32`, so Linux and macOS run none of it. #3023 fixed a launcher on macOS and Linux by reading the foreground job from the kernel; on Windows, where hc names only the shell, the process table and the PID record are what there is. Known limits: - An outer Claude that is itself the session but whose record cannot be read as live at that moment (torn, say), with a `claude -p` or SDK child below it that holds a record, now resolves to the child, where main finds no session. A live session's record is rarely rewritten (none in 8 minutes of watching one), so this is rare. - A `claude attach` client keeps no PID record: with one in front and a holder beside it, the holder's session is shown, where main refuses. Both must run under one shell. ## Validation - Failing first: the final tests on main's source (`0011d8fd`) fail 8 tests on Linux and on Windows: `server/conversation.test.ts` "finds a Windows pane's session by the child claude.exe holding the PID record, not the one its shell started" (`found` is null), `server/session-history.test.ts` "lists the store of the child Claude holding the PID record, not the launcher's" (`claude_store_unknown`) and "keeps a resume from a shell pane that came up on ~/.claude, and closes one on another store" (`[409, "launch_store_unknown", ["t-new"]]`), and 5 in `server/claude-store.test.ts`. All 8 pass with the fix. - Linux (WSL Ubuntu, Bun 1.4.2, clean checkout of this commit on `5b51cbe3`): `bun run typecheck` passes; `server/claude-store.test.ts`, `server/conversation.test.ts`, `server/session-history.test.ts`, `scripts/changelog-fold.test.ts`: 420 pass, 2 skip, 0 fail. - Windows 11, native (Bun 1.4.2), on this commit's rebase on `f2bbee9e`: `server/claude-store.test.ts` and `server/session-history.test.ts`: 156 pass, 43 skip, 6 fail; the same 6 fail on main on that PC (`EPERM` making a symlink, SQLite). `bun run typecheck`: exit 0. - Linux and macOS paths: the diff outside `windowsShellClaude` moves one `const home` above a `win32`-only block and passes it in. Tests set the platform to `linux` and `darwin`: a shell-only pane reads no process table, in Chat and in Earlier sessions. - Read-only dry run on a real PC (the real process table and the real PID records, no hc command, no socket, no server), for each shell with a Claude below it: | Shell | Claudes below it | Before | Now | | --- | --- | --- | --- | | `powershell.exe` 34500 | 15056 `claude.exe` (no record) -> 3760 `claude.exe --permission-mode` (record in `.claude`) | 15056 | **3760** | | `powershell.exe` 18424 | 46128 `claude.exe` (record in `.claude`) | 46128 | 46128 | The lookup took 0.9 ms and 0.1 ms with the table in hand. - Everything together on GitHub's hosted machines, this branch merged with the other open Windows herdr-core branches on `f2bbee9e`, on my fork: upstream's `ci.yml` ([run](https://github.com/photojupi/ovis/actions/runs/42078075843)): `bun run check fast` passes (unit: 12253 pass, 0 fail, 39 skip), and so do Integration tests, every Browser regression shard, Rust PTY host, the herdr-core live contract and Native Windows install. That job's `Release guards` step fails on a fork with no `v*` tags; with the tags it says this is no release. `Native macOS session identity` got no `macos-15` runner, twice; its test files ran in the whole unit suite below, with no failure among them. - macOS: a real hc 0.1.17 and the real server on GitHub's `macos-14` and `macos-26` (arm64) and `macos-15-intel`, with a stand-in agent: `.github/platform-check/e2e.ts` on my fork gives 19 ok, 0 failed on each ([arm64 run](https://github.com/photojupi/ovis/actions/runs/42084111704), [Intel run](https://github.com/photojupi/ovis/actions/runs/42078075706)): a workspace opens, the terminal takes typing through the API and the browser, the agent starts with `it's`, `$HOME`, `a\b`, `"q" r`, an empty argument and `한글 인자` intact, and closing leaves nothing. `server/herdr-core/live.test.ts` on that hc: 7 pass, 1 fail, the schema test, which pins hc 0.1.14 as `ci.yml` does and fails without these branches too. The whole unit suite on `macos-15-intel`, all these branches merged against their base: the base's 41 failures are the same on both sides (`tar --transform`, workflow files and the like, none in a file these branches change but `server/session-history.test.ts`, whose failures match by name); a first, slower run's two extra timeouts, in files these branches do not touch (`server/pi-delivery.test.ts`, `server/windows-native.test.ts`), did not come back on the rerun. No real MacBook: the Herdr app's own window on macOS was not checked. - Not run: `bun run check full` as one command; Chat, Earlier sessions and Resume end to end against a running herdr-core with a real Claude on such a chain (the dry run stops at the lookup). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: devswha <devswha@gmail.com>
Fixes #3009
What was wrong
On herdr-core (macOS and Linux),
terminal.getreports only the first process of a terminal's foreground job as itsforeground; herdr reports every process of the job. Chat looks for the Claude process in front of the pane to read its native PID record (~/.claude/sessions/<pid>.json), and herdr-core reports no session for Claude. So two common setups fell back to the terminal (GET /api/pane/conversation->{"source":"scrollback","turns":[]}), although hc labels both terminalsapp=claude-code:bun launch.js), Claude is its child.hc attach <t>: the foreground ishc attach t5, Claude runs int5.Fix
Only on a server serving herdr-core, and only when no Claude is in front (the Windows path from #2903 is unchanged):
/proc/<pid>/task/*/children,statandcmdlineon Linux; sysctlKERN_PROC_PGRPandKERN_PROCARGS2on macOS (darwin-process.ts, in the server's own thread, nops). New moduleserver/foreground-job.ts. Exactly one Claude there is taken; zero or two or more bind nothing, so the pane keeps the terminal as today.hc attach <t>: when the foreground ishc/herdr-corewith exactlyattach <word>, the Claude of that terminal is used, only when all hold: the client process reaches this server's core (its socket, worked out from the client's own environment as hc does it,discovery.tscoreSocket, is the socket of the core the server'shc connectreaches; an environment that cannot be read follows nothing), since terminal numbers repeat in every core; hc's ownpane.process_infoanswer for the word reports that same address (a name hc resolves to another terminal, or another machine'sx/t1, is not followed); and only one hop.The PID record still has to match the process's start (
/procticks,ps -o lstarton macOS), as before.Validation
Failing first: the new
conversation.test.tscasesfinds the one Claude a launcher runs, and its session by the PID recordandshows the conversation of the local terminal an \hc attach ` tab viewsfail on main (foundundefined) and pass with the fix. The fail-closed cases (two Claudes below; an attach word hc resolves to another address; an attach client that reaches another core) were checked to fail when their guard is removed. They use real stand-in processes (argv[0]claude`) and real PID records, with only the herdr RPC mocked.server/foreground-job.test.tsreads the real kernel: passes on Linux and on macOS (eamac, macOS 27.0.1 arm64, fresh/tmpreference clone).Focused:
server/conversation.test.ts,claude-store.test.ts,claude-session.contract.test.ts,session-history.test.ts(412 pass);bun run typecheckexit 0.Live, isolated hc 0.1.18 core (own
HC_SESSION, state/config/runtime folders), Ovis withOVIS_RUNTIME=herdr-core, tempHOME, telemetry off, a Claude stand-in that reports OSC 7501app=claude-codeand writes a PID record and transcript:bun launcher.js-> Claudescrollbackclaude-transcript(its turns)claude-transcriptclaude-transcripthc attach <that terminal>scrollbackclaude-transcript(the viewed terminal's turns)scrollbackscrollbackHC_SOCKET=<a second core> hc attach t8(that core's t8 runs another Claude; this core's t8 runs one too)scrollbackscrollbackSame result on Linux (P620) and on macOS (eamac, hc 0.1.18 macOS build; the second-core row on Linux only).
bun run check fast: all green after the review fix and a merge of main (12197 pass, 0 fail; run on old-desktop through the heavy runner).Not run:
check full, and a real signed-in Claude Code (the stand-in writes the same PID record format).Review round
omo + Codex (Terra, xhigh) on 9e8cb69: one reproduced HIGH, an
hc attachclient pointed at another core (HC_SESSION/HC_SOCKET) bound this core's same-numbered terminal. Fixed in 464f4a5/42da5b4 as above, with a test that fails without the check, and checked live with two cores.