Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .ai/contexts/cli-session-state.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,17 @@ session. Both `seed()` and `handleFile()` apply this gate before writing to
`statusBySession`; `handleFile()` also deletes the entry outright once the
liveness check fails, same as it does when the file itself disappears.

**That gate only runs on a file event — a CLI killed without a clean exit
writes no such event, so its last status stayed cached forever until this app
restarted (F2, audit-fable-2026-09-11).** `getStatus(sessionId)` now keeps the
`pid` alongside the cached `{status, statusUpdatedAt}` and re-probes
`isProcessAlive(pid)` itself, lazily, throttled to once per
`GET_STATUS_PROBE_THROTTLE_MS` (5 s) per sessionId (`now()` is injected so
tests use a fake clock instead of real delays) — a dead pid deletes the entry
and the call returns `undefined`, same as a file event would have done. This
still never touches disk and never arms `onIdle` — "the one invariant" above
is unchanged, it is a read-path liveness check, not a new trigger.

## Canary tests

`test/canary-*.test.js` is a convention this module introduces. A canary
Expand Down
72 changes: 71 additions & 1 deletion .ai/contexts/session-cache.md
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,24 @@ untouched.
plain array to `{ files, sessions }`; every stub of `transport.listFiles`
across `remote-mirror.test.js`, `remote-index.test.js` and
`remote-indexing-e2e.test.js` was updated to match in the same change.
- **A descriptor's pid is checked for liveness on the host itself (F9,
audit-fable-2026-09-11)** — until this fix `parseSessions` kept every
descriptor unconditionally, so a killed remote CLI's file (deleted only on
a clean exit, same as the local one — see `.ai/contexts/cli-session-state.md`)
surfaced as permanently live, and `sidebar.js`'s host-dot `liveCount`
counted it. `LIST_COMMAND`'s per-file loop now emits one more line after
each descriptor: `printf '\002ALIVE:%s\n' "$( [ -d "/proc/$pid" ] && echo
1 || echo 0 )"`, `$pid` taken from the filename via `basename "$f" .json`
— no second ssh round trip. `parseSessions` matches that exact
`ALIVE:0`/`ALIVE:1` line immediately following a descriptor,
consumes it either way (so it can never itself be mis-parsed as a bogus
descriptor), drops the descriptor on `ALIVE:0`, and counts the drops in
its returned `dropped` field; `listFiles` logs `dropped N dead session
descriptor(s)` when non-zero. **Backward compatible by construction, not
by a version check**: a descriptor with no marker line following it (an
older host script, or simply the last line of the block) is kept exactly
as before — the absence of the marker is the compatibility signal, there
is no protocol version field.
- **`remote-index.js` keeps the latest descriptors per alias, keyed and
pruned exactly like folder keys.** `createRemoteIndexer()`'s private
`remoteSessions` map is set from `result.sessions` inside `refreshHost()`
Expand Down Expand Up @@ -659,6 +677,58 @@ Launching a new remote session (#222) and injection over the messaging socket
a no-op on real data; it only changes routing for a hand-built or malformed
descriptor.

- **Pid-reuse guard (F6, audit-fable-2026-09-11).** Descriptors now survive a
failed refresh cycle for up to the backoff window (#255, above) — long
enough for the CLI to die and the OS to hand its pid to an unrelated
process on the same host. Before this fix, `buildProbeCommand` read
*whatever* process now holds that pid's `TMUX` environment variable and, if
it was solo, reconfigured and attached to whatever tmux session that
process happened to be in — no check that it was still the session the
descriptor named. `buildProbeCommand` now appends one more
`PROBE_SEP`-delimited segment: `tr '\0' ' ' < /proc/<pid>/cmdline | grep -qi
claude && echo 1 || echo 0` (`buildProcCmdlineCheck`), read back by
`parseDiscoveryProbeOutput` as `cmdlineHasClaude` (`true`/`false`, or `null`
for a probe predating this segment). `attach()` runs this check only when
`descriptor.procStart != null`; on a `false` result it returns `{ ok:
false, error: 'pid <n> no longer belongs to this session (process start
differs)' }` before any `spawnPty` call — no attach, no `set`. A descriptor
with no `procStart` keeps today's unverified behavior.
**Not what the finding asked for, and why:** the audit wanted
`descriptor.procStart` compared against `/proc/<pid>/stat`'s starttime
(field 22, clock ticks since boot). The one measured `procStart` sample
this repo has (`.ai/contexts/cli-session-state.md`) is
`"134319945380279381"`, captured on a **Windows** CLI — 18 digits, the
right order of magnitude for a Windows `FILETIME` (100 ns since 1601), not
for Linux clock ticks since boot (which would need centuries of uptime to
reach 18 digits at 100 Hz). Whether the CLI's own remote/Linux code path
produces something on the *same* scale as `/proc/<pid>/stat` field 22 is
unverified — ssh access to a real host to check was out of scope for this
fix (`no ssh to any host` was a hard constraint). Comparing two values on
possibly-incompatible scales risks shipping a check that either always
refuses (units never line up) or silently never refuses (units happen to
overlap by coincidence) — worse than the cmdline check in both directions.
The `cmdline`-contains-`claude` check is strictly weaker than an exact
start-time match (it would not catch a *second* claude CLI reusing the
pid), but it does catch the audited scenario — pid reused by an unrelated
process in another tmux server — without depending on that unverified
format match. `descriptor.procStart != null` is still the gate, matching
the interface the finding asked for.

- **`ConnectTimeout=5` on the probe and restore-on-detach ssh calls (F10,
audit-fable-2026-09-11).** `defaultRunRemoteCommand`'s ssh spawn had a kill
timer (`DEFAULT_PROBE_TIMEOUT_MS`, 15 s) but no `ConnectTimeout` — a
half-open connection (portable asleep, NAT gone stale) took the full 15 s
to fail instead of failing fast at the TCP handshake. `buildRemoteCommandArgs
(alias, command)` now builds the argv (`-o BatchMode=yes -o
ConnectTimeout=5 -n <alias> <command>`), exported so the argv shape is
tested directly without spawning ssh. **Known, accepted gap: if the app
crashes mid-session, `before-quit`'s `detach()` never runs, so a solo
attach's restore-on-detach ssh call (`status off`/`mouse on`/…) never
fires** — the remote tmux session is left with the solo-attach options set
until something else attaches and detaches cleanly. `ConnectTimeout` bounds
how long a *reachable-but-slow* restore takes; it does nothing for a
restore that never gets scheduled at all.

### `stop()` cancels, `dispose()` ends -- they are not the same thing

`createSshTransport().dispose()` is **terminal**: it sets a flag every later
Expand Down Expand Up @@ -686,7 +756,7 @@ usable" and "dispose() is terminal", in `test/remote-index.test.js`.
- `remote-hosts.test.js` — covers folder-key parsing, alias validation and the `isSafeRelPath` guard
- `remote-mirror.test.js` — covers the inventory diff, the no-op second pull, deletions, and both failure modes, against a fake transport
- `remote-transport.test.js` — covers the ssh/scp argv, inventory parsing, the timeout kill and `dispose()`, with `spawn` injected; also covers `LIST_COMMAND`'s exact text (issue #211's `.key`-exclusion and single-ssh-call pins), `splitListOutput()` and `parseSessions()`
- `remote-transport-shell.test.js` — runs `LIST_COMMAND` through a real `sh -c`, not a fake stdout fixture: a missing `.claude/projects` must exit non-zero, a missing `.claude/sessions` must still exit 0 with the marker present, and a `.key` file plus a directory named like a descriptor must both be excluded from what reaches stdout
- `remote-transport-shell.test.js` — runs `LIST_COMMAND` through a real `sh -c`, not a fake stdout fixture: a missing `.claude/projects` must exit non-zero, a missing `.claude/sessions` must still exit 0 with the marker present, a `.key` file plus a directory named like a descriptor must both be excluded from what reaches stdout, and (F9) the ALIVE marker reflects real `/proc` liveness for both a live pid (the shell's own `$$`, so it reads as alive on any host) and a dead one
- `remote-index.test.js` — covers "no host declared: no timer, no ssh call", the 60 s floor, per-host failure isolation and alias pruning, and that `getRemoteSessions()` is cleared (not left stale) after a cycle whose `sync()` throws
- `remote-indexing-e2e.test.js` — covers the `<alias>::` prefix reaching session rows, the search entries, the metrics and the sidebar
- `dom-sidebar-remote-session.test.js` — covers the remote badge and the read-only click routing
Expand Down
41 changes: 29 additions & 12 deletions cli-session-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,23 @@ const RESCAN_STATUS = 'idle';
const FLUSH_MS = 150;
const MIN_RESCAN_INTERVAL_MS = 1000;
const MAX_SEEDED_FILES = 200;
const GET_STATUS_PROBE_THROTTLE_MS = 5000;

let dir = DEFAULT_DIR;
let activeSessions = null;
let onIdle = null;
let log = null;
let isProcessAlive = defaultIsProcessAlive;
let now = Date.now;

let watcher = null;
let flushTimer = null;
const pending = new Set();
const known = new Map();
const lastRescanAt = new Map();
// sessionId -> { status, statusUpdatedAt } for live pids only -- see .ai/contexts/cli-session-state.md
// sessionId -> { status, statusUpdatedAt, pid } for live pids only -- see .ai/contexts/cli-session-state.md
const statusBySession = new Map();
const lastProbeAt = new Map();

function defaultIsProcessAlive(pid) {
try {
Expand All @@ -42,6 +45,7 @@ function init(ctx) {
onIdle = ctx.onIdle;
log = ctx.log || { info() {}, debug() {}, warn() {}, error() {} };
isProcessAlive = ctx.isProcessAlive || defaultIsProcessAlive;
now = ctx.now || Date.now;
stop();
}

Expand Down Expand Up @@ -77,7 +81,7 @@ function handleFile(name) {
text = fs.readFileSync(path.join(dir, name), 'utf8');
} catch {
const stale = known.get(name);
if (stale && stale.sessionId) statusBySession.delete(stale.sessionId);
if (stale && stale.sessionId) forgetSession(stale.sessionId);
known.delete(name);
return;
}
Expand All @@ -86,12 +90,12 @@ function handleFile(name) {
if (!state) return;

const prev = known.get(name);
if (prev && prev.sessionId && prev.sessionId !== state.sessionId) statusBySession.delete(prev.sessionId);
if (prev && prev.sessionId && prev.sessionId !== state.sessionId) forgetSession(prev.sessionId);
known.set(name, { procStart: state.procStart, status: state.status, sessionId: state.sessionId });
if (isProcessAlive(state.pid)) {
statusBySession.set(state.sessionId, { status: state.status, statusUpdatedAt: state.statusUpdatedAt });
statusBySession.set(state.sessionId, { status: state.status, statusUpdatedAt: state.statusUpdatedAt, pid: state.pid });
} else {
statusBySession.delete(state.sessionId);
forgetSession(state.sessionId);
}

const reused = !!prev && prev.procStart !== state.procStart;
Expand Down Expand Up @@ -137,12 +141,17 @@ function seed() {
if (state) {
known.set(name, { procStart: state.procStart, status: state.status, sessionId: state.sessionId });
if (isProcessAlive(state.pid)) {
statusBySession.set(state.sessionId, { status: state.status, statusUpdatedAt: state.statusUpdatedAt });
statusBySession.set(state.sessionId, { status: state.status, statusUpdatedAt: state.statusUpdatedAt, pid: state.pid });
}
}
}
}

function forgetSession(sessionId) {
statusBySession.delete(sessionId);
lastProbeAt.delete(sessionId);
}

function ensureWatching() {
if (watcher) return true;
if (!onIdle || !activeSessions) return false;
Expand Down Expand Up @@ -186,16 +195,24 @@ function stop() {
known.clear();
statusBySession.clear();
lastRescanAt.clear();
lastProbeAt.clear();
}

/**
* Pure lookup: the last {status, statusUpdatedAt} parsed for `sessionId`, or
* undefined if no state file has ever named it. Never touches disk, never
* arms anything -- see .ai/contexts/cli-session-state.md ("the one invariant").
*/
// Lookup + throttled lazy liveness re-probe -- see .ai/contexts/cli-session-state.md ("the one invariant" still holds: never arms onIdle).
function getStatus(sessionId) {
const entry = statusBySession.get(sessionId);
return entry ? { status: entry.status, statusUpdatedAt: entry.statusUpdatedAt } : undefined;
if (!entry) return undefined;

const t = now();
const last = lastProbeAt.get(sessionId) || 0;
if (t - last >= GET_STATUS_PROBE_THROTTLE_MS) {
lastProbeAt.set(sessionId, t);
if (!isProcessAlive(entry.pid)) {
forgetSession(sessionId);
return undefined;
}
}
return { status: entry.status, statusUpdatedAt: entry.statusUpdatedAt };
}

module.exports = {
Expand Down
6 changes: 1 addition & 5 deletions public/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -809,11 +809,7 @@ function updateRunningIndicators() {
const id = item.dataset.sessionId;
const running = activePtyIds.has(id);
item.classList.toggle('has-running-pty', running);
// A remote row's busy state is owned by the remote adapter (the watch
// channel), not by local PTY presence — it never enters activePtyIds,
// so purging it here on every unrelated local PTY start/stop would wipe
// its spinner. See .ai/contexts/session-cache.md ("Remote hosts — busy
// spinner").
// remote rows are owned by the remote adapter — see .ai/contexts/session-cache.md ("Remote hosts — busy spinner")
if (!running && !item.dataset.remoteAlias) {
item.classList.remove('has-busy-agents');
purgeActivityFor(id, 'pty-gone');
Expand Down
7 changes: 1 addition & 6 deletions public/remote-activity-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,7 @@ function clearRemoteActivityTimer(sessionId) {
function armRemoteDecayTimer(sessionId, ms) {
remoteActivityDecayTimers.set(sessionId, setTimeout(() => {
remoteActivityDecayTimers.delete(sessionId);
// 20s of transcript silence means "stopped writing", not "response
// ready" — a remote adapter has no PTY to confirm the turn actually
// ended (long tool call, parent delegating to subagents). Clear busy
// without arming the unread marker. Covers both onRemoteActivityEvent's
// decay and seedRemoteActivity's seed-decay — both arm through this
// function. See .ai/contexts/session-cache.md ("Remote hosts — busy spinner").
// silence is "stopped writing", not "response ready" — see .ai/contexts/session-cache.md ("Remote hosts — busy spinner")
setActivity(sessionId, false, 'remote-decay', { armReady: false });
}, ms));
}
Expand Down
14 changes: 2 additions & 12 deletions public/session-activity.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,7 @@ function applyActivityClasses(sessionId) {
if (window.ATRACE) window.atrace('class.apply', sessionId, { el: item.id || null, 'response-ready': ready, 'cli-busy': item.classList.contains('cli-busy'), fn: 'applyActivityClasses' });
}

// Drop all busy/unread/attention state for a session outside the normal
// active/idle transition — e.g. its PTY just stopped (app.js's
// updateRunningIndicators, via: 'pty-gone'). Sole writer of the three
// collections besides setActivity/rekeyActivityState, so a caller never
// deletes from them directly. Does not touch has-busy-agents/subagent state
// (sidebar.js's clearActiveSubagentsFor) or has-running-pty — those are
// owned elsewhere.
// Purge outside the active/idle transition (e.g. PTY gone); the only writer of the three collections besides setActivity/rekeyActivityState.
function purgeActivityFor(sessionId, via) {
if (window.ATRACE) window.atrace('store.purge', sessionId, { reason: via, busy: sessionBusyState.get(sessionId) ?? null, ready: responseReadySessions.has(sessionId), attention: attentionSessions.has(sessionId), fn: 'purgeActivityFor' });
attentionSessions.delete(sessionId);
Expand All @@ -53,11 +47,7 @@ function purgeActivityFor(sessionId, via) {
}

// Central activity dispatcher. `via` is trace-only — see docs/activity-trace.md.
// `opts.armReady` (default true) gates whether going idle may arm
// response-ready; it is an explicit opt-out, never derived from `via`. Pass
// `{ armReady: false }` for a source that can only infer "stopped writing"
// from silence (no PTY to ask "is a response actually ready?") — see
// .ai/contexts/session-cache.md ("Remote hosts — busy spinner").
// opts.armReady=false: going idle must not arm response-ready — see .ai/contexts/session-cache.md ("Remote hosts — busy spinner")
function setActivity(sessionId, active, via, opts) {
const armReady = !(opts && opts.armReady === false);
if (active) {
Expand Down
2 changes: 0 additions & 2 deletions public/sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -1318,8 +1318,6 @@ function buildSessionItem(session) {
if (parentHasActiveSubagent(session.sessionId)) item.classList.add('has-busy-agents');
if (window.ATRACE && item.className !== 'session-item js-stateful') window.atrace('class.render', session.sessionId, { el: item.id, cls: item.className, fn: 'buildSessionItem' });
item.dataset.sessionId = session.sessionId;
// Read by app.js's updateRunningIndicators — a remote row's busy state is
// owned by the remote adapter, not local PTY presence (F7).
if (session.remoteAlias) item.dataset.remoteAlias = session.remoteAlias;

const modified = new Date(session.modified);
Expand Down
Loading
Loading