Skip to content

feat(desktop): add the Astryx migration contract harness - #1653

Merged
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness
Jul 30, 2026
Merged

feat(desktop): add the Astryx migration contract harness#1653
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #1565. PR 0 of the Astryx renderer migration: the measurement instrument every later slice depends on. The mega-branch produced regressions on screens nobody edited, and slicing alone cannot localise them — the bisect signal has to come from a record of what the app looked like before.

  • check:visual-contract — one command compares two builds: it checks out the base ref into a cached temp worktree with its own npm ci, builds both sides, captures five representative fixture routes × light/dark × darwin/win32 from each, and diffs every visible element's box, paint, and alignment properties — plus a paint signature for every painted ::before/::after — in memory. Gates no-change, not correctness: a pre-existing bug on the base stays out of scope, and zero diff means "the migration did not move this element", never "this element is right".
  • check:hit-test — interactive elements must be reachable at their centre, free of pointer-events/visibility traps, and opaque through their ancestry; every route reports what it probed and what it skipped (invisible, out-of-scope, disabled, transparent, clipped) so the coverage is a number, not an assumption.
  • launch:fixture — a real, clickable fixture window for manual visual comparison, which a visually-neutral migration cannot do without.
  • Salvages the mega-branch's late regression fixes into the harness (item 3 below).

Both harnesses live in scripts/ as npm scripts and are deliberately outside CI, following the check:chat-visual precedent and #1565's own scoping — a two-build compare costs minutes per run, which is a per-slice gate, not a per-push one. They are migration-only and get deleted in PR 14 with the maka.legacy layer. scripts/fixture-env.mjs, scripts/fixture-window.mjs, and scripts/electron-lifecycle.mjs are the exception: audit-alignment.mjs runs in CI and the E2E suite shares them, so they outlive the harness.

How to use it

node scripts/check-visual-contract.mjs # compare main → working tree
node scripts/check-visual-contract.mjs --against <ref># a different base
node scripts/check-visual-contract.mjs --route chat --theme dark --platform win32

There is no baseline file and no capture step to remember. An earlier version had one, captured by hand on the base branch and compared by hand after switching; every step a human could skip — rebuild after switching, recapture after rebasing — produced a convincing zero-diff pass that had measured the same binary twice. Now the instrument owns the whole measurement: both sides are checked out, installed, built, and captured in the same run, on the same host (font metrics and the macOS traffic-light inset are host facts, so cross-host comparison was never sound anyway).

The base gets its own dependency closure: a fresh worktree of the ref with its own npm ci against its own lockfile, cached keyed by the resolved commit, so iterating on a slice rebuilds only the working tree. Sharing this checkout's node_modules was tried and rejected — the workspace links inside it resolve @maka/* back to the working tree's packages, so the base would have bundled and loaded the candidate's own code. A side effect of the honest install: a slice may legitimately change dependencies. Route/theme/platform arguments are closed sets, and every capture asserts the renderer actually reached the requested data-os and theme before reading a single style — a capture must prove it measured the cascade it claims.

The win32 column runs on any host: MAKA_E2E_FIXTURE_PLATFORM drives the production app:info → data-os path, which is what keys the per-OS CSS. It covers the cascade, not native chrome.

Route mapping

#1565 names product routes; these are the MAKA_E2E_FIXTURE scenarios that actually open them on main. There is no scenario literally called "chat" or "settings-providers".

RouteScenarioReadiness selector
chatturn-narrative.maka-session-workbar
settings/generalsettings-general.settingsRows
settings/providersprovider-workspace.providersPanel
mcp hubmodule-mcp.maka-module-main-header
onboardingfirst-run.maka-onboarding-surface

Known blind spots, stated in the tool's header rather than papered over: display: none subtrees (closed menus, unmounted dialogs) never enter the capture; pseudo-elements are paint signatures without a rect; native top-layer content (dialog.showModal, popover) would escape the opacity-prune, and nothing in the app uses it today.

Where this departs from #1565, and why

Each departure is a measurement, not a preference. The issue body has been corrected where it was factually wrong.

The fixture window was never unclickable.#1565 stated that app.dock.hide() makes fixture runs reject all real clicks. Measured on ae43cb291/darwin, that does not hold: MAKA_E2E_FIXTURE alone means the window is never shown at all (startHiddenkeepHiddenForE2eFixture, main-window.ts:114), and adding MAKA_E2E_SHOW_WINDOW=1 gives isVisible: true, isFocused: true, and fully working clicks — with dock.hide() still running.

scripts/desktop-real-window-smoke.mjs was already broken on main. It boots a real fixture window but never set MAKA_E2E_SHOW_WINDOW, so its own programmatic-window-visible check fails and its twelve human checks are unrunnable against an invisible window. Fixed here, extended with --manual rather than adding a second launcher, and now launched through the shared environment builder — it was the last launcher still inheriting process.env wholesale, which meant a developer with npm run dev open smoked the dev server instead of the build the script had just made.

The one product-code change is about reachability. The dock rule now keys off the same startHidden that decides whether the window shows, instead of re-deriving it from MAKA_E2E_FIXTURE — a re-derivation that had already drifted, since it missed the MAKA_E2E_SHOW_WINDOW escape hatch. An accessory app has no Dock tile and no Cmd+Tab entry, so a reviewer who switches away mid-comparison cannot switch back. Capture runs and CI e2e leave the variable unset and keep their accessory, no-focus-steal behaviour unchanged.

The window-drag question is not asked here.#1565 proposed probing it with a real click; that was tried and abandoned (an unmapped window does not route synthesized input, and a visible one fired real product actions — one opened a native file dialog and hung the run). It is also already answered: e2e/window-titlebar.spec.ts measures it in CI against rendered geometry and the document order Chromium composes drag rects in, and .maka-window-titlebar is the only element allowed to declare drag. The hit-test covers what that spec does not — whether a control anywhere in the window can be reached at all.

Centre is required; corners corroborate. All five probe points cannot pass on a clean main — sibling chrome legitimately covers a couple of edge pixels. Three lost corners means something is actually covering the control; one means the border has a neighbour.

Item 3: salvaged regressions

Six of the eight late "fix cascade / fix click" commits land on properties the snapshot already records. Bespoke rules would be redundant; what they do add is noticing when the harness stops watching, so each is pinned to an anchor that must keep appearing in a route where the element actually paints. That last clause is load-bearing: the session-row anchors originally pointed at the chat route, where the fixture keeps the panel collapsed under opacity: 0 — they "matched" records with no visible pixels until the capture started pruning invisible subtrees, at which point the check failed and exposed its own false assurance. They now point at mcp-hub, whose fixture opens the sidebar.

CommitRegressionAnchorRoute
be5e69584titlebar clusters wrapped onto a second rowmaka-shell-topbar-railchat
fd38a37cecomposer frame jumped on focusmaka-composer-inner (resting state only)chat
9aad59740session timestamp stacked above its titlemaka-list-row-metamcp-hub
3dbd68ca7sidebar nav rows had two styling authoritiesmaka-list-rowmcp-hub
9d20a9396task ledger recent-count collided with its labelmaka-session-workbar-countchat
be1406705settings content column painted over the nav railsettingsSidebarsettings-general

Two are not transcribable: 3e57d1951 (stat tile leading) and 400e8f4a9 (turn marker measure) fix files the mega-branch itself created, which do not exist on main. They are recorded here as fragile spots for whichever slice introduces those components.

What review changed

Four review rounds (Codex plus an independent fresh-context agent each time), every finding reproduced before being fixed. The branch is meaningfully different as a result.

Round 1

It could report ok for a window it never launched. The harness rolled its own launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a process-group kill, ...process.env. It never verified the CDP target belonged to the child it spawned, so a leftover Electron on the port was captured instead; reproduced with two concurrent runs both reading the same window, and 270 stale user-data dirs on the dev machine. Now launched through Playwright's Electron support, with the environment builder extracted to scripts/fixture-env.mjs and shared with the E2E suite.

It could not see the class of change the migration makes.boxShadow, textAlign, flexDirection, flexWrap and outline were not captured — a utility that starts beating a product rule after the @layer move can change elevation or text placement with an identical rect. Three INITIAL_VALUES entries guessed at values Chromium does not serialise and never matched once.

Inherited values were compared against the wrong ancestor. A zero-box or display: contents wrapper is skipped, so nothing in the capture carried its values — yet visible children omitted properties that matched it. Now compared against the nearest recorded ancestor, so every omission is recoverable by a reader of the file.

The drag-region resolver was built on a false premise. It parsed stylesheet rules in source order because "the property is absent from getComputedStyle". It is not: getComputedStyle(el).webkitAppRegion returns drag, and window-titlebar.spec.ts has read it that way in CI all along. Deleted, ~50 lines.

The product change had no automated coverage.resolveDockPresentation is now a pure function with tests, and the programmatic smoke layer is pinned by the source contract.

Round 2

The two-step baseline workflow could pass silently when a step was skipped. The checker never rebuilt the candidate, the baseline was a bare array with no provenance, and the gitignored file could not travel between worktrees — comparing a build to itself read as a clean slice. Replaced by the one-command --against compare; the baseline file, the --update flag, and the .gitignore entry are gone with it.

Teardown was unbounded, and CI inherits this launcher.app.close() has no deadline; one wedged launch turned a capture run — or the CI alignment audit, whose job has no timeout — into an infinite hang, reproduced once at ≥600s. The bounded close the E2E suite already owned (grace, then SIGKILL the tree) moved to scripts/electron-lifecycle.mjs and both consumers share it; renderer evaluate gained a deadline; the temp profile is removed even when close fails.

The shared-launcher refactor had silently changed the CI gate's conditions.audit-alignment's settle budget had dropped 2500ms → 1000ms against static-markup readiness, and buildFixtureEnv read process.env.CI inline — which made the "hidden run stays hidden" test pass on every laptop and fail on the Linux CI runner. The settle budget is restored with real per-fixture ready selectors, the window-visibility decision is a pure function of the builder's arguments, and the one ambient read lives in isCiLinuxDisplay for callers to compose.

Two capture-semantics gaps.textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79–85% of records re-recorded an ancestor's value; and descendants of an opacity: 0 element (own opacity 1, non-zero rect) were captured as visible — 105 phantom records under the collapsed session panel. Fixed, plus a static table test pinning every CSS-inherited property to the inherited route in both directions.

The darwin-only matrix contradicted the issue, and its stated justification was wrong. No Windows runner is needed: the fixture platform override flips data-os through the production path on any host. The matrix now includes win32.

Round 3

The base was not isolated. The base worktree borrowed this checkout's node_modules, and the workspace links inside it resolved @maka/* straight back to the working tree — the base bundled and loaded the candidate's own packages, so a slice changing @maka/core would have compared itself against itself. Both reviewers converged on this independently; reproduced byte-for-byte with require.resolve. The base now runs its own npm ci against its own lockfile (cached per commit, guarded by an atomic build lock), and the lockfile-parity gate plus the symlink list that existed to justify the sharing are deleted.

Pseudo-elements were invisible to the contract. 47 ::before/::after rules paint in this renderer — including the full-screen body::after film-grain overlay — and a cascade flip on any of them changed pixels while every element record stayed byte-identical. Painted pseudo-elements are now captured as paint signatures that inherit from their host.

The instrument now proves its own measurement identity. CLI arguments are closed sets; every capture waits for and asserts the requested data-os and theme on the live document before reading styles — the win32 column can no longer silently degrade into a second darwin column if the override seam regresses. Dead omission rules are counted at the raw sample inside the capture (not inferred from the compressed output), so a rule shadowed by the border/outline paint gates still proves itself.

Remaining teardown edges. The smoke gate's SIGTERM paths now escalate through the shared bounded close, and a failed report write no longer leaves a live Electron behind. Verifying that fix surfaced one more: the shared force path kills by process group, the smoke gate's child was not spawned detached, so the group signal hit ESRCH, the terminator read "tree already gone", and a visible window that ignores SIGTERM survived its own SIGKILL stage. The smoke child is now a group leader like every runtime consumer, and the bounded close backs the group kill with a direct kill on the root.

Round 4

The contract could not see the paint-only channels.filter, backdrop-filter, clip-path and transform move pixels without moving the rect — grayscale() on disabled provider rows, the glass theme's backdrop blur, clip-path as visually-hidden, rotate() on square chevrons — so a migration losing one diffed zero. All four are now sampled (plus transform-origin, gated on a transform actually painting), and the capture freezes animations and transitions before reading a single style: a running spinner serialises a different matrix at every phase, which would diff time rather than the cascade. Verified with a negative control — an injected filter rule fails the compare with filter: ∅ → sepia(0.1) — and a clean 20/20 run.

The hit-test's --route was an open set. It accepted any value and failed only when nothing matched, so --route chat --route chatt dropped the typo and exited 0 as "1 route(s) clean" — silent coverage shrink on a tool whose whole point is that coverage is a number. Each value is now validated against the closed route set (as the visual contract already does), and the CLI's exit codes are pinned by subprocess tests.

Two smaller honesty fixes. Both direct-run guards compare through pathToFileURL — a checkout path with a space or non-ASCII segment percent-encodes in import.meta.url, and the string-concatenation compare would have made the instrument exit 0 having measured nothing. The tool header still described the rejected node_modules-sharing base build; it now states the actual isolation model. The direct-kill backstop from round 3 gained a fake-based test alongside its live-window proof.

Verification

Run on main (ae43cb291) + this branch, macOS arm64.

npm run lint ✓
npm run format:check ✓
npm run typecheck ✓
npx knip ✓ exit 0 (2 config hints, same as main)
npm run test:scripts ✓ 209 pass, 0 fail
npm --workspace @maka/desktop run test:dist ✓ 2981 pass, 0 fail
node scripts/audit-alignment.mjs ✓ 12/12 fixtures clean
node scripts/check-visual-contract.mjs ✓ 20/20 comparisons clean against main
(fresh base: worktree + npm ci + build, all by the tool;
negative control: an injected filter rule fails the compare)
node scripts/check-hit-test.mjs ✓ 5/5 routes clean
npx playwright test first-run window-titlebar electron-lifecycle
✓ 7 passed
npm --workspace @maka/desktop run smoke:programmatic-window
✓ PASS incl. programmatic-dock-visible (was FAIL on main)

The real-window smoke launches the app with MAKA_E2E_FIXTURE, which makes
main.ts compute startHidden and main-window.ts keep the window hidden for
its whole lifecycle. The gate therefore had nothing to look at: on a clean
main its own programmatic-window-visible check reports visible=false and
exits 1, and every human check it prompts for — dragging window edges and
corners to resize, dragging the titlebar to move, Tab/Shift+Tab traversal
inside the modal — is unrunnable against an invisible window.
Set MAKA_E2E_SHOW_WINDOW on the launch env, which is exactly the escape
hatch main.ts already provides for runs where there is no focus to steal.
Capture runs and CI e2e do not set it and keep their hidden-window
behavior unchanged.
Hiding the dock icon makes the run an accessory app. Contrary to what the
migration tracker assumed, such a window is fully clickable — measured on
darwin: with MAKA_E2E_SHOW_WINDOW set, the fixture window reports
isVisible/isFocused true and takes clicks, drags, and keyboard input
normally. What it does not have is a Dock tile or a Cmd+Tab entry, so a
reviewer doing manual visual comparison who switches to another app has no
way back to it.
Key the dock icon off the same startHidden that decides whether the window
shows at all, instead of re-deriving the condition from MAKA_E2E_FIXTURE.
The two can no longer drift, and MAKA_E2E_SHOW_WINDOW now opts out of both
halves of staying out of sight. Capture runs and CI e2e leave the variable
unset, so they keep hiding the window and the dock exactly as before.
Lock it with a dock diagnostic and a programmatic-dock-visible check in the
real-window smoke, which reported dockVisible=false before the change and
true after.
A migration that must stay visually neutral needs to look at the running
app constantly, but the only way to boot a route with fixture data was the
smoke gate, which insists on walking a reviewer through twelve checklist
prompts and filing a report every time.
Add --manual: same launch path, then stop. The window stays up until it is
closed or interrupted. Reusing the gate's launcher keeps the window a
reviewer inspects identical to the window the gate checks, and leaves one
place that knows how to boot a fixture instance.
Expose it as `launch:fixture`, and lift the build chain the smoke scripts
repeated verbatim into `build:with-deps` rather than copying it a third
time.
#1565 PR 0. The mega-branch produced regressions on screens nobody edited,
and slicing alone does not localise them: the signal has to come from a
baseline that says what the app looked like before. This captures one.
check:visual-contract walks the five representative fixture routes in light
and dark, records every visible element's box, paint, and alignment
properties, and diffs the result against committed JSON. The alignment set
(align-items/justify-content/place-items/gap/grid-area) is there because
flex and grid misalignment is exactly the class a paint-only property set
cannot see. It gates no-change, not correctness: a pre-existing bug on main
stays out of scope, and zero diff means the migration did not move this
element, never that the element is right.
The boot sequence audit-alignment.mjs already worked out — spawn Electron
with MAKA_E2E_FIXTURE, wait for a CDP page target, evaluate in the renderer
— moves to fixture-cdp.mjs so both scripts share one launcher instead of
growing a second. It also gains what a contract harness needs and an audit
could do without: animations and webfont reflow frozen before capture, an
evaluate deadline so a wedged renderer fails loudly instead of hanging, and
process-group teardown so Electron's helpers do not pile up and starve
later windows.
Also carries #1565's third PR 0 item, salvaging the mega-branch's late
regression fixes. Six of the eight land on properties this snapshot already
records — a wrapped titlebar row, a timestamp stacking above its title, a
missing gap, a column painting over the settings rail all move a rect or a
recorded property, so the diff catches them without bespoke rules. What
bespoke rules do add is noticing when the harness stops watching, so each
one is pinned to an anchor that must keep appearing in the baseline. The
remaining two fix files the mega-branch itself created, which do not exist
on main; they are recorded in the PR body instead.
Determinism was verified by capturing the same build twice. The first
attempt was not clean, and both causes are excluded rather than tuned
around: element labels fell back to el.id, which Base UI regenerates every
render, and OverlayScrollbars' chrome fades with pointer activity, so its
visibility was a coin flip. Baselines stay at 1.5MB rather than 4MB because
properties equal to their initial value, or inherited unchanged from the
parent, are omitted and read back as unchanged.
Migration-only: deliberately outside CI, following check:chat-visual, and
removed in PR 14 with the maka.legacy layer.
#1565 PR 0. The mega-branch's most expensive regressions were controls that
render perfectly and refuse clicks. A computed-style snapshot cannot see
that, so it gets a contract of its own: every visible interactive element on
the five routes must be reachable at its centre, must not sit under a
pointer-events or visibility trap, must not be transparent through an
ancestor, and must not fall inside an -webkit-app-region: drag area.
Three parts of #1565's proposal did not survive contact with a clean main,
and each is replaced by something answering the same question:
The drag-region probe was specified as a real click plus a side effect.
Fixture windows are hidden, and an unmapped window does not route
synthesized input through the browser's hit-testing path, so every target
read as swallowed. Making the window visible to fix that turned the probe
into something that fires real product actions — one opened a native file
dialog and hung the run. Reading -webkit-app-region off the stylesheet rules
instead has no side effects: the property is missing from getComputedStyle,
but not from the rules that declare it.
All five probe points were specified as required. On a clean main that is
unreachable: sibling chrome legitimately covers a couple of edge pixels, so
a workbar resize handle would indict a control nobody struggles to click.
The centre is now required and corners corroborate — three lost corners
means something covers the control.
The overlay guard keyed on [role=dialog][open], which a React-rendered
div[role=dialog] never has, so the settings surface went undetected and
every control behind it was reported unhittable. It now keys on the
product's own data-modal="true".
Geometry probes do need a visible window — elementFromPoint disagrees with
getBoundingClientRect against a hidden one — so this check steals focus for
a few seconds per route. Memoising style and rect lookups took the densest
route from a 60s timeout to 3s, and routes are spaced so one window is fully
gone before the next boots.
All five routes are clean on ae43cb2.
@Astro-Han
Astro-Han marked this pull request as draft July 30, 2026 09:45
Review found the harness could report `ok` for a window it never
launched, could not see the class of change the migration actually
makes, and shipped its only product change with no automated coverage.
Launch through the shared E2E seam. The harness rolled its own Electron
launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a
process-group kill, `...process.env` — all of it already solved next
door in `apps/desktop/e2e/fixtures.ts`, and each hand-rolled version
worse. It never verified the CDP target belonged to the child it
spawned, and the port allocator restarted at 14600 in every process, so
a leftover Electron on that port got captured instead. Reproduced: two
concurrent runs requesting different scenarios both read the same
window. When the leftover is the same route from an older build — the
shape you get by re-running one route after an edit — the contract
reports `ok` for a stale window. `detached: true` with no signal handler
supplied the leftovers; this machine had 270 stale user-data dirs, none
ever removed. `buildE2eEnv` moves to `scripts/fixture-env.mjs`, shared
by both callers; its own comment already described the bug the harness
had, since inheriting the environment leaks `VITE_DEV_SERVER_URL` and
loads the dev server instead of the build under test. Launch, readiness
and teardown now go through Playwright's Electron support: fixed sleeps
become per-route readiness selectors, and teardown removes its temp dir.
235 lines become 139, and the hit-test retry goes with them, per the
sibling harness's rule that flakes should fail loudly.
Stop committing baselines. They encode the capturing host — font metrics
and the macOS traffic-light inset — so one machine's baseline reads as
thousands of diffs on another. Capture on the branch you compare
against, apply the slice, compare: the two captures that matter always
come from the same host. The timezone is pinned through the fixture's
IANA override for the same reason. This removes 70k lines from review
and makes the orphan-baseline check moot.
Record what a cascade flip actually changes. `boxShadow`, `textAlign`,
`flexDirection`, `flexWrap` and `outline` were missing, so a utility
that starts beating a product rule could change elevation or text
placement with an identical rect and be reported clean. Three
`INITIAL_VALUES` entries guessed at values Chromium does not serialise
and never matched once, leaving constants on all 3,964 records;
`findDeadOmissionRules` now fails the run when a rule stops matching —
it caught a fourth such entry immediately. Inherited values were
compared against the parent rather than the nearest recorded ancestor,
so a zero-box wrapper's color could change, repaint every visible child,
and leave the capture byte-identical: 129 masked values on the chat
route alone.
Delete the second drag-region resolver. It parsed stylesheet rules in
source order, ignoring specificity, `!important` and `@layer` — which
PR 1 and PR 2 introduce — on the premise that the property is absent
from computed style. It is not: `getComputedStyle(el).webkitAppRegion`
returns `drag`, and `e2e/window-titlebar.spec.ts` has read it that way
in CI all along, against rendered geometry and document order. The
hit-test keeps what that spec does not cover, and now reports how many
elements it actually probed rather than how many the selector matched.
Pin the product change. `resolveDockPresentation` is a pure function
with tests; the branch it replaced could only be exercised by launching
Electron, which is why it had none. The programmatic smoke checks are
now pinned by the contract test that exists to pin checks — including
the new dock one, which could previously be deleted with CI green — and
`e2e` shares `build:with-deps` instead of repeating the chain verbatim.
…budget
buildFixtureEnv read process.env.CI inline, so the fixture-env test
asserting a hidden run stays hidden passed on every laptop and failed on
the Linux CI runner. The ambient read now lives in isCiLinuxDisplay for
callers to compose explicitly.
audit-alignment gets its 2500ms settle back (the shared-launcher refactor
had silently dropped it to 1000ms against static-markup readiness) plus a
real ready selector per fixture, and its header now states the deliberate
environment change from the raw-spawn launcher it replaced.
withFixtureWindow tore down with a bare app.close(), which has no
deadline — one wedged launch turned a capture run (or the CI alignment
audit, which has no job timeout) into an infinite hang, and the temp
userData dir leaked because rm was sequenced after the close.
The bounded close the E2E suite already owned moves to
scripts/electron-lifecycle.mjs — the same shared-home precedent as
fixture-env.mjs, so a bare-node harness and the Playwright suite stop
splitting one launch concern across two directories. Renderer evaluate
gets a deadline too; teardown then reclaims the process either way.
Two measurement-semantics gaps in the visual capture, both instances of
a class the harness had already been bitten by:
- textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79-85%
of records re-recorded an ancestor's value and one container change
printed as N descendant lines. It moves to the inherited omission (and
out of INITIAL_VALUES — absent must mean exactly one thing). A static
table test now pins every CSS-inherited property to that route, which
is the guard findDeadOmissionRules could never provide.
- Descendants of an opacity:0 element report opacity 1 and a non-zero
rect, so the collapsed session panel put 105 invisible records into
the chat capture; a change under it would diff on pixels that never
paint. The walk now prunes zero-opacity subtrees.
The prune exposed that two salvaged anchors were matching those phantom
records: the chat fixture never shows the session rows at all. They now
point at mcp-hub, whose fixture opens the sidebar for real.
Replaces the two-step gitignored-baseline workflow. The checker now
builds the base ref in a cached temp worktree (node_modules shared;
the compare refuses to run across a package-lock change), builds the
working tree, captures every route x theme x platform from both builds,
and diffs in memory. Every step of the old workflow a human could skip
- rebuild after switching, recapture after rebasing, stash on a
committed slice - produced a zero-diff pass that had measured the same
binary twice; now the instrument owns the whole measurement and there
is no baseline file to trust or to stale.
The matrix gains the win32 column #1565 asked for: the fixture platform
override drives the production app:info -> data-os path, so the per-OS
cascade is captured on any host - no Windows runner involved.
desktop-real-window-smoke was the last launcher still spreading
process.env wholesale: a developer with npm run dev open smoked the dev
server instead of the build the script just made, and the run touched
the real $HOME (#1517's skills-deletion surface). It now launches
through buildFixtureEnv with an explicit visible window.
The dock contract drops its duplicated fourth case and pins the wiring
instead - the regression this module exists for was the call site
re-deriving startHidden, which no pure-function test can observe. The
reveal contract follows fixture-env's pure shape (the ambient CI read
now lives in isCiLinuxDisplay, composed at each call site).
…entity
Round-3 review found the compare's base was not a base: the borrowed
node_modules resolved @maka/* through workspace links straight back to
the working tree, so the base bundled and loaded the candidate's own
packages — a slice changing @maka/core would have compared itself
against itself. The base now runs its own npm ci against its own
lockfile in the cached worktree (atomic lock against concurrent
builds; the marker name encodes the scheme so old symlink-era caches
rebuild). The lockfile-parity gate and the symlink list existed only
to justify the sharing; both are deleted, and a slice may now
legitimately change dependencies.
The same round showed the instrument asserting nothing about what it
measured, so now it proves it: route/theme/platform are closed sets;
every capture waits for the requested data-os and theme on the live
document before reading a style; painted ::before/::after get paint
signatures (47 pseudo rules paint in this renderer, led by the
body::after film grain, and a cascade flip on any of them was
invisible); mixBlendMode joins the property set; and dead omission
rules are counted at the raw sample where the border/outline gates
cannot shadow them.
The smoke gate still had two unbounded exits: --manual waited forever
on 'exit' after one SIGTERM, and the report path fired SIGTERM and
called process.exit without waiting or escalating — plus a failed
report write skipped the kill entirely. All paths now stop through the
shared bounded close (SIGTERM as the graceful phase, 5s grace, SIGKILL
the tree), inside a finally.
electron-lifecycle's import of @maka/runtime now fails with an error
that says what to build; on a fresh clone the raw ERR_MODULE_NOT_FOUND
surfaced from four files deep in the import chain.
The shared teardown's force path kills by process group. The smoke gate
spawned Electron without detached, so the group signal hit ESRCH, the
terminator read that as "tree already gone", and a visible window that
had ignored SIGTERM survived its own SIGKILL stage. Spawn the smoke
child as a group leader like every runtime consumer does, and back the
group kill with a direct kill on the root so a future non-leader child
still dies inside the 2s exit deadline.
filter, backdrop-filter, clip-path and transform move pixels without
moving the rect — grayscale() on disabled provider rows, the glass
theme's backdrop blur, clip-path as visually-hidden, rotate() on square
chevrons — so losing one was invisible to every property the contract
sampled. Capture all four (plus transform-origin, gated on a transform
actually painting), and stop the renderer's clock first: a running
spinner serialises a different matrix at every phase, which would diff
time rather than the cascade. Verified with a negative control (an
injected filter rule fails the compare) and a clean 20/20 run.
Also correct the header, which still described the rejected
node_modules-sharing base build, and compare the direct-run guard
through pathToFileURL so a percent-encoding checkout path cannot turn
the instrument into a silent exit-0 no-op.
--route accepted any value and only failed when nothing matched, so
`--route chat --route chatt` dropped the typo and exited 0 as "1
route(s) clean" — silently shrinking the requested coverage. Validate
each value against the closed route set (as the visual contract already
does), guard the entry point through pathToFileURL, and pin the CLI's
exit codes with subprocess tests.
The ESRCH-swallowing group-kill path was only proven against a live
window; the fake now exercises it too — a terminator that returns
without killing must be followed by a direct SIGKILL on the root.
@Astro-Han
Astro-Han marked this pull request as ready for review July 30, 2026 13:04
@Astro-Han
Astro-Han merged commit 71e47a6 into mainJul 30, 2026
3 checks passed
@Astro-Han
Astro-Han deleted the refactor/1565-pr0-contract-harness branch July 30, 2026 13:04
Sign up for freeto 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

@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
feat(desktop): add the Astryx migration contract harness by Astro-Han · Pull Request #1653 · apache/maka · GitHub
Skip to content

feat(desktop): add the Astryx migration contract harness - #1653

Merged
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness
Jul 30, 2026
Merged

feat(desktop): add the Astryx migration contract harness#1653
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #1565. PR 0 of the Astryx renderer migration: the measurement instrument every later slice depends on. The mega-branch produced regressions on screens nobody edited, and slicing alone cannot localise them — the bisect signal has to come from a record of what the app looked like before.

  • check:visual-contract — one command compares two builds: it checks out the base ref into a cached temp worktree with its own npm ci, builds both sides, captures five representative fixture routes × light/dark × darwin/win32 from each, and diffs every visible element's box, paint, and alignment properties — plus a paint signature for every painted ::before/::after — in memory. Gates no-change, not correctness: a pre-existing bug on the base stays out of scope, and zero diff means "the migration did not move this element", never "this element is right".
  • check:hit-test — interactive elements must be reachable at their centre, free of pointer-events/visibility traps, and opaque through their ancestry; every route reports what it probed and what it skipped (invisible, out-of-scope, disabled, transparent, clipped) so the coverage is a number, not an assumption.
  • launch:fixture — a real, clickable fixture window for manual visual comparison, which a visually-neutral migration cannot do without.
  • Salvages the mega-branch's late regression fixes into the harness (item 3 below).

Both harnesses live in scripts/ as npm scripts and are deliberately outside CI, following the check:chat-visual precedent and #1565's own scoping — a two-build compare costs minutes per run, which is a per-slice gate, not a per-push one. They are migration-only and get deleted in PR 14 with the maka.legacy layer. scripts/fixture-env.mjs, scripts/fixture-window.mjs, and scripts/electron-lifecycle.mjs are the exception: audit-alignment.mjs runs in CI and the E2E suite shares them, so they outlive the harness.

How to use it

node scripts/check-visual-contract.mjs # compare main → working tree
node scripts/check-visual-contract.mjs --against <ref># a different base
node scripts/check-visual-contract.mjs --route chat --theme dark --platform win32

There is no baseline file and no capture step to remember. An earlier version had one, captured by hand on the base branch and compared by hand after switching; every step a human could skip — rebuild after switching, recapture after rebasing — produced a convincing zero-diff pass that had measured the same binary twice. Now the instrument owns the whole measurement: both sides are checked out, installed, built, and captured in the same run, on the same host (font metrics and the macOS traffic-light inset are host facts, so cross-host comparison was never sound anyway).

The base gets its own dependency closure: a fresh worktree of the ref with its own npm ci against its own lockfile, cached keyed by the resolved commit, so iterating on a slice rebuilds only the working tree. Sharing this checkout's node_modules was tried and rejected — the workspace links inside it resolve @maka/* back to the working tree's packages, so the base would have bundled and loaded the candidate's own code. A side effect of the honest install: a slice may legitimately change dependencies. Route/theme/platform arguments are closed sets, and every capture asserts the renderer actually reached the requested data-os and theme before reading a single style — a capture must prove it measured the cascade it claims.

The win32 column runs on any host: MAKA_E2E_FIXTURE_PLATFORM drives the production app:info → data-os path, which is what keys the per-OS CSS. It covers the cascade, not native chrome.

Route mapping

#1565 names product routes; these are the MAKA_E2E_FIXTURE scenarios that actually open them on main. There is no scenario literally called "chat" or "settings-providers".

RouteScenarioReadiness selector
chatturn-narrative.maka-session-workbar
settings/generalsettings-general.settingsRows
settings/providersprovider-workspace.providersPanel
mcp hubmodule-mcp.maka-module-main-header
onboardingfirst-run.maka-onboarding-surface

Known blind spots, stated in the tool's header rather than papered over: display: none subtrees (closed menus, unmounted dialogs) never enter the capture; pseudo-elements are paint signatures without a rect; native top-layer content (dialog.showModal, popover) would escape the opacity-prune, and nothing in the app uses it today.

Where this departs from #1565, and why

Each departure is a measurement, not a preference. The issue body has been corrected where it was factually wrong.

The fixture window was never unclickable.#1565 stated that app.dock.hide() makes fixture runs reject all real clicks. Measured on ae43cb291/darwin, that does not hold: MAKA_E2E_FIXTURE alone means the window is never shown at all (startHiddenkeepHiddenForE2eFixture, main-window.ts:114), and adding MAKA_E2E_SHOW_WINDOW=1 gives isVisible: true, isFocused: true, and fully working clicks — with dock.hide() still running.

scripts/desktop-real-window-smoke.mjs was already broken on main. It boots a real fixture window but never set MAKA_E2E_SHOW_WINDOW, so its own programmatic-window-visible check fails and its twelve human checks are unrunnable against an invisible window. Fixed here, extended with --manual rather than adding a second launcher, and now launched through the shared environment builder — it was the last launcher still inheriting process.env wholesale, which meant a developer with npm run dev open smoked the dev server instead of the build the script had just made.

The one product-code change is about reachability. The dock rule now keys off the same startHidden that decides whether the window shows, instead of re-deriving it from MAKA_E2E_FIXTURE — a re-derivation that had already drifted, since it missed the MAKA_E2E_SHOW_WINDOW escape hatch. An accessory app has no Dock tile and no Cmd+Tab entry, so a reviewer who switches away mid-comparison cannot switch back. Capture runs and CI e2e leave the variable unset and keep their accessory, no-focus-steal behaviour unchanged.

The window-drag question is not asked here.#1565 proposed probing it with a real click; that was tried and abandoned (an unmapped window does not route synthesized input, and a visible one fired real product actions — one opened a native file dialog and hung the run). It is also already answered: e2e/window-titlebar.spec.ts measures it in CI against rendered geometry and the document order Chromium composes drag rects in, and .maka-window-titlebar is the only element allowed to declare drag. The hit-test covers what that spec does not — whether a control anywhere in the window can be reached at all.

Centre is required; corners corroborate. All five probe points cannot pass on a clean main — sibling chrome legitimately covers a couple of edge pixels. Three lost corners means something is actually covering the control; one means the border has a neighbour.

Item 3: salvaged regressions

Six of the eight late "fix cascade / fix click" commits land on properties the snapshot already records. Bespoke rules would be redundant; what they do add is noticing when the harness stops watching, so each is pinned to an anchor that must keep appearing in a route where the element actually paints. That last clause is load-bearing: the session-row anchors originally pointed at the chat route, where the fixture keeps the panel collapsed under opacity: 0 — they "matched" records with no visible pixels until the capture started pruning invisible subtrees, at which point the check failed and exposed its own false assurance. They now point at mcp-hub, whose fixture opens the sidebar.

CommitRegressionAnchorRoute
be5e69584titlebar clusters wrapped onto a second rowmaka-shell-topbar-railchat
fd38a37cecomposer frame jumped on focusmaka-composer-inner (resting state only)chat
9aad59740session timestamp stacked above its titlemaka-list-row-metamcp-hub
3dbd68ca7sidebar nav rows had two styling authoritiesmaka-list-rowmcp-hub
9d20a9396task ledger recent-count collided with its labelmaka-session-workbar-countchat
be1406705settings content column painted over the nav railsettingsSidebarsettings-general

Two are not transcribable: 3e57d1951 (stat tile leading) and 400e8f4a9 (turn marker measure) fix files the mega-branch itself created, which do not exist on main. They are recorded here as fragile spots for whichever slice introduces those components.

What review changed

Four review rounds (Codex plus an independent fresh-context agent each time), every finding reproduced before being fixed. The branch is meaningfully different as a result.

Round 1

It could report ok for a window it never launched. The harness rolled its own launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a process-group kill, ...process.env. It never verified the CDP target belonged to the child it spawned, so a leftover Electron on the port was captured instead; reproduced with two concurrent runs both reading the same window, and 270 stale user-data dirs on the dev machine. Now launched through Playwright's Electron support, with the environment builder extracted to scripts/fixture-env.mjs and shared with the E2E suite.

It could not see the class of change the migration makes.boxShadow, textAlign, flexDirection, flexWrap and outline were not captured — a utility that starts beating a product rule after the @layer move can change elevation or text placement with an identical rect. Three INITIAL_VALUES entries guessed at values Chromium does not serialise and never matched once.

Inherited values were compared against the wrong ancestor. A zero-box or display: contents wrapper is skipped, so nothing in the capture carried its values — yet visible children omitted properties that matched it. Now compared against the nearest recorded ancestor, so every omission is recoverable by a reader of the file.

The drag-region resolver was built on a false premise. It parsed stylesheet rules in source order because "the property is absent from getComputedStyle". It is not: getComputedStyle(el).webkitAppRegion returns drag, and window-titlebar.spec.ts has read it that way in CI all along. Deleted, ~50 lines.

The product change had no automated coverage.resolveDockPresentation is now a pure function with tests, and the programmatic smoke layer is pinned by the source contract.

Round 2

The two-step baseline workflow could pass silently when a step was skipped. The checker never rebuilt the candidate, the baseline was a bare array with no provenance, and the gitignored file could not travel between worktrees — comparing a build to itself read as a clean slice. Replaced by the one-command --against compare; the baseline file, the --update flag, and the .gitignore entry are gone with it.

Teardown was unbounded, and CI inherits this launcher.app.close() has no deadline; one wedged launch turned a capture run — or the CI alignment audit, whose job has no timeout — into an infinite hang, reproduced once at ≥600s. The bounded close the E2E suite already owned (grace, then SIGKILL the tree) moved to scripts/electron-lifecycle.mjs and both consumers share it; renderer evaluate gained a deadline; the temp profile is removed even when close fails.

The shared-launcher refactor had silently changed the CI gate's conditions.audit-alignment's settle budget had dropped 2500ms → 1000ms against static-markup readiness, and buildFixtureEnv read process.env.CI inline — which made the "hidden run stays hidden" test pass on every laptop and fail on the Linux CI runner. The settle budget is restored with real per-fixture ready selectors, the window-visibility decision is a pure function of the builder's arguments, and the one ambient read lives in isCiLinuxDisplay for callers to compose.

Two capture-semantics gaps.textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79–85% of records re-recorded an ancestor's value; and descendants of an opacity: 0 element (own opacity 1, non-zero rect) were captured as visible — 105 phantom records under the collapsed session panel. Fixed, plus a static table test pinning every CSS-inherited property to the inherited route in both directions.

The darwin-only matrix contradicted the issue, and its stated justification was wrong. No Windows runner is needed: the fixture platform override flips data-os through the production path on any host. The matrix now includes win32.

Round 3

The base was not isolated. The base worktree borrowed this checkout's node_modules, and the workspace links inside it resolved @maka/* straight back to the working tree — the base bundled and loaded the candidate's own packages, so a slice changing @maka/core would have compared itself against itself. Both reviewers converged on this independently; reproduced byte-for-byte with require.resolve. The base now runs its own npm ci against its own lockfile (cached per commit, guarded by an atomic build lock), and the lockfile-parity gate plus the symlink list that existed to justify the sharing are deleted.

Pseudo-elements were invisible to the contract. 47 ::before/::after rules paint in this renderer — including the full-screen body::after film-grain overlay — and a cascade flip on any of them changed pixels while every element record stayed byte-identical. Painted pseudo-elements are now captured as paint signatures that inherit from their host.

The instrument now proves its own measurement identity. CLI arguments are closed sets; every capture waits for and asserts the requested data-os and theme on the live document before reading styles — the win32 column can no longer silently degrade into a second darwin column if the override seam regresses. Dead omission rules are counted at the raw sample inside the capture (not inferred from the compressed output), so a rule shadowed by the border/outline paint gates still proves itself.

Remaining teardown edges. The smoke gate's SIGTERM paths now escalate through the shared bounded close, and a failed report write no longer leaves a live Electron behind. Verifying that fix surfaced one more: the shared force path kills by process group, the smoke gate's child was not spawned detached, so the group signal hit ESRCH, the terminator read "tree already gone", and a visible window that ignores SIGTERM survived its own SIGKILL stage. The smoke child is now a group leader like every runtime consumer, and the bounded close backs the group kill with a direct kill on the root.

Round 4

The contract could not see the paint-only channels.filter, backdrop-filter, clip-path and transform move pixels without moving the rect — grayscale() on disabled provider rows, the glass theme's backdrop blur, clip-path as visually-hidden, rotate() on square chevrons — so a migration losing one diffed zero. All four are now sampled (plus transform-origin, gated on a transform actually painting), and the capture freezes animations and transitions before reading a single style: a running spinner serialises a different matrix at every phase, which would diff time rather than the cascade. Verified with a negative control — an injected filter rule fails the compare with filter: ∅ → sepia(0.1) — and a clean 20/20 run.

The hit-test's --route was an open set. It accepted any value and failed only when nothing matched, so --route chat --route chatt dropped the typo and exited 0 as "1 route(s) clean" — silent coverage shrink on a tool whose whole point is that coverage is a number. Each value is now validated against the closed route set (as the visual contract already does), and the CLI's exit codes are pinned by subprocess tests.

Two smaller honesty fixes. Both direct-run guards compare through pathToFileURL — a checkout path with a space or non-ASCII segment percent-encodes in import.meta.url, and the string-concatenation compare would have made the instrument exit 0 having measured nothing. The tool header still described the rejected node_modules-sharing base build; it now states the actual isolation model. The direct-kill backstop from round 3 gained a fake-based test alongside its live-window proof.

Verification

Run on main (ae43cb291) + this branch, macOS arm64.

npm run lint ✓
npm run format:check ✓
npm run typecheck ✓
npx knip ✓ exit 0 (2 config hints, same as main)
npm run test:scripts ✓ 209 pass, 0 fail
npm --workspace @maka/desktop run test:dist ✓ 2981 pass, 0 fail
node scripts/audit-alignment.mjs ✓ 12/12 fixtures clean
node scripts/check-visual-contract.mjs ✓ 20/20 comparisons clean against main
(fresh base: worktree + npm ci + build, all by the tool;
negative control: an injected filter rule fails the compare)
node scripts/check-hit-test.mjs ✓ 5/5 routes clean
npx playwright test first-run window-titlebar electron-lifecycle
✓ 7 passed
npm --workspace @maka/desktop run smoke:programmatic-window
✓ PASS incl. programmatic-dock-visible (was FAIL on main)

The real-window smoke launches the app with MAKA_E2E_FIXTURE, which makes
main.ts compute startHidden and main-window.ts keep the window hidden for
its whole lifecycle. The gate therefore had nothing to look at: on a clean
main its own programmatic-window-visible check reports visible=false and
exits 1, and every human check it prompts for — dragging window edges and
corners to resize, dragging the titlebar to move, Tab/Shift+Tab traversal
inside the modal — is unrunnable against an invisible window.
Set MAKA_E2E_SHOW_WINDOW on the launch env, which is exactly the escape
hatch main.ts already provides for runs where there is no focus to steal.
Capture runs and CI e2e do not set it and keep their hidden-window
behavior unchanged.
Hiding the dock icon makes the run an accessory app. Contrary to what the
migration tracker assumed, such a window is fully clickable — measured on
darwin: with MAKA_E2E_SHOW_WINDOW set, the fixture window reports
isVisible/isFocused true and takes clicks, drags, and keyboard input
normally. What it does not have is a Dock tile or a Cmd+Tab entry, so a
reviewer doing manual visual comparison who switches to another app has no
way back to it.
Key the dock icon off the same startHidden that decides whether the window
shows at all, instead of re-deriving the condition from MAKA_E2E_FIXTURE.
The two can no longer drift, and MAKA_E2E_SHOW_WINDOW now opts out of both
halves of staying out of sight. Capture runs and CI e2e leave the variable
unset, so they keep hiding the window and the dock exactly as before.
Lock it with a dock diagnostic and a programmatic-dock-visible check in the
real-window smoke, which reported dockVisible=false before the change and
true after.
A migration that must stay visually neutral needs to look at the running
app constantly, but the only way to boot a route with fixture data was the
smoke gate, which insists on walking a reviewer through twelve checklist
prompts and filing a report every time.
Add --manual: same launch path, then stop. The window stays up until it is
closed or interrupted. Reusing the gate's launcher keeps the window a
reviewer inspects identical to the window the gate checks, and leaves one
place that knows how to boot a fixture instance.
Expose it as `launch:fixture`, and lift the build chain the smoke scripts
repeated verbatim into `build:with-deps` rather than copying it a third
time.
#1565 PR 0. The mega-branch produced regressions on screens nobody edited,
and slicing alone does not localise them: the signal has to come from a
baseline that says what the app looked like before. This captures one.
check:visual-contract walks the five representative fixture routes in light
and dark, records every visible element's box, paint, and alignment
properties, and diffs the result against committed JSON. The alignment set
(align-items/justify-content/place-items/gap/grid-area) is there because
flex and grid misalignment is exactly the class a paint-only property set
cannot see. It gates no-change, not correctness: a pre-existing bug on main
stays out of scope, and zero diff means the migration did not move this
element, never that the element is right.
The boot sequence audit-alignment.mjs already worked out — spawn Electron
with MAKA_E2E_FIXTURE, wait for a CDP page target, evaluate in the renderer
— moves to fixture-cdp.mjs so both scripts share one launcher instead of
growing a second. It also gains what a contract harness needs and an audit
could do without: animations and webfont reflow frozen before capture, an
evaluate deadline so a wedged renderer fails loudly instead of hanging, and
process-group teardown so Electron's helpers do not pile up and starve
later windows.
Also carries #1565's third PR 0 item, salvaging the mega-branch's late
regression fixes. Six of the eight land on properties this snapshot already
records — a wrapped titlebar row, a timestamp stacking above its title, a
missing gap, a column painting over the settings rail all move a rect or a
recorded property, so the diff catches them without bespoke rules. What
bespoke rules do add is noticing when the harness stops watching, so each
one is pinned to an anchor that must keep appearing in the baseline. The
remaining two fix files the mega-branch itself created, which do not exist
on main; they are recorded in the PR body instead.
Determinism was verified by capturing the same build twice. The first
attempt was not clean, and both causes are excluded rather than tuned
around: element labels fell back to el.id, which Base UI regenerates every
render, and OverlayScrollbars' chrome fades with pointer activity, so its
visibility was a coin flip. Baselines stay at 1.5MB rather than 4MB because
properties equal to their initial value, or inherited unchanged from the
parent, are omitted and read back as unchanged.
Migration-only: deliberately outside CI, following check:chat-visual, and
removed in PR 14 with the maka.legacy layer.
#1565 PR 0. The mega-branch's most expensive regressions were controls that
render perfectly and refuse clicks. A computed-style snapshot cannot see
that, so it gets a contract of its own: every visible interactive element on
the five routes must be reachable at its centre, must not sit under a
pointer-events or visibility trap, must not be transparent through an
ancestor, and must not fall inside an -webkit-app-region: drag area.
Three parts of #1565's proposal did not survive contact with a clean main,
and each is replaced by something answering the same question:
The drag-region probe was specified as a real click plus a side effect.
Fixture windows are hidden, and an unmapped window does not route
synthesized input through the browser's hit-testing path, so every target
read as swallowed. Making the window visible to fix that turned the probe
into something that fires real product actions — one opened a native file
dialog and hung the run. Reading -webkit-app-region off the stylesheet rules
instead has no side effects: the property is missing from getComputedStyle,
but not from the rules that declare it.
All five probe points were specified as required. On a clean main that is
unreachable: sibling chrome legitimately covers a couple of edge pixels, so
a workbar resize handle would indict a control nobody struggles to click.
The centre is now required and corners corroborate — three lost corners
means something covers the control.
The overlay guard keyed on [role=dialog][open], which a React-rendered
div[role=dialog] never has, so the settings surface went undetected and
every control behind it was reported unhittable. It now keys on the
product's own data-modal="true".
Geometry probes do need a visible window — elementFromPoint disagrees with
getBoundingClientRect against a hidden one — so this check steals focus for
a few seconds per route. Memoising style and rect lookups took the densest
route from a 60s timeout to 3s, and routes are spaced so one window is fully
gone before the next boots.
All five routes are clean on ae43cb2.
@Astro-Han
Astro-Han marked this pull request as draft July 30, 2026 09:45
Review found the harness could report `ok` for a window it never
launched, could not see the class of change the migration actually
makes, and shipped its only product change with no automated coverage.
Launch through the shared E2E seam. The harness rolled its own Electron
launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a
process-group kill, `...process.env` — all of it already solved next
door in `apps/desktop/e2e/fixtures.ts`, and each hand-rolled version
worse. It never verified the CDP target belonged to the child it
spawned, and the port allocator restarted at 14600 in every process, so
a leftover Electron on that port got captured instead. Reproduced: two
concurrent runs requesting different scenarios both read the same
window. When the leftover is the same route from an older build — the
shape you get by re-running one route after an edit — the contract
reports `ok` for a stale window. `detached: true` with no signal handler
supplied the leftovers; this machine had 270 stale user-data dirs, none
ever removed. `buildE2eEnv` moves to `scripts/fixture-env.mjs`, shared
by both callers; its own comment already described the bug the harness
had, since inheriting the environment leaks `VITE_DEV_SERVER_URL` and
loads the dev server instead of the build under test. Launch, readiness
and teardown now go through Playwright's Electron support: fixed sleeps
become per-route readiness selectors, and teardown removes its temp dir.
235 lines become 139, and the hit-test retry goes with them, per the
sibling harness's rule that flakes should fail loudly.
Stop committing baselines. They encode the capturing host — font metrics
and the macOS traffic-light inset — so one machine's baseline reads as
thousands of diffs on another. Capture on the branch you compare
against, apply the slice, compare: the two captures that matter always
come from the same host. The timezone is pinned through the fixture's
IANA override for the same reason. This removes 70k lines from review
and makes the orphan-baseline check moot.
Record what a cascade flip actually changes. `boxShadow`, `textAlign`,
`flexDirection`, `flexWrap` and `outline` were missing, so a utility
that starts beating a product rule could change elevation or text
placement with an identical rect and be reported clean. Three
`INITIAL_VALUES` entries guessed at values Chromium does not serialise
and never matched once, leaving constants on all 3,964 records;
`findDeadOmissionRules` now fails the run when a rule stops matching —
it caught a fourth such entry immediately. Inherited values were
compared against the parent rather than the nearest recorded ancestor,
so a zero-box wrapper's color could change, repaint every visible child,
and leave the capture byte-identical: 129 masked values on the chat
route alone.
Delete the second drag-region resolver. It parsed stylesheet rules in
source order, ignoring specificity, `!important` and `@layer` — which
PR 1 and PR 2 introduce — on the premise that the property is absent
from computed style. It is not: `getComputedStyle(el).webkitAppRegion`
returns `drag`, and `e2e/window-titlebar.spec.ts` has read it that way
in CI all along, against rendered geometry and document order. The
hit-test keeps what that spec does not cover, and now reports how many
elements it actually probed rather than how many the selector matched.
Pin the product change. `resolveDockPresentation` is a pure function
with tests; the branch it replaced could only be exercised by launching
Electron, which is why it had none. The programmatic smoke checks are
now pinned by the contract test that exists to pin checks — including
the new dock one, which could previously be deleted with CI green — and
`e2e` shares `build:with-deps` instead of repeating the chain verbatim.
…budget
buildFixtureEnv read process.env.CI inline, so the fixture-env test
asserting a hidden run stays hidden passed on every laptop and failed on
the Linux CI runner. The ambient read now lives in isCiLinuxDisplay for
callers to compose explicitly.
audit-alignment gets its 2500ms settle back (the shared-launcher refactor
had silently dropped it to 1000ms against static-markup readiness) plus a
real ready selector per fixture, and its header now states the deliberate
environment change from the raw-spawn launcher it replaced.
withFixtureWindow tore down with a bare app.close(), which has no
deadline — one wedged launch turned a capture run (or the CI alignment
audit, which has no job timeout) into an infinite hang, and the temp
userData dir leaked because rm was sequenced after the close.
The bounded close the E2E suite already owned moves to
scripts/electron-lifecycle.mjs — the same shared-home precedent as
fixture-env.mjs, so a bare-node harness and the Playwright suite stop
splitting one launch concern across two directories. Renderer evaluate
gets a deadline too; teardown then reclaims the process either way.
Two measurement-semantics gaps in the visual capture, both instances of
a class the harness had already been bitten by:
- textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79-85%
of records re-recorded an ancestor's value and one container change
printed as N descendant lines. It moves to the inherited omission (and
out of INITIAL_VALUES — absent must mean exactly one thing). A static
table test now pins every CSS-inherited property to that route, which
is the guard findDeadOmissionRules could never provide.
- Descendants of an opacity:0 element report opacity 1 and a non-zero
rect, so the collapsed session panel put 105 invisible records into
the chat capture; a change under it would diff on pixels that never
paint. The walk now prunes zero-opacity subtrees.
The prune exposed that two salvaged anchors were matching those phantom
records: the chat fixture never shows the session rows at all. They now
point at mcp-hub, whose fixture opens the sidebar for real.
Replaces the two-step gitignored-baseline workflow. The checker now
builds the base ref in a cached temp worktree (node_modules shared;
the compare refuses to run across a package-lock change), builds the
working tree, captures every route x theme x platform from both builds,
and diffs in memory. Every step of the old workflow a human could skip
- rebuild after switching, recapture after rebasing, stash on a
committed slice - produced a zero-diff pass that had measured the same
binary twice; now the instrument owns the whole measurement and there
is no baseline file to trust or to stale.
The matrix gains the win32 column #1565 asked for: the fixture platform
override drives the production app:info -> data-os path, so the per-OS
cascade is captured on any host - no Windows runner involved.
desktop-real-window-smoke was the last launcher still spreading
process.env wholesale: a developer with npm run dev open smoked the dev
server instead of the build the script just made, and the run touched
the real $HOME (#1517's skills-deletion surface). It now launches
through buildFixtureEnv with an explicit visible window.
The dock contract drops its duplicated fourth case and pins the wiring
instead - the regression this module exists for was the call site
re-deriving startHidden, which no pure-function test can observe. The
reveal contract follows fixture-env's pure shape (the ambient CI read
now lives in isCiLinuxDisplay, composed at each call site).
…entity
Round-3 review found the compare's base was not a base: the borrowed
node_modules resolved @maka/* through workspace links straight back to
the working tree, so the base bundled and loaded the candidate's own
packages — a slice changing @maka/core would have compared itself
against itself. The base now runs its own npm ci against its own
lockfile in the cached worktree (atomic lock against concurrent
builds; the marker name encodes the scheme so old symlink-era caches
rebuild). The lockfile-parity gate and the symlink list existed only
to justify the sharing; both are deleted, and a slice may now
legitimately change dependencies.
The same round showed the instrument asserting nothing about what it
measured, so now it proves it: route/theme/platform are closed sets;
every capture waits for the requested data-os and theme on the live
document before reading a style; painted ::before/::after get paint
signatures (47 pseudo rules paint in this renderer, led by the
body::after film grain, and a cascade flip on any of them was
invisible); mixBlendMode joins the property set; and dead omission
rules are counted at the raw sample where the border/outline gates
cannot shadow them.
The smoke gate still had two unbounded exits: --manual waited forever
on 'exit' after one SIGTERM, and the report path fired SIGTERM and
called process.exit without waiting or escalating — plus a failed
report write skipped the kill entirely. All paths now stop through the
shared bounded close (SIGTERM as the graceful phase, 5s grace, SIGKILL
the tree), inside a finally.
electron-lifecycle's import of @maka/runtime now fails with an error
that says what to build; on a fresh clone the raw ERR_MODULE_NOT_FOUND
surfaced from four files deep in the import chain.
The shared teardown's force path kills by process group. The smoke gate
spawned Electron without detached, so the group signal hit ESRCH, the
terminator read that as "tree already gone", and a visible window that
had ignored SIGTERM survived its own SIGKILL stage. Spawn the smoke
child as a group leader like every runtime consumer does, and back the
group kill with a direct kill on the root so a future non-leader child
still dies inside the 2s exit deadline.
filter, backdrop-filter, clip-path and transform move pixels without
moving the rect — grayscale() on disabled provider rows, the glass
theme's backdrop blur, clip-path as visually-hidden, rotate() on square
chevrons — so losing one was invisible to every property the contract
sampled. Capture all four (plus transform-origin, gated on a transform
actually painting), and stop the renderer's clock first: a running
spinner serialises a different matrix at every phase, which would diff
time rather than the cascade. Verified with a negative control (an
injected filter rule fails the compare) and a clean 20/20 run.
Also correct the header, which still described the rejected
node_modules-sharing base build, and compare the direct-run guard
through pathToFileURL so a percent-encoding checkout path cannot turn
the instrument into a silent exit-0 no-op.
--route accepted any value and only failed when nothing matched, so
`--route chat --route chatt` dropped the typo and exited 0 as "1
route(s) clean" — silently shrinking the requested coverage. Validate
each value against the closed route set (as the visual contract already
does), guard the entry point through pathToFileURL, and pin the CLI's
exit codes with subprocess tests.
The ESRCH-swallowing group-kill path was only proven against a live
window; the fake now exercises it too — a terminator that returns
without killing must be followed by a direct SIGKILL on the root.
@Astro-Han
Astro-Han marked this pull request as ready for review July 30, 2026 13:04
@Astro-Han
Astro-Han merged commit 71e47a6 into mainJul 30, 2026
3 checks passed
@Astro-Han
Astro-Han deleted the refactor/1565-pr0-contract-harness branch July 30, 2026 13:04
Sign up for freeto 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

@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(desktop): add the Astryx migration contract harness by Astro-Han · Pull Request #1653 · apache/maka · GitHub
Skip to content

feat(desktop): add the Astryx migration contract harness - #1653

Merged
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness
Jul 30, 2026
Merged

feat(desktop): add the Astryx migration contract harness#1653
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #1565. PR 0 of the Astryx renderer migration: the measurement instrument every later slice depends on. The mega-branch produced regressions on screens nobody edited, and slicing alone cannot localise them — the bisect signal has to come from a record of what the app looked like before.

  • check:visual-contract — one command compares two builds: it checks out the base ref into a cached temp worktree with its own npm ci, builds both sides, captures five representative fixture routes × light/dark × darwin/win32 from each, and diffs every visible element's box, paint, and alignment properties — plus a paint signature for every painted ::before/::after — in memory. Gates no-change, not correctness: a pre-existing bug on the base stays out of scope, and zero diff means "the migration did not move this element", never "this element is right".
  • check:hit-test — interactive elements must be reachable at their centre, free of pointer-events/visibility traps, and opaque through their ancestry; every route reports what it probed and what it skipped (invisible, out-of-scope, disabled, transparent, clipped) so the coverage is a number, not an assumption.
  • launch:fixture — a real, clickable fixture window for manual visual comparison, which a visually-neutral migration cannot do without.
  • Salvages the mega-branch's late regression fixes into the harness (item 3 below).

Both harnesses live in scripts/ as npm scripts and are deliberately outside CI, following the check:chat-visual precedent and #1565's own scoping — a two-build compare costs minutes per run, which is a per-slice gate, not a per-push one. They are migration-only and get deleted in PR 14 with the maka.legacy layer. scripts/fixture-env.mjs, scripts/fixture-window.mjs, and scripts/electron-lifecycle.mjs are the exception: audit-alignment.mjs runs in CI and the E2E suite shares them, so they outlive the harness.

How to use it

node scripts/check-visual-contract.mjs # compare main → working tree
node scripts/check-visual-contract.mjs --against <ref># a different base
node scripts/check-visual-contract.mjs --route chat --theme dark --platform win32

There is no baseline file and no capture step to remember. An earlier version had one, captured by hand on the base branch and compared by hand after switching; every step a human could skip — rebuild after switching, recapture after rebasing — produced a convincing zero-diff pass that had measured the same binary twice. Now the instrument owns the whole measurement: both sides are checked out, installed, built, and captured in the same run, on the same host (font metrics and the macOS traffic-light inset are host facts, so cross-host comparison was never sound anyway).

The base gets its own dependency closure: a fresh worktree of the ref with its own npm ci against its own lockfile, cached keyed by the resolved commit, so iterating on a slice rebuilds only the working tree. Sharing this checkout's node_modules was tried and rejected — the workspace links inside it resolve @maka/* back to the working tree's packages, so the base would have bundled and loaded the candidate's own code. A side effect of the honest install: a slice may legitimately change dependencies. Route/theme/platform arguments are closed sets, and every capture asserts the renderer actually reached the requested data-os and theme before reading a single style — a capture must prove it measured the cascade it claims.

The win32 column runs on any host: MAKA_E2E_FIXTURE_PLATFORM drives the production app:info → data-os path, which is what keys the per-OS CSS. It covers the cascade, not native chrome.

Route mapping

#1565 names product routes; these are the MAKA_E2E_FIXTURE scenarios that actually open them on main. There is no scenario literally called "chat" or "settings-providers".

RouteScenarioReadiness selector
chatturn-narrative.maka-session-workbar
settings/generalsettings-general.settingsRows
settings/providersprovider-workspace.providersPanel
mcp hubmodule-mcp.maka-module-main-header
onboardingfirst-run.maka-onboarding-surface

Known blind spots, stated in the tool's header rather than papered over: display: none subtrees (closed menus, unmounted dialogs) never enter the capture; pseudo-elements are paint signatures without a rect; native top-layer content (dialog.showModal, popover) would escape the opacity-prune, and nothing in the app uses it today.

Where this departs from #1565, and why

Each departure is a measurement, not a preference. The issue body has been corrected where it was factually wrong.

The fixture window was never unclickable.#1565 stated that app.dock.hide() makes fixture runs reject all real clicks. Measured on ae43cb291/darwin, that does not hold: MAKA_E2E_FIXTURE alone means the window is never shown at all (startHiddenkeepHiddenForE2eFixture, main-window.ts:114), and adding MAKA_E2E_SHOW_WINDOW=1 gives isVisible: true, isFocused: true, and fully working clicks — with dock.hide() still running.

scripts/desktop-real-window-smoke.mjs was already broken on main. It boots a real fixture window but never set MAKA_E2E_SHOW_WINDOW, so its own programmatic-window-visible check fails and its twelve human checks are unrunnable against an invisible window. Fixed here, extended with --manual rather than adding a second launcher, and now launched through the shared environment builder — it was the last launcher still inheriting process.env wholesale, which meant a developer with npm run dev open smoked the dev server instead of the build the script had just made.

The one product-code change is about reachability. The dock rule now keys off the same startHidden that decides whether the window shows, instead of re-deriving it from MAKA_E2E_FIXTURE — a re-derivation that had already drifted, since it missed the MAKA_E2E_SHOW_WINDOW escape hatch. An accessory app has no Dock tile and no Cmd+Tab entry, so a reviewer who switches away mid-comparison cannot switch back. Capture runs and CI e2e leave the variable unset and keep their accessory, no-focus-steal behaviour unchanged.

The window-drag question is not asked here.#1565 proposed probing it with a real click; that was tried and abandoned (an unmapped window does not route synthesized input, and a visible one fired real product actions — one opened a native file dialog and hung the run). It is also already answered: e2e/window-titlebar.spec.ts measures it in CI against rendered geometry and the document order Chromium composes drag rects in, and .maka-window-titlebar is the only element allowed to declare drag. The hit-test covers what that spec does not — whether a control anywhere in the window can be reached at all.

Centre is required; corners corroborate. All five probe points cannot pass on a clean main — sibling chrome legitimately covers a couple of edge pixels. Three lost corners means something is actually covering the control; one means the border has a neighbour.

Item 3: salvaged regressions

Six of the eight late "fix cascade / fix click" commits land on properties the snapshot already records. Bespoke rules would be redundant; what they do add is noticing when the harness stops watching, so each is pinned to an anchor that must keep appearing in a route where the element actually paints. That last clause is load-bearing: the session-row anchors originally pointed at the chat route, where the fixture keeps the panel collapsed under opacity: 0 — they "matched" records with no visible pixels until the capture started pruning invisible subtrees, at which point the check failed and exposed its own false assurance. They now point at mcp-hub, whose fixture opens the sidebar.

CommitRegressionAnchorRoute
be5e69584titlebar clusters wrapped onto a second rowmaka-shell-topbar-railchat
fd38a37cecomposer frame jumped on focusmaka-composer-inner (resting state only)chat
9aad59740session timestamp stacked above its titlemaka-list-row-metamcp-hub
3dbd68ca7sidebar nav rows had two styling authoritiesmaka-list-rowmcp-hub
9d20a9396task ledger recent-count collided with its labelmaka-session-workbar-countchat
be1406705settings content column painted over the nav railsettingsSidebarsettings-general

Two are not transcribable: 3e57d1951 (stat tile leading) and 400e8f4a9 (turn marker measure) fix files the mega-branch itself created, which do not exist on main. They are recorded here as fragile spots for whichever slice introduces those components.

What review changed

Four review rounds (Codex plus an independent fresh-context agent each time), every finding reproduced before being fixed. The branch is meaningfully different as a result.

Round 1

It could report ok for a window it never launched. The harness rolled its own launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a process-group kill, ...process.env. It never verified the CDP target belonged to the child it spawned, so a leftover Electron on the port was captured instead; reproduced with two concurrent runs both reading the same window, and 270 stale user-data dirs on the dev machine. Now launched through Playwright's Electron support, with the environment builder extracted to scripts/fixture-env.mjs and shared with the E2E suite.

It could not see the class of change the migration makes.boxShadow, textAlign, flexDirection, flexWrap and outline were not captured — a utility that starts beating a product rule after the @layer move can change elevation or text placement with an identical rect. Three INITIAL_VALUES entries guessed at values Chromium does not serialise and never matched once.

Inherited values were compared against the wrong ancestor. A zero-box or display: contents wrapper is skipped, so nothing in the capture carried its values — yet visible children omitted properties that matched it. Now compared against the nearest recorded ancestor, so every omission is recoverable by a reader of the file.

The drag-region resolver was built on a false premise. It parsed stylesheet rules in source order because "the property is absent from getComputedStyle". It is not: getComputedStyle(el).webkitAppRegion returns drag, and window-titlebar.spec.ts has read it that way in CI all along. Deleted, ~50 lines.

The product change had no automated coverage.resolveDockPresentation is now a pure function with tests, and the programmatic smoke layer is pinned by the source contract.

Round 2

The two-step baseline workflow could pass silently when a step was skipped. The checker never rebuilt the candidate, the baseline was a bare array with no provenance, and the gitignored file could not travel between worktrees — comparing a build to itself read as a clean slice. Replaced by the one-command --against compare; the baseline file, the --update flag, and the .gitignore entry are gone with it.

Teardown was unbounded, and CI inherits this launcher.app.close() has no deadline; one wedged launch turned a capture run — or the CI alignment audit, whose job has no timeout — into an infinite hang, reproduced once at ≥600s. The bounded close the E2E suite already owned (grace, then SIGKILL the tree) moved to scripts/electron-lifecycle.mjs and both consumers share it; renderer evaluate gained a deadline; the temp profile is removed even when close fails.

The shared-launcher refactor had silently changed the CI gate's conditions.audit-alignment's settle budget had dropped 2500ms → 1000ms against static-markup readiness, and buildFixtureEnv read process.env.CI inline — which made the "hidden run stays hidden" test pass on every laptop and fail on the Linux CI runner. The settle budget is restored with real per-fixture ready selectors, the window-visibility decision is a pure function of the builder's arguments, and the one ambient read lives in isCiLinuxDisplay for callers to compose.

Two capture-semantics gaps.textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79–85% of records re-recorded an ancestor's value; and descendants of an opacity: 0 element (own opacity 1, non-zero rect) were captured as visible — 105 phantom records under the collapsed session panel. Fixed, plus a static table test pinning every CSS-inherited property to the inherited route in both directions.

The darwin-only matrix contradicted the issue, and its stated justification was wrong. No Windows runner is needed: the fixture platform override flips data-os through the production path on any host. The matrix now includes win32.

Round 3

The base was not isolated. The base worktree borrowed this checkout's node_modules, and the workspace links inside it resolved @maka/* straight back to the working tree — the base bundled and loaded the candidate's own packages, so a slice changing @maka/core would have compared itself against itself. Both reviewers converged on this independently; reproduced byte-for-byte with require.resolve. The base now runs its own npm ci against its own lockfile (cached per commit, guarded by an atomic build lock), and the lockfile-parity gate plus the symlink list that existed to justify the sharing are deleted.

Pseudo-elements were invisible to the contract. 47 ::before/::after rules paint in this renderer — including the full-screen body::after film-grain overlay — and a cascade flip on any of them changed pixels while every element record stayed byte-identical. Painted pseudo-elements are now captured as paint signatures that inherit from their host.

The instrument now proves its own measurement identity. CLI arguments are closed sets; every capture waits for and asserts the requested data-os and theme on the live document before reading styles — the win32 column can no longer silently degrade into a second darwin column if the override seam regresses. Dead omission rules are counted at the raw sample inside the capture (not inferred from the compressed output), so a rule shadowed by the border/outline paint gates still proves itself.

Remaining teardown edges. The smoke gate's SIGTERM paths now escalate through the shared bounded close, and a failed report write no longer leaves a live Electron behind. Verifying that fix surfaced one more: the shared force path kills by process group, the smoke gate's child was not spawned detached, so the group signal hit ESRCH, the terminator read "tree already gone", and a visible window that ignores SIGTERM survived its own SIGKILL stage. The smoke child is now a group leader like every runtime consumer, and the bounded close backs the group kill with a direct kill on the root.

Round 4

The contract could not see the paint-only channels.filter, backdrop-filter, clip-path and transform move pixels without moving the rect — grayscale() on disabled provider rows, the glass theme's backdrop blur, clip-path as visually-hidden, rotate() on square chevrons — so a migration losing one diffed zero. All four are now sampled (plus transform-origin, gated on a transform actually painting), and the capture freezes animations and transitions before reading a single style: a running spinner serialises a different matrix at every phase, which would diff time rather than the cascade. Verified with a negative control — an injected filter rule fails the compare with filter: ∅ → sepia(0.1) — and a clean 20/20 run.

The hit-test's --route was an open set. It accepted any value and failed only when nothing matched, so --route chat --route chatt dropped the typo and exited 0 as "1 route(s) clean" — silent coverage shrink on a tool whose whole point is that coverage is a number. Each value is now validated against the closed route set (as the visual contract already does), and the CLI's exit codes are pinned by subprocess tests.

Two smaller honesty fixes. Both direct-run guards compare through pathToFileURL — a checkout path with a space or non-ASCII segment percent-encodes in import.meta.url, and the string-concatenation compare would have made the instrument exit 0 having measured nothing. The tool header still described the rejected node_modules-sharing base build; it now states the actual isolation model. The direct-kill backstop from round 3 gained a fake-based test alongside its live-window proof.

Verification

Run on main (ae43cb291) + this branch, macOS arm64.

npm run lint ✓
npm run format:check ✓
npm run typecheck ✓
npx knip ✓ exit 0 (2 config hints, same as main)
npm run test:scripts ✓ 209 pass, 0 fail
npm --workspace @maka/desktop run test:dist ✓ 2981 pass, 0 fail
node scripts/audit-alignment.mjs ✓ 12/12 fixtures clean
node scripts/check-visual-contract.mjs ✓ 20/20 comparisons clean against main
(fresh base: worktree + npm ci + build, all by the tool;
negative control: an injected filter rule fails the compare)
node scripts/check-hit-test.mjs ✓ 5/5 routes clean
npx playwright test first-run window-titlebar electron-lifecycle
✓ 7 passed
npm --workspace @maka/desktop run smoke:programmatic-window
✓ PASS incl. programmatic-dock-visible (was FAIL on main)

The real-window smoke launches the app with MAKA_E2E_FIXTURE, which makes
main.ts compute startHidden and main-window.ts keep the window hidden for
its whole lifecycle. The gate therefore had nothing to look at: on a clean
main its own programmatic-window-visible check reports visible=false and
exits 1, and every human check it prompts for — dragging window edges and
corners to resize, dragging the titlebar to move, Tab/Shift+Tab traversal
inside the modal — is unrunnable against an invisible window.
Set MAKA_E2E_SHOW_WINDOW on the launch env, which is exactly the escape
hatch main.ts already provides for runs where there is no focus to steal.
Capture runs and CI e2e do not set it and keep their hidden-window
behavior unchanged.
Hiding the dock icon makes the run an accessory app. Contrary to what the
migration tracker assumed, such a window is fully clickable — measured on
darwin: with MAKA_E2E_SHOW_WINDOW set, the fixture window reports
isVisible/isFocused true and takes clicks, drags, and keyboard input
normally. What it does not have is a Dock tile or a Cmd+Tab entry, so a
reviewer doing manual visual comparison who switches to another app has no
way back to it.
Key the dock icon off the same startHidden that decides whether the window
shows at all, instead of re-deriving the condition from MAKA_E2E_FIXTURE.
The two can no longer drift, and MAKA_E2E_SHOW_WINDOW now opts out of both
halves of staying out of sight. Capture runs and CI e2e leave the variable
unset, so they keep hiding the window and the dock exactly as before.
Lock it with a dock diagnostic and a programmatic-dock-visible check in the
real-window smoke, which reported dockVisible=false before the change and
true after.
A migration that must stay visually neutral needs to look at the running
app constantly, but the only way to boot a route with fixture data was the
smoke gate, which insists on walking a reviewer through twelve checklist
prompts and filing a report every time.
Add --manual: same launch path, then stop. The window stays up until it is
closed or interrupted. Reusing the gate's launcher keeps the window a
reviewer inspects identical to the window the gate checks, and leaves one
place that knows how to boot a fixture instance.
Expose it as `launch:fixture`, and lift the build chain the smoke scripts
repeated verbatim into `build:with-deps` rather than copying it a third
time.
#1565 PR 0. The mega-branch produced regressions on screens nobody edited,
and slicing alone does not localise them: the signal has to come from a
baseline that says what the app looked like before. This captures one.
check:visual-contract walks the five representative fixture routes in light
and dark, records every visible element's box, paint, and alignment
properties, and diffs the result against committed JSON. The alignment set
(align-items/justify-content/place-items/gap/grid-area) is there because
flex and grid misalignment is exactly the class a paint-only property set
cannot see. It gates no-change, not correctness: a pre-existing bug on main
stays out of scope, and zero diff means the migration did not move this
element, never that the element is right.
The boot sequence audit-alignment.mjs already worked out — spawn Electron
with MAKA_E2E_FIXTURE, wait for a CDP page target, evaluate in the renderer
— moves to fixture-cdp.mjs so both scripts share one launcher instead of
growing a second. It also gains what a contract harness needs and an audit
could do without: animations and webfont reflow frozen before capture, an
evaluate deadline so a wedged renderer fails loudly instead of hanging, and
process-group teardown so Electron's helpers do not pile up and starve
later windows.
Also carries #1565's third PR 0 item, salvaging the mega-branch's late
regression fixes. Six of the eight land on properties this snapshot already
records — a wrapped titlebar row, a timestamp stacking above its title, a
missing gap, a column painting over the settings rail all move a rect or a
recorded property, so the diff catches them without bespoke rules. What
bespoke rules do add is noticing when the harness stops watching, so each
one is pinned to an anchor that must keep appearing in the baseline. The
remaining two fix files the mega-branch itself created, which do not exist
on main; they are recorded in the PR body instead.
Determinism was verified by capturing the same build twice. The first
attempt was not clean, and both causes are excluded rather than tuned
around: element labels fell back to el.id, which Base UI regenerates every
render, and OverlayScrollbars' chrome fades with pointer activity, so its
visibility was a coin flip. Baselines stay at 1.5MB rather than 4MB because
properties equal to their initial value, or inherited unchanged from the
parent, are omitted and read back as unchanged.
Migration-only: deliberately outside CI, following check:chat-visual, and
removed in PR 14 with the maka.legacy layer.
#1565 PR 0. The mega-branch's most expensive regressions were controls that
render perfectly and refuse clicks. A computed-style snapshot cannot see
that, so it gets a contract of its own: every visible interactive element on
the five routes must be reachable at its centre, must not sit under a
pointer-events or visibility trap, must not be transparent through an
ancestor, and must not fall inside an -webkit-app-region: drag area.
Three parts of #1565's proposal did not survive contact with a clean main,
and each is replaced by something answering the same question:
The drag-region probe was specified as a real click plus a side effect.
Fixture windows are hidden, and an unmapped window does not route
synthesized input through the browser's hit-testing path, so every target
read as swallowed. Making the window visible to fix that turned the probe
into something that fires real product actions — one opened a native file
dialog and hung the run. Reading -webkit-app-region off the stylesheet rules
instead has no side effects: the property is missing from getComputedStyle,
but not from the rules that declare it.
All five probe points were specified as required. On a clean main that is
unreachable: sibling chrome legitimately covers a couple of edge pixels, so
a workbar resize handle would indict a control nobody struggles to click.
The centre is now required and corners corroborate — three lost corners
means something covers the control.
The overlay guard keyed on [role=dialog][open], which a React-rendered
div[role=dialog] never has, so the settings surface went undetected and
every control behind it was reported unhittable. It now keys on the
product's own data-modal="true".
Geometry probes do need a visible window — elementFromPoint disagrees with
getBoundingClientRect against a hidden one — so this check steals focus for
a few seconds per route. Memoising style and rect lookups took the densest
route from a 60s timeout to 3s, and routes are spaced so one window is fully
gone before the next boots.
All five routes are clean on ae43cb2.
@Astro-Han
Astro-Han marked this pull request as draft July 30, 2026 09:45
Review found the harness could report `ok` for a window it never
launched, could not see the class of change the migration actually
makes, and shipped its only product change with no automated coverage.
Launch through the shared E2E seam. The harness rolled its own Electron
launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a
process-group kill, `...process.env` — all of it already solved next
door in `apps/desktop/e2e/fixtures.ts`, and each hand-rolled version
worse. It never verified the CDP target belonged to the child it
spawned, and the port allocator restarted at 14600 in every process, so
a leftover Electron on that port got captured instead. Reproduced: two
concurrent runs requesting different scenarios both read the same
window. When the leftover is the same route from an older build — the
shape you get by re-running one route after an edit — the contract
reports `ok` for a stale window. `detached: true` with no signal handler
supplied the leftovers; this machine had 270 stale user-data dirs, none
ever removed. `buildE2eEnv` moves to `scripts/fixture-env.mjs`, shared
by both callers; its own comment already described the bug the harness
had, since inheriting the environment leaks `VITE_DEV_SERVER_URL` and
loads the dev server instead of the build under test. Launch, readiness
and teardown now go through Playwright's Electron support: fixed sleeps
become per-route readiness selectors, and teardown removes its temp dir.
235 lines become 139, and the hit-test retry goes with them, per the
sibling harness's rule that flakes should fail loudly.
Stop committing baselines. They encode the capturing host — font metrics
and the macOS traffic-light inset — so one machine's baseline reads as
thousands of diffs on another. Capture on the branch you compare
against, apply the slice, compare: the two captures that matter always
come from the same host. The timezone is pinned through the fixture's
IANA override for the same reason. This removes 70k lines from review
and makes the orphan-baseline check moot.
Record what a cascade flip actually changes. `boxShadow`, `textAlign`,
`flexDirection`, `flexWrap` and `outline` were missing, so a utility
that starts beating a product rule could change elevation or text
placement with an identical rect and be reported clean. Three
`INITIAL_VALUES` entries guessed at values Chromium does not serialise
and never matched once, leaving constants on all 3,964 records;
`findDeadOmissionRules` now fails the run when a rule stops matching —
it caught a fourth such entry immediately. Inherited values were
compared against the parent rather than the nearest recorded ancestor,
so a zero-box wrapper's color could change, repaint every visible child,
and leave the capture byte-identical: 129 masked values on the chat
route alone.
Delete the second drag-region resolver. It parsed stylesheet rules in
source order, ignoring specificity, `!important` and `@layer` — which
PR 1 and PR 2 introduce — on the premise that the property is absent
from computed style. It is not: `getComputedStyle(el).webkitAppRegion`
returns `drag`, and `e2e/window-titlebar.spec.ts` has read it that way
in CI all along, against rendered geometry and document order. The
hit-test keeps what that spec does not cover, and now reports how many
elements it actually probed rather than how many the selector matched.
Pin the product change. `resolveDockPresentation` is a pure function
with tests; the branch it replaced could only be exercised by launching
Electron, which is why it had none. The programmatic smoke checks are
now pinned by the contract test that exists to pin checks — including
the new dock one, which could previously be deleted with CI green — and
`e2e` shares `build:with-deps` instead of repeating the chain verbatim.
…budget
buildFixtureEnv read process.env.CI inline, so the fixture-env test
asserting a hidden run stays hidden passed on every laptop and failed on
the Linux CI runner. The ambient read now lives in isCiLinuxDisplay for
callers to compose explicitly.
audit-alignment gets its 2500ms settle back (the shared-launcher refactor
had silently dropped it to 1000ms against static-markup readiness) plus a
real ready selector per fixture, and its header now states the deliberate
environment change from the raw-spawn launcher it replaced.
withFixtureWindow tore down with a bare app.close(), which has no
deadline — one wedged launch turned a capture run (or the CI alignment
audit, which has no job timeout) into an infinite hang, and the temp
userData dir leaked because rm was sequenced after the close.
The bounded close the E2E suite already owned moves to
scripts/electron-lifecycle.mjs — the same shared-home precedent as
fixture-env.mjs, so a bare-node harness and the Playwright suite stop
splitting one launch concern across two directories. Renderer evaluate
gets a deadline too; teardown then reclaims the process either way.
Two measurement-semantics gaps in the visual capture, both instances of
a class the harness had already been bitten by:
- textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79-85%
of records re-recorded an ancestor's value and one container change
printed as N descendant lines. It moves to the inherited omission (and
out of INITIAL_VALUES — absent must mean exactly one thing). A static
table test now pins every CSS-inherited property to that route, which
is the guard findDeadOmissionRules could never provide.
- Descendants of an opacity:0 element report opacity 1 and a non-zero
rect, so the collapsed session panel put 105 invisible records into
the chat capture; a change under it would diff on pixels that never
paint. The walk now prunes zero-opacity subtrees.
The prune exposed that two salvaged anchors were matching those phantom
records: the chat fixture never shows the session rows at all. They now
point at mcp-hub, whose fixture opens the sidebar for real.
Replaces the two-step gitignored-baseline workflow. The checker now
builds the base ref in a cached temp worktree (node_modules shared;
the compare refuses to run across a package-lock change), builds the
working tree, captures every route x theme x platform from both builds,
and diffs in memory. Every step of the old workflow a human could skip
- rebuild after switching, recapture after rebasing, stash on a
committed slice - produced a zero-diff pass that had measured the same
binary twice; now the instrument owns the whole measurement and there
is no baseline file to trust or to stale.
The matrix gains the win32 column #1565 asked for: the fixture platform
override drives the production app:info -> data-os path, so the per-OS
cascade is captured on any host - no Windows runner involved.
desktop-real-window-smoke was the last launcher still spreading
process.env wholesale: a developer with npm run dev open smoked the dev
server instead of the build the script just made, and the run touched
the real $HOME (#1517's skills-deletion surface). It now launches
through buildFixtureEnv with an explicit visible window.
The dock contract drops its duplicated fourth case and pins the wiring
instead - the regression this module exists for was the call site
re-deriving startHidden, which no pure-function test can observe. The
reveal contract follows fixture-env's pure shape (the ambient CI read
now lives in isCiLinuxDisplay, composed at each call site).
…entity
Round-3 review found the compare's base was not a base: the borrowed
node_modules resolved @maka/* through workspace links straight back to
the working tree, so the base bundled and loaded the candidate's own
packages — a slice changing @maka/core would have compared itself
against itself. The base now runs its own npm ci against its own
lockfile in the cached worktree (atomic lock against concurrent
builds; the marker name encodes the scheme so old symlink-era caches
rebuild). The lockfile-parity gate and the symlink list existed only
to justify the sharing; both are deleted, and a slice may now
legitimately change dependencies.
The same round showed the instrument asserting nothing about what it
measured, so now it proves it: route/theme/platform are closed sets;
every capture waits for the requested data-os and theme on the live
document before reading a style; painted ::before/::after get paint
signatures (47 pseudo rules paint in this renderer, led by the
body::after film grain, and a cascade flip on any of them was
invisible); mixBlendMode joins the property set; and dead omission
rules are counted at the raw sample where the border/outline gates
cannot shadow them.
The smoke gate still had two unbounded exits: --manual waited forever
on 'exit' after one SIGTERM, and the report path fired SIGTERM and
called process.exit without waiting or escalating — plus a failed
report write skipped the kill entirely. All paths now stop through the
shared bounded close (SIGTERM as the graceful phase, 5s grace, SIGKILL
the tree), inside a finally.
electron-lifecycle's import of @maka/runtime now fails with an error
that says what to build; on a fresh clone the raw ERR_MODULE_NOT_FOUND
surfaced from four files deep in the import chain.
The shared teardown's force path kills by process group. The smoke gate
spawned Electron without detached, so the group signal hit ESRCH, the
terminator read that as "tree already gone", and a visible window that
had ignored SIGTERM survived its own SIGKILL stage. Spawn the smoke
child as a group leader like every runtime consumer does, and back the
group kill with a direct kill on the root so a future non-leader child
still dies inside the 2s exit deadline.
filter, backdrop-filter, clip-path and transform move pixels without
moving the rect — grayscale() on disabled provider rows, the glass
theme's backdrop blur, clip-path as visually-hidden, rotate() on square
chevrons — so losing one was invisible to every property the contract
sampled. Capture all four (plus transform-origin, gated on a transform
actually painting), and stop the renderer's clock first: a running
spinner serialises a different matrix at every phase, which would diff
time rather than the cascade. Verified with a negative control (an
injected filter rule fails the compare) and a clean 20/20 run.
Also correct the header, which still described the rejected
node_modules-sharing base build, and compare the direct-run guard
through pathToFileURL so a percent-encoding checkout path cannot turn
the instrument into a silent exit-0 no-op.
--route accepted any value and only failed when nothing matched, so
`--route chat --route chatt` dropped the typo and exited 0 as "1
route(s) clean" — silently shrinking the requested coverage. Validate
each value against the closed route set (as the visual contract already
does), guard the entry point through pathToFileURL, and pin the CLI's
exit codes with subprocess tests.
The ESRCH-swallowing group-kill path was only proven against a live
window; the fake now exercises it too — a terminator that returns
without killing must be followed by a direct SIGKILL on the root.
@Astro-Han
Astro-Han marked this pull request as ready for review July 30, 2026 13:04
@Astro-Han
Astro-Han merged commit 71e47a6 into mainJul 30, 2026
3 checks passed
@Astro-Han
Astro-Han deleted the refactor/1565-pr0-contract-harness branch July 30, 2026 13:04
Sign up for freeto 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

@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(desktop): add the Astryx migration contract harness by Astro-Han · Pull Request #1653 · apache/maka · GitHub
Skip to content

feat(desktop): add the Astryx migration contract harness - #1653

Merged
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness
Jul 30, 2026
Merged

feat(desktop): add the Astryx migration contract harness#1653
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #1565. PR 0 of the Astryx renderer migration: the measurement instrument every later slice depends on. The mega-branch produced regressions on screens nobody edited, and slicing alone cannot localise them — the bisect signal has to come from a record of what the app looked like before.

  • check:visual-contract — one command compares two builds: it checks out the base ref into a cached temp worktree with its own npm ci, builds both sides, captures five representative fixture routes × light/dark × darwin/win32 from each, and diffs every visible element's box, paint, and alignment properties — plus a paint signature for every painted ::before/::after — in memory. Gates no-change, not correctness: a pre-existing bug on the base stays out of scope, and zero diff means "the migration did not move this element", never "this element is right".
  • check:hit-test — interactive elements must be reachable at their centre, free of pointer-events/visibility traps, and opaque through their ancestry; every route reports what it probed and what it skipped (invisible, out-of-scope, disabled, transparent, clipped) so the coverage is a number, not an assumption.
  • launch:fixture — a real, clickable fixture window for manual visual comparison, which a visually-neutral migration cannot do without.
  • Salvages the mega-branch's late regression fixes into the harness (item 3 below).

Both harnesses live in scripts/ as npm scripts and are deliberately outside CI, following the check:chat-visual precedent and #1565's own scoping — a two-build compare costs minutes per run, which is a per-slice gate, not a per-push one. They are migration-only and get deleted in PR 14 with the maka.legacy layer. scripts/fixture-env.mjs, scripts/fixture-window.mjs, and scripts/electron-lifecycle.mjs are the exception: audit-alignment.mjs runs in CI and the E2E suite shares them, so they outlive the harness.

How to use it

node scripts/check-visual-contract.mjs # compare main → working tree
node scripts/check-visual-contract.mjs --against <ref># a different base
node scripts/check-visual-contract.mjs --route chat --theme dark --platform win32

There is no baseline file and no capture step to remember. An earlier version had one, captured by hand on the base branch and compared by hand after switching; every step a human could skip — rebuild after switching, recapture after rebasing — produced a convincing zero-diff pass that had measured the same binary twice. Now the instrument owns the whole measurement: both sides are checked out, installed, built, and captured in the same run, on the same host (font metrics and the macOS traffic-light inset are host facts, so cross-host comparison was never sound anyway).

The base gets its own dependency closure: a fresh worktree of the ref with its own npm ci against its own lockfile, cached keyed by the resolved commit, so iterating on a slice rebuilds only the working tree. Sharing this checkout's node_modules was tried and rejected — the workspace links inside it resolve @maka/* back to the working tree's packages, so the base would have bundled and loaded the candidate's own code. A side effect of the honest install: a slice may legitimately change dependencies. Route/theme/platform arguments are closed sets, and every capture asserts the renderer actually reached the requested data-os and theme before reading a single style — a capture must prove it measured the cascade it claims.

The win32 column runs on any host: MAKA_E2E_FIXTURE_PLATFORM drives the production app:info → data-os path, which is what keys the per-OS CSS. It covers the cascade, not native chrome.

Route mapping

#1565 names product routes; these are the MAKA_E2E_FIXTURE scenarios that actually open them on main. There is no scenario literally called "chat" or "settings-providers".

RouteScenarioReadiness selector
chatturn-narrative.maka-session-workbar
settings/generalsettings-general.settingsRows
settings/providersprovider-workspace.providersPanel
mcp hubmodule-mcp.maka-module-main-header
onboardingfirst-run.maka-onboarding-surface

Known blind spots, stated in the tool's header rather than papered over: display: none subtrees (closed menus, unmounted dialogs) never enter the capture; pseudo-elements are paint signatures without a rect; native top-layer content (dialog.showModal, popover) would escape the opacity-prune, and nothing in the app uses it today.

Where this departs from #1565, and why

Each departure is a measurement, not a preference. The issue body has been corrected where it was factually wrong.

The fixture window was never unclickable.#1565 stated that app.dock.hide() makes fixture runs reject all real clicks. Measured on ae43cb291/darwin, that does not hold: MAKA_E2E_FIXTURE alone means the window is never shown at all (startHiddenkeepHiddenForE2eFixture, main-window.ts:114), and adding MAKA_E2E_SHOW_WINDOW=1 gives isVisible: true, isFocused: true, and fully working clicks — with dock.hide() still running.

scripts/desktop-real-window-smoke.mjs was already broken on main. It boots a real fixture window but never set MAKA_E2E_SHOW_WINDOW, so its own programmatic-window-visible check fails and its twelve human checks are unrunnable against an invisible window. Fixed here, extended with --manual rather than adding a second launcher, and now launched through the shared environment builder — it was the last launcher still inheriting process.env wholesale, which meant a developer with npm run dev open smoked the dev server instead of the build the script had just made.

The one product-code change is about reachability. The dock rule now keys off the same startHidden that decides whether the window shows, instead of re-deriving it from MAKA_E2E_FIXTURE — a re-derivation that had already drifted, since it missed the MAKA_E2E_SHOW_WINDOW escape hatch. An accessory app has no Dock tile and no Cmd+Tab entry, so a reviewer who switches away mid-comparison cannot switch back. Capture runs and CI e2e leave the variable unset and keep their accessory, no-focus-steal behaviour unchanged.

The window-drag question is not asked here.#1565 proposed probing it with a real click; that was tried and abandoned (an unmapped window does not route synthesized input, and a visible one fired real product actions — one opened a native file dialog and hung the run). It is also already answered: e2e/window-titlebar.spec.ts measures it in CI against rendered geometry and the document order Chromium composes drag rects in, and .maka-window-titlebar is the only element allowed to declare drag. The hit-test covers what that spec does not — whether a control anywhere in the window can be reached at all.

Centre is required; corners corroborate. All five probe points cannot pass on a clean main — sibling chrome legitimately covers a couple of edge pixels. Three lost corners means something is actually covering the control; one means the border has a neighbour.

Item 3: salvaged regressions

Six of the eight late "fix cascade / fix click" commits land on properties the snapshot already records. Bespoke rules would be redundant; what they do add is noticing when the harness stops watching, so each is pinned to an anchor that must keep appearing in a route where the element actually paints. That last clause is load-bearing: the session-row anchors originally pointed at the chat route, where the fixture keeps the panel collapsed under opacity: 0 — they "matched" records with no visible pixels until the capture started pruning invisible subtrees, at which point the check failed and exposed its own false assurance. They now point at mcp-hub, whose fixture opens the sidebar.

CommitRegressionAnchorRoute
be5e69584titlebar clusters wrapped onto a second rowmaka-shell-topbar-railchat
fd38a37cecomposer frame jumped on focusmaka-composer-inner (resting state only)chat
9aad59740session timestamp stacked above its titlemaka-list-row-metamcp-hub
3dbd68ca7sidebar nav rows had two styling authoritiesmaka-list-rowmcp-hub
9d20a9396task ledger recent-count collided with its labelmaka-session-workbar-countchat
be1406705settings content column painted over the nav railsettingsSidebarsettings-general

Two are not transcribable: 3e57d1951 (stat tile leading) and 400e8f4a9 (turn marker measure) fix files the mega-branch itself created, which do not exist on main. They are recorded here as fragile spots for whichever slice introduces those components.

What review changed

Four review rounds (Codex plus an independent fresh-context agent each time), every finding reproduced before being fixed. The branch is meaningfully different as a result.

Round 1

It could report ok for a window it never launched. The harness rolled its own launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a process-group kill, ...process.env. It never verified the CDP target belonged to the child it spawned, so a leftover Electron on the port was captured instead; reproduced with two concurrent runs both reading the same window, and 270 stale user-data dirs on the dev machine. Now launched through Playwright's Electron support, with the environment builder extracted to scripts/fixture-env.mjs and shared with the E2E suite.

It could not see the class of change the migration makes.boxShadow, textAlign, flexDirection, flexWrap and outline were not captured — a utility that starts beating a product rule after the @layer move can change elevation or text placement with an identical rect. Three INITIAL_VALUES entries guessed at values Chromium does not serialise and never matched once.

Inherited values were compared against the wrong ancestor. A zero-box or display: contents wrapper is skipped, so nothing in the capture carried its values — yet visible children omitted properties that matched it. Now compared against the nearest recorded ancestor, so every omission is recoverable by a reader of the file.

The drag-region resolver was built on a false premise. It parsed stylesheet rules in source order because "the property is absent from getComputedStyle". It is not: getComputedStyle(el).webkitAppRegion returns drag, and window-titlebar.spec.ts has read it that way in CI all along. Deleted, ~50 lines.

The product change had no automated coverage.resolveDockPresentation is now a pure function with tests, and the programmatic smoke layer is pinned by the source contract.

Round 2

The two-step baseline workflow could pass silently when a step was skipped. The checker never rebuilt the candidate, the baseline was a bare array with no provenance, and the gitignored file could not travel between worktrees — comparing a build to itself read as a clean slice. Replaced by the one-command --against compare; the baseline file, the --update flag, and the .gitignore entry are gone with it.

Teardown was unbounded, and CI inherits this launcher.app.close() has no deadline; one wedged launch turned a capture run — or the CI alignment audit, whose job has no timeout — into an infinite hang, reproduced once at ≥600s. The bounded close the E2E suite already owned (grace, then SIGKILL the tree) moved to scripts/electron-lifecycle.mjs and both consumers share it; renderer evaluate gained a deadline; the temp profile is removed even when close fails.

The shared-launcher refactor had silently changed the CI gate's conditions.audit-alignment's settle budget had dropped 2500ms → 1000ms against static-markup readiness, and buildFixtureEnv read process.env.CI inline — which made the "hidden run stays hidden" test pass on every laptop and fail on the Linux CI runner. The settle budget is restored with real per-fixture ready selectors, the window-visibility decision is a pure function of the builder's arguments, and the one ambient read lives in isCiLinuxDisplay for callers to compose.

Two capture-semantics gaps.textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79–85% of records re-recorded an ancestor's value; and descendants of an opacity: 0 element (own opacity 1, non-zero rect) were captured as visible — 105 phantom records under the collapsed session panel. Fixed, plus a static table test pinning every CSS-inherited property to the inherited route in both directions.

The darwin-only matrix contradicted the issue, and its stated justification was wrong. No Windows runner is needed: the fixture platform override flips data-os through the production path on any host. The matrix now includes win32.

Round 3

The base was not isolated. The base worktree borrowed this checkout's node_modules, and the workspace links inside it resolved @maka/* straight back to the working tree — the base bundled and loaded the candidate's own packages, so a slice changing @maka/core would have compared itself against itself. Both reviewers converged on this independently; reproduced byte-for-byte with require.resolve. The base now runs its own npm ci against its own lockfile (cached per commit, guarded by an atomic build lock), and the lockfile-parity gate plus the symlink list that existed to justify the sharing are deleted.

Pseudo-elements were invisible to the contract. 47 ::before/::after rules paint in this renderer — including the full-screen body::after film-grain overlay — and a cascade flip on any of them changed pixels while every element record stayed byte-identical. Painted pseudo-elements are now captured as paint signatures that inherit from their host.

The instrument now proves its own measurement identity. CLI arguments are closed sets; every capture waits for and asserts the requested data-os and theme on the live document before reading styles — the win32 column can no longer silently degrade into a second darwin column if the override seam regresses. Dead omission rules are counted at the raw sample inside the capture (not inferred from the compressed output), so a rule shadowed by the border/outline paint gates still proves itself.

Remaining teardown edges. The smoke gate's SIGTERM paths now escalate through the shared bounded close, and a failed report write no longer leaves a live Electron behind. Verifying that fix surfaced one more: the shared force path kills by process group, the smoke gate's child was not spawned detached, so the group signal hit ESRCH, the terminator read "tree already gone", and a visible window that ignores SIGTERM survived its own SIGKILL stage. The smoke child is now a group leader like every runtime consumer, and the bounded close backs the group kill with a direct kill on the root.

Round 4

The contract could not see the paint-only channels.filter, backdrop-filter, clip-path and transform move pixels without moving the rect — grayscale() on disabled provider rows, the glass theme's backdrop blur, clip-path as visually-hidden, rotate() on square chevrons — so a migration losing one diffed zero. All four are now sampled (plus transform-origin, gated on a transform actually painting), and the capture freezes animations and transitions before reading a single style: a running spinner serialises a different matrix at every phase, which would diff time rather than the cascade. Verified with a negative control — an injected filter rule fails the compare with filter: ∅ → sepia(0.1) — and a clean 20/20 run.

The hit-test's --route was an open set. It accepted any value and failed only when nothing matched, so --route chat --route chatt dropped the typo and exited 0 as "1 route(s) clean" — silent coverage shrink on a tool whose whole point is that coverage is a number. Each value is now validated against the closed route set (as the visual contract already does), and the CLI's exit codes are pinned by subprocess tests.

Two smaller honesty fixes. Both direct-run guards compare through pathToFileURL — a checkout path with a space or non-ASCII segment percent-encodes in import.meta.url, and the string-concatenation compare would have made the instrument exit 0 having measured nothing. The tool header still described the rejected node_modules-sharing base build; it now states the actual isolation model. The direct-kill backstop from round 3 gained a fake-based test alongside its live-window proof.

Verification

Run on main (ae43cb291) + this branch, macOS arm64.

npm run lint ✓
npm run format:check ✓
npm run typecheck ✓
npx knip ✓ exit 0 (2 config hints, same as main)
npm run test:scripts ✓ 209 pass, 0 fail
npm --workspace @maka/desktop run test:dist ✓ 2981 pass, 0 fail
node scripts/audit-alignment.mjs ✓ 12/12 fixtures clean
node scripts/check-visual-contract.mjs ✓ 20/20 comparisons clean against main
(fresh base: worktree + npm ci + build, all by the tool;
negative control: an injected filter rule fails the compare)
node scripts/check-hit-test.mjs ✓ 5/5 routes clean
npx playwright test first-run window-titlebar electron-lifecycle
✓ 7 passed
npm --workspace @maka/desktop run smoke:programmatic-window
✓ PASS incl. programmatic-dock-visible (was FAIL on main)

The real-window smoke launches the app with MAKA_E2E_FIXTURE, which makes
main.ts compute startHidden and main-window.ts keep the window hidden for
its whole lifecycle. The gate therefore had nothing to look at: on a clean
main its own programmatic-window-visible check reports visible=false and
exits 1, and every human check it prompts for — dragging window edges and
corners to resize, dragging the titlebar to move, Tab/Shift+Tab traversal
inside the modal — is unrunnable against an invisible window.
Set MAKA_E2E_SHOW_WINDOW on the launch env, which is exactly the escape
hatch main.ts already provides for runs where there is no focus to steal.
Capture runs and CI e2e do not set it and keep their hidden-window
behavior unchanged.
Hiding the dock icon makes the run an accessory app. Contrary to what the
migration tracker assumed, such a window is fully clickable — measured on
darwin: with MAKA_E2E_SHOW_WINDOW set, the fixture window reports
isVisible/isFocused true and takes clicks, drags, and keyboard input
normally. What it does not have is a Dock tile or a Cmd+Tab entry, so a
reviewer doing manual visual comparison who switches to another app has no
way back to it.
Key the dock icon off the same startHidden that decides whether the window
shows at all, instead of re-deriving the condition from MAKA_E2E_FIXTURE.
The two can no longer drift, and MAKA_E2E_SHOW_WINDOW now opts out of both
halves of staying out of sight. Capture runs and CI e2e leave the variable
unset, so they keep hiding the window and the dock exactly as before.
Lock it with a dock diagnostic and a programmatic-dock-visible check in the
real-window smoke, which reported dockVisible=false before the change and
true after.
A migration that must stay visually neutral needs to look at the running
app constantly, but the only way to boot a route with fixture data was the
smoke gate, which insists on walking a reviewer through twelve checklist
prompts and filing a report every time.
Add --manual: same launch path, then stop. The window stays up until it is
closed or interrupted. Reusing the gate's launcher keeps the window a
reviewer inspects identical to the window the gate checks, and leaves one
place that knows how to boot a fixture instance.
Expose it as `launch:fixture`, and lift the build chain the smoke scripts
repeated verbatim into `build:with-deps` rather than copying it a third
time.
#1565 PR 0. The mega-branch produced regressions on screens nobody edited,
and slicing alone does not localise them: the signal has to come from a
baseline that says what the app looked like before. This captures one.
check:visual-contract walks the five representative fixture routes in light
and dark, records every visible element's box, paint, and alignment
properties, and diffs the result against committed JSON. The alignment set
(align-items/justify-content/place-items/gap/grid-area) is there because
flex and grid misalignment is exactly the class a paint-only property set
cannot see. It gates no-change, not correctness: a pre-existing bug on main
stays out of scope, and zero diff means the migration did not move this
element, never that the element is right.
The boot sequence audit-alignment.mjs already worked out — spawn Electron
with MAKA_E2E_FIXTURE, wait for a CDP page target, evaluate in the renderer
— moves to fixture-cdp.mjs so both scripts share one launcher instead of
growing a second. It also gains what a contract harness needs and an audit
could do without: animations and webfont reflow frozen before capture, an
evaluate deadline so a wedged renderer fails loudly instead of hanging, and
process-group teardown so Electron's helpers do not pile up and starve
later windows.
Also carries #1565's third PR 0 item, salvaging the mega-branch's late
regression fixes. Six of the eight land on properties this snapshot already
records — a wrapped titlebar row, a timestamp stacking above its title, a
missing gap, a column painting over the settings rail all move a rect or a
recorded property, so the diff catches them without bespoke rules. What
bespoke rules do add is noticing when the harness stops watching, so each
one is pinned to an anchor that must keep appearing in the baseline. The
remaining two fix files the mega-branch itself created, which do not exist
on main; they are recorded in the PR body instead.
Determinism was verified by capturing the same build twice. The first
attempt was not clean, and both causes are excluded rather than tuned
around: element labels fell back to el.id, which Base UI regenerates every
render, and OverlayScrollbars' chrome fades with pointer activity, so its
visibility was a coin flip. Baselines stay at 1.5MB rather than 4MB because
properties equal to their initial value, or inherited unchanged from the
parent, are omitted and read back as unchanged.
Migration-only: deliberately outside CI, following check:chat-visual, and
removed in PR 14 with the maka.legacy layer.
#1565 PR 0. The mega-branch's most expensive regressions were controls that
render perfectly and refuse clicks. A computed-style snapshot cannot see
that, so it gets a contract of its own: every visible interactive element on
the five routes must be reachable at its centre, must not sit under a
pointer-events or visibility trap, must not be transparent through an
ancestor, and must not fall inside an -webkit-app-region: drag area.
Three parts of #1565's proposal did not survive contact with a clean main,
and each is replaced by something answering the same question:
The drag-region probe was specified as a real click plus a side effect.
Fixture windows are hidden, and an unmapped window does not route
synthesized input through the browser's hit-testing path, so every target
read as swallowed. Making the window visible to fix that turned the probe
into something that fires real product actions — one opened a native file
dialog and hung the run. Reading -webkit-app-region off the stylesheet rules
instead has no side effects: the property is missing from getComputedStyle,
but not from the rules that declare it.
All five probe points were specified as required. On a clean main that is
unreachable: sibling chrome legitimately covers a couple of edge pixels, so
a workbar resize handle would indict a control nobody struggles to click.
The centre is now required and corners corroborate — three lost corners
means something covers the control.
The overlay guard keyed on [role=dialog][open], which a React-rendered
div[role=dialog] never has, so the settings surface went undetected and
every control behind it was reported unhittable. It now keys on the
product's own data-modal="true".
Geometry probes do need a visible window — elementFromPoint disagrees with
getBoundingClientRect against a hidden one — so this check steals focus for
a few seconds per route. Memoising style and rect lookups took the densest
route from a 60s timeout to 3s, and routes are spaced so one window is fully
gone before the next boots.
All five routes are clean on ae43cb2.
@Astro-Han
Astro-Han marked this pull request as draft July 30, 2026 09:45
Review found the harness could report `ok` for a window it never
launched, could not see the class of change the migration actually
makes, and shipped its only product change with no automated coverage.
Launch through the shared E2E seam. The harness rolled its own Electron
launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a
process-group kill, `...process.env` — all of it already solved next
door in `apps/desktop/e2e/fixtures.ts`, and each hand-rolled version
worse. It never verified the CDP target belonged to the child it
spawned, and the port allocator restarted at 14600 in every process, so
a leftover Electron on that port got captured instead. Reproduced: two
concurrent runs requesting different scenarios both read the same
window. When the leftover is the same route from an older build — the
shape you get by re-running one route after an edit — the contract
reports `ok` for a stale window. `detached: true` with no signal handler
supplied the leftovers; this machine had 270 stale user-data dirs, none
ever removed. `buildE2eEnv` moves to `scripts/fixture-env.mjs`, shared
by both callers; its own comment already described the bug the harness
had, since inheriting the environment leaks `VITE_DEV_SERVER_URL` and
loads the dev server instead of the build under test. Launch, readiness
and teardown now go through Playwright's Electron support: fixed sleeps
become per-route readiness selectors, and teardown removes its temp dir.
235 lines become 139, and the hit-test retry goes with them, per the
sibling harness's rule that flakes should fail loudly.
Stop committing baselines. They encode the capturing host — font metrics
and the macOS traffic-light inset — so one machine's baseline reads as
thousands of diffs on another. Capture on the branch you compare
against, apply the slice, compare: the two captures that matter always
come from the same host. The timezone is pinned through the fixture's
IANA override for the same reason. This removes 70k lines from review
and makes the orphan-baseline check moot.
Record what a cascade flip actually changes. `boxShadow`, `textAlign`,
`flexDirection`, `flexWrap` and `outline` were missing, so a utility
that starts beating a product rule could change elevation or text
placement with an identical rect and be reported clean. Three
`INITIAL_VALUES` entries guessed at values Chromium does not serialise
and never matched once, leaving constants on all 3,964 records;
`findDeadOmissionRules` now fails the run when a rule stops matching —
it caught a fourth such entry immediately. Inherited values were
compared against the parent rather than the nearest recorded ancestor,
so a zero-box wrapper's color could change, repaint every visible child,
and leave the capture byte-identical: 129 masked values on the chat
route alone.
Delete the second drag-region resolver. It parsed stylesheet rules in
source order, ignoring specificity, `!important` and `@layer` — which
PR 1 and PR 2 introduce — on the premise that the property is absent
from computed style. It is not: `getComputedStyle(el).webkitAppRegion`
returns `drag`, and `e2e/window-titlebar.spec.ts` has read it that way
in CI all along, against rendered geometry and document order. The
hit-test keeps what that spec does not cover, and now reports how many
elements it actually probed rather than how many the selector matched.
Pin the product change. `resolveDockPresentation` is a pure function
with tests; the branch it replaced could only be exercised by launching
Electron, which is why it had none. The programmatic smoke checks are
now pinned by the contract test that exists to pin checks — including
the new dock one, which could previously be deleted with CI green — and
`e2e` shares `build:with-deps` instead of repeating the chain verbatim.
…budget
buildFixtureEnv read process.env.CI inline, so the fixture-env test
asserting a hidden run stays hidden passed on every laptop and failed on
the Linux CI runner. The ambient read now lives in isCiLinuxDisplay for
callers to compose explicitly.
audit-alignment gets its 2500ms settle back (the shared-launcher refactor
had silently dropped it to 1000ms against static-markup readiness) plus a
real ready selector per fixture, and its header now states the deliberate
environment change from the raw-spawn launcher it replaced.
withFixtureWindow tore down with a bare app.close(), which has no
deadline — one wedged launch turned a capture run (or the CI alignment
audit, which has no job timeout) into an infinite hang, and the temp
userData dir leaked because rm was sequenced after the close.
The bounded close the E2E suite already owned moves to
scripts/electron-lifecycle.mjs — the same shared-home precedent as
fixture-env.mjs, so a bare-node harness and the Playwright suite stop
splitting one launch concern across two directories. Renderer evaluate
gets a deadline too; teardown then reclaims the process either way.
Two measurement-semantics gaps in the visual capture, both instances of
a class the harness had already been bitten by:
- textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79-85%
of records re-recorded an ancestor's value and one container change
printed as N descendant lines. It moves to the inherited omission (and
out of INITIAL_VALUES — absent must mean exactly one thing). A static
table test now pins every CSS-inherited property to that route, which
is the guard findDeadOmissionRules could never provide.
- Descendants of an opacity:0 element report opacity 1 and a non-zero
rect, so the collapsed session panel put 105 invisible records into
the chat capture; a change under it would diff on pixels that never
paint. The walk now prunes zero-opacity subtrees.
The prune exposed that two salvaged anchors were matching those phantom
records: the chat fixture never shows the session rows at all. They now
point at mcp-hub, whose fixture opens the sidebar for real.
Replaces the two-step gitignored-baseline workflow. The checker now
builds the base ref in a cached temp worktree (node_modules shared;
the compare refuses to run across a package-lock change), builds the
working tree, captures every route x theme x platform from both builds,
and diffs in memory. Every step of the old workflow a human could skip
- rebuild after switching, recapture after rebasing, stash on a
committed slice - produced a zero-diff pass that had measured the same
binary twice; now the instrument owns the whole measurement and there
is no baseline file to trust or to stale.
The matrix gains the win32 column #1565 asked for: the fixture platform
override drives the production app:info -> data-os path, so the per-OS
cascade is captured on any host - no Windows runner involved.
desktop-real-window-smoke was the last launcher still spreading
process.env wholesale: a developer with npm run dev open smoked the dev
server instead of the build the script just made, and the run touched
the real $HOME (#1517's skills-deletion surface). It now launches
through buildFixtureEnv with an explicit visible window.
The dock contract drops its duplicated fourth case and pins the wiring
instead - the regression this module exists for was the call site
re-deriving startHidden, which no pure-function test can observe. The
reveal contract follows fixture-env's pure shape (the ambient CI read
now lives in isCiLinuxDisplay, composed at each call site).
…entity
Round-3 review found the compare's base was not a base: the borrowed
node_modules resolved @maka/* through workspace links straight back to
the working tree, so the base bundled and loaded the candidate's own
packages — a slice changing @maka/core would have compared itself
against itself. The base now runs its own npm ci against its own
lockfile in the cached worktree (atomic lock against concurrent
builds; the marker name encodes the scheme so old symlink-era caches
rebuild). The lockfile-parity gate and the symlink list existed only
to justify the sharing; both are deleted, and a slice may now
legitimately change dependencies.
The same round showed the instrument asserting nothing about what it
measured, so now it proves it: route/theme/platform are closed sets;
every capture waits for the requested data-os and theme on the live
document before reading a style; painted ::before/::after get paint
signatures (47 pseudo rules paint in this renderer, led by the
body::after film grain, and a cascade flip on any of them was
invisible); mixBlendMode joins the property set; and dead omission
rules are counted at the raw sample where the border/outline gates
cannot shadow them.
The smoke gate still had two unbounded exits: --manual waited forever
on 'exit' after one SIGTERM, and the report path fired SIGTERM and
called process.exit without waiting or escalating — plus a failed
report write skipped the kill entirely. All paths now stop through the
shared bounded close (SIGTERM as the graceful phase, 5s grace, SIGKILL
the tree), inside a finally.
electron-lifecycle's import of @maka/runtime now fails with an error
that says what to build; on a fresh clone the raw ERR_MODULE_NOT_FOUND
surfaced from four files deep in the import chain.
The shared teardown's force path kills by process group. The smoke gate
spawned Electron without detached, so the group signal hit ESRCH, the
terminator read that as "tree already gone", and a visible window that
had ignored SIGTERM survived its own SIGKILL stage. Spawn the smoke
child as a group leader like every runtime consumer does, and back the
group kill with a direct kill on the root so a future non-leader child
still dies inside the 2s exit deadline.
filter, backdrop-filter, clip-path and transform move pixels without
moving the rect — grayscale() on disabled provider rows, the glass
theme's backdrop blur, clip-path as visually-hidden, rotate() on square
chevrons — so losing one was invisible to every property the contract
sampled. Capture all four (plus transform-origin, gated on a transform
actually painting), and stop the renderer's clock first: a running
spinner serialises a different matrix at every phase, which would diff
time rather than the cascade. Verified with a negative control (an
injected filter rule fails the compare) and a clean 20/20 run.
Also correct the header, which still described the rejected
node_modules-sharing base build, and compare the direct-run guard
through pathToFileURL so a percent-encoding checkout path cannot turn
the instrument into a silent exit-0 no-op.
--route accepted any value and only failed when nothing matched, so
`--route chat --route chatt` dropped the typo and exited 0 as "1
route(s) clean" — silently shrinking the requested coverage. Validate
each value against the closed route set (as the visual contract already
does), guard the entry point through pathToFileURL, and pin the CLI's
exit codes with subprocess tests.
The ESRCH-swallowing group-kill path was only proven against a live
window; the fake now exercises it too — a terminator that returns
without killing must be followed by a direct SIGKILL on the root.
@Astro-Han
Astro-Han marked this pull request as ready for review July 30, 2026 13:04
@Astro-Han
Astro-Han merged commit 71e47a6 into mainJul 30, 2026
3 checks passed
@Astro-Han
Astro-Han deleted the refactor/1565-pr0-contract-harness branch July 30, 2026 13:04
Sign up for freeto 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

@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' feat(desktop): add the Astryx migration contract harness by Astro-Han · Pull Request #1653 · apache/maka · GitHub
Skip to content

feat(desktop): add the Astryx migration contract harness - #1653

Merged
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness
Jul 30, 2026
Merged

feat(desktop): add the Astryx migration contract harness#1653
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #1565. PR 0 of the Astryx renderer migration: the measurement instrument every later slice depends on. The mega-branch produced regressions on screens nobody edited, and slicing alone cannot localise them — the bisect signal has to come from a record of what the app looked like before.

  • check:visual-contract — one command compares two builds: it checks out the base ref into a cached temp worktree with its own npm ci, builds both sides, captures five representative fixture routes × light/dark × darwin/win32 from each, and diffs every visible element's box, paint, and alignment properties — plus a paint signature for every painted ::before/::after — in memory. Gates no-change, not correctness: a pre-existing bug on the base stays out of scope, and zero diff means "the migration did not move this element", never "this element is right".
  • check:hit-test — interactive elements must be reachable at their centre, free of pointer-events/visibility traps, and opaque through their ancestry; every route reports what it probed and what it skipped (invisible, out-of-scope, disabled, transparent, clipped) so the coverage is a number, not an assumption.
  • launch:fixture — a real, clickable fixture window for manual visual comparison, which a visually-neutral migration cannot do without.
  • Salvages the mega-branch's late regression fixes into the harness (item 3 below).

Both harnesses live in scripts/ as npm scripts and are deliberately outside CI, following the check:chat-visual precedent and #1565's own scoping — a two-build compare costs minutes per run, which is a per-slice gate, not a per-push one. They are migration-only and get deleted in PR 14 with the maka.legacy layer. scripts/fixture-env.mjs, scripts/fixture-window.mjs, and scripts/electron-lifecycle.mjs are the exception: audit-alignment.mjs runs in CI and the E2E suite shares them, so they outlive the harness.

How to use it

node scripts/check-visual-contract.mjs # compare main → working tree
node scripts/check-visual-contract.mjs --against <ref># a different base
node scripts/check-visual-contract.mjs --route chat --theme dark --platform win32

There is no baseline file and no capture step to remember. An earlier version had one, captured by hand on the base branch and compared by hand after switching; every step a human could skip — rebuild after switching, recapture after rebasing — produced a convincing zero-diff pass that had measured the same binary twice. Now the instrument owns the whole measurement: both sides are checked out, installed, built, and captured in the same run, on the same host (font metrics and the macOS traffic-light inset are host facts, so cross-host comparison was never sound anyway).

The base gets its own dependency closure: a fresh worktree of the ref with its own npm ci against its own lockfile, cached keyed by the resolved commit, so iterating on a slice rebuilds only the working tree. Sharing this checkout's node_modules was tried and rejected — the workspace links inside it resolve @maka/* back to the working tree's packages, so the base would have bundled and loaded the candidate's own code. A side effect of the honest install: a slice may legitimately change dependencies. Route/theme/platform arguments are closed sets, and every capture asserts the renderer actually reached the requested data-os and theme before reading a single style — a capture must prove it measured the cascade it claims.

The win32 column runs on any host: MAKA_E2E_FIXTURE_PLATFORM drives the production app:info → data-os path, which is what keys the per-OS CSS. It covers the cascade, not native chrome.

Route mapping

#1565 names product routes; these are the MAKA_E2E_FIXTURE scenarios that actually open them on main. There is no scenario literally called "chat" or "settings-providers".

RouteScenarioReadiness selector
chatturn-narrative.maka-session-workbar
settings/generalsettings-general.settingsRows
settings/providersprovider-workspace.providersPanel
mcp hubmodule-mcp.maka-module-main-header
onboardingfirst-run.maka-onboarding-surface

Known blind spots, stated in the tool's header rather than papered over: display: none subtrees (closed menus, unmounted dialogs) never enter the capture; pseudo-elements are paint signatures without a rect; native top-layer content (dialog.showModal, popover) would escape the opacity-prune, and nothing in the app uses it today.

Where this departs from #1565, and why

Each departure is a measurement, not a preference. The issue body has been corrected where it was factually wrong.

The fixture window was never unclickable.#1565 stated that app.dock.hide() makes fixture runs reject all real clicks. Measured on ae43cb291/darwin, that does not hold: MAKA_E2E_FIXTURE alone means the window is never shown at all (startHiddenkeepHiddenForE2eFixture, main-window.ts:114), and adding MAKA_E2E_SHOW_WINDOW=1 gives isVisible: true, isFocused: true, and fully working clicks — with dock.hide() still running.

scripts/desktop-real-window-smoke.mjs was already broken on main. It boots a real fixture window but never set MAKA_E2E_SHOW_WINDOW, so its own programmatic-window-visible check fails and its twelve human checks are unrunnable against an invisible window. Fixed here, extended with --manual rather than adding a second launcher, and now launched through the shared environment builder — it was the last launcher still inheriting process.env wholesale, which meant a developer with npm run dev open smoked the dev server instead of the build the script had just made.

The one product-code change is about reachability. The dock rule now keys off the same startHidden that decides whether the window shows, instead of re-deriving it from MAKA_E2E_FIXTURE — a re-derivation that had already drifted, since it missed the MAKA_E2E_SHOW_WINDOW escape hatch. An accessory app has no Dock tile and no Cmd+Tab entry, so a reviewer who switches away mid-comparison cannot switch back. Capture runs and CI e2e leave the variable unset and keep their accessory, no-focus-steal behaviour unchanged.

The window-drag question is not asked here.#1565 proposed probing it with a real click; that was tried and abandoned (an unmapped window does not route synthesized input, and a visible one fired real product actions — one opened a native file dialog and hung the run). It is also already answered: e2e/window-titlebar.spec.ts measures it in CI against rendered geometry and the document order Chromium composes drag rects in, and .maka-window-titlebar is the only element allowed to declare drag. The hit-test covers what that spec does not — whether a control anywhere in the window can be reached at all.

Centre is required; corners corroborate. All five probe points cannot pass on a clean main — sibling chrome legitimately covers a couple of edge pixels. Three lost corners means something is actually covering the control; one means the border has a neighbour.

Item 3: salvaged regressions

Six of the eight late "fix cascade / fix click" commits land on properties the snapshot already records. Bespoke rules would be redundant; what they do add is noticing when the harness stops watching, so each is pinned to an anchor that must keep appearing in a route where the element actually paints. That last clause is load-bearing: the session-row anchors originally pointed at the chat route, where the fixture keeps the panel collapsed under opacity: 0 — they "matched" records with no visible pixels until the capture started pruning invisible subtrees, at which point the check failed and exposed its own false assurance. They now point at mcp-hub, whose fixture opens the sidebar.

CommitRegressionAnchorRoute
be5e69584titlebar clusters wrapped onto a second rowmaka-shell-topbar-railchat
fd38a37cecomposer frame jumped on focusmaka-composer-inner (resting state only)chat
9aad59740session timestamp stacked above its titlemaka-list-row-metamcp-hub
3dbd68ca7sidebar nav rows had two styling authoritiesmaka-list-rowmcp-hub
9d20a9396task ledger recent-count collided with its labelmaka-session-workbar-countchat
be1406705settings content column painted over the nav railsettingsSidebarsettings-general

Two are not transcribable: 3e57d1951 (stat tile leading) and 400e8f4a9 (turn marker measure) fix files the mega-branch itself created, which do not exist on main. They are recorded here as fragile spots for whichever slice introduces those components.

What review changed

Four review rounds (Codex plus an independent fresh-context agent each time), every finding reproduced before being fixed. The branch is meaningfully different as a result.

Round 1

It could report ok for a window it never launched. The harness rolled its own launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a process-group kill, ...process.env. It never verified the CDP target belonged to the child it spawned, so a leftover Electron on the port was captured instead; reproduced with two concurrent runs both reading the same window, and 270 stale user-data dirs on the dev machine. Now launched through Playwright's Electron support, with the environment builder extracted to scripts/fixture-env.mjs and shared with the E2E suite.

It could not see the class of change the migration makes.boxShadow, textAlign, flexDirection, flexWrap and outline were not captured — a utility that starts beating a product rule after the @layer move can change elevation or text placement with an identical rect. Three INITIAL_VALUES entries guessed at values Chromium does not serialise and never matched once.

Inherited values were compared against the wrong ancestor. A zero-box or display: contents wrapper is skipped, so nothing in the capture carried its values — yet visible children omitted properties that matched it. Now compared against the nearest recorded ancestor, so every omission is recoverable by a reader of the file.

The drag-region resolver was built on a false premise. It parsed stylesheet rules in source order because "the property is absent from getComputedStyle". It is not: getComputedStyle(el).webkitAppRegion returns drag, and window-titlebar.spec.ts has read it that way in CI all along. Deleted, ~50 lines.

The product change had no automated coverage.resolveDockPresentation is now a pure function with tests, and the programmatic smoke layer is pinned by the source contract.

Round 2

The two-step baseline workflow could pass silently when a step was skipped. The checker never rebuilt the candidate, the baseline was a bare array with no provenance, and the gitignored file could not travel between worktrees — comparing a build to itself read as a clean slice. Replaced by the one-command --against compare; the baseline file, the --update flag, and the .gitignore entry are gone with it.

Teardown was unbounded, and CI inherits this launcher.app.close() has no deadline; one wedged launch turned a capture run — or the CI alignment audit, whose job has no timeout — into an infinite hang, reproduced once at ≥600s. The bounded close the E2E suite already owned (grace, then SIGKILL the tree) moved to scripts/electron-lifecycle.mjs and both consumers share it; renderer evaluate gained a deadline; the temp profile is removed even when close fails.

The shared-launcher refactor had silently changed the CI gate's conditions.audit-alignment's settle budget had dropped 2500ms → 1000ms against static-markup readiness, and buildFixtureEnv read process.env.CI inline — which made the "hidden run stays hidden" test pass on every laptop and fail on the Linux CI runner. The settle budget is restored with real per-fixture ready selectors, the window-visibility decision is a pure function of the builder's arguments, and the one ambient read lives in isCiLinuxDisplay for callers to compose.

Two capture-semantics gaps.textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79–85% of records re-recorded an ancestor's value; and descendants of an opacity: 0 element (own opacity 1, non-zero rect) were captured as visible — 105 phantom records under the collapsed session panel. Fixed, plus a static table test pinning every CSS-inherited property to the inherited route in both directions.

The darwin-only matrix contradicted the issue, and its stated justification was wrong. No Windows runner is needed: the fixture platform override flips data-os through the production path on any host. The matrix now includes win32.

Round 3

The base was not isolated. The base worktree borrowed this checkout's node_modules, and the workspace links inside it resolved @maka/* straight back to the working tree — the base bundled and loaded the candidate's own packages, so a slice changing @maka/core would have compared itself against itself. Both reviewers converged on this independently; reproduced byte-for-byte with require.resolve. The base now runs its own npm ci against its own lockfile (cached per commit, guarded by an atomic build lock), and the lockfile-parity gate plus the symlink list that existed to justify the sharing are deleted.

Pseudo-elements were invisible to the contract. 47 ::before/::after rules paint in this renderer — including the full-screen body::after film-grain overlay — and a cascade flip on any of them changed pixels while every element record stayed byte-identical. Painted pseudo-elements are now captured as paint signatures that inherit from their host.

The instrument now proves its own measurement identity. CLI arguments are closed sets; every capture waits for and asserts the requested data-os and theme on the live document before reading styles — the win32 column can no longer silently degrade into a second darwin column if the override seam regresses. Dead omission rules are counted at the raw sample inside the capture (not inferred from the compressed output), so a rule shadowed by the border/outline paint gates still proves itself.

Remaining teardown edges. The smoke gate's SIGTERM paths now escalate through the shared bounded close, and a failed report write no longer leaves a live Electron behind. Verifying that fix surfaced one more: the shared force path kills by process group, the smoke gate's child was not spawned detached, so the group signal hit ESRCH, the terminator read "tree already gone", and a visible window that ignores SIGTERM survived its own SIGKILL stage. The smoke child is now a group leader like every runtime consumer, and the bounded close backs the group kill with a direct kill on the root.

Round 4

The contract could not see the paint-only channels.filter, backdrop-filter, clip-path and transform move pixels without moving the rect — grayscale() on disabled provider rows, the glass theme's backdrop blur, clip-path as visually-hidden, rotate() on square chevrons — so a migration losing one diffed zero. All four are now sampled (plus transform-origin, gated on a transform actually painting), and the capture freezes animations and transitions before reading a single style: a running spinner serialises a different matrix at every phase, which would diff time rather than the cascade. Verified with a negative control — an injected filter rule fails the compare with filter: ∅ → sepia(0.1) — and a clean 20/20 run.

The hit-test's --route was an open set. It accepted any value and failed only when nothing matched, so --route chat --route chatt dropped the typo and exited 0 as "1 route(s) clean" — silent coverage shrink on a tool whose whole point is that coverage is a number. Each value is now validated against the closed route set (as the visual contract already does), and the CLI's exit codes are pinned by subprocess tests.

Two smaller honesty fixes. Both direct-run guards compare through pathToFileURL — a checkout path with a space or non-ASCII segment percent-encodes in import.meta.url, and the string-concatenation compare would have made the instrument exit 0 having measured nothing. The tool header still described the rejected node_modules-sharing base build; it now states the actual isolation model. The direct-kill backstop from round 3 gained a fake-based test alongside its live-window proof.

Verification

Run on main (ae43cb291) + this branch, macOS arm64.

npm run lint ✓
npm run format:check ✓
npm run typecheck ✓
npx knip ✓ exit 0 (2 config hints, same as main)
npm run test:scripts ✓ 209 pass, 0 fail
npm --workspace @maka/desktop run test:dist ✓ 2981 pass, 0 fail
node scripts/audit-alignment.mjs ✓ 12/12 fixtures clean
node scripts/check-visual-contract.mjs ✓ 20/20 comparisons clean against main
(fresh base: worktree + npm ci + build, all by the tool;
negative control: an injected filter rule fails the compare)
node scripts/check-hit-test.mjs ✓ 5/5 routes clean
npx playwright test first-run window-titlebar electron-lifecycle
✓ 7 passed
npm --workspace @maka/desktop run smoke:programmatic-window
✓ PASS incl. programmatic-dock-visible (was FAIL on main)

The real-window smoke launches the app with MAKA_E2E_FIXTURE, which makes
main.ts compute startHidden and main-window.ts keep the window hidden for
its whole lifecycle. The gate therefore had nothing to look at: on a clean
main its own programmatic-window-visible check reports visible=false and
exits 1, and every human check it prompts for — dragging window edges and
corners to resize, dragging the titlebar to move, Tab/Shift+Tab traversal
inside the modal — is unrunnable against an invisible window.
Set MAKA_E2E_SHOW_WINDOW on the launch env, which is exactly the escape
hatch main.ts already provides for runs where there is no focus to steal.
Capture runs and CI e2e do not set it and keep their hidden-window
behavior unchanged.
Hiding the dock icon makes the run an accessory app. Contrary to what the
migration tracker assumed, such a window is fully clickable — measured on
darwin: with MAKA_E2E_SHOW_WINDOW set, the fixture window reports
isVisible/isFocused true and takes clicks, drags, and keyboard input
normally. What it does not have is a Dock tile or a Cmd+Tab entry, so a
reviewer doing manual visual comparison who switches to another app has no
way back to it.
Key the dock icon off the same startHidden that decides whether the window
shows at all, instead of re-deriving the condition from MAKA_E2E_FIXTURE.
The two can no longer drift, and MAKA_E2E_SHOW_WINDOW now opts out of both
halves of staying out of sight. Capture runs and CI e2e leave the variable
unset, so they keep hiding the window and the dock exactly as before.
Lock it with a dock diagnostic and a programmatic-dock-visible check in the
real-window smoke, which reported dockVisible=false before the change and
true after.
A migration that must stay visually neutral needs to look at the running
app constantly, but the only way to boot a route with fixture data was the
smoke gate, which insists on walking a reviewer through twelve checklist
prompts and filing a report every time.
Add --manual: same launch path, then stop. The window stays up until it is
closed or interrupted. Reusing the gate's launcher keeps the window a
reviewer inspects identical to the window the gate checks, and leaves one
place that knows how to boot a fixture instance.
Expose it as `launch:fixture`, and lift the build chain the smoke scripts
repeated verbatim into `build:with-deps` rather than copying it a third
time.
#1565 PR 0. The mega-branch produced regressions on screens nobody edited,
and slicing alone does not localise them: the signal has to come from a
baseline that says what the app looked like before. This captures one.
check:visual-contract walks the five representative fixture routes in light
and dark, records every visible element's box, paint, and alignment
properties, and diffs the result against committed JSON. The alignment set
(align-items/justify-content/place-items/gap/grid-area) is there because
flex and grid misalignment is exactly the class a paint-only property set
cannot see. It gates no-change, not correctness: a pre-existing bug on main
stays out of scope, and zero diff means the migration did not move this
element, never that the element is right.
The boot sequence audit-alignment.mjs already worked out — spawn Electron
with MAKA_E2E_FIXTURE, wait for a CDP page target, evaluate in the renderer
— moves to fixture-cdp.mjs so both scripts share one launcher instead of
growing a second. It also gains what a contract harness needs and an audit
could do without: animations and webfont reflow frozen before capture, an
evaluate deadline so a wedged renderer fails loudly instead of hanging, and
process-group teardown so Electron's helpers do not pile up and starve
later windows.
Also carries #1565's third PR 0 item, salvaging the mega-branch's late
regression fixes. Six of the eight land on properties this snapshot already
records — a wrapped titlebar row, a timestamp stacking above its title, a
missing gap, a column painting over the settings rail all move a rect or a
recorded property, so the diff catches them without bespoke rules. What
bespoke rules do add is noticing when the harness stops watching, so each
one is pinned to an anchor that must keep appearing in the baseline. The
remaining two fix files the mega-branch itself created, which do not exist
on main; they are recorded in the PR body instead.
Determinism was verified by capturing the same build twice. The first
attempt was not clean, and both causes are excluded rather than tuned
around: element labels fell back to el.id, which Base UI regenerates every
render, and OverlayScrollbars' chrome fades with pointer activity, so its
visibility was a coin flip. Baselines stay at 1.5MB rather than 4MB because
properties equal to their initial value, or inherited unchanged from the
parent, are omitted and read back as unchanged.
Migration-only: deliberately outside CI, following check:chat-visual, and
removed in PR 14 with the maka.legacy layer.
#1565 PR 0. The mega-branch's most expensive regressions were controls that
render perfectly and refuse clicks. A computed-style snapshot cannot see
that, so it gets a contract of its own: every visible interactive element on
the five routes must be reachable at its centre, must not sit under a
pointer-events or visibility trap, must not be transparent through an
ancestor, and must not fall inside an -webkit-app-region: drag area.
Three parts of #1565's proposal did not survive contact with a clean main,
and each is replaced by something answering the same question:
The drag-region probe was specified as a real click plus a side effect.
Fixture windows are hidden, and an unmapped window does not route
synthesized input through the browser's hit-testing path, so every target
read as swallowed. Making the window visible to fix that turned the probe
into something that fires real product actions — one opened a native file
dialog and hung the run. Reading -webkit-app-region off the stylesheet rules
instead has no side effects: the property is missing from getComputedStyle,
but not from the rules that declare it.
All five probe points were specified as required. On a clean main that is
unreachable: sibling chrome legitimately covers a couple of edge pixels, so
a workbar resize handle would indict a control nobody struggles to click.
The centre is now required and corners corroborate — three lost corners
means something covers the control.
The overlay guard keyed on [role=dialog][open], which a React-rendered
div[role=dialog] never has, so the settings surface went undetected and
every control behind it was reported unhittable. It now keys on the
product's own data-modal="true".
Geometry probes do need a visible window — elementFromPoint disagrees with
getBoundingClientRect against a hidden one — so this check steals focus for
a few seconds per route. Memoising style and rect lookups took the densest
route from a 60s timeout to 3s, and routes are spaced so one window is fully
gone before the next boots.
All five routes are clean on ae43cb2.
@Astro-Han
Astro-Han marked this pull request as draft July 30, 2026 09:45
Review found the harness could report `ok` for a window it never
launched, could not see the class of change the migration actually
makes, and shipped its only product change with no automated coverage.
Launch through the shared E2E seam. The harness rolled its own Electron
launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a
process-group kill, `...process.env` — all of it already solved next
door in `apps/desktop/e2e/fixtures.ts`, and each hand-rolled version
worse. It never verified the CDP target belonged to the child it
spawned, and the port allocator restarted at 14600 in every process, so
a leftover Electron on that port got captured instead. Reproduced: two
concurrent runs requesting different scenarios both read the same
window. When the leftover is the same route from an older build — the
shape you get by re-running one route after an edit — the contract
reports `ok` for a stale window. `detached: true` with no signal handler
supplied the leftovers; this machine had 270 stale user-data dirs, none
ever removed. `buildE2eEnv` moves to `scripts/fixture-env.mjs`, shared
by both callers; its own comment already described the bug the harness
had, since inheriting the environment leaks `VITE_DEV_SERVER_URL` and
loads the dev server instead of the build under test. Launch, readiness
and teardown now go through Playwright's Electron support: fixed sleeps
become per-route readiness selectors, and teardown removes its temp dir.
235 lines become 139, and the hit-test retry goes with them, per the
sibling harness's rule that flakes should fail loudly.
Stop committing baselines. They encode the capturing host — font metrics
and the macOS traffic-light inset — so one machine's baseline reads as
thousands of diffs on another. Capture on the branch you compare
against, apply the slice, compare: the two captures that matter always
come from the same host. The timezone is pinned through the fixture's
IANA override for the same reason. This removes 70k lines from review
and makes the orphan-baseline check moot.
Record what a cascade flip actually changes. `boxShadow`, `textAlign`,
`flexDirection`, `flexWrap` and `outline` were missing, so a utility
that starts beating a product rule could change elevation or text
placement with an identical rect and be reported clean. Three
`INITIAL_VALUES` entries guessed at values Chromium does not serialise
and never matched once, leaving constants on all 3,964 records;
`findDeadOmissionRules` now fails the run when a rule stops matching —
it caught a fourth such entry immediately. Inherited values were
compared against the parent rather than the nearest recorded ancestor,
so a zero-box wrapper's color could change, repaint every visible child,
and leave the capture byte-identical: 129 masked values on the chat
route alone.
Delete the second drag-region resolver. It parsed stylesheet rules in
source order, ignoring specificity, `!important` and `@layer` — which
PR 1 and PR 2 introduce — on the premise that the property is absent
from computed style. It is not: `getComputedStyle(el).webkitAppRegion`
returns `drag`, and `e2e/window-titlebar.spec.ts` has read it that way
in CI all along, against rendered geometry and document order. The
hit-test keeps what that spec does not cover, and now reports how many
elements it actually probed rather than how many the selector matched.
Pin the product change. `resolveDockPresentation` is a pure function
with tests; the branch it replaced could only be exercised by launching
Electron, which is why it had none. The programmatic smoke checks are
now pinned by the contract test that exists to pin checks — including
the new dock one, which could previously be deleted with CI green — and
`e2e` shares `build:with-deps` instead of repeating the chain verbatim.
…budget
buildFixtureEnv read process.env.CI inline, so the fixture-env test
asserting a hidden run stays hidden passed on every laptop and failed on
the Linux CI runner. The ambient read now lives in isCiLinuxDisplay for
callers to compose explicitly.
audit-alignment gets its 2500ms settle back (the shared-launcher refactor
had silently dropped it to 1000ms against static-markup readiness) plus a
real ready selector per fixture, and its header now states the deliberate
environment change from the raw-spawn launcher it replaced.
withFixtureWindow tore down with a bare app.close(), which has no
deadline — one wedged launch turned a capture run (or the CI alignment
audit, which has no job timeout) into an infinite hang, and the temp
userData dir leaked because rm was sequenced after the close.
The bounded close the E2E suite already owned moves to
scripts/electron-lifecycle.mjs — the same shared-home precedent as
fixture-env.mjs, so a bare-node harness and the Playwright suite stop
splitting one launch concern across two directories. Renderer evaluate
gets a deadline too; teardown then reclaims the process either way.
Two measurement-semantics gaps in the visual capture, both instances of
a class the harness had already been bitten by:
- textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79-85%
of records re-recorded an ancestor's value and one container change
printed as N descendant lines. It moves to the inherited omission (and
out of INITIAL_VALUES — absent must mean exactly one thing). A static
table test now pins every CSS-inherited property to that route, which
is the guard findDeadOmissionRules could never provide.
- Descendants of an opacity:0 element report opacity 1 and a non-zero
rect, so the collapsed session panel put 105 invisible records into
the chat capture; a change under it would diff on pixels that never
paint. The walk now prunes zero-opacity subtrees.
The prune exposed that two salvaged anchors were matching those phantom
records: the chat fixture never shows the session rows at all. They now
point at mcp-hub, whose fixture opens the sidebar for real.
Replaces the two-step gitignored-baseline workflow. The checker now
builds the base ref in a cached temp worktree (node_modules shared;
the compare refuses to run across a package-lock change), builds the
working tree, captures every route x theme x platform from both builds,
and diffs in memory. Every step of the old workflow a human could skip
- rebuild after switching, recapture after rebasing, stash on a
committed slice - produced a zero-diff pass that had measured the same
binary twice; now the instrument owns the whole measurement and there
is no baseline file to trust or to stale.
The matrix gains the win32 column #1565 asked for: the fixture platform
override drives the production app:info -> data-os path, so the per-OS
cascade is captured on any host - no Windows runner involved.
desktop-real-window-smoke was the last launcher still spreading
process.env wholesale: a developer with npm run dev open smoked the dev
server instead of the build the script just made, and the run touched
the real $HOME (#1517's skills-deletion surface). It now launches
through buildFixtureEnv with an explicit visible window.
The dock contract drops its duplicated fourth case and pins the wiring
instead - the regression this module exists for was the call site
re-deriving startHidden, which no pure-function test can observe. The
reveal contract follows fixture-env's pure shape (the ambient CI read
now lives in isCiLinuxDisplay, composed at each call site).
…entity
Round-3 review found the compare's base was not a base: the borrowed
node_modules resolved @maka/* through workspace links straight back to
the working tree, so the base bundled and loaded the candidate's own
packages — a slice changing @maka/core would have compared itself
against itself. The base now runs its own npm ci against its own
lockfile in the cached worktree (atomic lock against concurrent
builds; the marker name encodes the scheme so old symlink-era caches
rebuild). The lockfile-parity gate and the symlink list existed only
to justify the sharing; both are deleted, and a slice may now
legitimately change dependencies.
The same round showed the instrument asserting nothing about what it
measured, so now it proves it: route/theme/platform are closed sets;
every capture waits for the requested data-os and theme on the live
document before reading a style; painted ::before/::after get paint
signatures (47 pseudo rules paint in this renderer, led by the
body::after film grain, and a cascade flip on any of them was
invisible); mixBlendMode joins the property set; and dead omission
rules are counted at the raw sample where the border/outline gates
cannot shadow them.
The smoke gate still had two unbounded exits: --manual waited forever
on 'exit' after one SIGTERM, and the report path fired SIGTERM and
called process.exit without waiting or escalating — plus a failed
report write skipped the kill entirely. All paths now stop through the
shared bounded close (SIGTERM as the graceful phase, 5s grace, SIGKILL
the tree), inside a finally.
electron-lifecycle's import of @maka/runtime now fails with an error
that says what to build; on a fresh clone the raw ERR_MODULE_NOT_FOUND
surfaced from four files deep in the import chain.
The shared teardown's force path kills by process group. The smoke gate
spawned Electron without detached, so the group signal hit ESRCH, the
terminator read that as "tree already gone", and a visible window that
had ignored SIGTERM survived its own SIGKILL stage. Spawn the smoke
child as a group leader like every runtime consumer does, and back the
group kill with a direct kill on the root so a future non-leader child
still dies inside the 2s exit deadline.
filter, backdrop-filter, clip-path and transform move pixels without
moving the rect — grayscale() on disabled provider rows, the glass
theme's backdrop blur, clip-path as visually-hidden, rotate() on square
chevrons — so losing one was invisible to every property the contract
sampled. Capture all four (plus transform-origin, gated on a transform
actually painting), and stop the renderer's clock first: a running
spinner serialises a different matrix at every phase, which would diff
time rather than the cascade. Verified with a negative control (an
injected filter rule fails the compare) and a clean 20/20 run.
Also correct the header, which still described the rejected
node_modules-sharing base build, and compare the direct-run guard
through pathToFileURL so a percent-encoding checkout path cannot turn
the instrument into a silent exit-0 no-op.
--route accepted any value and only failed when nothing matched, so
`--route chat --route chatt` dropped the typo and exited 0 as "1
route(s) clean" — silently shrinking the requested coverage. Validate
each value against the closed route set (as the visual contract already
does), guard the entry point through pathToFileURL, and pin the CLI's
exit codes with subprocess tests.
The ESRCH-swallowing group-kill path was only proven against a live
window; the fake now exercises it too — a terminator that returns
without killing must be followed by a direct SIGKILL on the root.
@Astro-Han
Astro-Han marked this pull request as ready for review July 30, 2026 13:04
@Astro-Han
Astro-Han merged commit 71e47a6 into mainJul 30, 2026
3 checks passed
@Astro-Han
Astro-Han deleted the refactor/1565-pr0-contract-harness branch July 30, 2026 13:04
Sign up for freeto 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

@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(desktop): add the Astryx migration contract harness by Astro-Han · Pull Request #1653 · apache/maka · GitHub
Skip to content

feat(desktop): add the Astryx migration contract harness - #1653

Merged
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness
Jul 30, 2026
Merged

feat(desktop): add the Astryx migration contract harness#1653
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #1565. PR 0 of the Astryx renderer migration: the measurement instrument every later slice depends on. The mega-branch produced regressions on screens nobody edited, and slicing alone cannot localise them — the bisect signal has to come from a record of what the app looked like before.

  • check:visual-contract — one command compares two builds: it checks out the base ref into a cached temp worktree with its own npm ci, builds both sides, captures five representative fixture routes × light/dark × darwin/win32 from each, and diffs every visible element's box, paint, and alignment properties — plus a paint signature for every painted ::before/::after — in memory. Gates no-change, not correctness: a pre-existing bug on the base stays out of scope, and zero diff means "the migration did not move this element", never "this element is right".
  • check:hit-test — interactive elements must be reachable at their centre, free of pointer-events/visibility traps, and opaque through their ancestry; every route reports what it probed and what it skipped (invisible, out-of-scope, disabled, transparent, clipped) so the coverage is a number, not an assumption.
  • launch:fixture — a real, clickable fixture window for manual visual comparison, which a visually-neutral migration cannot do without.
  • Salvages the mega-branch's late regression fixes into the harness (item 3 below).

Both harnesses live in scripts/ as npm scripts and are deliberately outside CI, following the check:chat-visual precedent and #1565's own scoping — a two-build compare costs minutes per run, which is a per-slice gate, not a per-push one. They are migration-only and get deleted in PR 14 with the maka.legacy layer. scripts/fixture-env.mjs, scripts/fixture-window.mjs, and scripts/electron-lifecycle.mjs are the exception: audit-alignment.mjs runs in CI and the E2E suite shares them, so they outlive the harness.

How to use it

node scripts/check-visual-contract.mjs # compare main → working tree
node scripts/check-visual-contract.mjs --against <ref># a different base
node scripts/check-visual-contract.mjs --route chat --theme dark --platform win32

There is no baseline file and no capture step to remember. An earlier version had one, captured by hand on the base branch and compared by hand after switching; every step a human could skip — rebuild after switching, recapture after rebasing — produced a convincing zero-diff pass that had measured the same binary twice. Now the instrument owns the whole measurement: both sides are checked out, installed, built, and captured in the same run, on the same host (font metrics and the macOS traffic-light inset are host facts, so cross-host comparison was never sound anyway).

The base gets its own dependency closure: a fresh worktree of the ref with its own npm ci against its own lockfile, cached keyed by the resolved commit, so iterating on a slice rebuilds only the working tree. Sharing this checkout's node_modules was tried and rejected — the workspace links inside it resolve @maka/* back to the working tree's packages, so the base would have bundled and loaded the candidate's own code. A side effect of the honest install: a slice may legitimately change dependencies. Route/theme/platform arguments are closed sets, and every capture asserts the renderer actually reached the requested data-os and theme before reading a single style — a capture must prove it measured the cascade it claims.

The win32 column runs on any host: MAKA_E2E_FIXTURE_PLATFORM drives the production app:info → data-os path, which is what keys the per-OS CSS. It covers the cascade, not native chrome.

Route mapping

#1565 names product routes; these are the MAKA_E2E_FIXTURE scenarios that actually open them on main. There is no scenario literally called "chat" or "settings-providers".

RouteScenarioReadiness selector
chatturn-narrative.maka-session-workbar
settings/generalsettings-general.settingsRows
settings/providersprovider-workspace.providersPanel
mcp hubmodule-mcp.maka-module-main-header
onboardingfirst-run.maka-onboarding-surface

Known blind spots, stated in the tool's header rather than papered over: display: none subtrees (closed menus, unmounted dialogs) never enter the capture; pseudo-elements are paint signatures without a rect; native top-layer content (dialog.showModal, popover) would escape the opacity-prune, and nothing in the app uses it today.

Where this departs from #1565, and why

Each departure is a measurement, not a preference. The issue body has been corrected where it was factually wrong.

The fixture window was never unclickable.#1565 stated that app.dock.hide() makes fixture runs reject all real clicks. Measured on ae43cb291/darwin, that does not hold: MAKA_E2E_FIXTURE alone means the window is never shown at all (startHiddenkeepHiddenForE2eFixture, main-window.ts:114), and adding MAKA_E2E_SHOW_WINDOW=1 gives isVisible: true, isFocused: true, and fully working clicks — with dock.hide() still running.

scripts/desktop-real-window-smoke.mjs was already broken on main. It boots a real fixture window but never set MAKA_E2E_SHOW_WINDOW, so its own programmatic-window-visible check fails and its twelve human checks are unrunnable against an invisible window. Fixed here, extended with --manual rather than adding a second launcher, and now launched through the shared environment builder — it was the last launcher still inheriting process.env wholesale, which meant a developer with npm run dev open smoked the dev server instead of the build the script had just made.

The one product-code change is about reachability. The dock rule now keys off the same startHidden that decides whether the window shows, instead of re-deriving it from MAKA_E2E_FIXTURE — a re-derivation that had already drifted, since it missed the MAKA_E2E_SHOW_WINDOW escape hatch. An accessory app has no Dock tile and no Cmd+Tab entry, so a reviewer who switches away mid-comparison cannot switch back. Capture runs and CI e2e leave the variable unset and keep their accessory, no-focus-steal behaviour unchanged.

The window-drag question is not asked here.#1565 proposed probing it with a real click; that was tried and abandoned (an unmapped window does not route synthesized input, and a visible one fired real product actions — one opened a native file dialog and hung the run). It is also already answered: e2e/window-titlebar.spec.ts measures it in CI against rendered geometry and the document order Chromium composes drag rects in, and .maka-window-titlebar is the only element allowed to declare drag. The hit-test covers what that spec does not — whether a control anywhere in the window can be reached at all.

Centre is required; corners corroborate. All five probe points cannot pass on a clean main — sibling chrome legitimately covers a couple of edge pixels. Three lost corners means something is actually covering the control; one means the border has a neighbour.

Item 3: salvaged regressions

Six of the eight late "fix cascade / fix click" commits land on properties the snapshot already records. Bespoke rules would be redundant; what they do add is noticing when the harness stops watching, so each is pinned to an anchor that must keep appearing in a route where the element actually paints. That last clause is load-bearing: the session-row anchors originally pointed at the chat route, where the fixture keeps the panel collapsed under opacity: 0 — they "matched" records with no visible pixels until the capture started pruning invisible subtrees, at which point the check failed and exposed its own false assurance. They now point at mcp-hub, whose fixture opens the sidebar.

CommitRegressionAnchorRoute
be5e69584titlebar clusters wrapped onto a second rowmaka-shell-topbar-railchat
fd38a37cecomposer frame jumped on focusmaka-composer-inner (resting state only)chat
9aad59740session timestamp stacked above its titlemaka-list-row-metamcp-hub
3dbd68ca7sidebar nav rows had two styling authoritiesmaka-list-rowmcp-hub
9d20a9396task ledger recent-count collided with its labelmaka-session-workbar-countchat
be1406705settings content column painted over the nav railsettingsSidebarsettings-general

Two are not transcribable: 3e57d1951 (stat tile leading) and 400e8f4a9 (turn marker measure) fix files the mega-branch itself created, which do not exist on main. They are recorded here as fragile spots for whichever slice introduces those components.

What review changed

Four review rounds (Codex plus an independent fresh-context agent each time), every finding reproduced before being fixed. The branch is meaningfully different as a result.

Round 1

It could report ok for a window it never launched. The harness rolled its own launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a process-group kill, ...process.env. It never verified the CDP target belonged to the child it spawned, so a leftover Electron on the port was captured instead; reproduced with two concurrent runs both reading the same window, and 270 stale user-data dirs on the dev machine. Now launched through Playwright's Electron support, with the environment builder extracted to scripts/fixture-env.mjs and shared with the E2E suite.

It could not see the class of change the migration makes.boxShadow, textAlign, flexDirection, flexWrap and outline were not captured — a utility that starts beating a product rule after the @layer move can change elevation or text placement with an identical rect. Three INITIAL_VALUES entries guessed at values Chromium does not serialise and never matched once.

Inherited values were compared against the wrong ancestor. A zero-box or display: contents wrapper is skipped, so nothing in the capture carried its values — yet visible children omitted properties that matched it. Now compared against the nearest recorded ancestor, so every omission is recoverable by a reader of the file.

The drag-region resolver was built on a false premise. It parsed stylesheet rules in source order because "the property is absent from getComputedStyle". It is not: getComputedStyle(el).webkitAppRegion returns drag, and window-titlebar.spec.ts has read it that way in CI all along. Deleted, ~50 lines.

The product change had no automated coverage.resolveDockPresentation is now a pure function with tests, and the programmatic smoke layer is pinned by the source contract.

Round 2

The two-step baseline workflow could pass silently when a step was skipped. The checker never rebuilt the candidate, the baseline was a bare array with no provenance, and the gitignored file could not travel between worktrees — comparing a build to itself read as a clean slice. Replaced by the one-command --against compare; the baseline file, the --update flag, and the .gitignore entry are gone with it.

Teardown was unbounded, and CI inherits this launcher.app.close() has no deadline; one wedged launch turned a capture run — or the CI alignment audit, whose job has no timeout — into an infinite hang, reproduced once at ≥600s. The bounded close the E2E suite already owned (grace, then SIGKILL the tree) moved to scripts/electron-lifecycle.mjs and both consumers share it; renderer evaluate gained a deadline; the temp profile is removed even when close fails.

The shared-launcher refactor had silently changed the CI gate's conditions.audit-alignment's settle budget had dropped 2500ms → 1000ms against static-markup readiness, and buildFixtureEnv read process.env.CI inline — which made the "hidden run stays hidden" test pass on every laptop and fail on the Linux CI runner. The settle budget is restored with real per-fixture ready selectors, the window-visibility decision is a pure function of the builder's arguments, and the one ambient read lives in isCiLinuxDisplay for callers to compose.

Two capture-semantics gaps.textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79–85% of records re-recorded an ancestor's value; and descendants of an opacity: 0 element (own opacity 1, non-zero rect) were captured as visible — 105 phantom records under the collapsed session panel. Fixed, plus a static table test pinning every CSS-inherited property to the inherited route in both directions.

The darwin-only matrix contradicted the issue, and its stated justification was wrong. No Windows runner is needed: the fixture platform override flips data-os through the production path on any host. The matrix now includes win32.

Round 3

The base was not isolated. The base worktree borrowed this checkout's node_modules, and the workspace links inside it resolved @maka/* straight back to the working tree — the base bundled and loaded the candidate's own packages, so a slice changing @maka/core would have compared itself against itself. Both reviewers converged on this independently; reproduced byte-for-byte with require.resolve. The base now runs its own npm ci against its own lockfile (cached per commit, guarded by an atomic build lock), and the lockfile-parity gate plus the symlink list that existed to justify the sharing are deleted.

Pseudo-elements were invisible to the contract. 47 ::before/::after rules paint in this renderer — including the full-screen body::after film-grain overlay — and a cascade flip on any of them changed pixels while every element record stayed byte-identical. Painted pseudo-elements are now captured as paint signatures that inherit from their host.

The instrument now proves its own measurement identity. CLI arguments are closed sets; every capture waits for and asserts the requested data-os and theme on the live document before reading styles — the win32 column can no longer silently degrade into a second darwin column if the override seam regresses. Dead omission rules are counted at the raw sample inside the capture (not inferred from the compressed output), so a rule shadowed by the border/outline paint gates still proves itself.

Remaining teardown edges. The smoke gate's SIGTERM paths now escalate through the shared bounded close, and a failed report write no longer leaves a live Electron behind. Verifying that fix surfaced one more: the shared force path kills by process group, the smoke gate's child was not spawned detached, so the group signal hit ESRCH, the terminator read "tree already gone", and a visible window that ignores SIGTERM survived its own SIGKILL stage. The smoke child is now a group leader like every runtime consumer, and the bounded close backs the group kill with a direct kill on the root.

Round 4

The contract could not see the paint-only channels.filter, backdrop-filter, clip-path and transform move pixels without moving the rect — grayscale() on disabled provider rows, the glass theme's backdrop blur, clip-path as visually-hidden, rotate() on square chevrons — so a migration losing one diffed zero. All four are now sampled (plus transform-origin, gated on a transform actually painting), and the capture freezes animations and transitions before reading a single style: a running spinner serialises a different matrix at every phase, which would diff time rather than the cascade. Verified with a negative control — an injected filter rule fails the compare with filter: ∅ → sepia(0.1) — and a clean 20/20 run.

The hit-test's --route was an open set. It accepted any value and failed only when nothing matched, so --route chat --route chatt dropped the typo and exited 0 as "1 route(s) clean" — silent coverage shrink on a tool whose whole point is that coverage is a number. Each value is now validated against the closed route set (as the visual contract already does), and the CLI's exit codes are pinned by subprocess tests.

Two smaller honesty fixes. Both direct-run guards compare through pathToFileURL — a checkout path with a space or non-ASCII segment percent-encodes in import.meta.url, and the string-concatenation compare would have made the instrument exit 0 having measured nothing. The tool header still described the rejected node_modules-sharing base build; it now states the actual isolation model. The direct-kill backstop from round 3 gained a fake-based test alongside its live-window proof.

Verification

Run on main (ae43cb291) + this branch, macOS arm64.

npm run lint ✓
npm run format:check ✓
npm run typecheck ✓
npx knip ✓ exit 0 (2 config hints, same as main)
npm run test:scripts ✓ 209 pass, 0 fail
npm --workspace @maka/desktop run test:dist ✓ 2981 pass, 0 fail
node scripts/audit-alignment.mjs ✓ 12/12 fixtures clean
node scripts/check-visual-contract.mjs ✓ 20/20 comparisons clean against main
(fresh base: worktree + npm ci + build, all by the tool;
negative control: an injected filter rule fails the compare)
node scripts/check-hit-test.mjs ✓ 5/5 routes clean
npx playwright test first-run window-titlebar electron-lifecycle
✓ 7 passed
npm --workspace @maka/desktop run smoke:programmatic-window
✓ PASS incl. programmatic-dock-visible (was FAIL on main)

The real-window smoke launches the app with MAKA_E2E_FIXTURE, which makes
main.ts compute startHidden and main-window.ts keep the window hidden for
its whole lifecycle. The gate therefore had nothing to look at: on a clean
main its own programmatic-window-visible check reports visible=false and
exits 1, and every human check it prompts for — dragging window edges and
corners to resize, dragging the titlebar to move, Tab/Shift+Tab traversal
inside the modal — is unrunnable against an invisible window.
Set MAKA_E2E_SHOW_WINDOW on the launch env, which is exactly the escape
hatch main.ts already provides for runs where there is no focus to steal.
Capture runs and CI e2e do not set it and keep their hidden-window
behavior unchanged.
Hiding the dock icon makes the run an accessory app. Contrary to what the
migration tracker assumed, such a window is fully clickable — measured on
darwin: with MAKA_E2E_SHOW_WINDOW set, the fixture window reports
isVisible/isFocused true and takes clicks, drags, and keyboard input
normally. What it does not have is a Dock tile or a Cmd+Tab entry, so a
reviewer doing manual visual comparison who switches to another app has no
way back to it.
Key the dock icon off the same startHidden that decides whether the window
shows at all, instead of re-deriving the condition from MAKA_E2E_FIXTURE.
The two can no longer drift, and MAKA_E2E_SHOW_WINDOW now opts out of both
halves of staying out of sight. Capture runs and CI e2e leave the variable
unset, so they keep hiding the window and the dock exactly as before.
Lock it with a dock diagnostic and a programmatic-dock-visible check in the
real-window smoke, which reported dockVisible=false before the change and
true after.
A migration that must stay visually neutral needs to look at the running
app constantly, but the only way to boot a route with fixture data was the
smoke gate, which insists on walking a reviewer through twelve checklist
prompts and filing a report every time.
Add --manual: same launch path, then stop. The window stays up until it is
closed or interrupted. Reusing the gate's launcher keeps the window a
reviewer inspects identical to the window the gate checks, and leaves one
place that knows how to boot a fixture instance.
Expose it as `launch:fixture`, and lift the build chain the smoke scripts
repeated verbatim into `build:with-deps` rather than copying it a third
time.
#1565 PR 0. The mega-branch produced regressions on screens nobody edited,
and slicing alone does not localise them: the signal has to come from a
baseline that says what the app looked like before. This captures one.
check:visual-contract walks the five representative fixture routes in light
and dark, records every visible element's box, paint, and alignment
properties, and diffs the result against committed JSON. The alignment set
(align-items/justify-content/place-items/gap/grid-area) is there because
flex and grid misalignment is exactly the class a paint-only property set
cannot see. It gates no-change, not correctness: a pre-existing bug on main
stays out of scope, and zero diff means the migration did not move this
element, never that the element is right.
The boot sequence audit-alignment.mjs already worked out — spawn Electron
with MAKA_E2E_FIXTURE, wait for a CDP page target, evaluate in the renderer
— moves to fixture-cdp.mjs so both scripts share one launcher instead of
growing a second. It also gains what a contract harness needs and an audit
could do without: animations and webfont reflow frozen before capture, an
evaluate deadline so a wedged renderer fails loudly instead of hanging, and
process-group teardown so Electron's helpers do not pile up and starve
later windows.
Also carries #1565's third PR 0 item, salvaging the mega-branch's late
regression fixes. Six of the eight land on properties this snapshot already
records — a wrapped titlebar row, a timestamp stacking above its title, a
missing gap, a column painting over the settings rail all move a rect or a
recorded property, so the diff catches them without bespoke rules. What
bespoke rules do add is noticing when the harness stops watching, so each
one is pinned to an anchor that must keep appearing in the baseline. The
remaining two fix files the mega-branch itself created, which do not exist
on main; they are recorded in the PR body instead.
Determinism was verified by capturing the same build twice. The first
attempt was not clean, and both causes are excluded rather than tuned
around: element labels fell back to el.id, which Base UI regenerates every
render, and OverlayScrollbars' chrome fades with pointer activity, so its
visibility was a coin flip. Baselines stay at 1.5MB rather than 4MB because
properties equal to their initial value, or inherited unchanged from the
parent, are omitted and read back as unchanged.
Migration-only: deliberately outside CI, following check:chat-visual, and
removed in PR 14 with the maka.legacy layer.
#1565 PR 0. The mega-branch's most expensive regressions were controls that
render perfectly and refuse clicks. A computed-style snapshot cannot see
that, so it gets a contract of its own: every visible interactive element on
the five routes must be reachable at its centre, must not sit under a
pointer-events or visibility trap, must not be transparent through an
ancestor, and must not fall inside an -webkit-app-region: drag area.
Three parts of #1565's proposal did not survive contact with a clean main,
and each is replaced by something answering the same question:
The drag-region probe was specified as a real click plus a side effect.
Fixture windows are hidden, and an unmapped window does not route
synthesized input through the browser's hit-testing path, so every target
read as swallowed. Making the window visible to fix that turned the probe
into something that fires real product actions — one opened a native file
dialog and hung the run. Reading -webkit-app-region off the stylesheet rules
instead has no side effects: the property is missing from getComputedStyle,
but not from the rules that declare it.
All five probe points were specified as required. On a clean main that is
unreachable: sibling chrome legitimately covers a couple of edge pixels, so
a workbar resize handle would indict a control nobody struggles to click.
The centre is now required and corners corroborate — three lost corners
means something covers the control.
The overlay guard keyed on [role=dialog][open], which a React-rendered
div[role=dialog] never has, so the settings surface went undetected and
every control behind it was reported unhittable. It now keys on the
product's own data-modal="true".
Geometry probes do need a visible window — elementFromPoint disagrees with
getBoundingClientRect against a hidden one — so this check steals focus for
a few seconds per route. Memoising style and rect lookups took the densest
route from a 60s timeout to 3s, and routes are spaced so one window is fully
gone before the next boots.
All five routes are clean on ae43cb2.
@Astro-Han
Astro-Han marked this pull request as draft July 30, 2026 09:45
Review found the harness could report `ok` for a window it never
launched, could not see the class of change the migration actually
makes, and shipped its only product change with no automated coverage.
Launch through the shared E2E seam. The harness rolled its own Electron
launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a
process-group kill, `...process.env` — all of it already solved next
door in `apps/desktop/e2e/fixtures.ts`, and each hand-rolled version
worse. It never verified the CDP target belonged to the child it
spawned, and the port allocator restarted at 14600 in every process, so
a leftover Electron on that port got captured instead. Reproduced: two
concurrent runs requesting different scenarios both read the same
window. When the leftover is the same route from an older build — the
shape you get by re-running one route after an edit — the contract
reports `ok` for a stale window. `detached: true` with no signal handler
supplied the leftovers; this machine had 270 stale user-data dirs, none
ever removed. `buildE2eEnv` moves to `scripts/fixture-env.mjs`, shared
by both callers; its own comment already described the bug the harness
had, since inheriting the environment leaks `VITE_DEV_SERVER_URL` and
loads the dev server instead of the build under test. Launch, readiness
and teardown now go through Playwright's Electron support: fixed sleeps
become per-route readiness selectors, and teardown removes its temp dir.
235 lines become 139, and the hit-test retry goes with them, per the
sibling harness's rule that flakes should fail loudly.
Stop committing baselines. They encode the capturing host — font metrics
and the macOS traffic-light inset — so one machine's baseline reads as
thousands of diffs on another. Capture on the branch you compare
against, apply the slice, compare: the two captures that matter always
come from the same host. The timezone is pinned through the fixture's
IANA override for the same reason. This removes 70k lines from review
and makes the orphan-baseline check moot.
Record what a cascade flip actually changes. `boxShadow`, `textAlign`,
`flexDirection`, `flexWrap` and `outline` were missing, so a utility
that starts beating a product rule could change elevation or text
placement with an identical rect and be reported clean. Three
`INITIAL_VALUES` entries guessed at values Chromium does not serialise
and never matched once, leaving constants on all 3,964 records;
`findDeadOmissionRules` now fails the run when a rule stops matching —
it caught a fourth such entry immediately. Inherited values were
compared against the parent rather than the nearest recorded ancestor,
so a zero-box wrapper's color could change, repaint every visible child,
and leave the capture byte-identical: 129 masked values on the chat
route alone.
Delete the second drag-region resolver. It parsed stylesheet rules in
source order, ignoring specificity, `!important` and `@layer` — which
PR 1 and PR 2 introduce — on the premise that the property is absent
from computed style. It is not: `getComputedStyle(el).webkitAppRegion`
returns `drag`, and `e2e/window-titlebar.spec.ts` has read it that way
in CI all along, against rendered geometry and document order. The
hit-test keeps what that spec does not cover, and now reports how many
elements it actually probed rather than how many the selector matched.
Pin the product change. `resolveDockPresentation` is a pure function
with tests; the branch it replaced could only be exercised by launching
Electron, which is why it had none. The programmatic smoke checks are
now pinned by the contract test that exists to pin checks — including
the new dock one, which could previously be deleted with CI green — and
`e2e` shares `build:with-deps` instead of repeating the chain verbatim.
…budget
buildFixtureEnv read process.env.CI inline, so the fixture-env test
asserting a hidden run stays hidden passed on every laptop and failed on
the Linux CI runner. The ambient read now lives in isCiLinuxDisplay for
callers to compose explicitly.
audit-alignment gets its 2500ms settle back (the shared-launcher refactor
had silently dropped it to 1000ms against static-markup readiness) plus a
real ready selector per fixture, and its header now states the deliberate
environment change from the raw-spawn launcher it replaced.
withFixtureWindow tore down with a bare app.close(), which has no
deadline — one wedged launch turned a capture run (or the CI alignment
audit, which has no job timeout) into an infinite hang, and the temp
userData dir leaked because rm was sequenced after the close.
The bounded close the E2E suite already owned moves to
scripts/electron-lifecycle.mjs — the same shared-home precedent as
fixture-env.mjs, so a bare-node harness and the Playwright suite stop
splitting one launch concern across two directories. Renderer evaluate
gets a deadline too; teardown then reclaims the process either way.
Two measurement-semantics gaps in the visual capture, both instances of
a class the harness had already been bitten by:
- textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79-85%
of records re-recorded an ancestor's value and one container change
printed as N descendant lines. It moves to the inherited omission (and
out of INITIAL_VALUES — absent must mean exactly one thing). A static
table test now pins every CSS-inherited property to that route, which
is the guard findDeadOmissionRules could never provide.
- Descendants of an opacity:0 element report opacity 1 and a non-zero
rect, so the collapsed session panel put 105 invisible records into
the chat capture; a change under it would diff on pixels that never
paint. The walk now prunes zero-opacity subtrees.
The prune exposed that two salvaged anchors were matching those phantom
records: the chat fixture never shows the session rows at all. They now
point at mcp-hub, whose fixture opens the sidebar for real.
Replaces the two-step gitignored-baseline workflow. The checker now
builds the base ref in a cached temp worktree (node_modules shared;
the compare refuses to run across a package-lock change), builds the
working tree, captures every route x theme x platform from both builds,
and diffs in memory. Every step of the old workflow a human could skip
- rebuild after switching, recapture after rebasing, stash on a
committed slice - produced a zero-diff pass that had measured the same
binary twice; now the instrument owns the whole measurement and there
is no baseline file to trust or to stale.
The matrix gains the win32 column #1565 asked for: the fixture platform
override drives the production app:info -> data-os path, so the per-OS
cascade is captured on any host - no Windows runner involved.
desktop-real-window-smoke was the last launcher still spreading
process.env wholesale: a developer with npm run dev open smoked the dev
server instead of the build the script just made, and the run touched
the real $HOME (#1517's skills-deletion surface). It now launches
through buildFixtureEnv with an explicit visible window.
The dock contract drops its duplicated fourth case and pins the wiring
instead - the regression this module exists for was the call site
re-deriving startHidden, which no pure-function test can observe. The
reveal contract follows fixture-env's pure shape (the ambient CI read
now lives in isCiLinuxDisplay, composed at each call site).
…entity
Round-3 review found the compare's base was not a base: the borrowed
node_modules resolved @maka/* through workspace links straight back to
the working tree, so the base bundled and loaded the candidate's own
packages — a slice changing @maka/core would have compared itself
against itself. The base now runs its own npm ci against its own
lockfile in the cached worktree (atomic lock against concurrent
builds; the marker name encodes the scheme so old symlink-era caches
rebuild). The lockfile-parity gate and the symlink list existed only
to justify the sharing; both are deleted, and a slice may now
legitimately change dependencies.
The same round showed the instrument asserting nothing about what it
measured, so now it proves it: route/theme/platform are closed sets;
every capture waits for the requested data-os and theme on the live
document before reading a style; painted ::before/::after get paint
signatures (47 pseudo rules paint in this renderer, led by the
body::after film grain, and a cascade flip on any of them was
invisible); mixBlendMode joins the property set; and dead omission
rules are counted at the raw sample where the border/outline gates
cannot shadow them.
The smoke gate still had two unbounded exits: --manual waited forever
on 'exit' after one SIGTERM, and the report path fired SIGTERM and
called process.exit without waiting or escalating — plus a failed
report write skipped the kill entirely. All paths now stop through the
shared bounded close (SIGTERM as the graceful phase, 5s grace, SIGKILL
the tree), inside a finally.
electron-lifecycle's import of @maka/runtime now fails with an error
that says what to build; on a fresh clone the raw ERR_MODULE_NOT_FOUND
surfaced from four files deep in the import chain.
The shared teardown's force path kills by process group. The smoke gate
spawned Electron without detached, so the group signal hit ESRCH, the
terminator read that as "tree already gone", and a visible window that
had ignored SIGTERM survived its own SIGKILL stage. Spawn the smoke
child as a group leader like every runtime consumer does, and back the
group kill with a direct kill on the root so a future non-leader child
still dies inside the 2s exit deadline.
filter, backdrop-filter, clip-path and transform move pixels without
moving the rect — grayscale() on disabled provider rows, the glass
theme's backdrop blur, clip-path as visually-hidden, rotate() on square
chevrons — so losing one was invisible to every property the contract
sampled. Capture all four (plus transform-origin, gated on a transform
actually painting), and stop the renderer's clock first: a running
spinner serialises a different matrix at every phase, which would diff
time rather than the cascade. Verified with a negative control (an
injected filter rule fails the compare) and a clean 20/20 run.
Also correct the header, which still described the rejected
node_modules-sharing base build, and compare the direct-run guard
through pathToFileURL so a percent-encoding checkout path cannot turn
the instrument into a silent exit-0 no-op.
--route accepted any value and only failed when nothing matched, so
`--route chat --route chatt` dropped the typo and exited 0 as "1
route(s) clean" — silently shrinking the requested coverage. Validate
each value against the closed route set (as the visual contract already
does), guard the entry point through pathToFileURL, and pin the CLI's
exit codes with subprocess tests.
The ESRCH-swallowing group-kill path was only proven against a live
window; the fake now exercises it too — a terminator that returns
without killing must be followed by a direct SIGKILL on the root.
@Astro-Han
Astro-Han marked this pull request as ready for review July 30, 2026 13:04
@Astro-Han
Astro-Han merged commit 71e47a6 into mainJul 30, 2026
3 checks passed
@Astro-Han
Astro-Han deleted the refactor/1565-pr0-contract-harness branch July 30, 2026 13:04
Sign up for freeto 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

@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(desktop): add the Astryx migration contract harness by Astro-Han · Pull Request #1653 · apache/maka · GitHub
Skip to content

feat(desktop): add the Astryx migration contract harness - #1653

Merged
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness
Jul 30, 2026
Merged

feat(desktop): add the Astryx migration contract harness#1653
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #1565. PR 0 of the Astryx renderer migration: the measurement instrument every later slice depends on. The mega-branch produced regressions on screens nobody edited, and slicing alone cannot localise them — the bisect signal has to come from a record of what the app looked like before.

  • check:visual-contract — one command compares two builds: it checks out the base ref into a cached temp worktree with its own npm ci, builds both sides, captures five representative fixture routes × light/dark × darwin/win32 from each, and diffs every visible element's box, paint, and alignment properties — plus a paint signature for every painted ::before/::after — in memory. Gates no-change, not correctness: a pre-existing bug on the base stays out of scope, and zero diff means "the migration did not move this element", never "this element is right".
  • check:hit-test — interactive elements must be reachable at their centre, free of pointer-events/visibility traps, and opaque through their ancestry; every route reports what it probed and what it skipped (invisible, out-of-scope, disabled, transparent, clipped) so the coverage is a number, not an assumption.
  • launch:fixture — a real, clickable fixture window for manual visual comparison, which a visually-neutral migration cannot do without.
  • Salvages the mega-branch's late regression fixes into the harness (item 3 below).

Both harnesses live in scripts/ as npm scripts and are deliberately outside CI, following the check:chat-visual precedent and #1565's own scoping — a two-build compare costs minutes per run, which is a per-slice gate, not a per-push one. They are migration-only and get deleted in PR 14 with the maka.legacy layer. scripts/fixture-env.mjs, scripts/fixture-window.mjs, and scripts/electron-lifecycle.mjs are the exception: audit-alignment.mjs runs in CI and the E2E suite shares them, so they outlive the harness.

How to use it

node scripts/check-visual-contract.mjs # compare main → working tree
node scripts/check-visual-contract.mjs --against <ref># a different base
node scripts/check-visual-contract.mjs --route chat --theme dark --platform win32

There is no baseline file and no capture step to remember. An earlier version had one, captured by hand on the base branch and compared by hand after switching; every step a human could skip — rebuild after switching, recapture after rebasing — produced a convincing zero-diff pass that had measured the same binary twice. Now the instrument owns the whole measurement: both sides are checked out, installed, built, and captured in the same run, on the same host (font metrics and the macOS traffic-light inset are host facts, so cross-host comparison was never sound anyway).

The base gets its own dependency closure: a fresh worktree of the ref with its own npm ci against its own lockfile, cached keyed by the resolved commit, so iterating on a slice rebuilds only the working tree. Sharing this checkout's node_modules was tried and rejected — the workspace links inside it resolve @maka/* back to the working tree's packages, so the base would have bundled and loaded the candidate's own code. A side effect of the honest install: a slice may legitimately change dependencies. Route/theme/platform arguments are closed sets, and every capture asserts the renderer actually reached the requested data-os and theme before reading a single style — a capture must prove it measured the cascade it claims.

The win32 column runs on any host: MAKA_E2E_FIXTURE_PLATFORM drives the production app:info → data-os path, which is what keys the per-OS CSS. It covers the cascade, not native chrome.

Route mapping

#1565 names product routes; these are the MAKA_E2E_FIXTURE scenarios that actually open them on main. There is no scenario literally called "chat" or "settings-providers".

RouteScenarioReadiness selector
chatturn-narrative.maka-session-workbar
settings/generalsettings-general.settingsRows
settings/providersprovider-workspace.providersPanel
mcp hubmodule-mcp.maka-module-main-header
onboardingfirst-run.maka-onboarding-surface

Known blind spots, stated in the tool's header rather than papered over: display: none subtrees (closed menus, unmounted dialogs) never enter the capture; pseudo-elements are paint signatures without a rect; native top-layer content (dialog.showModal, popover) would escape the opacity-prune, and nothing in the app uses it today.

Where this departs from #1565, and why

Each departure is a measurement, not a preference. The issue body has been corrected where it was factually wrong.

The fixture window was never unclickable.#1565 stated that app.dock.hide() makes fixture runs reject all real clicks. Measured on ae43cb291/darwin, that does not hold: MAKA_E2E_FIXTURE alone means the window is never shown at all (startHiddenkeepHiddenForE2eFixture, main-window.ts:114), and adding MAKA_E2E_SHOW_WINDOW=1 gives isVisible: true, isFocused: true, and fully working clicks — with dock.hide() still running.

scripts/desktop-real-window-smoke.mjs was already broken on main. It boots a real fixture window but never set MAKA_E2E_SHOW_WINDOW, so its own programmatic-window-visible check fails and its twelve human checks are unrunnable against an invisible window. Fixed here, extended with --manual rather than adding a second launcher, and now launched through the shared environment builder — it was the last launcher still inheriting process.env wholesale, which meant a developer with npm run dev open smoked the dev server instead of the build the script had just made.

The one product-code change is about reachability. The dock rule now keys off the same startHidden that decides whether the window shows, instead of re-deriving it from MAKA_E2E_FIXTURE — a re-derivation that had already drifted, since it missed the MAKA_E2E_SHOW_WINDOW escape hatch. An accessory app has no Dock tile and no Cmd+Tab entry, so a reviewer who switches away mid-comparison cannot switch back. Capture runs and CI e2e leave the variable unset and keep their accessory, no-focus-steal behaviour unchanged.

The window-drag question is not asked here.#1565 proposed probing it with a real click; that was tried and abandoned (an unmapped window does not route synthesized input, and a visible one fired real product actions — one opened a native file dialog and hung the run). It is also already answered: e2e/window-titlebar.spec.ts measures it in CI against rendered geometry and the document order Chromium composes drag rects in, and .maka-window-titlebar is the only element allowed to declare drag. The hit-test covers what that spec does not — whether a control anywhere in the window can be reached at all.

Centre is required; corners corroborate. All five probe points cannot pass on a clean main — sibling chrome legitimately covers a couple of edge pixels. Three lost corners means something is actually covering the control; one means the border has a neighbour.

Item 3: salvaged regressions

Six of the eight late "fix cascade / fix click" commits land on properties the snapshot already records. Bespoke rules would be redundant; what they do add is noticing when the harness stops watching, so each is pinned to an anchor that must keep appearing in a route where the element actually paints. That last clause is load-bearing: the session-row anchors originally pointed at the chat route, where the fixture keeps the panel collapsed under opacity: 0 — they "matched" records with no visible pixels until the capture started pruning invisible subtrees, at which point the check failed and exposed its own false assurance. They now point at mcp-hub, whose fixture opens the sidebar.

CommitRegressionAnchorRoute
be5e69584titlebar clusters wrapped onto a second rowmaka-shell-topbar-railchat
fd38a37cecomposer frame jumped on focusmaka-composer-inner (resting state only)chat
9aad59740session timestamp stacked above its titlemaka-list-row-metamcp-hub
3dbd68ca7sidebar nav rows had two styling authoritiesmaka-list-rowmcp-hub
9d20a9396task ledger recent-count collided with its labelmaka-session-workbar-countchat
be1406705settings content column painted over the nav railsettingsSidebarsettings-general

Two are not transcribable: 3e57d1951 (stat tile leading) and 400e8f4a9 (turn marker measure) fix files the mega-branch itself created, which do not exist on main. They are recorded here as fragile spots for whichever slice introduces those components.

What review changed

Four review rounds (Codex plus an independent fresh-context agent each time), every finding reproduced before being fixed. The branch is meaningfully different as a result.

Round 1

It could report ok for a window it never launched. The harness rolled its own launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a process-group kill, ...process.env. It never verified the CDP target belonged to the child it spawned, so a leftover Electron on the port was captured instead; reproduced with two concurrent runs both reading the same window, and 270 stale user-data dirs on the dev machine. Now launched through Playwright's Electron support, with the environment builder extracted to scripts/fixture-env.mjs and shared with the E2E suite.

It could not see the class of change the migration makes.boxShadow, textAlign, flexDirection, flexWrap and outline were not captured — a utility that starts beating a product rule after the @layer move can change elevation or text placement with an identical rect. Three INITIAL_VALUES entries guessed at values Chromium does not serialise and never matched once.

Inherited values were compared against the wrong ancestor. A zero-box or display: contents wrapper is skipped, so nothing in the capture carried its values — yet visible children omitted properties that matched it. Now compared against the nearest recorded ancestor, so every omission is recoverable by a reader of the file.

The drag-region resolver was built on a false premise. It parsed stylesheet rules in source order because "the property is absent from getComputedStyle". It is not: getComputedStyle(el).webkitAppRegion returns drag, and window-titlebar.spec.ts has read it that way in CI all along. Deleted, ~50 lines.

The product change had no automated coverage.resolveDockPresentation is now a pure function with tests, and the programmatic smoke layer is pinned by the source contract.

Round 2

The two-step baseline workflow could pass silently when a step was skipped. The checker never rebuilt the candidate, the baseline was a bare array with no provenance, and the gitignored file could not travel between worktrees — comparing a build to itself read as a clean slice. Replaced by the one-command --against compare; the baseline file, the --update flag, and the .gitignore entry are gone with it.

Teardown was unbounded, and CI inherits this launcher.app.close() has no deadline; one wedged launch turned a capture run — or the CI alignment audit, whose job has no timeout — into an infinite hang, reproduced once at ≥600s. The bounded close the E2E suite already owned (grace, then SIGKILL the tree) moved to scripts/electron-lifecycle.mjs and both consumers share it; renderer evaluate gained a deadline; the temp profile is removed even when close fails.

The shared-launcher refactor had silently changed the CI gate's conditions.audit-alignment's settle budget had dropped 2500ms → 1000ms against static-markup readiness, and buildFixtureEnv read process.env.CI inline — which made the "hidden run stays hidden" test pass on every laptop and fail on the Linux CI runner. The settle budget is restored with real per-fixture ready selectors, the window-visibility decision is a pure function of the builder's arguments, and the one ambient read lives in isCiLinuxDisplay for callers to compose.

Two capture-semantics gaps.textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79–85% of records re-recorded an ancestor's value; and descendants of an opacity: 0 element (own opacity 1, non-zero rect) were captured as visible — 105 phantom records under the collapsed session panel. Fixed, plus a static table test pinning every CSS-inherited property to the inherited route in both directions.

The darwin-only matrix contradicted the issue, and its stated justification was wrong. No Windows runner is needed: the fixture platform override flips data-os through the production path on any host. The matrix now includes win32.

Round 3

The base was not isolated. The base worktree borrowed this checkout's node_modules, and the workspace links inside it resolved @maka/* straight back to the working tree — the base bundled and loaded the candidate's own packages, so a slice changing @maka/core would have compared itself against itself. Both reviewers converged on this independently; reproduced byte-for-byte with require.resolve. The base now runs its own npm ci against its own lockfile (cached per commit, guarded by an atomic build lock), and the lockfile-parity gate plus the symlink list that existed to justify the sharing are deleted.

Pseudo-elements were invisible to the contract. 47 ::before/::after rules paint in this renderer — including the full-screen body::after film-grain overlay — and a cascade flip on any of them changed pixels while every element record stayed byte-identical. Painted pseudo-elements are now captured as paint signatures that inherit from their host.

The instrument now proves its own measurement identity. CLI arguments are closed sets; every capture waits for and asserts the requested data-os and theme on the live document before reading styles — the win32 column can no longer silently degrade into a second darwin column if the override seam regresses. Dead omission rules are counted at the raw sample inside the capture (not inferred from the compressed output), so a rule shadowed by the border/outline paint gates still proves itself.

Remaining teardown edges. The smoke gate's SIGTERM paths now escalate through the shared bounded close, and a failed report write no longer leaves a live Electron behind. Verifying that fix surfaced one more: the shared force path kills by process group, the smoke gate's child was not spawned detached, so the group signal hit ESRCH, the terminator read "tree already gone", and a visible window that ignores SIGTERM survived its own SIGKILL stage. The smoke child is now a group leader like every runtime consumer, and the bounded close backs the group kill with a direct kill on the root.

Round 4

The contract could not see the paint-only channels.filter, backdrop-filter, clip-path and transform move pixels without moving the rect — grayscale() on disabled provider rows, the glass theme's backdrop blur, clip-path as visually-hidden, rotate() on square chevrons — so a migration losing one diffed zero. All four are now sampled (plus transform-origin, gated on a transform actually painting), and the capture freezes animations and transitions before reading a single style: a running spinner serialises a different matrix at every phase, which would diff time rather than the cascade. Verified with a negative control — an injected filter rule fails the compare with filter: ∅ → sepia(0.1) — and a clean 20/20 run.

The hit-test's --route was an open set. It accepted any value and failed only when nothing matched, so --route chat --route chatt dropped the typo and exited 0 as "1 route(s) clean" — silent coverage shrink on a tool whose whole point is that coverage is a number. Each value is now validated against the closed route set (as the visual contract already does), and the CLI's exit codes are pinned by subprocess tests.

Two smaller honesty fixes. Both direct-run guards compare through pathToFileURL — a checkout path with a space or non-ASCII segment percent-encodes in import.meta.url, and the string-concatenation compare would have made the instrument exit 0 having measured nothing. The tool header still described the rejected node_modules-sharing base build; it now states the actual isolation model. The direct-kill backstop from round 3 gained a fake-based test alongside its live-window proof.

Verification

Run on main (ae43cb291) + this branch, macOS arm64.

npm run lint ✓
npm run format:check ✓
npm run typecheck ✓
npx knip ✓ exit 0 (2 config hints, same as main)
npm run test:scripts ✓ 209 pass, 0 fail
npm --workspace @maka/desktop run test:dist ✓ 2981 pass, 0 fail
node scripts/audit-alignment.mjs ✓ 12/12 fixtures clean
node scripts/check-visual-contract.mjs ✓ 20/20 comparisons clean against main
(fresh base: worktree + npm ci + build, all by the tool;
negative control: an injected filter rule fails the compare)
node scripts/check-hit-test.mjs ✓ 5/5 routes clean
npx playwright test first-run window-titlebar electron-lifecycle
✓ 7 passed
npm --workspace @maka/desktop run smoke:programmatic-window
✓ PASS incl. programmatic-dock-visible (was FAIL on main)

The real-window smoke launches the app with MAKA_E2E_FIXTURE, which makes
main.ts compute startHidden and main-window.ts keep the window hidden for
its whole lifecycle. The gate therefore had nothing to look at: on a clean
main its own programmatic-window-visible check reports visible=false and
exits 1, and every human check it prompts for — dragging window edges and
corners to resize, dragging the titlebar to move, Tab/Shift+Tab traversal
inside the modal — is unrunnable against an invisible window.
Set MAKA_E2E_SHOW_WINDOW on the launch env, which is exactly the escape
hatch main.ts already provides for runs where there is no focus to steal.
Capture runs and CI e2e do not set it and keep their hidden-window
behavior unchanged.
Hiding the dock icon makes the run an accessory app. Contrary to what the
migration tracker assumed, such a window is fully clickable — measured on
darwin: with MAKA_E2E_SHOW_WINDOW set, the fixture window reports
isVisible/isFocused true and takes clicks, drags, and keyboard input
normally. What it does not have is a Dock tile or a Cmd+Tab entry, so a
reviewer doing manual visual comparison who switches to another app has no
way back to it.
Key the dock icon off the same startHidden that decides whether the window
shows at all, instead of re-deriving the condition from MAKA_E2E_FIXTURE.
The two can no longer drift, and MAKA_E2E_SHOW_WINDOW now opts out of both
halves of staying out of sight. Capture runs and CI e2e leave the variable
unset, so they keep hiding the window and the dock exactly as before.
Lock it with a dock diagnostic and a programmatic-dock-visible check in the
real-window smoke, which reported dockVisible=false before the change and
true after.
A migration that must stay visually neutral needs to look at the running
app constantly, but the only way to boot a route with fixture data was the
smoke gate, which insists on walking a reviewer through twelve checklist
prompts and filing a report every time.
Add --manual: same launch path, then stop. The window stays up until it is
closed or interrupted. Reusing the gate's launcher keeps the window a
reviewer inspects identical to the window the gate checks, and leaves one
place that knows how to boot a fixture instance.
Expose it as `launch:fixture`, and lift the build chain the smoke scripts
repeated verbatim into `build:with-deps` rather than copying it a third
time.
#1565 PR 0. The mega-branch produced regressions on screens nobody edited,
and slicing alone does not localise them: the signal has to come from a
baseline that says what the app looked like before. This captures one.
check:visual-contract walks the five representative fixture routes in light
and dark, records every visible element's box, paint, and alignment
properties, and diffs the result against committed JSON. The alignment set
(align-items/justify-content/place-items/gap/grid-area) is there because
flex and grid misalignment is exactly the class a paint-only property set
cannot see. It gates no-change, not correctness: a pre-existing bug on main
stays out of scope, and zero diff means the migration did not move this
element, never that the element is right.
The boot sequence audit-alignment.mjs already worked out — spawn Electron
with MAKA_E2E_FIXTURE, wait for a CDP page target, evaluate in the renderer
— moves to fixture-cdp.mjs so both scripts share one launcher instead of
growing a second. It also gains what a contract harness needs and an audit
could do without: animations and webfont reflow frozen before capture, an
evaluate deadline so a wedged renderer fails loudly instead of hanging, and
process-group teardown so Electron's helpers do not pile up and starve
later windows.
Also carries #1565's third PR 0 item, salvaging the mega-branch's late
regression fixes. Six of the eight land on properties this snapshot already
records — a wrapped titlebar row, a timestamp stacking above its title, a
missing gap, a column painting over the settings rail all move a rect or a
recorded property, so the diff catches them without bespoke rules. What
bespoke rules do add is noticing when the harness stops watching, so each
one is pinned to an anchor that must keep appearing in the baseline. The
remaining two fix files the mega-branch itself created, which do not exist
on main; they are recorded in the PR body instead.
Determinism was verified by capturing the same build twice. The first
attempt was not clean, and both causes are excluded rather than tuned
around: element labels fell back to el.id, which Base UI regenerates every
render, and OverlayScrollbars' chrome fades with pointer activity, so its
visibility was a coin flip. Baselines stay at 1.5MB rather than 4MB because
properties equal to their initial value, or inherited unchanged from the
parent, are omitted and read back as unchanged.
Migration-only: deliberately outside CI, following check:chat-visual, and
removed in PR 14 with the maka.legacy layer.
#1565 PR 0. The mega-branch's most expensive regressions were controls that
render perfectly and refuse clicks. A computed-style snapshot cannot see
that, so it gets a contract of its own: every visible interactive element on
the five routes must be reachable at its centre, must not sit under a
pointer-events or visibility trap, must not be transparent through an
ancestor, and must not fall inside an -webkit-app-region: drag area.
Three parts of #1565's proposal did not survive contact with a clean main,
and each is replaced by something answering the same question:
The drag-region probe was specified as a real click plus a side effect.
Fixture windows are hidden, and an unmapped window does not route
synthesized input through the browser's hit-testing path, so every target
read as swallowed. Making the window visible to fix that turned the probe
into something that fires real product actions — one opened a native file
dialog and hung the run. Reading -webkit-app-region off the stylesheet rules
instead has no side effects: the property is missing from getComputedStyle,
but not from the rules that declare it.
All five probe points were specified as required. On a clean main that is
unreachable: sibling chrome legitimately covers a couple of edge pixels, so
a workbar resize handle would indict a control nobody struggles to click.
The centre is now required and corners corroborate — three lost corners
means something covers the control.
The overlay guard keyed on [role=dialog][open], which a React-rendered
div[role=dialog] never has, so the settings surface went undetected and
every control behind it was reported unhittable. It now keys on the
product's own data-modal="true".
Geometry probes do need a visible window — elementFromPoint disagrees with
getBoundingClientRect against a hidden one — so this check steals focus for
a few seconds per route. Memoising style and rect lookups took the densest
route from a 60s timeout to 3s, and routes are spaced so one window is fully
gone before the next boots.
All five routes are clean on ae43cb2.
@Astro-Han
Astro-Han marked this pull request as draft July 30, 2026 09:45
Review found the harness could report `ok` for a window it never
launched, could not see the class of change the migration actually
makes, and shipped its only product change with no automated coverage.
Launch through the shared E2E seam. The harness rolled its own Electron
launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a
process-group kill, `...process.env` — all of it already solved next
door in `apps/desktop/e2e/fixtures.ts`, and each hand-rolled version
worse. It never verified the CDP target belonged to the child it
spawned, and the port allocator restarted at 14600 in every process, so
a leftover Electron on that port got captured instead. Reproduced: two
concurrent runs requesting different scenarios both read the same
window. When the leftover is the same route from an older build — the
shape you get by re-running one route after an edit — the contract
reports `ok` for a stale window. `detached: true` with no signal handler
supplied the leftovers; this machine had 270 stale user-data dirs, none
ever removed. `buildE2eEnv` moves to `scripts/fixture-env.mjs`, shared
by both callers; its own comment already described the bug the harness
had, since inheriting the environment leaks `VITE_DEV_SERVER_URL` and
loads the dev server instead of the build under test. Launch, readiness
and teardown now go through Playwright's Electron support: fixed sleeps
become per-route readiness selectors, and teardown removes its temp dir.
235 lines become 139, and the hit-test retry goes with them, per the
sibling harness's rule that flakes should fail loudly.
Stop committing baselines. They encode the capturing host — font metrics
and the macOS traffic-light inset — so one machine's baseline reads as
thousands of diffs on another. Capture on the branch you compare
against, apply the slice, compare: the two captures that matter always
come from the same host. The timezone is pinned through the fixture's
IANA override for the same reason. This removes 70k lines from review
and makes the orphan-baseline check moot.
Record what a cascade flip actually changes. `boxShadow`, `textAlign`,
`flexDirection`, `flexWrap` and `outline` were missing, so a utility
that starts beating a product rule could change elevation or text
placement with an identical rect and be reported clean. Three
`INITIAL_VALUES` entries guessed at values Chromium does not serialise
and never matched once, leaving constants on all 3,964 records;
`findDeadOmissionRules` now fails the run when a rule stops matching —
it caught a fourth such entry immediately. Inherited values were
compared against the parent rather than the nearest recorded ancestor,
so a zero-box wrapper's color could change, repaint every visible child,
and leave the capture byte-identical: 129 masked values on the chat
route alone.
Delete the second drag-region resolver. It parsed stylesheet rules in
source order, ignoring specificity, `!important` and `@layer` — which
PR 1 and PR 2 introduce — on the premise that the property is absent
from computed style. It is not: `getComputedStyle(el).webkitAppRegion`
returns `drag`, and `e2e/window-titlebar.spec.ts` has read it that way
in CI all along, against rendered geometry and document order. The
hit-test keeps what that spec does not cover, and now reports how many
elements it actually probed rather than how many the selector matched.
Pin the product change. `resolveDockPresentation` is a pure function
with tests; the branch it replaced could only be exercised by launching
Electron, which is why it had none. The programmatic smoke checks are
now pinned by the contract test that exists to pin checks — including
the new dock one, which could previously be deleted with CI green — and
`e2e` shares `build:with-deps` instead of repeating the chain verbatim.
…budget
buildFixtureEnv read process.env.CI inline, so the fixture-env test
asserting a hidden run stays hidden passed on every laptop and failed on
the Linux CI runner. The ambient read now lives in isCiLinuxDisplay for
callers to compose explicitly.
audit-alignment gets its 2500ms settle back (the shared-launcher refactor
had silently dropped it to 1000ms against static-markup readiness) plus a
real ready selector per fixture, and its header now states the deliberate
environment change from the raw-spawn launcher it replaced.
withFixtureWindow tore down with a bare app.close(), which has no
deadline — one wedged launch turned a capture run (or the CI alignment
audit, which has no job timeout) into an infinite hang, and the temp
userData dir leaked because rm was sequenced after the close.
The bounded close the E2E suite already owned moves to
scripts/electron-lifecycle.mjs — the same shared-home precedent as
fixture-env.mjs, so a bare-node harness and the Playwright suite stop
splitting one launch concern across two directories. Renderer evaluate
gets a deadline too; teardown then reclaims the process either way.
Two measurement-semantics gaps in the visual capture, both instances of
a class the harness had already been bitten by:
- textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79-85%
of records re-recorded an ancestor's value and one container change
printed as N descendant lines. It moves to the inherited omission (and
out of INITIAL_VALUES — absent must mean exactly one thing). A static
table test now pins every CSS-inherited property to that route, which
is the guard findDeadOmissionRules could never provide.
- Descendants of an opacity:0 element report opacity 1 and a non-zero
rect, so the collapsed session panel put 105 invisible records into
the chat capture; a change under it would diff on pixels that never
paint. The walk now prunes zero-opacity subtrees.
The prune exposed that two salvaged anchors were matching those phantom
records: the chat fixture never shows the session rows at all. They now
point at mcp-hub, whose fixture opens the sidebar for real.
Replaces the two-step gitignored-baseline workflow. The checker now
builds the base ref in a cached temp worktree (node_modules shared;
the compare refuses to run across a package-lock change), builds the
working tree, captures every route x theme x platform from both builds,
and diffs in memory. Every step of the old workflow a human could skip
- rebuild after switching, recapture after rebasing, stash on a
committed slice - produced a zero-diff pass that had measured the same
binary twice; now the instrument owns the whole measurement and there
is no baseline file to trust or to stale.
The matrix gains the win32 column #1565 asked for: the fixture platform
override drives the production app:info -> data-os path, so the per-OS
cascade is captured on any host - no Windows runner involved.
desktop-real-window-smoke was the last launcher still spreading
process.env wholesale: a developer with npm run dev open smoked the dev
server instead of the build the script just made, and the run touched
the real $HOME (#1517's skills-deletion surface). It now launches
through buildFixtureEnv with an explicit visible window.
The dock contract drops its duplicated fourth case and pins the wiring
instead - the regression this module exists for was the call site
re-deriving startHidden, which no pure-function test can observe. The
reveal contract follows fixture-env's pure shape (the ambient CI read
now lives in isCiLinuxDisplay, composed at each call site).
…entity
Round-3 review found the compare's base was not a base: the borrowed
node_modules resolved @maka/* through workspace links straight back to
the working tree, so the base bundled and loaded the candidate's own
packages — a slice changing @maka/core would have compared itself
against itself. The base now runs its own npm ci against its own
lockfile in the cached worktree (atomic lock against concurrent
builds; the marker name encodes the scheme so old symlink-era caches
rebuild). The lockfile-parity gate and the symlink list existed only
to justify the sharing; both are deleted, and a slice may now
legitimately change dependencies.
The same round showed the instrument asserting nothing about what it
measured, so now it proves it: route/theme/platform are closed sets;
every capture waits for the requested data-os and theme on the live
document before reading a style; painted ::before/::after get paint
signatures (47 pseudo rules paint in this renderer, led by the
body::after film grain, and a cascade flip on any of them was
invisible); mixBlendMode joins the property set; and dead omission
rules are counted at the raw sample where the border/outline gates
cannot shadow them.
The smoke gate still had two unbounded exits: --manual waited forever
on 'exit' after one SIGTERM, and the report path fired SIGTERM and
called process.exit without waiting or escalating — plus a failed
report write skipped the kill entirely. All paths now stop through the
shared bounded close (SIGTERM as the graceful phase, 5s grace, SIGKILL
the tree), inside a finally.
electron-lifecycle's import of @maka/runtime now fails with an error
that says what to build; on a fresh clone the raw ERR_MODULE_NOT_FOUND
surfaced from four files deep in the import chain.
The shared teardown's force path kills by process group. The smoke gate
spawned Electron without detached, so the group signal hit ESRCH, the
terminator read that as "tree already gone", and a visible window that
had ignored SIGTERM survived its own SIGKILL stage. Spawn the smoke
child as a group leader like every runtime consumer does, and back the
group kill with a direct kill on the root so a future non-leader child
still dies inside the 2s exit deadline.
filter, backdrop-filter, clip-path and transform move pixels without
moving the rect — grayscale() on disabled provider rows, the glass
theme's backdrop blur, clip-path as visually-hidden, rotate() on square
chevrons — so losing one was invisible to every property the contract
sampled. Capture all four (plus transform-origin, gated on a transform
actually painting), and stop the renderer's clock first: a running
spinner serialises a different matrix at every phase, which would diff
time rather than the cascade. Verified with a negative control (an
injected filter rule fails the compare) and a clean 20/20 run.
Also correct the header, which still described the rejected
node_modules-sharing base build, and compare the direct-run guard
through pathToFileURL so a percent-encoding checkout path cannot turn
the instrument into a silent exit-0 no-op.
--route accepted any value and only failed when nothing matched, so
`--route chat --route chatt` dropped the typo and exited 0 as "1
route(s) clean" — silently shrinking the requested coverage. Validate
each value against the closed route set (as the visual contract already
does), guard the entry point through pathToFileURL, and pin the CLI's
exit codes with subprocess tests.
The ESRCH-swallowing group-kill path was only proven against a live
window; the fake now exercises it too — a terminator that returns
without killing must be followed by a direct SIGKILL on the root.
@Astro-Han
Astro-Han marked this pull request as ready for review July 30, 2026 13:04
@Astro-Han
Astro-Han merged commit 71e47a6 into mainJul 30, 2026
3 checks passed
@Astro-Han
Astro-Han deleted the refactor/1565-pr0-contract-harness branch July 30, 2026 13:04
Sign up for freeto 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

@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); feat(desktop): add the Astryx migration contract harness by Astro-Han · Pull Request #1653 · apache/maka · GitHub
Skip to content

feat(desktop): add the Astryx migration contract harness - #1653

Merged
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness
Jul 30, 2026
Merged

feat(desktop): add the Astryx migration contract harness#1653
Astro-Han merged 17 commits into
mainfrom
refactor/1565-pr0-contract-harness

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #1565. PR 0 of the Astryx renderer migration: the measurement instrument every later slice depends on. The mega-branch produced regressions on screens nobody edited, and slicing alone cannot localise them — the bisect signal has to come from a record of what the app looked like before.

  • check:visual-contract — one command compares two builds: it checks out the base ref into a cached temp worktree with its own npm ci, builds both sides, captures five representative fixture routes × light/dark × darwin/win32 from each, and diffs every visible element's box, paint, and alignment properties — plus a paint signature for every painted ::before/::after — in memory. Gates no-change, not correctness: a pre-existing bug on the base stays out of scope, and zero diff means "the migration did not move this element", never "this element is right".
  • check:hit-test — interactive elements must be reachable at their centre, free of pointer-events/visibility traps, and opaque through their ancestry; every route reports what it probed and what it skipped (invisible, out-of-scope, disabled, transparent, clipped) so the coverage is a number, not an assumption.
  • launch:fixture — a real, clickable fixture window for manual visual comparison, which a visually-neutral migration cannot do without.
  • Salvages the mega-branch's late regression fixes into the harness (item 3 below).

Both harnesses live in scripts/ as npm scripts and are deliberately outside CI, following the check:chat-visual precedent and #1565's own scoping — a two-build compare costs minutes per run, which is a per-slice gate, not a per-push one. They are migration-only and get deleted in PR 14 with the maka.legacy layer. scripts/fixture-env.mjs, scripts/fixture-window.mjs, and scripts/electron-lifecycle.mjs are the exception: audit-alignment.mjs runs in CI and the E2E suite shares them, so they outlive the harness.

How to use it

node scripts/check-visual-contract.mjs # compare main → working tree
node scripts/check-visual-contract.mjs --against <ref># a different base
node scripts/check-visual-contract.mjs --route chat --theme dark --platform win32

There is no baseline file and no capture step to remember. An earlier version had one, captured by hand on the base branch and compared by hand after switching; every step a human could skip — rebuild after switching, recapture after rebasing — produced a convincing zero-diff pass that had measured the same binary twice. Now the instrument owns the whole measurement: both sides are checked out, installed, built, and captured in the same run, on the same host (font metrics and the macOS traffic-light inset are host facts, so cross-host comparison was never sound anyway).

The base gets its own dependency closure: a fresh worktree of the ref with its own npm ci against its own lockfile, cached keyed by the resolved commit, so iterating on a slice rebuilds only the working tree. Sharing this checkout's node_modules was tried and rejected — the workspace links inside it resolve @maka/* back to the working tree's packages, so the base would have bundled and loaded the candidate's own code. A side effect of the honest install: a slice may legitimately change dependencies. Route/theme/platform arguments are closed sets, and every capture asserts the renderer actually reached the requested data-os and theme before reading a single style — a capture must prove it measured the cascade it claims.

The win32 column runs on any host: MAKA_E2E_FIXTURE_PLATFORM drives the production app:info → data-os path, which is what keys the per-OS CSS. It covers the cascade, not native chrome.

Route mapping

#1565 names product routes; these are the MAKA_E2E_FIXTURE scenarios that actually open them on main. There is no scenario literally called "chat" or "settings-providers".

RouteScenarioReadiness selector
chatturn-narrative.maka-session-workbar
settings/generalsettings-general.settingsRows
settings/providersprovider-workspace.providersPanel
mcp hubmodule-mcp.maka-module-main-header
onboardingfirst-run.maka-onboarding-surface

Known blind spots, stated in the tool's header rather than papered over: display: none subtrees (closed menus, unmounted dialogs) never enter the capture; pseudo-elements are paint signatures without a rect; native top-layer content (dialog.showModal, popover) would escape the opacity-prune, and nothing in the app uses it today.

Where this departs from #1565, and why

Each departure is a measurement, not a preference. The issue body has been corrected where it was factually wrong.

The fixture window was never unclickable.#1565 stated that app.dock.hide() makes fixture runs reject all real clicks. Measured on ae43cb291/darwin, that does not hold: MAKA_E2E_FIXTURE alone means the window is never shown at all (startHiddenkeepHiddenForE2eFixture, main-window.ts:114), and adding MAKA_E2E_SHOW_WINDOW=1 gives isVisible: true, isFocused: true, and fully working clicks — with dock.hide() still running.

scripts/desktop-real-window-smoke.mjs was already broken on main. It boots a real fixture window but never set MAKA_E2E_SHOW_WINDOW, so its own programmatic-window-visible check fails and its twelve human checks are unrunnable against an invisible window. Fixed here, extended with --manual rather than adding a second launcher, and now launched through the shared environment builder — it was the last launcher still inheriting process.env wholesale, which meant a developer with npm run dev open smoked the dev server instead of the build the script had just made.

The one product-code change is about reachability. The dock rule now keys off the same startHidden that decides whether the window shows, instead of re-deriving it from MAKA_E2E_FIXTURE — a re-derivation that had already drifted, since it missed the MAKA_E2E_SHOW_WINDOW escape hatch. An accessory app has no Dock tile and no Cmd+Tab entry, so a reviewer who switches away mid-comparison cannot switch back. Capture runs and CI e2e leave the variable unset and keep their accessory, no-focus-steal behaviour unchanged.

The window-drag question is not asked here.#1565 proposed probing it with a real click; that was tried and abandoned (an unmapped window does not route synthesized input, and a visible one fired real product actions — one opened a native file dialog and hung the run). It is also already answered: e2e/window-titlebar.spec.ts measures it in CI against rendered geometry and the document order Chromium composes drag rects in, and .maka-window-titlebar is the only element allowed to declare drag. The hit-test covers what that spec does not — whether a control anywhere in the window can be reached at all.

Centre is required; corners corroborate. All five probe points cannot pass on a clean main — sibling chrome legitimately covers a couple of edge pixels. Three lost corners means something is actually covering the control; one means the border has a neighbour.

Item 3: salvaged regressions

Six of the eight late "fix cascade / fix click" commits land on properties the snapshot already records. Bespoke rules would be redundant; what they do add is noticing when the harness stops watching, so each is pinned to an anchor that must keep appearing in a route where the element actually paints. That last clause is load-bearing: the session-row anchors originally pointed at the chat route, where the fixture keeps the panel collapsed under opacity: 0 — they "matched" records with no visible pixels until the capture started pruning invisible subtrees, at which point the check failed and exposed its own false assurance. They now point at mcp-hub, whose fixture opens the sidebar.

CommitRegressionAnchorRoute
be5e69584titlebar clusters wrapped onto a second rowmaka-shell-topbar-railchat
fd38a37cecomposer frame jumped on focusmaka-composer-inner (resting state only)chat
9aad59740session timestamp stacked above its titlemaka-list-row-metamcp-hub
3dbd68ca7sidebar nav rows had two styling authoritiesmaka-list-rowmcp-hub
9d20a9396task ledger recent-count collided with its labelmaka-session-workbar-countchat
be1406705settings content column painted over the nav railsettingsSidebarsettings-general

Two are not transcribable: 3e57d1951 (stat tile leading) and 400e8f4a9 (turn marker measure) fix files the mega-branch itself created, which do not exist on main. They are recorded here as fragile spots for whichever slice introduces those components.

What review changed

Four review rounds (Codex plus an independent fresh-context agent each time), every finding reproduced before being fixed. The branch is meaningfully different as a result.

Round 1

It could report ok for a window it never launched. The harness rolled its own launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a process-group kill, ...process.env. It never verified the CDP target belonged to the child it spawned, so a leftover Electron on the port was captured instead; reproduced with two concurrent runs both reading the same window, and 270 stale user-data dirs on the dev machine. Now launched through Playwright's Electron support, with the environment builder extracted to scripts/fixture-env.mjs and shared with the E2E suite.

It could not see the class of change the migration makes.boxShadow, textAlign, flexDirection, flexWrap and outline were not captured — a utility that starts beating a product rule after the @layer move can change elevation or text placement with an identical rect. Three INITIAL_VALUES entries guessed at values Chromium does not serialise and never matched once.

Inherited values were compared against the wrong ancestor. A zero-box or display: contents wrapper is skipped, so nothing in the capture carried its values — yet visible children omitted properties that matched it. Now compared against the nearest recorded ancestor, so every omission is recoverable by a reader of the file.

The drag-region resolver was built on a false premise. It parsed stylesheet rules in source order because "the property is absent from getComputedStyle". It is not: getComputedStyle(el).webkitAppRegion returns drag, and window-titlebar.spec.ts has read it that way in CI all along. Deleted, ~50 lines.

The product change had no automated coverage.resolveDockPresentation is now a pure function with tests, and the programmatic smoke layer is pinned by the source contract.

Round 2

The two-step baseline workflow could pass silently when a step was skipped. The checker never rebuilt the candidate, the baseline was a bare array with no provenance, and the gitignored file could not travel between worktrees — comparing a build to itself read as a clean slice. Replaced by the one-command --against compare; the baseline file, the --update flag, and the .gitignore entry are gone with it.

Teardown was unbounded, and CI inherits this launcher.app.close() has no deadline; one wedged launch turned a capture run — or the CI alignment audit, whose job has no timeout — into an infinite hang, reproduced once at ≥600s. The bounded close the E2E suite already owned (grace, then SIGKILL the tree) moved to scripts/electron-lifecycle.mjs and both consumers share it; renderer evaluate gained a deadline; the temp profile is removed even when close fails.

The shared-launcher refactor had silently changed the CI gate's conditions.audit-alignment's settle budget had dropped 2500ms → 1000ms against static-markup readiness, and buildFixtureEnv read process.env.CI inline — which made the "hidden run stays hidden" test pass on every laptop and fail on the Linux CI runner. The settle budget is restored with real per-fixture ready selectors, the window-visibility decision is a pure function of the builder's arguments, and the one ambient read lives in isCiLinuxDisplay for callers to compose.

Two capture-semantics gaps.textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79–85% of records re-recorded an ancestor's value; and descendants of an opacity: 0 element (own opacity 1, non-zero rect) were captured as visible — 105 phantom records under the collapsed session panel. Fixed, plus a static table test pinning every CSS-inherited property to the inherited route in both directions.

The darwin-only matrix contradicted the issue, and its stated justification was wrong. No Windows runner is needed: the fixture platform override flips data-os through the production path on any host. The matrix now includes win32.

Round 3

The base was not isolated. The base worktree borrowed this checkout's node_modules, and the workspace links inside it resolved @maka/* straight back to the working tree — the base bundled and loaded the candidate's own packages, so a slice changing @maka/core would have compared itself against itself. Both reviewers converged on this independently; reproduced byte-for-byte with require.resolve. The base now runs its own npm ci against its own lockfile (cached per commit, guarded by an atomic build lock), and the lockfile-parity gate plus the symlink list that existed to justify the sharing are deleted.

Pseudo-elements were invisible to the contract. 47 ::before/::after rules paint in this renderer — including the full-screen body::after film-grain overlay — and a cascade flip on any of them changed pixels while every element record stayed byte-identical. Painted pseudo-elements are now captured as paint signatures that inherit from their host.

The instrument now proves its own measurement identity. CLI arguments are closed sets; every capture waits for and asserts the requested data-os and theme on the live document before reading styles — the win32 column can no longer silently degrade into a second darwin column if the override seam regresses. Dead omission rules are counted at the raw sample inside the capture (not inferred from the compressed output), so a rule shadowed by the border/outline paint gates still proves itself.

Remaining teardown edges. The smoke gate's SIGTERM paths now escalate through the shared bounded close, and a failed report write no longer leaves a live Electron behind. Verifying that fix surfaced one more: the shared force path kills by process group, the smoke gate's child was not spawned detached, so the group signal hit ESRCH, the terminator read "tree already gone", and a visible window that ignores SIGTERM survived its own SIGKILL stage. The smoke child is now a group leader like every runtime consumer, and the bounded close backs the group kill with a direct kill on the root.

Round 4

The contract could not see the paint-only channels.filter, backdrop-filter, clip-path and transform move pixels without moving the rect — grayscale() on disabled provider rows, the glass theme's backdrop blur, clip-path as visually-hidden, rotate() on square chevrons — so a migration losing one diffed zero. All four are now sampled (plus transform-origin, gated on a transform actually painting), and the capture freezes animations and transitions before reading a single style: a running spinner serialises a different matrix at every phase, which would diff time rather than the cascade. Verified with a negative control — an injected filter rule fails the compare with filter: ∅ → sepia(0.1) — and a clean 20/20 run.

The hit-test's --route was an open set. It accepted any value and failed only when nothing matched, so --route chat --route chatt dropped the typo and exited 0 as "1 route(s) clean" — silent coverage shrink on a tool whose whole point is that coverage is a number. Each value is now validated against the closed route set (as the visual contract already does), and the CLI's exit codes are pinned by subprocess tests.

Two smaller honesty fixes. Both direct-run guards compare through pathToFileURL — a checkout path with a space or non-ASCII segment percent-encodes in import.meta.url, and the string-concatenation compare would have made the instrument exit 0 having measured nothing. The tool header still described the rejected node_modules-sharing base build; it now states the actual isolation model. The direct-kill backstop from round 3 gained a fake-based test alongside its live-window proof.

Verification

Run on main (ae43cb291) + this branch, macOS arm64.

npm run lint ✓
npm run format:check ✓
npm run typecheck ✓
npx knip ✓ exit 0 (2 config hints, same as main)
npm run test:scripts ✓ 209 pass, 0 fail
npm --workspace @maka/desktop run test:dist ✓ 2981 pass, 0 fail
node scripts/audit-alignment.mjs ✓ 12/12 fixtures clean
node scripts/check-visual-contract.mjs ✓ 20/20 comparisons clean against main
(fresh base: worktree + npm ci + build, all by the tool;
negative control: an injected filter rule fails the compare)
node scripts/check-hit-test.mjs ✓ 5/5 routes clean
npx playwright test first-run window-titlebar electron-lifecycle
✓ 7 passed
npm --workspace @maka/desktop run smoke:programmatic-window
✓ PASS incl. programmatic-dock-visible (was FAIL on main)

The real-window smoke launches the app with MAKA_E2E_FIXTURE, which makes
main.ts compute startHidden and main-window.ts keep the window hidden for
its whole lifecycle. The gate therefore had nothing to look at: on a clean
main its own programmatic-window-visible check reports visible=false and
exits 1, and every human check it prompts for — dragging window edges and
corners to resize, dragging the titlebar to move, Tab/Shift+Tab traversal
inside the modal — is unrunnable against an invisible window.
Set MAKA_E2E_SHOW_WINDOW on the launch env, which is exactly the escape
hatch main.ts already provides for runs where there is no focus to steal.
Capture runs and CI e2e do not set it and keep their hidden-window
behavior unchanged.
Hiding the dock icon makes the run an accessory app. Contrary to what the
migration tracker assumed, such a window is fully clickable — measured on
darwin: with MAKA_E2E_SHOW_WINDOW set, the fixture window reports
isVisible/isFocused true and takes clicks, drags, and keyboard input
normally. What it does not have is a Dock tile or a Cmd+Tab entry, so a
reviewer doing manual visual comparison who switches to another app has no
way back to it.
Key the dock icon off the same startHidden that decides whether the window
shows at all, instead of re-deriving the condition from MAKA_E2E_FIXTURE.
The two can no longer drift, and MAKA_E2E_SHOW_WINDOW now opts out of both
halves of staying out of sight. Capture runs and CI e2e leave the variable
unset, so they keep hiding the window and the dock exactly as before.
Lock it with a dock diagnostic and a programmatic-dock-visible check in the
real-window smoke, which reported dockVisible=false before the change and
true after.
A migration that must stay visually neutral needs to look at the running
app constantly, but the only way to boot a route with fixture data was the
smoke gate, which insists on walking a reviewer through twelve checklist
prompts and filing a report every time.
Add --manual: same launch path, then stop. The window stays up until it is
closed or interrupted. Reusing the gate's launcher keeps the window a
reviewer inspects identical to the window the gate checks, and leaves one
place that knows how to boot a fixture instance.
Expose it as `launch:fixture`, and lift the build chain the smoke scripts
repeated verbatim into `build:with-deps` rather than copying it a third
time.
#1565 PR 0. The mega-branch produced regressions on screens nobody edited,
and slicing alone does not localise them: the signal has to come from a
baseline that says what the app looked like before. This captures one.
check:visual-contract walks the five representative fixture routes in light
and dark, records every visible element's box, paint, and alignment
properties, and diffs the result against committed JSON. The alignment set
(align-items/justify-content/place-items/gap/grid-area) is there because
flex and grid misalignment is exactly the class a paint-only property set
cannot see. It gates no-change, not correctness: a pre-existing bug on main
stays out of scope, and zero diff means the migration did not move this
element, never that the element is right.
The boot sequence audit-alignment.mjs already worked out — spawn Electron
with MAKA_E2E_FIXTURE, wait for a CDP page target, evaluate in the renderer
— moves to fixture-cdp.mjs so both scripts share one launcher instead of
growing a second. It also gains what a contract harness needs and an audit
could do without: animations and webfont reflow frozen before capture, an
evaluate deadline so a wedged renderer fails loudly instead of hanging, and
process-group teardown so Electron's helpers do not pile up and starve
later windows.
Also carries #1565's third PR 0 item, salvaging the mega-branch's late
regression fixes. Six of the eight land on properties this snapshot already
records — a wrapped titlebar row, a timestamp stacking above its title, a
missing gap, a column painting over the settings rail all move a rect or a
recorded property, so the diff catches them without bespoke rules. What
bespoke rules do add is noticing when the harness stops watching, so each
one is pinned to an anchor that must keep appearing in the baseline. The
remaining two fix files the mega-branch itself created, which do not exist
on main; they are recorded in the PR body instead.
Determinism was verified by capturing the same build twice. The first
attempt was not clean, and both causes are excluded rather than tuned
around: element labels fell back to el.id, which Base UI regenerates every
render, and OverlayScrollbars' chrome fades with pointer activity, so its
visibility was a coin flip. Baselines stay at 1.5MB rather than 4MB because
properties equal to their initial value, or inherited unchanged from the
parent, are omitted and read back as unchanged.
Migration-only: deliberately outside CI, following check:chat-visual, and
removed in PR 14 with the maka.legacy layer.
#1565 PR 0. The mega-branch's most expensive regressions were controls that
render perfectly and refuse clicks. A computed-style snapshot cannot see
that, so it gets a contract of its own: every visible interactive element on
the five routes must be reachable at its centre, must not sit under a
pointer-events or visibility trap, must not be transparent through an
ancestor, and must not fall inside an -webkit-app-region: drag area.
Three parts of #1565's proposal did not survive contact with a clean main,
and each is replaced by something answering the same question:
The drag-region probe was specified as a real click plus a side effect.
Fixture windows are hidden, and an unmapped window does not route
synthesized input through the browser's hit-testing path, so every target
read as swallowed. Making the window visible to fix that turned the probe
into something that fires real product actions — one opened a native file
dialog and hung the run. Reading -webkit-app-region off the stylesheet rules
instead has no side effects: the property is missing from getComputedStyle,
but not from the rules that declare it.
All five probe points were specified as required. On a clean main that is
unreachable: sibling chrome legitimately covers a couple of edge pixels, so
a workbar resize handle would indict a control nobody struggles to click.
The centre is now required and corners corroborate — three lost corners
means something covers the control.
The overlay guard keyed on [role=dialog][open], which a React-rendered
div[role=dialog] never has, so the settings surface went undetected and
every control behind it was reported unhittable. It now keys on the
product's own data-modal="true".
Geometry probes do need a visible window — elementFromPoint disagrees with
getBoundingClientRect against a hidden one — so this check steals focus for
a few seconds per route. Memoising style and rect lookups took the densest
route from a 60s timeout to 3s, and routes are spaced so one window is fully
gone before the next boots.
All five routes are clean on ae43cb2.
@Astro-Han
Astro-Han marked this pull request as draft July 30, 2026 09:45
Review found the harness could report `ok` for a window it never
launched, could not see the class of change the migration actually
makes, and shipped its only product change with no automated coverage.
Launch through the shared E2E seam. The harness rolled its own Electron
launcher — fixed debug ports, a /json/list poll, a raw WebSocket, a
process-group kill, `...process.env` — all of it already solved next
door in `apps/desktop/e2e/fixtures.ts`, and each hand-rolled version
worse. It never verified the CDP target belonged to the child it
spawned, and the port allocator restarted at 14600 in every process, so
a leftover Electron on that port got captured instead. Reproduced: two
concurrent runs requesting different scenarios both read the same
window. When the leftover is the same route from an older build — the
shape you get by re-running one route after an edit — the contract
reports `ok` for a stale window. `detached: true` with no signal handler
supplied the leftovers; this machine had 270 stale user-data dirs, none
ever removed. `buildE2eEnv` moves to `scripts/fixture-env.mjs`, shared
by both callers; its own comment already described the bug the harness
had, since inheriting the environment leaks `VITE_DEV_SERVER_URL` and
loads the dev server instead of the build under test. Launch, readiness
and teardown now go through Playwright's Electron support: fixed sleeps
become per-route readiness selectors, and teardown removes its temp dir.
235 lines become 139, and the hit-test retry goes with them, per the
sibling harness's rule that flakes should fail loudly.
Stop committing baselines. They encode the capturing host — font metrics
and the macOS traffic-light inset — so one machine's baseline reads as
thousands of diffs on another. Capture on the branch you compare
against, apply the slice, compare: the two captures that matter always
come from the same host. The timezone is pinned through the fixture's
IANA override for the same reason. This removes 70k lines from review
and makes the orphan-baseline check moot.
Record what a cascade flip actually changes. `boxShadow`, `textAlign`,
`flexDirection`, `flexWrap` and `outline` were missing, so a utility
that starts beating a product rule could change elevation or text
placement with an identical rect and be reported clean. Three
`INITIAL_VALUES` entries guessed at values Chromium does not serialise
and never matched once, leaving constants on all 3,964 records;
`findDeadOmissionRules` now fails the run when a rule stops matching —
it caught a fourth such entry immediately. Inherited values were
compared against the parent rather than the nearest recorded ancestor,
so a zero-box wrapper's color could change, repaint every visible child,
and leave the capture byte-identical: 129 masked values on the chat
route alone.
Delete the second drag-region resolver. It parsed stylesheet rules in
source order, ignoring specificity, `!important` and `@layer` — which
PR 1 and PR 2 introduce — on the premise that the property is absent
from computed style. It is not: `getComputedStyle(el).webkitAppRegion`
returns `drag`, and `e2e/window-titlebar.spec.ts` has read it that way
in CI all along, against rendered geometry and document order. The
hit-test keeps what that spec does not cover, and now reports how many
elements it actually probed rather than how many the selector matched.
Pin the product change. `resolveDockPresentation` is a pure function
with tests; the branch it replaced could only be exercised by launching
Electron, which is why it had none. The programmatic smoke checks are
now pinned by the contract test that exists to pin checks — including
the new dock one, which could previously be deleted with CI green — and
`e2e` shares `build:with-deps` instead of repeating the chain verbatim.
…budget
buildFixtureEnv read process.env.CI inline, so the fixture-env test
asserting a hidden run stays hidden passed on every laptop and failed on
the Linux CI runner. The ambient read now lives in isCiLinuxDisplay for
callers to compose explicitly.
audit-alignment gets its 2500ms settle back (the shared-launcher refactor
had silently dropped it to 1000ms against static-markup readiness) plus a
real ready selector per fixture, and its header now states the deliberate
environment change from the raw-spawn launcher it replaced.
withFixtureWindow tore down with a bare app.close(), which has no
deadline — one wedged launch turned a capture run (or the CI alignment
audit, which has no job timeout) into an infinite hang, and the temp
userData dir leaked because rm was sequenced after the close.
The bounded close the E2E suite already owned moves to
scripts/electron-lifecycle.mjs — the same shared-home precedent as
fixture-env.mjs, so a bare-node harness and the Playwright suite stop
splitting one launch concern across two directories. Renderer evaluate
gets a deadline too; teardown then reclaims the process either way.
Two measurement-semantics gaps in the visual capture, both instances of
a class the harness had already been bitten by:
- textAlign was in PROPERTIES but not INHERITED_PROPERTIES, so 79-85%
of records re-recorded an ancestor's value and one container change
printed as N descendant lines. It moves to the inherited omission (and
out of INITIAL_VALUES — absent must mean exactly one thing). A static
table test now pins every CSS-inherited property to that route, which
is the guard findDeadOmissionRules could never provide.
- Descendants of an opacity:0 element report opacity 1 and a non-zero
rect, so the collapsed session panel put 105 invisible records into
the chat capture; a change under it would diff on pixels that never
paint. The walk now prunes zero-opacity subtrees.
The prune exposed that two salvaged anchors were matching those phantom
records: the chat fixture never shows the session rows at all. They now
point at mcp-hub, whose fixture opens the sidebar for real.
Replaces the two-step gitignored-baseline workflow. The checker now
builds the base ref in a cached temp worktree (node_modules shared;
the compare refuses to run across a package-lock change), builds the
working tree, captures every route x theme x platform from both builds,
and diffs in memory. Every step of the old workflow a human could skip
- rebuild after switching, recapture after rebasing, stash on a
committed slice - produced a zero-diff pass that had measured the same
binary twice; now the instrument owns the whole measurement and there
is no baseline file to trust or to stale.
The matrix gains the win32 column #1565 asked for: the fixture platform
override drives the production app:info -> data-os path, so the per-OS
cascade is captured on any host - no Windows runner involved.
desktop-real-window-smoke was the last launcher still spreading
process.env wholesale: a developer with npm run dev open smoked the dev
server instead of the build the script just made, and the run touched
the real $HOME (#1517's skills-deletion surface). It now launches
through buildFixtureEnv with an explicit visible window.
The dock contract drops its duplicated fourth case and pins the wiring
instead - the regression this module exists for was the call site
re-deriving startHidden, which no pure-function test can observe. The
reveal contract follows fixture-env's pure shape (the ambient CI read
now lives in isCiLinuxDisplay, composed at each call site).
…entity
Round-3 review found the compare's base was not a base: the borrowed
node_modules resolved @maka/* through workspace links straight back to
the working tree, so the base bundled and loaded the candidate's own
packages — a slice changing @maka/core would have compared itself
against itself. The base now runs its own npm ci against its own
lockfile in the cached worktree (atomic lock against concurrent
builds; the marker name encodes the scheme so old symlink-era caches
rebuild). The lockfile-parity gate and the symlink list existed only
to justify the sharing; both are deleted, and a slice may now
legitimately change dependencies.
The same round showed the instrument asserting nothing about what it
measured, so now it proves it: route/theme/platform are closed sets;
every capture waits for the requested data-os and theme on the live
document before reading a style; painted ::before/::after get paint
signatures (47 pseudo rules paint in this renderer, led by the
body::after film grain, and a cascade flip on any of them was
invisible); mixBlendMode joins the property set; and dead omission
rules are counted at the raw sample where the border/outline gates
cannot shadow them.
The smoke gate still had two unbounded exits: --manual waited forever
on 'exit' after one SIGTERM, and the report path fired SIGTERM and
called process.exit without waiting or escalating — plus a failed
report write skipped the kill entirely. All paths now stop through the
shared bounded close (SIGTERM as the graceful phase, 5s grace, SIGKILL
the tree), inside a finally.
electron-lifecycle's import of @maka/runtime now fails with an error
that says what to build; on a fresh clone the raw ERR_MODULE_NOT_FOUND
surfaced from four files deep in the import chain.
The shared teardown's force path kills by process group. The smoke gate
spawned Electron without detached, so the group signal hit ESRCH, the
terminator read that as "tree already gone", and a visible window that
had ignored SIGTERM survived its own SIGKILL stage. Spawn the smoke
child as a group leader like every runtime consumer does, and back the
group kill with a direct kill on the root so a future non-leader child
still dies inside the 2s exit deadline.
filter, backdrop-filter, clip-path and transform move pixels without
moving the rect — grayscale() on disabled provider rows, the glass
theme's backdrop blur, clip-path as visually-hidden, rotate() on square
chevrons — so losing one was invisible to every property the contract
sampled. Capture all four (plus transform-origin, gated on a transform
actually painting), and stop the renderer's clock first: a running
spinner serialises a different matrix at every phase, which would diff
time rather than the cascade. Verified with a negative control (an
injected filter rule fails the compare) and a clean 20/20 run.
Also correct the header, which still described the rejected
node_modules-sharing base build, and compare the direct-run guard
through pathToFileURL so a percent-encoding checkout path cannot turn
the instrument into a silent exit-0 no-op.
--route accepted any value and only failed when nothing matched, so
`--route chat --route chatt` dropped the typo and exited 0 as "1
route(s) clean" — silently shrinking the requested coverage. Validate
each value against the closed route set (as the visual contract already
does), guard the entry point through pathToFileURL, and pin the CLI's
exit codes with subprocess tests.
The ESRCH-swallowing group-kill path was only proven against a live
window; the fake now exercises it too — a terminator that returns
without killing must be followed by a direct SIGKILL on the root.
@Astro-Han
Astro-Han marked this pull request as ready for review July 30, 2026 13:04
@Astro-Han
Astro-Han merged commit 71e47a6 into mainJul 30, 2026
3 checks passed
@Astro-Han
Astro-Han deleted the refactor/1565-pr0-contract-harness branch July 30, 2026 13:04
Sign up for freeto 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

@Astro-Han