From 2099c52a2e8181f52af7541fa1826678d07b0cf0 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Fri, 11 Sep 2026 22:06:50 +0200 Subject: [PATCH] (session-state): one icon slot per sidebar row, resolved by the domain module Step 3b of #246. Each session and subagent row carries one .session-icon element written only by the projection file from renderSessionIcon(); the status dot and the spinner slot are absorbed into it. The rung is resolved in session-state.js and exposed as a session-icon-- class, so the CSS keys on that class alone: the row-class :not() chains are gone, and a source test keeps them out. Row classes stay for tint and tests; the age text stays next to the slot for local and remote rows alike. --- .ai/contexts/session-state.md | 158 ++++++++++++++++-- eslint.config.js | 7 +- public/app.js | 6 +- public/session-activity-dom.js | 45 ++++- public/session-state.js | 21 +-- public/sidebar.js | 23 ++- public/style.css | 51 +++--- test/dom-grid-sidebar-prune-collision.test.js | 3 + test/dom-sidebar-icon-slot.test.js | 141 ++++++++++++++++ test/dom-sidebar-subagent-running.test.js | 4 +- test/dom-subagent-transcript.test.js | 3 + test/dom-subagent-ttl-tick.test.js | 3 + test/running-indicators.test.js | 12 +- test/session-activity.test.js | 23 +-- test/session-icon-slot-css-boundary.test.js | 59 +++++++ test/session-state.test.js | 36 +++- test/sidebar-busy-agents-tint.test.js | 31 ++-- 17 files changed, 533 insertions(+), 93 deletions(-) create mode 100644 test/dom-sidebar-icon-slot.test.js create mode 100644 test/session-icon-slot-css-boundary.test.js diff --git a/.ai/contexts/session-state.md b/.ai/contexts/session-state.md index 85b2a6af..2972401e 100644 --- a/.ai/contexts/session-state.md +++ b/.ai/contexts/session-state.md @@ -6,17 +6,19 @@ what actually shipped, not the whole plan. ## Migration status -- **Steps 1-3: done.** `public/session-activity.js` split into a state part +- **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. -- **Steps 3b/4/5: pending.** The unified icon-slot markup (dot + age + spinner - in one element) is a separate PR (3b). There is no `local-transcript` - adapter (step 4). Subagent attribution is not routed through - `session-state.js` (`agentsBusy` exists in the model but nothing local-pty feeds - it yet — sidebar.js's `has-busy-agents` is still computed by - `parentHasActiveSubagent()`, independent of the domain module) (step 5). + 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. +- **Steps 4/5: pending.** There is no `local-transcript` adapter (step 4). + Subagent attribution is not routed through `session-state.js` (`agentsBusy` + exists in the model but nothing local-pty feeds it yet — sidebar.js's + `has-busy-agents` row class is still computed by `parentHasActiveSubagent()`, + independent of the domain module) (step 5). ### The remote-ssh adapter (step 3) @@ -104,11 +106,141 @@ mutually exclusive — going busy or attention clears the other two (and `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, glyph, title }` for **one icon slot**. Only the four -rungs that map to an existing CSS class (`needs-attention`, `response-ready`, -`cli-busy`, `has-busy-agents`) carry a class today; the rest carry a glyph/title -only — the sidebar HTML/CSS shape (replacing the dot/pip with the icon slot) -is a later step, not part of this migration. +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 +(`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. + +## The icon slot (step 3b) + +One `.session-icon` element per session/subagent row (`public/sidebar.js`'s +`buildSessionItem`/`buildSubagentItem`), replacing `.session-status-dot` for +those two rows — `.remote-host-dot` (the project header's per-host +reachability indicator, a different row entirely) is untouched and still +composes `class="session-status-dot remote-host-dot ..."`; both base CSS +rules (`.session-status-dot`, `.session-icon`) exist side by side in +`style.css` for exactly this reason. `.session-icon`'s box (6px, same as the +old dot) is fixed regardless of rung, so the row never shifts as the glyph +underneath it changes. + +`session-activity-dom.js` is the only file that writes the slot: + +- `writeIconSlot(el, icon)` clears any previous `session-icon--*` class + (robust to a rung's `slotClasses` shrinking or changing shape — a plain + diff of the old vs. new class would also work but this needs no diffing), + applies `icon.slotClasses`, and sets `el.title`/`el.dataset.glyph` from + `icon.title`/`icon.glyph`. The `session-icon--*` namespace is deliberately + 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. +- `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 + 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`. + +The `.running` class on `.session-icon` is **not** part of this model — it is +the old dot's orthogonal "a PTY/subagent process is attached" boolean +(`activePtyIds`/`isSubagentActive`), toggled exactly as before by +`app.js`/`sidebar.js`, independent of `renderSessionIcon`'s priority ladder. + +### The slot's CSS keys on its own rung class alone (coordinator follow-up, 2026-09-11) + +First pass of this step left `style.css` still resolving the slot's visual +with row-class `:not()` chains carried over verbatim from the old +`.session-status-dot` rules (`.session-item.cli-busy:not(.needs-attention) +.session-icon`, `.session-item.has-busy-agents:not(.cli-busy):not(...):not(...) +.session-icon`) — exactly the row-class arbitration this migration exists to +retire, just renamed. `renderSessionIcon()` already picks the single active +rung in JS before `writeIconSlot()` ever touches the DOM, so the CSS never +needs to re-derive it: every rung's visual now keys on its own +`.session-icon--` class alone, no ancestor `:not()` chain — + +```css +.session-icon--busy { background: transparent !important; animation: none !important; ... } +.session-icon--busy::before { content: "\280B"; animation: braille-spin ...; color: #4fc3f7; ... } +.session-icon--agents-busy { background: transparent !important; ... } +.session-icon--agents-busy::before { content: "\283F"; color: #8088ff; ... } +.session-icon--response-ready { background: #4fc3f7 !important; ... } +.session-icon--attention { background: #f0a050 !important; ... } +``` + +`background` (the only property `.session-icon.running`'s plain green also +sets) carries `!important` on every rung — busy/attention/response-ready/ +agentsBusy rows are very often also `.running` (a busy session almost always +has a live PTY), and unlike the old code the rung is now a single class with +no extra row-ancestor classes to lean on for specificity. + +**`has-busy-agents` is the one exception, and it stays row-level on purpose** +(point 1 of the original brief: "`.has-busy-agents` stays a row-level class +— a tint — but its glyph rung lives in the slot when nothing higher is +active"). A busy session with live subagents needs the busy spinner tinted +violet instead of its default blue; since `agentsBusy` never wins the JS +priority race while `busy` is active, there is no `session-icon--agents-busy` +class to key on in that case — so the tint is a **plain compound selector**, +row tint class + slot rung class, not a priority tie-break: + +```css +.session-item.has-busy-agents .session-icon--busy::before { color: #8088ff; } +``` + +This is the only place a row-level class still appears next to `.session-icon` +in the slot's own visual rules, and `test/session-icon-slot-css-boundary.test.js` +allows it explicitly (it only forbids `:not(`, not row classes generally) — +the boundary is "no priority re-arbitration in CSS", not "no row class may +ever touch the slot's selector". + +`test/session-icon-slot-css-boundary.test.js` source-greps `style.css` +(comments stripped first — a couple of them, including the one you're +reading in source, mention `:not()` in prose and would otherwise +false-positive a naive scan) for any selector mentioning `.session-icon` and +asserts none contains `:not(`; `.session-status-dot`/`.remote-host-dot` (the +project header's per-host reachability dot, a different row, untouched by +this migration) are excluded. Mutation-proven: reintroducing +`.session-item.cli-busy:not(.needs-attention) .session-icon--busy { ... }` +turns it red. + +`test/dom-sidebar-icon-slot.test.js` pins the slot markup itself (mutation- +proven: swapping the `busy`/`agentsBusy` entries in `ICON_BY_RUNG` turns three +of its DOM tests red, plus the pure-unit tests in `test/session-state.test.js`). +The five tests named in issue #246's step 3b brief +(`dom-sidebar-local-status`, `dom-sidebar-remote-session`, +`dom-sidebar-remote-freshness`, `sidebar-busy-agents-tint`, +`dom-sidebar-remote-activity-pip`) needed no markup changes beyond +`sidebar-busy-agents-tint.test.js`'s CSS-selector regexes (retargeted first +from `.session-status-dot` to `.session-icon`, then from the row-class +`:not()` chains to the flat `session-icon--` selectors above) — none of +the others assert on the dot/slot element itself, only on row classes and +`.session-status`. ## Design notes (deviations from the issue's literal text) diff --git a/eslint.config.js b/eslint.config.js index 3b37eb45..6b5580c4 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -154,15 +154,20 @@ const rendererCrossFileGlobals = { createSessionState: 'readonly', renderSessionIcon: 'readonly', // public/session-activity-dom.js — the only file allowed to write - // .cli-busy/.needs-attention/.response-ready/.has-busy-agents. + // .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', setCliBusy: 'readonly', setHasBusyAgents: 'readonly', + paintSessionIcon: 'readonly', // public/remote-activity-ui.js (remote-ssh adapter, see .ai/contexts/session-state.md) setRemoteAttached: 'readonly', + // public/sidebar.js, consumed by session-activity-dom.js's snapshotForLocal + // (see .ai/contexts/session-state.md, "The icon slot (step 3b)") + parentHasActiveSubagent: 'readonly', // Third-party renderer libs loaded as