diff --git a/docs/HERDR.md b/docs/HERDR.md index 358f7382..5d2e7dec 100644 --- a/docs/HERDR.md +++ b/docs/HERDR.md @@ -75,26 +75,53 @@ to their own direct dispatch so visibility never becomes a hard dependency. ### Sweep safety (FR-VIS-07, FR-VIS-10) -`devagent herdr-sweep` closes only panes that satisfy both per-pane guards: - -1. **Automation ownership**: pane cwd sits inside `.devagent-worktrees/` — - operator scratch panes in the same session are never listed, let alone - closed. -2. **No live dispatch**: `pane process-info` shows the foreground process; a - pane running `omp`/`pi`/`claude`/`opencode` is mid-run and skipped. This - guard exists because a busy pane can still report `agent_status: idle` - (the pane wrapper polls the done-marker, not the agent state machine) — - the 2026-09-05 in-flight-close regression. +`devagent herdr-sweep` orders its checks so each pane is judged by scope (may +it be swept at all) → operator at the wheel → orphan evidence → roster spare → +status classes. The two per-pane guards: + +1. **Automation ownership**: a pane whose cwd sits inside `.devagent-worktrees/` + was spawned by the dispatcher, so the status classes reach it. Anywhere else — + the main checkout, where the loop's research/PO dispatches run beside the + operator's own windows — the cwd proves nothing: the pane is swept only by the + orphan class (`--orphans`) and only on positive **dispatch evidence**, the + run's capture contract (`/devagent-herdr-/{out,err,done}`) open on + fd 1 or fd 2 of one of its foreground processes. A worker the operator ran by + hand points at the pane tty and never matches, so scratch panes are + structurally spared. +2. **No live dispatch**: `pane process-info` — asked **in the session under + sweep**; an unscoped probe answers for herdr's own default session, which made + every devagent pane read idle until 2026-09-13 — shows the foreground process, + and a pane running `omp`/`pi`/`claude`/`opencode` is mid-run and skipped. This + guard exists because a busy pane can still report `agent_status: idle` (the + pane wrapper polls the done-marker, not the agent state machine) — the + 2026-09-05 in-flight-close regression. + +The one exception to guard 2 is the orphan class itself: a live worker whose +collector is dead has nobody polling its done marker or closing its workspace, so +`--orphans` reaps it. Ownership is the process that survives the whole run — the +dispatching `devagent task` / `devagent pane-run` that polls the marker — **not** +the `herdr pane run` client, which types the script into the pane's shell and +exits; if that dispatcher is gone, or its `ps` ppid ancestry holds no live loop +driver, the run is an orphan. The probes must ANSWER before anything is reaped: +pgrep's own "no match" (exit 1, no output) means there is no collector and the +pane goes, while a `pgrep`/`ps` that could not run — absent binary, the 5s cap, a +rejected pattern, an ancestry walk that never completed — is no evidence and the +class goes inert, because reaping on a probe that never inspected anything would +close every live worker on a host that cannot read its own process table. +Attribution is process-wide: any dispatcher still hanging off a live driver +spares every live pane in the session, so the error direction is "leave a +leftover running", never "close a run somebody is still collecting". The session name alone is not a safety property (PRD §18 Q23), so two operator-side bounds sit on top of the per-pane checks: -3. **Operator-attach exemption**: a pane the FR-VIS-02 roster reports as live - (`state: running`) is never closed, and neither is anything in the session - while `DEVAGENT_OPERATOR_ATTACHED` is set in the sweep's environment — an - operator is at the wheel. Spared panes are still reported, with - `reason=operator-attached` (CLI line prefix `[spared]`), so a dry-run - explains why a pane survived. The exemption outranks the `--orphans` class. +3. **Operator-attach exemption**: nothing in the session is sweepable while + `DEVAGENT_OPERATOR_ATTACHED` is set in the sweep's environment — an operator + is at the wheel, and that outranks every other class including `--orphans`. + Below it, the FR-VIS-02 roster spares a worktree pane it reports as live + (`state: running`) when no foreground worker was found. Spared panes are still + reported, with `reason=operator-attached` (CLI line prefix `[spared]`), so a + dry-run explains why a pane survived. 4. **Managed deny toggle** (`devagent.json`): @@ -123,6 +150,14 @@ anything, and states the bound when the sweep is disabled or denied. ### Orphaned worker-helper class (`--orphan-brokers`) +**Status (2026-09-13): documented, not yet ported to Go.** `devagent +herdr-sweep --orphan-brokers` is rejected as an unknown flag by the current +binary (its surface is `--session`, `--dry-run`, `--orphans`); the class shipped +in the Node implementation retired by #205 and has no Go counterpart, so nothing +below runs. Read it as the port contract, not as live behavior — including the +answered-probe rule above: a reaper must never kill on a process table it could +not inspect. + The per-pane guards above cannot see a different leak class: omp's `__omp_worker_daemon_broker` worker outliving its session. When an omp process dies without taking its broker down, the broker reparents to launchd (ppid 1) @@ -151,6 +186,13 @@ against a functional stub CLI (`DEVAGENT_HERDR_BIN` injects the binary): stdout capture, exit-code propagation, env injection without leakage, timeout teardown, keep-panes mode, fallback behavior, and config validation — plus the sweep guards above (FR-VIS-07 per-pane checks, the FR-VIS-10 deny toggle, the operator-attach -exemption, `herdr.sweep` parsing/validation/env precedence). The orphaned-broker -class is tested against a real child process that traps SIGTERM, so the -SIGTERM→SIGKILL escalation is proven rather than stubbed. +exemption, `herdr.sweep` parsing/validation/env precedence). The orphan probes +are pinned from two directions: their test seams are keyed by what the sweep +actually asks (the `pgrep` seam by the pattern, the capture seam by pane id, a +miss counting as an unanswered probe), and the shapes behind those keys are +checked against reality — the real `lsof` probe against a child holding a capture +file on fd 1/2, and the owner pattern against the command lines +`internal/loopdriver` builds — because on 2026-09-13 the stubs stayed green while +live reaping matched nothing. The orphaned-broker class above has no Go +implementation and therefore no test; porting it inherits the same rule — an +unreachable process table yields no candidates, never a kill list. diff --git a/docs/PRD.html b/docs/PRD.html index a790a674..ea0b69e7 100644 --- a/docs/PRD.html +++ b/docs/PRD.html @@ -2771,14 +2771,15 @@

18. Open Questions

