fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

CB-117: reap orphaned worker panes via startup reconciliation #1

New Issue
Closed
opened 2026-07-16 08:32:20 +02:00 by ltms · 1 comment
ltms commented 2026-07-16 08:32:20 +02:00
Owner
Copy Link
Copy Source

Problem

Worker panes leak as orphans whenever their spawner dies before issuing the matching DELETE /workers/{paneId}. Observed: three idle claude-ollama-* worker panes (#1/#2/#3) left in the worker space wQ from earlier test/ad-hoc runs, tracked by herdr but owned by nobody.

Root cause — worker ownership lives only in the ephemeral spawner

Four facts, together, produce the leak:

  1. WorkerService keeps no registry. list() delegates to agents.list() (asks herdr). nameSeq/nameNonce are name-uniqueness counters, not a record of owned panes — the daemon has no list of its own workers to clean up.
  2. Teardown is caller-driven and pane-scoped. WorkerService.stop(paneId) needs someone to already hold the paneId and call DELETE /workers/{paneId} (BridgedApp:159). The only holder is whoever spawned it.
  3. No shutdown reaping. Bridged.java shutdown hooks close herdr/poller/messages/mcp — none stop workers. A daemon kill/restart leaves every live worker pane running.
  4. herdr deliberately outlives the daemon. Panes persist across daemon restarts by design, so a worker whose spawner is gone keeps running with nothing tracking it.

A worker becomes an orphan the moment its spawner dies before the matching DELETE — interrupted test run, --keep-worker, or (most common here) the owning daemon was killed. The code already anticipates survivors: the nameNonce comment in WorkerService exists so "same-profile workers that outlived a restart" don't get name collisions — it just never added reaping.

Not a broken teardown: stop(paneId) works (verified — a clean run's worker is closed correctly). This is an ownership gap.

Fix — startup reconciliation keyed on nameNonce

On boot, before serving: herdr agent list → for each agent whose name matches claude-<profile>-<nonce>-<seq> with a nonce ≠ this process's nameNonce, treat it as a predecessor's leak and reap it (WorkerService.stop(paneId), which also closes the now-empty dedicated tab). Log each reap.

Chosen over the alternatives because it is the only approach that survives kill -9 and reaps leaks from previous daemon lives (the actual case here):

  • shutdown-hook reap: only helps clean shutdown, not kill -9;
  • in-memory registry alone: lost on crash, never reaps predecessors.

Acceptance

  • Fresh daemon boot reaps every claude-* pane whose nonce differs from the current process, closing pane + empty tab; own-process and non-worker agents untouched.
  • Each reap is logged (name, pane, tab).
  • Unit coverage: reaper adopts foreign-nonce workers, skips current-nonce and non-matching names, tolerates already-gone panes.
  • mvn clean install green.

Notes

  • Nonce embeds the owning process identity; only foreign nonces are reaped, so two daemons never fight over each other's live workers... (single-daemon deployment today, but the keying keeps it correct if that changes).
  • Bonus finding while diagnosing: DELETE /workers/{paneId} returns 204 No Content; a JSON-parsing client (the e2e harness) throws on the empty body. Harmless (stop succeeds) but the harness should treat 204 as success rather than parsing it. Track separately if desired.
## Problem Worker panes leak as orphans whenever their spawner dies before issuing the matching `DELETE /workers/{paneId}`. Observed: three idle `claude-ollama-*` worker panes (`#1/#2/#3`) left in the worker space `wQ` from earlier test/ad-hoc runs, tracked by herdr but owned by nobody. ## Root cause — worker ownership lives only in the ephemeral spawner Four facts, together, produce the leak: 1. **`WorkerService` keeps no registry.** `list()` delegates to `agents.list()` (asks herdr). `nameSeq`/`nameNonce` are name-uniqueness counters, not a record of owned panes — the daemon has no list of its own workers to clean up. 2. **Teardown is caller-driven and pane-scoped.** `WorkerService.stop(paneId)` needs someone to already hold the paneId and call `DELETE /workers/{paneId}` (`BridgedApp:159`). The only holder is whoever spawned it. 3. **No shutdown reaping.** `Bridged.java` shutdown hooks close herdr/poller/messages/mcp — none stop workers. A daemon `kill`/restart leaves every live worker pane running. 4. **herdr deliberately outlives the daemon.** Panes persist across daemon restarts by design, so a worker whose spawner is gone keeps running with nothing tracking it. A worker becomes an orphan the moment its spawner dies before the matching DELETE — interrupted test run, `--keep-worker`, or (most common here) the owning daemon was killed. The code already *anticipates* survivors: the `nameNonce` comment in `WorkerService` exists so "same-profile workers that outlived a restart" don't get name collisions — it just never added reaping. Not a broken teardown: `stop(paneId)` works (verified — a clean run's worker is closed correctly). This is an ownership gap. ## Fix — startup reconciliation keyed on `nameNonce` On boot, before serving: `herdr agent list` → for each agent whose name matches `claude-<profile>-<nonce>-<seq>` with a nonce **≠ this process's `nameNonce`**, treat it as a predecessor's leak and reap it (`WorkerService.stop(paneId)`, which also closes the now-empty dedicated tab). Log each reap. Chosen over the alternatives because it is the only approach that survives `kill -9` and reaps leaks from *previous* daemon lives (the actual case here): - shutdown-hook reap: only helps clean shutdown, not `kill -9`; - in-memory registry alone: lost on crash, never reaps predecessors. ### Acceptance - Fresh daemon boot reaps every `claude-*` pane whose nonce differs from the current process, closing pane + empty tab; own-process and non-worker agents untouched. - Each reap is logged (name, pane, tab). - Unit coverage: reaper adopts foreign-nonce workers, skips current-nonce and non-matching names, tolerates already-gone panes. - `mvn clean install` green. ### Notes - Nonce embeds the owning process identity; only foreign nonces are reaped, so two daemons never fight over each other's live workers... (single-daemon deployment today, but the keying keeps it correct if that changes). - Bonus finding while diagnosing: `DELETE /workers/{paneId}` returns `204 No Content`; a JSON-parsing client (the e2e harness) throws on the empty body. Harmless (stop succeeds) but the harness should treat 204 as success rather than parsing it. Track separately if desired.
ltms commented 2026-07-16 08:40:00 +02:00
Author
Owner
Copy Link
Copy Source

Implemented in ff6aacd (local on main, pending push).

Startup reconciliation keyed on nameNonce, as proposed:

  • Agent now projects herdr's name (previously dropped) so the reaper can key on it.
  • WorkerService.reapOrphanWorkers() scans agent.list on boot and tears down every agent matching claude-<profile>-<nonce>-<seq> with a foreign nonce (pane + now-empty dedicated tab, via the existing stop). Current-nonce workers (ours, live) and unnamed user sessions are untouched. Best-effort: a listing/teardown failure is logged, never fatal to boot.
  • isForeignWorker/workerNonce are pure + package-private; the nonce/seq are tail-anchored so profile names containing - still parse.
  • Wired into Bridged startup before serving.

Tests (all green, mvn clean install → 133 pass):

  • isForeignWorker predicate: foreign nonce ⇒ reap; own nonce, unnamed, bare claude, user label, non-hex nonce ⇒ spare.
  • Reap wiring: reaps the one foreign orphan, spares our live worker and a user session, closes the empty tab; counts an already-gone pane as reaped; skips (returns 0, no throw) when agent.list fails.

The three observed orphans (wQ:t8/t9/tD) were also cleaned up manually.

Bonus finding (204-on-DELETE breaks JSON-parsing clients) left as a note for the e2e harness; not addressed here.

Closing as done — will auto-reference on push.

Implemented in `ff6aacd` (local on `main`, pending push). **Startup reconciliation keyed on `nameNonce`**, as proposed: - `Agent` now projects herdr's `name` (previously dropped) so the reaper can key on it. - `WorkerService.reapOrphanWorkers()` scans `agent.list` on boot and tears down every agent matching `claude-<profile>-<nonce>-<seq>` with a **foreign** nonce (pane + now-empty dedicated tab, via the existing `stop`). Current-nonce workers (ours, live) and unnamed user sessions are untouched. Best-effort: a listing/teardown failure is logged, never fatal to boot. - `isForeignWorker`/`workerNonce` are pure + package-private; the nonce/seq are tail-anchored so profile names containing `-` still parse. - Wired into `Bridged` startup before serving. **Tests (all green, `mvn clean install` → 133 pass):** - `isForeignWorker` predicate: foreign nonce ⇒ reap; own nonce, unnamed, bare `claude`, user label, non-hex nonce ⇒ spare. - Reap wiring: reaps the one foreign orphan, spares our live worker and a user session, closes the empty tab; counts an already-gone pane as reaped; skips (returns 0, no throw) when `agent.list` fails. The three observed orphans (`wQ:t8`/`t9`/`tD`) were also cleaned up manually. Bonus finding (204-on-DELETE breaks JSON-parsing clients) left as a note for the e2e harness; not addressed here. Closing as done — will auto-reference on push.
ltms closed this issue 2026-07-16 08:40:05 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#1
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?