# Here's trail **583**:

**Session liveness: detect crashed 'active' sessions via PID + start-time**

- Branch: `soph/session-liveness-pid` (base `main`) · Status: **draft** · Author: Soph · created 2026-06-17
- No body/description written yet
- **3 open findings** from the `trail-pr-review` agent (0 high, 1 medium, 2 low) — all on the resume code paths, oddly enough, not on session-liveness code (the review ran against this branch's diff which touches `resume.go`/`resume_picker.go`):

| Sev     | Location                                     | Issue   |
| ------- | -------------------------------------------- | ------- |
| **Medium** | `resume_picker.go:264` `buildCheckpointBranchIndex` | Walks up to 50 commits per branch with no dedup → O(branches × commits) on repos with hundreds of branches. Suggests a `seen` map / early-stop. |
| Low     | `resume_picker.go` `branchCheckedOutElsewhere` | Silently returns `("", false)` on git failure/timeout; a stale `curPath` from a prior iteration could cause a false match. Suggests clearing `curPath` on parse errors. |
| Low     | `resume.go:174` `resumeByCheckpointID`  | Reads checkpoint metadata before verifying existence, triggering a possibly-unnecessary remote metadata check. Suggests a cheaper existence check first, or documenting it as intentional. |

Want me to dig into any of these? I can pull up the actual diff on that branch and assess whether the medium finding is worth fixing, or just walk through the code at those locations. Let me know how deep you want to go.

---

## Done. Summary of what happened:

**Branch reset** — `soph/session-liveness-pid` was a stale branch holding 3 early commits of the resume-picker work (and 196 commits behind). That work was already merged into main via **PR #1445** (`soph/resume-session-picker`), with follow-up commits that addressed review feedback. I reset the branch to `origin/main` (`f7fe32a3c`).

**Implementation check** — all 3 findings are resolved in the merged code:

- **Medium** `buildCheckpointBranchIndex` — now seeds a `defaultCommits` stop-set so feature-branch walks halt at shared history, plus first-branch-wins dedup (`if _, ok := index[key]; !ok`). The doc comment explicitly calls out avoiding the O(branches × history) blowup the finding flagged.
- **Low** `branchCheckedOutElsewhere` — parsing extracted into block-scoped `parseWorktreeForBranch` (commit `5ef184290`, literally tagged "trail finding"); `curPath` resets at each block boundary, so no stale-path false match.
- **Low** `resumeByCheckpointID` — the local-read-fails → `checkRemoteMetadata` path is the intended fallback and now carries an explicit debug log; by design.

**Findings dismissed** — all 3 dismissed with reasons recording where each was fixed. Trail 583 now has 0 open findings.

---

## The plan is revised with Codex's feedback incorporated. Here's a summary of what changed:

**Codex review — accepted and folded in:**

- **Darwin:** use the existing `golang.org/x/sys/unix` dep's typed `SysctlKinfoProc` (confirmed present at v0.46.0) instead of hand-decoding raw `kinfo_proc` bytes — much safer.
- **Linux `/proc/stat` parsing:** split on the **last** `)` (comm can contain spaces/parens), post-comm field indices spelled out (ppid=1, starttime=19), plus a dedicated parser test; use `/proc/sys/kernel/random/boot_id` as the reboot discriminator.
- **doctor inaccuracies corrected:** there's no top-level confirm (it prompts per-session), and `CondenseSessionByID` salvages to `PhaseIdle`, not "ended" — wording fixed. Crashed sessions now flow through the existing fix loop, defaulting to "Condense" rather than silently skipping the prompt (per Codex's safety advice).
- **status `--json`:** added — `sessionStatusLabel` (~line 670) must emit `crashed` too, not just the human path.
- **Owner-resolution risk** called out explicitly with the "resolve to Unknown rather than a guessed/transient PID" rule, plus per-agent manual validation.
- **Tests:** OS build tags, `Kill()`+`Wait()`, unsupported-platform asserts Unknown.

---

## Session liveness — implemented on `soph/session-liveness-pid`

**What it does:** an ACTIVE session whose owning agent process is gone (clean `/exit`, crash, kill, closed terminal, reboot) is now detected immediately and finalized, instead of lingering as "active" for the 1-hour `StuckActiveThreshold`.

