milestone 23: close the remaining codex findings

the vitest "something prevents Vite server from exiting" warning was @stylexjs/unplugin, not our code: configureServer starts a 150ms polling interval for HMR and clears it from server.httpServer's close event, which vitest resolves to null in middleware mode. drop that one hook under VITEST and keep the transform. textMuted was swept rather than judged. measured against their grounds, the 39 flat text-zinc-500 sites went 4.12:1 -> 7.56:1 in dark mode, repairing a WCAG AA failure, so that stands; the 4 already-adaptive sites went 7.40:1 -> 4.62:1 in light mode for nothing, so a textSecondary token restores their original zinc-600/zinc-400 pair. append the symbol families to the reset's font stack: Select renders U+25BE and Segoe UI does not carry it. correct two acceptance criteria that were literally false: both greps match only comments.
2026-08-12 23:43:22 +02:00
parent 0ea2e2905a
commit 4aaf6d3815
8 changed files with 108 additions and 22 deletions
@@ -264,8 +264,9 @@ Owns: `web/package.json`, `web/package-lock.json`, `web/src/ui/Dialog.tsx`
 - S2.4 license review per ruling 8 — every package `assert-bundled` names.

 Acceptance (S2):
- [ ] `grep -rn 'window.confirm' web/src` → only hits, if any, are in files
-      S3 owns (expected: none; ruling 5 call sites all live in S2 files).
+- [ ] `grep -rn 'window\.confirm(' web/src` → zero matches. The bare name
+      still appears in three comments in `ui/` that record what replaced it,
+      which is why the check is for a call and not for the name.
 - [ ] Delete flows on upstreams, blocklists, rules, zones and records drive a
      `role="alertdialog"` element; confirm and cancel both covered by tests.
 - [ ] Tabs on `/local-dns` expose `role="tablist"`; keyboard arrow keys move
@@ -298,9 +299,12 @@ tests, `web/src/ui/classes.ts` (delete), `web/package.json`,
 - S3.4 PLAN.md per ruling 9.

 Acceptance (S3):
- [ ] `grep -rn 'className="' web/src` returns zero matches;
-      `grep -rn 'tailwind' web` matches nothing outside `package-lock.json`
-      history (i.e. no source, config or dependency reference).
+- [ ] `grep -rn 'className="' web/src` returns zero matches. No Tailwind
+      dependency in `web/package.json`, no plugin in `web/vite.config.ts`, no
+      `@import "tailwindcss"` in `web/src/styles.css`. The word still appears
+      in seven comments in `web/src` that name the ramps the tokens were taken
+      from and the reset the preflight used to supply; those are the record of
+      where the values came from, so the check is not for the name.
 - [ ] `ui/classes.ts` is gone; nothing imports it.
 - [ ] Dark scheme still follows `prefers-color-scheme` (vitest: token var
      resolves differently under a mocked dark media query, or a recorded
@@ -491,6 +495,47 @@ Vitest began reporting "Tests closed successfully but something prevents Vite
 server from exiting" with the StyleX plugin in the pipeline. The suite passes
 and exits 0. It should not be allowed to become the accepted baseline.

+### Both S3 notes are now settled
+
+**The vitest warning is fixed, and it was not our code.** Bisecting a trivial
+one-assertion test against a bare config showed `react()` exits clean and
+`stylex.vite()` does not. `@stylexjs/unplugin@0.19.0`'s
+`lib/es/vite.mjs:59` starts a 150ms `setInterval` in `configureServer` to poll
+its CSS store for HMR, and clears it from the `close` event of
+`server.httpServer`. A probe plugin confirmed vitest resolves `httpServer` to
+`null`, because it runs the dev server in middleware mode — so the timer is
+never cleared and holds the event loop open until vitest kills the worker on a
+ten-second timeout. `web/vite.config.ts` now strips that one hook when
+`process.env.VITEST` is set; the transform hook, which the tests do need, is
+untouched, and `ui/styles.test.tsx` still proves it runs. `npm test` reports 31
+files and 183 tests passed, with no warning, and exits 0.
+
+**`textMuted` was swept; here is the per-call-site judgement.** Measuring the
+two shifts against their grounds settles it in opposite directions:
+
+- 39 call sites were a flat `text-zinc-500` with no dark override. On the dark
+  ground that measures **4.12:1**, which fails WCAG AA. The token's zinc-400
+  dark value measures **7.56:1**. The sweep repaired a real accessibility
+  defect there, so it stands, and the token comment now records why.
+- 4 call sites were `text-zinc-600 dark:text-zinc-400` — already adaptive. The
+  sweep moved their light value to zinc-500, from **7.40:1** to **4.62:1**.
+  That clears AA by a hair on small text: the query-log table header, the
+  dashboard period button, the inactive nav item and the chart legend. It buys
+  nothing, so a second token `textSecondary` (zinc-600 light, zinc-400 dark)
+  restores the original pair at exactly those four.
+
+Contrast was computed from the tokens' own oklch values through linear sRGB to
+WCAG relative luminance, not read off a chart.
+
+### The reset's font stack is not Tailwind's, deliberately
+
+Tailwind is uninstalled, so its default stack cannot be reproduced from the
+tree, and ruling 7 accepts the metric drift. One part is not drift: `Select.tsx`
+renders U+25BE for its chevron, and Segoe UI does not carry that glyph, so on
+Windows the stack has to reach a font that does. `"Segoe UI Symbol"` and
+`"Noto Sans Symbols 2"` are appended for that glyph. No emoji families are
+listed, because the app renders no emoji.
+
 ## Anti-requirements

 - No router or query changes: TanStack Router and TanStack Query stay,
@@ -55,7 +55,7 @@ const styles = stylex.create({
 	},
 	periodIdle: {
 		backgroundColor: { default: "transparent", ":hover": colors.surfaceHover },
-		color: colors.textMuted,
+		color: colors.textSecondary,
 	},
 	/** Dynamic: the caller sizes the placeholder to the widget it stands in for. */
 	skeletonHeight: (height: number) => ({ height }),
@@ -110,7 +110,7 @@ const styles = stylex.create({
 		rowGap: "0.25rem",
 		fontSize: "0.75rem",
 		lineHeight: "1rem",
-		color: colors.textMuted,
+		color: colors.textSecondary,
 	},
 	legendItem: {
 		display: "flex",
@@ -88,7 +88,7 @@ const styles = stylex.create({
 		paddingInline: "0.75rem",
 		paddingBlock: "0.5rem",
 		fontWeight: 500,
-		color: colors.textMuted,
+		color: colors.textSecondary,
 	},
 	/** `divide-y`: a hairline between rows, so the first row carries none. */
 	row: {
@@ -50,7 +50,7 @@ const styles = stylex.create({
 	},
 	navIdle: {
 		backgroundColor: { default: "transparent", ":hover": colors.surfaceHover },
-		color: { default: colors.textMuted, ":hover": colors.text },
+		color: { default: colors.textSecondary, ":hover": colors.text },
 	},
 	versionFooter: {
 		paddingInline: "1rem",
@@ -25,6 +25,13 @@
 		padding: 0;
 	}

+	/*
+	 * This is not a reproduction of Tailwind's default stack, which is gone with
+	 * the package; text metrics drift here, which ruling 7 accepts. The symbol
+	 * families at the end are not decoration: `Select.tsx` renders U+25BE for its
+	 * chevron, and Segoe UI does not carry that glyph, so on Windows the stack
+	 * has to reach a font that does before it falls back to a substitute box.
+	 */
 	html {
 		line-height: 1.5;
 		-webkit-text-size-adjust: 100%;
@@ -35,7 +42,9 @@
 			Roboto,
 			"Helvetica Neue",
 			Arial,
-			sans-serif;
+			sans-serif,
+			"Segoe UI Symbol",
+			"Noto Sans Symbols 2";
 	}

 	/* Headings carry their scale from StyleX, not from the user agent. */
@@ -27,7 +27,20 @@ export const colors = stylex.defineVars({
 	/** Control outlines, which need more contrast than a row divider. */
 	borderStrong: { default: "oklch(87.1% 0.006 286.286)", [DARK]: "oklch(37% 0.013 285.805)" },
 	text: { default: "oklch(21% 0.006 285.885)", [DARK]: "oklch(96.7% 0.001 286.375)" },
+	/**
+	 * Secondary text. Most call sites were a flat zinc-500 before the conversion,
+	 * with no dark override, which measured 4.12:1 on the dark ground and so
+	 * failed WCAG AA; the dark value here is zinc-400, which measures 7.56:1.
+	 */
 	textMuted: { default: "oklch(55.2% 0.016 285.938)", [DARK]: "oklch(70.5% 0.015 286.067)" },
+	/**
+	 * Secondary text that already adapted before the conversion: zinc-600 on the
+	 * light ground, zinc-400 on the dark one. It is a separate token because
+	 * `textMuted`'s lighter light value measures 4.62:1 against 7.40:1 here, and
+	 * these call sites are small text — table headers, an inactive nav item, a
+	 * chart legend — where that loss shows.
+	 */
+	textSecondary: { default: "oklch(44.2% 0.017 285.786)", [DARK]: "oklch(70.5% 0.015 286.067)" },
 	/** Primary actions. Identical in both schemes, as before the conversion. */
 	primary: { default: "oklch(54.6% 0.245 262.881)", [DARK]: "oklch(54.6% 0.245 262.881)" },
 	primaryText: { default: "#fff", [DARK]: "#fff" },
@@ -2,20 +2,39 @@
 import { fileURLToPath } from "node:url";
 import stylex from "@stylexjs/unplugin";
 import react from "@vitejs/plugin-react";
-import { defineConfig } from "vite";
+import { type Plugin, defineConfig } from "vite";
+
+/**
+ * StyleX's Vite plugin polls its CSS store every 150ms from `configureServer`
+ * and pushes an HMR event when it changes. It clears that timer from the close
+ * event of `server.httpServer`. Vitest runs the dev server in middleware mode,
+ * where `httpServer` is null, so the timer is never cleared: the run finishes,
+ * the event loop stays awake, and vitest kills the worker after ten seconds with
+ * "something prevents Vite server from exiting". A test needs StyleX's
+ * transform, which is a different hook, and never needs its HMR — so drop this
+ * one hook under vitest and keep the plugin.
+ *
+ * Remove this once @stylexjs/unplugin clears the timer on `buildEnd` or guards
+ * the hook, and check that `npm test` still exits without the warning.
+ */
+function withoutDevServerPolling(plugin: Plugin | Plugin[]): Plugin[] {
+	return (Array.isArray(plugin) ? plugin : [plugin]).map(({ configureServer: _dropped, ...rest }) => rest);
+}
+
+function stylexPlugin(mode: string): Plugin | Plugin[] {
+	// StyleX resolves the `.stylex.ts` theme file itself, so it needs the `@`
+	// alias too — `resolve.alias` below is Vite's and the plugin cannot see it.
+	const configured = stylex.vite({
+		useCSSLayers: true,
+		dev: mode === "development",
+		runtimeInjection: false,
+		aliases: { "@/*": [fileURLToPath(new URL("./src/*", import.meta.url))] },
+	});
+	return process.env.VITEST === undefined ? configured : withoutDevServerPolling(configured);
+}

 export default defineConfig(({ mode }) => ({
-	plugins: [
-		// StyleX resolves the `.stylex.ts` theme file itself, so it needs the `@`
-		// alias too — `resolve.alias` below is Vite's and the plugin cannot see it.
-		stylex.vite({
-			useCSSLayers: true,
-			dev: mode === "development",
-			runtimeInjection: false,
-			aliases: { "@/*": [fileURLToPath(new URL("./src/*", import.meta.url))] },
-		}),
-		react(),
-	],
+	plugins: [stylexPlugin(mode), react()],
 	resolve: {
 		alias: {
 			"@": fileURLToPath(new URL("./src", import.meta.url)),