Warn when an html surface hardcodes scheme colors

.changeset/warn-hardcoded-surface-colors.md

@@ -0,0 +1,10 @@
+---
+"sideshow": patch
+---
+
+Warn at publish time when an html surface hardcodes scheme colors. A
+`background`/`color` set to a literal (hex/rgb/hsl/white/black) instead of a
+`--color-*` theme token renders washed-out on a board in the opposite scheme;
+the publish/revise API now returns a non-blocking `warnings` array pointing the
+agent at the theme tokens. Token-driven surfaces (and other surface kinds) are
+unaffected.

server/app.ts

@@ -33,6 +33,7 @@ import {
   type TraceStep,
 } from "./types.ts";
 import { validateSurfaces } from "./postSurfaces.ts";
+import { lintSurfaces } from "./surfaceLint.ts";
 
 export type { FeedEvent } from "./events.ts";
 
@@ -581,7 +582,8 @@ export function createApp({
     agent?: string;
     cwd?: string;
   }): Promise<
-    { surface: Post; userFeedback?: Feedback[] } | { error: string; status: 400 | 404 | 413 }
+    | { surface: Post; userFeedback?: Feedback[]; warnings?: string[] }
+    | { error: string; status: 400 | 404 | 413 }
   > {
     if (input.parts.length === 0) {
       return { error: "a post needs at least one surface", status: 400 };
@@ -611,7 +613,12 @@ export function createApp({
     });
     if (!surface) return { error: "session not found", status: 404 };
     bus.broadcast({ type: "post-created", id: surface.id, sessionId, version: 1 });
-    return { surface, userFeedback: await collectFeedback(sessionId) };
+    const warnings = lintSurfaces(input.parts);
+    return {
+      surface,
+      userFeedback: await collectFeedback(sessionId),
+      ...(warnings.length && { warnings }),
+    };
   }
 
   // Store an uploaded blob. Like publishSurface, an explicit session is
@@ -654,7 +661,8 @@ export function createApp({
     id: string,
     patch: { parts?: Surface[]; title?: string },
   ): Promise<
-    { surface: Post; userFeedback?: Feedback[] } | { error: string; status: 400 | 404 | 413 }
+    | { surface: Post; userFeedback?: Feedback[]; warnings?: string[] }
+    | { error: string; status: 400 | 404 | 413 }
   > {
     if (patch.parts) {
       if (patch.parts.length === 0) {
@@ -673,7 +681,12 @@ export function createApp({
       sessionId: surface.sessionId,
       version: surface.version,
     });
-    return { surface, userFeedback: await collectFeedback(surface.sessionId) };
+    const warnings = patch.parts ? lintSurfaces(patch.parts) : [];
+    return {
+      surface,
+      userFeedback: await collectFeedback(surface.sessionId),
+      ...(warnings.length && { warnings }),
+    };
   }
 
   // --- per-surface flow functions (append / replace / remove / reorder) ---
@@ -686,7 +699,8 @@ export function createApp({
     surface: Surface,
     pos?: { before?: string; after?: string },
   ): Promise<
-    { surface: Post; userFeedback?: Feedback[] } | { error: string; status: 400 | 404 | 413 }
+    | { surface: Post; userFeedback?: Feedback[]; warnings?: string[] }
+    | { error: string; status: 400 | 404 | 413 }
   > {
     const existing = await store.getPost(id);
     if (!existing) return { error: "post not found", status: 404 };
@@ -710,7 +724,8 @@ export function createApp({
     target: string,
     replacement: { surface?: Surface; content?: string; kits?: unknown },
   ): Promise<
-    { surface: Post; userFeedback?: Feedback[] } | { error: string; status: 400 | 404 | 413 }
+    | { surface: Post; userFeedback?: Feedback[]; warnings?: string[] }
+    | { error: string; status: 400 | 404 | 413 }
   > {
     const existing = await store.getPost(id);
     if (!existing) return { error: "post not found", status: 404 };
@@ -754,7 +769,8 @@ export function createApp({
     id: string,
     target: string,
   ): Promise<
-    { surface: Post; userFeedback?: Feedback[] } | { error: string; status: 400 | 404 | 413 }
+    | { surface: Post; userFeedback?: Feedback[]; warnings?: string[] }
+    | { error: string; status: 400 | 404 | 413 }
   > {
     const existing = await store.getPost(id);
     if (!existing) return { error: "post not found", status: 404 };
@@ -771,7 +787,8 @@ export function createApp({
     id: string,
     order: (string | number)[],
   ): Promise<
-    { surface: Post; userFeedback?: Feedback[] } | { error: string; status: 400 | 404 | 413 }
+    | { surface: Post; userFeedback?: Feedback[]; warnings?: string[] }
+    | { error: string; status: 400 | 404 | 413 }
   > {
     const existing = await store.getPost(id);
     if (!existing) return { error: "post not found", status: 404 };
@@ -1313,6 +1330,7 @@ export function createApp({
       {
         ...writeResult(result.surface),
         ...(result.userFeedback && { userFeedback: result.userFeedback }),
+        ...(result.warnings && { warnings: result.warnings }),
       },
       201,
     );
@@ -1347,6 +1365,7 @@ export function createApp({
     return c.json({
       ...writeResult(result.surface),
       ...(result.userFeedback && { userFeedback: result.userFeedback }),
+      ...(result.warnings && { warnings: result.warnings }),
     });
   };
   app.put("/api/surfaces/:id", revise);
@@ -1412,6 +1431,7 @@ export function createApp({
     return c.json({
       ...writeResult(result.surface),
       ...(result.userFeedback && { userFeedback: result.userFeedback }),
+      ...(result.warnings && { warnings: result.warnings }),
     });
   });
 
@@ -1434,6 +1454,7 @@ export function createApp({
     return c.json({
       ...writeResult(result.surface),
       ...(result.userFeedback && { userFeedback: result.userFeedback }),
+      ...(result.warnings && { warnings: result.warnings }),
     });
   });
 
@@ -1460,6 +1481,7 @@ export function createApp({
     return c.json({
       ...writeResult(result.surface),
       ...(result.userFeedback && { userFeedback: result.userFeedback }),
+      ...(result.warnings && { warnings: result.warnings }),
     });
   });
 
@@ -1471,6 +1493,7 @@ export function createApp({
     return c.json({
       ...writeResult(result.surface),
       ...(result.userFeedback && { userFeedback: result.userFeedback }),
+      ...(result.warnings && { warnings: result.warnings }),
     });
   });
 
@@ -1485,6 +1508,7 @@ export function createApp({
     return c.json({
       ...writeResult(result.surface),
       ...(result.userFeedback && { userFeedback: result.userFeedback }),
+      ...(result.warnings && { warnings: result.warnings }),
     });
   });
 

server/surfaceLint.ts

@@ -0,0 +1,63 @@
+// Advisory publish-time lint for surfaces. The one firm design rule is "dark
+// mode is mandatory" (guide/DESIGN_GUIDE.md): an html surface must drive every
+// color from the --color-* theme tokens so it adapts to the board's light/dark
+// scheme. An agent that hardcodes a hex/rgb color instead renders washed-out on
+// a board in the opposite scheme (e.g. a `background:#fff` note on a dark board).
+// The renderer can't fix this — it can't tell an accidental light card from a
+// deliberate one — so the publish path flags it back to the author as a
+// non-blocking warning. It never rejects: an intentional light/dark design is
+// flagged too, and the agent is free to ignore it.
+import type { Surface } from "./types.ts";
+
+// Adaptiveness-critical CSS properties. A hardcoded value on one of these is
+// what makes a surface ignore the board scheme. Deliberately limited to
+// background + text: SVG fill/stroke and decorative borders carry literal colors
+// far more often without breaking adaptiveness, so including them would mostly
+// add false positives.
+const SCHEME_PROPS = ["background", "background-color", "color"];
+
+// A literal color value: hex, rgb()/rgba(), hsl()/hsla(), or the two named
+// colors that actually break a scheme (white/black). A value that reads a theme
+// token (var(--…)) is adaptive by construction and never flagged — including
+// `var(--x, #fff)`, where the hardcoded hex is only a last-resort fallback.
+const LITERAL_COLOR = /#[0-9a-f]{3,8}\b|\brgba?\(|\bhsla?\(|\b(?:white|black)\b/i;
+
+// Pull hardcoded scheme colors out of an html surface's markup. Scans both
+// inline `style="…"` attributes and `<style>…</style>` rules for a
+// background/color declaration whose value is a literal (and not a var()).
+// Returns the offending declarations, deduped and capped, for the warning text.
+export function findHardcodedColors(html: string): string[] {
+  // The leading lookbehind keeps `border-color` / `caret-color` from matching as
+  // `color` (and `-background` from matching `background`).
+  const decl = new RegExp(`(?<![\\w-])(${SCHEME_PROPS.join("|")})\\s*:\\s*([^;"'{}]+)`, "gi");
+  const hits: string[] = [];
+  for (let m = decl.exec(html); m; m = decl.exec(html)) {
+    const value = m[2].trim();
+    if (value.includes("var(")) continue; // token-driven → adaptive
+    if (!LITERAL_COLOR.test(value)) continue;
+    const text = `${m[1].toLowerCase()}: ${value}`.slice(0, 60);
+    if (!hits.includes(text)) hits.push(text);
+    if (hits.length >= 4) break;
+  }
+  return hits;
+}
+
+// One advisory warning per html surface that hardcodes scheme colors. Other
+// kinds (markdown/code/diff/mermaid/terminal) are themed by the trusted renderer
+// — markdown even escapes raw HTML — so they can't carry an applied hardcoded
+// color; they're skipped.
+export function lintSurfaces(parts: Surface[]): string[] {
+  const warnings: string[] = [];
+  parts.forEach((part, i) => {
+    if (part.kind !== "html") return;
+    const hits = findHardcodedColors(part.html);
+    if (hits.length === 0) return;
+    const where = parts.length > 1 ? `surface ${i + 1}` : "surface";
+    warnings.push(
+      `${where} hardcodes colors (${hits.join(", ")}) — drive color from the ` +
+        `--color-* theme tokens so it adapts to light/dark; hardcoded colors render ` +
+        `washed-out on a board in the opposite scheme.`,
+    );
+  });
+  return warnings;
+}

test/api.test.ts

@@ -61,6 +61,45 @@ test("publish without session auto-creates one", async () => {
   assert.equal(sessions[0].surfaceCount, 1);
 });
 
+test("publishing an html surface that hardcodes colors returns a warning", async () => {
+  const app = makeApp();
+  const res = await app.request(
+    "/api/snippets",
+    json({ html: '<div style="background:#ffffff;color:#57606a">note</div>' }),
+  );
+  assert.equal(res.status, 201);
+  const out = (await res.json()) as any;
+  assert.ok(Array.isArray(out.warnings), "response should carry a warnings array");
+  assert.match(out.warnings[0], /hardcodes colors/);
+  assert.match(out.warnings[0], /--color-\* theme tokens/);
+});
+
+test("a token-driven html surface publishes with no warnings", async () => {
+  const app = makeApp();
+  const res = await app.request(
+    "/api/snippets",
+    json({ html: '<div style="background:var(--color-background-primary)">ok</div>' }),
+  );
+  assert.equal(res.status, 201);
+  const out = (await res.json()) as any;
+  assert.equal(out.warnings, undefined);
+});
+
+test("revising into hardcoded colors warns too", async () => {
+  const app = makeApp();
+  const first = (await (
+    await app.request("/api/snippets", json({ html: "<p>clean</p>" }))
+  ).json()) as any;
+  assert.equal(first.warnings, undefined);
+  const res = await app.request(`/api/posts/${first.id}`, {
+    ...json({ surfaces: [{ kind: "html", html: '<div style="color:#111">x</div>' }] }),
+    method: "PUT",
+  });
+  assert.equal(res.status, 200);
+  const out = (await res.json()) as any;
+  assert.match(out.warnings?.[0] ?? "", /hardcodes colors/);
+});
+
 test("onEvent receives published feed events", async () => {
   const events: unknown[] = [];
   const app = makeApp(undefined, { onEvent: (event) => events.push(event) });

test/surfaceLint.test.ts

@@ -0,0 +1,74 @@
+import assert from "node:assert/strict";
+import { test } from "node:test";
+import { findHardcodedColors, lintSurfaces } from "../server/surfaceLint.ts";
+import type { Surface } from "../server/types.ts";
+
+const html = (s: string): Surface => ({ kind: "html", html: s });
+
+test("flags a hardcoded background + text color in an inline style", () => {
+  const hits = findHardcodedColors('<div style="background:#ffffff;color:#57606a">x</div>');
+  assert.deepEqual(hits, ["background: #ffffff", "color: #57606a"]);
+});
+
+test("flags rgb()/rgba()/hsl() and the named scheme-breakers", () => {
+  assert.ok(findHardcodedColors('<p style="color:rgb(20,20,20)">x</p>').length);
+  assert.ok(findHardcodedColors('<p style="background:rgba(0,0,0,.5)">x</p>').length);
+  assert.ok(findHardcodedColors('<p style="background:hsl(0,0%,100%)">x</p>').length);
+  assert.ok(findHardcodedColors('<p style="background:white">x</p>').length);
+  assert.ok(findHardcodedColors('<p style="color:black">x</p>').length);
+});
+
+test("flags colors in a <style> block too", () => {
+  const hits = findHardcodedColors("<style>.box{background:#0d1117}</style><div class=box></div>");
+  assert.deepEqual(hits, ["background: #0d1117"]);
+});
+
+test("does NOT flag theme-token values (var(--color-*))", () => {
+  assert.deepEqual(
+    findHardcodedColors(
+      '<div style="background:var(--color-background-primary);color:var(--color-text-primary)">x</div>',
+    ),
+    [],
+  );
+});
+
+test("does NOT flag a var() with a hardcoded fallback (the literal is last-resort)", () => {
+  assert.deepEqual(
+    findHardcodedColors('<div style="color:var(--color-text-primary, #111)">x</div>'),
+    [],
+  );
+});
+
+test("ignores non-scheme properties (SVG fill/stroke, borders)", () => {
+  // fill/stroke carry literal colors constantly without breaking adaptiveness.
+  assert.deepEqual(findHardcodedColors('<rect fill="#fff" stroke="#000" />'), []);
+  assert.deepEqual(findHardcodedColors('<div style="border-color:#ccc">x</div>'), []);
+});
+
+test("caps the reported declarations at 4", () => {
+  const many = Array.from({ length: 10 }, (_, i) => `color:#${i}${i}${i}`).join(";");
+  assert.equal(findHardcodedColors(`<div style="${many}">x</div>`).length, 4);
+});
+
+test("lintSurfaces only warns on html surfaces, with a 1-based surface label", () => {
+  const parts: Surface[] = [
+    { kind: "markdown", markdown: "color:#fff is just text here" },
+    html('<div style="background:#fff">x</div>'),
+  ];
+  const warnings = lintSurfaces(parts);
+  assert.equal(warnings.length, 1);
+  assert.match(warnings[0], /^surface 2 hardcodes colors \(background: #fff\)/);
+  assert.match(warnings[0], /--color-\* theme tokens/);
+});
+
+test("lintSurfaces says just 'surface' when there is only one", () => {
+  const warnings = lintSurfaces([html('<div style="background:#fff">x</div>')]);
+  assert.match(warnings[0], /^surface hardcodes colors/);
+});
+
+test("lintSurfaces is silent for a fully token-driven surface", () => {
+  assert.deepEqual(
+    lintSurfaces([html('<div style="background:var(--color-background-primary)">ok</div>')]),
+    [],
+  );
+});