fix(reconcile): runtime dead-session sweep — end manual runs whose tmux session is gone (#93) #95

Merged

dominik.polakovics merged 1 commit from afk/93 into main

2026-07-10 13:52:06 +02:00

dominik.polakovics commented

2026-07-10 13:41:46 +02:00

Owner

Closes #93

What

Generalizes readopt's death rule to a periodic runtime tick, owned by internal/reconcile: every 30s, any active manual run (store.RunKindManual) whose tmux session is gone gets EndRun(death, "session gone") and a run.changed publish. This closes the zombie-row gap — previously a manual run whose agent died at runtime stayed active forever (instance list showed live=false against an active row; the chat tailer polled the corpse until the next lab restart). It is also the hard-evidence half of #62's backstop and the future notification layer's "run died" chokepoint.

Design (per the issue)

Rule in reconcile (sweepDeadSessions, internal/reconcile/sweep.go): the runtime sibling of readopt, with a distinct pinned reason string ("session gone" vs readopt's "session gone at startup" — startup behavior unchanged).
Start-race discipline: store ActiveRuns first (a row inserted later is invisible), guard.Snapshot() strictly before runner.List, reap only if the session is in neither set. Verified against instance.Launch ordering (Mark at launch.go:78 → CreateRun at :169 → tmux Start at :222 → deferred Clear), the only production CreateRun site.
AFK kinds excluded: done-vs-death classification stays exclusively the AFK reaper's (session gone + PR present = done, not death).
Cadence: of the issue's two sanctioned options, the sweep gets its own ticker (DeadSessionLoop, 30s, sweepTick family) rather than riding the reaper's sweep_interval_minutes hook — that hook is throttled to 10 minutes by default, and a dead run should flip to ended within one short tick. No new settings key; fixed constant matches the existing sweepTick idiom. Wired in cmd/lab/main.go beside the reaper/scheduler loops, nil-guarded for degraded mode.
A List error reaps nothing (never misread as "everything is dead"); a missing tmux server already reads as zero sessions in tmuxx, which is hard evidence.
Tailer disarm, spool GC, and credential keep-set cleanup are existing downstream behaviors of run-end + run.changed — nothing re-implemented here.

Acceptance mapping

Dead manual run → terminal death/"session gone" + run.changed within one tick: TestSweepDeadSessions_deadManualReapedLiveManualKept, TestSweepDeadSessions_publishesRunChanged (tailer disarm/spool GC ride the existing run.changed sync, internal/chat/tailer.go).
AFK runs untouched: TestSweepDeadSessions_afkKindsUntouched (reaper classification code untouched by this PR).
No reap mid-Start: TestSweepDeadSessions_startguardRace (Mark → not reaped; Clear → reaped), beside the existing startguard fixtures.
Startup unchanged: readopt untouched; existing readopt tests pass; reason strings asserted distinct.

Verification

gofmt -l clean, go build ./..., go vet ./..., go test ./... — all green except the pre-existing internal/tmuxx real-tmux integration failures, which fail identically on a clean tree in this sandbox (no working pane shell) and are unrelated.

🤖 Generated with Claude Code

https://claude.ai/code/session_014MZmEYoNVSCQGvKawAzGf3

Closes #93 ## What Generalizes readopt's death rule to a periodic runtime tick, owned by `internal/reconcile`: every 30s, any active **manual** run (`store.RunKindManual`) whose tmux session is gone gets `EndRun(death, "session gone")` and a `run.changed` publish. This closes the zombie-row gap — previously a manual run whose agent died at runtime stayed active forever (instance list showed `live=false` against an active row; the chat tailer polled the corpse until the next lab restart). It is also the hard-evidence half of #62's backstop and the future notification layer's "run died" chokepoint. ## Design (per the issue) - **Rule in reconcile** (`sweepDeadSessions`, `internal/reconcile/sweep.go`): the runtime sibling of readopt, with a distinct pinned reason string (`"session gone"` vs readopt's `"session gone at startup"` — startup behavior unchanged). - **Start-race discipline**: store `ActiveRuns` first (a row inserted later is invisible), `guard.Snapshot()` strictly before `runner.List`, reap only if the session is in neither set. Verified against `instance.Launch` ordering (Mark at `launch.go:78` → `CreateRun` at `:169` → tmux `Start` at `:222` → deferred Clear), the only production `CreateRun` site. - **AFK kinds excluded**: done-vs-death classification stays exclusively the AFK reaper's (session gone + PR present = done, not death). - **Cadence**: of the issue's two sanctioned options, the sweep gets its own ticker (`DeadSessionLoop`, 30s, `sweepTick` family) rather than riding the reaper's `sweep_interval_minutes` hook — that hook is throttled to 10 minutes by default, and a dead run should flip to ended within one short tick. No new settings key; fixed constant matches the existing `sweepTick` idiom. Wired in `cmd/lab/main.go` beside the reaper/scheduler loops, nil-guarded for degraded mode. - **A `List` error reaps nothing** (never misread as "everything is dead"); a missing tmux server already reads as zero sessions in `tmuxx`, which *is* hard evidence. - Tailer disarm, spool GC, and credential keep-set cleanup are existing downstream behaviors of run-end + `run.changed` — nothing re-implemented here. ## Acceptance mapping - Dead manual run → terminal `death`/`"session gone"` + `run.changed` within one tick: `TestSweepDeadSessions_deadManualReapedLiveManualKept`, `TestSweepDeadSessions_publishesRunChanged` (tailer disarm/spool GC ride the existing `run.changed` sync, `internal/chat/tailer.go`). - AFK runs untouched: `TestSweepDeadSessions_afkKindsUntouched` (reaper classification code untouched by this PR). - No reap mid-Start: `TestSweepDeadSessions_startguardRace` (Mark → not reaped; Clear → reaped), beside the existing startguard fixtures. - Startup unchanged: readopt untouched; existing readopt tests pass; reason strings asserted distinct. ## Verification `gofmt -l` clean, `go build ./...`, `go vet ./...`, `go test ./...` — all green except the pre-existing `internal/tmuxx` real-tmux integration failures, which fail identically on a clean tree in this sandbox (no working pane shell) and are unrelated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_014MZmEYoNVSCQGvKawAzGf3

dominik.polakovics added 1 commit

2026-07-10 13:41:46 +02:00

fix(reconcile): runtime dead-session sweep ends manual runs whose tmux session is gone

ci / native (pull_request) Successful in 4m54s

Details

5707496a20

Generalize readopt's death rule to a periodic tick (issue #93). A manual
run whose agent process died at runtime was never reconciled: the row
stayed active forever, the instance list showed live=false against an
active row, and the chat tailer polled the corpse until the next restart.
Under #62's liveness design a dead session is the hard-evidence backstop,
so this graduates from cosmetic to correctness.

- sweepDeadSessions (internal/reconcile): active RunKindManual runs whose
  session is in neither guard.Snapshot() nor runner.List get
  EndRun(death, "session gone") + run.changed. Read order is pinned to
  the startguard discipline: store ActiveRuns first, Snapshot() strictly
  before List, so a run mid-Start is never reaped.
- AFK kinds are untouched: done-vs-death classification stays exclusively
  the AFK reaper's (session gone + PR present = done, not death).
- DeadSessionLoop: own 30s ticker, deliberately not the throttled
  sweep_interval_minutes cadence — a dead run flips to ended within one
  short tick. Startup stays readopt's with its distinct reason string.
- Tailer disarm, spool GC, and credential keep-set cleanup are existing
  downstream behaviors of run-end + run.changed; nothing re-implemented.

Closes #93

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014MZmEYoNVSCQGvKawAzGf3