Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/stale-pids-linger.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@northguild/worktree": patch
---

`worktree cleanup` no longer removes a worktree whose agent session the runtime reports as done while its process is still running. Such a session now counts as live, so it holds the worktree back unless it is idle, and `worktree list --agents` no longer marks it `[done]`.
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ To name the agent session living in each worktree:
worktree list --agents
```

Sessions are found from two places, joined on the directory they run in: `herdr agent list` when `herdr` is installed, and the runtime's own `<program> agents --json`, where the program is `agent.command`'s first word or else `herdr.agent`. A finished session shows `[done]`, unless Herdr still shows it in a pane. With neither source available the list simply shows no sessions.
Sessions are found from two places, joined on the directory they run in: `herdr agent list` when `herdr` is installed, and the runtime's own `<program> agents --json`, where the program is `agent.command`'s first word or else `herdr.agent`. A finished session shows `[done]`, unless Herdr still shows it in a pane or its process is still running. With neither source available the list simply shows no sessions.

### Reopen a worktree in your editor

Expand Down Expand Up @@ -360,7 +360,8 @@ receipt**: check it, not just the exit code, when a brief matters.

They are joined on the directory each session runs in, compared as real paths, and a Herdr entry is named by
the runtime session with the same session id. A session without a `pid` is kept. `live` is `false` for a
session the runtime reports as finished (`[done]` in the text list), unless Herdr still shows it in a pane. With
session the runtime reports as finished (`[done]` in the text list), unless Herdr still shows it in a pane or the
process behind it is still running. With
neither source, the result is no agents and no error. `cleanup` holds a worktree back for any session that is still `live` and not just idle; a finished or idle one does not hold it back. A session counts as idle when the runtime's `status` or Herdr's status is `idle` (or Herdr's `done`) and none of the statuses it has says otherwise.

### A coordinator driving worker agents
Expand Down
2 changes: 1 addition & 1 deletion context/executors.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ always did. Sessions are found from two sources joined on the real path of their
list` when `herdr` is on PATH, and the runtime's `<program> agents --json`, where the program is
`agent.command`'s head or else `herdr.agent`. So a repository with only `herdr.agent` set is covered, and a
Herdr entry is named by the runtime session it matches. What comes back per worktree is a session name
(a pid only where the runtime has one), and markers for live, interactive, waiting and `[done]` (a runtime-finished session that Herdr still shows in a pane stays live). With
(a pid only where the runtime has one), and markers for live, interactive, waiting and `[done]` (a runtime-finished session stays live when Herdr still shows it in a pane or its process is still running). With
neither source available the answer is no sessions, not an error — which is "no answer", not "nobody is
there"; see the next paragraph.

Expand Down
2 changes: 1 addition & 1 deletion context/verify.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ interactive shell. If a console assertion ever starts failing only on someone's
first.

`pnpm test` covers `src/` and `scripts/`; `pnpm docs:test` covers the `docs` workspace including the Cloudflare Worker.
CI runs both. As of 2026-10-01 that is 960 and 66 tests respectively — a snapshot for recognising a suite
CI runs both. As of 2026-10-02 that is 987 and 68 tests respectively — a snapshot for recognising a suite
that did not run, not a figure to assert against. It goes stale on any commit that adds a test, so correct
it in passing rather than treating a mismatch as a failure.

Expand Down
4 changes: 3 additions & 1 deletion docs/src/app/docs/commands/cleanup/page.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ and deleting it out from under either is the failure this check exists to preven

A session that has finished does not hold anything back — the worktree goes back to being
ordinary stale work. A session the runtime calls finished but Herdr still shows in a pane is judged by
what Herdr says about it.
what Herdr says about it, and one whose process is still running is judged as any live session is, by
its status below. The runtime has been seen to report a session finished while its process was still
working in the worktree.

