From ed9e79ac6ae8003982955ec77e1179d053955041 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Fri, 28 Aug 2026 23:37:06 +0000 Subject: [PATCH 1/4] fix(theme): declare --mt-font-size on body so it can see --vscode-font-size CSS resolves var() inside a custom-property declaration at the element where that property is declared. --mt-font-size was declared only on :root, so it could not see the --vscode-font-size that applyTheme() writes to document.body.style. It resolved to nothing, making `body { font-size: var(--mt-font-size) }` invalid at computed-value time, and body fell back to the UA default 16px instead of 13px in every host where applyTheme() is the sole writer: standalone, website, Pocket. VS Code was unaffected because that host supplies --vscode-font-size at the document level itself. Mirror the token onto the body block next to --mt-font-family, and add a test that pins every --vscode-*-bound token declared above the body block to a matching body-level declaration, so the next one added cannot go missing the same way. Closes #474 --- docs/specs/theme.md | 5 +--- lib/src/lib/themes/consumed-keys.test.ts | 37 ++++++++++++++++++++++++ lib/src/theme.css | 12 +++++--- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/docs/specs/theme.md b/docs/specs/theme.md index faef4dafd..1060d1cf4 100644 --- a/docs/specs/theme.md +++ b/docs/specs/theme.md @@ -107,10 +107,7 @@ are still visible on `document.body`. ThemePicker also performs a browser layout-effect restore after mount so website hydration cannot leave the picker state saying a theme is active while xterm.js sees fallback colors. -`theme.css` declares the theme-dependent `--color-*` tokens on `body` because -`--vscode-*` variables also live there. Keep the parallel `@theme` -declarations so Tailwind can generate utility classes, but treat the body-level -declarations as the runtime source of truth. +`theme.css` declares the theme-dependent `--color-*` tokens — and the `--mt-font-size` / `--mt-font-family` typography tokens — on `body` because `--vscode-*` variables also live there. Keep the parallel `@theme` and `:root` declarations so Tailwind can generate utility classes, but treat the body-level declarations as the runtime source of truth. Every token whose value reads a `--vscode-*` variable must appear in both places: CSS resolves `var()` inside a custom-property declaration at the element where the property is declared, so a document-level declaration cannot see a variable `applyTheme()` wrote to `body.style`, and the token resolves to nothing in every host where `applyTheme()` is the sole writer (standalone, website, Pocket). `lib/src/lib/themes/consumed-keys.test.ts` pins the two lists together. Dynamic palette tokens (`--color-door-bg`, `--color-door-fg`, `--color-focus-ring`, and the four `--color-alarm-vs-*` tokens) also have body-level baseline bindings matching the `@theme` declarations, so direct diff --git a/lib/src/lib/themes/consumed-keys.test.ts b/lib/src/lib/themes/consumed-keys.test.ts index 0b4d1e9d5..b17699950 100644 --- a/lib/src/lib/themes/consumed-keys.test.ts +++ b/lib/src/lib/themes/consumed-keys.test.ts @@ -31,3 +31,40 @@ describe('CONSUMED_VSCODE_KEYS / bundle-themes.mjs parity', () => { expect(extra).toEqual([]); }); }); + +// theme.css declares the same tokens twice: once in `@theme` (so Tailwind +// generates utility classes) and once on `body` (so they can actually see the +// --vscode-* variables applyTheme() writes to body.style). CSS resolves var() +// inside a custom-property declaration at the element where the property is +// declared, so a token bound to a --vscode-* variable *only* above the body +// block resolves to nothing in every host where applyTheme() is the sole +// writer — standalone, website, Pocket. This pins the two lists together. +describe('theme.css --vscode-* bindings are mirrored onto body', () => { + const here = dirname(fileURLToPath(import.meta.url)); + const themeCss = readFileSync(resolve(here, '../../theme.css'), 'utf8'); + + function declarations(block: string): Map { + const out = new Map(); + for (const m of block.matchAll(/^\s*(--[\w-]+)\s*:\s*([^;]+);/gm)) out.set(m[1], m[2].trim()); + return out; + } + + function blockBody(pattern: RegExp): string { + const match = themeCss.match(pattern); + if (!match) throw new Error(`Could not locate ${pattern} in theme.css`); + return match[1]; + } + + const documentLevel = new Map([ + ...declarations(blockBody(/@theme \{([\s\S]*?)\n\}/)), + ...declarations(blockBody(/\n:root \{([\s\S]*?)\n\}/)), + ]); + const bodyLevel = declarations(blockBody(/\nbody \{([\s\S]*?)\n\}/)); + + it('every document-level token bound to a --vscode-* variable is re-declared on body', () => { + const missing = [...documentLevel] + .filter(([name, value]) => value.includes('var(--vscode-') && !bodyLevel.has(name)) + .map(([name]) => name); + expect(missing).toEqual([]); + }); +}); diff --git a/lib/src/theme.css b/lib/src/theme.css index bd5c9af8b..b196eee2c 100644 --- a/lib/src/theme.css +++ b/lib/src/theme.css @@ -97,11 +97,15 @@ /* --- Body-level dynamic theme bindings --- * - * These mirror the @theme block above but are declared on `body`. CSS resolves - * var() inside a custom-property declaration at the element where the property - * is declared, so body-level declarations can see --vscode-* variables that - * applyTheme() writes to body.style. */ + * These mirror the :root and @theme blocks above but are declared on `body`. + * CSS resolves var() inside a custom-property declaration at the element where + * the property is declared, so body-level declarations can see --vscode-* + * variables that applyTheme() writes to body.style. Every token bound to a + * --vscode-* variable above must be repeated here or it resolves to nothing in + * the hosts where applyTheme() is the only writer (standalone, website, + * Pocket); consumed-keys.test.ts pins the two lists together. */ body { + --mt-font-size: var(--vscode-font-size); --mt-font-family: var(--vscode-editor-font-family); --font-sans: var(--vscode-editor-font-family); --font-mono: var(--vscode-editor-font-family); From 8510168401b1fc42c71a2dd69f1bf507ced98679 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Fri, 28 Aug 2026 23:47:59 +0000 Subject: [PATCH 2/4] review(theme): widen the mirror guard to any var() chain, and correct the spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies both inline suggestions from the review on ed9e79a. The guard now matches any `var()` chain rather than a literal `var(--vscode-`, so indirect bindings like `--color-door-bg: var(--color-header-inactive-bg)` are covered too, and it compares the two declarations' values instead of just checking presence, so repointing one level's binding while the other goes stale fails as well. Still green against the current theme.css, and still load-bearing: removing the `--mt-font-size` line from the body block reports `['--mt-font-size']`. The spec paragraph is rewrapped to the file's ~76-column width (it shipped as one 819-character line) and no longer implies the `:root` copies feed Tailwind utility generation — Tailwind v4 reads `@theme` only. --- docs/specs/theme.md | 13 ++++++++++++- lib/src/lib/themes/consumed-keys.test.ts | 22 ++++++++++++---------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/docs/specs/theme.md b/docs/specs/theme.md index 1060d1cf4..82d97e650 100644 --- a/docs/specs/theme.md +++ b/docs/specs/theme.md @@ -107,7 +107,18 @@ are still visible on `document.body`. ThemePicker also performs a browser layout-effect restore after mount so website hydration cannot leave the picker state saying a theme is active while xterm.js sees fallback colors. -`theme.css` declares the theme-dependent `--color-*` tokens — and the `--mt-font-size` / `--mt-font-family` typography tokens — on `body` because `--vscode-*` variables also live there. Keep the parallel `@theme` and `:root` declarations so Tailwind can generate utility classes, but treat the body-level declarations as the runtime source of truth. Every token whose value reads a `--vscode-*` variable must appear in both places: CSS resolves `var()` inside a custom-property declaration at the element where the property is declared, so a document-level declaration cannot see a variable `applyTheme()` wrote to `body.style`, and the token resolves to nothing in every host where `applyTheme()` is the sole writer (standalone, website, Pocket). `lib/src/lib/themes/consumed-keys.test.ts` pins the two lists together. +`theme.css` declares the theme-dependent `--color-*` tokens — and the +`--mt-font-size` / `--mt-font-family` typography tokens — on `body` because +`--vscode-*` variables also live there. Keep the parallel `@theme` declarations +so Tailwind can generate utility classes, and the `:root` copies of the +`--mt-*` font tokens, but treat the body-level declarations as the runtime +source of truth. Every token whose value reads a `--vscode-*` variable must +appear at both levels: CSS resolves `var()` inside a custom-property +declaration at the element where the property is declared, so a document-level +declaration cannot see a variable `applyTheme()` wrote to `body.style`, and the +token resolves to nothing in every host where `applyTheme()` is the sole writer +(standalone, website, Pocket). `lib/src/lib/themes/consumed-keys.test.ts` pins +the two lists together. Dynamic palette tokens (`--color-door-bg`, `--color-door-fg`, `--color-focus-ring`, and the four `--color-alarm-vs-*` tokens) also have body-level baseline bindings matching the `@theme` declarations, so direct diff --git a/lib/src/lib/themes/consumed-keys.test.ts b/lib/src/lib/themes/consumed-keys.test.ts index b17699950..2fc933c93 100644 --- a/lib/src/lib/themes/consumed-keys.test.ts +++ b/lib/src/lib/themes/consumed-keys.test.ts @@ -32,14 +32,16 @@ describe('CONSUMED_VSCODE_KEYS / bundle-themes.mjs parity', () => { }); }); -// theme.css declares the same tokens twice: once in `@theme` (so Tailwind -// generates utility classes) and once on `body` (so they can actually see the -// --vscode-* variables applyTheme() writes to body.style). CSS resolves var() -// inside a custom-property declaration at the element where the property is -// declared, so a token bound to a --vscode-* variable *only* above the body -// block resolves to nothing in every host where applyTheme() is the sole -// writer — standalone, website, Pocket. This pins the two lists together. -describe('theme.css --vscode-* bindings are mirrored onto body', () => { +// theme.css declares the same tokens twice: once above the body block (in +// `@theme`, so Tailwind generates utility classes, or in `:root`) and once on +// `body` (so they can actually see the --vscode-* variables applyTheme() writes +// to body.style). CSS resolves var() inside a custom-property declaration at +// the element where the property is declared, so a token whose value reads any +// var() chain *only* above the body block resolves to nothing in every host +// where applyTheme() is the sole writer — standalone, website, Pocket. The +// check compares values, not just presence, so repointing one level's binding +// without the other fails too. This pins the two lists together. +describe('theme.css var() bindings are mirrored onto body', () => { const here = dirname(fileURLToPath(import.meta.url)); const themeCss = readFileSync(resolve(here, '../../theme.css'), 'utf8'); @@ -61,9 +63,9 @@ describe('theme.css --vscode-* bindings are mirrored onto body', () => { ]); const bodyLevel = declarations(blockBody(/\nbody \{([\s\S]*?)\n\}/)); - it('every document-level token bound to a --vscode-* variable is re-declared on body', () => { + it('every document-level token bound to a var() chain is mirrored onto body', () => { const missing = [...documentLevel] - .filter(([name, value]) => value.includes('var(--vscode-') && !bodyLevel.has(name)) + .filter(([name, value]) => value.includes('var(') && bodyLevel.get(name) !== value) .map(([name]) => name); expect(missing).toEqual([]); }); From 25cd1a5afa1ea6f8bb15cfc8aeef2a6cb6f11a08 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:01:30 +0000 Subject: [PATCH 3/4] docs(theme): state the mirror rule as the test enforces it The spec paragraph and the theme.css body-block comment both said a token must be mirrored onto body when its value reads a --vscode-* variable. consumed-keys.test.ts is wider than that: it flags any var() chain, so an indirect binding like --color-door-bg: var(--color-header-inactive-bg) must be mirrored too, and it compares the two declarations' values, so repointing one level without the other fails even though both exist. Both docs now say that. No behavior change. --- docs/specs/theme.md | 23 +++++++++++++---------- lib/src/theme.css | 10 ++++++---- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/docs/specs/theme.md b/docs/specs/theme.md index 82d97e650..8da1dfb2c 100644 --- a/docs/specs/theme.md +++ b/docs/specs/theme.md @@ -109,16 +109,19 @@ state saying a theme is active while xterm.js sees fallback colors. `theme.css` declares the theme-dependent `--color-*` tokens — and the `--mt-font-size` / `--mt-font-family` typography tokens — on `body` because -`--vscode-*` variables also live there. Keep the parallel `@theme` declarations -so Tailwind can generate utility classes, and the `:root` copies of the -`--mt-*` font tokens, but treat the body-level declarations as the runtime -source of truth. Every token whose value reads a `--vscode-*` variable must -appear at both levels: CSS resolves `var()` inside a custom-property -declaration at the element where the property is declared, so a document-level -declaration cannot see a variable `applyTheme()` wrote to `body.style`, and the -token resolves to nothing in every host where `applyTheme()` is the sole writer -(standalone, website, Pocket). `lib/src/lib/themes/consumed-keys.test.ts` pins -the two lists together. +`--vscode-*` variables also live there. Keep the parallel `@theme` +declarations so Tailwind can generate utility classes, and the `:root` +copies of the `--mt-*` font tokens, but treat the body-level declarations as +the runtime source of truth. Every token whose value reads any `var()` +chain — an indirect binding like +`--color-door-bg: var(--color-header-inactive-bg)` counts, not just a direct +`--vscode-*` read — must appear at both levels, with the same value: CSS +resolves `var()` inside a custom-property declaration at the element where +the property is declared, so a document-level declaration cannot see a +variable `applyTheme()` wrote to `body.style`, and the token resolves to +nothing in every host where `applyTheme()` is the sole writer (standalone, +website, Pocket). `lib/src/lib/themes/consumed-keys.test.ts` pins the two +lists together. Dynamic palette tokens (`--color-door-bg`, `--color-door-fg`, `--color-focus-ring`, and the four `--color-alarm-vs-*` tokens) also have body-level baseline bindings matching the `@theme` declarations, so direct diff --git a/lib/src/theme.css b/lib/src/theme.css index b196eee2c..031306885 100644 --- a/lib/src/theme.css +++ b/lib/src/theme.css @@ -100,10 +100,12 @@ * These mirror the :root and @theme blocks above but are declared on `body`. * CSS resolves var() inside a custom-property declaration at the element where * the property is declared, so body-level declarations can see --vscode-* - * variables that applyTheme() writes to body.style. Every token bound to a - * --vscode-* variable above must be repeated here or it resolves to nothing in - * the hosts where applyTheme() is the only writer (standalone, website, - * Pocket); consumed-keys.test.ts pins the two lists together. */ + * variables that applyTheme() writes to body.style. Every token above whose + * value reads any var() chain — indirect bindings like var(--color-door-bg) + * included, not just direct --vscode-* reads — must be repeated here with the + * same value, or it resolves to nothing in the hosts where applyTheme() is the + * only writer (standalone, website, Pocket); consumed-keys.test.ts pins the two + * lists together. */ body { --mt-font-size: var(--vscode-font-size); --mt-font-family: var(--vscode-editor-font-family); From 40433b9a0565477f18ca94d7789d0317ace6fc3d Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 29 Aug 2026 09:54:16 -0700 Subject: [PATCH 4/4] review(theme): state the mirror rule once, in the spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The var()-resolution rationale appeared three times nearly verbatim — theme.css comment, theme.md paragraph, test comment. The spec now owns it; the other two are pointers. Also adds a say-it-once rule to AGENTS.md so future fixes keep prose proportional to the diff. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FP1xPkGZe8JAVtnSgG5Fvt --- AGENTS.md | 2 ++ docs/specs/theme.md | 25 ++++++++++-------------- lib/src/lib/themes/consumed-keys.test.ts | 13 ++++-------- lib/src/theme.css | 13 ++++-------- 4 files changed, 20 insertions(+), 33 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fd392a55a..06721a8f4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,6 +61,8 @@ When updating code covered by a spec, update the spec to match. When the two spe **Narrative docs are not specs.** `docs/stories/pairing.mdx` is a Storybook page that walks the self-hosted remote-control setup end to end, embedding the real screens from `lib/src/stories/`. It restates specs for narrative flow rather than owning anything, `scripts/spec-lint.mjs` does not check it, and the `## Future` fold does not apply. When a remote spec changes, check whether it needs the same edit — the specs win where they disagree. +**Say it once.** An invariant and its rationale live in exactly one place — the owning spec when one covers it, otherwise a comment at the code it constrains. Everywhere else gets at most a one-line pointer. Write a comment only for a constraint the code cannot show, and keep prose proportional to the change: a one-line fix earns a sentence of spec plus pointers, not three restatements of the same explanation. + When editing specs, keep them concise but do not replace invariants or edge cases with only a code pointer. Use `Source of truth:` for implementation references, and include direction/scope for protocols, command orchestration, and cross-package boundaries. For docs-only compression, spot-check referenced symbols, message directions, and root-vs-package script ownership against code before committing. Every spec that uses Session / Pane / Door / baseboard / passthrough vocabulary leads with a `> See \`docs/specs/glossary.md\` for ...` blockquote (see `layout.md`, `alert.md`, `terminal-state.md`). When introducing glossary vocabulary into a spec that lacks the callout, add it in the same edit. diff --git a/docs/specs/theme.md b/docs/specs/theme.md index 8da1dfb2c..fe210dccc 100644 --- a/docs/specs/theme.md +++ b/docs/specs/theme.md @@ -107,21 +107,16 @@ are still visible on `document.body`. ThemePicker also performs a browser layout-effect restore after mount so website hydration cannot leave the picker state saying a theme is active while xterm.js sees fallback colors. -`theme.css` declares the theme-dependent `--color-*` tokens — and the -`--mt-font-size` / `--mt-font-family` typography tokens — on `body` because -`--vscode-*` variables also live there. Keep the parallel `@theme` -declarations so Tailwind can generate utility classes, and the `:root` -copies of the `--mt-*` font tokens, but treat the body-level declarations as -the runtime source of truth. Every token whose value reads any `var()` -chain — an indirect binding like -`--color-door-bg: var(--color-header-inactive-bg)` counts, not just a direct -`--vscode-*` read — must appear at both levels, with the same value: CSS -resolves `var()` inside a custom-property declaration at the element where -the property is declared, so a document-level declaration cannot see a -variable `applyTheme()` wrote to `body.style`, and the token resolves to -nothing in every host where `applyTheme()` is the sole writer (standalone, -website, Pocket). `lib/src/lib/themes/consumed-keys.test.ts` pins the two -lists together. +`theme.css` declares the theme-dependent tokens twice: at document level +(`@theme` so Tailwind generates utility classes, or `:root`) and on `body`, +the runtime source of truth. CSS resolves `var()` inside a custom-property +declaration at the element where the property is declared, so only the `body` +copy can see the `--vscode-*` variables `applyTheme()` writes to `body.style`; +a token declared only at document level resolves to nothing wherever +`applyTheme()` is the sole writer (standalone, website, Pocket). Every token +whose value contains `var()` — indirect chains included — must therefore +appear at both levels with the same value. +`lib/src/lib/themes/consumed-keys.test.ts` enforces this. Dynamic palette tokens (`--color-door-bg`, `--color-door-fg`, `--color-focus-ring`, and the four `--color-alarm-vs-*` tokens) also have body-level baseline bindings matching the `@theme` declarations, so direct diff --git a/lib/src/lib/themes/consumed-keys.test.ts b/lib/src/lib/themes/consumed-keys.test.ts index 2fc933c93..13a429117 100644 --- a/lib/src/lib/themes/consumed-keys.test.ts +++ b/lib/src/lib/themes/consumed-keys.test.ts @@ -32,15 +32,10 @@ describe('CONSUMED_VSCODE_KEYS / bundle-themes.mjs parity', () => { }); }); -// theme.css declares the same tokens twice: once above the body block (in -// `@theme`, so Tailwind generates utility classes, or in `:root`) and once on -// `body` (so they can actually see the --vscode-* variables applyTheme() writes -// to body.style). CSS resolves var() inside a custom-property declaration at -// the element where the property is declared, so a token whose value reads any -// var() chain *only* above the body block resolves to nothing in every host -// where applyTheme() is the sole writer — standalone, website, Pocket. The -// check compares values, not just presence, so repointing one level's binding -// without the other fails too. This pins the two lists together. +// Every var()-bound token declared at document level (@theme or :root) must be +// mirrored onto body with the same value, or it resolves to nothing outside +// VS Code — rationale in docs/specs/theme.md. Values are compared, not just +// presence, so repointing one level's binding without the other fails too. describe('theme.css var() bindings are mirrored onto body', () => { const here = dirname(fileURLToPath(import.meta.url)); const themeCss = readFileSync(resolve(here, '../../theme.css'), 'utf8'); diff --git a/lib/src/theme.css b/lib/src/theme.css index 031306885..a2f67237b 100644 --- a/lib/src/theme.css +++ b/lib/src/theme.css @@ -97,15 +97,10 @@ /* --- Body-level dynamic theme bindings --- * - * These mirror the :root and @theme blocks above but are declared on `body`. - * CSS resolves var() inside a custom-property declaration at the element where - * the property is declared, so body-level declarations can see --vscode-* - * variables that applyTheme() writes to body.style. Every token above whose - * value reads any var() chain — indirect bindings like var(--color-door-bg) - * included, not just direct --vscode-* reads — must be repeated here with the - * same value, or it resolves to nothing in the hosts where applyTheme() is the - * only writer (standalone, website, Pocket); consumed-keys.test.ts pins the two - * lists together. */ + * Runtime source of truth: only body-level declarations can see the --vscode-* + * variables applyTheme() writes to body.style. Every var()-bound token above + * must be mirrored here with the same value (consumed-keys.test.ts enforces + * this; rationale in docs/specs/theme.md). */ body { --mt-font-size: var(--vscode-font-size); --mt-font-family: var(--vscode-editor-font-family);