build(guardrails): extend check-boundaries to lock in the shell primitives

Part of the ADR-0004 vanilla-shell investment track (see docs/ADR-0004-ui-shell.md); scoped from the 2026-08-03 architecture review.

Goal

Once #586 and #587 land, add mechanical checks to build/check-boundaries.mjs that lock in what they establish — one overlay-lifecycle implementation and a registry-driven panel model — so the six-copy-pasted-overlays problem #586 fixes can't silently regrow the same way it grew the first time.

Context

The repo's layering (src/core/ pure logic, src/net/ injected fetch, src/application/ no UI/editor imports, model/layouts <- application <- UI in src/dashboard/, editor seams, etc.) holds because build/check-boundaries.mjs enforces it mechanically, not because reviewers remember the rules — that's the whole reason the check exists.

Nothing today stops the pattern #586 is fixing from regrowing: #586's own evidence is that six independent overlay lifecycles (cell drawer, rows viewer, doc pane, detached-view, dialog-shell, popover) accumulated over time with five different Escape policies and five different focus-restore patterns, with no gate that would have caught a seventh copy being added. Once #586 collapses those into one SurfaceLifecycle + a docked inspectorHost slot, and #587 makes side panels registry-driven, a new feature that copies the old pattern instead of using the new primitive would pass npm test and tsc --noEmit today with nothing flagging it as a regression.