Nor does a session that is only idle, such as an agent pane left open at its prompt in a merged
worktree. The runtime's `idle` and Herdr's `idle` both count, and so does Herdr's `done`, a finished
Expand Down
8 changes: 5 additions & 3 deletions docs/src/app/docs/commands/list/page.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,11 @@ not list is named `<kind>@<pane>`, for example `claude@w5:p1`.
- **`[waiting]`** — a background agent that is live but not progressing, so it is
probably waiting on you.
- **`[done]`** — a session the runtime reports as finished. It no longer holds anything back, so
`cleanup` treats the worktree as ordinary stale work. The exception is a session Herdr still shows in
a pane: that one is not marked `[done]`, and it holds the worktree back only while Herdr reports it as
anything other than idle or done.
`cleanup` treats the worktree as ordinary stale work. The exceptions are a session Herdr still shows in
a pane and one whose process is still running, which the runtime has been seen to report as finished:
those are not marked `[done]`. A session Herdr shows holds the worktree back only while Herdr reports it
as anything other than idle or done; a running one is `[waiting]` when it is idle and otherwise holds
the worktree back.
- **`[interactive]`** — somebody's own terminal session, not an agent this tool
dispatched. It is listed because `cleanup` will refuse to remove a worktree
somebody is working in, and this is where you find out which those are. One that is only idle does
Expand Down
2 changes: 1 addition & 1 deletion skills/worktree/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ worktree list --agents
worktree list -a # alias
# Sessions come from `herdr agent list` (when herdr is on PATH) and from
# `<agent.command program or herdr.agent> agents --json`, joined on the real
# path of their directory. A finished session shows [done] (unless Herdr still shows it in a pane); with no source
# path of their directory. A finished session shows [done] (unless Herdr still shows it in a pane or its process is still running); with no source
# available there are simply no sessions.

# Reopen a worktree in your editor
Expand Down
160 changes: 159 additions & 1 deletion src/lib/agent.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { mkdtempSync, realpathSync, rmSync, symlinkSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { expectCommands, mockRun, mockRunCapturing } from "../test-setup.js";
import {
expectCommands,
mockIsProcessRunning,
mockRun,
mockRunCapturing,
} from "../test-setup.js";
import {
findSessionForPath,
getAgentSessions,
Expand Down Expand Up @@ -656,6 +661,159 @@ describe("getAgentSessions across Herdr and the runtime", () => {
});
});

