[bug] Nothing reaps an idle coding session now — #271 removed the only thing that did, and the one reaper needs a human to open a tab

[serge-ivo]

Follow-up to #271, which was a good fix — this is the consequence it left open.

What changed

Before #271, coding-session.ts ended the session after every Pilot run. That was the bug (it made
delegation single-use), and the fix is right: a run now closes only the session it opened
(shouldEndSessionAfterRun({ openedByRun })).

But ending-every-run was also, accidentally, the only thing that reaped sessions. Nothing replaced
it with a deliberate policy.

What now ends a coding session

Path	Trigger
routes/coding.ts:1384	the user clicks Kill
coding-session.ts:468	a run closes a session it opened
reconcileOrphanedSessions	an active session the runner is not tracking, after a 3-min grace

STALE_DRIVER_MS (15 min) expires the driver claim, not the session — different thing.

There is no idle timeout. A session opened by a delegated run and then left alone stays active
indefinitely, with a live claude --dangerously-skip-permissions child process on the user's
machine.

And the one reaper is opportunistic, not scheduled

reconcileOrphanedSessions has exactly one caller — routes/coding.ts:1506, inside the sessions
listing. It runs when somebody loads the console. The cron (index.ts:206-217) runs
runDueTriggers, runDueDeliveries and runStaleRunSweep; sessions are not swept.

So for a user who delegates via chat/MCP and never opens the Coding tab — exactly the workflow #271
was built to enable — nothing reaps anything.

Why it matters

• A persistent LLM CLI process per repo. Four repo coders under one Lead = four long-lived
  Claude Code processes, each holding a session and accumulating context, indefinitely.
• Stale active rows. After a runner restart the rows stay active until someone opens the
  console; until then ensureActiveSession re-attaches to them (idempotent, so it mostly works) and
  connectivity reporting shows sessions that are not really there.
• It is bounded but not small. One-active-session-per-repo caps the count at one per repo, so
  this is not unbounded growth — it is a permanent resident per repo.

Suggested shape

Decide the policy explicitly rather than inheriting the absence of one:

• An idle timeout on sessions (last driver activity / last capture older than N), reaped by the
  cron alongside runStaleRunSweep — which already exists for precisely this class ("a row stuck at
  running forever tells every supervisor its subordinate is still working").
• Move reconcileOrphanedSessions onto that cron too, so orphan detection does not depend on a
  human opening a tab.
• If long-lived sessions are wanted (warm engine = faster delegation, which is a real benefit),
  then say so and make the idle window generous — but make it a decision, with the process cost
  visible in the session view.

Also: the two new modules from #271 have no tests

• lib/coding-session-open.ts — ensureActiveSession + startSessionOnRunner: the create/reuse/
  race path, and the one that decides opened. No test file.
• lib/instance-connectivity.ts — runtimeConnectivity / runtimeConnectivityMany, now shared by
  the driver and supervision. No test file.

The pure classifiers are covered (subordinate-connectivity 7, coding-session-lifecycle 7). The
untested part is the orchestration, and specifically opened — get that wrong in either direction
and you either leak every session or destroy a session a human opened, which is #271 again from the
other side. The lost-race branch (catch → reuse winner → opened: false) is exactly the kind of
path that regresses silently.

[serge-ivo]

Fixed in b0b3a74 (CI + Deploy API Worker green; migration 0082 applied).

Verified at HEAD first. Your table was right, with one correction: reconcileOrphanedSessions'
single caller is /coding/diagnostics — the 🩺 Sessions panel — not the sessions listing. So it
was even more opportunistic than reported: it needed a human to open one specific panel.

Also confirmed: no existing column could answer "is anyone using this session". updated_at moves on
claim/reassign/end/suspend (lifecycle, not use — a session driven hard for an hour can have an
hour-old updated_at), touchSessionDriver writes only driver_at, and started_at is fixed.
That is why there was no idle timeout to tune — there was nothing to measure.

The policy

A session is closed after six hours with NOTHING touching it. Idle means no capture poll, no
message, no run, no Co-pilot turn, no timeline read — migration 0082 adds last_activity_at,
written by every one of those paths and throttled to one row a minute, so the 3s poll costs zero
rows written on 19 polls out of 20.

Idle deliberately does not mean "no work in flight". That is the whole safety of it: a Pilot
mid-run captures every 2s and a human watching the terminal polls every 3s, so anything anyone is
actually looking at keeps itself alive by being looked at. Making the signal interaction rather
than a lifecycle timestamp is what makes reaping safe.

Six hours, and it is a decision. You named the tradeoff correctly — a warm engine is a real
benefit — so the window is set by the longest legitimate silence rather than tidiness: longer than
any within-day break (lunch, a long meeting), shorter than a night. A session abandoned at the end
of the day is reclaimed before morning; one you left at noon is still there when you get back. The
exposed case is a human who walked away from a backgrounded console tab, which stops polling
(#272) — six hours covers that. And the errors are asymmetric: reaping late costs an idle process,
bounded at one per repo; reaping early destroys a CLI context that cannot be recovered, because the
next session gets a new id and starts cold.

Who opened it does not change the answer. Ownership decides who may close a session during a
run (#271's rule, untouched). Afterwards an unattended engine costs the same whoever started it —
and the one argument for short-cycling run-opened sessions cuts the other way: #271's own reasoning
is that "a failed run is the worst possible moment to take the session away — that is exactly when
they want to look at the terminal". That applies with more force to a run-opened session, not less.
So: one window, no opened_by column, no second weaker notion of ownership sitting next to the
run-scoped one.

The reaper stops the engine, not just the row. /coding/end on the machine first, then
endSession. Closing only D1 would tidy the database and leave the child process resident, which is
the leak. When the machine is gone there is no process to stop and the row still gets closed. Each
reaped session gets a timeline line saying why, so it does not just vanish.

Orphan reconcile on the cron

Moved next to runStaleRunSweep, as you suggested. One new hazard the route did not have:
reconcileOrphanedSessions ends every active session not in the set it is given, so a runner
reply missing tracked (older build, partial reply, shape change) would mean "end everything" —
once, if you opened the panel; every minute forever, on a cron. So trackedSessionIds returns
null for an unrecognisable answer and the sweep skips; an empty array is a real answer
("I hold nothing") and still reconciles. Both pinned in tests.

It is also gated: only instances whose sessions have been quiet 5+ minutes, and only where a runner
is actually live — "not tracked" carries no information when nothing is there to track.

STALE_DRIVER_MS untouched. It is in the reap predicate only as an extra guard: the claim must be
as stale as the session (6h, not 15min) before either is reaped.

Tests

• coding-session-open.test.ts — new. opened in every direction: reuse → false, create → true,
  lost race → false (the branch you flagged), plus the re-attach, the "engine never launched →
  close it and report why" path, and machine-switch relocation.
• coding-session-sweeper.test.ts — the null-vs-empty distinction above, runner-offline skip,
  engine-stopped-before-row, and the cutoffs.
• coding-store.test.ts — the throttle predicate, the driver_at guard, the updated_at fallback.

instance-connectivity.ts still has no dedicated test file; its behaviour is covered indirectly
through supervision.test.ts (which now exercises the live probe end-to-end). Worth its own file —
left out of this change to keep it to the two bugs.