fix(nikcli): release the herdr pane from the process that actually exits - #233
Conversation
Herdr clears an agent on its own only for the agents it detects by
process. nikcli is reported, not detected, so its row stayed in the agent
panel until the pane's shell itself exited — dispose() called
HerdrBridge.stop(), which only resets local runtime state.
releasePaneSync() shells out to the herdr CLI because process.on("exit")
cannot await a socket write. No signal handlers: nikcli reads ctrl+c as a
key in raw mode and owns its quit path, so taking SIGINT here would change
behaviour unrelated to herdr. A hard kill still leaves the row, same as
every other integration.
Fixes #232
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
lgtm |
The plugin's dispose() is not reachable on Windows. shutdownWorker() takes
the non-terminating branch there: it unrefs the worker and fires the
shutdown RPC without awaiting, so Instance.disposeAll() — and with it the
herdr plugin's dispose — is cut off when the TUI process exits. The agent
row survived until the pane's own shell exited.
Release from the TUI process instead, right after shutdownWorker returns.
That is the process that is about to exit on every platform, so this needs
no change to the worker teardown contract and does not make quit wait on a
hanging worker.
Drops installExitRelease(): a `process.on("exit")` listener registered in
the worker is exactly what worker.ts warns against, and it never fired on
the path that matters.
Fixes #232
SandroHub013
changed the title
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
stop() resets runtime.released, so calling it after releasePaneSync() could let a late report reclaim the pane. Flip the dispose order and pass windowsHide on the synchronous CLI spawn. Co-authored-by: Cursor <cursoragent@cursor.com>
nikomatt69
left a comment
nikomatt69
left a comment
There was a problem hiding this comment.
Review (local verification, 2026-08-18)
The Windows analysis is right: shutdownWorker() on win32 unrefs and does not await, so plugin dispose() never runs and HerdrBridge.stop() never handed the pane back. Releasing from the TUI process with a synchronous CLI spawn is the process that actually exits.
I pushed one follow-up on this branch (dc302edf9b) because the original dispose order had a real bug:
releasePaneSync()setruntime.released = trueHerdrBridge.stop()then setruntime.released = false
stop() is documented to reset local runtime state, including released. A late reportAgent after dispose could reclaim the pane. Dispose now does stop() then releasePaneSync(), and the CLI spawnSync gets windowsHide: true (repo convention; otherwise Windows can flash a console on quit).
Verified
- Overlay of this branch onto
live-main+bun test test/plugin/herdr/release-on-exit.test.ts test/tui/thread.test.ts→ 10 pass, 0 fail (argv contract, no-pane no-op, stop-then-release, and existing thread shutdown tests including “quit does not block on a hanging Windows worker”) - CI
typecheckpassed on the original commits - Merges clean onto current
live-main(no conflicts)
CI that is red is not this PR
test (windows): Bun remap / exit 255, same as #225/#227smoke (windows):SystemPrompt.Servicetimed out at 5s — unrelated to herdr; dispose log line ran and did not hang (noHERDR_PANE_IDon the runner)- Linux test still queued
Related, not this PR: #231 (Windows named-pipe dial) was closed without merge. Socket releasePane() still cannot work on Windows until that lands. This CLI path is the correct Windows workaround; please reopen #231 separately so reporting (not just release) works there.
releasePaneSync() no-ops if herdr is not on PATH / HERDR_BIN_PATH. That matches resolveHerdrBin(); worth knowing if a user still sees a sticky row.
2 participants
Issue for this PR
Closes #232
Type of change
What does this PR do?
Herdr drops an agent by itself only for the agents it detects by process. nikcli is reported, not detected, so its row stays until someone calls
pane.release_agent. Nothing did —dispose()calledHerdrBridge.stop(), which only resets local runtime state.Fixing
dispose()alone is not enough, and that's the interesting half. On WindowsshutdownWorker()takes the non-terminating branch: it unrefs the worker and fires the shutdown RPC without awaiting it.Instance.disposeAll()never finishes before the TUI process exits, so no plugin dispose runs at all. macOS and Linux await it, which is why the row only sticks on Windows.So the release happens in the TUI process, right after
shutdownWorker()returns — the process that is actually about to exit on every platform. No change to the worker teardown contract, and quit still doesn't wait on a hanging worker.releasePaneSync()shells out to the herdr CLI instead of using the socket, because the shutdown path has no event loop left to await a write on. A hard kill (taskkill /F) still leaves the row, same as every other herdr integration.How did you verify your code works?
bun test packages/nikcli/test/plugin/herdr/release-on-exit.test.ts packages/nikcli/test/tui/thread.test.ts: 9 pass. The argv assertion covers the contract herdr keys agent authority on;thread.test.tsstill proves quit doesn't block on a hanging worker.tsc --noEmitclean onpackages/utilandpackages/nikcli."agent":"nikcli"to no agent at all. Before, the row survived the process and only went away when the pane's shell exited.Reviewers on Windows:
report-protocol.test.tsfails on this branch for an unrelated reason — #230 / #231. Neither file touched here needs a socket.Screenshots / recordings
Not a UI change.
Checklist