// #73: the runtime has been seen reporting `done` for a session whose process
// was still running in the worktree. The probe is the mocked cli helper, so no
// result here depends on the process table of the machine running the suite.
describe("a runtime done state with a process behind it", () => {
beforeEach(() => {
vi.clearAllMocks();
mockConfig({ "agent.command": "claude --bg" });
vi.spyOn(cli, "commandExists").mockResolvedValue(false);
});

async function collect(...entries: Record<string, unknown>[]) {
expectCommands("claude agents --json");
mockRun.mockResolvedValueOnce(JSON.stringify(entries));
return getAgentSessions();
}

const done = (overrides: Record<string, unknown> = {}) =>
runtimeEntry({ kind: "background", state: "done", ...overrides });

it("is live, waiting and not blocking when the pid is running and the status is idle", async () => {
mockIsProcessRunning.mockReturnValue(true);

const [session] = await collect(done({ status: "idle" }));

expect(mockIsProcessRunning).toHaveBeenCalledWith(9187);
expect(isSessionLive(session as AgentSession)).toBe(true);
expect(isSessionWaiting(session as AgentSession)).toBe(true);
expect(isSessionBlocking(session as AgentSession)).toBe(false);
});

it.each([
["busy", { status: "busy" }],
["working", { status: "working" }],
["unrecognised", { status: "dreaming" }],
["null", { status: null }],
["absent", { status: undefined }],
])("is live and blocking when the pid is running and the status is %s", async (_label, overrides) => {
mockIsProcessRunning.mockReturnValue(true);

const [session] = await collect(done(overrides));

expect(isSessionLive(session as AgentSession)).toBe(true);
expect(isSessionBlocking(session as AgentSession)).toBe(true);
});

it("stays finished when the pid is not running", async () => {
mockIsProcessRunning.mockReturnValue(false);

const [session] = await collect(done({ status: "idle" }));

expect(mockIsProcessRunning).toHaveBeenCalledWith(9187);
expect(isSessionLive(session as AgentSession)).toBe(false);
expect(isSessionWaiting(session as AgentSession)).toBe(false);
expect(isSessionBlocking(session as AgentSession)).toBe(false);
});

it("stays finished when there is no pid, without probing", async () => {
mockIsProcessRunning.mockReturnValue(true);
const { pid: _pid, ...withoutPid } = done();

const [session] = await collect(withoutPid);

expect(mockIsProcessRunning).not.toHaveBeenCalled();
expect(isSessionLive(session as AgentSession)).toBe(false);
});

it.each([
0,
-1,
1.5,
"9187",
])("treats pid %j as no pid: done stands and nothing is probed", async (pid) => {
mockIsProcessRunning.mockReturnValue(true);

const [session] = await collect(done({ pid }));

expect(mockIsProcessRunning).not.toHaveBeenCalled();
expect(isSessionLive(session as AgentSession)).toBe(false);
});

it("does not probe a session that is not done", async () => {
await collect(
runtimeEntry({ kind: "background", state: "working" }),
runtimeEntry({ kind: "interactive" }),
);

expect(mockIsProcessRunning).not.toHaveBeenCalled();
});

it("blocks the worktree through findSessionForPath, and a working sibling still wins", async () => {
mockIsProcessRunning.mockImplementation((pid: number) => pid === 9187);

const sessions = await collect(
done({ pid: 9187, name: "stale-done", status: "busy" }),
done({ pid: 4, name: "really-done", status: "idle" }),
);
const found = findSessionForPath(sessions, worktreePath);

expect(found?.name).toBe("stale-done");
expect(isSessionBlocking(found as AgentSession)).toBe(true);

const withWorker = await collect(
done({ pid: 9187, name: "stale-done", status: "idle" }),
runtimeEntry({
kind: "background",
name: "worker",
pid: 5,
state: "working",
}),
);
expect(findSessionForPath(withWorker, worktreePath)?.name).toBe("worker");
});

it("leaves a finished session with no running process out of the way of an idle live one", async () => {
mockIsProcessRunning.mockImplementation((pid: number) => pid === 9187);

const sessions = await collect(
done({ pid: 4, name: "gone" }),
done({ pid: 9187, name: "stale-done", status: "idle" }),
);

expect(findSessionForPath(sessions, worktreePath)?.name).toBe("stale-done");
});

it("still joins to Herdr as before: the done is dropped and both signals are weighed", async () => {
mockIsProcessRunning.mockReturnValue(true);
mockConfig({ "herdr.agent": "claude" });
vi.spyOn(cli, "commandExists").mockResolvedValue(true);
expectCommands("claude agents --json");
mockRun.mockResolvedValueOnce(JSON.stringify([done({ status: "idle" })]));
mockHerdrAgents([herdrAgent({ agent_status: "working" })]);

const [session] = await getAgentSessions();

expect(session?.herdrAgent).toBe("wA:p1");
expect(isSessionLive(session as AgentSession)).toBe(true);
expect(isSessionBlocking(session as AgentSession)).toBe(true);
});

it("joined to Herdr, stays live even when the pid is not running", async () => {
mockIsProcessRunning.mockReturnValue(false);
mockConfig({ "herdr.agent": "claude" });
vi.spyOn(cli, "commandExists").mockResolvedValue(true);
expectCommands("claude agents --json");
mockRun.mockResolvedValueOnce(JSON.stringify([done()]));
mockHerdrAgents([herdrAgent()]);

const [session] = await getAgentSessions();

expect(isSessionLive(session as AgentSession)).toBe(true);
});
});