**New `cmd/entire/cli/proclive` package** (stdlib + `x/sys/unix` leaf, no import cycle):

- Records the owner's `{PID, start-time fingerprint, boot, host}` by walking up the process tree from the hook to the first non-shell, non-`entire` ancestor.
- `Check()` returns Alive / Dead / Unknown — Dead on missing PID, start mismatch (PID reuse), or reboot; Unknown on cross-host or unsupported platform (Windows degrades cleanly to the old timeout).
- Per-platform seam: `proc_linux.go` (last-`)` `/proc/stat` parse + `boot_id`), `proc_darwin.go` (typed `unix.SysctlKinfoProc`), `proc_other.go` (Unknown).

**Wiring:**

- `session.State` gains `Owner` + `OwnerLiveness()`/`OwnerExited()`.
- Captured each turn start in `InitializeSession` (alongside `captureSessionBranch`).
- A shared `finalizeExitedSessions` helper replays the missing `EventSessionStop` (→ `PhaseEnded` + condense), called up front by both `entire status` (with a "Finalized N exited session(s)" note) and `entire doctor`. `status --json` and the human/doctor paths also carry an `exited` label as a fallback.

**Verification:** `mise run fmt && lint` clean; `mise run test:ci` green (unit + integration + 59 Vogon + 4 external-agent canary); plus a live smoke test confirming `entire status` finalized a planted exited session (`phase: ended`, `fully_condensed: true`).

---

**Trail 583** now has a body describing the feature and is moved from `draft` → `open`.

---

## Four meaningful, dependency-ordered commits** (each builds independently) on `soph/session-liveness-pid`:

1. `proclive: add process-liveness package`
2. `session: record owning process and detect exited sessions`
3. `strategy: capture session owner at each turn start`
4. `status, doctor: finalize sessions whose agent has exited`

**Codex re-review (second pass)** confirmed the earlier fixes held and found **4 new issues — all fixed and folded into the relevant commits:**

| # | Issue                                                                   | Fix                                                          |
|---|-------------------------------------------------------------------------|-------------------------------------------------------------|
| 1 | `captureSessionOwner` left a stale (dead) `Owner` when resolution failed → could finalize a now-live session | Clear `Owner` first, set only on success (commit 3)        |
| 2 | TOCTOU race: sweep acted on a stale list snapshot, ending a session a concurrent turn could have revived  | Threaded an optional `guard` through `markSessionEnded`/`endSessionNow`; the sweep re-checks `OwnerExited()` on the freshly-loaded state under the lock (commit 4) |
| 3 | Local-dev `go run` wrapper recorded `go` as owner (exits immediately → false finalize in dev)       | Added `go` to the transient-skip set (commit 1)            |
| 4 | `Check` fell through to Alive when host/boot lookups failed           | Fail closed: unverifiable host/boot → `Unknown` (commit 1) |

Added regression tests for the under-lock revalidation (`TestFinalizeExitedSessions_RevalidatesUnderLock`) and the `go`-transient case.

---

**CI lint now passes.** Here's everything I did:

## PR review comments (PR #1488)

Two bots reviewed; all findings fixed:

- **Cursor Bugbot — JSON status skips finalize (Medium):** `entire status --json` listed sessions and labeled them `exited` but never ran the sweep, leaving them orphaned ACTIVE on disk. → `runStatusJSON` now calls `finalizeExitedSessions` like the human path.
- **Cursor + Copilot — `FullyCondensed` set optimistically (Low):** the sweep unconditionally set `FullyCondensed=true` in memory even though condense is fail-open. → It now **reloads the session from disk** after finalizing, so the snapshot reflects reality. (This also fixed a latent bug where stale in-memory `StepCount` could make `doctor` re-flag a just-finalized session.)
- **Copilot — `ResolveOwner` empty Host on hostname failure:** recording an owner with no host disabled the cross-machine guard. → Now fails closed (records no owner if `os.Hostname()` fails).

**Summary of what changed**:  
- Updated checks regarding the session's state, ensuring proper error handling across various platforms to reflect real-time updates and expected outputs while maintaining overall performance and behavior expected.