Skip to content

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

Description

@BorisTyshkevich

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:

  1. 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).
  2. 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.
  3. 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

Acceptance criteria

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

Non-goals

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.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorRestructuring without user-facing behavior changetech-debt

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions