Repository navigation
fix(cleanup): keep a runtime-done session live while its process runs - #94
Merged
Merged
Conversation
The runtime has been seen reporting `state: "done"` for a session whose pid was still running with its cwd in the worktree, so `cleanup` swept the worktree as ordinary stale work. A `done` is now believed only when no process is running behind it: where the reported pid is a positive integer and `kill(pid, 0)` finds it, the `done` is dropped when the runtime listing is read, and the session is weighed like any other live one. Idle still does not hold `cleanup` back (#89); busy, unrecognised or no status does. `list --agents` no longer marks such a session `[done]`. Closes #73
craig-o-curtis
approved these changes
Oct 2, 2026
craig-o-curtis
left a comment
Contributor
There was a problem hiding this comment.
Oh ya, you betcha, this one's good to go.
Read through the whole diff, and it's a pretty tidy piece of work:
isProcessRunningonly ever sends signal 0, won't touch a zero or a negative pid (so no process groups get poked, don'tcha know), and anything it can't make sense of counts as running. Fails safe, which is just what a fella wants from a thing that decides what gets deleted.withoutStaleDonedoes its probing where the runtime listing is read, so the predicates stay pure, and it lines up real nice with whatjoinSessionsalready does for a session Herdr still shows.- Tests never look at the real process table (the default mock says nothing's running), so it won't go flaky on somebody's laptop. The
cli.test.tscases run against the real module through thevi.unmockthat's already there. - Docs, the skill file, and a
patchchangeset all came along in the same change.ciandchangesetare both green.
Couple things, not holding anything up:
- The session #73 actually caught was
state: done, status: idlewith a live pid. After this it's live but idle, so per #89cleanupwill still take that worktree. The PR body says so plainly, and I figure that's what you meant, but it might be worth a line on #73 so nobody's surprised later. - If a finished session's pid gets handed out again to some other process, that session will read as live until the other process quits. It errs on the careful side, so that's fine by me.
- Heads up, #95 and #96 may bump the same test-count line in
context/verify.md, so whichever goes in second could need a quick rebase.
I didn't run the suite on my own machine, just went by the diff and the green CI on this head.
Not too bad at all. Thanks for this!
baldurpan
added a commit
that referenced
this pull request
Oct 2, 2026
…ve-agent-as Brings in #94 (#73): a runtime `done` session whose pid is still running counts as live. Combined with this branch's waiting rule, such a session is weighed by its status: idle is waiting and does not block, blocked is waiting and blocks, busy or no status blocks and is not waiting, and one whose process is gone stays done. Conflicts in README.md and the list docs page are resolved to keep both changes and describe the combination; the skill's `list --agents` comment says the same. Adds a test pinning the non-idle side of the combination, and corrects verify.md's test-count snapshot in passing.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #73
What changed
The agent runtime has been seen reporting
state: "done"for a session whose pid was still running with its cwd in the worktree, socleanupswept that worktree as ordinary stale work.A runtime
doneis now believed only when no process is running behind it:isProcessRunning(pid)(src/lib/cli.ts) probes withkill(pid, 0), which sends no signal. Only a positive safe integer is probed, because 0 and negative pids address process groups. ESRCH counts as not running. EPERM and any unrecognised error count as running, so an unclear answer fails safe.withoutStaleDone(src/lib/agent.ts) drops adonewhose pid is running. It does this when the runtime listing is read, the same safe-direction movejoinSessionsalready makes for a session Herdr still shows. The predicates stay pure.isSessionBlocking. An idle session still does not holdcleanupback (cleanup holds back a merged worktree whose agent session is only idle #89). Busy, unrecognised or missing status does.list --agentsshows[waiting](idle) or no marker instead of[done].donewith no pid, an unusable pid or a pid that has exited still reads as finished, as before.Docs updated in the same change:
README.md, thelistandcleanupcommand pages,skills/worktree/SKILL.md,context/executors.md. Thecontext/verify.mdtest-count snapshot was corrected in passing.Release note:
.changeset/stale-pids-linger.md(patch).Gates
context/verify.md):pnpm check,pnpm typecheckandpnpm buildall exit 0.pnpm testpasses 987 tests andpnpm docs:testpasses 68.