describe("findSessionForPath", () => {
it("matches a session whose cwd is the worktree itself", () => {
const target = session();
Expand Down
36 changes: 32 additions & 4 deletions src/lib/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
isHerdrInstalled,
listHerdrAgents,
} from "../integrations/herdr.js";
import { run } from "./cli.js";
import { isProcessRunning, run } from "./cli.js";
import { gitGetConfigValue } from "./git.js";
import type { AgentSession } from "./types.js";
import { splitCommandValue } from "./utils.js";
Expand Down Expand Up @@ -42,6 +42,30 @@ function readOptionalString(
return typeof value === "string" ? value : undefined;
}

// A runtime `done` is believed only when no process is behind it (#73). The
// runtime has been seen reporting `done` for a session whose process was still
// running with its cwd in the worktree, so a `done` that carries a running pid is
// dropped, as `joinSessions` drops one Herdr still shows: the contrary evidence
// wins in the safe direction, and the session is then weighed like any other
// live one — idle does not hold `cleanup` back, anything else does. The pid is
// another program's JSON, so only a positive integer is probed; 0 and negatives
// address process groups, and anything else is no pid. No pid, an unusable one,
// or one that is gone leaves `done` standing. The probe lives here, where
// sessions are collected, so the predicates below stay pure.
function withoutStaleDone(session: AgentSession): AgentSession {
const { pid } = session;
if (
session.state !== "done" ||
pid === undefined ||
!Number.isSafeInteger(pid) ||
pid <= 0 ||
!isProcessRunning(pid)
) {
return session;
}
return { ...session, state: undefined };
}

// Only `cwd` is load-bearing: it is the join key, so an entry without one
// cannot match a worktree. `name` and `pid` are rendering fields, and dropping
// an entry over one would yield no session where one exists — the direction
Expand Down Expand Up @@ -126,7 +150,8 @@ async function listRuntimeSessions(): Promise<AgentSession[]> {
}
return parsed
.map(toAgentSession)
.filter((session): session is AgentSession => session !== undefined);
.filter((session): session is AgentSession => session !== undefined)
.map(withoutStaleDone);
} catch {
return [];
}
Expand Down Expand Up @@ -174,7 +199,9 @@ function herdrOnlySession(entry: HerdrAgentEntry): AgentSession {
// beside the runtime's own so `isSessionBlocking` weighs both, except that a
// runtime `done` is dropped: Herdr still shows the session in a pane, and a live
// process has been seen reported `done` by the runtime, so the contrary evidence
// wins in the safe direction. A Herdr entry nothing matched stands on its own.
// wins in the safe direction. A `done` whose own pid is still running is already
// dropped when the runtime listing is read (#73); Herdr covers the one with no
// pid to probe. A Herdr entry nothing matched stands on its own.
// Every cwd is already a real path.
function joinSessions(
runtime: AgentSession[],
Expand Down Expand Up @@ -270,7 +297,8 @@ export function findSessionForPath(
// Fails safe: an absent or unrecognised state counts as live, so a session whose
// shape we no longer recognise blocks removal rather than being ignored. That is
// also what makes an interactive session live, since those carry no state at
// all. See D6 and §4.1.
// all. See D6 and §4.1. A runtime `done` reaches here believed only when no
// process is running behind it, and a Herdr-joined one not at all (#73, #70).
export function isSessionLive(session: AgentSession): boolean {
return session.state !== "done";
}
Expand Down
61 changes: 61 additions & 0 deletions src/lib/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { tmpdir } from "node:os";
import { basename, delimiter, dirname, join } from "node:path";
import {
commandExists,
isProcessRunning,
KILL_GRACE_MS,
run,
runCapturing,
Expand Down Expand Up @@ -461,3 +462,63 @@ describe("env option", () => {
await expect(run(node, ["-e", printVar])).resolves.toBe("");
});
});

// #73. process.kill is spied wherever the answer matters, so nothing here
// depends on which pids exist on the machine, and no signal is ever delivered.
describe("isProcessRunning", () => {
afterEach(() => {
vi.restoreAllMocks();
});

function killFailsWith(code: string) {
return vi.spyOn(process, "kill").mockImplementation(() => {
throw Object.assign(new Error(code), { code });
});
}

it("probes with signal 0 and reports a pid that exists as running", () => {
const kill = vi.spyOn(process, "kill").mockReturnValue(true);

expect(isProcessRunning(4242)).toBe(true);
expect(kill).toHaveBeenCalledWith(4242, 0);
});

it("reports the current process as running, for real", () => {
expect(isProcessRunning(process.pid)).toBe(true);
});

it("reports ESRCH as not running", () => {
killFailsWith("ESRCH");

expect(isProcessRunning(4242)).toBe(false);
});

it("reports EPERM as running: the process exists, under another user", () => {
killFailsWith("EPERM");

expect(isProcessRunning(4242)).toBe(true);
});

it("fails safe, as running, on an error it does not understand", () => {
killFailsWith("EINVAL");
expect(isProcessRunning(4242)).toBe(true);

vi.spyOn(process, "kill").mockImplementation(() => {
throw "not an error";
});
expect(isProcessRunning(4242)).toBe(true);
});

it.each([
0,
-1,
1.5,
Number.NaN,
Number.POSITIVE_INFINITY,
])("never signals %s: it addresses a group or is no pid", (pid) => {
const kill = vi.spyOn(process, "kill").mockReturnValue(true);

expect(isProcessRunning(pid)).toBe(false);
expect(kill).not.toHaveBeenCalled();
});
});
Loading
Loading