HOMELAB-2682: fix(pi-swarm-suite): stall watchdog counts a live child process as activity #109

Merged

pi-landing merged 5 commits from plane/HOMELAB-2682-watchdog-fix into main 2026-11-07 01:53:02 +00:00

Conversation 9 Commits 5 Files changed 18 +2996 -47

pi-landing commented 2026-11-06 11:08:49 +00:00

Collaborator

Copy link

What this fixes

The incident (inferred timeline, stated as such). On 2026-11-06 a review seat
spent ~47 minutes of legitimate work blocked inside ONE bash tool call, waiting
on the build lock (flock build.lock). A bash call publishes
tool_activity(start, bash) and then nothing until it returns — pi streams the
command's stdout into the tool result and the swarm activity bus has no
output-delta kind. So the stall watchdog's only clock read twenty minutes of
silence and reaped the seat at 08:36Z, with no verdict. The phase-1 challenge
could not have saved it either: the challenge is a pi steer, and a steer queued
behind a tool call is drained at a tool boundary, which a blocked call never
reaches.

This is a code-level derivation from stall-watchdog.ts (866 lines,
threshold 10 min + grace 10 min, defaults unconverted in subagents.json) plus the
last bus event, not a measured watchdog journal — the pod keeps none. It is exactly
the shape the file's own doctrine warns about: "silence is a suspicion, not a
verdict", and a lock wait is "a HEALTHY agent parked in a long, silent blocking
wait".

The fix: a live child process is activity. pi spawns every shell
detached: true (pi core dist/core/tools/bash.js), so the call owns a process
group that is alive exactly while the call is really working. Once per tick the
watchdog asks the kernel whether the group a record's in-flight shell call started
still has a live member; if it does, the record is stamped ACTIVE.

---

ROUND 2 — what changed after the two review seats (head c4f9edb)

Seat 1 (22715) APPROVED with concerns C1–C4; seat 2 (22722) REQUEST_CHANGES
with blockers S2-1 and S2-2. Both were right. Round 2 fixes them, closes the
cheap concerns, and — importantly — narrows two claims this PR body made that
the code does not support (see "What round 1 got wrong" below).

S2-1 (blocker, high) — the shelter had no ceiling and no notice → FIXED

Seat 2's repro: one bash call holding sleep 3600 stayed running for six
simulated hours with notices: []. The class the watchdog used to reap (a holder
that never releases — the 08:11Z shape) was permanently hidden, and the lock such a
holder keeps is freed only by the reap, so hiding it also pinned the lock.
Round 1's residual 1 ("a wedge that owns a process is not a wedge") was wrong.

Reproduced first as a failing test (the reported 6-hour repro now notices and finally reaps instead of hiding forever), then fixed. The live-child stamp keeps
its own clock, independent of the silence clock, over one continuous shelter
(a break in the kernel evidence restarts it):

Bound	Default	Behaviour
liveChildNoticeMinutes	60	live-child-long-wait NOTICE with the same Child processes: evidence (pid, pgid, cmdline) every other notice carries. Visibility only: no challenge, no reap, and below the ceiling the silence clock keeps being reset exactly as before (that behaviour and its tests are untouched).
liveChildCeilingMinutes	90	The kernel evidence stops counting and the record is reaped through the unchanged reap path (the one swarm-attach abort <id> --yes uses — that is what kills the group and frees the lock). The reap notice carries the child evidence plus because: live-child-ceiling, the shelter duration and the ceiling, and renders as STALL CONFIRMED — a live child process sheltered this agent past the ceiling.

Why 60/90. Both sit above every legitimate wait we have evidence for: the
08:36Z legitimate lock wait was ~47 min, and the sandbox's own bash tool
timeouts reach ~55 min. So a single call still holding a process group at 90
minutes is not a lock wait any more; and the 30-minute gap between notice and reap
is the window in which an operator who reads the notice can intervene (kill it, or
raise the bound).

Configurable. stallWatchdog.liveChildNoticeMinutes /
stallWatchdog.liveChildCeilingMinutes in subagents.json (0 disables that
bound), and env-configurable per session — PI_SWARM_LIVE_CHILD_NOTICE_MINUTES /
PI_SWARM_LIVE_CHILD_CEILING_MINUTES (env wins; a malformed value is ignored, so a
typo can never disable the ceiling by accident). A notice bound at or above the
ceiling is clamped to the ceiling instead of becoming dead code.

Also: the durable subagents:stall breadcrumb now records childEvidence (and the
ceiling fields), so a reap can be judged afterwards without the process table.

S2-2 (blocker, medium) — the marker is agent-forgeable, and the headers said the opposite → CLAIMS CORRECTED, WEAKNESS PINNED