Deliverables (after #586 and #587 merge)

Add checks to build/check-boundaries.mjs, in its existing deterministic, no-dependency style:

Forbid new panel mounts outside SurfaceLifecycle. Grep-level check that document.body.append/document.body.appendChild (panel-shaped mounts, not arbitrary DOM utility use) appears only inside the SurfaceLifecycle module itself, with an explicit allowlist for surfaces #586 deliberately keeps separate (toast; dialog-shell.ts/popover.ts if they remain distinct implementations for their own documented reasons per #586's body).
Forbid new position: fixed panel CSS in src/styles.css outside an explicit allowlist. #586 deletes the .cd-backdrop overlay CSS as part of its own work; this check stops a new rule with the same shape from being added later.
Assert exactly one overlay-lifecycle implementation. Grep-level check that no new capture-phase Escape/keydown handler implementing panel-close semantics exists outside SurfaceLifecycle (and dialog-shell.ts/popover.ts if the allowlist in (1) keeps them separate) — mirroring #586's finding of five independent Escape policies before this issue's checks existed.

Each check should follow check-boundaries.mjs's existing conventions: deterministic (no external deps, no network), fails with a clear file:line message pointing at the offending site and the allowed alternative, and is fast enough to run in the existing pretest/check:arch step.

Tests

A fixture-style test (matching however check-boundaries.mjs's existing checks are tested — e.g. a scratch file that intentionally violates each new rule) proves each of the three new checks fails on a violation and passes on the current, post-refactor(shell): one SurfaceLifecycle primitive and a docked right-inspector slot #586/refactor(shell): a side-panel registry so adding a navigation panel is a one-file change #587 tree.
npm test (coverage gate, if check-boundaries.mjs's existing checks have unit coverage) and tsc --noEmit pass.
npm run check:arch (or wherever check-boundaries.mjs is wired) runs clean on main after refactor(shell): one SurfaceLifecycle primitive and a docked right-inspector slot #586/refactor(shell): a side-panel registry so adding a navigation panel is a one-file change #587 land, before this issue's new rules are added — establishing the pre-check baseline the new rules must not immediately break.

Acceptance criteria

build/check-boundaries.mjs fails the build if a new panel is mounted via document.body.append/appendChild outside SurfaceLifecycle and the documented allowlist.
build/check-boundaries.mjs fails the build if new position: fixed panel CSS is added to src/styles.css outside the documented allowlist.
build/check-boundaries.mjs fails the build if a new capture-phase Escape/close handler is added outside SurfaceLifecycle (or the allowlisted exceptions).
All three checks run as part of the existing architecture-check step with no new runtime dependency.
npm test (coverage gate) and tsc --noEmit pass.

Non-goals

Any change to SurfaceLifecycle, the registry, or panel behavior itself — this issue only adds enforcement after refactor(shell): one SurfaceLifecycle primitive and a docked right-inspector slot #586/refactor(shell): a side-panel registry so adding a navigation panel is a one-file change #587 establish the pattern.
A general-purpose lint plugin or ESLint rule set — stays in the repo's existing grep/AST-light check-boundaries.mjs style, consistent with its other checks.
Enforcing anything about the left-side panel registry beyond what refactor(shell): a side-panel registry so adding a navigation panel is a one-file change #587 already establishes as the one-file-change contract; this issue's registry-facing check is about the right-inspector/overlay lifecycle specifically, not a general "one file per feature" rule.

Inherited from #586 (phase 1 of #593) — an unguarded JS/CSS constant duplication

Surfaced while reviewing PR #596 and verified. This is the guardrail-shaped part of a finding
whose behavioural half is filed as #597, and it belongs here because mechanically forbidding this
class of regrowth is this issue's whole purpose.

#586's dock-aware width ceiling reserves space for everything beside the inspector:

reservedPx: state.sidebarPx + HANDLE_PX * 2      // src/ui/app-shell.ts:174
const HANDLE_PX = 7;                              // src/ui/app-shell.ts:109

The real handle width is declared independently in CSS:

.col-resize, .inspector-resize { width: 7px; cursor: col-resize; }   /* src/styles.css:901 */

Nothing links the two. A CSS-only edit to the handle width leaves reservedPx wrong, which
silently narrows the centre surface below the minimum CENTRE_MIN_PX is supposed to guarantee —
and no test fails, because happy-dom evaluates no CSS layout and the e2e assertions are
inequalities rather than exact geometry.

This repo already treats JS↔CSS drift as worth a mechanical gate: FONT_BYTE_BUDGET and the whole
type ramp are asserted by tests/unit/typography-contract.test.js precisely so a CSS edit cannot
silently change a JS-side assumption.

Extra acceptance for this inherited item

A layout constant that JS reserves space for and CSS separately declares cannot drift
unnoticed — either single-source it (a CSS custom property or generated token JS reads, or a
JS constant the build injects into CSS) or add a check that fails when the two disagree.
The check covers HANDLE_PX / .col-resize / .inspector-resize as its first case.

refactor(shell): one SurfaceLifecycle primitive and a docked right-inspector slot #586 — SurfaceLifecycle primitive and docked right-inspector slot; this issue depends on it merging first.
refactor(shell): a side-panel registry so adding a navigation panel is a one-file change #587 — side-panel registry; this issue depends on it merging first.
ADR-0004 (docs/ADR-0004-ui-shell.md)

Phase 8 of umbrella #593.

build(#592): lock in #586/#587 shell primitive guardrails

Ship log — #592

Phase	Status	PR
(unphased — single unit)	shipped	#672

#592 — handoff

Landed: three mechanical check:arch guards in build/check-boundaries.mjs /
build/lib/check-legacy-owners.mjs (shell-body-mount, shell-fixed-position,
shell-capture-escape), a shared real-TypeScript-parser batch backing the first and
third, a focused comment/string/escape/brace-aware CSS lexical scanner backing the
second, plus an independent HANDLE_PX/CSS-width drift contract test
(tests/unit/resize-handle-thickness-contract.test.js) for the item inherited from
refactor(shell): one SurfaceLifecycle primitive and a docked right-inspector slot #586/Umbrella: V2 architecture refactor — shell primitives, composition root, state reactivity, transport adapter #593 phase 1. Enforcement-only — no SurfaceLifecycle, side-panel registry,
inspectorHost, or runtime UI/CSS behavior changed.
Decisions taken (not in the issue, resolved during plan authoring/review):
- results.ts's Data-Pane second-Escape-closes-pane handler and both
  explain-graph.ts detail-panel Escape handlers are allowlisted as distinct
  pre-existing surfaces refactor(shell): one SurfaceLifecycle primitive and a docked right-inspector slot #586 never touched, not migrated onto SurfaceLifecycle (that
  would be a runtime behavior change, out of scope here).
- menu.ts's Escape-close is allowlisted as a distinct menu primitive, same
  reasoning.
- Three non-panel capture-Escape gesture-cancellation exclusions (two in
  dashboard-tile-gestures.ts, one — found live during Fable pass 2 — in
  dashboard-chart-interaction.ts's beginSelection) are encoded as a separate
  policy category, not conflated with panel-close exceptions; each is
  occurrence-specific (a second listener in the same scope still fails).
- HANDLE_PX single-sourcing: chose the issue's "add a check" option (an independent
  drift-asserting test) over a CSS-custom-property refactor, to keep this unit
  enforcement-only with zero runtime change.
- Both .append(...) and .appendChild(...) are recognized as body-mount candidates;
  a body-mount exception justified by SurfaceLifecycle backing (the results.ts
  cell-detail overlay) additionally requires its openSurfaceLifecycle(...)
  composition to remain in the same scope — losing it while keeping the mount fails.
Deviations from the spec: none — live-tree inventory matched the plan's
reconciled inventory exactly (11 body-mount scopes, 14 fixed-position selectors, 11
capture-Escape scopes).
Deferred acceptance criteria: none — this is a single unphased unit; all 5
acceptance criteria plus the inherited HANDLE_PX item are claimed and met.
Follow-ups filed: none.
Plan review: approved on pass 3 (ChatGPT-author / Fable-high-approve loop),
conversation https://chatgpt.com/c/6a7c0506-b480-83eb-896b-6031fa4a6085 — pass 1 had 7
findings incorporated (native TS7 parser confirmation, branch equality, explicit
Vitest config, etc.); pass 2 had 1 finding incorporated (the
dashboard-chart-interaction.ts non-panel exclusion, found via live verification).
Review outcome: internal Medium-risk read-only review found nothing actionable
(verified real production wiring, exactness of every policy fingerprint, the
openSurfaceLifecycle companion check, CSS lexer edge cases, the dashboard.ts
false-positive exclusion, and the HANDLE_PX contract's independence — all live,
against the real analyzers, not just by reading the shipped tests).

Code review loop: FULL STOP — 3-pass cap exhausted without a certified head.
Every one of the 3 formal chatgpt-review pr passes (session
c644ced0-5e77-45d1-978e-19189df869f5, seeded from the plan-loop conversation) found
and the coordinator verified/fixed real, independently-confirmed defects — none
rejected, none unresolved, all in the same root-cause family (unsound AST-based
lexical scope resolution in the new shared alias-map helpers backing all three
guards):
- Pass 1 (head 36ef21c, comment
  build(#592): lock in the #586/#587 shell primitive guardrails #672 (comment)):
  2 P1s — flat file-wide alias maps (last-write-wins across sibling scopes) for
  Document/Window/capture-option/handler aliases; allowlist checks were upper-bound/
  membership-only instead of exact multisets (missing baseline entries and duplicate
  CSS selectors went undetected). Fixed in 570e089.
- Pass 2 (head 570e089, comment
  build(#592): lock in the #586/#587 shell primitive guardrails #672 (comment)):
  3-4 findings — same-function block-local shadowing (if/loop/bare-block bindings)
  still collapsed into one per-function bucket; resolveObjectCaptureLiteral only
  recognized an identifier-named capture key, so {'capture':true}/computed-key/
  shorthand forms resolved to false-not-null; the CSS scanner recorded only the
  nearest at-rule, so wrapping an approved rule in an extra outer at-rule produced the
  same fingerprint. Fixed in ad4f71f.
- Pass 3 (head ad4f71f, comment
  build(#592): lock in the #586/#587 shell primitive guardrails #672 (comment)):
  2+ more findings — var/for-loop-header bindings still resolved through the wrong
  scope granularity (declaration-kind-unaware); a capture-options spread appearing
  AFTER an explicit capture property was dropped instead of correctly overriding it.
  Fixed in e76c8a7 (current head, CI green).
Each pass narrowed to a finer-grained variant of the identical root cause — a
hand-rolled lexical-scope walker approximating real TypeScript binding resolution.
This matches this repo's own documented recurring failure class (unsound
architecture-guard prefilters/resolvers, previously seen across Epic: extract the Fetch-native ClickHouse client into a reusable package #630/Replace unsound regex comment stripping in two architecture source-contract tests #643). Per
per-issue-cycle.md's root-cause circuit breaker ("if two review rounds find variants
of one root cause, revise the invariant map before writing another fix"), 3 rounds of
the same class is past that threshold — reassessing the resolver's structural approach
(e.g. leaning on the TypeScript checker's own symbol resolution instead of a hand-rolled
walker) is worth a human decision before further ad hoc patching. No certified head
exists at e76c8a7 (gate green, CI green, but the formal 3-pass pr-mode cap for this
session is now spent). Coordinator is presenting this to the human per the skill's
merge-gate FULL STOP.

Human ruling: restructure the resolver now (not merge-as-is, not one more informal
round). Coordinator consulted ChatGPT ad hoc in the SAME conversation (issue-mode
session 68032803-4985-489d-95bf-c1da0546bc55, seeded from the pr-mode session) on
the structural tradeoff; it confirmed checker-based binding resolution (via
typescript/unstable/sync's Project.checker, getSymbolAtLocation) as the right
narrow fix — scoped to identifier binding only, not full type inference — and flagged
the biggest open risk as whether the checker resolves bindings for bare, import-free
synthetic snippets with no tsconfig (this file's existing test-fixture shape). The
coordinator independently spiked that exact question before committing effort: a
synthetic snippet with a same-function block-shadowed alias and a for-loop-header
shadow resolved correctly via checker.getSymbolAtLocation — proving the approach
before implementation.

A fresh coding agent then restructured build/lib/check-legacy-owners.mjs per plan
addendum "Architecture decision 6": deleted the hand-rolled scope-tracking machinery
(scopeOwnerOf/scopeChain/declarationScopeOwnerOf/buildGlobalAliasMap/
buildFunctionDeclMap/buildCaptureAliasMap/a body-alias map) entirely and replaced
every "what does this identifier resolve to" question with real checker-based symbol
resolution. Candidate discovery, the capture-options evaluator's own evaluation-order
logic, and the CSS shell-fixed-position scanner were left untouched, per the plan's
non-goals. No policy fingerprint, diagnostic shape, or public export changed (confirmed
byte-identical policy tables in review). Net code reduction (-26 lines) despite adding
new resolver-level tests. Committed as 0c3c6d4 (CI green). A second internal
read-only review (structural verification against 4 NEW shadow shapes the original
tests didn't cover — catch block, class-method-local, destructuring-parameter,
arrow-implicit-return — plus fail-closed/session-correctness checks) found nothing
actionable.

Second code review loop (new session 64bc5c7d-ee25-4ed2-8b0f-f6346abcc16b, seeded
from the ad hoc issue-mode consultation): also 3-pass cap exhausted, also no certified
head. Unlike the first loop, these findings were NOT variants of the scope/binding
root cause the restructuring fixed (confirmed structurally sound by a second internal
review) — they're a different, adjacent class: proving a captured JS value can't
change after its declaration (mutation/aliasing tracking), plus one syntax-coverage
gap:
- Pass 1 (head 0c3c6d4, comment
  build(#592): lock in the #586/#587 shell primitive guardrails #672 (comment)):
  a pre-existing (not restructuring-caused) whole-scope/whole-file-deletion blind spot
  in the reverse/missing-baseline check; resolveCaptureFlag/resolveHandlerNode
  trusted a let/var initializer without checking for later reassignment. Fixed in
  9b530ec.
- Pass 2 (head 9b530ec, comment
  build(#592): lock in the #586/#587 shell primitive guardrails #672 (comment)):
  const-object property mutation after declaration (opts.capture = true) and the
  same let-reassignment gap left inconsistently unfixed in the body resolver; an
  unresolved constant-alias of the literal 'Escape' string bypassed Escape-semantic
  detection entirely; a CSS nested-@media-only frame walked past a real declaration.
  Fixed in 5d5d166.
- Pass 3 (head 5d5d166, comment
  build(#592): lock in the #586/#587 shell primitive guardrails #672 (comment)):
  the Document/Window resolver itself still lacked the write-awareness the body/handler
  resolvers already had (let doc = window; doc = document;); capture-options mutation
  tracking only saw the ORIGINAL identifier, not an alias of it, and only plain =, not
  compound operators (||=); bracket-notation document['addEventListener'](...) was
  never considered a candidate at all. Fixed in aa5a4e5 (current head, CI green).
Every finding across both loops (6 formal passes total, 2 sessions) was accepted and
fixed — none rejected, none unresolved. But this second loop's pattern is different in
kind from the first: "does a captured JS value/identifier ever get mutated or aliased
after declaration" is an open-ended static-analysis problem (full points-to/alias
analysis) — every fix closes one concrete shape (property mutation, alias-mediated
mutation, compound operators, bracket-notation call syntax) and each pass has found a
new one, the same shape the FIRST loop's exhaustion had, but for a structurally
different (and likely genuinely unbounded) reason than the binding/scoping problem the
checker restructuring solved for good. Presenting to the human again rather than
spending a third session chasing more mutation/aliasing shapes.

Human ruling: merge as-is. Head aa5a4e5 CI-green/gate-green, 6 formal review
passes across 2 sessions found and fixed 15+ real defects (none rejected/unresolved).
Remaining risk (further mutation/aliasing shapes) judged a narrower threat than build(guardrails): extend check-boundaries to lock in the shell primitives #592's
actual concern and out of reach of a mechanical grep/AST-light guard by design.
Documented on the PR (comment
build(#592): lock in the #586/#587 shell primitive guardrails #672 (comment)).
Follow-up filed as Tighten shell-guardrail 'provably clean' classification to fail-closed by default #673 (inbox) proposing to tighten the "provably clean"
classification to fail-closed-by-default instead of continuing to chase individual
bypass shapes. Merged 39a7f62 (fast-forward merge commit) 2026-08-12. Local
main not diverged post-merge.

fix(#592): address review pass 1 findings

fix(#592): address review pass 2 findings

fix(#592): address review pass 3 findings

refactor(#592): resolve shell-guardrail identifier bindings via the r…

fix(#592): address review pass 1 findings

fix(#592): address review pass 2 findings

fix(#592): address review pass 3 findings

Merge pull request #672 from Altinity/build/592-lock-in-shell-guardrails

Navigation Menu

Repository navigation

build(guardrails): extend check-boundaries to lock in the shell primitives #592

Goal

Context

Deliverables (after #586 and #587 merge)

Tests

Acceptance criteria

Non-goals

Inherited from #586 (phase 1 of #593) — an unguarded JS/CSS constant duplication

Extra acceptance for this inherited item

Related

Ship log — #592

#592 — handoff

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

build(guardrails): extend check-boundaries to lock in the shell primitives #592

Description

Goal

Context

Deliverables (after #586 and #587 merge)

Tests

Acceptance criteria

Non-goals

Inherited from #586 (phase 1 of #593) — an unguarded JS/CSS constant duplication

Extra acceptance for this inherited item

Related

Activity

BorisTyshkevich commented on Aug 3, 2026

BorisTyshkevich commented on Aug 12, 2026

Ship log — #592

#592 — handoff

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions