Skip to content

fix(agents): join sessions in remove, name what cleanup sweeps past, report failed launches (#56) - #170

Merged
baldurpan merged 7 commits into
mainfrom
fix/56-agent-session-detection-and-display-are-wrong-in
Oct 27, 2026
Merged

fix(agents): join sessions in remove, name what cleanup sweeps past, report failed launches (#56)#170
baldurpan merged 7 commits into
mainfrom
fix/56-agent-session-detection-and-display-are-wrong-in

Conversation

@baldurpan

@baldurpan baldurpan commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Closes #56

Settles the seven agent-session findings (F-018, F-019, F-021, F-022, F-030, F-032, F-033). The plan and its finished ledger are in #56.

Already fixed, closed on evidence: F-021, F-022 (session side) and F-033 were fixed by 38cffe1 (#80).

Phases landed

  1. 77a9f75 pins worktree-side path normalisation in the agent join (F-022).
  2. 743d52a makes remove join agent sessions, prefix the prompt and warn under --force (F-032).
  3. 315dcdb makes cleanup --ignore-agents look, decide as before and name what it sweeps past (F-030).
  4. ec90170 reports a detached agent launch that fails as failed: agent: null, a warning, exit 1 (F-018).
  5. e4eac24 documents the supported agent shape (F-019): output is discarded, so the agent must keep its own record.

Release note: one patch note (.changeset/quiet-agents-join.md). This is waiting on the release pull request, so nothing here ships to npm on merge.

Open owner checks: the Codex session visibility check is filed as #169. The other two were waived.

@baldurpan
test(git): pin worktree-side path normalisation in the agent join (#56)
77a9f75 
Phase 1 of #56 (F-022). A worktree listed at a symlinked spelling now has
to resolve to its real directory before the session join, or the new case
in "gitGetWorktreeList agent join" goes red.

Mutation: with toRealPath(worktreePath) at src/lib/git.ts:401 replaced by
worktreePath, vitest gave 1 failed (the new case only), 1201 passed;
reverted.

Gate 1: pnpm check, typecheck, docs typecheck, build, test (1202),
docs:test (69), test-markers grep - all exit 0.
Gate 2: reviewer subagent, PASS WITH NOTES (notes in context/notes.md).
@baldurpan
fix(cleanup): --ignore-agents gathers sessions, decides as before and…
315dcdb 
… names what it sweeps past (#56)

Phase 3 of #56 (F-030). The override now overrules the verdict, not the
lookup: sessions are always gathered, the sweep is classified on
agent-stripped copies so the removed set and the three hold-back headings
match an agent-free run, and each removed worktree the agent-aware verdict
would have held back is named under "Removing <n> worktree branch(es)
despite a live agent session:", under --force and before the prompt.

Gate 2 loop: the first review (PASS WITH NOTES) found the heading could
also name a live session in a worktree whose directory is gone, with or
without the flag. Fixed in-host by requiring the original verdict to have
been unsafe, pinned by an it.each over both flag states (failed on the
earlier code), plus a zero-session case; the herdr-spaces guide no longer
overstates the flag. Re-review: PASS WITH NOTES.

Gate 1: pnpm check, typecheck, docs typecheck, build, test (1213),
docs:test (69), test-markers grep - all exit 0.
@baldurpan
fix(branch): report a detached agent launch that fails as failed (#56)
ec90170 
Phase 4 of #56 (F-018). spawnDetached now returns why a launch did not
start - pid undefined, or a synchronous spawn throw - and dispatchAgent
sequences on it: no "Agent started" line, the warning "The agent could not
be started (<message>), so the brief was not delivered." printed and in
warnings, agent null, exit 1. The success line, the async onError handler
and the argv-only spawn are unchanged.

Deviation from the plan: the synchronous-throw test drives a real throw
(a NUL byte in an argument, ERR_INVALID_ARG_VALUE) instead of mocking
spawn.

Gate 2 loop 1: FAIL - the warning was recorded for the document but never
printed, so a human run never saw it and a synchronous throw was silent.
Fixed by warn() in place of recordWarning(), pinned by a warn spy. Re-review:
PASS WITH NOTES (notes in context/notes.md).

By-hand pass (plan section 8) against bin/run.js, agent.command naming a
missing program: human and --json both print "Warning: The agent could
not be started (spawn worktree-no-such-agent failed to start), so the
brief was not delivered.", no "Agent started" line, "agent":null, exit 1.

Gate 1: pnpm check, typecheck, docs typecheck, build, test (1218, under
the pinned Node v26.12.1), docs:test (69), test-markers grep - all exit 0.
@baldurpan
fix(remove): join agent sessions, prefix the prompt and warn under --…
743d52a 
…force (#56)

Phase 2 of #56 (F-032). remove makes the run's one session lookup and
uses it on both paths: the picker renders the Agent: detail and a
worktree holding a blocking session lands under "Active branches" through
the existing isSafeToRemove; the named path prefixes "An agent session
(<name>) is working in this worktree." to whichever prompt applied before;
under --force (so every --json run) "Removed <branch> while an agent
session (<name>) was working in it." goes to stderr and into warnings.
Idle sessions neither warn nor move an entry. gitRemoveWorktree takes the
session from its caller and gathers no sessions itself.

Mutations (caller, reverted): dropping hasLiveAgent from the named-path
options gate failed 2 tests; dropping it from the warning gate failed 2.

Gate 1: pnpm check, typecheck, docs typecheck, build, test (1231),
docs:test (69), test-markers grep - all exit 0.
Gate 2: reviewer subagent, PASS WITH NOTES (notes in context/notes.md).
@baldurpan
docs(branch): say a detached agent's output is discarded and name age…
e4eac24 
…nts that keep their own record (#56)

Phase 5 of #56 (F-019, accepted per D8). The branch page, the
configuration page and the worktree skill now say that a detached
agent's output is not kept anywhere, so agent.command should name an
agent that keeps its own session record: claude --bg, reachable through
claude agents, or codex exec unless --ephemeral is passed. Checked
against claude --help and codex exec --help on this machine.

Gate 2 (PASS WITH NOTES) noted the branch page dropped the --ephemeral
caveat and left a short line and an ambiguous "It"; fixed in-host and
re-reviewed: PASS WITH NOTES.

Gate 1 (docs-only, per verify.md Rules): pnpm check exit 0, plus
pnpm --filter docs typecheck exit 0 and docs:test (69) since SKILL.md
is embedded into the docs chat.
@baldurpan
chore(release): retire #56 with a patch note and drop the branch notes
9134d60
craig-o-curtis
craig-o-curtis approved these changes Oct 27, 2026
View reviewed changes

@craig-o-curtis craig-o-curtis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving. Read the diff against 9134d60; CI is green so I didn't rerun the gates.

Checked specifically:

  • cleanup --ignore-agents now always gathers sessions. That's safe as an escape hatch: listRuntimeSessions/listHerdrEntries swallow errors and the runtime listing is bounded at 10s, so a broken session source can't block the sweep. The stripped-copy reclassification keeps the removed set and the hold-back headings identical to pre-agent behaviour, and carried is empty by construction without the flag.
  • remove: one lookup per run, the picker path returns the selected entries (which carry agent), so warnRemovedWhileWorking sees the session on both paths. warn records into the document, so --json gets it in warnings as the docs say.
  • Failed detached launch: pid === undefined is checked synchronously, the existing "brief reached no agent" path supplies exit 1, and the test pins agent null + exit 1.

Non-blocking nit: when the program is missing, the user sees two lines: the new warning with the generic spawn <x> failed to start, then the onError line Error: spawn <x> ENOENT on the next tick. Fine to leave; if you touch it again, consider dropping the onError log when spawnDetached already returned a failure.

@baldurpan

baldurpan commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. The nit is real: when the program is missing, spawnDetached returns its failure synchronously (pid is undefined), so the new warning prints first, and the error event still reaches onError a tick later with spawn <x> ENOENT. Leaving it as you allowed, since a fix commit here would dismiss the approval. When this is next touched, the fix belongs in spawnDetached (src/lib/cli.ts): stop forwarding the late error event to onError once a failure has already been returned. That keeps the one line that carries the exit code.

@baldurpan
baldurpan merged commit b95cb57 into main Oct 27, 2026
2 of 3 checks passed
@baldurpan
baldurpan deleted the fix/56-agent-session-detection-and-display-are-wrong-in branch October 27, 2026 22:25
@baldurpan baldurpan mentioned this pull request Oct 29, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@craig-o-curtis craig-o-curtis craig-o-curtis approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Agent session detection and display are wrong in seven recorded ways

2 participants

@baldurpan @craig-o-curtis