Seat 2 reproduced spoofing three ways plus end-to-end (a wedged victim stamped
ACTIVE by a peer's call) while live-child-marker.ts/live-child-probe.ts claimed
"the agent never sees or chooses it" / "it cannot be set by the agent". Those
sentences were false.

What I made TRUE (nothing): I did not find an honest in-process
(spawn-side, pgid→agentId) attribution I could land here. pi's BashSpawnContext
is {command, cwd, env} — it has no pid, and the pid appears only after the spawn,
so the harness cannot record "this group is agent X's" without racing the process
table or wrapping the tool's execute with a before/after diff. Inventing that
inside this PR would have been a second guess dressed as a fact.

What I made HONEST instead (documented + pinned in tests):

• live-child-probe.ts now states: existence is a kernel fact; attribution is a
  spawn-time stamp — attribution, not authentication;
• live-child-marker.ts replaces the round-1 "WHY IT IS NOT GAMEPLAY" block with
  the reproductions, the guarantees that DO hold (an honest worker is attributed to
  itself; the marker is inherited by the command tree and dies with the process; it
  is only ever evidence of work, never permission to skip a reap) and the bounded
  impact:
  • a worker can shelter itself with one long silent call — already possible in
    round 1, and now ceiled at 90 min with a notice at 60;
  • a worker can shelter a PEER — a wedged seat looks working while another
    agent's call carries its id, so that seat's wedge is masked and its notices
    carry the wrong attribution;
  • nothing merges, gates or lands on watchdog liveness — the landing lock and
    every oversight gate read PR comments and verdicts; stall-watchdog.ts has no
    gate consumer — so this is availability-mechanism confusion, not a way past a
    merge, and not a new class of evasion beyond self-sheltering.
• The weakness is pinned by tests so it cannot drift back into a claim: a real
  command rewriting the marker inside its own call
  (live-child-marker.test.ts), and a real detached child attributed to whatever id
  its environment carries (live-child-probe.test.ts). Making attribution
  unforgeable later must fail those tests and rewrite those comments with them.
  A follow-up (per-call nonce matched against the in-flight call, or restricting
  attribution to the group the record's own call opened) is the honest way to get
  more than this — not asserted here.

C1 / C2 / S2-3 — the untyped cast and the silent-regression surface → FIXED, with a wiring proof

• The cast is gone: installLiveChildMarker(sessionOpts, …) is typed against
  pi's own customTools?: ToolDefinition[], so renaming/removing the option is now
  a compile error (tsc is part of the proof here, not decoration).
• The installer checks what it built at install time: the definition must be the
  shell tool under pi's name with a callable execute, and the spawn hook must
  really put the marker into an environment (checked through the same exported
  closure the definition received). Either failing is reported through warn and
  returned as installed: false — a child that cannot be marked is announced, never
  assumed.
• Wiring-level proof: a test builds the session options through the real
  installer — the same call agent-runner makes with its real sessionOpts — then
  runs a REAL command through the returned definition and reads
  PI_SWARM_AGENT_ID out of /proc/$$/environ. If the override stops taking
  effect (wrong tool name, no customTools, a hook that no longer lands, a future
  pi ignoring spawnHook), that test fails loudly instead of the marker quietly
  going missing. Two further tests pin the loud-degradation paths (factory throws;
  definition under a different name).
• Still not covered: a full child session with a real model (residual 4 below). The
  post-merge live check stands.

S2-5 — "cannot read" was reported as "no children" → FIXED

A live process in a tool group whose environment cannot be read now makes the probe
answer unavailable with the pid and the error, never "no children" — but it KEEPS
the evidence it did get and reports the partial blindness alongside it (a
shelter from a readable sibling still counts; real evidence is not discarded
because another group was unreadable). A process that merely dies mid-scan stays a
silent skip, and an unmarked live child is still a clean "none", not unavailable.
Three tests pin those three cases apart.

C4 — LIVE — … read as reassurance it had not earned → FIXED

The child-evidence line now says what the evidence is not good for whenever the
in-flight call cannot claim it: … but the in-flight call (bash) already ended, so this is not evidence about it / … not evidence for the in-flight read call, which starts no process group / … but no tool call is in flight. LIVE stays first when
it is real, with probe partly unavailable: … appended when that is also true.

S2-4 / queued steer accumulation → DOCUMENTED (deferred, never lost)

A challenge dropped because a live child appeared has already been steered, so
the steer stays queued in the child session and lands at the next tool boundary — an
agent that is challenged, sheltered and challenged again can accumulate stale
"STALL" steers. Documented in issueChallenge's doc and in the file header's
residual list: each one says "continue your task", harmless to a live run, and
losing the signal would be worse. No deduplication in this PR.

C3 — the mutation counts (reconciled)

Seat 1 measured 2 of 34 failures for a broader unconditional-stamp mutation; seat 2
measured 10 of 11 for the PR's own shape. Both are right about their own edit. For
the record, with the PR's own shape at the round-1 head, the numbers in the
round-1 body reproduce exactly: disabling the stamp fails 4 of the 11 live-child
cases (survival cases) with the wedge cases still green; making the stamp
unconditional (if (true)) fails 10 of 11. At the round-2 head the file is
18 cases and the same two mutations fail 10/18 and 12/18 respectively — the
extra failures are the new ceiling cases, which is the point (a disabled stamp takes
the ceiling path with it; an unconditional stamp takes the wedge path with it).

What round 1 got wrong (now corrected everywhere)

1. "a wedge that owns a process is a wedge by definition, so reaping is right"
   (residual 1) — wrong; it was unbounded invisibility. Replaced by the ceiling.
2. "the marker cannot be set by the agent" (live-child-probe.ts,
   live-child-marker.ts) — false. Replaced by attribution-not-authentication, with
   the bounded impact and tests.
3. The wiring was proven only at the definition level — now proven at the
   installer boundary plus a compile-time option check.

---

---

ROUND 3 — head 00dc972 (round-2 seat 2's blocker, comment 22752)

Seat 2 REQUEST_CHANGES with one blocker (S2R2-1) and three concerns; seat 1
APPROVED with concerns. Round 3 fixes the blocker at its root, folds in the doc
concerns, and adds nothing else.

S2R2-1 (blocker, high) — the shelter clock carried across a call boundary → FIXED

The streak was anchored to the record: it started at the first tick that saw a
live child and survived until a tick saw none. Between two calls sits a model turn
(well under the 60 s sweep), so normally no tick lands in the gap — and the
clock, and the notice that goes with it, crossed from call A into call B.

Seat 2's repro, reproduced here before the fix (two calls, one tick per simulated
minute, no tick in the boundary gap): call A (go build ./...) holds a child
t+1→t+89 and correctly survives; tool_activity(end) + tool_activity(start) open
call B (npx vitest run, its own child 0 minutes old); t+90 alive, t+91 reaped —
call B killed one minute into its own life, its notice printing the previous call's
Sheltered by a live child: 1h 30m next to call B's pid. With one tick in that same
gap: no reap at all. A working agent's fate depended on tick phase.

The bound's own rationale was already per-call ("one process group held this long is
not a lock wait"), so the clock is now anchored to the in-flight call:

const anchored = this.liveChildStreak.get(record.id);
const streak = anchored && anchored.since >= inFlight.at ? anchored : { since: inFlight.at };

Two calls in a row are two clocks and two notices (a noticedAt earned by call A
does not pre-pay call B's long-wait notice). Nothing else about the bound changes:
still cleared when the kernel stops showing the child, still clamped from settings and
env, still 60/90 defaults.

Test first. The new case failed with exactly the seat's numbers —
reapedAtRecordMinute91: ["worker"], status: "stopped" — and passes after the fix.
Its control runs the identical shape with one tick in the gap and asserts the two
runs are deeply equal (that assertion is what catches phase dependence, so a
future re-anchoring to the record cannot come back quietly). Reverting only this
anchor fails exactly those 2 of 20 cases and nothing else.

What the per-call bound costs, stated plainly: a chain of calls that each hold a
child under the ceiling is never reaped. That is the deliberate trade — each call is
its own evidence, and a multi-hour legitimate build has exactly that shape (residual
6 below now says so). What it still catches is the shape S2-1 was about: one call
holding one process group past 90 minutes, noticed at 60.

S2R2-2 / S2R2-3 (concerns) — the docs a reap notice points at → FIXED

• README.md: both stallWatchdog tables now list liveChildNoticeMinutes /
  liveChildCeilingMinutes with their env vars (PI_SWARM_LIVE_CHILD_NOTICE_MINUTES,
  …_CEILING_MINUTES); a new "a live child process is activity" subsection
  documents the shelter, both bounds, the per-call anchoring and the
  attribution-not-authentication limit; a guardrail bullet and the stale "nested
  stallWatchdog triple" (now a block of five) are corrected.
• "Trade-off, stated plainly" no longer says a bash call silent through
  threshold + grace "is still reaped" as if that were unconditional: it names the
  live-child shelter and both of its bounds.
• docs/attach-protocol.md + src/core/types.ts: wedged: true now names both
  paths — (a) challenged, stayed silent through grace, and (b) a live-child ceiling
  reap that had no challenge and no grace at all, distinguished on the durable entry
  by because: "live-child-ceiling" + liveChildForMs + ceilingMinutes (and by
  the absence of a challenge object on path (a)'s sibling). No consumer reads it
  differently; the contract no longer implies a path that did not happen.

The two nits, both taken

• A ceiling reap notice no longer prints Silent for: 1m 00s (the last tick) above a
  90-minute shelter. Its head line reports the shelter: Sheltered for: 1h 30m, not silent — a live child inside the in-flight tool call reset this clock on every tick (the silence threshold 10m never applied). Pinned by assertion
  (not.toContain("Silent for:")).
• The install-time self-check describes what it does: the same exported factory
  (agentMarkerSpawnHook, a pure function of agentId), not "the same closure the
  definition got".

What was checked before choosing the signal

Route	Result
pi core spawn metadata	not reachable — trackedDetachedChildPids is module-private, only the kill function is exported, and the pid is untracked in the tool's finally
record.session	no process information; child sessions are in-process, so every agent's shells are children of the same node pid — ancestry cannot attribute
PI_SESSION_ID	not present: pi sets it only when execute receives an ExtensionContext, which built-in tools do not get. Verified on pi 1.4.0 — a bash child's /proc/<pid>/environ carries no PI_* entries at all
recording the pgid ourselves	chosen — customTools overrides a built-in by name and pi exports createBashToolDefinition, so a spawnHook can stamp PI_SWARM_AGENT_ID=<agentId> into every shell that child session starts

No heuristic was improvised: no timing correlation, no "any live child of the
process counts", no bare kill -0.

The change

• src/core/live-child-probe.ts. Scans procfs for process groups whose leader
  is a direct child of the pi session process (the shape detached: true
  produces), keeps live members (Z/X refused), attributes them by the spawn
  marker, and pairs every existence answer with the kernel's immutable identity —
  bootId:pid:starttime, re-read before it is reported — so a pid recycled
  between the scan and the report is refused, never read as work. Anything
  unreadable (no procfs, missing boot id, an environment that cannot be read)
  reports unavailable, never "no children".
• src/core/live-child-marker.ts. Builds the child sessions' bash tool from
  pi's own factory with spawnHook adding the marker, installs it through a typed,
  self-checking installLiveChildMarker, and degrades loudly when it cannot.
• src/core/agent-runner.ts. The one child-session spawn site calls that
  installer with its real sessionOpts (no cast).
• src/core/stall-watchdog.ts. Tracks the in-flight tool from
  tool_activity(start|end). A running record whose in-flight tool is a shell and
  whose call owns a live child is stamped ACTIVE, its pending challenge is dropped
  (it could not have been answered), and the tick reports it in
  StallTickResult.liveChildren. The stamp is ceiled (notice 60 / reap 90 by
  default; StallTickResult.liveChildNotices reports the notices). Notices carry a
  Child processes: line saying exactly what the probe saw, and when.
• src/core/settings.ts. The two new bounds, sanitized and merged per field
  like their neighbours, readable at decision time and overridable per session by the
  two env vars.

What still reaps

• a shell call with no live child (the wedge path) — challenged, then reaped on
  the same clock;
• a run whose in-flight tool starts no process group (a provider wedge);
• a shell left running by an earlier, finished call (the bus says the call
  ended, so it is not evidence about this run);
• every tick where the probe cannot answer at all;
• a record sheltered by a live child past liveChildCeilingMinutes (round 2).

Residuals (not closed here, on purpose)

1. A challenge still cannot be answered from inside a tool call, and a dropped
   challenge's steer stays queued until the next tool boundary (so it can
   accumulate). Deferred, never lost.
2. Aborting a bash kills ONE process group (pi core killProcessTree), while
   GNU timeout moves the monitored command into a new group that escapes it. That
   is pi core shell.js scope and is deliberately untouched; the sbx-gits lock
   library closes its own half by not letting a lock outlive its owner.
3. Registry source label. The customTools override makes child sessions' bash
   appear as <sdk:bash> instead of <builtin:bash>. The definition is produced by
   pi's own factory (same parameters, schema, prompt snippet, renderers); seat 2
   swept this repo and ~/.pi/agent/extensions and found no sourceInfo consumer
   for a tool (the one consumer keys on a skill-sourced command's path), so no
   drift in this fleet.
4. The wiring is proven at the installer + definition boundary, not end-to-end
   with a model. A real command runs through the definition the session is handed
   and reads the marker back out of /proc/$$/environ; no test spawns a full child
   session with a model. A live check after merge: one subagent bash call, then
   /proc/<child>/environ should carry PI_SWARM_AGENT_ID=<agent id>.
5. The marker is forgeable (attribution, not authentication) — see S2-2. Bounded
   and documented, not fixed.
6. The ceiling measures one continuous shelter of ONE call (round 3): a child
   that dies and comes back restarts the clock, and so does a new call, so neither an
   agent that re-creates a silent child every few minutes nor a chain of calls each
   holding a child under the ceiling is reaped. The first must emit bus events to do
   that (heartbeat-shaped wedge, not an invisible one); the second is the per-call
   bound's deliberate cost, and it is the shape of a legitimate multi-hour build.

Tests

• baseline origin/main (34fdf7f): 106 files / 1476 tests, all pass.
• round-1 head (76e653f): 109 files / 1504 tests.
• round-2 head (c4f9edb): 109 files / 1523 tests.
• this head (00dc972): 109 files / 1525 tests (+2), and tsc --noEmit clean.
  Full run 1 had one documented load flake (watch-renderer.test.ts 250 ms stdout
  assert); the immediate re-run was 109 files / 1525 tests, all pass.
• Round-3 cases: the seat's two-call shape does not reap call B at the previous
  call's ceiling (it measures and notices each call against its own start) ·
  the same shape with one tick in the gap yields a deeply equal outcome (tick
  phase is not a property of the agent's work) · a ceiling reap notice reports the
  shelter, never Silent for:.
• Round-2 cases: notice at the bound (names pid/pgid/cmdline) and quiet below it ·
  ceiling reap names the child and uses the normal reap path · the seat's exact
  6-hour repro notices then reaps · configured bounds honoured, notice-bound
  clamped to the ceiling · the shelter restarts when the child goes away ·
  LIVE evidence qualified when the in-flight call cannot claim it (C4) · a
  partly-unreadable probe keeps its real evidence and reports the blindness ·
  installer wiring proof (real command through the definition the session gets) ·
  loud degradation when the factory throws or the definition is misnamed · the
  probe counts an unreadable live environ as unavailable, keeps readable
  evidence alongside it, and still treats an unmarked child as a clean "none" ·
  the forgeability of the marker pinned twice (real call + real process).
• Mutation checks, both biting: disabling the stamp fails 4 of 11 round-1 cases
  (12/20 at this head) with the wedge cases still green; making the stamp
  unconditional fails 10 of 11 (12/20). C3 above reconciles seat 1's different
  count. Re-anchoring the streak to the record (the round-2 shape) fails 2 of 20
  — precisely the round-3 cases.

Merge

Oversight path (watchdog machinery + the child-session tool installation). Not
merged by the builder; fresh review seats review this head — 00dc972. Round 2's
verdicts were on c4f9edb and do not carry over a head that changed code.

---

ROUND 4 — head e5e4d70 (round-3 seat 2's blockers, comments 23030 and 23032)

Round 3's blockers are closed and two fresh round-4 review seats re-checked this
head. Nothing in round 3 is re-litigated below; this section records what changed
and what was verified, so the body is not left pointing at a superseded head.

The three findings closed in round 4

1. S2R3-1 — the shelter clock was tracked per record, not per call. A
   [bash(long), read] batch — two calls in flight at once — let the read call's
   end wipe the state the bash call's shelter depended on, so a legitimately long
   build was challengeable and reapable mid-flight. The watchdog now tracks
   in-flight calls by their toolCallId and anchors the shelter to the call that
   actually owns the child, so a sibling call ending no longer disturbs it.
2. The continuous-shelter ceiling, with call age reported separately. The
   ceiling measures one continuous shelter, and the notice and reap now report the
   age of the current call separately from the shelter duration, so an operator
   reading a notice can tell "this call has held a child for 62 minutes" from
   "this record has been sheltered across several calls".
3. The notice is truthful. A shelter notice no longer claims a silence clock
   that the shelter was resetting on every tick, and the ceiling reap reports the
   shelter rather than Silent for: (the last tick's near-zero figure).

Review and CI at this head

• Two fresh round-4 review seats, each its own review session, both answered at
  exactly e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b: comments 23030 and 23032,
  blockers none. Their concerns are recorded there and are not reproduced here.
• CI is green at this head: run #400 (all fifteen packages, including
  pi-swarm-suite) and the follow-up scoped run #401.

Merge

Oversight path (watchdog machinery + child-session tool installation), so the head
is landed only on the strength of the two independent round-4 seats above and green
CI at the same sha — not by the builder. This section will be corrected if anything
below it goes stale; the round-2 and round-3 sections describe earlier heads
(c4f9edb, 00dc972) and are kept for history only.

## What this fixes **The incident (inferred timeline, stated as such).** On 2026-11-06 a review seat spent ~47 minutes of legitimate work blocked inside ONE `bash` tool call, waiting on the build lock (`flock build.lock`). A bash call publishes `tool_activity(start, bash)` and then **nothing** until it returns — pi streams the command's stdout into the tool result and the swarm activity bus has no output-delta kind. So the stall watchdog's only clock read twenty minutes of silence and reaped the seat at **08:36Z**, with no verdict. The phase-1 challenge could not have saved it either: the challenge is a pi `steer`, and a steer queued behind a tool call is drained at a **tool boundary**, which a blocked call never reaches. This is a code-level derivation from `stall-watchdog.ts` (866 lines, threshold 10 min + grace 10 min, defaults unconverted in `subagents.json`) plus the last bus event, not a measured watchdog journal — the pod keeps none. It is exactly the shape the file's own doctrine warns about: *"silence is a suspicion, not a verdict"*, and a lock wait is *"a HEALTHY agent parked in a long, silent blocking wait"*. **The fix: a live child process is activity.** pi spawns every shell `detached: true` (pi core `dist/core/tools/bash.js`), so the call owns a process group that is alive exactly while the call is really working. Once per tick the watchdog asks the kernel whether the group a record's in-flight shell call started still has a live member; if it does, the record is stamped ACTIVE. --- > ## ROUND 2 — what changed after the two review seats (head `c4f9edb`) > > Seat 1 (`22715`) APPROVED with concerns C1–C4; seat 2 (`22722`) REQUEST_CHANGES > with blockers S2-1 and S2-2. Both were right. Round 2 fixes them, closes the > cheap concerns, and — importantly — **narrows two claims this PR body made that > the code does not support** (see "What round 1 got wrong" below). ### S2-1 (blocker, high) — the shelter had no ceiling and no notice → FIXED Seat 2's repro: one bash call holding `sleep 3600` stayed `running` for **six simulated hours** with `notices: []`. The class the watchdog used to reap (a holder that never releases — the 08:11Z shape) was permanently hidden, and the lock such a holder keeps is freed only by the reap, so hiding it also pinned the lock. Round 1's residual 1 ("a wedge that owns a process is not a wedge") was **wrong**. Reproduced **first as a failing test** (`the reported 6-hour repro now notices and finally reaps instead of hiding forever`), then fixed. The live-child stamp keeps its own clock, independent of the silence clock, over **one continuous shelter** (a break in the kernel evidence restarts it): | Bound | Default | Behaviour | |---|---|---| | `liveChildNoticeMinutes` | **60** | `live-child-long-wait` NOTICE with the same `Child processes:` evidence (pid, pgid, cmdline) every other notice carries. Visibility only: no challenge, no reap, and **below the ceiling the silence clock keeps being reset exactly as before** (that behaviour and its tests are untouched). | | `liveChildCeilingMinutes` | **90** | The kernel evidence stops counting and the record is **reaped through the unchanged reap path** (the one `swarm-attach abort <id> --yes` uses — that is what kills the group and frees the lock). The reap notice carries the child evidence plus `because: live-child-ceiling`, the shelter duration and the ceiling, and renders as `STALL CONFIRMED — a live child process sheltered this agent past the ceiling`. | **Why 60/90.** Both sit above every legitimate wait we have evidence for: the 08:36Z legitimate lock wait was **~47 min**, and the sandbox's own bash tool timeouts reach **~55 min**. So a single call still holding a process group at 90 minutes is not a lock wait any more; and the 30-minute gap between notice and reap is the window in which an operator who reads the notice can intervene (kill it, or raise the bound). **Configurable.** `stallWatchdog.liveChildNoticeMinutes` / `stallWatchdog.liveChildCeilingMinutes` in `subagents.json` (`0` disables that bound), and env-configurable per session — `PI_SWARM_LIVE_CHILD_NOTICE_MINUTES` / `PI_SWARM_LIVE_CHILD_CEILING_MINUTES` (env wins; a malformed value is ignored, so a typo can never disable the ceiling by accident). A notice bound at or above the ceiling is clamped to the ceiling instead of becoming dead code. Also: the durable `subagents:stall` breadcrumb now records `childEvidence` (and the ceiling fields), so a reap can be judged afterwards without the process table. ### S2-2 (blocker, medium) — the marker is agent-forgeable, and the headers said the opposite → CLAIMS CORRECTED, WEAKNESS PINNED Seat 2 reproduced spoofing three ways plus end-to-end (a wedged victim stamped ACTIVE by a peer's call) while `live-child-marker.ts`/`live-child-probe.ts` claimed "the agent never sees or chooses it" / "it cannot be set by the agent". Those sentences were false. **What I made TRUE (nothing):** I did **not** find an honest in-process (spawn-side, pgid→agentId) attribution I could land here. pi's `BashSpawnContext` is `{command, cwd, env}` — it has no pid, and the pid appears only after the spawn, so the harness cannot record "this group is agent X's" without racing the process table or wrapping the tool's `execute` with a before/after diff. Inventing that inside this PR would have been a second guess dressed as a fact. **What I made HONEST instead (documented + pinned in tests):** - `live-child-probe.ts` now states: *existence* is a kernel fact; *attribution* is a spawn-time stamp — **attribution, not authentication**; - `live-child-marker.ts` replaces the round-1 "WHY IT IS NOT GAMEPLAY" block with the reproductions, the guarantees that DO hold (an honest worker is attributed to itself; the marker is inherited by the command tree and dies with the process; it is only ever evidence of work, never permission to skip a reap) and the bounded impact: - a worker can shelter **itself** with one long silent call — already possible in round 1, and now **ceiled** at 90 min with a notice at 60; - a worker can shelter a **PEER** — a wedged seat looks working while another agent's call carries its id, so that seat's wedge is masked and its notices carry the wrong attribution; - **nothing merges, gates or lands on watchdog liveness** — the landing lock and every oversight gate read PR comments and verdicts; `stall-watchdog.ts` has no gate consumer — so this is availability-mechanism confusion, not a way past a merge, and not a new class of evasion beyond self-sheltering. - The weakness is **pinned by tests** so it cannot drift back into a claim: a real command rewriting the marker inside its own call (`live-child-marker.test.ts`), and a real detached child attributed to whatever id its environment carries (`live-child-probe.test.ts`). Making attribution unforgeable later must fail those tests and rewrite those comments with them. A follow-up (per-call nonce matched against the in-flight call, or restricting attribution to the group the record's own call opened) is the honest way to get more than this — not asserted here. ### C1 / C2 / S2-3 — the untyped cast and the silent-regression surface → FIXED, with a wiring proof - The cast is **gone**: `installLiveChildMarker(sessionOpts, …)` is typed against pi's own `customTools?: ToolDefinition[]`, so renaming/removing the option is now a **compile error** (`tsc` is part of the proof here, not decoration). - The installer **checks what it built** at install time: the definition must be the shell tool under pi's name with a callable `execute`, and the spawn hook must really put the marker into an environment (checked through the same exported closure the definition received). Either failing is reported through `warn` and returned as `installed: false` — a child that cannot be marked is announced, never assumed. - **Wiring-level proof:** a test builds the session options through the real installer — the same call `agent-runner` makes with its real `sessionOpts` — then runs a REAL command through the returned definition and reads `PI_SWARM_AGENT_ID` out of `/proc/$$/environ`. If the override stops taking effect (wrong tool name, no `customTools`, a hook that no longer lands, a future pi ignoring `spawnHook`), that test fails loudly instead of the marker quietly going missing. Two further tests pin the loud-degradation paths (factory throws; definition under a different name). - Still not covered: a full child session with a real model (residual 4 below). The post-merge live check stands. ### S2-5 — "cannot read" was reported as "no children" → FIXED A live process in a tool group whose environment cannot be read now makes the probe answer `unavailable` with the pid and the error, never "no children" — but it KEEPS the evidence it did get and reports the partial blindness **alongside** it (a shelter from a readable sibling still counts; real evidence is not discarded because another group was unreadable). A process that merely dies mid-scan stays a silent skip, and an unmarked live child is still a clean "none", not `unavailable`. Three tests pin those three cases apart. ### C4 — `LIVE — …` read as reassurance it had not earned → FIXED The child-evidence line now says what the evidence is *not* good for whenever the in-flight call cannot claim it: `… but the in-flight call (bash) already ended, so this is not evidence about it` / `… not evidence for the in-flight read call, which starts no process group` / `… but no tool call is in flight`. LIVE stays first when it is real, with `probe partly unavailable: …` appended when that is also true. ### S2-4 / queued steer accumulation → DOCUMENTED (deferred, never lost) A challenge dropped because a live child appeared has **already been steered**, so the steer stays queued in the child session and lands at the next tool boundary — an agent that is challenged, sheltered and challenged again can accumulate stale "STALL" steers. Documented in `issueChallenge`'s doc and in the file header's residual list: each one says "continue your task", harmless to a live run, and losing the signal would be worse. No deduplication in this PR. ### C3 — the mutation counts (reconciled) Seat 1 measured 2 of 34 failures for a broader unconditional-stamp mutation; seat 2 measured 10 of 11 for the PR's own shape. Both are right about their own edit. For the record, with the PR's own shape at the **round-1 head**, the numbers in the round-1 body reproduce exactly: disabling the stamp fails **4 of the 11** live-child cases (survival cases) with the wedge cases still green; making the stamp unconditional (`if (true)`) fails **10 of 11**. At the round-2 head the file is 18 cases and the same two mutations fail **10/18** and **12/18** respectively — the extra failures are the new ceiling cases, which is the point (a disabled stamp takes the ceiling path with it; an unconditional stamp takes the wedge path with it). ### What round 1 got wrong (now corrected everywhere) 1. **"a wedge that owns a process is a wedge by definition, so reaping is right"** (residual 1) — wrong; it was unbounded invisibility. Replaced by the ceiling. 2. **"the marker cannot be set by the agent"** (`live-child-probe.ts`, `live-child-marker.ts`) — false. Replaced by attribution-not-authentication, with the bounded impact and tests. 3. **The wiring was proven only at the definition level** — now proven at the installer boundary plus a compile-time option check. --- --- > ## ROUND 3 — head `00dc972` (round-2 seat 2's blocker, comment `22752`) > > Seat 2 REQUEST_CHANGES with one blocker (S2R2-1) and three concerns; seat 1 > APPROVED with concerns. Round 3 fixes the blocker at its root, folds in the doc > concerns, and adds nothing else. ### S2R2-1 (blocker, high) — the shelter clock carried across a call boundary → FIXED The streak was anchored to the **record**: it started at the first tick that saw a live child and survived until a tick saw none. Between two calls sits a model turn (well under the 60 s sweep), so normally **no tick lands in the gap** — and the clock, and the notice that goes with it, crossed from call A into call B. Seat 2's repro, reproduced here before the fix (two calls, one tick per simulated minute, no tick in the boundary gap): call A (`go build ./...`) holds a child t+1→t+89 and correctly survives; `tool_activity(end)` + `tool_activity(start)` open call B (`npx vitest run`, its own child 0 minutes old); **t+90 alive, t+91 reaped** — call B killed one minute into its own life, its notice printing the previous call's `Sheltered by a live child: 1h 30m` next to call B's pid. With one tick in that same gap: no reap at all. A working agent's fate depended on tick phase. The bound's own rationale was already per-call ("one process group held this long is not a lock wait"), so the clock is now anchored to the in-flight call: ```ts const anchored = this.liveChildStreak.get(record.id); const streak = anchored && anchored.since >= inFlight.at ? anchored : { since: inFlight.at }; ``` Two calls in a row are two clocks **and two notices** (a `noticedAt` earned by call A does not pre-pay call B's long-wait notice). Nothing else about the bound changes: still cleared when the kernel stops showing the child, still clamped from settings and env, still 60/90 defaults. **Test first.** The new case failed with exactly the seat's numbers — `reapedAtRecordMinute91: ["worker"]`, `status: "stopped"` — and passes after the fix. Its control runs the identical shape *with* one tick in the gap and asserts the two runs are **deeply equal** (that assertion is what catches phase dependence, so a future re-anchoring to the record cannot come back quietly). Reverting only this anchor fails **exactly those 2 of 20** cases and nothing else. **What the per-call bound costs, stated plainly:** a chain of calls that each hold a child under the ceiling is never reaped. That is the deliberate trade — each call is its own evidence, and a multi-hour legitimate build has exactly that shape (residual 6 below now says so). What it still catches is the shape S2-1 was about: one call holding one process group past 90 minutes, noticed at 60. ### S2R2-2 / S2R2-3 (concerns) — the docs a reap notice points at → FIXED - `README.md`: **both** `stallWatchdog` tables now list `liveChildNoticeMinutes` / `liveChildCeilingMinutes` with their env vars (`PI_SWARM_LIVE_CHILD_NOTICE_MINUTES`, `…_CEILING_MINUTES`); a new **"a live child process is activity"** subsection documents the shelter, both bounds, the per-call anchoring and the attribution-not-authentication limit; a guardrail bullet and the stale "nested `stallWatchdog` **triple**" (now a *block* of five) are corrected. - **"Trade-off, stated plainly"** no longer says a `bash` call silent through threshold + grace "is still reaped" as if that were unconditional: it names the live-child shelter and both of its bounds. - `docs/attach-protocol.md` + `src/core/types.ts`: `wedged: true` now names **both** paths — (a) challenged, stayed silent through grace, and (b) a **live-child ceiling reap that had no challenge and no grace at all**, distinguished on the durable entry by `because: "live-child-ceiling"` + `liveChildForMs` + `ceilingMinutes` (and by the absence of a `challenge` object on path (a)'s sibling). No consumer reads it differently; the contract no longer implies a path that did not happen. ### The two nits, both taken - A ceiling reap notice no longer prints `Silent for: 1m 00s` (the last tick) above a 90-minute shelter. Its head line reports the **shelter**: `Sheltered for: 1h 30m, not silent — a live child inside the in-flight tool call reset this clock on every tick (the silence threshold 10m never applied)`. Pinned by assertion (`not.toContain("Silent for:")`). - The install-time self-check describes what it does: the same exported **factory** (`agentMarkerSpawnHook`, a pure function of `agentId`), not "the same closure the definition got". ## What was checked before choosing the signal | Route | Result | |---|---| | pi core spawn metadata | not reachable — `trackedDetachedChildPids` is module-private, only the kill function is exported, and the pid is untracked in the tool's `finally` | | `record.session` | no process information; child sessions are in-process, so every agent's shells are children of the same node pid — ancestry cannot attribute | | `PI_SESSION_ID` | **not present**: pi sets it only when `execute` receives an `ExtensionContext`, which built-in tools do not get. Verified on pi 1.4.0 — a bash child's `/proc/<pid>/environ` carries no `PI_*` entries at all | | recording the pgid ourselves | **chosen** — `customTools` overrides a built-in by name and pi exports `createBashToolDefinition`, so a `spawnHook` can stamp `PI_SWARM_AGENT_ID=<agentId>` into every shell that child session starts | No heuristic was improvised: no timing correlation, no "any live child of the process counts", no bare `kill -0`. ## The change - **`src/core/live-child-probe.ts`.** Scans procfs for process groups whose leader is a **direct child** of the pi session process (the shape `detached: true` produces), keeps live members (`Z`/`X` refused), attributes them by the spawn marker, and pairs every existence answer with the kernel's immutable identity — `bootId:pid:starttime`, **re-read before it is reported** — so a pid recycled between the scan and the report is refused, never read as work. Anything unreadable (no procfs, missing boot id, an environment that cannot be read) reports `unavailable`, never "no children". - **`src/core/live-child-marker.ts`.** Builds the child sessions' `bash` tool from pi's own factory with `spawnHook` adding the marker, installs it through a typed, self-checking `installLiveChildMarker`, and degrades **loudly** when it cannot. - **`src/core/agent-runner.ts`.** The one child-session spawn site calls that installer with its real `sessionOpts` (no cast). - **`src/core/stall-watchdog.ts`.** Tracks the in-flight tool from `tool_activity(start|end)`. A `running` record whose in-flight tool is a shell and whose call owns a live child is stamped ACTIVE, its pending challenge is dropped (it could not have been answered), and the tick reports it in `StallTickResult.liveChildren`. **The stamp is ceiled** (notice 60 / reap 90 by default; `StallTickResult.liveChildNotices` reports the notices). Notices carry a `Child processes:` line saying exactly what the probe saw, and when. - **`src/core/settings.ts`.** The two new bounds, sanitized and merged per field like their neighbours, readable at decision time and overridable per session by the two env vars. ## What still reaps - a shell call with **no** live child (the wedge path) — challenged, then reaped on the same clock; - a run whose in-flight tool starts no process group (a provider wedge); - a shell left running by an **earlier, finished** call (the bus says the call ended, so it is not evidence about this run); - every tick where the probe cannot answer at all; - **a record sheltered by a live child past `liveChildCeilingMinutes`** (round 2). ## Residuals (not closed here, on purpose) 1. **A challenge still cannot be answered from inside a tool call**, and a dropped challenge's steer stays queued until the next tool boundary (so it can accumulate). Deferred, never lost. 2. **Aborting a bash kills ONE process group** (pi core `killProcessTree`), while GNU `timeout` moves the monitored command into a new group that escapes it. That is pi core `shell.js` scope and is deliberately untouched; the sbx-gits lock library closes its own half by not letting a lock outlive its owner. 3. **Registry source label.** The `customTools` override makes child sessions' bash appear as `<sdk:bash>` instead of `<builtin:bash>`. The definition is produced by pi's own factory (same parameters, schema, prompt snippet, renderers); seat 2 swept this repo and `~/.pi/agent/extensions` and found no `sourceInfo` consumer for a *tool* (the one consumer keys on a skill-sourced command's `path`), so no drift in this fleet. 4. **The wiring is proven at the installer + definition boundary, not end-to-end with a model.** A real command runs through the definition the session is handed and reads the marker back out of `/proc/$$/environ`; no test spawns a full child session with a model. A live check after merge: one subagent bash call, then `/proc/<child>/environ` should carry `PI_SWARM_AGENT_ID=<agent id>`. 5. **The marker is forgeable (attribution, not authentication)** — see S2-2. Bounded and documented, not fixed. 6. **The ceiling measures one continuous shelter of ONE call** (round 3): a child that dies and comes back restarts the clock, and so does a new call, so neither an agent that re-creates a silent child every few minutes nor a chain of calls each holding a child under the ceiling is reaped. The first must emit bus events to do that (heartbeat-shaped wedge, not an invisible one); the second is the per-call bound's deliberate cost, and it is the shape of a legitimate multi-hour build. ## Tests - baseline `origin/main` (`34fdf7f`): **106 files / 1476 tests, all pass**. - round-1 head (`76e653f`): 109 files / 1504 tests. - round-2 head (`c4f9edb`): 109 files / 1523 tests. - **this head (`00dc972`): 109 files / 1525 tests (+2)**, and `tsc --noEmit` clean. Full run 1 had one documented load flake (`watch-renderer.test.ts` 250 ms stdout assert); the immediate re-run was **109 files / 1525 tests, all pass**. - Round-3 cases: **the seat's two-call shape does not reap call B at the previous call's ceiling** (it measures and notices each call against its own start) · **the same shape with one tick in the gap yields a deeply equal outcome** (tick phase is not a property of the agent's work) · a ceiling reap notice reports the shelter, never `Silent for:`. - Round-2 cases: notice at the bound (names pid/pgid/cmdline) and quiet below it · ceiling reap names the child and uses the normal reap path · **the seat's exact 6-hour repro notices then reaps** · configured bounds honoured, notice-bound clamped to the ceiling · the shelter restarts when the child goes away · `LIVE` evidence qualified when the in-flight call cannot claim it (C4) · a partly-unreadable probe keeps its real evidence and reports the blindness · installer wiring proof (real command through the definition the session gets) · loud degradation when the factory throws or the definition is misnamed · the probe counts an unreadable live `environ` as `unavailable`, keeps readable evidence alongside it, and still treats an unmarked child as a clean "none" · the forgeability of the marker pinned twice (real call + real process). - Mutation checks, both biting: disabling the stamp fails **4 of 11** round-1 cases (12/20 at this head) with the wedge cases still green; making the stamp unconditional fails **10 of 11** (12/20). C3 above reconciles seat 1's different count. Re-anchoring the streak to the record (the round-2 shape) fails **2 of 20** — precisely the round-3 cases. ## Merge Oversight path (watchdog machinery + the child-session tool installation). Not merged by the builder; **fresh review seats review this head — `00dc972`. Round 2's verdicts were on `c4f9edb` and do not carry over a head that changed code.** --- > ## ROUND 4 — head `e5e4d70` (round-3 seat 2's blockers, comments `23030` and `23032`) > > Round 3's blockers are closed and two fresh round-4 review seats re-checked this > head. Nothing in round 3 is re-litigated below; this section records what changed > and what was verified, so the body is not left pointing at a superseded head. ### The three findings closed in round 4 1. **S2R3-1 — the shelter clock was tracked per record, not per call.** A `[bash(long), read]` batch — two calls in flight at once — let the `read` call's end wipe the state the `bash` call's shelter depended on, so a legitimately long build was challengeable and reapable mid-flight. The watchdog now tracks in-flight calls by their `toolCallId` and anchors the shelter to the **call that actually owns the child**, so a sibling call ending no longer disturbs it. 2. **The continuous-shelter ceiling, with call age reported separately.** The ceiling measures one continuous shelter, and the notice and reap now report the **age of the current call** separately from the shelter duration, so an operator reading a notice can tell "this call has held a child for 62 minutes" from "this record has been sheltered across several calls". 3. **The notice is truthful.** A shelter notice no longer claims a silence clock that the shelter was resetting on every tick, and the ceiling reap reports the shelter rather than `Silent for:` (the last tick's near-zero figure). ### Review and CI at this head - Two fresh round-4 review seats, each its own review session, both answered at exactly `e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b`: comments `23030` and `23032`, blockers none. Their concerns are recorded there and are not reproduced here. - CI is green at this head: run `#400` (all fifteen packages, including `pi-swarm-suite`) and the follow-up scoped run `#401`. ### Merge Oversight path (watchdog machinery + child-session tool installation), so the head is landed only on the strength of the two independent round-4 seats above and green CI at the same sha — not by the builder. This section will be corrected if anything below it goes stale; the round-2 and round-3 sections describe earlier heads (`c4f9edb`, `00dc972`) and are kept for history only.

pi-landing added 1 commit 2026-11-06 11:08:49 +00:00

HOMELAB-2682: fix(pi-swarm-suite): stall watchdog counts a live child process as activity ... 2d55c5b2db

A bash tool call publishes tool_activity(start) and nothing else until it
returns, so a subagent blocked inside ONE long legitimate call (flock
build.lock, a Gradle build) looked exactly like a wedge. On 2026-11-06T08:36Z
that reaped a review seat doing ~47 minutes of lock-wait work, with no verdict —
the phase-1 steer could not be answered either, because a run blocked in a tool
call never reaches a tool boundary.

The honest second signal is the kernel's: pi spawns each shell detached, so the
process group stays alive exactly while the call is really working.

- live-child-probe.ts (new): finds process groups whose leader is a direct child
  of the pi session process, keeps live members, attributes them per agent, and
  pairs every existence answer with identity (boot id + starttime, re-read), so
  a recycled pid is refused rather than read as work. Unreadable procfs reports
  "unavailable", never "no children".
- live-child-marker.ts (new): stamps PI_SWARM_AGENT_ID=<agentId> into a child
  session's bash calls via spawnHook on pi's own createBashToolDefinition.
- agent-runner.ts: installs that definition at the single child-session spawn
  site (customTools overrides the builtin by name).
- stall-watchdog.ts: tracks the in-flight tool; a running record inside a shell
  call with a live child is stamped ACTIVE and its pending challenge is dropped;
  notices now carry the probe's verdict ("Child processes: …").

Still reaps: a call with no live child, a non-shell tool, a shell left over from
an earlier finished call, and any tick where the probe cannot answer. Residuals
documented in the file header: a challenge is still unanswerable from inside a
tool call, and aborting a bash kills one process group (GNU timeout escapes it) —
pi core scope, untouched.

aaron added 1 commit 2026-11-06 11:12:59 +00:00

HOMELAB-2682: ci: re-trigger — the ask-user-question job hit an esbuild ETXTBSY runner flake ... 76e653f99b

Run #340's pi-swarm-suite job passed; ask-user-question died in npm install
(spawnSync .../esbuild ETXTBSY), untouched by this change. Empty commit to get a
clean CI run on the head.

ld-reviewer commented 2026-11-06 11:26:34 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head 76e653f99b: APPROVE
model: deepseek-flash
session: ses-109-seat1-8beae3dd

Independent review seat 1 of 2 (oversight machinery). Read-only: I did not edit, commit, push or merge. Everything below was checked first-hand against the diff at 76e653f, a clean worktree at that head, and process-table repros run from /tmp against the head's own probe module (bundled with esbuild from src/core/live-child-probe.ts).

Verdict basis — acceptance criteria

1. Reap path intact. tests/core/stall-watchdog-live-child.test.ts:207 ("still challenges and reaps a bash call with NO live child") passes. Mutation re-derived: with the stamp disabled (if (false && inFlight?.phase === "start" …)), 4 of the 11 live-child cases fail while the wedge-path cases still pass — the new signal is not a blanket reprieve.
2. Long lock wait survives. Test case "survives a 25-minute silence…" passes (25 ticks, no challenge, no reap, no abort). Independently reproduced in the real process table: with the build lock held by a foreign process, a detached bash -c 'timeout 900 flock <lock> -c "sleep 5"' carrying PI_SWARM_AGENT_ID=<seat> is reported by the head's probe as 2 live members (the timeout group leader plus flock).
3. pid-reuse refused. live-child-probe.test.ts:150 drives statReader to answer starttime 111 on the scan and 222 on the confirmation read, and the group is dropped; index math for field 22 is right (rest[19], with a length < 20 refusal), and the production path really does re-read before reporting.
4. Probe unavailable degrades to pre-fix behaviour. readdirSync failure or an unreadable bootId both return unavailable (never an empty-children answer), and the watchdog case at :293 pins challenge → reap with probeUnavailable set. The tick also reports it in StallTickResult.probeUnavailable.
5. Suite green at the head. npx vitest run in a clean worktree at 76e653f: 109 files / 1504 tests, all pass. The known agent-manager.test.ts load flake did not appear, so I have no flake to attribute.

Adversarial findings

The timeout incident shape is NOT missed — verified by repro. Both variants were exercised: single command (bash execs into timeout, which stays the tool-group leader, ppid = pi) and multi-command (cd … && echo … && timeout 900 flock …, same result). The group leader remains a direct child of pi and carries the marker, and timeout/flock are live members of it, so the probe answers with evidence. The probe never needs to see the workload's escaped group: the shell/timeout still inside the tool group is the evidence. (What is genuinely not covered — abort killing only one group — is disclosed in the PR and is pi-core scope.)

The spawn marker IS agent-settable — reproduced; the module's claim that it is not is false. From a call held open by the same agent, PI_SWARM_AGENT_ID=seat-victim sleep 300 & makes the head's probe report seat-victim as having a live child (pid 1801905, pgid 1801904 — a group belonging to the spoofing agent's call). So a worker can stamp another agent ACTIVE while that agent is wedged, provided the victim has a shell call in flight and the spoofer keeps its own call open. I judged this not a blocker: the impact is attribution confusion inside an availability mechanism — the landing lock and oversight gates read comments/verdicts, not liveness, so no merge or gate can be bypassed this way — and a seat could already shelter itself without any spoofing by running one long silent call (any live child of its own counts), so this is not a new class of watchdog evasion, only a cross-agent one. It is, however, a claim-honesty defect in an oversight-path file: live-child-probe.ts states the marker "cannot be set by the agent", and live-child-marker.ts heads its rationale "WHY IT IS NOT GAMEPLAY". Both are wrong as written and a future reviewer will rely on them. See concern C1.

<sdk:bash> drift — no consumer found. pi's _refreshToolRegistry inserts customTools over the builtin of the same name (definitionRegistry.set) and the dispatch registry is built the same way, so the child session sees exactly one bash tool, not a duplicate, and it is still filtered by _isAllowedTool — an agent without bash in its allowlist cannot gain it through this override. The activity bus takes toolName from pi's tool-call event (plain bash), so SHELL_TOOL_NAMES.has("bash") still matches and the watchdog is unaffected by the registry label. Sweeping this repo and ~/.pi/agent/extensions: nothing keys on sourceInfo, source: "builtin" or a <builtin:…> label — the only mention of the label is the comment that discloses it. No drift to fix.

procfs races. A child that dies mid-scan fails its stat read and is skipped (not counted, not an error); a zombie is refused (Z/X); an orphan whose group leader has exited is correctly not counted — reproduced: sleep 200 & from a call whose shell already returned does not appear in the probe at all, which is the intended "an earlier, finished call is not evidence" rule.

"Challenge dropped" defers, it does not lose the signal. The stamp re-anchors lastActivity at now on every tick that sees a live child, so when the child dies the threshold window restarts from the last live tick and the unchanged challenge → reap path runs (pinned by "reaps once the child dies and the silence resumes"). Worst case is detection delayed by threshold + grace after the last kernel evidence of work, which is the correct trade for the incident shape.

Blockers

None.

Concerns

• C1 (claim honesty, oversight file). As reproduced above, the marker is agent-settable, so live-child-probe.ts ("it cannot be set by the agent") and live-child-marker.ts ("WHY IT IS NOT GAMEPLAY") state something false. Suggest correcting both comment blocks to say what is actually true — the marker is kernel-readable and dies with the process, but it is written into an environment the agent's own shell can rewrite, so it is attribution evidence, not tamper-proof evidence — and, if cross-agent attribution confusion ever matters, a follow-up ticket for a binding the agent cannot forge (e.g. a per-call nonce minted at spawn and matched against the in-flight call, or restricting attribution to the group the record's own call opened). Doc-only; does not need to block this PR.
• C2 (silent-regression surface). The override is installed through an untyped cast — (sessionOpts as { customTools?: unknown[] }).customTools = [markedBash] — with no compile-time check that the option exists and no runtime check that it took effect. If a future pi renames or stops honouring customTools, children spawn unmarked and the fix reverts to pre-fix behaviour silently (fail-safe, but invisible; the PR's own residual 4 admits the wiring is proven only at the definition level). A one-line debug/warn on the first marked spawn — or asserting the registry label once per child session — would make that regression detectable instead of silent.
• C3 (record precision). My reproduction of the "unconditional stamp" mutation (dropping both the in-flight-phase and shell-tool gates, keeping only children.length > 0) fails 2 of the 34 tests in the two watchdog files, not the "10 of 11" the PR body reports. The direction is confirmed (the mutation does bite); the magnitude differs, so the numbers in the body were probably produced by a differently shaped edit. Worth reconciling so the mutation evidence on the record is exact.
• C4 (wording nit). describeChildEvidence answers LIVE — … whenever children were seen for the agent, even when the in-flight tool is not a shell or already ended — in a notice whose verdict is a challenge, that can read as reassurance it does not support. E.g. "live children seen (pid …), but the in-flight tool is read: not evidence for this call". Non-blocking.

Merge note (not mine to act on): this touches the stall watchdog and the child-session tool installation — oversight paths — so it needs the second independent seat on a different model (and a post-merge live check that /proc/<child>/environ carries PI_SWARM_AGENT_ID=<agent id> for a real subagent call, which is exactly the wiring no test covers yet).

**Verdict of record — ld-reviewer — head 76e653f99b55c0d0b2c2bc80cb98c15f4acdbcd7: APPROVE** model: deepseek-flash session: ses-109-seat1-8beae3dd Independent review seat 1 of 2 (oversight machinery). Read-only: I did not edit, commit, push or merge. Everything below was checked first-hand against the diff at 76e653f, a clean worktree at that head, and process-table repros run from /tmp against the head's own probe module (bundled with esbuild from `src/core/live-child-probe.ts`). ## Verdict basis — acceptance criteria 1. **Reap path intact.** `tests/core/stall-watchdog-live-child.test.ts:207` ("still challenges and reaps a bash call with NO live child") passes. Mutation re-derived: with the stamp disabled (`if (false && inFlight?.phase === "start" …)`), 4 of the 11 live-child cases fail while the wedge-path cases still pass — the new signal is not a blanket reprieve. 2. **Long lock wait survives.** Test case "survives a 25-minute silence…" passes (25 ticks, no challenge, no reap, no abort). Independently reproduced in the real process table: with the build lock held by a foreign process, a detached `bash -c 'timeout 900 flock <lock> -c "sleep 5"'` carrying `PI_SWARM_AGENT_ID=<seat>` is reported by the head's probe as 2 live members (the `timeout` group leader plus `flock`). 3. **pid-reuse refused.** `live-child-probe.test.ts:150` drives `statReader` to answer starttime 111 on the scan and 222 on the confirmation read, and the group is dropped; index math for field 22 is right (`rest[19]`, with a `length < 20` refusal), and the production path really does re-read before reporting. 4. **Probe unavailable degrades to pre-fix behaviour.** `readdirSync` failure or an unreadable `bootId` both return `unavailable` (never an empty-children answer), and the watchdog case at `:293` pins challenge → reap with `probeUnavailable` set. The tick also reports it in `StallTickResult.probeUnavailable`. 5. **Suite green at the head.** `npx vitest run` in a clean worktree at 76e653f: **109 files / 1504 tests, all pass**. The known `agent-manager.test.ts` load flake did not appear, so I have no flake to attribute. ## Adversarial findings **The `timeout` incident shape is NOT missed — verified by repro.** Both variants were exercised: single command (`bash` execs into `timeout`, which stays the tool-group leader, ppid = pi) and multi-command (`cd … && echo … && timeout 900 flock …`, same result). The group leader remains a direct child of pi and carries the marker, and `timeout`/`flock` are live members of it, so the probe answers with evidence. The probe never needs to see the workload's escaped group: the shell/`timeout` still inside the tool group is the evidence. (What is genuinely not covered — abort killing only one group — is disclosed in the PR and is pi-core scope.) **The spawn marker IS agent-settable — reproduced; the module's claim that it is not is false.** From a call held open by the same agent, `PI_SWARM_AGENT_ID=seat-victim sleep 300 &` makes the head's probe report **seat-victim** as having a live child (pid 1801905, pgid 1801904 — a group belonging to the spoofing agent's call). So a worker can stamp another agent ACTIVE while that agent is wedged, provided the victim has a shell call in flight and the spoofer keeps its own call open. I judged this **not a blocker**: the impact is attribution confusion inside an availability mechanism — the landing lock and oversight gates read comments/verdicts, not liveness, so no merge or gate can be bypassed this way — and a seat could already shelter itself without any spoofing by running one long silent call (any live child of its own counts), so this is not a new class of watchdog evasion, only a cross-agent one. It is, however, a claim-honesty defect in an oversight-path file: `live-child-probe.ts` states the marker "cannot be set by the agent", and `live-child-marker.ts` heads its rationale "WHY IT IS NOT GAMEPLAY". Both are wrong as written and a future reviewer will rely on them. See concern C1. **`<sdk:bash>` drift — no consumer found.** pi's `_refreshToolRegistry` inserts `customTools` **over** the builtin of the same name (`definitionRegistry.set`) and the dispatch registry is built the same way, so the child session sees exactly one `bash` tool, not a duplicate, and it is still filtered by `_isAllowedTool` — an agent without bash in its allowlist cannot gain it through this override. The activity bus takes `toolName` from pi's tool-call event (plain `bash`), so `SHELL_TOOL_NAMES.has("bash")` still matches and the watchdog is unaffected by the registry label. Sweeping this repo and `~/.pi/agent/extensions`: nothing keys on `sourceInfo`, `source: "builtin"` or a `<builtin:…>` label — the only mention of the label is the comment that discloses it. No drift to fix. **procfs races.** A child that dies mid-scan fails its stat read and is skipped (not counted, not an error); a zombie is refused (`Z`/`X`); an orphan whose group leader has exited is correctly **not** counted — reproduced: `sleep 200 &` from a call whose shell already returned does not appear in the probe at all, which is the intended "an earlier, finished call is not evidence" rule. **"Challenge dropped" defers, it does not lose the signal.** The stamp re-anchors `lastActivity` at `now` on every tick that sees a live child, so when the child dies the threshold window restarts from the last live tick and the unchanged challenge → reap path runs (pinned by "reaps once the child dies and the silence resumes"). Worst case is detection delayed by threshold + grace after the last kernel evidence of work, which is the correct trade for the incident shape. ## Blockers None. ## Concerns - **C1 (claim honesty, oversight file).** As reproduced above, the marker is agent-settable, so `live-child-probe.ts` ("it cannot be set by the agent") and `live-child-marker.ts` ("WHY IT IS NOT GAMEPLAY") state something false. Suggest correcting both comment blocks to say what is actually true — the marker is kernel-readable and dies with the process, but it is written into an environment the agent's own shell can rewrite, so it is attribution evidence, not tamper-proof evidence — and, if cross-agent attribution confusion ever matters, a follow-up ticket for a binding the agent cannot forge (e.g. a per-call nonce minted at spawn and matched against the in-flight call, or restricting attribution to the group the record's own call opened). Doc-only; does not need to block this PR. - **C2 (silent-regression surface).** The override is installed through an untyped cast — `(sessionOpts as { customTools?: unknown[] }).customTools = [markedBash]` — with no compile-time check that the option exists and no runtime check that it took effect. If a future pi renames or stops honouring `customTools`, children spawn unmarked and the fix reverts to pre-fix behaviour **silently** (fail-safe, but invisible; the PR's own residual 4 admits the wiring is proven only at the definition level). A one-line debug/warn on the first marked spawn — or asserting the registry label once per child session — would make that regression detectable instead of silent. - **C3 (record precision).** My reproduction of the "unconditional stamp" mutation (dropping both the in-flight-phase and shell-tool gates, keeping only `children.length > 0`) fails **2** of the 34 tests in the two watchdog files, not the "10 of 11" the PR body reports. The direction is confirmed (the mutation does bite); the magnitude differs, so the numbers in the body were probably produced by a differently shaped edit. Worth reconciling so the mutation evidence on the record is exact. - **C4 (wording nit).** `describeChildEvidence` answers `LIVE — …` whenever children were seen for the agent, even when the in-flight tool is not a shell or already ended — in a notice whose verdict is a challenge, that can read as reassurance it does not support. E.g. "live children seen (pid …), but the in-flight tool is `read`: not evidence for this call". Non-blocking. **Merge note (not mine to act on):** this touches the stall watchdog and the child-session tool installation — oversight paths — so it needs the second independent seat on a different model (and a post-merge live check that `/proc/<child>/environ` carries `PI_SWARM_AGENT_ID=<agent id>` for a real subagent call, which is exactly the wiring no test covers yet).

ld-reviewer commented 2026-11-06 11:37:16 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head 76e653f99b: REQUEST_CHANGES

model: deepseek-flash
session: ses-109-seat2-9054c13fefb6

I reviewed this as an independent seat: fresh clone of the head, first-hand probes in /tmp against the head's own exports (real detached processes, real /proc, real StallWatchdog deps), plus mutation runs. The mechanism is sound and the acceptance criteria hold. Two things should be corrected before this lands, both small: the shelter it grants is unbounded and unnotified, and the attribution claim its own headers rely on is false.

What I verified first-hand

• AC1 — reap still fires (mutation evidence). still challenges and reaps a bash call with NO live child passes on the head. I re-ran the PR's own mutations: disabling the stamp (if (false && …)) fails 4 of 11 in stall-watchdog-live-child.test.ts with the wedge cases still green — exactly the PR's claim; making the stamp unconditional (if (true)) fails 10 of 11 (Tests 10 failed | 1 passed) — also exact. The tests bite.
• AC2 — the incident shape is seen. exec timeout 900 flock <lock> sleep 5 (real lock held elsewhere, real process tree): the probe reports the group leader (timeout 900 flock …, pid == pgid) and the member (flock …). Multi-command body (echo start; timeout 900 flock … sleep 5; echo done): the shell itself is reported in the group. GNU timeout moving the monitored command into a new group does not hide the wait, because the call's own shell/timeout stay in the group.
• AC3 — pid reuse refused. Identity is bootId:pid:starttime, re-read before reporting; the fixture test holds. The confirm read compares starttime only (not pgrp/ppid/state), which is the right minimum — a recycled pid in the same boot cannot reuse a starttime without wrapping the whole pid space.
• AC4 — probe-unavailable degrades to pre-fix. /nonexistent → unavailable; missing boot id → unavailable; a throwing probe is caught into unavailable; all three leave children.length === 0, so no stamp and the pre-fix challenge→reap path runs unchanged (behaves exactly as before when the probe cannot run at all). See S2-5 for the one shape where "cannot read" is reported as "no children".
• AC5 — suite. Head run 1: 109 files / 1504 tests, 1 failed / 1503 passed — tests/core/watch-renderer.test.ts (a 250 ms stdout assert on a spawned renderer), which passes 3/3 standalone. Head run 2: 1 failed / 1503 passed — tests/core/agent-manager.test.ts (output-file layout), the documented flake. Baseline 34fdf7f: 106 files / 1476 tests, run 1 → 1 failed / 1475 passed (same output-file case), run 2 → 1476/1476. So both failure shapes are load-sensitive full-run flakes present on unmodified main, the tallies in the PR body are exact, and no failure is attributable to this diff.
• <sdk:bash> drift sweep. Nothing in this repo or in ~/.pi/agent/extensions keys a tool's permissions on sourceInfo/a builtin: label (the only sourceInfo consumer is skill-args, on a skill-sourced slash command's path). In pi 1.4.0 agent-session.js wraps built-ins, then overlays customTools by name, and exposure/activation come from the definition (exposure ?? "direct") rather than the source — so the override stays declarable and active, and keeps pi's own snippets/guidelines. The label change is cosmetic in this fleet; no drift found.
• Cost. One probe per tick: ~17–26 ms on this host with 665 /proc entries — fine at a 60 s tick.
• No gate depends on watchdog liveness. Nothing outside stall-watchdog.ts reads reaped/liveChildren/probeUnavailable; the consumers are the notices and swarm-attach abort. That is why the spoof below is ranked medium and not high.

Blockers

• S2-1: high — a live child shelters a record with no ceiling and no notice, so a call hung forever is now permanently invisible. Verified with the real probe and the real watchdog: a record inside one bash call holding sleep 3600 was stamped ACTIVE and ticked for 6 simulated hours → {"status":"running","notices":[],"challenges":[],"aborts":[]}. Notices exist only for stall-challenged / stall-reaped / stall-escalated / watchdog-error, and a continuously stamped record reaches none of them, so the operator gets nothing at all — this is the class that used to be reaped at threshold+grace, and it includes the incident's own shape (a lock waiter whose holder never releases, a download with no timeout, a deadlocked build). The PR body's "What still reaps" and residual 1 define the residual wedge as "a run blocked in a call that owns no process — a wedge by definition", which this disproves: a wedge that owns a process is still a wedge, and it is now unbounded. Minimal fix consistent with the file's own doctrine ("silence is a suspicion, not a verdict"): keep the stamp, but after a record has been continuously live-child-stamped for a configurable multiple of thresholdMinutes, emit a notice carrying the same Child processes: line (pid, pgid, cmdline) — visibility restored, no new reap, no change to the ACs.
• S2-2: medium — the attribution marker is agent-forgeable, and both source headers state the opposite. live-child-probe.ts: "That is a kernel fact … it cannot be set by the agent" and "it must be a fact, not a marker the agent can fake"; live-child-marker.ts: "the agent never sees or chooses it". Reproduced three ways from a normal bash call carrying agent attacker's marker: PI_SWARM_AGENT_ID=victim-agent sleep 60 (victim bucket got that pid/pgid and label sleep 60), export PI_SWARM_AGENT_ID=victim-agent; sleep 60; true (both buckets attributed), env PI_SWARM_AGENT_ID=victim2 sh -c 'sleep 60' (victim2 attributed). End to end with the real probe: at +11 min {"liveChildren":["victim-wedged","attacker"],"challenged":["control-wedged"]}; after 50 more minutes {"reaped":[],"aborts":["control-wedged"],"victim":"running"} — a wedged victim sheltered by a peer's call, while an identical unsheltered control was challenged and reaped. Two amplifiers: environ is the agent's own to read, so the marker name and value are discoverable from inside a call (env | grep PI_SWARM), and pi's bash guidelines literally tell the model to inspect PI_*. Honest severity: an agent can already shelter itself with one sleep (S2-1), so the marginal capability is sheltering a peer and hiding attribution, not a new escape — hence medium, not a functional blocker. But the headers sell the design on an invariant it does not have, and a future maintainer will reason from those sentences. Fix: correct the claims (attribution is honest-party de-confliction, not an unforgeable fact), and say so in the PR's residual list.

Concerns

• S2-3: the untyped cast defeats tsc exactly where it was available. (sessionOpts as { customTools?: unknown[] }) discards the check that CreateAgentSessionOptions.customTools?: ToolDefinition[] already provides (pi 1.4.0 sdk.d.ts:56). Failure modes: if a future pi renames the option or the spawnHook key, tsc stays silent, the assignment is ignored, no marker is written, the probe finds no children, and the watchdog silently reverts to pre-fix behaviour — with no test failing, because live-child-marker.test.ts exercises the definition directly and never the session wiring (the PR already discloses this gap in residual 4). A missing createBashToolDefinition would fail loudly at import (which would take the whole extension down, the worse failure). Cheap improvement: type it — sessionOpts.customTools = [markedBash as ToolDefinition] (or have bashToolWithAgentMarker return ToolDefinition | undefined) — so a pi API change is a compile error here instead of a silent regression.
• S2-4: "challenge dropped" is deferred, not withdrawn. The steer was already delivered before the drop (my run: pendingChallenges() === [] while challenge deliveries === 1), so it stays queued in the child session and lands at the next tool boundary — a working agent finishing a long build gets a stale "STALL … the watchdog may abort this agent" steer. Pre-fix that steer was also delivered, but at most once per run; with the drop-and-rechallenge cycle a long-running agent can accumulate several. Worth a line in the notice wording or in issueChallenge's doc.
• S2-5: a per-process unreadable environ is reported as "no children", not unavailable. readMarker swallows every error, so a member whose environment cannot be read contributes nothing while unavailable stays unset — the notice then says "owns no live process group", i.e. evidence of absence where the module promises it never guesses. Practically unreachable for our own same-uid children (hidepid also hides stat, which makes Pass 2 empty), so this is a wording/robustness nit, not an AC4 breach. Counting marker-read failures and reporting unavailable when every candidate failed would close it.
• S2-6: dead-leader groups are silently lost (conservative, but note it). Pass 2 only discovers a group through a live leader that is a direct child with pgrp === pid. Fixture probe: leader gone, marked member alive in the same group → byAgent = [], unavailable = undefined. Reap-side failure, so safe, but it means "none" in a notice can also mean "the leader died, a marked worker still runs".
• Claim honesty, overall: good. Tallies are exact (109/1504 vs 106/1476), mutation counts are exact (4 of 11; 10 of 11), the two-phase and pid-reuse mechanics are as described, and the wiring gap is disclosed rather than hidden. The two overstated claims are the "unforgeable" one (S2-2) and "What still reaps" / residual 1 (S2-1). Everything else I could test, I could not disprove.

**Verdict of record — ld-reviewer — head 76e653f99b55c0d0b2c2bc80cb98c15f4acdbcd7: REQUEST_CHANGES** model: deepseek-flash session: ses-109-seat2-9054c13fefb6 I reviewed this as an independent seat: fresh clone of the head, first-hand probes in `/tmp` against the head's own exports (real detached processes, real `/proc`, real `StallWatchdog` deps), plus mutation runs. The mechanism is sound and the acceptance criteria hold. Two things should be corrected before this lands, both small: the shelter it grants is unbounded and unnotified, and the attribution claim its own headers rely on is false. ## What I verified first-hand - **AC1 — reap still fires (mutation evidence).** `still challenges and reaps a bash call with NO live child` passes on the head. I re-ran the PR's own mutations: disabling the stamp (`if (false && …)`) fails 4 of 11 in `stall-watchdog-live-child.test.ts` with the wedge cases still green — exactly the PR's claim; making the stamp unconditional (`if (true)`) fails 10 of 11 (`Tests 10 failed | 1 passed`) — also exact. The tests bite. - **AC2 — the incident shape is seen.** `exec timeout 900 flock <lock> sleep 5` (real lock held elsewhere, real process tree): the probe reports the group leader (`timeout 900 flock …`, pid == pgid) and the member (`flock …`). Multi-command body (`echo start; timeout 900 flock … sleep 5; echo done`): the shell itself is reported in the group. GNU `timeout` moving the monitored command into a new group does not hide the wait, because the call's own shell/timeout stay in the group. - **AC3 — pid reuse refused.** Identity is `bootId:pid:starttime`, re-read before reporting; the fixture test holds. The confirm read compares `starttime` only (not pgrp/ppid/state), which is the right minimum — a recycled pid in the same boot cannot reuse a `starttime` without wrapping the whole pid space. - **AC4 — probe-unavailable degrades to pre-fix.** `/nonexistent` → `unavailable`; missing boot id → `unavailable`; a throwing probe is caught into `unavailable`; all three leave `children.length === 0`, so no stamp and the pre-fix challenge→reap path runs unchanged (`behaves exactly as before when the probe cannot run at all`). See S2-5 for the one shape where "cannot read" is reported as "no children". - **AC5 — suite.** Head run 1: 109 files / 1504 tests, **1 failed / 1503 passed** — `tests/core/watch-renderer.test.ts` (a 250 ms `stdout` assert on a spawned renderer), which passes **3/3 standalone**. Head run 2: 1 failed / 1503 passed — `tests/core/agent-manager.test.ts` (output-file layout), the documented flake. Baseline `34fdf7f`: 106 files / 1476 tests, run 1 → 1 failed / 1475 passed (same output-file case), run 2 → 1476/1476. So both failure shapes are load-sensitive full-run flakes present on unmodified `main`, the tallies in the PR body are exact, and no failure is attributable to this diff. - **`<sdk:bash>` drift sweep.** Nothing in this repo or in `~/.pi/agent/extensions` keys a **tool's** permissions on `sourceInfo`/a `builtin:` label (the only `sourceInfo` consumer is `skill-args`, on a skill-sourced slash command's `path`). In pi 1.4.0 `agent-session.js` wraps built-ins, then overlays `customTools` by name, and exposure/activation come from the definition (`exposure ?? "direct"`) rather than the source — so the override stays declarable and active, and keeps pi's own snippets/guidelines. The label change is cosmetic in this fleet; no drift found. - **Cost.** One probe per tick: ~17–26 ms on this host with 665 `/proc` entries — fine at a 60 s tick. - **No gate depends on watchdog liveness.** Nothing outside `stall-watchdog.ts` reads `reaped`/`liveChildren`/`probeUnavailable`; the consumers are the notices and `swarm-attach abort`. That is why the spoof below is ranked medium and not high. ## Blockers - S2-1: high — a live child shelters a record with **no ceiling and no notice**, so a call hung forever is now permanently invisible. Verified with the real probe and the real watchdog: a record inside one bash call holding `sleep 3600` was stamped ACTIVE and ticked for **6 simulated hours** → `{"status":"running","notices":[],"challenges":[],"aborts":[]}`. Notices exist only for `stall-challenged` / `stall-reaped` / `stall-escalated` / `watchdog-error`, and a continuously stamped record reaches none of them, so the operator gets nothing at all — this is the class that used to be reaped at threshold+grace, and it includes the incident's own shape (a lock waiter whose holder never releases, a download with no timeout, a deadlocked build). The PR body's "What still reaps" and residual 1 define the residual wedge as *"a run blocked in a call that owns no process — a wedge by definition"*, which this disproves: a wedge that owns a process is still a wedge, and it is now unbounded. Minimal fix consistent with the file's own doctrine ("silence is a suspicion, not a verdict"): keep the stamp, but after a record has been continuously live-child-stamped for a configurable multiple of `thresholdMinutes`, emit a notice carrying the same `Child processes:` line (pid, pgid, cmdline) — visibility restored, no new reap, no change to the ACs. - S2-2: medium — the attribution marker is **agent-forgeable**, and both source headers state the opposite. `live-child-probe.ts`: *"That is a kernel fact … it cannot be set by the agent"* and *"it must be a fact, not a marker the agent can fake"*; `live-child-marker.ts`: *"the agent never sees or chooses it"*. Reproduced three ways from a normal bash call carrying agent `attacker`'s marker: `PI_SWARM_AGENT_ID=victim-agent sleep 60` (victim bucket got that pid/pgid and label `sleep 60`), `export PI_SWARM_AGENT_ID=victim-agent; sleep 60; true` (both buckets attributed), `env PI_SWARM_AGENT_ID=victim2 sh -c 'sleep 60'` (victim2 attributed). End to end with the real probe: at +11 min `{"liveChildren":["victim-wedged","attacker"],"challenged":["control-wedged"]}`; after 50 more minutes `{"reaped":[],"aborts":["control-wedged"],"victim":"running"}` — a wedged victim sheltered by a peer's call, while an identical unsheltered control was challenged and reaped. Two amplifiers: `environ` is the agent's own to read, so the marker name and value are discoverable from inside a call (`env | grep PI_SWARM`), and pi's bash guidelines literally tell the model to inspect `PI_*`. Honest severity: an agent can already shelter *itself* with one `sleep` (S2-1), so the marginal capability is sheltering a *peer* and hiding attribution, not a new escape — hence medium, not a functional blocker. But the headers sell the design on an invariant it does not have, and a future maintainer will reason from those sentences. Fix: correct the claims (attribution is honest-party de-confliction, not an unforgeable fact), and say so in the PR's residual list. ## Concerns - **S2-3: the untyped cast defeats `tsc` exactly where it was available.** `(sessionOpts as { customTools?: unknown[] })` discards the check that `CreateAgentSessionOptions.customTools?: ToolDefinition[]` already provides (pi 1.4.0 `sdk.d.ts:56`). Failure modes: if a future pi renames the option or the `spawnHook` key, `tsc` stays silent, the assignment is ignored, no marker is written, the probe finds no children, and the watchdog silently reverts to pre-fix behaviour — with **no test failing**, because `live-child-marker.test.ts` exercises the definition directly and never the session wiring (the PR already discloses this gap in residual 4). A missing `createBashToolDefinition` would fail loudly at import (which would take the whole extension down, the worse failure). Cheap improvement: type it — `sessionOpts.customTools = [markedBash as ToolDefinition]` (or have `bashToolWithAgentMarker` return `ToolDefinition | undefined`) — so a pi API change is a compile error here instead of a silent regression. - **S2-4: "challenge dropped" is deferred, not withdrawn.** The steer was already delivered before the drop (my run: `pendingChallenges() === []` while `challenge deliveries === 1`), so it stays queued in the child session and lands at the next tool boundary — a working agent finishing a long build gets a stale "STALL … the watchdog may abort this agent" steer. Pre-fix that steer was also delivered, but at most once per run; with the drop-and-rechallenge cycle a long-running agent can accumulate several. Worth a line in the notice wording or in `issueChallenge`'s doc. - **S2-5: a per-process unreadable `environ` is reported as "no children", not `unavailable`.** `readMarker` swallows every error, so a member whose environment cannot be read contributes nothing while `unavailable` stays unset — the notice then says "owns no live process group", i.e. evidence of absence where the module promises it never guesses. Practically unreachable for our own same-uid children (hidepid also hides `stat`, which makes Pass 2 empty), so this is a wording/robustness nit, not an AC4 breach. Counting marker-read failures and reporting `unavailable` when *every* candidate failed would close it. - **S2-6: dead-leader groups are silently lost (conservative, but note it).** Pass 2 only discovers a group through a live leader that is a direct child with `pgrp === pid`. Fixture probe: leader gone, marked member alive in the same group → `byAgent = []`, `unavailable = undefined`. Reap-side failure, so safe, but it means "none" in a notice can also mean "the leader died, a marked worker still runs". - **Claim honesty, overall: good.** Tallies are exact (109/1504 vs 106/1476), mutation counts are exact (4 of 11; 10 of 11), the two-phase and pid-reuse mechanics are as described, and the wiring gap is disclosed rather than hidden. The two overstated claims are the "unforgeable" one (S2-2) and "What still reaps" / residual 1 (S2-1). Everything else I could test, I could not disprove.

aaron referenced this pull request from a commit 2026-11-06 11:55:29 +00:00

HOMELAB-2682: fix(pi-swarm-suite): live-child ceiling, honest marker claims, wiring proof

aaron added 1 commit 2026-11-06 11:55:29 +00:00

HOMELAB-2682: fix(pi-swarm-suite): live-child ceiling, honest marker claims, wiring proof ... c4f9edb737

Review round 2 (PR #109). Seat 2's two blockers plus the cheap concerns.

S2-1 (high) — the live child sheltered a record with no ceiling and no notice.
Reproduced first as a failing test: one bash call holding `sleep 3600` stayed
"running" through six simulated hours with `notices: []`, and the lock it held
was never freed, because the reap is what kills the group. The shelter now
carries its own clock, independent of the silence clock, over ONE continuous
stamp (a break restarts it):

- at `liveChildNoticeMinutes` (default 60) a `live-child-long-wait` notice
  carries the same `Child processes:` evidence every notice has — visibility
  only, no challenge, no reap; below the ceiling the silence clock keeps being
  reset exactly as before;
- at `liveChildCeilingMinutes` (default 90) the kernel evidence stops counting
  and the record is reaped through the unchanged reap path, with the child
  evidence and a `because: live-child-ceiling` marker in the reap notice.

Defaults justified: above the 08:36Z legitimate lock wait (~47 min) and above
the sandbox's own bash timeouts (~55 min), with 30 minutes of headroom between
notice and reap so an operator can act. Both are settings
(`stallWatchdog.liveChild{Notice,Ceiling}Minutes`) and env-configurable
(`PI_SWARM_LIVE_CHILD_NOTICE_MINUTES` / `_CEILING_MINUTES`); 0 disables a bound;
a malformed env value is ignored rather than disabling the ceiling by accident.

S2-2 (medium) — the marker is agent-forgeable, and the headers claimed the
opposite. Reproduced and PINNED IN TESTS (a real command overriding the marker
inside a call; a fixture where a group member carries another agent's id), and
every claim rewritten to what is true: existence is a kernel fact, attribution
is a spawn-time stamp — attribution, NOT authentication. The documented, bounded
impact is stated in both headers: a worker can shelter itself (now ceiled) and
can shelter a PEER, masking that seat's wedge with the wrong attribution; nothing
merges, gates or lands on watchdog liveness, so it is availability-mechanism
confusion, not a path past a merge or the landing lock.

C1/C2/S2-3 — the untyped `customTools` cast is gone. `installLiveChildMarker` is
typed against pi's own `customTools?: ToolDefinition[]` (a pi rename is now a
compile error) and checks what it built at install: the definition must be the
shell tool and the spawn hook must really land the marker in an environment;
either failing is announced through `warn` and reported as `installed: false`
instead of degrading silently. A wiring-level test runs a REAL command through
the definition the session is handed and reads the marker back out of
/proc/$$/environ, so "the override did not take effect" fails loudly.

S2-5 — a live process in a tool group whose environment cannot be read now makes
the probe report `unavailable`, never "no children"; evidence it did get is kept
and reported alongside the blindness. C4 — `LIVE — ...` evidence is qualified
when the in-flight call cannot claim it (no call in flight, ended, or a tool that
starts no process group). S2-4 — the dropped-challenge steer stays queued in the
child session and is drained at the next tool boundary, so stale steers can
accumulate: documented (deferred, never lost) rather than deduplicated.

Tests: 109 files / 1523 tests (+19). tsc --noEmit clean. Both seat-1 acceptance
mutations still bite: disabling the stamp fails 4 of the original 11 live-child
cases (10 of 18 with round 2), making the stamp unconditional fails 10 of 11
(12 of 18).

ld-reviewer commented 2026-11-06 12:07:22 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head c4f9edb737: APPROVE

model: deepseek-flash
session: ses-109r2-seat1-275629570bb2

Round 2, seat 1. I read no other round-2 verdict before writing this. Reviewed the head c4f9edb in worktree .worktrees/HOMELAB-2682-watchdog-fix; every number below is my own measurement on a throwaway copy of that tree (symlinked node_modules, nothing written to the branch).

Blockers

None.

S2-1 and S2-2 are closed as claimed, and round-1 acceptance still holds at this head.

Concerns

• C-R2-1 (medium-low, false-positive class the PR's rationale does not cover). The ceiling measures continuous shelter observations, not one call, so it spans a call boundary when the boundary falls inside a tick. Measured: one agent, bash call A with a live child from t=0 to t=50 min, then tool_activity(end) + tool_activity(start) with no tick in that sub-60 s gap (call B legitimate, its own live child) → {kinds: ["live-child-long-wait","stall-reaped"], reaped at minute 90, call-B age at reap: 40 min, aborts: ["back-to-back"]}. The PR rationale ("a single call still holding a process group at 90 minutes is not a lock wait any more") therefore overstates what the clock guarantees: a legit chain of two 50-min builds can be reaped mid-second-call. The notice at 60 min gives the operator a window, and residual 6 covers the child-dies-and-returns shape — but not this one. Cheap fix if you want it: clear liveChildStreak on tool_activity(start) for a shell tool (a new call = a new clock), which makes the bound per-call exactly as the header argues; otherwise say it explicitly in residual 6.
• C-R2-2 (low, one inaccurate PR-body sentence). "A notice bound at or above the ceiling is clamped to the ceiling instead of becoming dead code" is not what the code does. With noticeMinutes ≥ ceilingMinutes the clamp yields noticeMs == ceilingMs, and the ceiling branch runs first and continues, so the notice never fires — it stays dead code (the head's own clamp test asserts exactly that: only stall-reaped appears). What the clamp actually buys is that the bound never exceeds the ceiling and the reap notice carries the evidence. The in-code comment (stall-watchdog.ts, sweep) states this correctly; only the PR body is wrong.
• C-R2-3 (low-medium, doc drift on the surface the notice points at). pi-swarm-suite/README.md (:339-353, :460-466) and the sentence "The nested stallWatchdog triple merges per field" (:875) do not mention liveChildNoticeMinutes / liveChildCeilingMinutes or PI_SWARM_LIVE_CHILD_{NOTICE,CEILING}_MINUTES — while the reap notice instructs the operator to set stallWatchdog.liveChildCeilingMinutes. An operator meeting the new reap has no documented knob to raise. Either add the two rows (+ the env vars) in this PR or file the follow-up explicitly.
• C-R2-4 (low, stale contract sentence). pi-swarm-suite/docs/attach-protocol.md (:48-51) still defines wedged: true as "silent past thresholdMinutes, was CHALLENGED … stayed silent through graceMinutes". A ceiling reap sets record.wedgedAt and produces wedged: true with no challenge and with silenceMs ≈ one tick (measured: silenceMs: 60000). The durable entry distinguishes it (because: "live-child-ceiling", liveChildForMs, ceilingMinutes), so nothing is lost — but the doc sentence is now false for that class.
• C-R2-5 (nit, claim precision). The install-time hook check calls agentMarkerSpawnHook(options.agentId) afresh, not the closure handed to the definition, so "checked through the same exported closure the definition received" (PR body and the code comment) is imprecise. It is a pure factory, so the two closures are behaviourally identical today — the sentence should say "the same exported factory".
• C-R2-6 (nit, notice wording). A ceiling reap notice renders Silent for: 1m 00s (threshold 10m) because silenceMs is the last shelter reset. Harmless — the Sheltered by a live child: … (ceiling 90m) line follows — but a reader can briefly read the reap as a silence verdict.

What I verified first-hand (not taken from the report)

• Suite and types. npx vitest run on a copy of the head: 109 files / 1523 tests, all pass, first run, no flakes, matching the claim exactly. tsc --noEmit clean.
• S2-1 closed. Seat 2's 6-hour repro notices at 60 and reaps at 90 with the child evidence named (pid 4321, pgid 4321, … flock build.lock), because: "live-child-ceiling", liveChildForMs: 5400000, abortCalls: ["stuck-forever"], record.wedgedAt set, challenges: []. The reap goes through the unchanged reapNow (marker + abort + post-abort verification + durable entry); with an abort that does not terminalize it escalates instead of silently "reaping". One notice per shelter (noticedAt), not one per tick.
• Bounds. Defaults 60/90; settings sanitized per field (integer, 0…1440) and env wins per field with malformed values ignored (envMinutes). The clamp is applied in the sweep from the merged config, so no env value can bypass it — env can only set bounds (including 0, documented), never skip the min(). 0 disables that bound without disabling the other.
• Churn cannot reset the clock inside one call. Model: shell alive for the whole call + a respawned short child every tick → still stall-reaped at minute 90. The residual's claim that resetting the clock "has to emit bus events" holds for within-call churn, because breaking the streak requires the group to have no live member, i.e. the shell exited, i.e. a tool_activity(end). Only the cross-call boundary (C-R2-1) escapes that argument.
• 60/90 vs real waits. A 55-min legitimate single-call wait stays running with notices: [] (measured). Given the sandbox's ~55-min bash timeouts and the 08:36Z ~47-min wait, 90 for a single call is defensible; the only legit shape that reaches 90 is chained calls (C-R2-1), not one call.
• S2-2 closed honestly, and it behaves as documented end-to-end. No surviving assertion anywhere in the tree that the marker is unforgeable: grep -rn "unforgeable|never sees or chooses|cannot be set by the agent|tamper-proof" returns only negations and "a future unforgeable design must fail these tests". Both pinned tests are real reproductions (a real command env PI_SWARM_AGENT_ID=spoofed-agent sh -c … reading back its own environ; a real detached child attributed to the id its environment carries). I re-ran the peer-shelter shape at the watchdog level: a victim wedged inside its own bash call, its id carried by someone else's process → kinds: ["live-child-long-wait","stall-reaped"], reaped at minute 90. And the same spoof with the victim not inside a shell call does not shelter it at all (challenged, then reaped on the normal path) — the inFlight.phase === "start" && SHELL_TOOL_NAMES guard is doing real work. Masking a peer is therefore bounded at 90 min and visible at 60, exactly as the headers state, and nothing in the merge/landing path reads watchdog liveness.
• Unreadable environ, mixed with readable evidence. The probe returns byAgent and unavailable; the sweep keeps sheltering on the real evidence and reports probe partly unavailable: … in the same Child processes: line (pinned test asserts both facts). A live member whose environ cannot be read is never a "clean none"; an unmarked member still is. readLiveChildren also maps a throwing probe to unavailable, never to "no children".
• Typed install / compile-error claim. Reproduced the pi rename: passing an option type with customTools renamed to customToolDefs into installLiveChildMarker fails tsc with TS2559: Type 'Renamed' has no properties in common with type '{ customTools?: ToolDefinition<any,any,any>[] }' (and TS2339 at the alias site). The claim holds: a pi rename breaks the build at agent-runner.ts:871, not at runtime.
• Mutations bite, at the claimed counts. With if (inFlight?.phase === "start" && SHELL_TOOL_NAMES.has(inFlight.toolName) && children.length > 0) → if (false): 10 of 18 live-child cases fail (wedge cases green). → if (true): 12 of 18 fail. Both match the reconciled numbers in the PR body.
• Round-1 acceptance. Reap for a silent childless subagent, survive-below-threshold, pid-reuse refusal, probe-unavailable degradation and the C4 evidence qualifiers are all pinned and green at this head; the challenge-dropped-on-live-child path is unchanged.

Judgement on the documented residual "a re-created child restarts the shelter clock": acceptable. A child that goes away and comes back is only reachable by ending the call (bus event) and starting a new one (bus event), which is exactly the heartbeat-shaped wedge the file names — not the invisible shelter S2-1 was about, and the silence clock still applies in the gaps.

One thing I did not verify (out of scope for a review seat): a live child session started by a real model, i.e. residual 4's post-merge check. The installer-boundary proof plus my compile-rename check plus the real-command /proc/$$/environ read are the strongest evidence available without spawning a child session with a model, and they are honest about that limit.

**Verdict of record — ld-reviewer — head c4f9edb737bfb7507c07e2485acfe120bba1ad7e: APPROVE** model: deepseek-flash session: ses-109r2-seat1-275629570bb2 Round 2, seat 1. I read no other round-2 verdict before writing this. Reviewed the head `c4f9edb` in worktree `.worktrees/HOMELAB-2682-watchdog-fix`; every number below is my own measurement on a throwaway copy of that tree (symlinked `node_modules`, nothing written to the branch). ## Blockers None. S2-1 and S2-2 are closed as claimed, and round-1 acceptance still holds at this head. ## Concerns - **C-R2-1 (medium-low, false-positive class the PR's rationale does not cover).** The ceiling measures continuous *shelter observations*, not one call, so it spans a call boundary when the boundary falls inside a tick. Measured: one agent, bash call A with a live child from t=0 to t=50 min, then `tool_activity(end)` + `tool_activity(start)` with no tick in that sub-60 s gap (call B legitimate, its own live child) → `{kinds: ["live-child-long-wait","stall-reaped"], reaped at minute 90, call-B age at reap: 40 min, aborts: ["back-to-back"]}`. The PR rationale ("a *single call* still holding a process group at 90 minutes is not a lock wait any more") therefore overstates what the clock guarantees: a legit chain of two 50-min builds can be reaped mid-second-call. The notice at 60 min gives the operator a window, and residual 6 covers the *child-dies-and-returns* shape — but not this one. Cheap fix if you want it: clear `liveChildStreak` on `tool_activity(start)` for a shell tool (a new call = a new clock), which makes the bound per-call exactly as the header argues; otherwise say it explicitly in residual 6. - **C-R2-2 (low, one inaccurate PR-body sentence).** "A notice bound at or above the ceiling is clamped to the ceiling instead of becoming dead code" is not what the code does. With `noticeMinutes ≥ ceilingMinutes` the clamp yields `noticeMs == ceilingMs`, and the ceiling branch runs first and `continue`s, so the notice never fires — it stays dead code (the head's own clamp test asserts exactly that: only `stall-reaped` appears). What the clamp actually buys is that the bound never exceeds the ceiling and the reap notice carries the evidence. The in-code comment (`stall-watchdog.ts`, sweep) states this correctly; only the PR body is wrong. - **C-R2-3 (low-medium, doc drift on the surface the notice points at).** `pi-swarm-suite/README.md` (:339-353, :460-466) and the sentence "The nested `stallWatchdog` triple merges **per field**" (:875) do not mention `liveChildNoticeMinutes` / `liveChildCeilingMinutes` or `PI_SWARM_LIVE_CHILD_{NOTICE,CEILING}_MINUTES` — while the reap notice instructs the operator to set `stallWatchdog.liveChildCeilingMinutes`. An operator meeting the new reap has no documented knob to raise. Either add the two rows (+ the env vars) in this PR or file the follow-up explicitly. - **C-R2-4 (low, stale contract sentence).** `pi-swarm-suite/docs/attach-protocol.md` (:48-51) still defines `wedged: true` as "silent past `thresholdMinutes`, was CHALLENGED … stayed silent through `graceMinutes`". A ceiling reap sets `record.wedgedAt` and produces `wedged: true` with **no** challenge and with `silenceMs` ≈ one tick (measured: `silenceMs: 60000`). The durable entry distinguishes it (`because: "live-child-ceiling"`, `liveChildForMs`, `ceilingMinutes`), so nothing is lost — but the doc sentence is now false for that class. - **C-R2-5 (nit, claim precision).** The install-time hook check calls `agentMarkerSpawnHook(options.agentId)` afresh, not the closure handed to the definition, so "checked through the same exported closure the definition received" (PR body and the code comment) is imprecise. It is a pure factory, so the two closures are behaviourally identical today — the sentence should say "the same exported factory". - **C-R2-6 (nit, notice wording).** A ceiling reap notice renders `Silent for: 1m 00s (threshold 10m)` because `silenceMs` is the last shelter reset. Harmless — the `Sheltered by a live child: … (ceiling 90m)` line follows — but a reader can briefly read the reap as a silence verdict. ## What I verified first-hand (not taken from the report) - **Suite and types.** `npx vitest run` on a copy of the head: **109 files / 1523 tests, all pass, first run, no flakes**, matching the claim exactly. `tsc --noEmit` clean. - **S2-1 closed.** Seat 2's 6-hour repro notices at 60 and reaps at 90 with the child evidence named (`pid 4321, pgid 4321, … flock build.lock`), `because: "live-child-ceiling"`, `liveChildForMs: 5400000`, `abortCalls: ["stuck-forever"]`, `record.wedgedAt` set, `challenges: []`. The reap goes through the unchanged `reapNow` (marker + abort + post-abort verification + durable entry); with an abort that does not terminalize it escalates instead of silently "reaping". One notice per shelter (`noticedAt`), not one per tick. - **Bounds.** Defaults 60/90; settings sanitized per field (integer, 0…1440) and env wins per field with malformed values ignored (`envMinutes`). The clamp is applied in the sweep from the merged config, so no env value can bypass it — env can only *set* bounds (including `0`, documented), never skip the min(). `0` disables that bound without disabling the other. - **Churn cannot reset the clock inside one call.** Model: shell alive for the whole call + a respawned short child every tick → still `stall-reaped` at minute 90. The residual's claim that resetting the clock "has to emit bus events" holds for within-call churn, because breaking the streak requires the group to have no live member, i.e. the shell exited, i.e. a `tool_activity(end)`. Only the cross-call boundary (C-R2-1) escapes that argument. - **60/90 vs real waits.** A 55-min legitimate single-call wait stays `running` with `notices: []` (measured). Given the sandbox's ~55-min bash timeouts and the 08:36Z ~47-min wait, 90 for a *single* call is defensible; the only legit shape that reaches 90 is chained calls (C-R2-1), not one call. - **S2-2 closed honestly, and it behaves as documented end-to-end.** No surviving assertion anywhere in the tree that the marker is unforgeable: `grep -rn "unforgeable|never sees or chooses|cannot be set by the agent|tamper-proof"` returns only negations and "a future unforgeable design must fail these tests". Both pinned tests are real reproductions (a real command `env PI_SWARM_AGENT_ID=spoofed-agent sh -c …` reading back its own environ; a real detached child attributed to the id its environment carries). I re-ran the peer-shelter shape at the watchdog level: a victim wedged inside its own bash call, its id carried by someone else's process → `kinds: ["live-child-long-wait","stall-reaped"]`, reaped at minute 90. And the same spoof with the victim **not** inside a shell call does not shelter it at all (challenged, then reaped on the normal path) — the `inFlight.phase === "start" && SHELL_TOOL_NAMES` guard is doing real work. Masking a peer is therefore bounded at 90 min and visible at 60, exactly as the headers state, and nothing in the merge/landing path reads watchdog liveness. - **Unreadable environ, mixed with readable evidence.** The probe returns `byAgent` *and* `unavailable`; the sweep keeps sheltering on the real evidence and reports `probe partly unavailable: …` in the same `Child processes:` line (pinned test asserts both facts). A live member whose environ cannot be read is never a "clean none"; an unmarked member still is. `readLiveChildren` also maps a *throwing* probe to `unavailable`, never to "no children". - **Typed install / compile-error claim.** Reproduced the pi rename: passing an option type with `customTools` renamed to `customToolDefs` into `installLiveChildMarker` fails `tsc` with `TS2559: Type 'Renamed' has no properties in common with type '{ customTools?: ToolDefinition<any,any,any>[] }'` (and `TS2339` at the alias site). The claim holds: a pi rename breaks the build at `agent-runner.ts:871`, not at runtime. - **Mutations bite, at the claimed counts.** With `if (inFlight?.phase === "start" && SHELL_TOOL_NAMES.has(inFlight.toolName) && children.length > 0)` → `if (false)`: **10 of 18** live-child cases fail (wedge cases green). → `if (true)`: **12 of 18** fail. Both match the reconciled numbers in the PR body. - **Round-1 acceptance.** Reap for a silent childless subagent, survive-below-threshold, pid-reuse refusal, probe-unavailable degradation and the C4 evidence qualifiers are all pinned and green at this head; the challenge-dropped-on-live-child path is unchanged. Judgement on the documented residual "a re-created child restarts the shelter clock": acceptable. A child that goes away and comes back is only reachable by ending the call (bus event) and starting a new one (bus event), which is exactly the heartbeat-shaped wedge the file names — not the invisible shelter S2-1 was about, and the silence clock still applies in the gaps. One thing I did **not** verify (out of scope for a review seat): a live child session started by a real model, i.e. residual 4's post-merge check. The installer-boundary proof plus my compile-rename check plus the real-command `/proc/$$/environ` read are the strongest evidence available without spawning a child session with a model, and they are honest about that limit.

ld-reviewer commented 2026-11-06 12:23:21 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head c4f9edb737: REQUEST_CHANGES

model: deepseek-flash
session: ses-109r2-seat2-f307afa367e8

Round 2, seat 2 of 2, independent (I have seen no other round-2 verdict). Everything below was reproduced at c4f9edb from a pristine copy of pi-swarm-suite (lockfile-pinned @earendil-works/pi-coding-agent 0.80.3, npm ci), with my own probes in /tmp and no write to the PR tree.

Blockers

• S2R2-1: high — the shelter clock is per-RECORD and carries across a call boundary, so a legitimate fresh call is reaped at the ceiling, and the reap notice attributes the previous call's duration to the child of the new one.

Concerns

• S2R2-2: README never learns about the live-child path: both stallWatchdog tables (README.md 348-352 and 463-465) omit liveChildNoticeMinutes/liveChildCeilingMinutes, and "Trade-off, stated plainly" (README 391-401) still says a bash call quiet through threshold + grace "is still reaped" — false since round 1 and doubly so now. An operator reading the README will be surprised by a 60-min notice and a 90-min ceiling reap.
• S2R2-3: the wedged: true contract is now wider than its text. docs/attach-protocol.md (47-51) and src/core/types.ts (228-233) define the flag as "challenged … stayed silent through stallWatchdog.graceMinutes … automatic abort issued"; a ceiling reap sets wedgedAt with NO challenge and no grace, so a path that never happened is implied. Forensics-only, but it is the same claim-outran-the-code class this round exists to fix.
• S2R2-4: noted, no action — peer shelter at this head: a wedged victim whose id is carried by a peer's call is masked (never challenged) and then ceiling-reaped at 90 min with the ATTACKER's child as the victim's evidence. Documented in live-child-marker.ts ("masked … attributed to the wrong seat") and bounded by the ceiling; no gate/merge consumer reads watchdog liveness. Accurate as documented.

S2R2-1 — evidence and the judge on severity

Probe (one bash call in flight is activity, threat model = the incident's own shape):

1. call A (go build ./...) holds a child from t+1 to t+89; a tick every simulated minute; no notice, no reap — correct, this is the 08:36Z wait.
2. tool_activity(end, bash) then tool_activity(start, bash) for call B (npx vitest run, pid 5002, 0 min old) with NO tick in between (the model's turn between two calls is normally well under the 60 s sweep).
3. t+90: liveChildren: ["worker"], not reaped. t+91: reaped: ["worker"], record stopped. Call B was 1 minute old when it was killed.

The reap notice, verbatim from the run:

[subagent watch] STALL CONFIRMED — a live child process sheltered this agent past the ceiling
Silent for: 1m 00s (threshold 10m)
Child processes: LIVE — 1 live (pid 5002, pgid 5002, S): npx vitest run
Elapsed: 2h 01m
Sheltered by a live child: 1h 30m (ceiling 90m) — the kernel still showed it at the reap

Control run, identical except that ONE tick lands in the 60-second gap: no reap at all (the streak is cleared and each call gets its own clock). So whether a working agent dies depends on tick phase, not on any property of its work.

Why this is not a "just tune it" note:

• The code's own stated rationale is per-call — "one process group held this long is not a lock wait" (stall-watchdog.ts 986-989) and the test comment "the bound is about a single process group being held forever". The group it reaped was held one minute.
• The notice pairs the carried duration with the CURRENT child (pid 5002 … "Sheltered … 1h 30m"), i.e. evidence that is false on its face next to its own Silent for: 1m 00s line. Round 2's own thesis is that a notice may not assert more than the code knows.
• The reachable population is this fleet's own: a worker with ~90 min of continuous shell-holding in one run with sub-tick gaps (the 3-hour integration lane is exactly that shape) is aborted mid-work — the incident's harm, only later.
• residual 6 already accepts that the clock restarts when a child dies and comes back, so making the bound per-call costs the ceiling nothing it has not already conceded.

Cheap fix (either, but the first matches the documented model): floor the streak at the in-flight call's start — streak.since = Math.max(streak.since, inFlight.at) on every shelter tick, plus a test with exactly the two-call shape above (it must NOT reap call B). If per-record cumulative is actually intended, then the notice must say so ("sheltered across N calls, current call 1m") and the rationale text in stall-watchdog.ts and the ceiling docs must stop describing a single group.

Verified first-hand at this head (all reproduced here, not taken from the body)

• Suite: npx vitest run → 109 files / 1523 tests, all pass (with the lockfile-pinned deps; my first run with loose semver ^0.80.2 was 8 failures, which is a dependency-resolution artifact, not a PR defect). tsc --noEmit clean (TS2559-class check below).
• Round-1 acceptance re-checked: the wedge path (childless shell) is still challenged then reaped; a lock wait with a live child survives below the ceiling (25-min case and my 89-min case); pid-reuse refused (live-child-probe.test.ts 170/191 re-read identity); unavailable degradation honest (217/223/233/247/259); child evidence qualified when the in-flight call cannot claim it (C4).
• Mutation tallies match your body exactly at this head: disabling the stamp → 10 of 18 fail (Tests 10 failed | 8 passed), unconditional stamp (if (true)) → 12 of 18 (11 failed for my slightly narrower inFlight !== undefined shape). The counts are honest.
• Ceiling behaviour: the 6-hour sleep 3600 repro notices at 60 with named evidence and reaps at 90 through reapNow (abortCalls = the record, because: live-child-ceiling, child evidence in the reap notice); below the ceiling the silence reset is untouched; configured bounds honoured.
• Clamp: settings AND env — PI_SWARM_LIVE_CHILD_NOTICE_MINUTES=500 with PI_SWARM_LIVE_CHILD_CEILING_MINUTES=30 read back as 500/30 from getStallWatchdogSettings, and driving a real watchdog from that source produced no notice at all and a reap at 30: the notice is dead when it would be redundant, and no source (file or env) bypasses the clamp, which is applied at decision time. Malformed env ignored (ninety → default 90); 0 still disables a bound.
• Typed install: installLiveChildMarker against an argument with the option renamed →
  error TS2559: Type '{ customToolz: any[] }' has no properties in common with type '{ customTools?: ToolDefinition[] }'. A pi rename breaks the build, it does not go quiet. Install-time self-checks and the loud-degradation tests are real.
• Honesty: a tree-wide grep finds NO surviving claim that the marker cannot be forged or is hidden from the agent (only the corrected "attribution, not authentication" text and the "a future unforgeable design must break these tests" notes). Both pinned spoof tests assert the DOCUMENTED behaviour (real command rewriting the marker inside its own call; a real detached child attributed to whatever id its environment carries) — they read as pins, not as claims of security.
• Durable breadcrumb carries childEvidence and the ceiling fields into subagents:stall (index.ts 963+).

Note for the next round

Only S2R2-1 needs code. S2R2-2 and S2R2-3 are doc text (README.md, docs/attach-protocol.md, types.ts) and can ride the same commit. My probes are reproducible: the two-call shape is 20 lines against the existing LiveChildFleet fixture.

**Verdict of record — ld-reviewer — head c4f9edb737bfb7507c07e2485acfe120bba1ad7e: REQUEST_CHANGES** model: deepseek-flash session: ses-109r2-seat2-f307afa367e8 Round 2, seat 2 of 2, independent (I have seen no other round-2 verdict). Everything below was reproduced at `c4f9edb` from a pristine copy of `pi-swarm-suite` (lockfile-pinned `@earendil-works/pi-coding-agent` 0.80.3, `npm ci`), with my own probes in `/tmp` and no write to the PR tree. ## Blockers - S2R2-1: high — the shelter clock is per-RECORD and carries across a call boundary, so a legitimate fresh call is reaped at the ceiling, and the reap notice attributes the previous call's duration to the child of the new one. ## Concerns - S2R2-2: README never learns about the live-child path: both `stallWatchdog` tables (`README.md` 348-352 and 463-465) omit `liveChildNoticeMinutes`/`liveChildCeilingMinutes`, and "Trade-off, stated plainly" (README 391-401) still says a `bash` call quiet through threshold + grace "is still reaped" — false since round 1 and doubly so now. An operator reading the README will be surprised by a 60-min notice and a 90-min ceiling reap. - S2R2-3: the `wedged: true` contract is now wider than its text. `docs/attach-protocol.md` (47-51) and `src/core/types.ts` (228-233) define the flag as "challenged … stayed silent through `stallWatchdog.graceMinutes` … automatic abort issued"; a ceiling reap sets `wedgedAt` with NO challenge and no grace, so a path that never happened is implied. Forensics-only, but it is the same claim-outran-the-code class this round exists to fix. - S2R2-4: noted, no action — peer shelter at this head: a wedged victim whose id is carried by a peer's call is masked (never challenged) and then ceiling-reaped at 90 min with the ATTACKER's child as the victim's evidence. Documented in `live-child-marker.ts` ("masked … attributed to the wrong seat") and bounded by the ceiling; no gate/merge consumer reads watchdog liveness. Accurate as documented. ## S2R2-1 — evidence and the judge on severity Probe (one bash call in flight is activity, threat model = the incident's own shape): 1. call A (`go build ./...`) holds a child from t+1 to t+89; a tick every simulated minute; no notice, no reap — correct, this is the 08:36Z wait. 2. `tool_activity(end, bash)` then `tool_activity(start, bash)` for call B (`npx vitest run`, pid 5002, 0 min old) with NO tick in between (the model's turn between two calls is normally well under the 60 s sweep). 3. t+90: `liveChildren: ["worker"]`, not reaped. t+91: `reaped: ["worker"]`, record `stopped`. Call B was 1 minute old when it was killed. The reap notice, verbatim from the run: ``` [subagent watch] STALL CONFIRMED — a live child process sheltered this agent past the ceiling Silent for: 1m 00s (threshold 10m) Child processes: LIVE — 1 live (pid 5002, pgid 5002, S): npx vitest run Elapsed: 2h 01m Sheltered by a live child: 1h 30m (ceiling 90m) — the kernel still showed it at the reap ``` Control run, identical except that ONE tick lands in the 60-second gap: no reap at all (the streak is cleared and each call gets its own clock). So whether a working agent dies depends on tick phase, not on any property of its work. Why this is not a "just tune it" note: - The code's own stated rationale is per-call — "one process group held this long is not a lock wait" (`stall-watchdog.ts` 986-989) and the test comment "the bound is about a single process group being held forever". The group it reaped was held one minute. - The notice pairs the carried duration with the CURRENT child (`pid 5002` … "Sheltered … 1h 30m"), i.e. evidence that is false on its face next to its own `Silent for: 1m 00s` line. Round 2's own thesis is that a notice may not assert more than the code knows. - The reachable population is this fleet's own: a worker with ~90 min of continuous shell-holding in one run with sub-tick gaps (the 3-hour integration lane is exactly that shape) is aborted mid-work — the incident's harm, only later. - `residual 6` already accepts that the clock restarts when a child dies and comes back, so making the bound per-call costs the ceiling nothing it has not already conceded. Cheap fix (either, but the first matches the documented model): floor the streak at the in-flight call's start — `streak.since = Math.max(streak.since, inFlight.at)` on every shelter tick, plus a test with exactly the two-call shape above (it must NOT reap call B). If per-record cumulative is actually intended, then the notice must say so ("sheltered across N calls, current call 1m") and the rationale text in `stall-watchdog.ts` and the ceiling docs must stop describing a single group. ## Verified first-hand at this head (all reproduced here, not taken from the body) - Suite: `npx vitest run` → **109 files / 1523 tests, all pass** (with the lockfile-pinned deps; my first run with loose semver `^0.80.2` was 8 failures, which is a dependency-resolution artifact, not a PR defect). `tsc --noEmit` clean (TS2559-class check below). - Round-1 acceptance re-checked: the wedge path (childless shell) is still challenged then reaped; a lock wait with a live child survives below the ceiling (25-min case and my 89-min case); pid-reuse refused (`live-child-probe.test.ts` 170/191 re-read identity); `unavailable` degradation honest (217/223/233/247/259); child evidence qualified when the in-flight call cannot claim it (C4). - Mutation tallies match your body exactly at this head: disabling the stamp → **10 of 18** fail (`Tests 10 failed | 8 passed`), unconditional stamp (`if (true)`) → **12 of 18** (`11 failed` for my slightly narrower `inFlight !== undefined` shape). The counts are honest. - Ceiling behaviour: the 6-hour `sleep 3600` repro notices at 60 with named evidence and reaps at 90 through `reapNow` (abortCalls = the record, `because: live-child-ceiling`, child evidence in the reap notice); below the ceiling the silence reset is untouched; configured bounds honoured. - Clamp: settings AND env — `PI_SWARM_LIVE_CHILD_NOTICE_MINUTES=500` with `PI_SWARM_LIVE_CHILD_CEILING_MINUTES=30` read back as 500/30 from `getStallWatchdogSettings`, and driving a real watchdog from that source produced **no notice at all** and a reap at 30: the notice is dead when it would be redundant, and no source (file or env) bypasses the clamp, which is applied at decision time. Malformed env ignored (`ninety` → default 90); `0` still disables a bound. - Typed install: `installLiveChildMarker` against an argument with the option renamed → `error TS2559: Type '{ customToolz: any[] }' has no properties in common with type '{ customTools?: ToolDefinition[] }'`. A pi rename breaks the build, it does not go quiet. Install-time self-checks and the loud-degradation tests are real. - Honesty: a tree-wide grep finds NO surviving claim that the marker cannot be forged or is hidden from the agent (only the corrected "attribution, not authentication" text and the "a future unforgeable design must break these tests" notes). Both pinned spoof tests assert the DOCUMENTED behaviour (real command rewriting the marker inside its own call; a real detached child attributed to whatever id its environment carries) — they read as pins, not as claims of security. - Durable breadcrumb carries `childEvidence` and the ceiling fields into `subagents:stall` (`index.ts` 963+). ## Note for the next round Only S2R2-1 needs code. S2R2-2 and S2R2-3 are doc text (`README.md`, `docs/attach-protocol.md`, `types.ts`) and can ride the same commit. My probes are reproducible: the two-call shape is 20 lines against the existing `LiveChildFleet` fixture.

aaron added 1 commit 2026-11-06 12:34:34 +00:00

HOMELAB-2682: fix(pi-swarm-suite): live-child streak anchored per call + docs ... 00dc9721d0

Round 3, closing review seat 2's S2R2-1 (round-2 blocker, comment 22752).

The shelter clock was anchored to the RECORD, so it carried across a call
boundary whenever no sweep landed in the gap between two calls — which is the
normal case, since the gap is a model turn and the sweep is 60 s. Seat 2's repro
(a 89-minute `go build` followed by a fresh `npx vitest run`) reaped the FRESH
call one minute into its own life at t+91, its notice printing the previous
call's "Sheltered by a live child: 1h 30m" against the new call's pid; with one
tick in the gap there was no reap at all. An agent's fate depended on tick phase.

Fix: the clock is anchored to the in-flight call (`streak.since = max(since,
inFlight.at)`, and a new call also gets a new notice), which is what the bound's
own rationale already claimed ("one process group held this long"). Two calls in
a row are now two clocks.

Test first: the seat's exact two-call shape (call A 89 min -> end -> start call B
with no intervening tick) failed with `reapedAtRecordMinute91: ["worker"]`,
`status: "stopped"` — the seat's numbers — and passes after the fix. A control
running the same shape WITH one tick in the gap must produce an identical
outcome; it does (that assertion is what catches phase dependence). Reverting
just this anchor fails exactly those 2 of 20 cases and nothing else.

Docs (round-2 concerns, folded in):
* README: both `stallWatchdog` tables gain `liveChildNoticeMinutes` /
  `liveChildCeilingMinutes` and their env vars; a new "a live child process is
  activity" subsection documents the shelter and its two per-call bounds; the
  "Trade-off, stated plainly" reap sentence is qualified by the live-child path;
  the stale "nested stallWatchdog triple" is now "block".
* docs/attach-protocol.md + types.ts: `wedged: true` names BOTH paths — a
  challenge unanswered through grace, and a live-child ceiling reap that had no
  challenge at all (distinguished by `because` / `liveChildForMs` on the entry).
* A ceiling reap notice no longer prints `Silent for: 1m 00s` (the last tick)
  next to a 90-minute shelter: its head line reports the shelter instead.
* The install-time self-check describes the same exported FACTORY (a pure
  function), not "the same closure the definition got".

Suite: 109 files / 1525 tests green (1523 + 2 new), tsc --noEmit clean.
Mutations still bite: guard disabled 12/20, guard unconditional 12/20,
per-record streak 2/20.

ld-reviewer commented 2026-11-06 12:53:37 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head 00dc9721d0: APPROVE

model: deepseek-flash
session: ses-109r3-seat1-0b51d976

Blockers

None.

Concerns

• C-S1-R3-1 (medium) — the ceiling now measures the CALL's age, not a continuous shelter, and three documents plus the reap notice still say it measures the shelter. With streak = anchored && anchored.since >= inFlight.at ? anchored : { since: inFlight.at }, a re-sighting inside the SAME call is re-anchored to the call start, so within one call shelteredMs is always now - inFlight.at. Measured at this head (my own harness, one call): child 4001 live T+1…T+50, no child T+51…T+60, a NEW child 4200 from T+61 → live-child-long-wait fires at T+61 carrying liveChildForMs 3 660 000 (61 min for a child 0 minutes old), and the ceiling reaps at T+90 with liveChildForMs 5 400 000 (90 min, of which the current child lived 30). Continuous-shelter semantics would predict ≈T+120 / ≈T+151. The docs claim the opposite: README "A break in the kernel evidence restarts it too. Values are clamps on a single continuity — they never accumulate"; the file header "broken when the kernel stops showing a live child for it"; residual 6 "A child that dies and comes back restarts the clock". Operator-facing case (probe E, same head): child live T+1…T+50, absent T+51…T+65, new child T+66 → the silence path does challenge the record at T+62 (challenged: ["w"], stall-challenged delivered), the challenge is dropped when the child returns at T+66, and the T+90 ceiling reap still prints Sheltered for: 1h 30m, not silent — a live child inside the in-flight tool call reset this clock on every tick (the silence threshold 10m never applied) — the threshold did apply and a steer was issued; the same durable entry records that challenge. So the round-3 head line is false in exactly the shape it was added for (a break in the evidence inside one call). Not a blocker: the error is toward reaping, it only reaches calls already ≥90 min old (which the design declares non-legitimate — sandbox bash timeouts ≈55 min), and because / liveChildForMs / ceilingMinutes / childEvidence are all accurate. Two one-line fixes — pick one and pin it with a test: { since: now } on a re-sighting (restores the documented continuity), or state in README + header that the bound is the in-flight call's age. Today the code and its own documentation disagree silently.

• C-S1-R3-2 (low, pre-existing, unchanged by this diff) — inFlightTool holds ONE event per record; two shell calls in flight lose the shelter. start A, start B, end A → the end overwrites the phase, the shelter is dropped while B is still running its live child, and the record is challenged at +10 and reaped at +20 (observed). Unreachable if pi executes a session's tool calls serially, but round 3 makes that single map entry load-bearing for the clock's correctness, so it deserves a residual line rather than being silently true.

• C-S1-R3-3 (informational) — cross-call churn is unbounded, exactly as residual 6 states. 40 consecutive 50-minute calls, each holding a live child, produced 0 notices and 0 reaps over ≈2000 simulated minutes. Recorded because I measured it rather than assuming it.

Verified first-hand at this head

• CI: run #353 (python) success; run #352 all 14 jobs success including pi-swarm-suite 4m1s — 0 pending, 0 failure.
• S2R2-1 closed: the seat's two-call shape is not reaped at record-minute 90/91 (status: running), call B gets its own notice ≈60 min into B (pid 5002) and its own ceiling reap ≈90 min into B — not ~150/~180; the with-tick control is deeply equal.
• Mutations reproduced exactly (my own copy of the tree): guard→false 12/20; guard→true 12/20 (note: with the literal if (true) shape, 2 of those fail via TypeError: Cannot read properties of undefined (reading 'kind') on inFlight.at, so the number is right but partly for the wrong reason — the faithful "stamp whenever a shell is in flight, children or not" variant fails 7/20); streak back to per-record (anchored ?? { since: now }) fails exactly the two round-3 cases and nothing else — 2/20.
• Suite: npx vitest run → 109 files / 1525 tests, all pass on the first run (no flake this time); tsc --noEmit clean.
• Prior behaviour intact: 6-hour single call → notice at 60, wedgedAt == START + 90m, abort through reapNow with the pid/pgid/cmdline evidence; a silent childless call still challenges at +11 and reaps at +21 (my own probe); pid-reuse refusal and unavailable degradation green; settings and env clamps (envMinutes wins per field, malformed ignored, 0 disables, notice ≥ ceiling clamped) covered; customTools?: ToolDefinition[] typed with no cast; no unforgeability claim anywhere (live-child-probe.ts says "never an unforgeable fact"); both spoof tests pin the documented behaviour.
• Docs match the code on the bounds, both env vars, both wedged: true paths, and Sheltered for: on ceiling reap notices — and the durable subagents:stall entry (src/core/index.ts) writes because/liveChildForMs/ceilingMinutes only on the ceiling path and challenge only when present, exactly as docs/attach-protocol.md now says. The single doc-vs-code gap is C-S1-R3-1.

**Verdict of record — ld-reviewer — head 00dc9721d04d2013c13e6bb52c828a9e1c5e0d52: APPROVE** model: deepseek-flash session: ses-109r3-seat1-0b51d976 ## Blockers None. ## Concerns - **C-S1-R3-1 (medium) — the ceiling now measures the CALL's age, not a continuous shelter, and three documents plus the reap notice still say it measures the shelter.** With `streak = anchored && anchored.since >= inFlight.at ? anchored : { since: inFlight.at }`, a re-sighting inside the SAME call is re-anchored to the call start, so within one call `shelteredMs` is always `now - inFlight.at`. Measured at this head (my own harness, one call): child 4001 live T+1…T+50, no child T+51…T+60, a NEW child 4200 from T+61 → `live-child-long-wait` fires at T+61 carrying `liveChildForMs` 3 660 000 (61 min for a child 0 minutes old), and the ceiling reaps at T+90 with `liveChildForMs` 5 400 000 (90 min, of which the current child lived 30). Continuous-shelter semantics would predict ≈T+120 / ≈T+151. The docs claim the opposite: README *"A break in the kernel evidence restarts it too. Values are clamps on a single continuity — they never accumulate"*; the file header *"broken when the kernel stops showing a live child for it"*; residual 6 *"A child that dies and comes back restarts the clock"*. **Operator-facing case (probe E, same head):** child live T+1…T+50, absent T+51…T+65, new child T+66 → the silence path **does** challenge the record at T+62 (`challenged: ["w"]`, `stall-challenged` delivered), the challenge is dropped when the child returns at T+66, and the T+90 ceiling reap still prints `Sheltered for: 1h 30m, not silent — a live child inside the in-flight tool call reset this clock on every tick (the silence threshold 10m never applied)` — the threshold did apply and a steer was issued; the same durable entry records that challenge. So the round-3 head line is false in exactly the shape it was added for (a break in the evidence inside one call). Not a blocker: the error is toward reaping, it only reaches calls already ≥90 min old (which the design declares non-legitimate — sandbox bash timeouts ≈55 min), and `because` / `liveChildForMs` / `ceilingMinutes` / `childEvidence` are all accurate. Two one-line fixes — pick one and pin it with a test: `{ since: now }` on a re-sighting (restores the documented continuity), or state in README + header that the bound is the in-flight call's age. Today the code and its own documentation disagree silently. - **C-S1-R3-2 (low, pre-existing, unchanged by this diff) — `inFlightTool` holds ONE event per record; two shell calls in flight lose the shelter.** start A, start B, `end A` → the `end` overwrites the phase, the shelter is dropped while B is still running its live child, and the record is challenged at +10 and reaped at +20 (observed). Unreachable if pi executes a session's tool calls serially, but round 3 makes that single map entry load-bearing for the clock's correctness, so it deserves a residual line rather than being silently true. - **C-S1-R3-3 (informational) — cross-call churn is unbounded, exactly as residual 6 states.** 40 consecutive 50-minute calls, each holding a live child, produced 0 notices and 0 reaps over ≈2000 simulated minutes. Recorded because I measured it rather than assuming it. ## Verified first-hand at this head - **CI:** run #353 (`python`) success; run #352 all 14 jobs success including `pi-swarm-suite` 4m1s — 0 pending, 0 failure. - **S2R2-1 closed:** the seat's two-call shape is not reaped at record-minute 90/91 (`status: running`), call B gets its own notice ≈60 min into B (pid 5002) and its own ceiling reap ≈90 min into B — not ~150/~180; the with-tick control is deeply equal. - **Mutations reproduced exactly** (my own copy of the tree): guard→`false` **12/20**; guard→`true` **12/20** (note: with the literal `if (true)` shape, 2 of those fail via `TypeError: Cannot read properties of undefined (reading 'kind')` on `inFlight.at`, so the number is right but partly for the wrong reason — the faithful "stamp whenever a shell is in flight, children or not" variant fails 7/20); streak back to per-record (`anchored ?? { since: now }`) fails **exactly the two round-3 cases and nothing else — 2/20**. - **Suite:** `npx vitest run` → **109 files / 1525 tests, all pass on the first run** (no flake this time); `tsc --noEmit` clean. - **Prior behaviour intact:** 6-hour single call → notice at 60, `wedgedAt == START + 90m`, abort through `reapNow` with the pid/pgid/cmdline evidence; a silent childless call still challenges at +11 and reaps at +21 (my own probe); pid-reuse refusal and `unavailable` degradation green; settings **and** env clamps (`envMinutes` wins per field, malformed ignored, `0` disables, notice ≥ ceiling clamped) covered; `customTools?: ToolDefinition[]` typed with no cast; no unforgeability claim anywhere (`live-child-probe.ts` says "never an unforgeable fact"); both spoof tests pin the documented behaviour. - **Docs match the code** on the bounds, both env vars, both `wedged: true` paths, and `Sheltered for:` on ceiling reap notices — and the durable `subagents:stall` entry (`src/core/index.ts`) writes `because`/`liveChildForMs`/`ceilingMinutes` only on the ceiling path and `challenge` only when present, exactly as `docs/attach-protocol.md` now says. The single doc-vs-code gap is C-S1-R3-1.

ld-reviewer commented 2026-11-06 13:10:57 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head 00dc9721d0: REQUEST_CHANGES

model: deepseek-flash
session: ses-109r3-seat2-4c8aad30

Seat 2 of 2, final round. I formed this verdict before reading seat 1's.

Blockers

• S2R3-1: high — a sibling tool call's end clears the shelter of a still-in-flight shell call, so a working agent is challenged and reaped on the silence path (~threshold + grace) while it is genuinely inside a live bash call holding a live process group. inFlightTool is a single slot keyed by record id (stall-watchdog.ts:766), overwritten on every tool_activity(start|end), and the gate is inFlight?.phase === "start" && SHELL_TOOL_NAMES.has(inFlight.toolName). pi executes a tool batch in parallel by default — pi-agent-core/dist/agent-loop.js:257 goes sequential only on config.toolExecution === "sequential" or a tool with executionMode: "sequential"; agent.js:130 defaults to "parallel"; pi-coding-agent sets neither, the bash definition declares no executionMode (dist/core/tools/bash.js:187), and agent-runner passes nothing, so a child session's batch is parallel. executeToolCallsParallel emits every tool_execution_start first, then each tool_execution_end as it completes — so the quick member of the batch ends while the long bash call is still running, overwriting the slot with {phase:"end"} and switching the shelter off for the rest of the long call. My probes (scratch copy of this head, its own LiveChildFleet harness, one tick per simulated minute):
  • batch [bash(long, child alive), bash(second)], second ends at t+10, first still running with its child → challenged@21, REAPED@31, kinds: ["stall-challenged","stall-reaped"], record stopped, one abort;
  • batch [bash(long), read(quick)] — the shape a model emits most often — read ends at t+2 → challenged@13, REAPED@23.
    This is the 08:36Z incident reproduced with a wrong notice (silence-based), by this PR's own round-1 code, on the default pi execution mode, and nothing in the PR documents or accepts it: the body and the code say the loss of evidence applies to "a shell left running by an earlier, finished call" / none (the last tool call, bash, already ended), which is not what happens here — the record is still inside a call that never ended. Fix direction (not prescriptive): keep the set of in-flight shell calls keyed by event.toolCallId (pi's tool_execution_start/end carry it; ToolActivity in agent-runner.ts:312 drops it) so an end closes only its own call, and shelter while any in-flight shell call owns a live child; state the anchor explicitly when several overlap.

Concerns

• C-S2R3-2: medium — same-call child churn: the clock is anchored at the call start, not at the child's appearance, so the notice/reap report a shelter longer than any child existed and a child that comes back inside the same call does not restart the clock. Probe: call opens at t0, child 7001 alive min 1–40, two ticks see it gone (min 41–42), child 7002 appears at min 43 → live-child-long-wait fires at minute 60 of the call with liveChildForMs 60m and Child processes: … pid 7002, and the ceiling reaps at minute 90 printing Sheltered for: 1h 30m while 7002 is 48 minutes old. Two consequences: (a) the file header and residual 6 still say "a child that dies and comes back restarts the clock" — under the new anchoring the streak is deleted on the tick that sees the absence but is immediately re-created with since = inFlight.at (the past), so what it actually measures for churn is call age, not child tenure; (b) the notice asserts a shelter duration that did not happen, which is the same class as the round-2 C4 nit this PR just fixed. Impact is small (a single call past 90 minutes is already declared out of bounds, so the reap is not a new false positive), hence a concern — but the sentence and the notice should say "this call has been running N with a live child", or the anchor should be max(inFlight.at, first tick that saw a child).
• C-S2R3-3: low — the round-2 case named "restarts the shelter when the child goes away — the clock measures ONE continuous shelter" passes only because it issues a new startBash before the second shelter; with per-call anchoring, churn inside one call is exactly the untested case that C-S2R3-2 falls into. Add the same-call variant so the name and the coverage agree.

What I verified first-hand at this head

• CI green on 00dc972: run #352 all 13 jobs success (pi-swarm-suite 4m1s), run #353 python success; no red job.
• Round-3 blocker (S2R2-1) is genuinely closed. Independent repro, no tick in the gap and with one tick in the gap: call A 89 min with child 4001 (go build ./...), then end + start + child 5002; no reap at t+90 or t+91; B's own live-child-long-wait at 60 min into B with pid 5002; B's own stall-reaped at 90 min into B with liveChildForMs 90m (not ~180m); exactly one abort; and the two runs are deeply equal (JSON.stringify comparison of the whole outcome: notices, durations, evidence, reap, status, aborts). Phase dependence is gone.
• Prior acceptance re-probed: one 91-minute call with one continuous child → notice@60, reaped@90, evidence LIVE — 1 live (pid 9001, …) through reapNow, notice head Sheltered for: 1h 30m … never applied with no Silent for:; below-ceiling silence reset, childless wedge path, pid-reuse refusal, unavailable degradation, clamp from settings + env are covered by green cases I re-ran; the two spoof tests are present (live-child-marker.test.ts:85 and the probe case); the only "cannot be set by the agent" strings left in the tree quote the old claim as false (live-child-marker.ts:18-19), so no unforgeability claim survives; installLiveChildMarker is typed against pi's customTools (no cast) and tsc --noEmit is clean, so a pi rename is a compile error.
• Suite: independent copy of src/, tests/, configs → npx vitest run = 109 files / 1525 tests, all pass in one run, no flake; tsc --noEmit exit 0.
• Mutations measured at this head (20 live-child cases): if (true) unconditional stamp → 12 failed; guard disabled → 12 failed; streak back to per-record (anchored ?? { since: now }) → exactly 2 failed, and they are precisely the two round-3 cases. The round-3 fix has its own knife-edge mutation with no collateral — the claim reproduces.
• Docs: README lines 353-354 and 499-500 carry both bounds with PI_SWARM_LIVE_CHILD_NOTICE_MINUTES / …_CEILING_MINUTES, the 0-disables and clamp-at-ceiling semantics, and the "A live child process is activity (HOMELAB-2682)" section; docs/attach-protocol.md:56-60 names both paths to wedged: true including "no challenge ever happened" with because: "live-child-ceiling", liveChildForMs, ceilingMinutes.

Posting exactly one comment; no file, commit or push from this seat.

**Verdict of record — ld-reviewer — head 00dc9721d04d2013c13e6bb52c828a9e1c5e0d52: REQUEST_CHANGES** model: deepseek-flash session: ses-109r3-seat2-4c8aad30 Seat 2 of 2, final round. I formed this verdict before reading seat 1's. ## Blockers - S2R3-1: high — a *sibling* tool call's `end` clears the shelter of a still-in-flight shell call, so a working agent is challenged and reaped on the silence path (~threshold + grace) while it is genuinely inside a live bash call holding a live process group. `inFlightTool` is a single slot keyed by record id (`stall-watchdog.ts:766`), overwritten on every `tool_activity(start|end)`, and the gate is `inFlight?.phase === "start" && SHELL_TOOL_NAMES.has(inFlight.toolName)`. pi executes a tool batch **in parallel by default** — `pi-agent-core/dist/agent-loop.js:257` goes sequential only on `config.toolExecution === "sequential"` or a tool with `executionMode: "sequential"`; `agent.js:130` defaults to `"parallel"`; `pi-coding-agent` sets neither, the `bash` definition declares no `executionMode` (`dist/core/tools/bash.js:187`), and `agent-runner` passes nothing, so a child session's batch is parallel. `executeToolCallsParallel` emits every `tool_execution_start` first, then each `tool_execution_end` as it completes — so the quick member of the batch ends while the long bash call is still running, overwriting the slot with `{phase:"end"}` and switching the shelter off for the rest of the long call. My probes (scratch copy of this head, its own `LiveChildFleet` harness, one tick per simulated minute): - batch `[bash(long, child alive), bash(second)]`, second ends at t+10, first still running with its child → `challenged@21`, `REAPED@31`, `kinds: ["stall-challenged","stall-reaped"]`, record `stopped`, one abort; - batch `[bash(long), read(quick)]` — the shape a model emits most often — read ends at t+2 → `challenged@13`, `REAPED@23`. This is the 08:36Z incident reproduced with a *wrong* notice (silence-based), by this PR's own round-1 code, on the default pi execution mode, and nothing in the PR documents or accepts it: the body and the code say the loss of evidence applies to "a shell left running by an earlier, **finished** call" / `none (the last tool call, bash, already ended)`, which is not what happens here — the record is still inside a call that never ended. Fix direction (not prescriptive): keep the set of in-flight shell calls keyed by `event.toolCallId` (pi's `tool_execution_start/end` carry it; `ToolActivity` in `agent-runner.ts:312` drops it) so an `end` closes only its own call, and shelter while any in-flight shell call owns a live child; state the anchor explicitly when several overlap. ## Concerns - C-S2R3-2: medium — same-call child churn: the clock is anchored at the **call start**, not at the child's appearance, so the notice/reap report a shelter longer than any child existed and a child that comes back inside the same call does *not* restart the clock. Probe: call opens at t0, child 7001 alive min 1–40, two ticks see it gone (min 41–42), child 7002 appears at min 43 → `live-child-long-wait` fires at **minute 60 of the call** with `liveChildForMs` 60m and `Child processes: … pid 7002`, and the ceiling reaps at **minute 90** printing `Sheltered for: 1h 30m` while 7002 is 48 minutes old. Two consequences: (a) the file header and residual 6 still say "a child that dies and comes back restarts the clock" — under the new anchoring the streak is deleted on the tick that sees the absence but is immediately re-created with `since = inFlight.at` (the *past*), so what it actually measures for churn is **call age**, not child tenure; (b) the notice asserts a shelter duration that did not happen, which is the same class as the round-2 C4 nit this PR just fixed. Impact is small (a single call past 90 minutes is already declared out of bounds, so the reap is not a new false positive), hence a concern — but the sentence and the notice should say "this call has been running N with a live child", or the anchor should be `max(inFlight.at, first tick that saw a child)`. - C-S2R3-3: low — the round-2 case named "restarts the shelter when the child goes away — the clock measures ONE continuous shelter" passes only because it issues a **new** `startBash` before the second shelter; with per-call anchoring, churn *inside* one call is exactly the untested case that C-S2R3-2 falls into. Add the same-call variant so the name and the coverage agree. ## What I verified first-hand at this head - **CI green** on `00dc972`: run #352 all 13 jobs `success` (`pi-swarm-suite` 4m1s), run #353 `python` `success`; no red job. - **Round-3 blocker (S2R2-1) is genuinely closed.** Independent repro, no tick in the gap and with one tick in the gap: call A 89 min with child 4001 (`go build ./...`), then `end` + `start` + child 5002; **no reap at t+90 or t+91**; B's own `live-child-long-wait` at 60 min into B with `pid 5002`; B's own `stall-reaped` at 90 min into B with `liveChildForMs` 90m (not ~180m); exactly one abort; and the two runs are **deeply equal** (`JSON.stringify` comparison of the whole outcome: notices, durations, evidence, reap, status, aborts). Phase dependence is gone. - **Prior acceptance re-probed**: one 91-minute call with one continuous child → `notice@60`, `reaped@90`, evidence `LIVE — 1 live (pid 9001, …)` through `reapNow`, notice head `Sheltered for: 1h 30m … never applied` with **no** `Silent for:`; below-ceiling silence reset, childless wedge path, pid-reuse refusal, `unavailable` degradation, clamp from settings + env are covered by green cases I re-ran; the two spoof tests are present (`live-child-marker.test.ts:85` and the probe case); the only "cannot be set by the agent" strings left in the tree quote the old claim as **false** (`live-child-marker.ts:18-19`), so no unforgeability claim survives; `installLiveChildMarker` is typed against pi's `customTools` (no cast) and `tsc --noEmit` is clean, so a pi rename is a compile error. - **Suite**: independent copy of `src/`, `tests/`, configs → `npx vitest run` = **109 files / 1525 tests, all pass** in one run, no flake; `tsc --noEmit` exit 0. - **Mutations measured at this head** (20 live-child cases): `if (true)` unconditional stamp → **12 failed**; guard disabled → **12 failed**; streak back to per-record (`anchored ?? { since: now }`) → **exactly 2 failed**, and they are precisely the two round-3 cases. The round-3 fix has its own knife-edge mutation with no collateral — the claim reproduces. - **Docs**: README lines 353-354 and 499-500 carry both bounds with `PI_SWARM_LIVE_CHILD_NOTICE_MINUTES` / `…_CEILING_MINUTES`, the `0`-disables and clamp-at-ceiling semantics, and the "A live child process is activity (HOMELAB-2682)" section; `docs/attach-protocol.md:56-60` names **both** paths to `wedged: true` including "no challenge ever happened" with `because: "live-child-ceiling"`, `liveChildForMs`, `ceilingMinutes`. Posting exactly one comment; no file, commit or push from this seat.

aaron added 1 commit 2026-11-07 01:05:11 +00:00

HOMELAB-2682: fix(pi-swarm-suite): per-call in-flight tracking; truthful shelter notice/ceiling (scoped) ... e5e4d702b6

Round 4, scoped to the three findings open at 00dc972.

S2R3-1 (blocker): inFlightTool was one slot per record, so a sibling call's
end in the same parallel tool batch switched the shelter off while a long bash
was still live (reproduced as challenged@13/reaped@23 on [bash(long), read]).
In-flight calls are now tracked per call by pi's toolCallId; an end closes only
its own call and the shelter holds while any open shell call owns live kernel
evidence.

C-S1-R3-1 / C-S2R3-2 (concerns): the ceiling measured the CALL's age, not
continuous shelter, contradicting README/header/residual 6 and overstating the
shelter in the notices. Chosen semantics: continuous sheltered time, anchored
to one call (oldest open shell call). A break in the kernel evidence restarts
the clock at that tick, and since starts at the first tick that SAW the child,
so the measurement is a lower bound and never overstates. Notices now state the
basis and the time left, report call age separately for contrast, and the reap
head line no longer claims the silence threshold never applied (probe E).

ld-reviewer commented 2026-11-07 01:20:10 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head e5e4d702b6: APPROVE

model: omniroute/deepseek-flash
session: 01a1b390-0d3e-70d7-89c1-cc0aca46fd8d

Round 4, seat 1 of 2 — a fresh review session; I read the three round-3 verdicts and re-litigate nothing except where this delta touches them. Reviewer only: I did not edit, commit, push or merge, and this is my only write.

Blockers

None.

Concerns

• C-R4S1-1 (low, doc) — the PR body was never updated for round 4 and still points reviewers at the superseded head. Fetched at this head: the last section reads "fresh review seats review this head — 00dc972"; the strings ROUND 4 and e5e4d70 do not appear in the body (23 589 chars). So the operator-facing record of what this head contains is absent, and the body's residual 6 sentence ("A child that dies and comes back restarts the clock") describes behaviour that was only true from this head on. In-repo contracts (README, the file header, the header's residual list) are accurate at this head; the body is the stale surface. Cheap fix: add a ROUND 4 section and repoint the Merge section at e5e4d70.
• C-R4S1-2 (low, mechanism gap; measured benign) — resumeAgent still publishes tool activity with no toolCallId. agent-runner.ts:1029-1030 (the resume/wake path, reached through turnWiring → emitActivity) omits the id even though the pi event carries it (pi-agent-core/dist/types.d.ts:383 — toolCallId: string on tool_execution_start, same on end), while runAgent at :938/:941 plumbs it. On a resumed turn every call is therefore a synthetic:* call and an end closes the oldest open call of the same tool name (stall-watchdog.ts:889-895). Measured on the real watchdog: batch [bash, bash] with no ids (exactly the resume shape), short bash ends at t+4 → shelter survives, liveChildren: ["w"] every tick to t+30, no challenge, no reap, record running — the open set keeps the other bash, so the phantom close cannot empty it and produce a false reap. It is an inconsistency in the stated mechanism ("keyed by toolCallId"), not a demonstrated defect; one line at :1029-1030 makes it uniform.
• Nit — the long-wait notice's Basis: line hardcodes "the in-flight bash call" (an anchor named powershell would be misnamed), and the notice prints the ceiling twice ("ceiling 90m" in the head line, then "reap ceiling 90m — 30m left"). Cosmetic only.

What I measured (all at e5e4d70, my own runs)

1. The [bash(long), read] batch shape — fixed. Real StallWatchdog (head sources, the harness's own LiveChildFleet-style deps) with tool_activity carrying ids: start bash c1, start read c2, end read c2 at t+2, one tick per minute to t+25 → challenged: [], reaped: [], liveChildren: ["w"] every tick, record running, 0 aborts, 0 notices. The seat's challenged@13/reaped@23 does not occur. Also: a no-id end for a call that never started (the runner's tools-error: synthetic events) mid-bash → shelter intact to t+20, nothing challenged.
2. The phase evidence reflects the still-live bash. Holding the batch open to the 60-minute notice, the notice reads Child processes: LIVE — 1 live (pid 5001, pgid 5001, S) with no "already ended" qualifier, and the Basis: line names the in-flight bash call. The evidence is about the call that is still working, not the sibling that ended.
3. Continuous-shelter semantics — as claimed. Child 4001 live T+1…T+50, absent T+51…T+60, new child 4200 from T+61: the live-child-long-wait notice fires at record minute 121 (= 60 min after the new sighting) with liveChildForMs 60m and callAgeMs 121m, and the ceiling reaps at record minute 151 (= 90 min after the new sighting) with because: live-child-ceiling, liveChildForMs 90m. Nothing reaps at record minute 90 (the round-3 code reaped there). since is set to the tick that first saw the child (stall-watchdog.ts:1180), so the measured span can never exceed the observed evidence — a lower bound, so the reap is never early. The notice states the basis in words and the remaining time (30m left).
4. Anchor change is conservative, and documented. Two bash calls, the oldest ends at t+40 while the other keeps a live child: notice at 60 min after the anchor change, ceiling reap at record minute 131 (90 min after it). That is a clock restart, which README ("the anchor changing starts a new clock") and the file header state; the cost is later reaping, never earlier.
5. The batch shape is real in this pi version. pi-agent-core/dist/agent-loop.js:299 executeToolCallsParallel emits every tool_execution_start before the ends, and :256-260 goes sequential only on toolExecution === "sequential" or a tool's executionMode; toolCallId is on both events (types.d.ts:382-385). So the plumbing has a field to carry and the shape has a producer.
6. The new tests bite (my own mutations, throwaway copies). Single-slot simulation (every start overwrites one "slot" key, every end clears it): 5 of 27 cases fail — both round-4 sibling cases, "still drops the shelter when the SHELL call itself ends", and 2 round-3 per-call cases. Call-anchored since ({ since: anchor.at }, the round-3 shape): 5 of 27 fail — all continuous-shelter cases (does not notice a brand-new child…, reaps at the ceiling measured from the new child…, the notice-basis case, probe E, and round-2's "restarts the shelter when the child goes away"). Neither mutation is survivable.
7. Suite at the head, my run. npx vitest run: 109 files / 1532 tests, all pass, exit 0, no flake. tsc --noEmit: clean. live-child-probe.test.ts + live-child-marker.test.ts: 27/27 pass (the real detached-process probe still answers on this host). CI on e5e4d70: run #400 all 15 jobs success (incl. pi-swarm-suite 3m46s), run #401 python success — 0 failures, 0 pending.
8. Delta scope is exactly what was declared. 00dc9721..e5e4d702 touches 6 files: stall-watchdog.ts, activity-bus.ts, agent-runner.ts, agent-manager.ts, README.md, tests/core/stall-watchdog-live-child.test.ts. live-child-probe.ts and live-child-marker.ts are untouched — no change to marker/probe code beyond nothing at all. The runner/manager/bus hunks are pure plumbing (an optional toolCallId on the event and the type), and the README diff is the continuity/anchor/batch text.

Exposure

Read: the three round-3 verdict comments and the PR body (via the public API), the full 00dc9721..e5e4d702 diff, the head's stall-watchdog.ts (shelter/reap/notice regions, header and residual list), agent-runner.ts (both activity emission sites), activity-bus.ts, agent-manager.ts forwarding, README's shelter sections, and the installed pi 0.80.3 agent-loop.js / types.d.ts. Ran: the full suite, tsc --noEmit, the probe/marker test files, my own 6-probe test file and two mutation copies — all in throwaway copies under /tmp (the branch worktree was read only; nothing was written to the PR tree). Disclosure: my first fetch of the PR body used the -H "Authorization: token $(cat …)" shell-substitution form for a GET — the token file does not exist on this host, so the header carried an empty string and the request was an anonymous read of a public repo; no write of mine used any token in argv, and this verdict is posted only through the staged-credential script.

Verdict of record — ld-reviewer — head e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b: APPROVE model: omniroute/deepseek-flash session: 01a1b390-0d3e-70d7-89c1-cc0aca46fd8d Round 4, seat 1 of 2 — a fresh review session; I read the three round-3 verdicts and re-litigate nothing except where this delta touches them. Reviewer only: I did not edit, commit, push or merge, and this is my only write. ## Blockers None. ## Concerns - **C-R4S1-1 (low, doc) — the PR body was never updated for round 4 and still points reviewers at the superseded head.** Fetched at this head: the last section reads "fresh review seats review this head — `00dc972`"; the strings `ROUND 4` and `e5e4d70` do not appear in the body (23 589 chars). So the operator-facing record of what this head contains is absent, and the body's residual 6 sentence ("A child that dies and comes back restarts the clock") describes behaviour that was only true from this head on. In-repo contracts (README, the file header, the header's residual list) are accurate at this head; the body is the stale surface. Cheap fix: add a ROUND 4 section and repoint the Merge section at `e5e4d70`. - **C-R4S1-2 (low, mechanism gap; measured benign) — `resumeAgent` still publishes tool activity with no `toolCallId`.** `agent-runner.ts:1029-1030` (the resume/wake path, reached through `turnWiring` → `emitActivity`) omits the id even though the pi event carries it (`pi-agent-core/dist/types.d.ts:383` — `toolCallId: string` on `tool_execution_start`, same on `end`), while `runAgent` at `:938/:941` plumbs it. On a resumed turn every call is therefore a `synthetic:*` call and an `end` closes the **oldest open call of the same tool name** (`stall-watchdog.ts:889-895`). Measured on the real watchdog: batch `[bash, bash]` with no ids (exactly the resume shape), short bash ends at t+4 → shelter **survives**, `liveChildren: ["w"]` every tick to t+30, no challenge, no reap, record `running` — the open set keeps the other bash, so the phantom close cannot empty it and produce a false reap. It is an inconsistency in the stated mechanism ("keyed by toolCallId"), not a demonstrated defect; one line at `:1029-1030` makes it uniform. - **Nit** — the long-wait notice's `Basis:` line hardcodes "the in-flight **bash** call" (an anchor named `powershell` would be misnamed), and the notice prints the ceiling twice ("ceiling 90m" in the head line, then "reap ceiling 90m — 30m left"). Cosmetic only. ## What I measured (all at e5e4d70, my own runs) 1. **The `[bash(long), read]` batch shape — fixed.** Real `StallWatchdog` (head sources, the harness's own `LiveChildFleet`-style deps) with `tool_activity` carrying ids: `start bash c1`, `start read c2`, `end read c2` at t+2, one tick per minute to t+25 → `challenged: []`, `reaped: []`, `liveChildren: ["w"]` every tick, record `running`, 0 aborts, 0 notices. The seat's `challenged@13/reaped@23` does not occur. Also: a no-id `end` for a call that never started (the runner's `tools-error:` synthetic events) mid-bash → shelter intact to t+20, nothing challenged. 2. **The phase evidence reflects the still-live bash.** Holding the batch open to the 60-minute notice, the notice reads `Child processes: LIVE — 1 live (pid 5001, pgid 5001, S)` with **no** "already ended" qualifier, and the `Basis:` line names the in-flight bash call. The evidence is about the call that is still working, not the sibling that ended. 3. **Continuous-shelter semantics — as claimed.** Child 4001 live T+1…T+50, absent T+51…T+60, new child 4200 from T+61: the `live-child-long-wait` notice fires at **record minute 121** (= 60 min after the new sighting) with `liveChildForMs` 60m and `callAgeMs` 121m, and the ceiling reaps at **record minute 151** (= 90 min after the new sighting) with `because: live-child-ceiling`, `liveChildForMs` 90m. Nothing reaps at record minute 90 (the round-3 code reaped there). `since` is set to the tick that first **saw** the child (`stall-watchdog.ts:1180`), so the measured span can never exceed the observed evidence — a lower bound, so the reap is never early. The notice states the basis in words and the remaining time (`30m left`). 4. **Anchor change is conservative, and documented.** Two bash calls, the oldest ends at t+40 while the other keeps a live child: notice at 60 min after the anchor change, ceiling reap at **record minute 131** (90 min after it). That is a clock restart, which README ("the anchor changing starts a new clock") and the file header state; the cost is later reaping, never earlier. 5. **The batch shape is real in this pi version.** `pi-agent-core/dist/agent-loop.js:299` `executeToolCallsParallel` emits every `tool_execution_start` before the ends, and `:256-260` goes sequential only on `toolExecution === "sequential"` or a tool's `executionMode`; `toolCallId` is on both events (`types.d.ts:382-385`). So the plumbing has a field to carry and the shape has a producer. 6. **The new tests bite (my own mutations, throwaway copies).** Single-slot simulation (every `start` overwrites one `"slot"` key, every `end` clears it): **5 of 27** cases fail — both round-4 sibling cases, "still drops the shelter when the SHELL call itself ends", and 2 round-3 per-call cases. Call-anchored `since` (`{ since: anchor.at }`, the round-3 shape): **5 of 27** fail — all continuous-shelter cases (`does not notice a brand-new child…`, `reaps at the ceiling measured from the new child…`, the notice-basis case, probe E, and round-2's "restarts the shelter when the child goes away"). Neither mutation is survivable. 7. **Suite at the head, my run.** `npx vitest run`: **109 files / 1532 tests, all pass**, exit 0, no flake. `tsc --noEmit`: clean. `live-child-probe.test.ts` + `live-child-marker.test.ts`: 27/27 pass (the real detached-process probe still answers on this host). CI on `e5e4d70`: run #400 all 15 jobs success (incl. `pi-swarm-suite` 3m46s), run #401 `python` success — 0 failures, 0 pending. 8. **Delta scope is exactly what was declared.** `00dc9721..e5e4d702` touches 6 files: `stall-watchdog.ts`, `activity-bus.ts`, `agent-runner.ts`, `agent-manager.ts`, `README.md`, `tests/core/stall-watchdog-live-child.test.ts`. `live-child-probe.ts` and `live-child-marker.ts` are untouched — no change to marker/probe code beyond nothing at all. The runner/manager/bus hunks are pure plumbing (an optional `toolCallId` on the event and the type), and the README diff is the continuity/anchor/batch text. ## Exposure Read: the three round-3 verdict comments and the PR body (via the public API), the full `00dc9721..e5e4d702` diff, the head's `stall-watchdog.ts` (shelter/reap/notice regions, header and residual list), `agent-runner.ts` (both activity emission sites), `activity-bus.ts`, `agent-manager.ts` forwarding, README's shelter sections, and the installed pi 0.80.3 `agent-loop.js` / `types.d.ts`. Ran: the full suite, `tsc --noEmit`, the probe/marker test files, my own 6-probe test file and two mutation copies — all in throwaway copies under `/tmp` (the branch worktree was read only; nothing was written to the PR tree). Disclosure: my first fetch of the PR body used the `-H "Authorization: token $(cat …)"` shell-substitution form for a **GET** — the token file does not exist on this host, so the header carried an empty string and the request was an anonymous read of a public repo; no write of mine used any token in argv, and this verdict is posted only through the staged-credential script.

ld-reviewer commented 2026-11-07 01:22:52 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head e5e4d702b6: APPROVE
model: omniroute/deepseek-flash
session: 01a1b391-f16a-70d7-89c1-cc0d4b813b5b

Round 4, seat 2 of 2, a genuinely separate fresh session (I read no round-4 verdict; I read the three round-3 verdicts because the brief directed it). Reviewer only: I edited nothing, committed nothing, pushed nothing, merged nothing. The head was verified live immediately before this comment and had not moved (e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b, branch tip equal, PR state: OPEN).

Blockers

None.

What I measured myself

I did not run or re-read the PR's own cases as evidence: I wrote my own harbour
(/tmp, a byte copy of the head's pi-swarm-suite, node_modules symlinked to the
lockfile-pinned tree), my own fake probe, my own event order and my own tick
schedule, then deleted it.

1. S2R3-1 — the [bash(long), read] batch. Driven in pi's real parallel shape
(both tool_execution_starts before the first end, ids call-1/call-2), child
live throughout, one tick per simulated minute: ticks t+1…t+40 all reported
liveChildren: ["w"], challenged: [], reaped: [], no notice, record running,
zero aborts — 40 minutes past the seat's challenged@13/reaped@23. The child-evidence
line read LIVE — … and did not mention the ended read. Same shape with no
toolCallId on either event: shelter held 30 minutes. Two overlapping bash calls
with the sibling's end at t+10: quiet through t+45. Control (the other half of the
blocker, so per-call end handling is not simply ignored): the shell call's own
end still runs the silence path — challenged at +11, reaped 10 minutes later,
record stopped. Three parallel shell calls whose ends arrive id-less: shelter held.

2. Continuous shelter — C-S1-R3-1 / C-S2R3-2. Seat 1's churn shape (child 4001
live T+1…T+50, absent T+51…T+60, new child 4200 from T+61, one call open throughout):
no live-child-long-wait at T+61 (the round-3 head fired one claiming 61 min of
"shelter" for a child one tick old); the 10-minute gap did breach the silence
threshold, so a stall-challenged exists on the pre-existing path and the returning
child dropped the pending challenge; the notice then landed after 60 minutes of
the new stretch with liveChildForMs exactly 60 min and callAgeMs 2h 01m
reported separately; the ceiling reaped at record-minute 151 (T+61+90), not 90,
with liveChildForMs 90 min and because: live-child-ceiling. Lower bound:
single call, child first visible on the tick at T+1 → reap at T+91, one sweep
later than the true child age and never earlier, so the clock can only understate.
Anchor change: two overlapping shell calls, the oldest (the anchor) ending at
minute 70 → clock restarts, no reap, no second notice, and the notice already sent
carried liveChildForMs 60 min with callAgeMs 61 min.

3. The words. Rendered both notices through formatStallNotice: the long-wait head
says Sheltered for: 1h 00m of continuous kernel evidence, the body adds
— 30m left and a Basis: block stating that a break restarts the clock and that
call age is not what the bound measures; the ceiling-reap head says
Sheltered for: 1h 30m of continuous kernel evidence and prints neither
Silent for: nor "never applied". Cross-read against README's two stallWatchdog
tables, its "a live child process is activity" subsection and the file header: every
sentence there (continuous stretch, starts at the first tick that saw the child as a
lower bound, restarts on a child-less tick, batch = a set of calls, anchor = oldest
open shell call, restarts when the anchor changes) matches one of the measurements
above. I found no doc sentence in those surfaces that the code does not do.

4. My own mutations (applied to the head's stall-watchdog.ts in my copy, then
reverted). Re-introducing the round-3 single slot (calls.clear() on every start)
fails exactly my 3 batch cases and nothing else. Re-introducing call-age anchoring
({ since: anchor.at }) fails exactly my 3 continuity cases and nothing else. So
both fixes are load-bearing under a measurement that is not the PR's.

5. Scope. The delta is exactly 6 files — stall-watchdog.ts, activity-bus.ts,
agent-runner.ts, agent-manager.ts, README.md,
tests/core/stall-watchdog-live-child.test.ts. Nothing outside the three findings
changed. The event field the fix rests on is real, not assumed:
pi-agent-core@0.80.3 dist/types.d.ts declares toolCallId: string on both
tool_execution_start and tool_execution_end, and dist/agent-loop.js
executeToolCallsParallel emits every start (carrying toolCall.id) before any end.

6. Suite and typecheck at the head, my run. Whole package: 109 files / 1532
tests — 1 failed / 1531 passed, the failure being
tests/core/watch-renderer.test.ts > orphan exit … (5.3 s, a spawn-timing assert
under full-run load); that file passes 18/18 three times standalone and is
untouched by this delta, so I attribute nothing to the change. stall-watchdog-live-child.test.ts
27/27. tsc --noEmit exit 0. CI at the head: run #400 all 14 jobs success
(pi-swarm-suite 3m46s), #401 python success.

Observations (no action demanded by this seat)

• O-1 (bookkeeping, not code). The PR description still describes 00dc972 in its
  "Tests" and "Merge" sections (1525 tests; "review this head 00dc972"). The round-4
  numbers (1532, this head) exist in the fix notes but not in the body. For the landing
  worker, not a finding against the delta.
• O-2 (measured, not a finding). resumeAgent's subscribe
  (agent-runner.ts:1029-1030) still publishes tool_activity without toolCallId,
  so resumed/woken runs use the id-less fallback (an end closes the oldest open call
  of the same name). I checked whether that reopens S2R3-1 there and it does not: the
  [bash, read] shape survives, and a three-shell batch with id-less ends keeps the
  shelter, because starts and ends balance per tool name so the set's cardinality stays
  right; only lastEnded/anchor identity can land on a sibling. Recorded so a later
  round does not have to re-derive it.
• O-3 (nit, judgement). The shared shelter head line still reads
  not a silence verdict: … reset this clock on every tick of that stretch. In seat 1's
  probe-E shape a challenge was issued during a gap; the sentence is literally true of
  the stretch it measures and, unlike the round-3 wording, no longer claims the
  threshold "never applied" — but a reader could still hear "the silence path never
  ran". Not a blocker; noted so the residual ambiguity is on the record rather than
  rediscovered.

Exposure line

Read: the delta diff git diff 00dc9721 e5e4d70 (6 files, +548/−98) in a detached
worktree at the head; the three round-3 verdict comments; pi-agent-core@0.80.3
dist/types.d.ts and dist/agent-loop.js; stall-watchdog.ts (header, InFlightCall/
InFlightState, describeChildEvidence, the tick's shelter branch, formatStallNotice);
both agent-runner.ts subscriptions; agent-manager.ts turnWiring; README's two
stallWatchdog tables and the live-child subsection.
Ran, in /tmp against a copy of the head's tree: my own 7-case harness plus two
extra probes (id-less three-shell batch; anchor change with two overlapping shells,
including rendering both notices), the two mutations above, one full vitest run, one
standalone watch-renderer.test.ts ×3, one standalone
stall-watchdog-live-child.test.ts, and tsc --noEmit; plus forgejo_ci_status for
the head sha. No write to the PR branch, no write to any repository; this comment is my
only write.

Verdict of record — ld-reviewer — head e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b: APPROVE model: omniroute/deepseek-flash session: 01a1b391-f16a-70d7-89c1-cc0d4b813b5b Round 4, seat 2 of 2, a genuinely separate fresh session (I read no round-4 verdict; I read the three round-3 verdicts because the brief directed it). Reviewer only: I edited nothing, committed nothing, pushed nothing, merged nothing. The head was verified live immediately before this comment and had not moved (`e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b`, branch tip equal, PR `state: OPEN`). ## Blockers None. ## What I measured myself I did not run or re-read the PR's own cases as evidence: I wrote my own harbour (`/tmp`, a byte copy of the head's `pi-swarm-suite`, `node_modules` symlinked to the lockfile-pinned tree), my own fake probe, my own event order and my own tick schedule, then deleted it. **1. S2R3-1 — the `[bash(long), read]` batch.** Driven in pi's real parallel shape (both `tool_execution_start`s before the first `end`, ids `call-1`/`call-2`), child live throughout, one tick per simulated minute: ticks t+1…t+40 all reported `liveChildren: ["w"]`, `challenged: []`, `reaped: []`, no notice, record `running`, zero aborts — 40 minutes past the seat's `challenged@13/reaped@23`. The child-evidence line read `LIVE — …` and did **not** mention the ended `read`. Same shape with **no** `toolCallId` on either event: shelter held 30 minutes. Two overlapping `bash` calls with the sibling's `end` at t+10: quiet through t+45. Control (the other half of the blocker, so per-call `end` handling is not simply ignored): the shell call's **own** `end` still runs the silence path — challenged at +11, reaped 10 minutes later, record `stopped`. Three parallel shell calls whose `end`s arrive id-less: shelter held. **2. Continuous shelter — C-S1-R3-1 / C-S2R3-2.** Seat 1's churn shape (child 4001 live T+1…T+50, absent T+51…T+60, new child 4200 from T+61, one call open throughout): no `live-child-long-wait` at T+61 (the round-3 head fired one claiming 61 min of "shelter" for a child one tick old); the 10-minute gap did breach the silence threshold, so a `stall-challenged` exists on the pre-existing path and the returning child **dropped** the pending challenge; the notice then landed after 60 minutes of the **new** stretch with `liveChildForMs` exactly 60 min and `callAgeMs` 2h 01m reported separately; the ceiling reaped at record-minute **151** (T+61+90), not 90, with `liveChildForMs` 90 min and `because: live-child-ceiling`. **Lower bound:** single call, child first visible on the tick at T+1 → reap at **T+91**, one sweep later than the true child age and never earlier, so the clock can only understate. **Anchor change:** two overlapping shell calls, the oldest (the anchor) ending at minute 70 → clock restarts, no reap, no second notice, and the notice already sent carried `liveChildForMs` 60 min with `callAgeMs` 61 min. **3. The words.** Rendered both notices through `formatStallNotice`: the long-wait head says `Sheltered for: 1h 00m of continuous kernel evidence`, the body adds `— 30m left` and a `Basis:` block stating that a break restarts the clock and that call age is **not** what the bound measures; the ceiling-reap head says `Sheltered for: 1h 30m of continuous kernel evidence` and prints neither `Silent for:` nor "never applied". Cross-read against README's two `stallWatchdog` tables, its "a live child process is activity" subsection and the file header: every sentence there (continuous stretch, starts at the first tick that saw the child as a lower bound, restarts on a child-less tick, batch = a set of calls, anchor = oldest open shell call, restarts when the anchor changes) matches one of the measurements above. I found no doc sentence in those surfaces that the code does not do. **4. My own mutations (applied to the head's `stall-watchdog.ts` in my copy, then reverted).** Re-introducing the round-3 single slot (`calls.clear()` on every `start`) fails **exactly my 3 batch cases** and nothing else. Re-introducing call-age anchoring (`{ since: anchor.at }`) fails **exactly my 3 continuity cases** and nothing else. So both fixes are load-bearing under a measurement that is not the PR's. **5. Scope.** The delta is exactly 6 files — `stall-watchdog.ts`, `activity-bus.ts`, `agent-runner.ts`, `agent-manager.ts`, `README.md`, `tests/core/stall-watchdog-live-child.test.ts`. Nothing outside the three findings changed. The event field the fix rests on is real, not assumed: `pi-agent-core@0.80.3` `dist/types.d.ts` declares `toolCallId: string` on both `tool_execution_start` and `tool_execution_end`, and `dist/agent-loop.js` `executeToolCallsParallel` emits every start (carrying `toolCall.id`) before any end. **6. Suite and typecheck at the head, my run.** Whole package: **109 files / 1532 tests — 1 failed / 1531 passed**, the failure being `tests/core/watch-renderer.test.ts > orphan exit …` (5.3 s, a spawn-timing assert under full-run load); that file passes **18/18 three times standalone** and is untouched by this delta, so I attribute nothing to the change. `stall-watchdog-live-child.test.ts` **27/27**. `tsc --noEmit` **exit 0**. CI at the head: run #400 all 14 jobs success (`pi-swarm-suite` 3m46s), #401 `python` success. ## Observations (no action demanded by this seat) - **O-1 (bookkeeping, not code).** The PR description still describes `00dc972` in its "Tests" and "Merge" sections (1525 tests; "review this head 00dc972"). The round-4 numbers (1532, this head) exist in the fix notes but not in the body. For the landing worker, not a finding against the delta. - **O-2 (measured, not a finding).** `resumeAgent`'s subscribe (`agent-runner.ts:1029-1030`) still publishes `tool_activity` without `toolCallId`, so resumed/woken runs use the id-less fallback (an `end` closes the oldest open call of the same name). I checked whether that reopens S2R3-1 there and it does not: the `[bash, read]` shape survives, and a three-shell batch with id-less `end`s keeps the shelter, because starts and ends balance per tool name so the set's cardinality stays right; only `lastEnded`/anchor identity can land on a sibling. Recorded so a later round does not have to re-derive it. - **O-3 (nit, judgement).** The shared shelter head line still reads `not a silence verdict: … reset this clock on every tick of that stretch`. In seat 1's probe-E shape a challenge *was* issued during a gap; the sentence is literally true of the stretch it measures and, unlike the round-3 wording, no longer claims the threshold "never applied" — but a reader could still hear "the silence path never ran". Not a blocker; noted so the residual ambiguity is on the record rather than rediscovered. ## Exposure line **Read:** the delta diff `git diff 00dc9721 e5e4d70` (6 files, +548/−98) in a detached worktree at the head; the three round-3 verdict comments; `pi-agent-core@0.80.3` `dist/types.d.ts` and `dist/agent-loop.js`; `stall-watchdog.ts` (header, `InFlightCall`/ `InFlightState`, `describeChildEvidence`, the tick's shelter branch, `formatStallNotice`); both `agent-runner.ts` subscriptions; `agent-manager.ts` `turnWiring`; README's two `stallWatchdog` tables and the live-child subsection. **Ran,** in `/tmp` against a copy of the head's tree: my own 7-case harness plus two extra probes (id-less three-shell batch; anchor change with two overlapping shells, including rendering both notices), the two mutations above, one full `vitest run`, one standalone `watch-renderer.test.ts` ×3, one standalone `stall-watchdog-live-child.test.ts`, and `tsc --noEmit`; plus `forgejo_ci_status` for the head sha. No write to the PR branch, no write to any repository; this comment is my only write.

ld-reviewer commented 2026-11-07 01:39:28 +00:00

Collaborator

Copy link

Closure of record — ld-reviewer — head e5e4d702b6

model: omniroute/deepseek-flash
session: 01a1b39f-973c-70d7-89c1-cc127d81aa73

Closure seat for the findings the deployed lock still carries open at this head. I verified every one of them myself before closing anything: a copy of this head's pi-swarm-suite in /tmp, its own StallWatchdog driven through its own exported deps, my own fake probe, my own event orders (with toolCallId and without it) and my own one-tick-per-simulated-minute schedule. I did not run the PR's own cases as evidence, and I took no number from an earlier seat. Reviewer only: I edited nothing, committed nothing, pushed nothing, merged nothing — this comment is my only write.

What I measured

Raised in review 22752 (S2R2-1, round 2): "the shelter clock is per-RECORD and carries across a call boundary". I drove that shape exactly: one bash call opens at record minute 0 with its own live child and holds through minute 89, then the call ends and a second bash call opens (its own child, zero minutes old) with no tick landing in the sub-sweep gap between them. At this head there is no challenge and no reap at record minutes 90 to 95; the second call gets its own long-wait notice at record minute 150 (61 minutes into its own life) and its own ceiling reap at record minute 180 (91 minutes into it). Nothing reaps while the second call is under its own ceiling. The clock belongs to the call, which is what the finding asked for. The finding S2R2-1, which this lock keys as S2, is resolved on head e5e4d702b6.

Raised in review 22773 (S2R3-1, round 3): "a sibling tool call's end clears the shelter of a still-in-flight shell call". Two batch shapes, both driven in pi's real parallel order (every start emitted before the first end). (a) [bash(long, child alive), read]: the read's end lands at minute 2, and every tick from minute 3 to minute 25 reports the working record as sheltered, with no challenge, no reap, no abort and the record still running — the round-3 seat's challenged@13 / reaped@23 does not occur. (b) [bash(long), bash(short)], the shape that produced challenged@21 / REAPED@31: the sibling ends at minute 10 and the shelter holds unchanged through minute 31. The same shape with no toolCallId on any event also holds for 25 minutes, so the fix is not confined to well-formed ids. Taken past the notice bound, the notice my run produced reads Sheltered for: 1h 00m of continuous kernel evidence — not a silence verdict: a live child inside the in-flight tool call reset this clock on every tick of that stretch (ceiling 90m) and carries no Silent for: line and no "already ended" qualifier: the in-flight bash really can claim the child, so the notice is about the call that is still working and not about the sibling that ended. The finding S2R3-1, which this lock keys as S2, is resolved on head e5e4d702b6.

Same review 22773 raised three further findings the lock reports with no id, and they are that one defect seen from other angles: its two repro bullets are the two batch shapes I re-drove in (a) and (b), and its narrative line about a silence-based notice is the notice those reaps used to carry, which I checked above at this head. The evidence that closes the labelled finding closes these with it.

The blocker raised in review 22773 is resolved on head e5e4d702b6.

Raised in review 23032, where the lock reads a finding with no id. That line is the wrapped continuation of a CONTROL sentence in the round-4 seat's measurement — "Control (the other half of the blocker, so per-call end handling is not simply ignored): the shell call's own end still runs the silence path". It alleges no defect: it records that the shelter belongs to a call rather than to a single in-flight flag, which is the correct half of the S2R3-1 fix. I verified that half still holds at this head. A bash call opens with its own live child, the call's own end arrives, and the silence path then runs untouched — challenged 11 minutes later, reaped 10 minutes after that, record stopped, one abort. Nothing in that line is outstanding at this head.

The blocker raised in review 23032 is resolved on head e5e4d702b6.

Why the two labelled findings are cited as S2

The round 2 and round 3 reviews label their findings S2R2-1 and S2R3-1. This lock's finding-id grammar reads both labels as the token S2, so the two closure lines above cite that token, and each of them names the review that raised its finding and pins this head. No comment has been edited: the labels stand exactly as their authors wrote them.

The head's own suite

A full run at this head comes back green, and tsc --noEmit is clean. I ran the suite on a copy of the head's pi-swarm-suite with its lockfile-pinned dependencies symlinked; the results are in the table below rather than taken from the review seats.

Check	Result
Full suite at this head	109 files / 1532 tests, all pass, exit 0 (no flake in this run)
Typecheck (tsc --noEmit)	exit 0

What this comment is not

It is not a new review round and not a re-litigation of the round-4 verdicts on this head, which stand on their own. It closes, by name, exactly the findings the lock still reports open, each after my own reproduction at this head. The head has not moved: this was composed against e5e4d702b6, and it pins that sha throughout.

**Closure of record — ld-reviewer — head e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b** model: omniroute/deepseek-flash session: 01a1b39f-973c-70d7-89c1-cc127d81aa73 Closure seat for the findings the deployed lock still carries open at this head. I verified every one of them myself before closing anything: a copy of this head's `pi-swarm-suite` in `/tmp`, its own `StallWatchdog` driven through its own exported deps, my own fake probe, my own event orders (with `toolCallId` and without it) and my own one-tick-per-simulated-minute schedule. I did not run the PR's own cases as evidence, and I took no number from an earlier seat. Reviewer only: I edited nothing, committed nothing, pushed nothing, merged nothing — this comment is my only write. ## What I measured **Raised in review 22752 (S2R2-1, round 2): "the shelter clock is per-RECORD and carries across a call boundary".** I drove that shape exactly: one bash call opens at record minute 0 with its own live child and holds through minute 89, then the call ends and a second bash call opens (its own child, zero minutes old) with no tick landing in the sub-sweep gap between them. At this head there is no challenge and no reap at record minutes 90 to 95; the second call gets its own long-wait notice at record minute 150 (61 minutes into its own life) and its own ceiling reap at record minute 180 (91 minutes into it). Nothing reaps while the second call is under its own ceiling. The clock belongs to the call, which is what the finding asked for. The finding S2R2-1, which this lock keys as S2, is resolved on head e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b. **Raised in review 22773 (S2R3-1, round 3): "a sibling tool call's `end` clears the shelter of a still-in-flight shell call".** Two batch shapes, both driven in pi's real parallel order (every start emitted before the first end). (a) `[bash(long, child alive), read]`: the read's end lands at minute 2, and every tick from minute 3 to minute 25 reports the working record as sheltered, with no challenge, no reap, no abort and the record still running — the round-3 seat's `challenged@13 / reaped@23` does not occur. (b) `[bash(long), bash(short)]`, the shape that produced `challenged@21 / REAPED@31`: the sibling ends at minute 10 and the shelter holds unchanged through minute 31. The same shape with no `toolCallId` on any event also holds for 25 minutes, so the fix is not confined to well-formed ids. Taken past the notice bound, the notice my run produced reads `Sheltered for: 1h 00m of continuous kernel evidence — not a silence verdict: a live child inside the in-flight tool call reset this clock on every tick of that stretch (ceiling 90m)` and carries no `Silent for:` line and no "already ended" qualifier: the in-flight bash really can claim the child, so the notice is about the call that is still working and not about the sibling that ended. The finding S2R3-1, which this lock keys as S2, is resolved on head e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b. Same review 22773 raised three further findings the lock reports with no id, and they are that one defect seen from other angles: its two repro bullets are the two batch shapes I re-drove in (a) and (b), and its narrative line about a silence-based notice is the notice those reaps used to carry, which I checked above at this head. The evidence that closes the labelled finding closes these with it. The blocker raised in review 22773 is resolved on head e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b. **Raised in review 23032, where the lock reads a finding with no id.** That line is the wrapped continuation of a CONTROL sentence in the round-4 seat's measurement — "Control (the other half of the blocker, so per-call `end` handling is not simply ignored): the shell call's own `end` still runs the silence path". It alleges no defect: it records that the shelter belongs to a call rather than to a single in-flight flag, which is the correct half of the S2R3-1 fix. I verified that half still holds at this head. A bash call opens with its own live child, the call's own end arrives, and the silence path then runs untouched — challenged 11 minutes later, reaped 10 minutes after that, record stopped, one abort. Nothing in that line is outstanding at this head. The blocker raised in review 23032 is resolved on head e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b. ## Why the two labelled findings are cited as S2 The round 2 and round 3 reviews label their findings `S2R2-1` and `S2R3-1`. This lock's finding-id grammar reads both labels as the token S2, so the two closure lines above cite that token, and each of them names the review that raised its finding and pins this head. No comment has been edited: the labels stand exactly as their authors wrote them. ## The head's own suite A full run at this head comes back green, and `tsc --noEmit` is clean. I ran the suite on a copy of the head's `pi-swarm-suite` with its lockfile-pinned dependencies symlinked; the results are in the table below rather than taken from the review seats. | Check | Result | |---|---| | Full suite at this head | 109 files / 1532 tests, all pass, exit 0 (no flake in this run) | | Typecheck (`tsc --noEmit`) | exit 0 | ## What this comment is not It is not a new review round and not a re-litigation of the round-4 verdicts on this head, which stand on their own. It closes, by name, exactly the findings the lock still reports open, each after my own reproduction at this head. The head has not moved: this was composed against e5e4d702b6242c9db1bf0f54c4481014f6fc7b0b, and it pins that sha throughout.

pi-landing merged commit aa6d00972a into main 2026-11-07 01:53:02 +00:00

pi-landing referenced this pull request from a commit 2026-11-07 01:53:04 +00:00

Merge pull request 'HOMELAB-2682: fix(pi-swarm-suite): stall watchdog counts a live child process as activity' (#109) from plane/HOMELAB-2682-watchdog-fix into main

ld-reviewer referenced this pull request 2026-11-07 04:49:20 +00:00

HOMELAB-2682: feat(forgejo-ci): read FORGEJO_API_KEY_FILE first (scoped) #117

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Homelab/pi-extensions!109

No description provided.