Cloonar/coding-lab

Fork 0

Runtime dead-session sweep: end manual runs whose tmux session is gone (generalize readopt's rule to a periodic tick) #93

New issue

Closed

opened 2026-07-10 12:25:45 +02:00 by dominik.polakovics · 1 comment

dominik.polakovics commented

2026-07-10 12:25:45 +02:00

Owner

Context

Decision from the 2026-07-10 grill on #62's design (full decision record on #62). Dead tmux sessions end runs in exactly two places today: reconcile.readopt at startup (EndRun(death, "session gone at startup")) and the AFK reaper at runtime (ActiveAFKRuns only — AFK kinds). A manual run (store.RunKindManual) whose agent process dies at runtime is never reconciled: the row stays active forever, the instance list shows live=false against an active row, and the chat tailer polls the corpse until the next lab restart.

Under #62's liveness design this graduates from cosmetic to correctness: the chosen backstop rule is downgrades only on hard evidence (no staleness timeouts, no transcript inference — they risk spurious "done" flips and, later, spurious notifications). A dead session is the hard evidence, and this sweep is where it gets enforced. Without it, a crash mid-turn pins working forever on both adapters — claude-code's future working-edge marker is never cleared by a Stop hook that will never fire, and codex's rollout already ends on task_started in exactly that case.

The sweep also becomes the single chokepoint a future notification layer hangs a one-shot "run died" event off.

Design

Owner: internal/reconcile — it already owns the death rule (readopt) and the start-race discipline. The periodic tick joins the existing reaper-driven sweep hook (afk.Service already ticks s.sweep on SettingSweepIntervalMinutes) or gets its own ~30–45s ticker in the same family — implementer's choice, but the rule lives in reconcile.
Coverage: active non-AFK runs only (kind = manual). AFK kinds stay exclusively the AFK reaper's: it classifies with tracker context (session gone + PR present = done, not death), and a generic session-gone rule racing it would misclassify finished AFK runs.
Rule: active manual run whose tmux session is gone → EndRun(death, "session gone") (runtime sibling of readopt's pinned startup reason) + publish run-changed, so the chat view flips to ended immediately, the tailer disarms on its next sync, and the spool is GC'd by the existing keep-set sweep.
Start race: startguard ordering — read Snapshot() STRICTLY BEFORE SessionRunner.List (the same discipline readopt and Pass A/B document), so a run between row-insert and tmux-create is never reaped.

Acceptance

Kill a manual run's tmux session out from under lab → within one sweep tick the run is terminal (death, reason "session gone"), run-changed is published, the chat view shows ended, the tailer is disarmed, and the run's spool/settings files are GC'd on the next tailer sync.
AFK runs are untouched by the new sweep (done-vs-death classification remains exclusively the AFK reaper's; a finished AFK run with a merged PR still reaps as done).
No reap of a run mid-Start (startguard snapshot-before-List; covered by a test alongside the existing startguard fixtures).
Startup behavior unchanged (readopt still owns the at-startup pass with its own reason string).

Out of scope

Liveness edges and state derivation (#62).
The notification layer (future — this issue only creates the chokepoint).
Any change to AFK classification.

Sequencing

Independently landable now; useful on its own (fixes the zombie-row gap). #62's implementation phase is blocked on this (it is the hard-evidence half of the backstop).

## Context Decision from the 2026-07-10 grill on #62's design (full decision record on #62). Dead tmux sessions end runs in exactly two places today: `reconcile.readopt` at startup (`EndRun(death, "session gone at startup")`) and the AFK reaper at runtime (`ActiveAFKRuns` only — AFK kinds). A **manual** run (`store.RunKindManual`) whose agent process dies at runtime is never reconciled: the row stays active forever, the instance list shows `live=false` against an active row, and the chat tailer polls the corpse until the next lab restart. Under #62's liveness design this graduates from cosmetic to correctness: the chosen backstop rule is **downgrades only on hard evidence** (no staleness timeouts, no transcript inference — they risk spurious "done" flips and, later, spurious notifications). A dead session is the hard evidence, and this sweep is where it gets enforced. Without it, a crash mid-turn pins `working` forever on both adapters — claude-code's future working-edge marker is never cleared by a Stop hook that will never fire, and codex's rollout already ends on `task_started` in exactly that case. The sweep also becomes the single chokepoint a future notification layer hangs a one-shot "run died" event off. ## Design - **Owner: `internal/reconcile`** — it already owns the death rule (`readopt`) and the start-race discipline. The periodic tick joins the existing reaper-driven sweep hook (`afk.Service` already ticks `s.sweep` on `SettingSweepIntervalMinutes`) or gets its own ~30–45s ticker in the same family — implementer's choice, but the rule lives in reconcile. - **Coverage: active non-AFK runs only** (`kind = manual`). AFK kinds stay exclusively the AFK reaper's: it classifies with tracker context (session gone + PR present = `done`, not `death`), and a generic session-gone rule racing it would misclassify finished AFK runs. - **Rule:** active manual run whose tmux session is gone → `EndRun(death, "session gone")` (runtime sibling of readopt's pinned startup reason) + publish run-changed, so the chat view flips to ended immediately, the tailer disarms on its next sync, and the spool is GC'd by the existing keep-set sweep. - **Start race:** startguard ordering — read `Snapshot()` STRICTLY BEFORE `SessionRunner.List` (the same discipline readopt and Pass A/B document), so a run between row-insert and tmux-create is never reaped. ## Acceptance - [ ] Kill a manual run's tmux session out from under lab → within one sweep tick the run is terminal (`death`, reason `"session gone"`), run-changed is published, the chat view shows ended, the tailer is disarmed, and the run's spool/settings files are GC'd on the next tailer sync. - [ ] AFK runs are untouched by the new sweep (done-vs-death classification remains exclusively the AFK reaper's; a finished AFK run with a merged PR still reaps as `done`). - [ ] No reap of a run mid-Start (startguard snapshot-before-List; covered by a test alongside the existing startguard fixtures). - [ ] Startup behavior unchanged (`readopt` still owns the at-startup pass with its own reason string). ## Out of scope - Liveness edges and state derivation (#62). - The notification layer (future — this issue only creates the chokepoint). - Any change to AFK classification. ## Sequencing Independently landable now; useful on its own (fixes the zombie-row gap). #62's implementation phase is blocked on this (it is the hard-evidence half of the backstop).

dominik.polakovics added the

ready-for-agent

bug

labels

2026-07-10 12:25:45 +02:00

dominik.polakovics referenced this issue

2026-07-10 12:26:38 +02:00

Chat: derive "working" from a real liveness signal, not the transcript-tail guess (root cause of #38 + #39) #62

dominik.polakovics referenced this issue

2026-07-10 12:26:51 +02:00

Chat: the Interrupt button does nothing #38

dominik.polakovics referenced this issue

2026-07-10 12:27:05 +02:00

Chat: a running Claude Code workflow (background subagents) reads as needs-input / idle, not working #39

dominik.polakovics referenced this issue from a commit

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

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

dominik.polakovics referenced this issue from a pull request that will close it,

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

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

dominik.polakovics commented

2026-07-10 13:48:47 +02:00

Author

Owner

This was generated by AI while landing a PR.

Landing audit — PR #95 (afk/93) → PASS

Verification signal relied on: Forgejo Actions CI ci / native (pull_request) — success (4m54s, run #100). Not re-run.

Checks:

✅ Open, non-draft; title is Conventional Commits (fix(reconcile): …).
✅ AFK contract: head afk/93, body carries a valid Closes #93.
✅ Mergeable: origin/main is an ancestor of the branch — no conflict.
✅ Diff reviewed against issue intent (5 files, +243/-3):
- sweepDeadSessions reaps only store.RunKindManual active runs whose tmux session is gone → EndRun(death, "session gone") + publishRunChanged; AFK kinds skipped, leaving done-vs-death classification to the AFK reaper.
- Read order is the documented startguard discipline: ActiveRuns (store) → guard.Snapshot() → runner.List (tmux strictly last); union of starting-set + live protects a run mid-Start. Snapshot() returns a fresh slice, so append(starting, live...) is a safe copy.
- Own unthrottled DeadSessionLoop (30s deadSessionTick), nil-guarded in cmd/lab/main.go; distinct reason string from readopt's startup pass. A List error reaps nothing.
✅ Tests: 5 TestSweepDeadSessions_* covering dead-reap/live-kept, AFK-untouched, startguard race, list-failure-safety, run-changed publish — map 1:1 to the issue's acceptance criteria.

Verdict: PASS. Awaiting free-text merge confirmation.

> *This was generated by AI while landing a PR.* **Landing audit — PR #95 (`afk/93`) → PASS** **Verification signal relied on:** Forgejo Actions CI `ci / native (pull_request)` — **success** (4m54s, run #100). Not re-run. **Checks:** - ✅ Open, non-draft; title is Conventional Commits (`fix(reconcile): …`). - ✅ AFK contract: head `afk/93`, body carries a valid `Closes #93`. - ✅ Mergeable: `origin/main` is an ancestor of the branch — no conflict. - ✅ Diff reviewed against issue intent (5 files, +243/-3): - `sweepDeadSessions` reaps only `store.RunKindManual` active runs whose tmux session is gone → `EndRun(death, "session gone")` + `publishRunChanged`; AFK kinds skipped, leaving done-vs-death classification to the AFK reaper. - Read order is the documented startguard discipline: `ActiveRuns` (store) → `guard.Snapshot()` → `runner.List` (tmux strictly last); union of starting-set + live protects a run mid-Start. `Snapshot()` returns a fresh slice, so `append(starting, live...)` is a safe copy. - Own unthrottled `DeadSessionLoop` (30s `deadSessionTick`), nil-guarded in `cmd/lab/main.go`; distinct reason string from readopt's startup pass. A `List` error reaps nothing. - ✅ Tests: 5 `TestSweepDeadSessions_*` covering dead-reap/live-kept, AFK-untouched, startguard race, list-failure-safety, run-changed publish — map 1:1 to the issue's acceptance criteria. **Verdict: PASS.** Awaiting free-text merge confirmation.

dominik.polakovics closed this issue

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

dominik.polakovics referenced this issue from a commit

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

Merge pull request 'fix(reconcile): runtime dead-session sweep — end manual runs whose tmux session is gone (#93)' (#95) from afk/93 into main

dominik.polakovics referenced this issue

2026-07-10 14:08:16 +02:00

Interactive codex spawn never connects: rollout is born at first turn, queued first message deadlocks on the transcript gate #96

dominik.polakovics referenced this issue

2026-07-10 14:08:47 +02:00

Interactive codex spawn never connects: rollout is born at first turn, queued first message deadlocks on the transcript gate #96