From 3bfa202daf890395289c901feb41f9cedb0f6506 Mon Sep 17 00:00:00 2001 From: Alessandro Boni Date: Tue, 18 Aug 2026 16:16:37 +0200 Subject: [PATCH 1/3] fix(nikcli): hand the herdr pane back when nikcli exits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/nikcli/src/plugin/herdr/index.ts | 4 ++ .../test/plugin/herdr/release-on-exit.test.ts | 50 +++++++++++++++++++ packages/util/src/herdr-bridge.ts | 48 ++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 packages/nikcli/test/plugin/herdr/release-on-exit.test.ts diff --git a/packages/nikcli/src/plugin/herdr/index.ts b/packages/nikcli/src/plugin/herdr/index.ts index c4239e75a..6da9a1f55 100644 --- a/packages/nikcli/src/plugin/herdr/index.ts +++ b/packages/nikcli/src/plugin/herdr/index.ts @@ -311,6 +311,9 @@ export async function HerdrPlugin(_input: PluginInput): Promise { } else { log.info("running inside a Herdr pane; auto-enabling bridge") HerdrBridge.setEnabled(true) + // Nothing else clears the row: herdr only auto-drops agents it + // detects by process, so quitting nikcli has to hand the pane back. + HerdrBridge.installExitRelease() } } else { log.debug("not inside a Herdr pane; bridge stays disabled until manually enabled") @@ -319,6 +322,7 @@ export async function HerdrPlugin(_input: PluginInput): Promise { return { async dispose() { log.info("disposing herdr plugin") + HerdrBridge.releasePaneSync() HerdrBridge.stop() }, async event(input) { diff --git a/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts b/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts new file mode 100644 index 000000000..01a81d5ec --- /dev/null +++ b/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts @@ -0,0 +1,50 @@ +/** + * Shutdown contract for the herdr integration. + * + * Herdr only clears agents it recognizes by process. nikcli is reported, + * not detected, so quitting has to hand the pane back explicitly or the + * agent panel keeps a zombie row until the pane's shell exits. + */ +import { afterAll, afterEach, describe, expect, it } from "bun:test" +import * as bridge from "@nikcli-ai/util/herdr-bridge" + +const originalEnv = { ...process.env } + +afterEach(() => { + process.env = { ...originalEnv } + bridge.setReleased(false) +}) + +afterAll(() => { + // installExitRelease() leaves a real listener behind. Mark the bridge + // released so it cannot hand back the pane of whoever ran the suite. + bridge.setReleased(true) +}) + +describe("herdr shutdown", () => { + it("registers exactly one exit release, however many times it is installed", () => { + const before = process.listenerCount("exit") + bridge.installExitRelease() + bridge.installExitRelease() + expect(process.listenerCount("exit")).toBe(before + 1) + }) + + it("hands the pane back under the source herdr granted authority to", () => { + expect(bridge.releaseAgentArgv("w1:p1", 42)).toEqual([ + "pane", + "release-agent", + "w1:p1", + "--source", + "herdr:nikcli", + "--agent", + "nikcli", + "--seq", + "42", + ]) + }) + + it("does nothing when there is no pane to hand back", () => { + delete process.env.HERDR_PANE_ID + expect(() => bridge.releasePaneSync()).not.toThrow() + }) +}) diff --git a/packages/util/src/herdr-bridge.ts b/packages/util/src/herdr-bridge.ts index 1cb7743a6..e0dc1f814 100644 --- a/packages/util/src/herdr-bridge.ts +++ b/packages/util/src/herdr-bridge.ts @@ -34,6 +34,7 @@ * command) — never on import. This protects the chat session stream * from being hooked while the user has no pane registered. */ +import { spawnSync } from "node:child_process" import { createConnection, type NetConnectOpts, type Socket } from "node:net" import { platform } from "node:os" import fs from "fs/promises" @@ -556,6 +557,51 @@ export async function releasePane(input?: { paneId?: string; socketPath?: string }) } +/** + * Release the pane without an event loop. + * + * Herdr keeps a reported agent until someone releases it — it only clears + * agents it recognizes by process, and nikcli is not one of those, so a + * quit that skips the release leaves a zombie row in herdr's agent panel + * until the pane's shell itself exits. `process.on("exit")` cannot await a + * socket write, so the shutdown path goes through the herdr CLI instead, + * which is synchronous. + */ +export function releaseAgentArgv(paneId: string, seq: number): string[] { + return ["pane", "release-agent", paneId, "--source", HERDR_SOURCE, "--agent", HERDR_AGENT, "--seq", String(seq)] +} + +export function releasePaneSync(): void { + if (runtime.released) return + const paneId = process.env["HERDR_PANE_ID"] + const bin = resolveHerdrBin() + if (!paneId || !bin) return + runtime.released = true + try { + spawnSync(bin, releaseAgentArgv(paneId, nextReportSeq()), { stdio: "ignore", timeout: 2000 }) + } catch (error) { + log.debug("herdr release_agent (cli) failed", { error: errorMessage(error) }) + } +} + +let exitReleaseInstalled = false + +/** + * Release the pane when this process goes away. Idempotent, and only ever + * registered from a real herdr pane. + * + * Deliberately no signal handlers: nikcli reads ctrl+c as a key in raw + * mode and owns its own quit path, so intercepting SIGINT here would + * change behaviour that has nothing to do with herdr. A hard kill + * (`SIGKILL`, `taskkill /F`) still leaves the row behind — the same as + * every other herdr integration. + */ +export function installExitRelease(): void { + if (exitReleaseInstalled) return + exitReleaseInstalled = true + process.on("exit", releasePaneSync) +} + /** * Report a nikcli session as a herdr agent. No-op when the bridge is not * enabled or the socket is unreachable. Failures are logged, never thrown, @@ -982,6 +1028,8 @@ export const HerdrBridge = { handleEvent, handleChatMessage, releasePane, + releasePaneSync, + installExitRelease, isInHerdrPane, nextReportSeq, normalizeSnapshot, From 186e097da91f077e0b0a537516d5bc4c9c0409d3 Mon Sep 17 00:00:00 2001 From: Alessandro Boni Date: Tue, 18 Aug 2026 16:27:15 +0200 Subject: [PATCH 2/3] fix(nikcli): release the herdr pane from the process that actually exits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/nikcli/src/cli/cmd/tui/thread.ts | 7 +++++++ packages/nikcli/src/plugin/herdr/index.ts | 3 --- .../test/plugin/herdr/release-on-exit.test.ts | 15 +-------------- packages/util/src/herdr-bridge.ts | 18 ------------------ 4 files changed, 8 insertions(+), 35 deletions(-) diff --git a/packages/nikcli/src/cli/cmd/tui/thread.ts b/packages/nikcli/src/cli/cmd/tui/thread.ts index a390b0f6f..5fd802157 100644 --- a/packages/nikcli/src/cli/cmd/tui/thread.ts +++ b/packages/nikcli/src/cli/cmd/tui/thread.ts @@ -12,6 +12,7 @@ import { createNikcliClient, type Event } from "@nikcli-ai/sdk/httpapi" import type { EventSource } from "@nikcli-ai/tui/context/sdk" import { win32DisableProcessedInput, win32InstallCtrlCGuard } from "@nikcli-ai/util/win32" import { errorMessage } from "@nikcli-ai/util/error-format" +import { HerdrBridge } from "@nikcli-ai/util/herdr-bridge" import { Process } from "@nikcli-ai/util/process" import { SessionPrimitives } from "@nikcli-ai/util/session-primitives" @@ -265,6 +266,12 @@ export const TuiThreadCommand = cmd({ error: errorMessage(error), }) }) + // The worker owns the herdr plugin, but on Windows its shutdown is + // fire-and-forget (see shutdownWorker), so the plugin's dispose is cut + // off before it can hand the pane back. This is the process that is + // actually about to exit, so release from here. Synchronous and + // idempotent; a no-op outside a herdr pane. + HerdrBridge.releasePaneSync() simulation?.backend.stop() } diff --git a/packages/nikcli/src/plugin/herdr/index.ts b/packages/nikcli/src/plugin/herdr/index.ts index 6da9a1f55..91f637245 100644 --- a/packages/nikcli/src/plugin/herdr/index.ts +++ b/packages/nikcli/src/plugin/herdr/index.ts @@ -311,9 +311,6 @@ export async function HerdrPlugin(_input: PluginInput): Promise { } else { log.info("running inside a Herdr pane; auto-enabling bridge") HerdrBridge.setEnabled(true) - // Nothing else clears the row: herdr only auto-drops agents it - // detects by process, so quitting nikcli has to hand the pane back. - HerdrBridge.installExitRelease() } } else { log.debug("not inside a Herdr pane; bridge stays disabled until manually enabled") diff --git a/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts b/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts index 01a81d5ec..b0ad69112 100644 --- a/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts +++ b/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts @@ -5,7 +5,7 @@ * not detected, so quitting has to hand the pane back explicitly or the * agent panel keeps a zombie row until the pane's shell exits. */ -import { afterAll, afterEach, describe, expect, it } from "bun:test" +import { afterEach, describe, expect, it } from "bun:test" import * as bridge from "@nikcli-ai/util/herdr-bridge" const originalEnv = { ...process.env } @@ -15,20 +15,7 @@ afterEach(() => { bridge.setReleased(false) }) -afterAll(() => { - // installExitRelease() leaves a real listener behind. Mark the bridge - // released so it cannot hand back the pane of whoever ran the suite. - bridge.setReleased(true) -}) - describe("herdr shutdown", () => { - it("registers exactly one exit release, however many times it is installed", () => { - const before = process.listenerCount("exit") - bridge.installExitRelease() - bridge.installExitRelease() - expect(process.listenerCount("exit")).toBe(before + 1) - }) - it("hands the pane back under the source herdr granted authority to", () => { expect(bridge.releaseAgentArgv("w1:p1", 42)).toEqual([ "pane", diff --git a/packages/util/src/herdr-bridge.ts b/packages/util/src/herdr-bridge.ts index e0dc1f814..69535b7ec 100644 --- a/packages/util/src/herdr-bridge.ts +++ b/packages/util/src/herdr-bridge.ts @@ -584,23 +584,6 @@ export function releasePaneSync(): void { } } -let exitReleaseInstalled = false - -/** - * Release the pane when this process goes away. Idempotent, and only ever - * registered from a real herdr pane. - * - * Deliberately no signal handlers: nikcli reads ctrl+c as a key in raw - * mode and owns its own quit path, so intercepting SIGINT here would - * change behaviour that has nothing to do with herdr. A hard kill - * (`SIGKILL`, `taskkill /F`) still leaves the row behind — the same as - * every other herdr integration. - */ -export function installExitRelease(): void { - if (exitReleaseInstalled) return - exitReleaseInstalled = true - process.on("exit", releasePaneSync) -} /** * Report a nikcli session as a herdr agent. No-op when the bridge is not @@ -1029,7 +1012,6 @@ export const HerdrBridge = { handleChatMessage, releasePane, releasePaneSync, - installExitRelease, isInHerdrPane, nextReportSeq, normalizeSnapshot, From dc302edf9bb6379f067086d7927988214ccfe215 Mon Sep 17 00:00:00 2001 From: nikomatt69 Date: Tue, 18 Aug 2026 17:43:06 +0200 Subject: [PATCH 3/3] fix(herdr): release the pane after stop() and hide the Windows console 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 --- packages/nikcli/src/plugin/herdr/index.ts | 6 +++++- packages/nikcli/test/plugin/herdr/release-on-exit.test.ts | 6 ++++++ packages/util/src/herdr-bridge.ts | 6 +++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/nikcli/src/plugin/herdr/index.ts b/packages/nikcli/src/plugin/herdr/index.ts index 91f637245..30f528d0a 100644 --- a/packages/nikcli/src/plugin/herdr/index.ts +++ b/packages/nikcli/src/plugin/herdr/index.ts @@ -319,8 +319,12 @@ export async function HerdrPlugin(_input: PluginInput): Promise { return { async dispose() { log.info("disposing herdr plugin") - HerdrBridge.releasePaneSync() + // stop() resets runtime.released, so the CLI release has to run after + // it. Otherwise a late report could reclaim the pane we just handed + // back. The TUI process also calls releasePaneSync() because Windows + // never awaits this dispose. HerdrBridge.stop() + HerdrBridge.releasePaneSync() }, async event(input) { // Session lifecycle is already covered by the bridge's GlobalBus diff --git a/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts b/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts index b0ad69112..580e0de31 100644 --- a/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts +++ b/packages/nikcli/test/plugin/herdr/release-on-exit.test.ts @@ -34,4 +34,10 @@ describe("herdr shutdown", () => { delete process.env.HERDR_PANE_ID expect(() => bridge.releasePaneSync()).not.toThrow() }) + + it("can still hand the pane back after stop() resets local runtime state", () => { + delete process.env.HERDR_PANE_ID + bridge.stop() + expect(() => bridge.releasePaneSync()).not.toThrow() + }) }) diff --git a/packages/util/src/herdr-bridge.ts b/packages/util/src/herdr-bridge.ts index 69535b7ec..81d8dee03 100644 --- a/packages/util/src/herdr-bridge.ts +++ b/packages/util/src/herdr-bridge.ts @@ -578,7 +578,11 @@ export function releasePaneSync(): void { if (!paneId || !bin) return runtime.released = true try { - spawnSync(bin, releaseAgentArgv(paneId, nextReportSeq()), { stdio: "ignore", timeout: 2000 }) + spawnSync(bin, releaseAgentArgv(paneId, nextReportSeq()), { + stdio: "ignore", + timeout: 2000, + windowsHide: true, + }) } catch (error) { log.debug("herdr release_agent (cli) failed", { error: errorMessage(error) }) }