diff --git a/.ai/contexts/ipc-bridge.md b/.ai/contexts/ipc-bridge.md index b1c9a04b..99175d97 100644 --- a/.ai/contexts/ipc-bridge.md +++ b/.ai/contexts/ipc-bridge.md @@ -206,7 +206,7 @@ session object exists. Three things make that safe: - **The response-ready lock only blocks idle.** `setActivity(id, true)` always writes, and drops the session from `responseReadySessions` — a session that resumed generating has no unread answer left to announce. `setActivity(id, false)` on a response-ready session is still ignored, so an unread marker survives duplicate idle signals. Before that split, a session that finished a turn off-screen and restarted without a click (cron, trigger-watcher, resume) had *every* subsequent busy event swallowed. -- **`cli-busy` and `response-ready` are mutually exclusive.** `applyActivityClasses()` is the only writer of either class. The cascade would in fact favour the spinner anyway (`.session-item.cli-busy:not(.needs-attention) .session-status-dot` carries `!important` and one more class than the response-ready rule that follows it in `style.css`), but the state, not the cascade, is what decides. +- **`cli-busy` and `response-ready` are mutually exclusive.** `applyStateClasses()` (`public/session-activity-dom.js`) is the only writer of either class, and the exclusivity itself is the domain's own invariant (`public/session-state.js`'s `apply()`) — see `.ai/contexts/session-state.md` ("The local-pty adapter"). The state, not the cascade, is what decides. - **A poll reply cannot overwrite a fresher event.** `setActivity` bumps a monotonic counter per session; the poll snapshots it via `currentActivitySeq()` *before* the IPC round-trip and `reconcileBusyState` skips any session that moved in between. ### The OSC 0 title is the primary busy channel diff --git a/.ai/contexts/session-state.md b/.ai/contexts/session-state.md index e0c50520..0b2628ae 100644 --- a/.ai/contexts/session-state.md +++ b/.ai/contexts/session-state.md @@ -6,11 +6,16 @@ what actually shipped, not the whole plan. ## Migration status +**Complete.** All three kinds (`local-pty`, `remote-ssh`, `local-transcript`) +are persistent-state adapters of the same shape: one `createSessionState(kind)` +per session id, events applied to it, `session-activity-dom.js`'s +`applyStateClasses(sessionId, snapshot)` as the single DOM projection path. +Issue #246 is closed. + - **Steps 1-3b: done.** `public/session-activity.js` split into a state part (itself) and a DOM part (`public/session-activity-dom.js`); `public/session-state.js` - introduced and wired behind `applyActivityClasses` for local-pty, and behind - a persistent `remote-ssh` adapter (`public/remote-activity-ui.js`) for - remote sessions — see "The remote-ssh adapter" below. Step 3b (one + introduced, and a persistent `remote-ssh` adapter (`public/remote-activity-ui.js`) + shipped for remote sessions — see "The remote-ssh adapter" below. Step 3b (one `.session-icon` slot per sidebar row, replacing `.session-status-dot` for session/subagent rows) shipped separately — see "The icon slot (step 3b)" below. @@ -24,15 +29,21 @@ what actually shipped, not the whole plan. `subagentCompleted` — see `.ai/contexts/subagent-observability.md` ("Attribution across sources") for the full wiring. A **local-pty** parent is untouched: it keeps going through the IPC path - (`session-transitions.js:detectSubagentTransitions()`), never double-fed. - `sidebar.js`'s `parentHasActiveSubagent()` now also consults the remote-ssh - and local-transcript adapters' own snapshots (`remoteSessionStates` / - `localTranscriptStates`) so `has-busy-agents` survives a full - `renderProjects()` re-render for those two kinds, the same way it already - did for local-pty via `activeSubagentsByParent`. + (`session-transitions.js:detectSubagentTransitions()`), never double-fed — + `sidebar.js`'s `reflectSubagentRunningState()` mirrors + `activeSubagentsByParent`'s live count into the local-pty adapter's own + state (`syncLocalPtyAgentsBusy`, see "The local-pty adapter" below) instead + of a second IPC feed. `sidebar.js`'s `parentHasActiveSubagent()` still + consults `activeSubagentsByParent` plus the remote-ssh and local-transcript + adapters' own snapshots (`remoteSessionStates` / `localTranscriptStates`) + for the row-level `has-busy-agents` class — see "migration status" note + under "The local-pty adapter" for why that reader is not migrated onto the + adapter's own `agentsBusy` field. - **Lifecycle decisions (2026-09-11): done.** The two verbs — detach and stop — are both real now; see "The two lifecycle verbs: detach and stop" below. +- **The local-pty adapter: done** (this pass, closing #246). See "The + local-pty adapter" below. ## The two lifecycle verbs: detach and stop @@ -172,10 +183,11 @@ per remote session id in `remoteSessionStates` (a `Map`, pruned in `has-running-pty` already reads. **The adapter never writes DOM itself.** Every event ends in -`projectRemoteState(sessionId)`, which calls `session-activity-dom.js`'s new -`applyStateClasses(sessionId, snapshot)` — the same two-class output -(`cli-busy`/`response-ready`) `applyActivityClasses` produces for local-pty, -but computed from the adapter's own snapshot instead of the local-pty Maps. +`projectRemoteState(sessionId)`, which calls `session-activity-dom.js`'s +`applyStateClasses(sessionId, snapshot)` — the same single projection path +`setActivity`/`clearUnread`/`setAttention` use for local-pty (via +`projectLocalPtyState`, see "The local-pty adapter" below), just fed from the +remote-ssh adapter's own snapshot instead. ### Row ownership: attached vs unattached (issue #273) @@ -215,10 +227,12 @@ ready-flap replay in `test/remote-row-ownership.test.js` red; disabling the `setRemoteAttached` handoff block turns the pty.exit/20s-tail replay in the same file red. -**`setActivity()`/the Maps in `session-activity.js` are still fed for remote -ids in parallel** (`markRemoteBusy`/`decayRemoteBusy` call both, when not -attached). Two readers were not migrated onto the adapter in this step, so -removing the dual-feed would regress them: +**`setActivity()` is still called for remote ids in parallel** +(`markRemoteBusy`/`decayRemoteBusy` call both, when not attached) — see "The +local-pty adapter" below for what `setActivity` now writes underneath +`sessionBusyState`/`responseReadySessions`. Two readers were not migrated +onto a persistent adapter snapshot in this step, so removing the dual-feed +would regress them: - `sidebar.js`'s `buildSessionItem` reads `sessionBusyState`/ `responseReadySessions`/`attentionSessions` directly at initial paint. - `app.js`'s grid-card busy dot (`updateRunningIndicators`'s `gridCards` @@ -228,6 +242,151 @@ Both are driven by the same `active`/`armReady` inputs as the adapter, so the two projections never disagree in practice; the dual-feed is a known, temporary duplication, not a race. +### The local-pty adapter + +`public/session-activity.js` keeps one persistent `createSessionState('local-pty')` +per session id, in `localPtyStates` — same shape as `remoteSessionStates` / +`localTranscriptStates` above. `setActivity`/`clearUnread`/`setAttention`/ +`syncLocalPtyAgentsBusy`/`rekeyActivityState`/`purgeActivityFor` all apply +events to it (`localPtyState(sessionId)`, auto-vivifying); every path ends in +`projectLocalPtyState(sessionId)` → `applyStateClasses()`, the same projection +the other two adapters use. The busy/waitingForInput/attention/responseReady +exclusivity invariant that `setActivity`'s bookkeeping used to encode by hand +(deleting from three independent collections in the right order) now lives +only in `session-state.js`'s `apply()` — `test/local-pty-adapter.test.js` +pins that forcing busy and response-ready (or attention and response-ready) +together is no longer reachable through the public API, only through +`session-state.js`'s own domain tests directly. + +Inputs: + +- **`busy` (OSC 0 / OSC 9;4)** — `setActivity(sessionId, active, via, opts)`, + called from `app.js`'s `onCliBusyState` and `onTerminalNotification` + ("waiting for your input"), and from `reconcileBusyState` (the + `get-active-sessions` poll). `opts.armReady` and the "was this session + focused" check are adapter-level judgments the pure domain cannot make on + its own (it has no notion of `activeSessionId` or "was busy a moment ago"); + `setActivity` computes them and passes the result in as the `busy` event's + `armReady`, the same contract the remote-ssh/local-transcript adapters + already use for their own reasons. +- **`attention` (OSC 9, not a busy/idle notification)** — `setAttention(sessionId, on, via)`, + called from `app.js`'s `onTerminalNotification` (set) and `clearNotifications` + (clear). An `attention` event still clears `busy`/`waitingForInput`/`responseReady` + (`clearExclusive()`, `session-state.js`) — clearing attention does not + restore whatever was cleared. But the reverse no longer holds (**revised + 2026-09-13**): a `busy` event never clears `attention` — an OSC-0 busy + title and an OSC-9 permission prompt are independent IPC streams, and a + busy edge arriving mid-prompt must not silently dismiss it. `attention` is + cleared only by an explicit `attention: false`. See the exclusivity + invariant below "Shape" for the full statement. +- **`subagentSpawned`/`subagentCompleted`** — not a second IPC feed. `sidebar.js`'s + `activeSubagentsByParent` (precise spawn/complete events plus a 60s TTL for + a parent that stops emitting) stays the ground truth for the row-level + `has-busy-agents` class and the per-agent `.running` toggle; `syncLocalPtyAgentsBusy(sessionId, active)` + mirrors its live count into the adapter's own `agentsBusy` field from + `sidebar.js`'s `reflectSubagentRunningState()` — the one repaint point every + `activeSubagentsByParent` mutation (spawn, complete, the pty-gone bulk + clear, and the TTL prune) already funnels through — so `snapshot()` is + complete for a local row without a second, independently-decayed source of + truth for the same fact. +- **`liveness`/`descriptorStatus`** — seeded on every icon paint from the + session object's `status`/`statusUpdatedAt` (`snapshotForLocal`, + `session-activity-dom.js`), the same two calls `applyRemoteDescriptor`/ + `seedLocalTranscriptDescriptor` make for the other two kinds. + +**Two readers still bypass the adapter's snapshot on purpose** — see "Row +ownership" above for why removing them isn't free in this pass: +`sidebar.js`'s `buildSessionItem`/`buildSubagentItem` (initial paint of +`cli-busy`/`response-ready`/`needs-attention`/`has-busy-agents`) and `app.js`'s +grid-card busy dot read `sessionBusyState`/`responseReadySessions`/ +`attentionSessions` directly rather than `localPtyState(id).snapshot()`. Those +three names are no longer independent `Map`/`Set` instances, though — they are +thin views over `localPtyStates` (`.get`/`.has`/`.set`/`.add`/`.delete`/`.size`), +so a direct write through them (as `reconcileBusyState`'s initial poll, or a +test's precondition setup, legitimately does before any row or `setActivity` +call exists) lands in the same persisted object `snapshotForLocal`/ +`paintSessionIcon` read from. + +#### Decided: attention supersedes and consumes the unseen-response state + +**Behavior change, not a regression to fix.** Before this migration, +`responseReadySessions` and `attentionSessions` were two independent `Set`s: a +row could carry both `response-ready` and `needs-attention` at once (CSS gave +`needs-attention` visual precedence over `response-ready`), and clearing +attention left `responseReadySessions` untouched — the row fell back to +showing `response-ready`, "Claude finished, you haven't looked", because that +fact had never actually been erased underneath the attention overlay. + +Concretely, the old sequence: session goes idle unseen (`response-ready` +armed) → an OSC 9 notification fires (`needs-attention`, drawn on top) → the +user handles it and attention clears → the row reverts to `response-ready`, +because the unseen-response fact was still sitting in the Set the whole time. + +Now the two facts live as fields on one persisted object +(`localPtyState(id)`), and `session-state.js`'s `apply()` treats `attention`, +`busy` and `waitingForInput` (which `responseReady` is a subset of) as +mutually exclusive: setting `attention: true` calls `clearExclusive()`, which +zeroes `responseReady` along with `busy`/`waitingForInput`, not just the +rung the icon happens to render. Clearing attention afterwards does not +restore it — the row lands on plain idle, not back on `response-ready`. Same +sequence today: idle unseen → attention fires (response-ready fact erased, +not just outshone) → attention clears → idle, unread marker gone. + +This is a deliberate consequence of unifying local-pty into the same domain +model the remote-ssh/local-transcript adapters already used (`test/session-state.test.js`'s +pre-existing "exclusivity: attention while busy..." pins the same +`clearExclusive()` behavior for those kinds) — not something to special-case +back for local-pty. Read "attention" as *consuming* whatever unseen-response +state it interrupts, the same way going busy again already consumed it before +this change. `test/local-pty-adapter.test.js`'s "attention after +response-ready, then clearing attention does not restore response-ready" pins +this exact scenario so a future reader finds it intentional, not a bug to fix. + +One remaining consequence flagged, not fixed: `.has()` on +`attentionSessions`/`responseReadySessions` (the legacy views above) now means +"that facet is currently true", not "was ever added and not yet removed" — no +shipped behavior currently depends on reading a stale `responseReady`/`busy` +value after an attention transition, but a future feature reintroducing that +combination would need its own domain field, not a Set-level workaround. + +#### Decided: a local row idling while active (or with `armReady:false`) now shows "Waiting for input", not "Idle" + +**Behavior change, kept deliberately.** Before this migration, +`snapshotForLocal`'s predecessor (`computeBusyReadyClasses` combined with the +throwaway per-render reconstruction) only ever applied a `busy` event to its +scratch domain object when the session was busy, or idle-and-unseen +(`responseReady`) — a session that went idle while **focused**, or via a +`setActivity(id, false, via, { armReady: false })` call, got no event applied +at all, so `waitingForInput` stayed at its default `false` and +`renderSessionIcon` fell through every named rung to the `idle` fallback: +empty glyph, title "Idle". + +Now `setActivity` always applies a real `busy: false` event to the +**persisted** local-pty state on every idle transition, focused or not, +`armReady` true or false — `session-state.js`'s `apply()` unconditionally sets +`waitingForInput = true` on that branch regardless of `armReady` (`armReady` +only gates `responseReady`). So a focused session idling, or any session +idling via an `armReady:false` source, now resolves to the `waitingForInput` +rung: `session-icon--waiting`, title "Waiting for input" — where main showed +nothing (`idle`, title "Idle"). + +**Kept, not reverted**, because it converges local-pty with the other two +kinds: `remote-ssh` and `local-transcript` already report `waitingForInput` +(never `idle`) the instant they go quiet with `armReady:false` — a CLI +sitting at its prompt genuinely *is* waiting for input, whether or not +Switchboard currently has a tab open on it, and whether or not the last idle +edge happened to arm the unseen-response marker. The `idle` rung still exists +in `renderSessionIcon`'s priority ladder (a session `apply()` has literally +never touched resolves there, e.g. a brand-new `localPtyState()` before any +event lands), it is simply no longer reachable for a local session that has +gone through at least one busy→idle cycle. + +`test/local-pty-adapter.test.js`'s "a local row going idle while active shows +'Waiting for input', not 'Idle'" and "...idle NOT active still arms +response-ready (unchanged)" pin both branches of the decision — the icon +slot's `session-icon--waiting`/`session-icon--response-ready` class and +title, side by side, so a future reader finds the split intentional. + ### The local-transcript adapter (step 4) `public/local-transcript-adapter.js` keeps one persistent @@ -279,8 +438,9 @@ path already owns, mirroring the renderer-side guard below one layer down. **Guarded against a PTY takeover in both directions**: `onLocalTranscriptActivity` checks `activePtyIds.has(sessionId)` and refuses to even allocate state for a session already carrying a PTY in this app (the -OSC path owns it — see `session-activity-dom.js`'s `applyActivityClassesToElement`, -fed by `main.js`'s OSC 0/9 parsing, not this adapter). Going the other way, +OSC path owns it — see `session-activity.js`'s `setActivity`, fed by +`main.js`'s OSC 0/9 parsing via `onCliBusyState`/`onTerminalNotification`, not +this adapter). Going the other way, `app.js`'s `updateRunningIndicators()` calls `localTranscriptPtyTakeover(id)` for a non-remote row the instant it transitions into `activePtyIds` (the user opened it), which clears the pending decay timer and deletes the row's @@ -301,14 +461,18 @@ public/session-state.js pure domain — no DOM, no IPC, no electron public/session-activity-dom.js DOM projection — the only file allowed to write .cli-busy/.needs-attention/ .response-ready/.has-busy-agents -public/session-activity.js Maps/Sets + setActivity/purgeActivityFor/ +public/session-activity.js the local-pty adapter — one persistent + createSessionState('local-pty') per session + id (localPtyStates) + setActivity/clearUnread/ + setAttention/purgeActivityFor/ rekeyActivityState/reconcileBusyState — calls into session-activity-dom.js to render ``` `createSessionState(kind)` returns `{ apply(event), snapshot() }`. `kind` is -`'local-pty' | 'local-transcript' | 'remote-ssh'` (only `'local-pty'` is fed -today). Snapshot fields: +`'local-pty' | 'local-transcript' | 'remote-ssh'` — all three are fed by a +persistent per-session-id adapter (`localPtyStates` / `localTranscriptStates` / +`remoteSessionStates`). Snapshot fields: | field | meaning | |---|---| @@ -320,26 +484,47 @@ today). Snapshot fields: | `attention` | OSC 9 — needs the user right now (permission/approval/plan) | | `responseReady` | subset of `waitingForInput`: idle **and** unseen when it went idle (the legacy "Claude finished, you haven't looked" rung). Not in the issue's original field list — added because the priority order names it as its own rung, distinct from plain `waitingForInput`; see "Design notes" below. | | `agentsBusy` | subagents running under this session | -| `lastActivityAt` / `lastActivitySource` | last touch, for `local-transcript`/`remote-ssh` (unused by local-pty today) | +| `lastActivityAt` / `lastActivitySource` | last touch — `transcriptTouched`/`descriptorStatus` events carry `at`/`source`; local-pty now feeds `descriptorStatus` too (`snapshotForLocal`'s seed from `session.status`), so this is populated on all three kinds. Not read by `renderSessionIcon` on any kind today. | | `label` / `labelConfidence` | reserved, unused | | `attachable` | reserved, unused | | `archived` / `stale` | reserved, unused | -Invariant enforced by `apply()`: `busy` / `waitingForInput` / `attention` are -mutually exclusive — going busy or attention clears the other two (and -`responseReady`, which only means something under `waitingForInput`). +Invariant enforced by `apply()` (**revised 2026-09-13, adversarial review of +PR #282**): `busy` and `waitingForInput` (and `responseReady`, which only +means something under `waitingForInput`) are mutually exclusive — going busy +clears the other two. `attention` is **orthogonal to busy**: an `attention` +event still clears `busy`/`waitingForInput`/`responseReady` (unchanged), but +a `busy` event — either direction — never clears `attention`. `attention` is +exclusive with `responseReady` only, one-directionally: setting it clears +`responseReady` (consumes the unseen-response fact, see "Decided" below), but +clearing `attention` does not resurrect anything. Rationale: an OSC-0 busy +title and an OSC-9 permission prompt are two independent IPC streams: a busy +edge arriving while attention is pending must not silently dismiss the +prompt's indicator. `attention` is cleared only by an explicit +`attention: false` (`clearNotifications`) — never as a side effect of a busy +edge. `test/session-state.test.js`'s "exclusivity: going busy again clears +waitingForInput/responseReady but NOT attention" and +`test/local-pty-adapter.test.js`'s "a busy edge after attention does NOT +clear attention" pin this at the domain and adapter levels respectively. `renderSessionIcon(snapshot)` resolves the priority order — attention > responseReady > busy > agentsBusy > waitingForInput > idle+age > stale > archived — defensively (it does not trust the caller kept exclusivity) and -returns `{ classes, slotClasses, glyph, title }` for **one icon slot**. Only -the four rungs that map to an existing row-level CSS class (`needs-attention`, -`response-ready`, `cli-busy`, `has-busy-agents`, in `classes`) carry one; every -rung — including those four — also carries exactly one `slotClasses` entry +returns `{ classes, slotClasses, glyph, title }` for **one icon slot**: only +the single winning rung's `classes`/`slotClasses`/`glyph`/`title` come back, +never a union across rungs. Every rung carries exactly one `slotClasses` entry (`session-icon--attention`, `session-icon--response-ready`, `session-icon--busy`, `session-icon--agents-busy`, `session-icon--waiting`, `session-icon--idle`, -`session-icon--stale`, `session-icon--archived`). See "The icon slot (step 3b)" -below for how `classes` and `slotClasses` are used differently. +`session-icon--stale`, `session-icon--archived`), the only thing `writeIconSlot` +reads — see "The icon slot (step 3b)" below. **`classes` is no longer read by +`applyStateClasses` for the row-level classes** (revised 2026-09-13, PR #282 +review): `needs-attention`/`response-ready`/`cli-busy` are read straight off +`snapshot.attention`/`.responseReady`/`.busy` instead, because those three can +now coexist with `busy` (attention) in a way the single-winning-rung `classes` +array cannot represent — deriving row classes from the priority winner alone +was silently dropping `cli-busy` whenever `attention` also won the rung. +`classes` remains on the return value (pinned by `test/session-state.test.js`'s +priority-order tests) but has no other production reader today. ## The icon slot (step 3b) @@ -363,34 +548,35 @@ underneath it changes. distinct from the row-level `classes` (`cli-busy` etc.) both for the eslint boundary (see "Enforcement" below) and so a reader never confuses "this paints the row" with "this paints the slot". -- `session-activity-dom.js` also references `parentHasActiveSubagent` - (`sidebar.js`) and `sessionMap` (`app.js`) now, alongside the pre-existing - `sessionBusyState`/`responseReadySessions`/`attentionSessions` - (`session-activity.js`). Safe despite loading before all three in - index.html's script order — every reference is inside a function body, - resolved at call time after the whole page has loaded, same pattern - `sidebar.js`'s own header comment documents for its dependencies. -- `snapshotForLocal(sessionId, session)` builds a local-pty snapshot the same - way `computeBusyReadyClasses` does for busy/responseReady, extended with - `attention` (`attentionSessions`), `agentsBusy` (`parentHasActiveSubagent()`), - and `liveness`/`descriptorStatus` from `session.status`/`statusUpdatedAt` - when present — cli-session-state.js only keeps an entry while the pid is - alive (`.ai/contexts/cli-session-state.md`), so `session.status` being - present at all is itself the local liveness signal; `session` is optional - and falls back to a `sessionMap` lookup for call sites that only have a - sessionId. +- `session-activity-dom.js` also references `localPtyState` and `sessionMap` + (`app.js`) now, alongside the pre-existing `sessionBusyState`/ + `responseReadySessions`/`attentionSessions` view objects (`session-activity.js`). + Safe despite loading before `session-activity.js` in index.html's script + order — every reference is inside a function body, resolved at call time + after the whole page has loaded, same pattern `sidebar.js`'s own header + comment documents for its dependencies. +- `snapshotForLocal(sessionId, session)` is a thin wrapper over the local-pty + adapter's own persisted state (`localPtyState(sessionId).snapshot()`) — see + "The local-pty adapter" above. It seeds `liveness`/`descriptorStatus` from + `session.status`/`statusUpdatedAt` on every call, idempotently — cli-session-state.js + only keeps an entry while the pid is alive (`.ai/contexts/cli-session-state.md`), + so `session.status` being present at all is itself the local liveness + signal; `session` is optional and falls back to a `sessionMap` lookup for + call sites that only have a sessionId. `busy`/`attention`/`responseReady`/ + `agentsBusy` are already on the persisted state (fed by `setActivity`/ + `setAttention`/`syncLocalPtyAgentsBusy`), not recomputed here. - `paintSessionIcon(el, sessionId, session)` composes the two: `writeIconSlot(el, renderSessionIcon(snapshotForLocal(sessionId, session)))`. Called from `sidebar.js` at row construction (both `buildSessionItem` and - `buildSubagentItem`), from `applyActivityClassesToElement` on every local - busy/ready/attention/subagent transition, and from - `reflectSubagentRunningState` on the **parent** row (agentsBusy is part of - the priority ladder the slot resolves, so a subagent spawn/complete must - repaint the parent's slot, not just its `has-busy-agents` row class). -- `applyStateClasses(sessionId, snapshot)` (the remote-ssh path, called from - `remote-activity-ui.js`'s `projectRemoteState`) now also calls - `writeIconSlot` with the same `renderSessionIcon(snapshot)` result it uses - for the row's `cli-busy`/`response-ready` classes — this is what makes a + `buildSubagentItem`) and from `reflectSubagentRunningState` on the + **parent** row (agentsBusy is part of the priority ladder the slot + resolves, so a subagent spawn/complete must repaint the parent's slot, not + just its `has-busy-agents` row class). +- `applyStateClasses(sessionId, snapshot)` — the single projection path all + three kinds' transitions go through (`projectLocalPtyState`/ + `projectRemoteState`/`projectLocalTranscriptState`) — calls `writeIconSlot` + with the same `renderSessionIcon(snapshot)` result it uses for the row's + `needs-attention`/`cli-busy`/`response-ready` classes. This is what makes a local busy row and a remote busy row render the identical slot markup (classes, title, glyph), pinned in `test/dom-sidebar-icon-slot.test.js`. @@ -483,10 +669,12 @@ the others assert on the dot/slot element itself, only on row classes and cannot stand in for "the user is looking at this row right now". The `armReady` flag on the `busy` event carries that judgment in from the adapter, same as before the split. -- Ports (`transcriptTouched`, `descriptorStatus`, `subagentSpawned/Completed`, - `attachable`, `label`, `archived`, `stale`) are implemented in `apply()` but - **not fed by any adapter yet** — they exist so steps 3-5 don't need another - domain-shape change. +- Ports (`attachable`, `label`, `archived`, `stale`) are implemented in + `apply()` but **not fed by any adapter yet** — they exist so a future step + doesn't need another domain-shape change. `transcriptTouched`, + `descriptorStatus` and `subagentSpawned`/`subagentCompleted` are all wired + now (see the ports table below) — this bullet used to list them too, before + the local-pty adapter closed the last gap. ## Enforcement @@ -520,11 +708,11 @@ the others assert on the dot/slot element itself, only on row classes and | event | local-pty | local-transcript | remote-ssh | |---|---|---|---| -| `busy` / `attention` (OSC 0 / 9) | yes | never | wired via the watch channel (transcript writes), not OSC — OSC-while-attached is not wired | -| `transcriptTouched(at)` | yes | yes (only signal) — `onLocalTranscriptActivity` | yes — `onRemoteActivityEvent`/`markRemoteBusy` | -| `descriptorStatus(status, at)` / `liveness` | yes | yes — `seedLocalTranscriptDescriptor`, from `sessionMap`'s `status`/`statusUpdatedAt` (see "The local-transcript adapter" above for why this widens the issue's original "no (no live CLI)") | yes (`main.js:539` → `applyRemoteDescriptor`) | +| `busy` / `attention` (OSC 0 / 9) | yes — `setActivity`/`setAttention` | never | wired via the watch channel (transcript writes), not OSC — OSC-while-attached is not wired | +| `transcriptTouched(at)` | no — local-pty's busy signal is the OSC title, not a transcript write; a real PTY makes this port redundant for it | yes (only signal) — `onLocalTranscriptActivity` | yes — `onRemoteActivityEvent`/`markRemoteBusy` | +| `descriptorStatus(status, at)` / `liveness` | yes — `snapshotForLocal`'s seed from `session.status`/`statusUpdatedAt`, same two calls the other kinds make | yes — `seedLocalTranscriptDescriptor`, from `sessionMap`'s `status`/`statusUpdatedAt` (see "The local-transcript adapter" above for why this widens the issue's original "no (no live CLI)") | yes (`main.js:539` → `applyRemoteDescriptor`) | | `attached` | reserved, unused | reserved, unused | yes — `setRemoteAttached`, driven by the per-row `activePtyIds` transition; also the row-ownership arbiter since #273 (see "Row ownership" above) | -| `subagentSpawned` / `subagentCompleted` | yes — via `detectSubagentTransitions()` IPC | yes (issue #247) — `onLocalTranscriptSubagentActivity`, gated on the parent having no PTY | yes (issue #247) — `onRemoteActivityEvent({kind:'subagent'})`, attributed by `subagentParentFromParts()` | +| `subagentSpawned` / `subagentCompleted` | yes — `syncLocalPtyAgentsBusy`, mirroring `activeSubagentsByParent` (fed by `detectSubagentTransitions()` IPC) rather than a second IPC feed — see "The local-pty adapter" above | yes (issue #247) — `onLocalTranscriptSubagentActivity`, gated on the parent having no PTY | yes (issue #247) — `onRemoteActivityEvent({kind:'subagent'})`, attributed by `subagentParentFromParts()` | An adapter without a PTY must never claim `waitingForInput` or `responseReady` from a completion signal it cannot verify — that is why the remote-ssh and diff --git a/docs/activity-trace.md b/docs/activity-trace.md index 6e54d22d..088fc156 100644 --- a/docs/activity-trace.md +++ b/docs/activity-trace.md @@ -174,8 +174,8 @@ no `sent`. | `store.purge` | State dropped because the PTY is gone | `reason`, `busy`, `ready`, `attention` | | `store.rekey` | Activity state carried across a fork | `from`, `busy`, `ready`, `attention` | | `subagents.prune` | The 60 s TTL sweep ran | `parents`, `agents` | -| `class.apply` | `cli-busy` / `response-ready` written | `el`, both class states | -| `class.toggle` | `needs-attention` / `has-running-pty` written | `el`, `cls`, `on` | +| `class.apply` | `needs-attention` / `cli-busy` / `response-ready` / `has-busy-agents` / `is-alive` written (`applyStateClasses`, all three session kinds) | `el`, `needs-attention`, `cli-busy`, `response-ready`, `has-busy-agents`, `is-alive`, `kind` | +| `class.toggle` | `has-running-pty` written | `el`, `cls`, `on` | | `class.subagent` | Subagent `running` / `has-running-child` / `has-busy-agents` written | `el` ids, `running` | | `class.render` | A full sidebar render reconstructed an item's classes from the stores | `el`, `cls` | | `poll.recv` | The poll reply reaches the renderer | `sinceSeq`, `entries` | diff --git a/eslint.config.js b/eslint.config.js index c6537ace..2da2def3 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -141,7 +141,8 @@ const rendererCrossFileGlobals = { saveExpandedSlugs: 'readonly', setActivity: 'readonly', trackActivity: 'readonly', - applyActivityClasses: 'readonly', + setAttention: 'readonly', + syncLocalPtyAgentsBusy: 'readonly', sessionItemEl: 'readonly', seedRemoteActivity: 'readonly', rekeyActivityState: 'readonly', @@ -150,6 +151,9 @@ const rendererCrossFileGlobals = { forgetActivitySeq: 'readonly', purgeActivityFor: 'readonly', pruneRemoteActivityTimers: 'readonly', + // public/session-activity.js's persisted per-session state — see .ai/contexts/session-state.md ("The local-pty adapter") + localPtyState: 'readonly', + localPtyStates: 'readonly', // Changes panel no-polling refresh hook (issue #251, public/file-panel.js) onSessionIdle: 'readonly', // public/session-state.js (pure domain, see .ai/contexts/session-state.md) @@ -158,7 +162,6 @@ const rendererCrossFileGlobals = { // public/session-activity-dom.js — the only file allowed to write // .cli-busy/.needs-attention/.response-ready/.has-busy-agents, and the only // file allowed to write the .session-icon slot (issue #246, step 3b). - applyActivityClassesToElement: 'readonly', applyStateClasses: 'readonly', setNeedsAttention: 'readonly', setResponseReady: 'readonly', diff --git a/public/app.js b/public/app.js index bbbb721b..b3daad70 100644 --- a/public/app.js +++ b/public/app.js @@ -306,9 +306,7 @@ function trackActivity(sessionId, data) { function clearNotifications(sessionId) { clearUnread(sessionId, 'clearNotifications'); - if (window.ATRACE && attentionSessions.has(sessionId)) window.atrace('store.mutate', sessionId, { map: 'attentionSessions', op: 'delete', from: true, to: false, fn: 'clearNotifications' }); - attentionSessions.delete(sessionId); - setNeedsAttention(sessionItemEl(sessionId), false); + setAttention(sessionId, false, 'clearNotifications'); } // Terminal themes, utils (cleanDisplayName, formatDate, escapeHtml, shellEscape) // are defined in terminal-themes.js and utils.js (loaded before app.js). @@ -460,11 +458,7 @@ window.api.onTerminalNotification((sessionId, message) => { // 3. "Claude needs your permission to use {tool}" → permission, needs your // 4. "Claude Code wants to enter plan mode" → wants to enter if (/attention|approval|permission|needs your|wants to enter/i.test(message) && sessionId !== activeSessionId) { - if (window.ATRACE) window.atrace('store.mutate', sessionId, { map: 'attentionSessions', op: 'add', from: attentionSessions.has(sessionId), to: true, fn: 'onTerminalNotification' }); - attentionSessions.add(sessionId); - const item = sessionItemEl(sessionId); - if (window.ATRACE) window.atrace('class.toggle', sessionId, { el: item ? item.id : null, cls: 'needs-attention', on: true, fn: 'onTerminalNotification' }); - setNeedsAttention(item, true); + setAttention(sessionId, true, 'onTerminalNotification'); } else if (/waiting for your input/i.test(message)) { // "Claude is waiting for your input" — delayed idle notification, mark response-ready setActivity(sessionId, false, 'onTerminalNotification'); @@ -836,11 +830,12 @@ function updateRunningIndicators() { // remote rows are owned by the remote adapter — see .ai/contexts/session-cache.md ("Remote hosts — busy spinner") if (!running && !item.dataset.remoteAlias) { setHasBusyAgents(item, false); - purgeActivityFor(id, 'pty-gone'); // A stopped PTY can never emit subagent-completed (stop-session kills // the process; detectSubagentTransitions skips exited sessions), so // drop the live-subagent state now instead of waiting for the TTL. clearActiveSubagentsFor(id); + // Runs after clearActiveSubagentsFor — see .ai/contexts/session-state.md ("The local-pty adapter") + purgeActivityFor(id, 'pty-gone'); } if (item.dataset.remoteAlias) setRemoteAttached(id, running); // local-pty takes over a row the user just opened — see .ai/contexts/session-state.md diff --git a/public/remote-activity-ui.js b/public/remote-activity-ui.js index e3b6d282..0b115ffc 100644 --- a/public/remote-activity-ui.js +++ b/public/remote-activity-ui.js @@ -148,6 +148,8 @@ function setRemoteAttached(sessionId, attached) { remoteSeedFloors.set(sessionId, Date.now()); state.apply({ type: 'busy', active: false, armReady: false }); setActivity(sessionId, false, 'remote-attach-handoff', { armReady: false }); + // Drops the shadow local-pty entry setActivity() just touched above — see .ai/contexts/session-state.md ("The local-pty adapter") + purgeActivityFor(sessionId, 'remote-detach'); } projectRemoteState(sessionId); } diff --git a/public/session-activity-dom.js b/public/session-activity-dom.js index e9a47eed..1a256f05 100644 --- a/public/session-activity-dom.js +++ b/public/session-activity-dom.js @@ -31,47 +31,24 @@ function isSessionAlive(sessionId) { return !!(el && el.classList.contains('is-alive')); } -// local-pty only for now — see session-state.md "migration status". -function computeBusyReadyClasses(sessionId) { - const busy = sessionBusyState.get(sessionId) === true; - const ready = !busy && responseReadySessions.has(sessionId); - const state = createSessionState('local-pty'); - if (busy) state.apply({ type: 'busy', active: true }); - else if (ready) state.apply({ type: 'busy', active: false, armReady: true }); - return renderSessionIcon(state.snapshot()).classes; -} - -// The only writer of .cli-busy and .response-ready — they are mutually exclusive. -function applyActivityClassesToElement(item, sessionId) { - if (!item) return; - const classes = computeBusyReadyClasses(sessionId); - const ready = classes.includes('response-ready'); - const busy = classes.includes('cli-busy'); - setResponseReady(item, ready); - setCliBusy(item, busy); - paintSessionIcon(item.querySelector('.session-icon'), sessionId); - if (window.ATRACE) window.atrace('class.apply', sessionId, { el: item.id || null, 'response-ready': ready, 'cli-busy': busy, fn: 'applyActivityClasses' }); -} - -function applyActivityClasses(sessionId) { - applyActivityClassesToElement(sessionItemEl(sessionId), sessionId); -} - -// Snapshot-driven projection for adapter-owned state; has-busy-agents read off the snapshot — see .ai/contexts/session-state.md +// Snapshot-driven projection — the one path for all three kinds — see .ai/contexts/session-state.md ("The local-pty adapter") function applyStateClasses(sessionId, snapshot) { const item = sessionItemEl(sessionId); if (!item) return; const icon = renderSessionIcon(snapshot); - const ready = icon.classes.includes('response-ready'); - const busy = icon.classes.includes('cli-busy'); + // Row classes read snapshot fields directly, not icon.classes — see .ai/contexts/session-state.md ("The local-pty adapter") + const attention = !!(snapshot && snapshot.attention); + const ready = !!(snapshot && snapshot.responseReady); + const busy = !!(snapshot && snapshot.busy); const agentsBusy = !!(snapshot && snapshot.agentsBusy); const alive = !!(snapshot && snapshot.liveness === 'alive'); + setNeedsAttention(item, attention); setResponseReady(item, ready); setCliBusy(item, busy); setHasBusyAgents(item, agentsBusy); setIsAlive(item, alive); writeIconSlot(item.querySelector('.session-icon'), icon); - if (window.ATRACE) window.atrace('class.apply', sessionId, { el: item.id || null, 'response-ready': ready, 'cli-busy': busy, 'has-busy-agents': agentsBusy, 'is-alive': alive, fn: 'applyStateClasses', kind: snapshot && snapshot.kind }); + if (window.ATRACE) window.atrace('class.apply', sessionId, { el: item.id || null, 'needs-attention': attention, 'response-ready': ready, 'cli-busy': busy, 'has-busy-agents': agentsBusy, 'is-alive': alive, fn: 'applyStateClasses', kind: snapshot && snapshot.kind }); } // One icon slot per row, written here and nowhere else — see .ai/contexts/session-state.md @@ -85,17 +62,9 @@ function writeIconSlot(el, icon) { el.dataset.glyph = icon.glyph || ''; } -// local-pty snapshot for the icon slot (full priority ladder, unlike computeBusyReadyClasses) — see .ai/contexts/session-state.md +// Thin wrapper over the local-pty adapter's own persisted state — see .ai/contexts/session-state.md ("The local-pty adapter") function snapshotForLocal(sessionId, session) { - const state = createSessionState('local-pty'); - const busy = sessionBusyState.get(sessionId) === true; - const ready = !busy && responseReadySessions.has(sessionId); - if (busy) state.apply({ type: 'busy', active: true }); - else if (ready) state.apply({ type: 'busy', active: false, armReady: true }); - if (attentionSessions.has(sessionId)) state.apply({ type: 'attention', active: true }); - if (typeof parentHasActiveSubagent === 'function' && parentHasActiveSubagent(sessionId)) { - state.apply({ type: 'subagentSpawned' }); - } + const state = localPtyState(sessionId); const sess = session || (typeof sessionMap !== 'undefined' && sessionMap.get(sessionId)); if (sess && sess.status !== undefined) { // session.status present is itself the liveness signal — see .ai/contexts/cli-session-state.md diff --git a/public/session-activity.js b/public/session-activity.js index 7d2c68e7..7bfd004a 100644 --- a/public/session-activity.js +++ b/public/session-activity.js @@ -1,11 +1,65 @@ -// Session activity state — busy / response-ready / attention. -// See .ai/contexts/ipc-bridge.md "Busy-state reconciliation" and -// .ai/contexts/session-state.md (DOM projection split out to -// session-activity-dom.js). +// Session activity state — the local-pty adapter — see .ai/contexts/session-state.md ("The local-pty adapter") +const localPtyStates = new Map(); -const attentionSessions = new Set(); // sessions needing user action (OSC 9) -const responseReadySessions = new Set(); // Claude finished, user hasn't looked (terminal state) -const sessionBusyState = new Map(); // sessionId → boolean (currently active) +function localPtyState(sessionId) { + let state = localPtyStates.get(sessionId); + if (!state) { + state = createSessionState('local-pty'); + localPtyStates.set(sessionId, state); + } + return state; +} + +function projectLocalPtyState(sessionId) { + applyStateClasses(sessionId, localPtyState(sessionId).snapshot()); +} + +// Map-/Set-like views over localPtyStates, for readers not yet migrated onto the adapter — see .ai/contexts/session-state.md ("The local-pty adapter") +const sessionBusyState = { + get(sessionId) { + const s = localPtyStates.get(sessionId); + return s ? s.snapshot().busy : undefined; + }, + has(sessionId) { return localPtyStates.has(sessionId); }, + set(sessionId, val) { + localPtyState(sessionId).apply({ type: 'busy', active: !!val, armReady: false }); + return this; + }, + delete(sessionId) { return localPtyStates.delete(sessionId); }, + get size() { return localPtyStates.size; }, +}; + +const responseReadySessions = { + has(sessionId) { + const s = localPtyStates.get(sessionId); + return !!s && s.snapshot().responseReady === true; + }, + add(sessionId) { + localPtyState(sessionId).apply({ type: 'busy', active: false, armReady: true }); + return this; + }, + delete(sessionId) { + if (!localPtyStates.has(sessionId)) return false; + localPtyState(sessionId).apply({ type: 'clearUnread' }); + return true; + }, +}; + +const attentionSessions = { + has(sessionId) { + const s = localPtyStates.get(sessionId); + return !!s && s.snapshot().attention === true; + }, + add(sessionId) { + localPtyState(sessionId).apply({ type: 'attention', active: true }); + return this; + }, + delete(sessionId) { + if (!localPtyStates.has(sessionId)) return false; + localPtyState(sessionId).apply({ type: 'attention', active: false }); + return true; + }, +}; // see .ai/contexts/changes-view.md ("Refresh triggers") const idleListeners = new Set(); @@ -27,83 +81,109 @@ function currentActivitySeq() { return activitySeq; } -// Called from updateRunningIndicators() when a session leaves activePtyIds, -// next to the purge of the three collections above. +// Called from updateRunningIndicators() alongside the purge of the local-pty state above. function forgetActivitySeq(sessionId) { if (window.ATRACE) window.atrace('store.mutate', sessionId, { map: 'activitySeqBySession', op: 'delete', from: activitySeqBySession.get(sessionId) ?? null, to: null, fn: 'forgetActivitySeq' }); activitySeqBySession.delete(sessionId); } -// Purge outside the active/idle transition (e.g. PTY gone); the only writer of the three collections besides setActivity/rekeyActivityState. +// Drops the whole per-session state; repaints from a throwaway blank state, not localPtyState(), which would re-vivify an entry — see .ai/contexts/session-state.md ("The local-pty adapter") 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); - responseReadySessions.delete(sessionId); - sessionBusyState.delete(sessionId); + const before = localPtyStates.get(sessionId); + const beforeSnap = before ? before.snapshot() : null; + if (window.ATRACE) window.atrace('store.purge', sessionId, { reason: via, busy: beforeSnap ? beforeSnap.busy : null, ready: beforeSnap ? beforeSnap.responseReady : false, attention: beforeSnap ? beforeSnap.attention : false, fn: 'purgeActivityFor' }); + localPtyStates.delete(sessionId); forgetActivitySeq(sessionId); - setNeedsAttention(sessionItemEl(sessionId), false); - applyActivityClasses(sessionId); + applyStateClasses(sessionId, createSessionState('local-pty').snapshot()); } // Central activity dispatcher. `via` is trace-only — see docs/activity-trace.md. // 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) { - if (window.ATRACE && responseReadySessions.has(sessionId)) window.atrace('store.mutate', sessionId, { map: 'responseReadySessions', op: 'delete', from: true, to: false, fn: 'setActivity', via }); - responseReadySessions.delete(sessionId); - } else if (responseReadySessions.has(sessionId)) { + const state = localPtyState(sessionId); + const before = state.snapshot(); + + // response-ready-holds-idle: a duplicate/late idle signal is not a transition — see .ai/contexts/session-state.md + if (!active && before.responseReady) { if (window.ATRACE) window.atrace('store.skip', sessionId, { map: 'sessionBusyState', reason: 'response-ready-holds-idle', fn: 'setActivity', via }); return; } - const wasActive = sessionBusyState.get(sessionId) || false; - sessionBusyState.set(sessionId, active); + if (window.ATRACE && active && before.responseReady) { + window.atrace('store.mutate', sessionId, { map: 'responseReadySessions', op: 'delete', from: true, to: false, fn: 'setActivity', via }); + } + + // armReady is computed here, not in apply() — see .ai/contexts/session-state.md ("The local-pty adapter") + const wasBusy = before.busy; + const effectiveArmReady = active ? armReady : (wasBusy && armReady && sessionId !== activeSessionId); + + state.apply({ type: 'busy', active, armReady: effectiveArmReady }); + const after = state.snapshot(); + activitySeq += 1; activitySeqBySession.set(sessionId, activitySeq); - if (window.ATRACE) window.atrace('store.mutate', sessionId, { map: 'sessionBusyState', op: 'set', from: wasActive, to: active, actSeq: activitySeq, fn: 'setActivity', via }); + if (window.ATRACE) window.atrace('store.mutate', sessionId, { map: 'sessionBusyState', op: 'set', from: wasBusy, to: after.busy, actSeq: activitySeq, fn: 'setActivity', via }); - if (wasActive && !active && sessionId !== activeSessionId && armReady) { - if (window.ATRACE) window.atrace('store.mutate', sessionId, { map: 'responseReadySessions', op: 'add', from: false, to: true, fn: 'setActivity', via }); - responseReadySessions.add(sessionId); + if (window.ATRACE && wasBusy && !active && after.responseReady) { + window.atrace('store.mutate', sessionId, { map: 'responseReadySessions', op: 'add', from: false, to: true, fn: 'setActivity', via }); } - applyActivityClasses(sessionId); + projectLocalPtyState(sessionId); // Fire only on a genuine busy->idle edge — see .ai/contexts/changes-view.md ("Refresh triggers"). - if (wasActive && !active) notifySessionIdle(sessionId); + if (wasBusy && !active) notifySessionIdle(sessionId); } function clearUnread(sessionId, via) { - if (window.ATRACE && responseReadySessions.has(sessionId)) window.atrace('store.mutate', sessionId, { map: 'responseReadySessions', op: 'delete', from: true, to: false, fn: 'clearUnread', via }); - responseReadySessions.delete(sessionId); - applyActivityClasses(sessionId); + const state = localPtyState(sessionId); + if (window.ATRACE && state.snapshot().responseReady) window.atrace('store.mutate', sessionId, { map: 'responseReadySessions', op: 'delete', from: true, to: false, fn: 'clearUnread', via }); + state.apply({ type: 'clearUnread' }); + projectLocalPtyState(sessionId); +} + +// OSC 9 "needs attention" — supersedes and consumes response-ready, see .ai/contexts/session-state.md ("The local-pty adapter") +function setAttention(sessionId, on, via) { + const state = localPtyState(sessionId); + const before = state.snapshot(); + if (window.ATRACE && before.attention !== !!on) window.atrace('store.mutate', sessionId, { map: 'attentionSessions', op: on ? 'add' : 'delete', from: before.attention, to: !!on, fn: 'setAttention', via }); + state.apply({ type: 'attention', active: !!on }); + projectLocalPtyState(sessionId); +} + +// Mirrors activeSubagentsByParent into the adapter's own agentsBusy — see .ai/contexts/session-state.md ("The local-pty adapter") +function syncLocalPtyAgentsBusy(sessionId, active) { + // Touch only if an entry already exists, never auto-vivify — see .ai/contexts/session-state.md ("The local-pty adapter") + const state = localPtyStates.get(sessionId); + if (!state) return; + if (state.snapshot().agentsBusy === !!active) return; + state.apply(active ? { type: 'subagentSpawned' } : { type: 'subagentCompleted', stillActive: false }); } // Carry the activity state across a session-detected / session-forked re-key. function rekeyActivityState(oldId, newId) { if (oldId === newId) return; - if (window.ATRACE) window.atrace('store.rekey', newId, { from: oldId, busy: sessionBusyState.get(oldId) ?? null, ready: responseReadySessions.has(oldId), attention: attentionSessions.has(oldId), fn: 'rekeyActivityState' }); + const state = localPtyStates.get(oldId); + if (window.ATRACE) { + const snap = state ? state.snapshot() : null; + window.atrace('store.rekey', newId, { from: oldId, busy: snap ? snap.busy : null, ready: snap ? snap.responseReady : false, attention: snap ? snap.attention : false, fn: 'rekeyActivityState' }); + } const oldItem = sessionItemEl(oldId); setCliBusy(oldItem, false); setResponseReady(oldItem, false); setNeedsAttention(oldItem, false); - if (sessionBusyState.has(oldId)) { - sessionBusyState.set(newId, sessionBusyState.get(oldId)); - sessionBusyState.delete(oldId); - } - if (responseReadySessions.delete(oldId)) responseReadySessions.add(newId); - if (attentionSessions.delete(oldId)) { - attentionSessions.add(newId); - setNeedsAttention(sessionItemEl(newId), true); + if (state) { + localPtyStates.delete(oldId); + localPtyStates.set(newId, state); } + const seq = activitySeqBySession.get(oldId); if (seq !== undefined) { activitySeqBySession.delete(oldId); activitySeqBySession.set(newId, seq); } - applyActivityClasses(newId); + projectLocalPtyState(newId); } // Realign against the backend snapshot from get-active-sessions. diff --git a/public/session-state.js b/public/session-state.js index deb30814..cc3ce08b 100644 --- a/public/session-state.js +++ b/public/session-state.js @@ -34,7 +34,6 @@ function createSessionState(kind) { function clearExclusive() { busy = false; waitingForInput = false; - attention = false; responseReady = false; } diff --git a/public/sidebar.js b/public/sidebar.js index 08389818..cb025de6 100644 --- a/public/sidebar.js +++ b/public/sidebar.js @@ -249,6 +249,11 @@ function reflectSubagentRunningState(parentSessionId, agentId) { } const caret = document.getElementById(caretIdFor(parentSessionId)); if (caret) caret.classList.toggle('has-running-child', parentHasActiveSubagent(parentSessionId)); + // Mirror the live-subagent count into the local-pty adapter — see .ai/contexts/session-state.md ("The local-pty adapter") + if (typeof syncLocalPtyAgentsBusy === 'function') { + const localMap = activeSubagentsByParent.get(parentSessionId); + syncLocalPtyAgentsBusy(parentSessionId, !!(localMap && localMap.size > 0)); + } // Parent session item: "subagents are working under this session" indicator. // Unlike the caret badge, the parent item is always visible, so this shows // whether the subagent group is expanded or collapsed. CSS gives the diff --git a/test/activity-trace-renderer.test.js b/test/activity-trace-renderer.test.js index 72d40bfb..29b3fff8 100644 --- a/test/activity-trace-renderer.test.js +++ b/test/activity-trace-renderer.test.js @@ -95,6 +95,18 @@ test('an enabled trace forwards each mutation with its before/after and caller', const cls = sent.find(e => e.cat === 'class.apply'); assert.equal(cls.fields.el, 'si-s1'); assert.equal(cls.fields['cli-busy'], true); + assert.equal(cls.fields['needs-attention'], false, 'class.apply carries needs-attention on every kind now, not just cli-busy/response-ready'); +}); + +test('class.apply — not class.toggle — is the needs-attention writer (docs/activity-trace.md)', () => { + const { sent, run } = setup({ traceEnabled: true }); + run('setAttention("s1", true, "onTerminalNotification")'); + + const cls = sent.find(e => e.cat === 'class.apply'); + assert.ok(cls, 'setAttention must go through the same applyStateClasses projection as busy/responseReady'); + assert.equal(cls.fields['needs-attention'], true); + assert.equal(sent.some(e => e.cat === 'class.toggle' && e.fields && e.fields.cls === 'needs-attention'), false, + 'needs-attention is no longer written via class.toggle — see docs/activity-trace.md'); }); test('an enabled trace records the response-ready lock that swallows an idle', () => { diff --git a/test/dom-setup.js b/test/dom-setup.js index 4e21b5d6..fe2e65a7 100644 --- a/test/dom-setup.js +++ b/test/dom-setup.js @@ -131,9 +131,10 @@ function setupSidebarDom() { evalInWindow(dom, path.join(PUBLIC_DIR, 'subagent-timing.js')); // session-state.js (pure domain) + session-activity-dom.js (DOM projection) - // + session-activity.js (Maps/Sets, setActivity/purgeActivityFor) — load - // order mirrors index.html. sidebar.js and remote-activity-ui.js call - // setActivity/applyActivityClasses/sessionItemEl from these. + // + session-activity.js (the local-pty adapter: localPtyStates, + // setActivity/setAttention/purgeActivityFor) — load order mirrors + // index.html. sidebar.js and remote-activity-ui.js call + // setActivity/applyStateClasses/sessionItemEl from these. evalInWindow(dom, path.join(PUBLIC_DIR, 'session-state.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'session-activity-dom.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'session-activity.js')); diff --git a/test/local-pty-adapter.test.js b/test/local-pty-adapter.test.js new file mode 100644 index 00000000..aba9abd0 --- /dev/null +++ b/test/local-pty-adapter.test.js @@ -0,0 +1,321 @@ +// Tests for the local-pty adapter in public/session-activity.js — see +// .ai/contexts/session-state.md ("The local-pty adapter"). Same eval-in-jsdom +// technique as test/remote-session-adapter.test.js and +// test/local-transcript-adapter.test.js: the real session-state.js / +// session-activity-dom.js / session-activity.js, loaded in index.html order. +// +// test/session-activity.test.js already pins the caller-facing contract +// (setActivity/clearUnread/rekeyActivityState/purgeActivityFor/reconcileBusyState +// signatures and the response-ready-holds-idle rule). This file pins the +// adapter's own persisted state: one createSessionState('local-pty') per +// session id, event sequence -> snapshot, and that busy/responseReady/ +// attention exclusivity is now enforced by the domain, not reconstructable +// through the public API. + +'use strict'; +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const { JSDOM } = require('jsdom'); + +const STATE_SRC = path.join(__dirname, '..', 'public', 'session-state.js'); +const DOM_SRC = path.join(__dirname, '..', 'public', 'session-activity-dom.js'); +const SRC = path.join(__dirname, '..', 'public', 'session-activity.js'); + +function setup(sessionIds = ['s1']) { + const items = sessionIds + .map(id => `
`) + .join(''); + const dom = new JSDOM(`${items}`, + { url: 'http://localhost/', runScripts: 'outside-only' }); + const { window } = dom; + + Object.defineProperty(window, 'activeSessionId', { value: null, writable: true, configurable: true }); + + const ctx = dom.getInternalVMContext(); + vm.runInContext(fs.readFileSync(STATE_SRC, 'utf8'), ctx, { filename: STATE_SRC }); + vm.runInContext(fs.readFileSync(DOM_SRC, 'utf8'), ctx, { filename: DOM_SRC }); + vm.runInContext(fs.readFileSync(SRC, 'utf8'), ctx, { filename: SRC }); + + const read = (expr) => vm.runInContext(expr, ctx); + return { + window, + document: window.document, + item: (id) => window.document.querySelector(`.session-item[data-session-id="${id}"]`), + icon: (id) => window.document.querySelector(`.session-item[data-session-id="${id}"] .session-icon`), + setActivity: read('setActivity'), + clearUnread: read('clearUnread'), + setAttention: read('setAttention'), + syncLocalPtyAgentsBusy: read('syncLocalPtyAgentsBusy'), + purgeActivityFor: read('purgeActivityFor'), + rekeyActivityState: read('rekeyActivityState'), + snapshot: (id) => vm.runInContext(`localPtyState(${JSON.stringify(id)}).snapshot()`, ctx), + hasState: (id) => vm.runInContext(`localPtyStates.has(${JSON.stringify(id)})`, ctx), + destroy: () => window.close(), + }; +} + +// --------------------------------------------------------------------------- +// Event sequence -> snapshot +// --------------------------------------------------------------------------- + +test('a fresh session has no persisted state until first touched', () => { + const t = setup(); + assert.equal(t.hasState('s1'), false); + t.destroy(); +}); + +test('setActivity(true) then setActivity(false) drives the persisted snapshot, not a throwaway one', () => { + const t = setup(); + t.window.activeSessionId = 's2'; // s1 unfocused + + t.setActivity('s1', true, 'onCliBusyState'); + assert.equal(t.snapshot('s1').busy, true); + assert.equal(t.snapshot('s1').kind, 'local-pty'); + + t.setActivity('s1', false, 'onCliBusyState'); + const snap = t.snapshot('s1'); + assert.equal(snap.busy, false); + assert.equal(snap.waitingForInput, true); + assert.equal(snap.responseReady, true); + + t.destroy(); +}); + +test('setAttention feeds the same persisted state (event sequence -> snapshot)', () => { + const t = setup(); + t.setAttention('s1', true, 'onTerminalNotification'); + assert.equal(t.snapshot('s1').attention, true); + assert.ok(t.item('s1').classList.contains('needs-attention')); + + t.setAttention('s1', false, 'clearNotifications'); + assert.equal(t.snapshot('s1').attention, false); + assert.ok(!t.item('s1').classList.contains('needs-attention')); + + t.destroy(); +}); + +test('syncLocalPtyAgentsBusy feeds agentsBusy into an existing persisted state', () => { + const t = setup(); + t.setActivity('s1', true); // an entry must already exist — see the next test + + t.syncLocalPtyAgentsBusy('s1', true); + assert.equal(t.snapshot('s1').agentsBusy, true); + + t.syncLocalPtyAgentsBusy('s1', false); + assert.equal(t.snapshot('s1').agentsBusy, false); + t.destroy(); +}); + +// Adversarial review of PR #282 (item 4): syncLocalPtyAgentsBusy used to call +// the auto-vivifying localPtyState(), leaking a blank entry for every parent +// id reflectSubagentRunningState is ever called with — including a parent +// whose row is gone, filtered, or that will never be a local-pty row at all. +test('syncLocalPtyAgentsBusy never creates a state for a parent with no existing entry', () => { + const t = setup(); + assert.equal(t.hasState('s1'), false, 'precondition: untouched'); + + t.syncLocalPtyAgentsBusy('s1', true); + + assert.equal(t.hasState('s1'), false, 'must not auto-vivify — see .ai/contexts/session-state.md ("The local-pty adapter")'); + t.destroy(); +}); + +test('snapshot() is complete for a local row: busy, attention and agentsBusy all present together', () => { + const t = setup(); + t.setActivity('s1', true); + t.syncLocalPtyAgentsBusy('s1', true); + const snap = t.snapshot('s1'); + assert.equal(snap.busy, true); + assert.equal(snap.agentsBusy, true, 'agentsBusy survives an unrelated busy transition'); + t.destroy(); +}); + +// --------------------------------------------------------------------------- +// Exclusivity now enforced by the domain — impossible through the public API +// --------------------------------------------------------------------------- + +test('busy and response-ready can never both be true through the public API', () => { + const t = setup(); + t.window.activeSessionId = 's2'; + + for (const active of [true, false, true, false, true]) { + t.setActivity('s1', active); + const snap = t.snapshot('s1'); + assert.ok(!(snap.busy && snap.responseReady), 'busy and responseReady must never coexist'); + } + t.destroy(); +}); + +test('attention fired while busy clears busy — an attention edge still wins', () => { + const t = setup(); + t.setActivity('s1', true); + assert.equal(t.snapshot('s1').busy, true); + + t.setAttention('s1', true, 'onTerminalNotification'); + const snap = t.snapshot('s1'); + assert.equal(snap.attention, true); + assert.equal(snap.busy, false, 'an attention EVENT still clears busy — only a later busy edge must not clear attention back (see the next test)'); + t.destroy(); +}); + +// Regression (adversarial review of PR #282): an OSC-0 busy title arriving +// while a permission prompt is open (two independent IPC streams) used to +// wipe needs-attention. Decided: attention is cleared only by an explicit +// attention:false (clearNotifications) — never by a busy edge. See +// .ai/contexts/session-state.md ("The local-pty adapter"). +test('a busy edge after attention does NOT clear attention — attention is orthogonal to busy', () => { + const t = setup(); + t.setAttention('s1', true, 'onTerminalNotification'); + assert.equal(t.snapshot('s1').attention, true); + + t.setActivity('s1', true, 'onCliBusyState'); // OSC 0 busy title fires independently + const busySnap = t.snapshot('s1'); + assert.equal(busySnap.attention, true, 'a busy edge must never clear attention'); + assert.equal(busySnap.busy, true); + assert.ok(t.item('s1').classList.contains('needs-attention')); + assert.ok(t.item('s1').classList.contains('cli-busy')); + + t.setAttention('s1', false, 'clearNotifications'); // only an explicit clear removes it + const clearedSnap = t.snapshot('s1'); + assert.equal(clearedSnap.attention, false); + assert.equal(clearedSnap.busy, true, 'busy remains untouched by the attention clear'); + assert.ok(!t.item('s1').classList.contains('needs-attention')); + assert.ok(t.item('s1').classList.contains('cli-busy')); + t.destroy(); +}); + +test('attention while response-ready clears response-ready — cannot force the combination', () => { + const t = setup(); + t.window.activeSessionId = 's2'; + t.setActivity('s1', true); + t.setActivity('s1', false); // response-ready armed + assert.equal(t.snapshot('s1').responseReady, true); + + t.setAttention('s1', true, 'onTerminalNotification'); + const snap = t.snapshot('s1'); + assert.equal(snap.attention, true); + assert.equal(snap.responseReady, false, 'the two are exclusive by construction, not by caller discipline'); + t.destroy(); +}); + +test('decided (not a regression): attention after response-ready, then clearing attention does not restore response-ready', () => { + // Behavior change from the pre-migration independent Sets, documented in + // .ai/contexts/session-state.md ("Decided: attention supersedes and + // consumes the unseen-response state"). Old behavior: responseReadySessions + // and attentionSessions were independent, so clearing attention re-exposed + // response-ready underneath it. New behavior: attention:true calls the + // domain's clearExclusive(), erasing responseReady, not just outshining it + // — clearing attention afterwards lands on plain idle, never back on + // response-ready. + const t = setup(); + t.window.activeSessionId = 's2'; // s1 unfocused, the case that arms response-ready + + t.setActivity('s1', true); + t.setActivity('s1', false); // idle, unseen -> response-ready armed + assert.equal(t.snapshot('s1').responseReady, true, 'precondition: response-ready armed'); + + t.setAttention('s1', true, 'onTerminalNotification'); // OSC 9 interrupts it + assert.equal(t.snapshot('s1').attention, true); + assert.equal(t.snapshot('s1').responseReady, false, 'response-ready consumed, not merely outshone'); + + t.setAttention('s1', false, 'clearNotifications'); // user handles it, attention clears + const snap = t.snapshot('s1'); + assert.equal(snap.attention, false); + assert.equal(snap.responseReady, false, 'must NOT fall back to response-ready — that fact is gone, not hidden'); + assert.ok(!t.item('s1').classList.contains('response-ready')); + assert.ok(!t.item('s1').classList.contains('needs-attention')); + t.destroy(); +}); + +// --------------------------------------------------------------------------- +// Decided: a local row idling while active (or armReady:false) now shows +// "Waiting for input", not "Idle" — see .ai/contexts/session-state.md +// ("The local-pty adapter"). +// --------------------------------------------------------------------------- + +test('decided: a local row going idle while active shows "Waiting for input", not "Idle"', () => { + const t = setup(); + t.window.activeSessionId = 's1'; // s1 IS the focused session + + t.setActivity('s1', true); + t.setActivity('s1', false); // idle while active -> must not arm response-ready + const snap = t.snapshot('s1'); + assert.equal(snap.busy, false); + assert.equal(snap.waitingForInput, true); + assert.equal(snap.responseReady, false); + + const icon = t.icon('s1'); + assert.ok(icon.classList.contains('session-icon--waiting')); + assert.equal(icon.title, 'Waiting for input'); + t.destroy(); +}); + +test('decided: a local row going idle NOT active still arms response-ready (unchanged)', () => { + const t = setup(); + t.window.activeSessionId = 's2'; // s1 not focused + + t.setActivity('s1', true); + t.setActivity('s1', false); + const snap = t.snapshot('s1'); + assert.equal(snap.responseReady, true); + + const icon = t.icon('s1'); + assert.ok(icon.classList.contains('session-icon--response-ready')); + assert.equal(icon.title, 'Response ready'); + t.destroy(); +}); + +// --------------------------------------------------------------------------- +// rekey/purge move/drop the whole persisted state, not per-field collections +// --------------------------------------------------------------------------- + +test('rekeyActivityState moves the whole persisted state object, not a copy', () => { + const t = setup(['old', 'new']); + t.setActivity('old', true); + t.syncLocalPtyAgentsBusy('old', true); + + t.rekeyActivityState('old', 'new'); + + assert.equal(t.hasState('old'), false, 'the old id carries no state after rekey'); + assert.equal(t.hasState('new'), true); + const snap = t.snapshot('new'); + assert.equal(snap.busy, true); + assert.equal(snap.agentsBusy, true, 'every facet of the state moves together, not just busy'); + t.destroy(); +}); + +test('purgeActivityFor drops the whole persisted state object', () => { + const t = setup(); + t.setActivity('s1', true); + t.setAttention('s1', true, 'x'); + assert.equal(t.hasState('s1'), true); + + t.purgeActivityFor('s1', 'pty-gone'); + + assert.equal(t.hasState('s1'), false, 'the entire state is dropped, not individually cleared fields'); + t.destroy(); +}); + +// --------------------------------------------------------------------------- +// Mutation target: setActivity must go through state.apply(), not write the +// legacy view directly — bypassing the domain drops waitingForInput/ +// responseReady, fields only apply() knows how to set. +// --------------------------------------------------------------------------- + +test('mutation guard: setActivity must arm waitingForInput via apply(), not merely flip a busy boolean', () => { + // If setActivity were rewritten to call sessionBusyState.set(id, active) + // instead of localPtyState(id).apply({ type: 'busy', ... }), this goes red: + // waitingForInput/responseReady only exist because apply() derives them, + // a raw boolean flip on the legacy view cannot produce them. + const t = setup(); + t.window.activeSessionId = 's2'; + t.setActivity('s1', true); + t.setActivity('s1', false); + const snap = t.snapshot('s1'); + assert.equal(snap.waitingForInput, true, 'waitingForInput must come from apply(), not a bypassed write'); + assert.equal(snap.responseReady, true); + t.destroy(); +}); diff --git a/test/remote-row-ownership.test.js b/test/remote-row-ownership.test.js index c9d452bf..eef33d15 100644 --- a/test/remote-row-ownership.test.js +++ b/test/remote-row-ownership.test.js @@ -130,3 +130,20 @@ test('symptom 2 (#273): pty.exit of a remote attach clears busy at once, no 20s assert.ok(!t.item('s1').classList.contains('cli-busy'), 'a stale seed window must not re-arm busy after the handoff'); t.destroy(); }); + +// Adversarial review of PR #282 (item 4): the true->false handoff routes +// through setActivity() (the two-legacy-reader dual feed), which creates a +// shadow localPtyStates entry for the remote id — left alone, that entry is +// never purged, since app.js's pty-set purge explicitly skips remote rows +// (dataset.remoteAlias). See .ai/contexts/session-state.md ("The local-pty adapter"). +test('setRemoteAttached(id, false) purges the shadow local-pty entry it just fed, not just the busy flag', () => { + const t = setup(['s1']); + t.emit({ sessionId: 's1', at: Date.now() }); + t.setRemoteAttached('s1', true); + assert.equal(t.sessionBusyState.has('s1'), true, 'precondition: the dual-feed created a shadow entry'); + + t.setRemoteAttached('s1', false); + + assert.equal(t.sessionBusyState.has('s1'), false, 'the shadow local-pty entry must be dropped on detach, not left idle forever'); + t.destroy(); +}); diff --git a/test/session-activity.test.js b/test/session-activity.test.js index 2c42b388..328a6868 100644 --- a/test/session-activity.test.js +++ b/test/session-activity.test.js @@ -139,19 +139,19 @@ test('a focused session going idle is not marked response-ready', () => { t.destroy(); }); -test('clearUnread re-exposes the spinner when the session is still generating', () => { +test('clearUnread does not clear cli-busy when the session is generating again', () => { const t = setup(); t.window.activeSessionId = 's2'; t.setActivity('s1', true); - t.setActivity('s1', false); - t.setActivity('s1', true); // busy again, marker already dropped - t.responseReadySessions.add('s1'); // force the stale combination - t.item('s1').classList.add('response-ready'); + t.setActivity('s1', false); // response-ready armed + t.setActivity('s1', true); // busy again — going busy already drops the marker + assert.ok(!t.responseReadySessions.has('s1'), 'precondition: marker already cleared by the busy transition itself'); + assert.ok(t.item('s1').classList.contains('cli-busy'), 'precondition: busy'); t.clearUnread('s1'); - assert.ok(t.item('s1').classList.contains('cli-busy'), 'cli-busy restored from sessionBusyState'); + assert.ok(t.item('s1').classList.contains('cli-busy'), 'cli-busy untouched by clearUnread'); assert.ok(!t.item('s1').classList.contains('response-ready')); t.destroy(); @@ -208,27 +208,40 @@ test('armReady:false does not block the response-ready CLEAR when the session go // F7: purgeActivityFor — the single writer for the PTY-gone purge // --------------------------------------------------------------------------- -test('purgeActivityFor drops busy/unread/attention state and their classes', () => { +test('purgeActivityFor drops response-ready state and its class', () => { const t = setup(); t.window.activeSessionId = 's2'; t.setActivity('s1', true); t.setActivity('s1', false); // response-ready armed - t.attentionSessions.add('s1'); - t.item('s1').classList.add('needs-attention'); - assert.ok(t.responseReadySessions.has('s1') && t.attentionSessions.has('s1'), 'preconditions'); + assert.ok(t.responseReadySessions.has('s1'), 'precondition'); t.purgeActivityFor('s1', 'pty-gone'); - assert.ok(!t.attentionSessions.has('s1'), 'attentionSessions cleared'); assert.ok(!t.responseReadySessions.has('s1'), 'responseReadySessions cleared'); assert.ok(!t.sessionBusyState.has('s1'), 'sessionBusyState cleared'); - assert.ok(!t.item('s1').classList.contains('needs-attention')); assert.ok(!t.item('s1').classList.contains('response-ready')); assert.ok(!t.item('s1').classList.contains('cli-busy')); t.destroy(); }); +test('purgeActivityFor drops attention state and its class', () => { + // Attention alone (not combined with busy/response-ready — the domain's own + // exclusivity invariant, session-state.js apply(), makes that combination + // unreachable through the public API; see test/local-pty-adapter.test.js). + const t = setup(); + + t.attentionSessions.add('s1'); + t.item('s1').classList.add('needs-attention'); + assert.ok(t.attentionSessions.has('s1'), 'precondition'); + + t.purgeActivityFor('s1', 'pty-gone'); + + assert.ok(!t.attentionSessions.has('s1'), 'attentionSessions cleared'); + assert.ok(!t.item('s1').classList.contains('needs-attention')); + t.destroy(); +}); + test('purgeActivityFor on a busy session removes .cli-busy too', () => { const t = setup(); t.setActivity('s1', true); @@ -262,23 +275,37 @@ test('rekeyActivityState carries busy state and DOM class from oldId to newId', t.destroy(); }); -test('rekeyActivityState carries response-ready and needs-attention too', () => { +test('rekeyActivityState carries response-ready too', () => { const t = setup(['old', 'new']); t.window.activeSessionId = 'other'; t.setActivity('old', true); t.setActivity('old', false); - t.attentionSessions.add('old'); - t.item('old').classList.add('needs-attention'); assert.ok(t.responseReadySessions.has('old')); t.rekeyActivityState('old', 'new'); assert.ok(t.responseReadySessions.has('new') && !t.responseReadySessions.has('old')); - assert.ok(t.attentionSessions.has('new') && !t.attentionSessions.has('old')); assert.ok(t.item('new').classList.contains('response-ready')); - assert.ok(t.item('new').classList.contains('needs-attention')); assert.ok(!t.item('old').classList.contains('response-ready')); + + t.destroy(); +}); + +test('rekeyActivityState carries needs-attention too', () => { + // Attention alone — see the "purgeActivityFor drops attention state" note + // above for why it is not combined with response-ready here. + const t = setup(['old', 'new']); + t.window.activeSessionId = 'other'; + + t.attentionSessions.add('old'); + t.item('old').classList.add('needs-attention'); + assert.ok(t.attentionSessions.has('old')); + + t.rekeyActivityState('old', 'new'); + + assert.ok(t.attentionSessions.has('new') && !t.attentionSessions.has('old')); + assert.ok(t.item('new').classList.contains('needs-attention')); assert.ok(!t.item('old').classList.contains('needs-attention')); t.destroy(); @@ -378,8 +405,10 @@ test('reconcileBusyState ignores malformed payloads', () => { test('busy wins over response-ready: the two classes are mutually exclusive by construction', () => { // Decision: a session that resumed generating is busy, not "answer waiting". - // applyActivityClasses is the only writer of both classes and never sets - // them together, so the CSS cascade is never asked to arbitrate. + // applyStateClasses (session-activity-dom.js) is the only writer of both + // classes, and the domain's own exclusivity invariant (session-state.js + // apply()) keeps them from ever being true together, so the CSS cascade is + // never asked to arbitrate. const t = setup(); t.window.activeSessionId = 's2'; diff --git a/test/session-state.test.js b/test/session-state.test.js index 38b0cd0b..f0219bc6 100644 --- a/test/session-state.test.js +++ b/test/session-state.test.js @@ -104,17 +104,17 @@ test('exclusivity: attention while busy clears busy and any pending unread', () assert.equal(snap.responseReady, false); }); -test('exclusivity: going busy again clears attention and waitingForInput/responseReady', () => { +test('exclusivity: going busy again clears waitingForInput/responseReady but NOT attention (decided: attention is cleared only by an explicit attention:false, never by a busy edge)', () => { const s = createSessionState('local-pty'); s.apply({ type: 'attention', active: true }); s.apply({ type: 'busy', active: true }); const snap = s.snapshot(); assert.equal(snap.busy, true); - assert.equal(snap.attention, false); + assert.equal(snap.attention, true, 'attention outranks busy in the priority ladder and survives a busy edge — see .ai/contexts/session-state.md'); assert.equal(snap.waitingForInput, false); }); -test('exclusivity: at most one of busy/waitingForInput/attention is ever true', () => { +test('exclusivity: at most one of busy/waitingForInput is ever true; attention is orthogonal to busy', () => { const s = createSessionState('local-pty'); const events = [ { type: 'busy', active: true }, @@ -128,11 +128,34 @@ test('exclusivity: at most one of busy/waitingForInput/attention is ever true', for (const e of events) { s.apply(e); const snap = s.snapshot(); - const trueCount = [snap.busy, snap.waitingForInput, snap.attention].filter(Boolean).length; - assert.ok(trueCount <= 1, `busy/waitingForInput/attention must stay exclusive, got ${trueCount} true after ${JSON.stringify(e)}`); + const trueCount = [snap.busy, snap.waitingForInput].filter(Boolean).length; + assert.ok(trueCount <= 1, `busy/waitingForInput must stay exclusive, got ${trueCount} true after ${JSON.stringify(e)}`); } }); +test('exclusivity: attention set while already busy leaves busy untouched — orthogonal in both directions', () => { + const s = createSessionState('local-pty'); + s.apply({ type: 'busy', active: true }); + s.apply({ type: 'attention', active: true }); + s.apply({ type: 'busy', active: true }); // a second, independent busy edge (e.g. re-armed OSC 0 title) + const snap = s.snapshot(); + assert.equal(snap.busy, true); + assert.equal(snap.attention, true, 'a busy edge must never clear attention'); +}); + +test('exclusivity: attention is exclusive with responseReady only — clearing attention does not resurrect it', () => { + const s = createSessionState('local-pty'); + s.apply({ type: 'busy', active: true }); + s.apply({ type: 'busy', active: false, armReady: true }); // idle, unseen -> responseReady armed + assert.equal(s.snapshot().responseReady, true); + + s.apply({ type: 'attention', active: true }); + assert.equal(s.snapshot().responseReady, false, 'attention consumes the pending unread state'); + + s.apply({ type: 'attention', active: false }); + assert.equal(s.snapshot().responseReady, false, 'clearing attention must not restore responseReady — see .ai/contexts/session-state.md'); +}); + // --------------------------------------------------------------------------- // Priority order (attention > responseReady > busy > agentsBusy > // waitingForInput > idle+age > stale > archived)