Skip to content

fix(herdr): re-report pane agent state when the Herdr server is replaced - #4744

Closed
nahyeongjin1 wants to merge 1 commit into
Yeachan-Heo:devfrom
nahyeongjin1:fix/herdr-reporter-reassert
Closed

fix(herdr): re-report pane agent state when the Herdr server is replaced#4744
nahyeongjin1 wants to merge 1 commit into
Yeachan-Heo:devfrom
nahyeongjin1:fix/herdr-reporter-reassert

Conversation

@nahyeongjin1

@nahyeongjin1 nahyeongjin1 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

What

The Herdr reporter now detects that the Herdr server was replaced under a live pane and re-asserts the session's agent state and title, instead of staying silent until the session happens to change state.

Why

A gjc session running inside a Herdr pane disappears from Herdr's sidebar the moment the Herdr server is replaced under it — a server restart, or herdr update --handoff, which explicitly keeps pane processes alive across the swap.

The replacement server starts with an empty agent registry, and custom:gjc reports are the only thing that would repopulate it. But the reporter is deduplicated against the last state it sent:

const report = (state: HerdrAgentState): void => {
    if (released || state === currentState) return;

A session waiting at its prompt is already idle, so it never reports again. It stays invisible until it changes state — for an idle session, that means until the user types into it.

Measured on a 13-pane workspace after herdr update --handoff: every pane process survived, and herdr agent list returned exactly one agent — the pane that happened to be mid-turn. The other twelve sessions were alive, working directories intact, and absent from the sidebar. The same gap follows any herdr server stop + herdr cycle, so it is not specific to the update path.

What this changes

Detection is the socket's identity, not its existence. Herdr names its API socket in HERDR_SOCKET_PATH, and a replacement rebinds that same path:

before stop : inode=310402479
after start : inode=310402664

The identity is (inode, ctime), not the inode alone. Linux hands the just-freed inode straight back to the socket that replaces it — verified identical across unlink/bind under oven/bun:1 — so an inode-only comparison sees no change and never fires. macOS does not reuse the number. The pair is correct on both.

The watch is on the containing directory, because the socket itself is unlinked and recreated; a watch bound to the old inode would die with it.

An event schedules a bounded re-check instead of deciding immediately. Replacement arrives as a single coalesced rename event, and at that instant the path frequently has no inode at all. An event now debounces 150 ms and re-checks up to 20 times while the path is empty.

On replacement the reporter clears its state memo, re-sends the current state, and re-sends the last reported title — the replaced server's metadata store is empty too, and the session name lives in the session, so nothing else would ever resend it. Panes whose environment names no socket keep exactly the previous behavior, and every failure mode stays best-effort: an unwatchable directory costs the re-assert, never a session.

Testing

bun test packages/coding-agent/test/herdr-pane.test.ts — 50 pass, on macOS and on Linux (docker run --rm -v $PWD:/work -w /work oven/bun:1).

Seven new cases: state re-report on replacement, sequence strictly rising so the new server accepts it, state memo preserved across the re-assert, title re-sent, watch disposed on release(), no watch when the environment names no socket, and socket path carried through resolveHerdrPaneEnvironment.

The last case drives the real watcher against a real socket file — create, unlink, recreate — and asserts exactly one re-report. That test is what caught both portability bugs above; the six injected-watcher tests passed against each broken implementation.

GJC verdict

gajae.pr-review-verdict.v1 needs-human sha256:f85f5dce731d88adfc29a196522fa7766df46efcd90226d2635bf57d614e8319 reviewer:human reviewer-id:nahyeongjin1 evidence:https://github.com/Yeachan-Heo/gajae-code/actions/runs/32325229381

  • Target branch is dev
  • bun test packages/coding-agent/test/herdr-pane.test.ts passes (macOS + Linux)
  • Tested locally
  • CHANGELOG updated (if user-facing)
  • Verdict above matches the exact PR head, not an earlier commit

A Herdr server restart or `herdr update --handoff` replaces the server
under live panes and starts with an empty agent registry. The reporter
deduplicates against the last state it sent, so a session sitting at its
prompt never reports again and stays invisible in Herdr's sidebar until
it happens to change state — for an idle session, that means until the
user types into it.

Watch the pane's API socket and re-assert on replacement. Replacement is
unlink-then-bind on the same path, so the socket's inode is the signal,
and the watch is on the containing directory because the socket itself
is recreated. The event usually arrives while the path has no inode at
all, so an event schedules a bounded re-check instead of deciding at
once. The last reported title is re-sent with the state, since the
replaced server's metadata store is empty too and nothing else would
ever resend it.

Co-Authored-By: Claude <noreply@anthropic.com>

@Yeachan-Heo Yeachan-Heo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking defect confirmed on exact head 5704de400914d16ab8fd541a24c01af46997e170. CI run 32324071869, job 96292725556, has 49 pass / 1 fail: the real replacement test times out on Linux. I reproduced it locally before the repair.

Root cause: watchSocketReplacement() decides replacement solely from statSync(socketPath).ino. Linux immediately reused the same inode for an actual Unix-domain socket close/rebind, so the post-debounce value equaled seen and the watcher never re-reported. The test used a regular file and a sleep loop, so it did not prove the production contract.

Exact repair is in maintainer PR #4745 at b0f0f0e0c77c2b474197cae68a3f7b4cacbfc234: it observes a real net.Server Unix socket, rejects symlink/non-socket identities with lstatSync, and retains a replacementPending bit when the matching directory emits rename (or omits its filename):

if (event === "rename" || !filename) replacementPending = true;
// after bounded settle/retry:
if (current === seen && !replacementPending) return;
seen = current;
replacementPending = false;
onReplaced();

The replacement test awaits the semantic second report-agent invocation rather than polling with sleeps and asserts exactly one re-report. Validation: 20 fresh focused runs (50/50 each) and bun --cwd=packages/coding-agent run check.

Fork permission is pull-only (nahyeongjin1/gajae-code push:false); the repair is therefore owned on authorized upstream branch fix/herdr-socket-reassert-repair. Owner lane: review/merge #4745 into dev; #4744 remains blocked. Its zero verdict line also keeps the exact-head contract red.

@Yeachan-Heo

Copy link
Copy Markdown
Owner

Superseded by #4745 to preserve a single canonical PR for this item. Evidence: current #4744 head 9e68a134 does not contain the Linux socket-inode-reuse repair; #4745 contains original contributor commit 5704de400914d16ab8fd541a24c01af46997e170 plus tested repair b0f0f0e0c77c2b474197cae68a3f7b4cacbfc234 (+52/-26 across the two Herdr files). Credit for the original server-reassert implementation remains @nahyeongjin1 / #4744.

The original CI is obsolete with this close. Canonical review, verdict, CI, and merge are tracked only on #4745.

@nahyeongjin1

Copy link
Copy Markdown
Contributor Author

Small correction for the record — no objection to #4745 as the canonical PR.

Head 9e68a134 does contain the inode-reuse repair. watchSocketReplacement() on that head compares identity() returning ${stats.ino}:${stats.ctimeMs} (packages/coding-agent/src/utils/herdr-pane.ts), precisely so a recycled inode number does not read as unchanged.

The 49 pass / 1 fail in run 32324071869 is the pre-force-push head 5704de400. That failure is real, and it is what prompted the repair: I reproduced it under oven/bun:1 (ino before/after: 954859 954859, reused = true; ctimeMs differing), fixed the identity, and force-pushed. Dev CI run 32325264933, which ran on exact head 9e68a134, reports Affected path validation / test:packages/coding-agent/test/herdr-pane.test.ts as success, alongside ts-build, native-build, evidence producer, install-methods and the gjc-state-gates set. The only red on that run is the needs-human contract gate, which is working as intended.

So #4745's base commit 5704de400 is one commit behind the branch as it stood at close. The rename-as-proof approach in b0f0f0e0 is the more conservative of the two — it also covers a same-millisecond ctime collision, which the pair comparison does not — so the outcome is better either way. Just noting the repair was not missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants