feat(theme): ordered environmental adaptations in defineTheme - #5543

* fix(ci): own canonical visual frames by component, not story title
Two holes in the visual gate, both from how a plan decides what it may
compare.
A published Core component whose only story is titled under another group
lost every canonical frame, with no way to seed one: the release plan, the
trusted PR plan, and therefore the manual baseline workflow's capture all
filtered stories by their Storybook title group. Ownership now comes from
the component source the Storybook index records for a story, so a Core
component keeps its frames wherever its story is titled, while a composed
demo that merely imports Core still owns none. The title group stays the
fallback for stories that declare no component at all, which is the case
the group filter was written for. On the current index this restores
exactly one component and removes nothing.
An empty plan also reported clean outside the trusted lane. A plan with no
shots captures nothing and compares nothing, so passing it is reporting the
absence of evidence as evidence. Every lane that captures pixels now
refuses one, in the same words, and names the manual baseline workflow as
the way to seed the missing frames.
Tests cover both: canonical ownership resolved from the index in either
direction, and each lane's refusal of a zero-shot plan.
* fix(ci): make the spec-owner gate fail closed on the exact head
Three PRs merged with `spec-owner-approval` pending, and two of them then
showed "Approved by @cixzhang" for a head that had already merged. Each
owner comment was literally `/approve-spec` with no SHA.
Repository-side defects behind that record:
- The comment trigger required a trailing space, so a bare `/approve-spec`
 skipped the job entirely. The owner got no reply, so an inert command
 looked like an approval. The trigger now admits any owner-command shape
 and the reconciler answers a near-miss once per head with the exact
 command to copy.
- The parser trimmed and lowercased what the trigger matched raw, so an
 indented or capitalized command parsed as valid while never dispatching —
 and, because decisions read every comment, could have counted as approval
 in a run started by some other event. Both now share one
 `isDispatchableOwnerCommand` precondition, and a contract test derives the
 trigger's prefixes from the parser's own list. The SHA argument itself is
 accepted in either casing, so the parser and the help validator agree on
 every recognized command.
- A `ready_for_review` event from any owner published a self-attestation
 that satisfied every owner group. `.github/DESIGNOWNERS` grants that
 self-attestation to design owners for the design group only, so it is now
 published only for a DESIGNOWNER author, read only by that group, and read
 back only for a handle still in that file — markers predating this rule
 (live example: `spec-owner-ready/imdreamrunner` on #5543) authorize nothing.
- A run that started after the merge still published a decision on the
 merged head. Every gate-status write, including the restore that yields to
 a newer run, now refuses a settled or moved head. Terminal publication
 reads the live pull request as the last call before the write and — since
 GitHub has no conditional status write — verifies afterwards, reporting a
 status that raced a merge as unverified and stopping before auto-merge.
`workflow_dispatch` gains a `backfill` input that publishes the status and
returns before auto-merge, so the heads that predate the required check can
be given one without landing a pull request nobody asked to land.
This does not by itself block a merge: `spec-owner-approval` is not in the
required status contexts on `main`, which is why a pending gate did not hold
those PRs. `.github/REVIEW_GATE.md` records the ordered backfill-then-require
sequence, verified against the 7 open heads that currently lack the context.
Test plan: 105 spec-owner decision, reconcile, and workflow contract tests,
including mutation checks that each new guard is load-bearing;
`pnpm check:knowledge`; `pnpm check:repo`; prettier; actionlint on
`.github/workflows/spec-owner-gate.yml`.
No Changeset: repository workflow policy only.
* fix(cli): report matched result totals, not the cap
`build.kit`'s `matchCount` and a recorded run's `output.resultCount` were
both the length of the result list AFTER `--limit` had cut it, so a query
matching two hundred things and one matching exactly twenty filed the same
number. Nothing downstream could tell a capped answer from a complete one,
and a thin kit read as "the package has nothing" when the cap had hidden
the rest.
`search()` now returns `matchCount` — the size of the ranked set the limit
was applied to — and `build` reports that total instead of counting its own
slice. The payloads are unchanged: `results` is still bounded by `--limit`,
and the kit still caps at 3 pages / 5 blocks / 6 components. The text view
mirrors the JSON, saying `Results for "x" (3 of 47)` only when the list was
actually cut short.
Test plan:
- vitest run packages/cli/api/search packages/cli/api/build
- vitest run packages/cli/clients/cli/commands/search.test.mjs
 packages/cli/clients/cli/commands/debug-result-summary.test.mjs
- pnpm -F @astryxdesign/cli readme:check
- astryx --json search button --limit 3 -> matchCount 239, results 3
* fix(cli): stop recording a raw agent session id, and scrub the env snapshot
A DebugEvent claimed `redacted: true` while `env` had never been through the
scrubbing pass, and it stored the raw `agentSessionId` beside its hash. A
session id is a stable identifier for the person running the CLI, and a
handler may forward these records anywhere — so every record shipped that
identifier, and an agent name pasted in from the environment went out
verbatim, both under a flag that said neither had happened.
The contract is now stated on `DebugEventEnv` and enforced:
- `env.agentSessionId` is always null. `agentSessionIdHash` is the join key,
 which is all the raw value was ever used for.
- `env` goes through `redactEnv`, an explicit classification: fields this CLI
 derives from a fixed vocabulary stay verbatim (a scrubbed platform or hash
 is worthless), and everything else is scrubbed like argv. The allowlist is
 positive, so a field added later is scrubbed until someone decides.
- `redacted` is set only on the sealed copy, after every pass has run. An
 in-flight event now says `false`, because it is.
- `parseDebugEvent` enforces the same rule on the way back in, per version: a
 v1 record may carry the raw id (that is what v1 meant), a v2 record may not
 and is rejected if it does. A record that reaches a reader from a warehouse
 or a hand-edited file cannot smuggle the identifier past the boundary that
 exists to keep it out.
`DebugSchemaVersion` widens to `1 | 2` and the CLI emits 2, so a consumer
switching on it is forced to handle both instead of silently grouping every
run under a null session id. Not a breaking release: `debug` and the whole
DebugEvent surface are unreleased and land together, so no published
consumer ever saw the raw identifier.
Test plan:
- vitest run packages/cli/foundation/debug packages/cli/authoring
- new foundation/debug/privacy-contract.test.mjs covers all four promises
- new authoring/debug/parse.test.mjs covers the validator per version:
 v1 with a raw id parses, v2 with one is rejected naming the field and the
 hash to use instead
- pnpm -F @astryxdesign/cli typecheck:authoring (drift-lock intact)
- real run with ASTRYX_AGENT_SESSION_ID set: schemaVersion 2, redacted true,
 agentSessionId null, hash present, raw value absent from the whole record
* fix(cli): let the declared packageManager outrank a stray lockfile
One `yarn install` inside a pnpm project leaves a yarn.lock behind forever.
A single lockfile used to outrank the `packageManager` field, so the CLI
answered "yarn" for a project that says pnpm and printed `yarn astryx ...` in
every command it suggested — including the invocation line written into
agent docs, where agents copy it. `astryx doctor` called that setup healthy,
so nothing ever surfaced the contradiction.
The declaration now decides, whatever lockfiles sit beside it, and the
resolution carries the reasoning: `declared`, `source`, and the
`strayLockfiles` it ignored. Every documented fallback is unchanged — with
nothing declared a single lockfile still answers, a committed
pnpm-workspace.yaml / .yarnrc.yml / bunfig.toml still breaks a tie, an
unbroken tie is still the neutral `npx` plus a doctor FAIL, and the runner
is still consulted only after the whole walk finds nothing.
`astryx doctor` now WARNs on the contradiction, names the lockfile, and says
what to delete.
Test plan:
- vitest run packages/cli/foundation/env/package-manager.test.mjs
 packages/cli/api/doctor
- regressions: declaration beats a single stray lockfile; beats lockfiles
 that exclude it; stray list reported; every fallback pinned
- real project (packageManager pnpm + stray yarn.lock): doctor warns and
 names yarn.lock; invocation prints `pnpm exec astryx`
* fix(markdown): render a streamed line whose only pipes are escaped
A streaming Markdown line is held back while it could still be an
unfinished table header, so no partial pipe syntax flashes on screen.
That test asked whether the line contained `|` at all, so `Costs 5 \| 10`
— where the pipe is escaped literal text, never a cell delimiter — was
suppressed too. When the line was the whole document, nothing rendered
at all.
Classify the trailing line by whether it holds an *unescaped* pipe.
Genuine partial table syntax is still suppressed, an established table
still streams rows containing `\|`, and the header lookup keeps mirroring
the block parser's own `includes('|')` so a header of escaped pipes still
establishes its table once the separator arrives.
The regression tests walk every prefix of a streamed line and assert the
tail reads back exactly, with and without settled text above it, so a
future suppression rule cannot blank it again unnoticed. Parsing stays
bounded to the stream tail.
* fix(ChatComposerInput): place the caret deliberately on programmatic focus
Clicking the composer's padding focuses the editable through
ChatComposer's body-click handler, which called `focus()` and left the
caret wherever the engine put it. Measured in Chromium against the real
helpers: `focus()` collapses the caret to offset 0 — the START of the
draft — whether the editable is empty or not, and whether or not a
selection existed before. That is the one position where ArrowUp means
"recall history", so the first ArrowUp after clicking the padding
replaced whatever the user had typed.
The composer states the caret itself instead, and the two focus paths
want different things:
- The shell's click-to-focus lands after the draft. Clicking the space
 after the text means "put me there", so it overrides a stale caret.
- The imperative `handle.focus()` preserves a caret or selection the
 user already has inside the editable, since a consumer returning them
 to the composer must not move them. The selection is captured BEFORE
 focusing — afterwards the engine's own offset-0 caret is
 indistinguishable from theirs — and restored after. With nothing to
 preserve it falls back to the end, so the original bug cannot return
 through this path either.
The public handle contract is unchanged; the end-placing variant is
internal to the shell wiring.
History recall then follows the caret in every engine: an empty composer
is at its start and its end at once, so ArrowUp still recalls; a pending
draft has the caret at the end, so ArrowUp moves the caret. A caret the
user placed themselves is honored — a start-of-draft caret still
recalls, and the stashed draft still comes back on ArrowDown.
`ensureCaretInside` stays as the weaker fallback for a caret we never
placed, and the module docs no longer claim browsers create no Range on
focus — they do.
* fix(vega): honor the inert data contract and stop needless View rebuilds
`data` is documented as initial dataset values, read once when the View
is built and never reactive — but it sat in the Effect's dependency
list, so the documented usage (`data={{table: rows}}` written inline)
tore the View down and rebuilt it on every parent render, discarding the
chart's zoom, hover, and signal state. The same held for an inline
`spec` or options object: a new reference each render, an identical
runtime each time.
Read `data` through an Effect Event, so it is exactly what the docs
promise, and rebuild on a change of VALUE in the five lifecycle props.
Value, not reference — and not "differs from the previous props"
either. A caller that keeps its spec in a ref or a module constant and
edits it in place hands both renders the same object, so a
props-to-props comparison has nothing left to compare and the chart goes
stale; that would also regress the old reference check, which at least
rebuilt when the top-level object was replaced. So the latch keeps a
structural copy of the values the live View was built from, and compares
incoming props against that: an equivalent inline literal keeps the
View, an in-place mutation rebuilds it.
Two shapes cannot be copied, and each is kept BY REFERENCE rather than
reported as always-changed: a reference cycle, and nesting past 100
levels. Always-changed would be a crash, not a slow path — the latch is
refreshed during render, so a part that never compares equal re-renders
forever ("Too many re-renders") and a cyclic spec would never paint.
Cycles are detected by tracking the ancestor path, so they cost nothing
and hide nothing: a cycle's re-entry edge points back at an object the
walk already copied, so mutations in a cyclic spec are still caught.
Past the depth bound an in-place edit is invisible until the caller
passes a different object — the one real limitation, documented in the
README and pinned by a test.
Functions and class instances still compare by reference, their behavior
living in methods a copy cannot capture. Vega still receives the
caller's own objects; the copy is only ever the comparison's memory.
`@astryxdesign/vega` is private and canary-only, so no changeset.
* fix(eslint-plugin-astryx): read transforms conservatively in the RTL centering checks
`no-physical-properties` decides two things from a sibling `transform`:
whether renaming a physical `left` would break RTL centering, and whether a
logical 50% anchor's horizontal translate is mirrored under RTL. Both rested
on a scan that could only see `translate(` and `translateX(`, that read a
transform list as an unordered bag of functions, and that answered
"compensated" whenever it could not compare two values.
- `left: '50%'` beside `translate3d(-50%, ...)` or `matrix(1,0,0,1,-50,0)` was
 AUTOFIXED to `insetInlineStart`, silently breaking the centering it was
 written for. The transform is now split into its function calls, and a
 horizontal translation the rule cannot rule out — a matrix, a scaleX, an
 interpolated argument, two translations, unreadable syntax — withholds the
 fix and reports `inlineCenteringUnknown`.
- A transform list COMPOSES, and order decides direction:
 `rotate(90deg) translateY(-50%)` rotates the axes first, so the translation
 that follows moves the element sideways by half its height. Reading the
 functions as an unordered bag called both harmless and autofixed the anchor.
 Functions are now read in order, and everything after a rotate or a skew is
 opaque, while `translateY(-50%) rotate(45deg)` — which spins the element in
 place after translating on the axis it was written for — still reads clean
 and still autofixes.
- `calc()`, `var()`, and a unit mismatch (`-50%` against `50px`) all passed as
 an RTL reversal, so real mismatches went unreported. A reversal is now
 recognised only in a plain negated length with a matching unit; a pairing
 that can be neither confirmed nor refuted reports
 `logicalCenteringUnverified` rather than being cleared or condemned.
The relationship diagnostic still judges only a literal 50%/-50% anchor, so no
new shape of code comes into its scope — the wider net is used solely to
withhold the autofix, where a wrong guess rewrites working code.
`prefer-center-inline` reads the same statuses and now stays quiet about
compensation it cannot verify.
* test(vitest): run the sandbox palette suite in the node project
`apps/sandbox` carried its own `vitest.config.ts`, and the root config's two
projects — the only thing `pnpm test` runs, and so the only thing CI runs —
did not include it. The 18 palette-generator tests therefore belonged to no
project and no job: nothing in CI ever executed them, and nothing failed when
they broke.
They are pure modules with no DOM, so they join the `node` project's include
list rather than gaining a second runner, and the app's own config is removed
so nothing shadows that routing. `vitest list` shows each file exactly once,
under `[node]`.
The app keeps a `test` script, now forwarding to the root config the way
`packages/cli` and `internal/vibe-tests` do (`vitest run --root ../..` scoped
to its own path). That is what the page's README tells contributors to run,
and a filtered script that does not exist is not an error: pnpm exits 0,
prints nothing, and runs no tests. Forwarding costs no duplicate execution —
CI runs the root `pnpm test`, never per-package scripts.
Both halves of that contract are now asserted in `src/test-routing.test.ts`,
each verified to fail when its regression is reintroduced: the script exists,
it forwards to the root config, no app-level Vitest config shadows the root
projects, and the README documents the command that actually exists.
The `node` project does not extend the root config, so it carries its own
`resolve.alias` for the theme packages the corpus imports: those resolve to
`dist/`, which no test run builds, and the bare specifier now points at the
authored source. Subpath imports (`/built`, `/theme.css`) are untouched.
* test(vibe-tests): stop the pattern escape hatches reading prose as code
#5856 made `hardcoded-important` syntactic because a lexical scan failed runs
for prose: an executor told by the guidance itself not to reach for
`!important` wrote that down, and the check failed it for saying the right
thing. The two hatches still matched by pattern had the same fault. A JSDoc
block explaining that `all: unset` would take the host apart trips
`blanket-reset`; a comment recording that the host's own `color-scheme: light`
arm was deliberately left alone trips `dark-mode-disabled`, whose semantic
reading covers paired mode arms but not the sentence beside them.
Rather than teach each pattern to recognize a comment, the comments are
located once by the parser that owns the file — postcss for stylesheets, the
TypeScript parser for scripts — and blanked out of the line before any pattern
sees it. Blanking preserves offsets and line boundaries, so a hatch written on
the same line as the comment about it is still found and still reported on its
own line.
Markup is WALKED rather than scanned, because where a comment can begin is the
whole point. `<!--` opens a comment only in markup text: inside `<script>` and
`<style>`, and inside a tag's attribute values, those four characters are
ordinary content. Scanning for the delimiters would exempt them anywhere —
which is not a false positive but a way past the check, since
`const c = '<!-- all: unset -->'` would be blanked while
`style.cssText = c.slice(4, -3)` still applied the reset. Each raw-text
element's body is handed to the analyzer for its own language instead, where
only a real `/* ... */` or `//` counts.
Nothing is exempted on a guess: an extension with no analyzer, and a file its
analyzer cannot read, report no comments at all, leaving every character
subject to the scan. Only comments are exempt — a string, a template literal,
an attribute value, or JSX text that names a hatch still counts, which is why
`//` inside a URL cannot blank the code after it.
* fix(vibe-tests): stop an override hiding behind markup comment delimiters
`hardcoded-important` could be defeated by writing the override inside a
comment-shaped string and reassembling it:
 <script>
 const cloak = '<!-- color: red !important -->';
 document.body.style.cssText = cloak.slice(4, -3).trim();
 </script>
The host gets the override; the check reported nothing. Two separate faults
had to line up, and both are fixed here.
First, the markup analyzer blanked every `<!-- ... -->` before handing the
document to its per-language parsers. But `<!--` opens a comment only where
markup TEXT can appear: inside `<script>` and `<style>`, whose content is
JavaScript and CSS, and inside a tag's attribute values, those four characters
are ordinary content. Blanking them wherever they occurred was not a false
positive, it was the way past the check. The document is now WALKED, and each
region is read as what it is.
Second, a declaration was only recognized at the start of the text or just
after `;{}`, so ANY prefix defeated it — the `<!-- ` above, but equally
`'XX color: red !important'`, and in a plain `.ts` file where no markup is
involved. Dropping that anchor alone would have failed runs for prose, because
guidance spells a declaration too ("Never write `color: red !important`"), and
reporting that is the exact fault #5856 fixed. So a declaration in a script now
counts on EVIDENCE that something applies it: the literal sits in a CSS sink (a
`style.*` assignment, a style-object entry, a JSX `style`, `setProperty`,
`setAttribute('style', ...)`, `insertRule`), or it is bound to a name such a sink
reads — which is what catches the cloak, since `el.style.cssText =
cloak.slice(4, -3)` reads `cloak`. A literal carrying a whole braced RULE is a
stylesheet on its face and still counts wherever it is written. Templates are
held to the same requirement: backticks around guidance do not make it CSS.
That reachability is keyed on the BINDING, resolved through the scope chain,
not on the identifier's name. Two functions may each declare `value`; only the
one whose `value` a sink reads is applying anything, and a name-keyed check
reports the other — the prose false positive again. Block scoping for
let/const/class, function scoping for var and function declarations, plus
parameters, is walked directly, since these literals are parsed without a
TypeChecker to ask. A name declared twice — `var v` redeclared — is ONE
binding, so every CSS write to it is reported rather than guessing which write
the sink sees; which one it is depends on flow, and an `if` would otherwise
defeat the check.
Two more places the analysis had to read the syntax rather than a shape:
`el.style['cssText']` sets the same property as the dotted spelling, so member
access is read either way; and a reference is only a use of the VALUE when the
member chain ends in a call, so `cloak.slice(4, -3)` and `parts[0].slice(...)`
count while `guidance.length` deciding a branch does not.
The markup walk matches the raw-text tag names EXACTLY. `<style-note>` is an
ordinary custom element, but a prefix test read it as a `<style>` opener and
then hunted for a `</style>` that never comes — swallowing the rest of the
document, so every attribute, comment and override after it went unexamined.
The walk is `setup-markup.mjs`, shared with `setup-comments.mjs` rather than
written twice: both files were asking the same structural question — where the
comments are, where the CSS is — and two scanners answering it separately is
how they drift apart. The comments module loses its own copy and reads the
same regions.
The walk TOKENIZES each tag rather than pattern-matching its attributes,
because a pattern gets both halves of "is this a style attribute" wrong. The
name must be exactly `style`: `\bstyle\s*=` also matches `data-style=` and
`my-style=`, and those were reported as overrides though the browser never
applies them as CSS. The value may be double-quoted, single-quoted, or
UNQUOTED — `style=color:red!important` is valid HTML, and a quoted-only
pattern missed it entirely, which is a real override going unseen. Tokenizing
also fixes the tag boundary, so a `>` inside a quoted value no longer ends the
tag early.
Also closes a gap the sink analysis exposed: `setProperty('color', 'red
!important')` carries the flag in its VALUE argument, which neither the
declaration branch nor the property-name branch reached.
Real markup comments remain exempt, including one that wraps a script or a
style block. A `style=` written in text content, or inside another attribute's
value, is not an attribute. Guidance prose naming the flag is silent, and the
file header records the one gap left standing: a declaration split across
separate variables is not followed, which is real dataflow analysis.
* ci: split the test jobs into parallel lanes in both workflows
Both workflows that run the suite were over the runner's execution budget.
ci.yml's `test` ran both Vitest projects in one job on a 2-core runner and was
terminated at around 20 minutes with no assertion output and no summary — so
the log named no failing suite, and a red check said nothing about what broke.
`main` was already at ~19m15 before this branch's 22 new tests, so the budget
was the defect rather than any one test, and retrying or trimming tests would
only move the cliff.
deploy.yml's `test` — main's post-merge push gate — carried strictly more on
the same runner: the same suite, plus a full `pnpm build`, plus eight
typecheck gates. Same defect, later surface.
The two projects are independent by construction (vitest.config.ts: `ui` is
jsdom + StyleX, `node` is everything else), so running them as separate jobs
costs no duplicate execution — every file belongs to exactly one project.
Locally the node project runs 570s against the ui project's 244s, so node
takes `4-core-ubuntu` — a label this repo already uses in five workflows —
while ui stays on the 2-core arm box.
ci.yml: `test-ui` + `test-node`, with the repo-wide guardrail steps riding the
lighter lane so the slow one is nothing but tests. deploy.yml: the same two,
plus `typecheck`, which keeps the build and the gates together because
`typecheck:strict` resolves the built @astryxdesign/* types.
`test` remains a job in both, now a join asserting every lane succeeded. In
ci.yml that keeps the required status check reporting; in deploy.yml it keeps
`deploy`'s `needs.test.result == 'success'` gate meaning what it meant, so a
red suite still blocks the publish. This is the shape ci.yml's `build` already
uses over build-storybook + build-sandbox. Both ci.yml lanes keep the
job-level `always()` and the step-level scope conditions, so a docs-only or
spec-only PR still skips the suite and still reports success.
`ci-test-routing.test.mjs` holds the contract for both workflows, each case
verified against the mutation it is meant to catch: every declared project runs
in exactly one lane (a third project with no job would be collected by nobody —
the `belongs to no CI job` defect one level up; two lanes running the same one
would double the work the split was meant to halve), no lane runs the whole
suite, the join exists under its historical name and fails when any lane
fails, ci.yml keeps its lightweight-docs skip, and on the deploy side the
publish still gates on the join, the typecheck lane keeps its build ordering
and every gate that guarded main, and each lane still pins `ref: main`.
The runner-label check reads a fixed allowlist rather than the labels found in
the workflow under test. Deriving the set from that file made the assertion
vacuous — a typo'd label is in the file, so it is in the set, so it validates
itself while the job queues forever against a runner that does not exist. A
typo in either workflow now fails, and a genuinely new label is a deliberate
edit to the list.
* docs(changesets): align package-manager precedence