Skip to content

(session-state): pure domain module, single DOM writer, enforced by lint - #266

Merged
devsuitup merged 2 commits into
mainfrom
feat/246-session-state-domain
Sep 11, 2026
Merged

devsuitup merged 2 commits into
mainfrom
feat/246-session-state-domain

Conversation

@devsuitup

Copy link
Copy Markdown
Owner

Steps 1-2 of #246 (migration order in the issue). Steps 3-5 follow in separate PRs.

What

  • public/session-activity.js is split: it keeps the Maps/Sets, setActivity, purgeActivityFor, rekey/reconcile; the new public/session-activity-dom.js holds sessionItemEl, applyActivityClasses and four setters that are the only code allowed to write .cli-busy / .needs-attention / .response-ready / .has-busy-agents. The direct writers in app.js (3) and sidebar.js (4) are routed through them.
  • public/session-state.js: pure, dual-loadable domain module. createSessionState(kind).apply(event).snapshot() with liveness and attached as two separate facts (lifecycle decision recorded on the issue), exclusivity of busy / waitingForInput / attention, and the priority order. renderSessionIcon(snapshot) gives classes, glyph and title for one icon slot; only its classes are consumed for now, behind applyActivityClasses, local-pty only. Local DOM output is unchanged.
  • Enforcement: no-restricted-syntax in eslint.config.js (classList add/remove/toggle with those class names outside the projection file) and test/session-state-boundary.test.js (no document / window / electron / ipcRenderer in the domain source).
  • .ai/contexts/session-state.md (shape, ports, enforcement, migration status), linked from the contexts README and the orientation table.

Two deviations from the issue text, documented in the context note: a responseReady field (the "finished, not yet seen" rung is distinct from waitingForInput), and the seen/unseen flag stays caller-supplied (armReady) because attached means "has a PTY", not "is the focused tab".

Test infra changes: loaders updated for the split files; one source-regex pin (sidebar-busy-agents-tint.test.js) rewritten against the real module; one pin in running-indicators.test.js updated to the new call shape.

Proof

  • Lint rule: an injected classList.add('cli-busy') in app.js fails npx eslint public/app.js with one no-restricted-syntax error; removed, 0 errors.
  • Mutation: reordering PRIORITY turns three priority tests red.
  • task check: 1176 + 119 pass, 0 fail, 8 pre-existing skips.

Steps 1-2 of #246. Split public/session-activity.js into state and DOM
projection (session-activity-dom.js, the only writer of the four activity
classes); add public/session-state.js (createSessionState / renderSessionIcon,
dual-loadable, liveness and attached as separate facts) behind
applyActivityClasses for local-pty. An eslint no-restricted-syntax rule and a
boundary test make a bypass fail instead of relying on discipline.
@devsuitup

Copy link
Copy Markdown
Owner Author

Review pass (adversarial, six axes): local DOM output, remote rows, boundary test, load order, dual-load clean. One finding kept: the lint rule only matched classList.add/remove/toggle('<literal>'), so className =, innerHTML, setAttribute('class', …) and classList['toggle'](…) went through. Second commit broadens it (string and template literals in those sinks, computed classList[...] refused outright); probe: 6/6 bypass shapes red, unrelated class green, 0 errors on the renderer. A class name held in a variable stays out of lint's reach — stated in the context note. Also noted there: the busy/response-ready tie now resolves as busy (was response-ready), unreachable given setActivity's exclusivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant