Bug report: plan-mode restart, pgid-reuse race, schedule duplicate-reminder window

[nkbeast]

Found during a code audit of master (47f9438). Reporting here per CONTRIBUTING.md (no external PRs).

All findings below were verified by reading the current source.

---

1. Plan-mode pending selection is held only in-memory and is lost on restart

File: packages/plan/plan-mode/src/index.ts:195

private readonly pendingIntents = new WeakMap<Session, { active: boolean; narrate: boolean }>()

The "latest selection per session awaiting the next accepted in-turn pre-step" lives in a process-local WeakMap. The projection is a pure fold over durable session events, but the pending intent (used to narrate/apply the selection at the next pre-step, index.ts:210-219) does not survive a process restart. After a crash/restart the session still shows the recorded selection in the UI while the runtime no longer applies or narrates it at the next step — the two views silently diverge.

Suggested fix: persist the pending intent with the session (e.g., alongside the plan-mode event it is waiting on) and rehydrate it at startup, or derive the pending intent purely from the durable events.

---

2. Process-group-ID reuse race in the kill escalation ladder

File: packages/subprocess/subprocess-local/src/spawn.ts:452 (graceTimer → kill('SIGKILL')), :381-410 (treeAlive), :260-267 (killGroup)

Termination signals the process group via process.kill(-pid, sig) where pid is also the pgid. The liveness guard only checks group liveness, never group identity. If the whole tree exits and the OS reallocates that pgid to a different, unrelated process group within the graceMs window, SIGTERM/SIGKILL is delivered to the innocent group. Mitigations exist (ESRCH probe, zombie-only scan, bounded timer) and the terminal path already uses a stronger pid+start-time identity check (process-inspector.ts:242-244) — but the group-kill path does not. Low likelihood, high blast radius on busy long-lived hosts.

Suggested fix: use pidfd (Linux) to bind termination to process identity instead of a reusable integer, or at least verify the group identity (e.g., read /proc/<pid>/stat pgrp + starttime) immediately before signalling.

---

3. Schedule reminder can duplicate after a crash in the delivery window

File: packages/schedule/schedule/src/runtime.ts:275 (the try/catch around this.agent.followup(message))

The one-shot reminder path: decide() returns a fired record → followup(message) is sent → the consumed state is persisted afterwards. A crash between delivery and persistence (or a throw inside the framing/followup window after the decision was computed) causes the reminder to be re-decided on restart and delivered again. The try/catch swallows framing failures but returns false, and nothing records the record as consumed on that path.

Suggested fix: persist the consumed/claimed state (or the followup outcome) atomically with the decision, and treat a failed delivery as a distinct state so restart does not replay.

---

4. Note: web_fetch SSRF primitive (already tracked as deferred)

packages/web/web-fetch-http/src/policy.ts:24-41 performs no private/loopback/link-local address validation on the initial request (only protocol + credentials + same-origin redirects). This is explicitly documented as deferred in the package note (policy.ts:18) and the tool is not mounted in shipped rosters — listing it here only as a heads-up that the known primitive is still open if any future default roster mounts it.