toggle, or keep --all out of scope permanently? Resolved 2026-09-07: both halves of the safety requirement, and --all stays out of scope permanently — the sweep lists -exactly one session and nothing widens it. Per-pane verification shipped +exactly one session and nothing widens it. Per-pane verification ships as FR-VIS-07; the managed-settings-style toggle ships as FR-VIS-10: herdr.sweep (src/config.ts:39, resolved by herdrSweepConfig at src/config.ts:427) carries enabled (env `DEVAGENT_HERDR_SWEEP=0 1) and denySessions, and findStalePanes/sweepStalePanes (src/integrations/herdr.ts:462, :699) return nothing when the sweep is disabled or the resolved session is denied. A pane the FR-VIS-02 roster reports as live, or any sweep launched while DEVAGENT_OPERATOR_ATTACHEDis set, is spared ahead of every other class — including--orphans— and reported withreason: -operator-attachedinstead of closed. Unset defaults are today's behavior; an invalidherdr.sweep` -block fails closed. Removed. +operator-attachedinstead of closed. Unset defaults are today's behavior; an invalidherdr.sweepblock fails closed. **Corrected 2026-09-13:** only the env half of that ordering survives — the roster'srunningstate is derived from the same foreground-worker probe the orphan class reads, so sparing it ahead of--orphansleft the class unreachable; a live-worker pane is now orphan-reaped on collector evidence (FR-VIS-07),DEVAGENT_OPERATOR_ATTACHED` +still outranks everything, and the roster spare still covers worktree +panes that are busy with no live worker. Removed. product @@ -4170,7 +4171,13 @@

20.8 an idle/unknown row whose pane process-info foreground process is a worker binary maps to running, agent_status stays the signal for interactive panes -(internal/herdr/roster.go paneState) +(internal/herdr/roster.go paneState). +Fixed 2026-09-13: that discriminator was inert in +production — the pane process-info probe went out +without --session, so herdr answered for +its own default session and every devagent pane reported no foreground +process; PaneForegroundWorker now takes the session and +paneState passes it, so the upgrade actually fires M @@ -4233,12 +4240,25 @@

20.8 FR-VIS-07 -Sweep safety: herdr-sweep closes only panes whose cwd -sits inside .devagent-worktrees/ (automation-owned; -operator scratch panes in the same session are untouchable) AND whose -foreground process is not a worker CLI (pane process-info -distinguishes a live omp/pi/claude/opencode from an idle shell) — an -in-flight pane is never sweepable +Sweep safety: herdr-sweep closes a pane only where it +can prove automation owns it. Inside .devagent-worktrees/ +the cwd is that proof (the dispatcher cd's the run into +the task's worktree), and the pane must additionally not be running a +worker CLI — pane process-info, asked in the session under +sweep, distinguishes a live omp/pi/claude/opencode from an idle shell +(the 2026-09-05 in-flight-close regression). Outside the worktrees the +cwd proves nothing, so a pane joins the sweep only when the orphan class +is armed (--orphans / herdr.sweep.orphans) +and carries positive dispatch evidence — the run's +capture contract +(<tmp>/devagent-herdr-<n>/{out,err,done}) open +on fd 1/2 of one of its foreground processes; an operator's hand-run +worker never carries it. An in-flight pane is closed only by that orphan +class, and only when its collector is dead: the dispatching +devagent task / devagent pane-run (the process +that polls the done marker) is missing or detached from any live loop +driver. Repaired 2026-09-13 — both probes were dead and +the class unreachable; see the 2026-09-13 footer entry M @@ -4925,8 +4945,58 @@

23.1 Requirements

chaos schedule) so "the driver works perfectly" is a checkable claim, not a hope.

-

*Last updated: 2026-09-12 (the rescue reaches CLOSED-but-unmerged -pull requests: a goal-named closed PR is reopened and merged, or lands +

*Last updated: 2026-09-13 (herdr orphan-pane reaping: both probes +were dead, repaired as one change) — +devagent herdr-sweep --orphans had never reaped a live +orphan: pane process-info went out without +--session, so herdr answered for its own default session +and "no such pane" was every devagent pane's liveness, which left the +FR-VIS-07 in-flight guard, the FR-VIS-02 roster upgrade (#317) and the +orphan class — gated on a live foreground worker — inert at once; the +owner probe was herdr.*pane run .*<pane_id>, a +process that does not exist while a pane runs (pane run +types the script into the pane's shell and exits; verified live parent +chain +omp -> -zsh -> herdr --session <s> server -> launchd), +so PaneRunOwnerOrphaned always took its "no owner CLI -> +orphaned" exit and the driver spare behind it was unreachable. Each half +alone is a regression — a working owner probe over an inert liveness +probe closes live workers — so the fix lands atomically: +PaneForegroundWorker(cli, session, paneID) scopes the probe +(roster and sweep share it), the owner is now the surviving dispatcher +(argv0-anchored devagent/devagent-go +task/pane-run, optionally behind the driver's +timeout wrapper, so a prompt quoting devagent cannot +impersonate its own collector), non-worktree panes need positive +per-pane dispatch evidence (the capture contract on the worker's fd +1/2), and the orphan class moved ahead of the roster spare. Reaping also +fails closed: psPidsMatching and +pidAncestryCommands now report whether the probe answered +at all — pgrep's exit-1 "no match" is an answer (no collector, reap), an +absent binary / the 5s cap / a rejected pattern / an unfinished ancestry +walk is not (spare) — because a reaper that read "could not inspect" as +"no collector" would close every live pane on such a host. Test seams: +DEVAGENT_SWEEP_OWNER_PIDS_JSON is keyed by the pattern +asked for and a miss now counts as an unanswered probe (never a clean +no-match), DEVAGENT_SWEEP_PANE_CAPTURE_JSON supplies +pane-keyed evidence — and because the 2026-09-13 bug survived a green +stub suite, both shapes are pinned from outside: +TestProcFdsCaptureSeesRealCaptureContract runs the real +lsof probe against a live child holding +<tmp>/devagent-herdr-<n>/out on fd 1/2 (skipped +where lsof is absent), +TestPaneRunOwnerPatternMatchesRealDispatchShapes checks +paneRunOwnerPattern against the command lines +internal/loopdriver really builds and against the transient +herdr pane run shape it used to search for, and +TestOrphanClassGoesInertWhenProbesCannotAnswer pins the +spare-on-no-answer direction at the matrix level. +TestPsPidsMatchingAnswersOnlyWhenPgrepAnswers pins the same +split against real pgrep exit codes: 1 means no collector +(reap), 2 means the probe could not run (spare). Docs corrected: +FR-VIS-02/07/10, §18 Q23, docs/HERDR.md sweep safety. *Last +updated: 2026-09-12 (the rescue reaches CLOSED-but-unmerged pull +requests: a goal-named closed PR is reopened and merged, or lands nothing) — the driver-side verify-and-merge rescue, and the goal-text derivation feeding it, only accepted subjects that read OPEN before the dispatch, so the false closes were diff --git a/docs/PRD.md b/docs/PRD.md index a1157c18..8ab3fc75 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -1118,7 +1118,7 @@ Direction addendum 2026-09-03 (section 20): DevAgent becomes the local-first, BY | Q35 | ~~CI-Fixer (PRs #96/#97) re-dispatches `TASK-fix-` but fixer outcomes only surface as `autoReviewAndMergeOne` log lines and the terminal `ci-fix-failed` result — should fixer dispatches and outcomes write ledger rows like PR events do, so failure-cluster analytics can count fixer round-trips per goal, or stay log-only?~~ Resolved 2026-09-01 (PR #100 writes one dispatch + one outcome ledger row per `TASK-fix-` with still-red round-trip tests); removed. | eng | Phase 4 | | Q36 | ~~The CI-Fixer fix run (`TASK-fix-`) gets its own dispatch budget outside the originating task's `maxTaskRetries` — should fixer attempts count against the parent task's retry budget (linking to Q17's cumulative-attempt concern), or is one bounded fix attempt per PR run the right isolation?~~ Resolved 2026-09-07: isolated — the fixer gets exactly one bounded `TASK-fix-` re-dispatch per PR before the verdict falls back to request-changes (`src/integrations/autopr.ts:583-597`), outside the parent task's `maxTaskRetries`; the cross-round runaway concern is answered by Q17's lifetime `totalAttempts` cap (`--max-total-attempts`, `src/cli.ts:136`), the shared Q17/Q36 mechanism. Removed. | eng | Phase 4 | | Q22 | ~~PR #73 re-bridges a queued goal in the same cycle as board archive, but the archive itself burns the board's salvage history — should archived boards write a compact post-mortem (goal, failure class, gate excerpts) to the ledger so the next bridge can plan around the same failure mode, or is the existing ledger analytics query surface enough?~~ Resolved 2026-08-31: PRs #96/#97's `ci-fix-failed` outcome gives the ledger a structured failure record with summary evidence (matching the Q24 taxonomy); remaining failure-evidence work is tracked by the backlog item "Executor failure surface". | eng | Phase 4 | -| Q23 | ~~PR #77's herdr-sweep trusts the session name alone; if the sweep ever gains an `--all` mode, what stops it from killing a user-attached interactive claude pane that happens to sit in an automation-spawned session (2026-08-26 mass-kill class)? Require per-pane agent-state verification plus a managed-settings-style deny toggle, or keep `--all` out of scope permanently?~~ Resolved 2026-09-07: both halves of the safety requirement, and `--all` stays out of scope permanently — the sweep lists exactly one session and nothing widens it. Per-pane verification shipped as FR-VIS-07; the managed-settings-style toggle ships as FR-VIS-10: `herdr.sweep` (`src/config.ts:39`, resolved by `herdrSweepConfig` at `src/config.ts:427`) carries `enabled` (env `DEVAGENT_HERDR_SWEEP=0|1`) and `denySessions`, and `findStalePanes`/`sweepStalePanes` (`src/integrations/herdr.ts:462`, `:699`) return nothing when the sweep is disabled or the resolved session is denied. A pane the FR-VIS-02 roster reports as live, or any sweep launched while `DEVAGENT_OPERATOR_ATTACHED` is set, is spared ahead of every other class — including `--orphans` — and reported with `reason: operator-attached` instead of closed. Unset defaults are today's behavior; an invalid `herdr.sweep` block fails closed. Removed. | product | Phase 4 | +| Q23 | ~~PR #77's herdr-sweep trusts the session name alone; if the sweep ever gains an `--all` mode, what stops it from killing a user-attached interactive claude pane that happens to sit in an automation-spawned session (2026-08-26 mass-kill class)? Require per-pane agent-state verification plus a managed-settings-style deny toggle, or keep `--all` out of scope permanently?~~ Resolved 2026-09-07: both halves of the safety requirement, and `--all` stays out of scope permanently — the sweep lists exactly one session and nothing widens it. Per-pane verification ships as FR-VIS-07; the managed-settings-style toggle ships as FR-VIS-10: `herdr.sweep` (`src/config.ts:39`, resolved by `herdrSweepConfig` at `src/config.ts:427`) carries `enabled` (env `DEVAGENT_HERDR_SWEEP=0|1`) and `denySessions`, and `findStalePanes`/`sweepStalePanes` (`src/integrations/herdr.ts:462`, `:699`) return nothing when the sweep is disabled or the resolved session is denied. A pane the FR-VIS-02 roster reports as live, or any sweep launched while `DEVAGENT_OPERATOR_ATTACHED` is set, is spared ahead of every other class — including `--orphans` — and reported with `reason: operator-attached` instead of closed. Unset defaults are today's behavior; an invalid `herdr.sweep` block fails closed. **Corrected 2026-09-13:** only the env half of that ordering survives — the roster's `running` state is derived from the same foreground-worker probe the orphan class reads, so sparing it ahead of `--orphans` left the class unreachable; a live-worker pane is now orphan-reaped on collector evidence (FR-VIS-07), `DEVAGENT_OPERATOR_ATTACHED` still outranks everything, and the roster spare still covers worktree panes that are busy with no live worker. Removed. | product | Phase 4 | | Q24 | ~~Per-task PRs (PR #71) plus the auto-tag release workflow (PR #75) mean a fully-merged board can produce several PRs and a release in one cycle — should the ledger record release/tag events as first-class outcomes (so per-loop spend-to-shipped-artifact math counts a release), or stay PR-URL-only?~~ Resolved 2026-09-04: first-class — `ReleaseLedgerRecord` (`release-created`: tag, sha, version, source) at `src/orchestrator/ledger.ts:195` with `appendReleaseRecord`, `devagent record release` CLI, and the `release.yml` "Record release event in ledger" step write it; the §17 run-24 backlog bullet was stale and is struck. Removed. | eng | Phase 4 | | Q25 | ~~G5:STRIDE blocks on HIGH/CRITICAL with no suppression path; fixture credentials in test files (the exact pattern the golden suites ship) will trip it and stall autoMerge — add a per-path/per-finding allowlist committed with the PR, or keep it hard and force workers to rename literals?~~ Resolved: a PR may commit `.devagent/stride-allowlist.json` (`{"paths": [...glob patterns...]}`) read from the PR branch at gate time; findings whose file matches an allowed path are suppressed. Absent or malformed allowlists fail closed. | eng | Phase 4 | | Q26 | ~~PR #84 auto-stashes a dirty main before merge-back, but the stash is keyed by SHA and never re-offered — if a curation PR (like #83) is open in the same worktree when the factory merges back, should the popped stash be surfaced as a ledger warning (operator reapplies by hand) or re-applied automatically on the next dispatch?~~ Resolved 2026-09-07 (PR #172): ledger warning — `restoreAutoStash` pops the auto-stash by SHA after merge-back and writes a `merge-back-stash` ledger row with outcome `restored` or `retained` (`src/orchestrator/merge.ts:116-124`); a failed pop leaves the stash intact for manual recovery — no automatic re-apply on the next dispatch. Removed. | eng | Phase 4 | @@ -1434,7 +1434,7 @@ that posture. | ID | Requirement | Pri | |---|---|---| | FR-VIS-01 | Worker launches default to visible panes: the herdr runtime becomes the default spawn path when the `herdr` binary is present (config `herdr.enabled` default flips to `true`); fallback to invisible child processes stays automatic but is **loud** — one stderr warning per spawn site plus a `visibility=fallback` field on the run's ledger row, never silent | M | -| FR-VIS-02 | `devagent sessions` lists live worker sessions (task id, role, worker CLI, pane id, workspace, elapsed, agent_status from the herdr pane list) so the operator can find the pane to jump into; `devagent attach ` prints/opens the attach command for that pane. **Fixed 2026-09-12 (#317):** herdr's agent status machine never advances for **headless** worker invocations (`omp -p --mode json` via pane-run), so the roster (`/agents` → TUI chips, `devagent sessions`) rendered mid-run workers as "● idle"; `mapPaneState` now mirrors FR-VIS-07's sweep discriminator — an idle/unknown row whose `pane process-info` foreground process is a worker binary maps to `running`, agent_status stays the signal for interactive panes (`internal/herdr/roster.go` `paneState`) | M | +| FR-VIS-02 | `devagent sessions` lists live worker sessions (task id, role, worker CLI, pane id, workspace, elapsed, agent_status from the herdr pane list) so the operator can find the pane to jump into; `devagent attach ` prints/opens the attach command for that pane. **Fixed 2026-09-12 (#317):** herdr's agent status machine never advances for **headless** worker invocations (`omp -p --mode json` via pane-run), so the roster (`/agents` → TUI chips, `devagent sessions`) rendered mid-run workers as "● idle"; `mapPaneState` now mirrors FR-VIS-07's sweep discriminator — an idle/unknown row whose `pane process-info` foreground process is a worker binary maps to `running`, agent_status stays the signal for interactive panes (`internal/herdr/roster.go` `paneState`). **Fixed 2026-09-13:** that discriminator was inert in production — the `pane process-info` probe went out **without `--session`**, so herdr answered for its own default session and every devagent pane reported no foreground process; `PaneForegroundWorker` now takes the session and `paneState` passes it, so the upgrade actually fires | M | | FR-VIS-03 | Jump-in steering: attaching to a pane puts the operator in the worker's live terminal exactly like a human-run coding agent session — type into it, watch it think, interrupt it. The orchestrator's no-progress watchdog must not fight a human at the wheel: pane-level operator presence (attach time) suppresses auto-kill timers for that run and the ledger records `operator-attached` | S | | FR-VIS-04 | `pilot start`-style flags on the devagent loop drivers: `--headless` (explicit opt-out for CI/servers/LaunchAgents — the old invisible behavior becomes the named mode) and `--visible` (default); `devagent.json` `spawn.visibility: "visible" \| "headless"` per project with the env override `DEVAGENT_VISIBILITY` | M | | FR-VIS-05 | CI/factory parity: LaunchAgent-installed roles (scout/builder/tracker/orchestrator) keep running headless by default (no terminal to attach to), but every run they dispatch still lands in an attachable pane — visibility is a property of the **worker run**, not of the loop process | S | @@ -1449,10 +1449,10 @@ machine). FR-VIS-06..08 close them; FR-VIS-09 removes the double-driver failure | ID | Requirement | Pri | |---|---|---| | FR-VIS-06 | Every agent role dispatches through the pane runtime, not just coding workers: loop research and PO selection run inside herdr panes via `devagent pane-run` (falls back to a direct child only when herdr is unreachable, exit code 3 — loud, never silent); the scout routes its worker spawn through `runWorkerCli` so discovery lands in an attachable pane too | M | -| FR-VIS-07 | Sweep safety: `herdr-sweep` closes only panes whose cwd sits inside `.devagent-worktrees/` (automation-owned; operator scratch panes in the same session are untouchable) AND whose foreground process is not a worker CLI (`pane process-info` distinguishes a live omp/pi/claude/opencode from an idle shell) — an in-flight pane is never sweepable | M | +| FR-VIS-07 | Sweep safety: `herdr-sweep` closes a pane only where it can prove automation owns it. Inside `.devagent-worktrees/` the cwd **is** that proof (the dispatcher cd's the run into the task's worktree), and the pane must additionally not be running a worker CLI — `pane process-info`, asked in the session under sweep, distinguishes a live omp/pi/claude/opencode from an idle shell (the 2026-09-05 in-flight-close regression). Outside the worktrees the cwd proves nothing, so a pane joins the sweep only when the orphan class is armed (`--orphans` / `herdr.sweep.orphans`) **and** carries positive dispatch evidence — the run's capture contract (`/devagent-herdr-/{out,err,done}`) open on fd 1/2 of one of its foreground processes; an operator's hand-run worker never carries it. An in-flight pane is closed only by that orphan class, and only when its collector is dead: the dispatching `devagent task` / `devagent pane-run` (the process that polls the done marker) is missing or detached from any live loop driver. **Repaired 2026-09-13** — both probes were dead and the class unreachable; see the 2026-09-13 footer entry | M | | FR-VIS-08 | Loop-process visibility: loop-phase events (research/po/task) stay on the SSE stream and TUI cards (FR-VIS-02 surface), with the phase's pane discoverable via `devagent sessions`/`attach` while it runs | S | | FR-VIS-09 | Single-instance locking: `selfbuild-loop.sh` holds a portable mkdir lock (`.selfbuild/loop.lock.d`, stale-holder recovery via pid liveness) so an orphaned ppid=1 driver and a fresh start can never race loop numbering, ledger writes, or pane sweeps. **Extended 2026-09-07 to queue claims:** the same single-owner guarantee covers task claiming — an atomic `link()` claim lock per task, an incrementing fencing token (`leaseGeneration`/`leaseOwner`/`leaseExpiresAt`), expired leases reclaimable with a generation bump, and complete/fail/requeue writes refused when their token is stale | M | -| FR-VIS-10 | Sweep blast radius from the operator's side (Q23, added 2026-09-07): `herdr.sweep` is the managed-settings-style deny toggle — `enabled` (env `DEVAGENT_HERDR_SWEEP=0|1`) stops the sweep before it lists a pane, `denySessions` names sessions it must never touch even when one is the resolved target, and `orphans` (env `DEVAGENT_HERDR_SWEEP_ORPHANS=0|1`) overrides a loop driver's own `--orphans` either way; on top of that, any pane the FR-VIS-02 roster reports as live or any sweep launched under `DEVAGENT_OPERATOR_ATTACHED` is spared ahead of every other class and reported `operator-attached`. Unset = FR-VIS-07 behavior; invalid config fails closed | M | +| FR-VIS-10 | Sweep blast radius from the operator's side (Q23, added 2026-09-07): `herdr.sweep` is the managed-settings-style deny toggle — `enabled` (env `DEVAGENT_HERDR_SWEEP=0|1`) stops the sweep before it lists a pane, `denySessions` names sessions it must never touch even when one is the resolved target, and `orphans` (env `DEVAGENT_HERDR_SWEEP_ORPHANS=0|1`) overrides a loop driver's own `--orphans` either way; on top of that, a sweep launched under `DEVAGENT_OPERATOR_ATTACHED` spares the whole session ahead of every other class, and the FR-VIS-02 roster spare covers worktree panes that read busy with no live foreground worker. Spared panes are still reported, `operator-attached`. **Corrected 2026-09-13:** the exemption order is now scope → operator-at-the-wheel → orphan evidence → roster spare → status classes; the roster spare used to sit ahead of the orphan class, and because both are derived from the same `pane process-info` probe that ordering made the class structurally unreachable | M | **TUI dashboard (FR-TUI)** — pilot's dashboard as the reference layout (`docs/research/pilot-probe.md` §2; BSL 1.1 — patterns only, no code): @@ -1695,6 +1695,7 @@ existing driver with validation surfaces (test, command, telemetry, chaos schedule) so "the driver works perfectly" is a checkable claim, not a hope. --- +*Last updated: 2026-09-13 (herdr orphan-pane reaping: both probes were dead, repaired as one change) — `devagent herdr-sweep --orphans` had never reaped a live orphan: `pane process-info` went out without `--session`, so herdr answered for its own default session and "no such pane" was every devagent pane's liveness, which left the FR-VIS-07 in-flight guard, the FR-VIS-02 roster upgrade (#317) and the orphan class — gated on a live foreground worker — inert at once; the owner probe was `herdr.*pane run .*`, a process that does not exist while a pane runs (`pane run` types the script into the pane's shell and exits; verified live parent chain `omp -> -zsh -> herdr --session server -> launchd`), so `PaneRunOwnerOrphaned` always took its "no owner CLI -> orphaned" exit and the driver spare behind it was unreachable. Each half alone is a regression — a working owner probe over an inert liveness probe closes live workers — so the fix lands atomically: `PaneForegroundWorker(cli, session, paneID)` scopes the probe (roster and sweep share it), the owner is now the surviving dispatcher (argv0-anchored `devagent`/`devagent-go` `task`/`pane-run`, optionally behind the driver's `timeout` wrapper, so a prompt quoting devagent cannot impersonate its own collector), non-worktree panes need positive per-pane dispatch evidence (the capture contract on the worker's fd 1/2), and the orphan class moved ahead of the roster spare. Reaping also fails closed: `psPidsMatching` and `pidAncestryCommands` now report whether the probe answered at all — pgrep's exit-1 "no match" is an answer (no collector, reap), an absent binary / the 5s cap / a rejected pattern / an unfinished ancestry walk is not (spare) — because a reaper that read "could not inspect" as "no collector" would close every live pane on such a host. Test seams: `DEVAGENT_SWEEP_OWNER_PIDS_JSON` is keyed by the pattern asked for and a miss now counts as an unanswered probe (never a clean no-match), `DEVAGENT_SWEEP_PANE_CAPTURE_JSON` supplies pane-keyed evidence — and because the 2026-09-13 bug survived a green stub suite, both shapes are pinned from outside: `TestProcFdsCaptureSeesRealCaptureContract` runs the real `lsof` probe against a live child holding `/devagent-herdr-/out` on fd 1/2 (skipped where lsof is absent), `TestPaneRunOwnerPatternMatchesRealDispatchShapes` checks `paneRunOwnerPattern` against the command lines `internal/loopdriver` really builds and against the transient `herdr pane run` shape it used to search for, and `TestOrphanClassGoesInertWhenProbesCannotAnswer` pins the spare-on-no-answer direction at the matrix level. `TestPsPidsMatchingAnswersOnlyWhenPgrepAnswers` pins the same split against real `pgrep` exit codes: 1 means no collector (reap), 2 means the probe could not run (spare). Docs corrected: FR-VIS-02/07/10, §18 Q23, `docs/HERDR.md` sweep safety. *Last updated: 2026-09-12 (the rescue reaches CLOSED-but-unmerged pull requests: a goal-named closed PR is reopened and merged, or lands nothing) — the driver-side verify-and-merge rescue, and the goal-text derivation feeding it, only accepted subjects that read `OPEN` before the dispatch, so the false closes were unreachable: PR #346/#347 (auto-closed by the zombie sweep's `BaseBranchGone(main)` transport-noise bug minutes after they opened, green and mergeable) and loop 282's #345 each recorded `no-pr` with a landable branch left closed. `prView` now decodes `baseRefName`/`headRefName`, and a goal-named pull request that is CLOSED, unmerged, based on `main` with its head ref still on origin becomes the merge subject: `verifyAndMergeRescue` reopens it (`gh pr reopen`, repo-scoped, 60s wall, drain-tolerant), re-reads the state (gh's reopen is verified, never believed) and only then merges through the existing `mergePRBounded` pipeline. Everything else lands nothing — another/superseded base, a head ref gone from origin, a merge stamp, a reopen gh refuses — so no ship is ever fabricated. Pinned by `TestRunLoopClosedUnmergedGoalPRReopenedAndMerged`, `TestRunLoopClosedGoalPRLandsNothingWhenNotReopenable` and `TestVerifyAndMergeRescueRefusesNonReopenablePR` (`internal/loopdriver/run.go`, `issues.go`). *Last updated: 2026-09-12 (both green heads landed straight onto main — no pull request, no re-implementation) — the zombie sweep was still armed when PR #347 and PR #346 were opened, and its `BaseBranchGone(main)` transport-noise bug auto-closed both within minutes, so the two verified heads were landed straight onto main instead: `1f2ef8c` (`devagent/TASK-mtxyflr1-fq6y` — three-valued hygiene probes plus the auto-close age floor) fast-forwarded main, then `635214d` (`devagent/TASK-mtxwr39q-nva6` — the landing-certification merge window) merged on top. The code paths are disjoint (`internal/orchestrator` vs `internal/loopdriver`) and only `docs/PRD.*` overlapped. Neither PR was reopened or closed by this landing; both stay CLOSED, unmerged as pull requests. Verified on the merged tree: `go test ./...` green, including `TestAutoMergeTreatsBaseProbeNoiseAsAlive` and `TestGhNotFoundOnlyForRealGone`, and `git merge-base --is-ancestor` true for both heads. *Last updated: 2026-09-12 (zombie-PR hygiene probes are three-valued, with an auto-close age floor) — both gh probes behind the `pr-hygiene` sweep collapsed "gh never answered" into a decisive answer: `BaseBranchGone` returned `err != nil`, so a network hiccup, 5xx, or rate-limit response on the `branches/` lookup read as a dead base and the sweep CLOSED a live PR as `base-superseded` (PR #346: created 05:27:21Z, closed 05:28:37Z, 76s later, never merged); `landingCommitOnMain` returned empty on a failed or unparseable commit listing, so transport noise read as "no landing commit" and the PR was closed as `superseded` with an explicit "not shipped" comment. `ProbeBaseBranch` (internal/orchestrator/autopr.go) now answers `BaseAlive` / `BaseGone` / `BaseUnknown`, judging "gone" only on gh's own not-found stderr (`ghNotFound` reads the diagnostic, never the argv-embedding message, whose ref names may themselves contain "404"; `GhError.Code` is the gh exit code and is never consulted), and `landingCommitOnMain` returns `(sha, ok)` with `ok` false whenever the listing failed. On `unknown` the sweep records `untouched`/`base-unknown` or `untouched`/`landing-unknown` and acts on nothing; the merge-queue gate treats `BaseUnknown` as alive, so a probe hiccup no longer parks a mergeable PR. Both auto-close arms now also wait out an age floor: `closeAgeFloorHours` (24h) capped by `prHygiene.graceHours`, with an unknown PR age treated as floored — a fresh PR is reported `untouched`/`base-superseded-age-floor` or `untouched`/`landing-age-floor` and the `--grace-hours` help states the floor. Regression-pinned by `TestSweepTaskPrHygiene` (base-probe noise, fresh and unknown-age floors), `TestSweepTaskPrHygieneLandingEvidence` (landing noise, unparseable listing, landing floor), `TestAutoMergeTreatsBaseProbeNoiseAsAlive`, and `TestGhNotFoundOnlyForRealGone` — each fails against the pre-fix behavior.* diff --git a/internal/daemon/daemon_test.go b/internal/daemon/daemon_test.go index 558f1d70..9e071f19 100644 --- a/internal/daemon/daemon_test.go +++ b/internal/daemon/daemon_test.go @@ -669,7 +669,7 @@ func TestKillViaAnswer(t *testing.T) { if len(fake.calls) != 4 { t.Fatalf("herdr calls = %v", fake.calls) } - wantProbe := []string{"pane", "process-info", "--pane", "p2"} + wantProbe := []string{"--session", "sess", "pane", "process-info", "--pane", "p2"} wantKeys := []string{"--session", "sess", "pane", "send-keys", "p1", "ctrl+c"} wantClose := []string{"--session", "sess", "workspace", "close", "w1"} if strings.Join(fake.calls[1], " ") != strings.Join(wantProbe, " ") { diff --git a/internal/herdr/herdr_test.go b/internal/herdr/herdr_test.go index 057879d7..f0854a06 100644 --- a/internal/herdr/herdr_test.go +++ b/internal/herdr/herdr_test.go @@ -399,17 +399,23 @@ func TestSessionResolution(t *testing.T) { func TestPaneForegroundWorkerContract(t *testing.T) { cli := newFakeCli(t) cli.procInfo["wX:p1"] = "testdata/sweep/process-info-omp.json" - if !PaneForegroundWorker(cli, "wX:p1") { + if !PaneForegroundWorker(cli, "devagent", "wX:p1") { t.Error("omp foreground should read as live") } + // The probe is scoped to the session it asks about. Unscoped, herdr + // answers for its own default session, every devagent pane reports no + // foreground process, and liveness was false for all of them (2026-09-13). + if !cli.called("--session devagent pane process-info --pane wX:p1") { + t.Fatalf("process-info argv = %v", cli.calls) + } cli.procInfo["wX:p2"] = "testdata/sweep/process-info-zsh.json" - if PaneForegroundWorker(cli, "wX:p2") { + if PaneForegroundWorker(cli, "devagent", "wX:p2") { t.Error("zsh foreground should read as idle") } - if PaneForegroundWorker(cli, "") { + if PaneForegroundWorker(cli, "devagent", "") { t.Error("empty pane id should be false") } - if PaneForegroundWorker(cli, "wX:missing") { + if PaneForegroundWorker(cli, "devagent", "wX:missing") { t.Error("unsupported reply should be false (best-effort)") } } @@ -459,11 +465,21 @@ func TestLedgerNowShape(t *testing.T) { } func TestPidSeamsDirect(t *testing.T) { - orphanSeams(t, "4242\n0\nbogus\n", map[string][]string{"4242": {"bash scripts/selfbuild-loop.sh"}}) - got := psPidsMatching("ignored") - if len(got) != 1 || got[0] != 4242 { + // The real probe drops junk and self/pid-1 rows; parsePidList owns that. + if got := parsePidList("4242\n0\nbogus\n"); len(got) != 1 || got[0] != 4242 { t.Fatalf("pids = %v, want [4242]", got) } + orphanSeams(t, "4242\n", map[string][]string{"4242": {"bash scripts/selfbuild-loop.sh"}}) + // The collector stub is keyed by the pattern asked for: only what the + // sweep actually searches for gets an answer, and anything else is an + // unanswered probe (never a clean "no match", which would reap). + got, answered := psPidsMatching(paneRunOwnerPattern) + if !answered || len(got) != 1 || got[0] != 4242 { + t.Fatalf("pids = %v (answered=%v), want [4242] answered", got, answered) + } + if pids, ok := psPidsMatching("herdr.*pane run .*wX:p9"); ok || len(pids) != 0 { + t.Fatalf("unstubbed pattern = %v (answered=%v), want no answer", pids, ok) + } if PaneRunOwnerOrphaned("wX:p9") { t.Fatal("owner with live driver ancestry should NOT be orphaned") } @@ -479,11 +495,18 @@ func TestPidSeamsDirect(t *testing.T) { if !HasLoopDriverAncestor([]string{"nohup devagent loop"}) { t.Error("installed-binary loop driver ancestor undetected") } - // Missing ancestry entry = no evidence = orphaned. - orphanSeams(t, "4242\n", map[string][]string{}) + // A walk that completed with no driver above the collector is an answer: + // detached owner -> orphaned. + orphanSeams(t, "4242\n", map[string][]string{"4242": {}}) if !PaneRunOwnerOrphaned("wX:p9") { t.Fatal("detached owner should be orphaned") } + // A collector whose ancestry was never walked is no evidence at all: the + // reaper leaves it running. + orphanSeams(t, "4242\n", map[string][]string{}) + if PaneRunOwnerOrphaned("wX:p9") { + t.Fatal("unanswered ancestry probe must not assert orphanhood") + } } func TestResolveSweepSettingsFailsClosedOnBadConfig(t *testing.T) { @@ -549,7 +572,9 @@ func TestFixtureFilesExist(t *testing.T) { "testdata/sweep/pane-list-scratch.json", "testdata/sweep/pane-list-agentless.json", "testdata/sweep/pane-list-two-idle.json", + "testdata/sweep/pane-list-main-checkout.json", "testdata/sweep/process-info-omp.json", + "testdata/sweep/process-info-omp-handrun.json", "testdata/sweep/process-info-zsh.json", "testdata/sweep/agent-list-running.json", "testdata/sweep/agent-list-idle.json", diff --git a/internal/herdr/roster.go b/internal/herdr/roster.go index f1d43429..05e35d5d 100644 --- a/internal/herdr/roster.go +++ b/internal/herdr/roster.go @@ -45,10 +45,12 @@ func mapPaneState(agentStatus, cwd string) string { // paneState maps a roster row's agent_status to a display state, upgrading // idle/unknown rows with a live worker foreground process to "running" // (regression 2026-09-11, #317). Best-effort: an uninspectable pane keeps the -// status-derived state. -func paneState(cli CliRunner, agentStatus, cwd, paneID string) string { +// status-derived state. The probe is session-scoped like every herdr call — +// unscoped it read herdr's own default session and liveness was always false +// (2026-09-13, the same bug that left the sweep's orphan class unreachable). +func paneState(cli CliRunner, session, agentStatus, cwd, paneID string) string { state := mapPaneState(agentStatus, cwd) - if idleStatuses[agentStatus] && PaneForegroundWorker(cli, paneID) { + if idleStatuses[agentStatus] && PaneForegroundWorker(cli, session, paneID) { return "running" } return state @@ -159,7 +161,7 @@ func ListSessionPanes(cli CliRunner, session string) []SessionPaneInfo { Label: label, Cwd: cwd, AgentStatus: agentStatus, - State: paneState(cli, agentStatus, cwd, paneID), + State: paneState(cli, s, agentStatus, cwd, paneID), PaneID: paneID, // Never fabricated: only what herdr reports. StartedAt: derefOr(a.CreatedAt, ""), diff --git a/internal/herdr/sweep.go b/internal/herdr/sweep.go index 9ab1f27c..a04cc4f8 100644 --- a/internal/herdr/sweep.go +++ b/internal/herdr/sweep.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "os/exec" + "regexp" "strconv" "strings" "time" @@ -108,6 +109,14 @@ func derefOr(s *string, def string) string { // running a worker CLI right now. (b) uses herdr's own pane process-info: a // LIVE pane's foreground process is the worker binary (omp/pi/claude/opencode); // an idle pane sits at its shell. +// +// 2026-09-13 (a) stopped proving anything on its own and (b) was always false: +// the process-info probe went out unscoped (no --session), so herdr answered +// for its own default session and every live worker read as idle — while the +// roster, which shares that probe, kept claiming the same pane was stale. The +// matrix now probes once, in session, and orders the exemptions so the orphan +// class is reachable: scope (where the pane may be swept at all) → operator at +// the wheel → orphan evidence → roster spare → status classes. func FindStalePanes(cli CliRunner, session string, opts SweepOptions) []StalePane { var sweep config.HerdrSweepSettings if opts.Sweep != nil { @@ -147,20 +156,28 @@ func FindStalePanes(cli CliRunner, session string, opts SweepOptions) []StalePan // sweepable. attachEnv := os.Getenv(PaneEnvOpAttach) envAttached := attachEnv != "" && attachEnv != "0" && !strings.EqualFold(attachEnv, "false") - // Roster lookup is lazy: only paid for when a worktree candidate exists. + // Roster lookup is lazy: only paid for when a worktree candidate is left + // standing after the liveness probe. var runningPanes map[string]bool rosterLoaded := false for _, p := range rows { status := derefOr(p.AgentStatus, "unknown") - cwd := p.cwd() - if !strings.Contains(cwd, ".devagent-worktrees") { + paneID := p.paneID() + // FR-VIS-07 automation ownership. A `.devagent-worktrees` checkout is + // itself proof devagent spawned the pane (the dispatcher cd's the + // run into the task's worktree), so the status classes below reach it. + // Anywhere else — the main checkout, where the research/PO phases run + // their dispatches (FR-VIS-06) beside the operator's own panes — the + // cwd proves nothing, so the pane only joins the sweep when the + // orphan class is armed, and then only on positive dispatch evidence. + automation := strings.Contains(p.cwd(), ".devagent-worktrees") + if !automation && !orphans { continue } - paneID := p.paneID() - // FR-VIS-10 (Q23): an operator at the wheel outranks every stale - // signal — including the orphan class. The pane is reported with - // reason `operator-attached` so a dry-run explains why it survived, - // and SweepStalePanes never closes it. + // Q23: an operator at the wheel outranks every stale signal, including + // the orphan class. The pane is reported with reason + // `operator-attached` so a dry-run explains why it survived, and + // SweepStalePanes never closes it. if envAttached { stale = append(stale, StalePane{ WorkspaceID: p.workspaceID(), PaneID: paneID, Label: p.label(), @@ -168,6 +185,45 @@ func FindStalePanes(cli CliRunner, session string, opts SweepOptions) []StalePan }) continue } + info := paneProcessInfoCli(cli, session, paneID) + if paneWorkerRunning(info) { + // 2026-09-07: a live foreground worker is normally "leave it + // alone" — but when the run has no live collector nobody polls + // the done marker or closes the workspace: the driver died + // mid-task (2026-09-07 v11 OOM-kill orphaned pane w68 + omp + // 45472 for hours, burning tokens with no collector). Only the + // loop driver may assert orphanhood (opts.orphans) — at an + // iteration head the driver is synchronous, so a live worker it + // does not own is definitionally a dead driver's leftover. + // + // This outranks the roster spare on purpose: the FR-VIS-02 + // "running" state is derived from this same probe (#317), so a + // pane whose roster says running is exactly what an orphan looks + // like — sparing there first left the class unreachable. An + // operator really at the wheel is the env gate above, not the + // roster. + // + // Collector evidence is process-wide: no pane id joins any + // surviving argv, so ANY dispatcher still owned by a live loop + // driver spares every live pane in the session. Dispatch evidence + // (the capture contract on the worker's own fds) is per-pane and + // narrows the non-worktree class: an operator's hand-run `omp` + // never carries it, so scratch panes are structurally spared. + if orphans && PaneRunOwnerOrphaned(paneID) && + (automation || paneDispatchEvidence(paneID, info)) { + stale = append(stale, StalePane{ + WorkspaceID: p.workspaceID(), PaneID: paneID, Label: p.label(), + AgentStatus: status, Reason: "orphaned-driver", + }) + } + continue + } + if !automation { + // Outside the worktrees and not a live orphan: a pane at its own + // shell in the main checkout is the operator's window, and the + // sweep has no evidence it ever belonged to anyone else. + continue + } if !rosterLoaded { runningPanes = rosteredRunningPaneIDs(cli, session) rosterLoaded = true @@ -179,26 +235,6 @@ func FindStalePanes(cli CliRunner, session string, opts SweepOptions) []StalePan }) continue } - if PaneForegroundWorker(cli, paneID) { - // 2026-09-07: a live foreground worker is normally "leave it - // alone" — but when its OWNER (the `herdr pane run` CLI the - // dispatching `devagent task` spawned) is gone or detached from - // the loop driver, nobody polls the run or closes the pane: the - // driver died mid-task (2026-09-07 v11 OOM-kill orphaned pane w68 - // + omp 45472 for hours, burning tokens with no collector). Only - // the loop driver may assert orphanhood (opts.orphans) — at an - // iteration head the driver is synchronous, so a live worker it - // does not own is definitionally a dead driver's leftover. - // Operator manual tasks keep their shell in the ancestry and are - // spared. - if orphans && PaneRunOwnerOrphaned(paneID) { - stale = append(stale, StalePane{ - WorkspaceID: p.workspaceID(), PaneID: paneID, Label: p.label(), - AgentStatus: status, Reason: "orphaned-driver", - }) - } - continue - } // No agent at all (bare shell in a worktree) => leftover; known agent // => only idle ones. if p.AgentStatus == nil { @@ -238,40 +274,137 @@ var workerBinNames = map[string]bool{ "opencode2": true, } -// PaneForegroundWorker mirrors paneForegroundWorker(): true when the pane's -// foreground process is a worker CLI (live dispatch). Best-effort: a -// process-info failure returns false so the status-based sweep still applies — -// a pane we cannot inspect is treated like before. -func PaneForegroundWorker(cli CliRunner, paneID string) bool { +// paneRunPid: one foreground process of a pane, with the pid needed to check +// its file descriptors (the capture contract) for dispatch evidence. +type paneRunPid struct { + Name *string `json:"name"` + Argv0 *string `json:"argv0"` + Pid *int `json:"pid"` +} + +func (p paneRunPid) key() string { + if p.Name != nil && *p.Name != "" { + return *p.Name + } + if p.Argv0 != nil { + return *p.Argv0 + } + return "" +} + +// paneRunProcessInfo mirrors the `pane process-info` reply (the subset the +// sweep consumes). +type paneRunProcessInfo struct { + ForegroundProcesses []paneRunPid `json:"foreground_processes"` +} + +// paneProcessInfoCli fetches one pane's process-info in session, or nil when +// the probe fails (best-effort contract). +func paneProcessInfoCli(cli CliRunner, session, paneID string) *paneRunProcessInfo { if paneID == "" { - return false + return nil } - r := cli.HerdrCli([]string{"pane", "process-info", "--pane", paneID}, 5_000) + r := cli.HerdrCli([]string{"--session", session, "pane", "process-info", "--pane", paneID}, 5_000) if r.Code != 0 { - return false + return nil } var envelope struct { Result *struct { - ProcessInfo *struct { - ForegroundProcesses []struct { - Name *string `json:"name"` - Argv0 *string `json:"argv0"` - } `json:"foreground_processes"` - } `json:"process_info"` + ProcessInfo *paneRunProcessInfo `json:"process_info"` } `json:"result"` } if err := json.Unmarshal([]byte(r.Stdout), &envelope); err != nil || envelope.Result == nil || envelope.Result.ProcessInfo == nil { + return nil + } + return envelope.Result.ProcessInfo +} + +// PaneForegroundWorker mirrors paneForegroundWorker(): true when the pane's +// foreground process is a worker CLI (live dispatch). Best-effort: a +// process-info failure returns false so the status-based sweep still applies — +// a pane we cannot inspect is treated like before. +// +// `session` is load-bearing: herdr scopes every subcommand, so an unscoped +// probe answers for whatever session the binary defaults to. For a devagent +// pane that is "no such pane", which made liveness read false for every pane +// (2026-09-13) and left both the busy-pane guard and the orphan class inert. +func PaneForegroundWorker(cli CliRunner, session, paneID string) bool { + return paneWorkerRunning(paneProcessInfoCli(cli, session, paneID)) +} + +func paneWorkerRunning(info *paneRunProcessInfo) bool { + if info == nil { return false } - for _, proc := range envelope.Result.ProcessInfo.ForegroundProcesses { - key := derefOr(proc.Name, derefOr(proc.Argv0, "")) - if workerBinNames[strings.ToLower(key)] { + for _, proc := range info.ForegroundProcesses { + if workerBinNames[strings.ToLower(proc.key())] { return true } } return false } +// captureFileRe matches the capture-contract paths of a pane run: +// `/devagent-herdr-/{out,err,done}`, written by +// RunCommandInHerdrPane's script (herdr.go) and removed by its deferred +// cleanup — so the descriptor is open exactly while a dispatch owns the run. +var captureFileRe = regexp.MustCompile(`/devagent-herdr-\d+/(?:out|err|done)`) + +// paneDispatchEvidence is the per-pane proof that devagent started this pane's +// worker: one of its foreground processes holds the capture contract on stdout +// or stderr. `pane run` types the script into the pane's interactive shell and +// exits, so no argv in the pane's tree names the contract (verified live: +// `omp -> -zsh -> herdr --session server`), and the `herdr pane run` +// process the 2026-09-07 probe looked for is never in the table at all. A +// hand-run operator worker points at the pane tty and never matches — which is +// what keeps a pane outside an automation worktree sweepable only when devagent +// really dispatched it. Best-effort: a failed probe is no evidence. +// +// Test seam: DEVAGENT_SWEEP_PANE_CAPTURE_JSON = { "": "" } — +// the same pane-keyed shape as the process-info fixtures, so dispatch evidence +// is deterministic in CI without real panes (the pid-keyed form would make a +// fixture's pid field load-bearing for two sweeps that disagree about it). +// +// ponytail: the ceiling is the file-name shape — if the capture contract's +// scratch prefix or names change, captureFileRe has to follow. A dispatch +// manifest (pane id -> capture dir -> dispatcher pid), written by openPane and +// read here, would retire both the regex and the fd probe. +func paneDispatchEvidence(paneID string, info *paneRunProcessInfo) bool { + if info == nil { + return false + } + if stub, ok := os.LookupEnv("DEVAGENT_SWEEP_PANE_CAPTURE_JSON"); ok { + var m map[string]string + if err := json.Unmarshal([]byte(stub), &m); err != nil { + return false + } + return captureFileRe.MatchString(m[paneID]) + } + for _, proc := range info.ForegroundProcesses { + if proc.Pid == nil || *proc.Pid <= 1 { + continue + } + if captureFileRe.MatchString(procFdsCapture(*proc.Pid)) { + return true + } + } + return false +} + +// procFdsCapture runs the lsof fd probe for pid and returns the `n`-prefixed +// path lines of fd 1/2 joined — the descriptor targets that carry the capture +// contract. Empty on lsof absence/failure (conservative: no evidence). +func procFdsCapture(pid int) string { + out, _ := runProc("lsof", []string{"-a", "-p", strconv.Itoa(pid), "-d", "1", "-d", "2", "-Fn"}) + var paths []string + for _, line := range strings.Split(out, "\n") { + if strings.HasPrefix(line, "n") && line != "n" { + paths = append(paths, line[1:]) + } + } + return strings.Join(paths, " ") +} + // HasLoopDriverAncestor mirrors hasLoopDriverAncestor(): true when any // ancestor command of the pane-run owner names the live selfbuild loop // driver — the Go `devagent loop` command (argv[0] is the built binary, so @@ -289,44 +422,108 @@ func HasLoopDriverAncestor(ancestryCommands []string) bool { return false } -// PaneRunOwnerOrphaned mirrors paneRunOwnerOrphaned(): orphan check for one -// pane's live worker — find the `herdr pane run ` owner CLI process -// (spawned by the dispatching `devagent task`), walk its ppid ancestry via ps, -// and require a live selfbuild loop driver somewhere in it. Missing owner -// CLI = the poller died = orphaned. ps failures are conservative: without -// evidence the pane is left alone. +// paneRunOwnerPattern is the pgrep -f pattern for a pane run's OWNER: the +// dispatching devagent CLI (`devagent task --prompt …`, `devagent pane-run …`) +// with or without the driver's `timeout ` wrapper. +// +// 2026-09-13: it used to match `herdr.*pane run .*`. That process does +// not exist while a pane runs: herdr types the script into the pane's +// interactive shell and exits, and the pane tree reparents under the herdr +// server (verified against a live in-flight dispatch — empty pgrep, parent +// chain `omp -> -zsh -> herdr --session server -> launchd`). So the probe +// matched nothing for every pane, PaneRunOwnerOrphaned always took its +// "no owner CLI -> orphaned" exit, and the HasLoopDriverAncestor spare below +// was unreachable. The CLI that survives the whole run — the one polling the +// done marker — is the dispatcher. +// +// Anchored at argv0 on purpose: an `omp -p ` command line quotes +// devagent commands (ticket text does), and an unanchored pattern would let a +// worker impersonate its own collector. +const paneRunOwnerPattern = `^(timeout [0-9]+ )?[^ ]*devagent(-go)? (task|pane-run)( |$)` + +// PaneRunOwnerOrphaned mirrors paneRunOwnerOrphaned(): orphan check for a live +// pane worker — find the dispatching devagent CLI that owns the run, walk its +// ppid ancestry via ps, and require a live selfbuild loop driver somewhere in +// it. Missing owner CLI = the poller died with its driver = orphaned. +// +// Both probes have to ANSWER before anything is called an orphan. A pgrep that +// could not run (absent binary, the 5s cap, a rejected pattern) or an ancestry +// walk that never completed is no evidence, and asserting orphanhood on no +// evidence closes live workers — the exact failure class the 2026-09-05 and +// 2026-09-07 regressions were about. So the reaper fails toward leaving a pane +// running: an unanswered probe spares the session. +// +// The pane id cannot join the match: RunCommandInHerdrPane opens the pane +// inside the dispatching process, so no argv anywhere carries the pair. +// Ownership is therefore asserted process-wide — any dispatcher still hanging +// off a live driver spares every live pane in the session. +// +// ponytail: ceiling is per-session attribution, not correctness of the spare: +// the error direction is "leave a leftover running", never "close a run +// somebody is still collecting". Upgrading to per-pane ownership needs a +// dispatch manifest on disk (pane id -> capture dir -> dispatcher pid) written +// by openPane and read here. func PaneRunOwnerOrphaned(paneID string) bool { if paneID == "" { return false } - pids := psPidsMatching("herdr.*pane run .*" + paneID) + pids, answered := psPidsMatching(paneRunOwnerPattern) + if !answered { + return false // the process table never answered: nothing to assert + } if len(pids) == 0 { - return true // no owner CLI at all -> nothing polls this run + return true // it answered "no collector": nothing polls this run } for _, pid := range pids { - if HasLoopDriverAncestor(pidAncestryCommands(pid)) { + ancestry, walked := pidAncestryCommands(pid) + if !walked { + return false // an uninspectable collector is not a dead one + } + if HasLoopDriverAncestor(ancestry) { return false // live driver owns it } } return true // owner exists but detached from any live driver } -// psPidsMatching mirrors psPidsMatching(): pgrep -f for the pane-run owner -// CLI; empty on pgrep absence/failure. +// psPidsMatching mirrors psPidsMatching(): pgrep -f over the process table. +// The second result is whether the probe ANSWERED: exit 1 with no output is +// pgrep's definitive "no match" (answered, empty), while an absent binary, the +// 5s cap, or a rejected pattern is no answer — and a caller that read "no +// answer" as "no collector" would reap every live pane on a host that cannot +// inspect its own process table. Its one caller asks for the pane-run OWNER, +// which since 2026-09-13 is the dispatching devagent CLI rather than the +// transient `herdr pane run` client (see paneRunOwnerPattern). // -// Test seam: DEVAGENT_SWEEP_OWNER_PIDS stubs the process table (pgrep output -// format, one pid per line) so the orphan path is deterministic without -// spawning real pane-run CLIs. -func psPidsMatching(pattern string) []int { - if stubPids, ok := os.LookupEnv("DEVAGENT_SWEEP_OWNER_PIDS"); ok { - return parsePidList(stubPids) +// Test seam: DEVAGENT_SWEEP_OWNER_PIDS_JSON = { "": "" } (pids +// in pgrep output format, one per line) stubs the process table so the +// collector path is deterministic without spawning real dispatchers. It is +// keyed BY the pattern asked for, so a probe retargeted away from +// paneRunOwnerPattern misses — and misses as an unanswered probe, never as a +// clean "no match" — and its test goes red instead of a global stub quietly +// handing out the old answer; per-pane dispatch evidence is stubbed separately, +// keyed by pane (DEVAGENT_SWEEP_PANE_CAPTURE_JSON). +func psPidsMatching(pattern string) ([]int, bool) { + if stub, ok := os.LookupEnv("DEVAGENT_SWEEP_OWNER_PIDS_JSON"); ok { + var byPattern map[string]string + if err := json.Unmarshal([]byte(stub), &byPattern); err != nil { + return nil, false + } + raw, answered := byPattern[pattern] + if !answered { + return nil, false + } + return parsePidList(raw), true } out, code := runProc("pgrep", []string{"-f", pattern}) - _ = code // pgrep exits 1 on "no match" — that is an answer, not a failure - if code != 0 && strings.TrimSpace(out) == "" { - return nil + switch { + case code == 0: + return parsePidList(out), true + case code == 1 && strings.TrimSpace(out) == "": + return nil, true // "no match" is an answer, not a failure + default: + return nil, false // spawn failure, timeout, or pgrep usage error } - return parsePidList(out) } func parsePidList(out string) []int { @@ -363,43 +560,55 @@ func runProc(name string, args []string) (string, int) { } // pidAncestryCommands mirrors pidAncestryCommands(): walk the ppid chain from -// `pid` toward init, collecting each ancestor's command line. Depth-capped; -// ps failure yields nil (callers treat that as "no evidence" and stay -// conservative). +// `pid` toward init, collecting each ancestor's command line. Depth-capped. +// The second result is whether the walk ANSWERED: reaching init (ppid <= 1), or +// a ps reporting the process as already gone, completes it; a ps that could not +// run or output it cannot parse does not, and the caller leaves the pane alone. // // Test seam: DEVAGENT_SWEEP_ANCESTRY_JSON = { "": ["cmd", ...] } — the -// stubbed ppid walk for orphan tests (no real ps in CI). -func pidAncestryCommands(pid int) []string { +// stubbed ppid walk for orphan tests (no real ps in CI). A pid key present with +// an empty list is a walk that completed with no driver above it; a missing key +// is an unanswered probe. +func pidAncestryCommands(pid int) ([]string, bool) { if stub, ok := os.LookupEnv("DEVAGENT_SWEEP_ANCESTRY_JSON"); ok { var m map[string][]string if err := json.Unmarshal([]byte(stub), &m); err != nil { - return nil + return nil, false } - return m[strconv.Itoa(pid)] + commands, walked := m[strconv.Itoa(pid)] + return commands, walked } var commands []string current := pid for range 24 { out, code := runProc("ps", []string{"-o", "ppid=,command=", "-p", strconv.Itoa(current)}) if code != 0 { - break + // ps exits 1 with no row for a pid that already died: that is an + // answer (no driver above it). Anything else inspected nothing. + if code == 1 && strings.TrimSpace(out) == "" { + break + } + return commands, false } line := strings.TrimSpace(out) if line == "" { - break + break // accepted query, no row: the process is gone } sep := strings.IndexByte(line, ' ') if sep <= 0 { - break + return commands, false // unparseable row: no evidence either way } commands = append(commands, strings.TrimSpace(line[sep+1:])) ppid, err := strconv.Atoi(strings.TrimSpace(line[:sep])) - if err != nil || ppid <= 1 { - break + if err != nil { + return commands, false + } + if ppid <= 1 { + break // reached init/launchd: the chain is complete } current = ppid } - return commands + return commands, true } // SweepStalePanes mirrors sweepStalePanes(): close every stale pane workspace diff --git a/internal/herdr/sweep_test.go b/internal/herdr/sweep_test.go index c0b448c0..a7a4545d 100644 --- a/internal/herdr/sweep_test.go +++ b/internal/herdr/sweep_test.go @@ -4,7 +4,9 @@ import ( "encoding/json" "fmt" "os" + "os/exec" "path/filepath" + "regexp" "strconv" "strings" "testing" @@ -16,6 +18,13 @@ import ( // vitest stub CLIs (test/herdr-sweep.test.ts): `pane list` from a pane-list // fixture, `pane process-info` per pane, `agent list` from a roster fixture, // and `workspace close` logged instead of executed. +// +// It also enforces the session-scoping contract: herdr resolves every +// subcommand inside one session, so a command that ships without `--session` +// answers for whatever session the binary defaults to and the caller reads a +// false negative as an answer. `pane process-info` is the case that mattered +// (2026-09-13: unscoped, so every live worker looked idle and the orphan class +// was unreachable); the fake fails loudly instead of serving it. type fakeCli struct { t *testing.T // paneList: testdata path (or inline JSON) for `pane list`. @@ -29,19 +38,39 @@ type fakeCli struct { failList bool // closed records workspace close invocations. closed []string - // calls records every argv (joined with spaces) in order. + // calls records every argv as invoked (joined with spaces, --session + // included), in order. calls []string + // dir is this package's directory at construction, so fixture paths stay + // valid when a test chdirs to control the config the sweep resolves. + dir string } func newFakeCli(t *testing.T) *fakeCli { t.Helper() - return &fakeCli{t: t, procInfo: map[string]string{}} + dir, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + return &fakeCli{t: t, procInfo: map[string]string{}, dir: dir} +} + +// called reports whether any recorded argv contains want. +func (f *fakeCli) called(want string) bool { + for _, c := range f.calls { + if strings.Contains(c, want) { + return true + } + } + return false } -// fixture reads a testdata file relative to this package. +// fixture reads a testdata file from this package's directory, captured when +// the fake was built: a sweep test that chdirs (to control which devagent.json +// the resolver reads) must still find its fixtures. func (f *fakeCli) fixture(rel string) string { f.t.Helper() - data, err := os.ReadFile(filepath.Join("testdata", rel)) + data, err := os.ReadFile(filepath.Join(f.dir, "testdata", rel)) if err != nil { f.t.Fatalf("read fixture %s: %v", rel, err) } @@ -49,50 +78,51 @@ func (f *fakeCli) fixture(rel string) string { } func (f *fakeCli) HerdrCli(args []string, timeoutMs int) CliResult { - if len(args) > 1 && args[0] == "--session" { - args = args[2:] + raw := strings.Join(args, " ") + session := "" + cmd := args + if len(args) >= 2 && args[0] == "--session" { + session, cmd = args[1], args[2:] + } + f.calls = append(f.calls, raw) + body := func(s string) string { + if strings.HasPrefix(s, "testdata/") { + return f.fixture(strings.TrimPrefix(s, "testdata/")) + } + return s } - f.calls = append(f.calls, strings.Join(args, " ")) switch { - case len(args) >= 2 && args[0] == "pane" && args[1] == "list": + case len(cmd) >= 2 && cmd[0] == "pane" && cmd[1] == "list": if f.failList { return CliResult{Code: 1, Stderr: "server_not_running"} } - body := f.paneList - if strings.HasPrefix(body, "testdata/") { - body = f.fixture(strings.TrimPrefix(body, "testdata/")) - } - return CliResult{Code: 0, Stdout: body} - case len(args) >= 2 && args[0] == "agent" && args[1] == "list": + return CliResult{Code: 0, Stdout: body(f.paneList)} + case len(cmd) >= 2 && cmd[0] == "agent" && cmd[1] == "list": if f.failList { return CliResult{Code: 1, Stderr: "server_not_running"} } - body := f.agentList - if strings.HasPrefix(body, "testdata/") { - body = f.fixture(strings.TrimPrefix(body, "testdata/")) + return CliResult{Code: 0, Stdout: body(f.agentList)} + case len(cmd) >= 2 && cmd[0] == "pane" && cmd[1] == "process-info": + if session == "" { + f.t.Errorf("pane process-info invoked without --session (argv %q): the probe would answer for herdr's default session", raw) + return CliResult{Code: 1, Stderr: "fakeCli: unscoped process-info"} } - return CliResult{Code: 0, Stdout: body} - case len(args) >= 2 && args[0] == "pane" && args[1] == "process-info": pane := "" - for i, a := range args { - if a == "--pane" && i+1 < len(args) { - pane = args[i+1] + for i, a := range cmd { + if a == "--pane" && i+1 < len(cmd) { + pane = cmd[i+1] } } - body := f.procInfo[pane] - if strings.HasPrefix(body, "testdata/") { - body = f.fixture(strings.TrimPrefix(body, "testdata/")) - } - return CliResult{Code: 0, Stdout: body} - case len(args) >= 2 && args[0] == "workspace" && args[1] == "close": - f.closed = append(f.closed, args[2]) + return CliResult{Code: 0, Stdout: body(f.procInfo[pane])} + case len(cmd) >= 2 && cmd[0] == "workspace" && cmd[1] == "close": + f.closed = append(f.closed, cmd[2]) return CliResult{Code: 0, Stdout: `{"id":"x","result":{}}`} - case len(args) >= 2 && args[0] == "workspace" && args[1] == "list": + case len(cmd) >= 2 && cmd[0] == "workspace" && cmd[1] == "list": // The pane-list fixtures double as workspace_list answers so // EnsureHerdrServer short-circuits. return CliResult{Code: 0, Stdout: `{"id":"x","result":{"type":"workspace_list","workspaces":[]}}`} default: - return CliResult{Code: 2, Stderr: "fakeCli: unsupported " + strings.Join(args, " ")} + return CliResult{Code: 2, Stderr: "fakeCli: unsupported " + raw} } } @@ -105,11 +135,22 @@ func sweepEnabled() *config.HerdrSweepSettings { func boolPtr(b bool) *bool { return &b } -// orphanSeams installs the DEVAGENT_SWEEP_* test seams. +// orphanSeams installs the DEVAGENT_SWEEP_* test seams. The collector stub is +// keyed by the pattern the sweep asks for (what production passes to +// `pgrep -f`), so a probe retargeted away from paneRunOwnerPattern misses and +// the test fails instead of being handed the old answer. func orphanSeams(t *testing.T, ownerPids string, ancestry map[string][]string) { t.Helper() - t.Setenv("DEVAGENT_SWEEP_OWNER_PIDS", ownerPids) + assertSeamPids(t, "owner pids", ownerPids) + stub, err := json.Marshal(map[string]string{paneRunOwnerPattern: ownerPids}) + if err != nil { + t.Fatal(err) + } + t.Setenv("DEVAGENT_SWEEP_OWNER_PIDS_JSON", string(stub)) if ancestry != nil { + for pid := range ancestry { + assertSeamPids(t, "ancestry pid", pid) + } data, err := json.Marshal(ancestry) if err != nil { t.Fatal(err) @@ -118,6 +159,37 @@ func orphanSeams(t *testing.T, ownerPids string, ancestry map[string][]string) { } } +// assertSeamPids fails a seam that would resolve pids <= 1 the same way the +// real probe drops them (parsePidList): a stub handing out "0" or "bogus" is +// an answer, not a matched owner. +func assertSeamPids(t *testing.T, what, raw string) { + t.Helper() + for _, field := range strings.Fields(raw) { + n, err := strconv.Atoi(field) + if err != nil || n <= 1 { + t.Fatalf("%s = %q: seam pids must be real pids (>1), matching parsePidList", what, raw) + } + } +} + +// captureSeams stubs the per-pane fd table behind paneDispatchEvidence: +// pane id -> the paths lsof would report for the pane's fd 1/2. Keyed by pane +// like the process-info fixtures, so it can never depend on a fixture pid +// disagreeing with the process-info reply for the same pane. +func captureSeams(t *testing.T, fds map[string]string) { + t.Helper() + for paneID := range fds { + if !strings.Contains(paneID, ":") { + t.Fatalf("capture seam key %q is not a pane id", paneID) + } + } + data, err := json.Marshal(fds) + if err != nil { + t.Fatal(err) + } + t.Setenv("DEVAGENT_SWEEP_PANE_CAPTURE_JSON", string(data)) +} + // ---------- Sweep safety (FR-VIS-07) ---------- func TestSweepNeverClosesLiveWorkerEvenWhenIdle(t *testing.T) { @@ -239,6 +311,37 @@ func TestOrphanClosesLiveWorkerWithNoOwnerCLI(t *testing.T) { } } +// Repairing the orphan class also made its probes load-bearing: a pgrep or ps +// that cannot run (absent binary, the 5s cap, a rejected pattern) used to read +// as "no collector" and would close every live worker in the session on a host +// that cannot inspect its own process table. An unanswered probe spares; only an +// answer reaps. +func TestOrphanClassGoesInertWhenProbesCannotAnswer(t *testing.T) { + cli := newFakeCli(t) + cli.paneList = "testdata/sweep/pane-list-live-worker.json" + cli.procInfo["wX:p1"] = "testdata/sweep/process-info-omp.json" + + // No seam entry for the pattern the sweep asks for = no answer at all, + // which must not be confused with the probe answering "no collector". + t.Setenv("DEVAGENT_SWEEP_OWNER_PIDS_JSON", `{"some-other-probe":"4242\n"}`) + if got := FindStalePanes(cli, "devagent", SweepOptions{Orphans: true, Sweep: sweepEnabled()}); len(got) != 0 { + t.Fatalf("stale = %+v, want empty (unanswered owner probe spares)", got) + } + + // Collector found, ancestry walk never completed: still no evidence. + orphanSeams(t, "4242\n", map[string][]string{}) + if got := FindStalePanes(cli, "devagent", SweepOptions{Orphans: true, Sweep: sweepEnabled()}); len(got) != 0 { + t.Fatalf("stale = %+v, want empty (unanswered ancestry spares)", got) + } + + // The same walk completed with no driver above it IS an answer, and it reaps. + orphanSeams(t, "4242\n", map[string][]string{"4242": {}}) + got := FindStalePanes(cli, "devagent", SweepOptions{Orphans: true, Sweep: sweepEnabled()}) + if len(got) != 1 || got[0].Reason != "orphaned-driver" { + t.Fatalf("stale = %+v, want [orphaned-driver] on a completed driverless walk", got) + } +} + func TestSweepOrphansConfigArmsClassWithoutCallerFlag(t *testing.T) { cli := newFakeCli(t) cli.paneList = "testdata/sweep/pane-list-live-worker.json" @@ -254,6 +357,202 @@ func TestSweepOrphansConfigArmsClassWithoutCallerFlag(t *testing.T) { } } +// ---------- Orphaned dispatches outside the worktrees (2026-09-13) ---------- +// +// The loop's research/PO phases dispatch panes in the driver's own checkout +// (FR-VIS-06), where cwd proves nothing about who started the pane — an +// operator's own omp sits in the same directory. Those panes are sweepable only +// with dispatch evidence: the capture contract held on the worker's own +// stdout/stderr, plus a run nobody collects any more. + +// mainCheckoutCli: one devagent-dispatched pane (wM:p1, worker pid 42 with the +// capture contract) beside one hand-run operator pane (wS:p1, worker pid 43 +// without it), both live, both in the main checkout. +func mainCheckoutCli(t *testing.T) *fakeCli { + t.Helper() + cli := newFakeCli(t) + cli.paneList = "testdata/sweep/pane-list-main-checkout.json" + cli.procInfo["wM:p1"] = "testdata/sweep/process-info-omp.json" + cli.procInfo["wS:p1"] = "testdata/sweep/process-info-omp-handrun.json" + return cli +} + +const mainCheckoutCapture = "/private/tmp/devagent-herdr-4242/out /private/tmp/devagent-herdr-4242/err" + +func TestOrphanReapsMainCheckoutDispatchAndSparesHandRun(t *testing.T) { + cli := mainCheckoutCli(t) + orphanSeams(t, "", nil) // no dispatcher alive -> nobody collects the run + captureSeams(t, map[string]string{"wM:p1": mainCheckoutCapture}) + opts := SweepOptions{Orphans: true, Sweep: sweepEnabled()} + got := FindStalePanes(cli, "devagent", opts) + if len(got) != 1 || got[0].PaneID != "wM:p1" || got[0].Reason != "orphaned-driver" { + t.Fatalf("stale = %+v, want [wM:p1 orphaned-driver]", got) + } + SweepStalePanes(cli, "devagent", opts, false) + if len(cli.closed) != 1 || cli.closed[0] != "wM" { + t.Fatalf("closed = %v, want [wM] (the hand-run pane stays open)", cli.closed) + } +} + +func TestOrphanMainCheckoutSparedWhileDispatcherRuns(t *testing.T) { + cli := mainCheckoutCli(t) + // A dispatcher still hanging off the live driver is collecting the run — + // the sweep may not assert orphanhood, however stale the pane looks. + orphanSeams(t, "4242\n", map[string][]string{"4242": {"./devagent-go loop --max-iterations 0"}}) + captureSeams(t, map[string]string{"wM:p1": mainCheckoutCapture}) + if got := FindStalePanes(cli, "devagent", SweepOptions{Orphans: true, Sweep: sweepEnabled()}); len(got) != 0 { + t.Fatalf("stale = %+v, want empty (live dispatcher collects it)", got) + } +} + +func TestOrphanMainCheckoutWithoutDispatchEvidenceIsSpared(t *testing.T) { + cli := mainCheckoutCli(t) + orphanSeams(t, "", nil) + captureSeams(t, map[string]string{}) // neither worker holds the contract + if got := FindStalePanes(cli, "devagent", SweepOptions{Orphans: true, Sweep: sweepEnabled()}); len(got) != 0 { + t.Fatalf("stale = %+v, want empty (no pane proves a devagent dispatch)", got) + } +} + +func TestDefaultSweepNeverListsMainCheckoutPanes(t *testing.T) { + cli := mainCheckoutCli(t) + orphanSeams(t, "", nil) + captureSeams(t, map[string]string{"wM:p1": mainCheckoutCapture}) + if got := FindStalePanes(cli, "devagent", SweepOptions{Sweep: sweepEnabled()}); len(got) != 0 { + t.Fatalf("stale = %+v, want empty (orphan class unarmed)", got) + } +} + +// The kill switch has to cover the widened class: DEVAGENT_HERDR_SWEEP_ORPHANS=0 +// reins in a driver that always passes --orphans, with no code or config edit. +func TestEnvOrphansKillSwitchDisarmsMainCheckoutClass(t *testing.T) { + cli := mainCheckoutCli(t) // before the chdir: fixtures resolve from the package dir + // No devagent.json in this dir: the resolver falls back to defaults with + // the env override on top, which is exactly what the driver sees. + chdir(t, t.TempDir()) + t.Setenv("DEVAGENT_HERDR_SWEEP", "") + t.Setenv("DEVAGENT_HERDR_SWEEP_ORPHANS", "0") + orphanSeams(t, "", nil) + captureSeams(t, map[string]string{"wM:p1": mainCheckoutCapture}) + if got := FindStalePanes(cli, "devagent", SweepOptions{Orphans: true}); len(got) != 0 { + t.Fatalf("stale = %+v, want empty (orphans killed by env)", got) + } +} + +// The dispatch discriminator is a file-descriptor shape, so prove it against a +// real process and the real lsof probe rather than only against +// DEVAGENT_SWEEP_PANE_CAPTURE_JSON: a child with stdout+stderr on +// `/devagent-herdr-/out` is dispatch evidence, and the same child +// writing anywhere else is not. +func TestProcFdsCaptureSeesRealCaptureContract(t *testing.T) { + if _, err := exec.LookPath("lsof"); err != nil { + t.Skip("lsof unavailable") + } + captureDir, err := os.MkdirTemp("", "devagent-herdr-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(captureDir) }) + captureOut, err := os.Create(filepath.Join(captureDir, "out")) + if err != nil { + t.Fatal(err) + } + defer func() { _ = captureOut.Close() }() + child := startWriter(t, captureOut) + + if got := procFdsCapture(child.Process.Pid); !captureFileRe.MatchString(got) { + t.Fatalf("fd probe = %q, want the capture path %s", got, captureDir) + } + + plainDir, err := os.MkdirTemp("", "herdr-scratch-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(plainDir) }) + plainOut, err := os.Create(filepath.Join(plainDir, "out")) + if err != nil { + t.Fatal(err) + } + defer func() { _ = plainOut.Close() }() + plain := startWriter(t, plainOut) + if got := procFdsCapture(plain.Process.Pid); captureFileRe.MatchString(got) { + t.Fatalf("fd probe = %q, want no capture contract", got) + } +} + +// startWriter runs a live child with both stdout and stderr on w (the fd pair +// the sweep probes). +func startWriter(t *testing.T, w *os.File) *exec.Cmd { + t.Helper() + child := exec.Command("/bin/sh", "-c", "sleep 10") + child.Stdout, child.Stderr = w, w + if err := child.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _ = child.Process.Kill() + _ = child.Wait() + }) + return child +} + +// The owner probe is a command-line shape, and the 2026-09-13 failure was a +// shape no real process ever carries: `herdr pane run ` types the script +// into the pane's shell and exits, so the pattern it searched for matched +// nothing and every pane read as orphaned-by-absence. Pin the pattern against +// the argv the dispatchers actually build (internal/loopdriver/dispatch.go: +// `task --prompt …`, `pane-run --cwd … -- …`, both behind the bash +// driver's `timeout N` wrapper), and against the shapes that must NOT match — +// the transient pane-run client and a worker whose quoted prompt names devagent +// commands (the argv0 anchor is what stops it impersonating its own collector). +func TestPaneRunOwnerPatternMatchesRealDispatchShapes(t *testing.T) { + re := regexp.MustCompile(paneRunOwnerPattern) + mustMatch := []string{ + "/usr/local/bin/devagent-go task --prompt Goal: land #1 --repo /repo --worker omp", + "devagent task --prompt hi", + "timeout 5400 /usr/local/bin/devagent-go task --prompt hi --auto-pr", + "/tmp/x/devagent pane-run --cwd /repo --timeout 900 --out /tmp/o --err /tmp/e --done /tmp/d -- omp -p hi", + } + for _, argv := range mustMatch { + if !re.MatchString(argv) { + t.Errorf("owner pattern missed a real dispatch: %q", argv) + } + } + mustNotMatch := []string{ + // the pre-fix target: it never exists while the pane runs. + "herdr --session devagent pane run wX:p1 echo hi", + // a worker quoting devagent commands inside its prompt. + "omp -p run devagent task --prompt x", + // the sweep itself and the driver that runs it must not spare panes. + "/usr/local/bin/devagent-go herdr-sweep --orphans", + "timeout 5400 /usr/local/bin/devagent-go loop --max-iterations 4", + "/usr/local/bin/devagent-review task --prompt x", + } + for _, argv := range mustNotMatch { + if re.MatchString(argv) { + t.Errorf("owner pattern matched a non-collector: %q", argv) + } + } +} + +// The answered/unanswered split lives in the seam-free branch, so the stubs +// cannot prove it: run the real `pgrep` and pin its own exit codes. "No process +// matches" is evidence a collector is absent (the sweep may reap); a probe that +// could not inspect the process table is not (it must spare). +func TestPsPidsMatchingAnswersOnlyWhenPgrepAnswers(t *testing.T) { + if _, err := exec.LookPath("pgrep"); err != nil { + t.Skip("pgrep unavailable") + } + if pids, answered := psPidsMatching(`^zzq-no-such-process-\d{9}$`); !answered || len(pids) != 0 { + t.Fatalf("no-match probe = %v (answered=%v), want an answered empty set", pids, answered) + } + // An unclosed bracket class is a regex the probe cannot run at all: exit 2, + // which must never be reported as "answered, nothing found". + if pids, answered := psPidsMatching(`^[a-`); answered || len(pids) != 0 { + t.Fatalf("failed probe = %v (answered=%v), want no answer", pids, answered) + } +} + // ---------- Deny toggle (FR-VIS-10 / Q23) ---------- func TestEnvSweepToggleStopsSweepBeforeListing(t *testing.T) { @@ -376,16 +675,45 @@ func TestOperatorAttachedEnvSparesWholeSession(t *testing.T) { } } -func TestExemptionOutranksOrphanClass(t *testing.T) { +// FR-VIS-10's exemption is herdr's own attach detection, surfaced through +// PANE_ENV_OP_ATTACH: while an operator is at the wheel nothing in the session +// is sweepable, orphan evidence included. +func TestEnvOperatorAttachOutranksOrphanEvidence(t *testing.T) { cli := newFakeCli(t) cli.paneList = "testdata/sweep/pane-list-idle-worktree.json" cli.agentList = "testdata/sweep/agent-list-running.json" cli.procInfo["wX:p1"] = "testdata/sweep/process-info-omp.json" orphanSeams(t, "", nil) // owner gone -> orphan candidate - got := FindStalePanes(cli, "devagent", SweepOptions{Orphans: true}) + t.Setenv(PaneEnvOpAttach, "1") + got := FindStalePanes(cli, "devagent", SweepOptions{Orphans: true, Sweep: sweepEnabled()}) if len(got) != 1 || got[0].Reason != SweepReasonOperatorAttached { t.Fatalf("stale = %+v, want [operator-attached]", got) } + SweepStalePanes(cli, "devagent", SweepOptions{Orphans: true, Sweep: sweepEnabled()}, false) + if len(cli.closed) != 0 { + t.Fatalf("closed = %v, want none (attached operator never closes)", cli.closed) + } +} + +// Companion: the FR-VIS-02 roster's "running" state does NOT outrank orphan +// evidence. Since #317 it is derived from the same foreground-worker probe, so +// sparing on it first reported every orphan as `operator-attached` and the +// class could never fire (2026-09-13). +func TestOrphanEvidenceOutranksRosterRunningSpare(t *testing.T) { + cli := newFakeCli(t) + cli.paneList = "testdata/sweep/pane-list-idle-worktree.json" + cli.agentList = "testdata/sweep/agent-list-running.json" + cli.procInfo["wX:p1"] = "testdata/sweep/process-info-omp.json" + orphanSeams(t, "", nil) + opts := SweepOptions{Orphans: true, Sweep: sweepEnabled()} + got := FindStalePanes(cli, "devagent", opts) + if len(got) != 1 || got[0].Reason != "orphaned-driver" || got[0].PaneID != "wX:p1" { + t.Fatalf("stale = %+v, want [wX:p1 orphaned-driver]", got) + } + SweepStalePanes(cli, "devagent", opts, false) + if len(cli.closed) != 1 || cli.closed[0] != "wX" { + t.Fatalf("closed = %v, want [wX]", cli.closed) + } } // ---------- SweepStalePanes close behavior + dry-run ---------- diff --git a/internal/herdr/testdata/sweep/pane-list-main-checkout.json b/internal/herdr/testdata/sweep/pane-list-main-checkout.json new file mode 100644 index 00000000..e3b697ae --- /dev/null +++ b/internal/herdr/testdata/sweep/pane-list-main-checkout.json @@ -0,0 +1,22 @@ +{ + "id": "x", + "result": { + "type": "pane_list", + "panes": [ + { + "pane_id": "wM:p1", + "workspace_id": "wM", + "label": "research-a1", + "agent_status": "idle", + "cwd": "/repo" + }, + { + "pane_id": "wS:p1", + "workspace_id": "wS", + "label": "scratch", + "agent_status": "idle", + "cwd": "/repo" + } + ] + } +} diff --git a/internal/herdr/testdata/sweep/process-info-omp-handrun.json b/internal/herdr/testdata/sweep/process-info-omp-handrun.json new file mode 100644 index 00000000..ca479d81 --- /dev/null +++ b/internal/herdr/testdata/sweep/process-info-omp-handrun.json @@ -0,0 +1,11 @@ +{ + "id": "x", + "result": { + "type": "pane_process_info", + "process_info": { + "foreground_processes": [ + { "name": "omp", "argv0": "omp", "pid": 43 } + ] + } + } +}