feat(fork): fail CI on a lint warning in fork-owned code

.fork/customizations.yaml

@@ -564,3 +564,49 @@
     - apps/web/package.json
   verify:
     - apps/web/src/__fork_guards__/geistTypography.test.ts
+
+- id: fork-lint-cleanliness
+  intent: >
+    Fork-owned code carries zero lint warnings, and CI fails on the first one.
+    Nothing in this repository otherwise gates on a warning: vp check lints
+    everything but exits 0, and no --max-warnings is set anywhere, so warnings
+    in fork-authored files accumulate in silence. Nine dead imports survived
+    three pull requests that way — removing them was never the difficulty,
+    noticing was. The guard apparatus exists to catch "the fork drifted and
+    nothing noticed", and this is that failure one level up, in the one place
+    the apparatus did not look.
+    The gate is scoped to fork-owned paths on purpose rather than being a
+    repo-wide --max-warnings count. A repo-wide count ratchets against
+    upstream: the first sync that lands an upstream warning turns the build red
+    for code this fork does not own and cannot fix, and the only available
+    response is to raise the number, which is how a ratchet stops meaning
+    anything. Scoped, upstream can add as many warnings as it likes and this
+    stays green because it never looks there. The twelve upstream warnings
+    present when this landed are deliberately untouched.
+    Scope is the three fork-owned directories plus every lintable files: entry
+    in this manifest, deduplicated — most of those entries already sit under
+    one of the directories — and filtered to .ts/.tsx, because files: also
+    lists images, CSS, YAML and shell. The gate compares oxlint's
+    number_of_files against the count it passed in, which is what keeps it
+    honest: an explicitly-passed path matching lint.ignorePatterns is skipped
+    silently, so without that comparison a fork path landing in an ignore list
+    would produce a green run that inspected nothing.
+    The guard test deliberately does not run the lint. It asserts the path
+    selection and the CI wiring, because selection is the half that fails
+    quietly — a gate pointed at the wrong paths still exits 0 — while the lint
+    itself is a multi-second subprocess that belongs in CI rather than in a
+    unit suite that finishes 1600 tests in ten seconds.
+  tier: 1
+  files:
+    - .fork/lint-owned.mjs
+  shadows: []
+  watch:
+    # Carries the fenced step that runs the gate.
+    - .github/workflows/ci.yml
+    # lint.ignorePatterns lives here. An upstream addition that happens to
+    # match a fork-owned path would silently narrow the gate; the file-count
+    # check turns that into a failure, and this entry makes the change visible
+    # to the drift detector in the first place.
+    - vite.config.ts
+  verify:
+    - apps/web/src/__fork_guards__/forkLintCleanliness.test.ts

.fork/lint-owned.mjs

@@ -0,0 +1,183 @@
+#!/usr/bin/env node
+/**
+ * Fork-owned lint gate — see `.fork/notes/FORK-LINT-GUARD-HANDOFF.md`.
+ *
+ * Usage: node .fork/lint-owned.mjs
+ *
+ * Lints every file the fork owns and fails on a single warning. Upstream's own
+ * warnings are untouched: this never looks outside fork-owned paths, so an
+ * upstream commit that adds a warning can never turn this red for code the
+ * fork cannot fix. That is the whole reason it is scoped rather than a
+ * repo-wide `--max-warnings`, which would ratchet against upstream and train
+ * everyone to raise the number.
+ *
+ * Dependency-free by design, like detect-drift.mjs: it runs in a bare Actions
+ * runner with no install step beyond what CI already does for `vp`.
+ */
+
+import * as NodeChildProcess from "node:child_process";
+import * as NodeFS from "node:fs";
+import * as NodePath from "node:path";
+import * as NodeURL from "node:url";
+
+import { parseCustomizations } from "./detect-drift.mjs";
+
+const FORK_DIR = NodePath.dirname(NodeURL.fileURLToPath(import.meta.url));
+const REPO_ROOT = NodePath.resolve(FORK_DIR, "..");
+
+/**
+ * Directories the fork owns outright. Everything under them is fork-authored,
+ * so all of it is in scope regardless of whether the manifest names each file.
+ */
+export const FORK_OWNED_DIRECTORIES = [
+  "apps/web/src/custom",
+  "apps/web/src/overrides",
+  "apps/web/src/__fork_guards__",
+];
+
+/**
+ * Only these are lintable. `files:` also lists .md, .png, .ico, .css, .yml and
+ * .sh. `.mjs` is included so the fork's own tooling — this file and
+ * detect-drift.mjs — is held to the standard it enforces on everything else;
+ * the repo-wide lint does cover them, and it caught a namespace-node-imports
+ * violation in this very script that an earlier `.ts`-only filter missed.
+ */
+const LINTABLE = new Set([".ts", ".tsx", ".mjs"]);
+
+const isLintable = (path) => LINTABLE.has(NodePath.extname(path));
+
+/**
+ * oxlint prefixes its JSON with a human line ("No files found to lint…") when a
+ * path is skipped, which is exactly the case this script most needs to read.
+ * Parse from the first brace rather than assuming the payload starts at byte 0.
+ */
+const parseReport = (stdout) => {
+  const start = stdout.indexOf("{");
+  if (start === -1) {
+    return undefined;
+  }
+  try {
+    return JSON.parse(stdout.slice(start));
+  } catch {
+    return undefined;
+  }
+};
+
+const walk = (absoluteDir) => {
+  const out = [];
+  if (!NodeFS.existsSync(absoluteDir)) {
+    return out;
+  }
+  for (const entry of NodeFS.readdirSync(absoluteDir, { withFileTypes: true })) {
+    const absolute = NodePath.join(absoluteDir, entry.name);
+    if (entry.isDirectory()) {
+      out.push(...walk(absolute));
+    } else if (entry.isFile() && isLintable(entry.name)) {
+      out.push(absolute);
+    }
+  }
+  return out;
+};
+
+/**
+ * Every fork-owned lintable file, repo-relative and deduplicated.
+ *
+ * Most `files:` entries already live under a fork-owned directory, so a naive
+ * concatenation hands oxlint the same path repeatedly. Measured: dropping the
+ * dedup takes 29 paths to 39, and oxlint counts each occurrence rather than
+ * collapsing them — so the file-count check below still agrees and the gate
+ * still passes. The cost is duplicated work and a file count that overstates
+ * what was actually covered, not a false green.
+ */
+export const collectForkOwnedFiles = (manifestText, repoRoot = REPO_ROOT) => {
+  const fromDirectories = FORK_OWNED_DIRECTORIES.flatMap((directory) =>
+    walk(NodePath.join(repoRoot, directory)),
+  );
+  const fromManifest = parseCustomizations(manifestText)
+    .flatMap((entry) => entry.files)
+    .filter(isLintable)
+    .map((relative) => NodePath.join(repoRoot, relative))
+    .filter((absolute) => NodeFS.existsSync(absolute));
+
+  const relative = [...fromDirectories, ...fromManifest].map((absolute) =>
+    NodePath.relative(repoRoot, absolute),
+  );
+  return [...new Set(relative)].sort();
+};
+
+function main() {
+  const manifestText = NodeFS.readFileSync(NodePath.join(FORK_DIR, "customizations.yaml"), "utf8");
+  const files = collectForkOwnedFiles(manifestText);
+
+  if (files.length === 0) {
+    console.error("fork-lint: no fork-owned files found — the path list is wrong, not clean.");
+    process.exit(2);
+  }
+
+  const result = NodeChildProcess.spawnSync(
+    "vp",
+    ["lint", ...files, "--format", "json", "--max-warnings", "0"],
+    {
+      cwd: REPO_ROOT,
+      encoding: "utf8",
+    },
+  );
+
+  if (result.error) {
+    console.error(`fork-lint: could not run vp lint — ${result.error.message}`);
+    process.exit(2);
+  }
+
+  const report = parseReport(result.stdout);
+  if (report === undefined) {
+    console.error("fork-lint: could not parse oxlint JSON. Raw output follows.\n");
+    console.error(result.stdout || result.stderr);
+    process.exit(2);
+  }
+
+  // The check that keeps this honest. An explicitly-passed path that matches
+  // `lint.ignorePatterns` is silently skipped — verified: oxlint reports
+  // number_of_files 0 for one. Skip every path and it exits non-zero on its
+  // own, but skip *some* and the rest lint clean and this would pass while
+  // looking at less than it claims. Compare counts so that cannot happen.
+  if (report.number_of_files !== files.length) {
+    console.error(
+      `fork-lint: expected to lint ${files.length} fork-owned files, oxlint reported ` +
+        `${report.number_of_files}. Some path was skipped — most likely it now matches ` +
+        `lint.ignorePatterns in vite.config.ts. Skipped paths:\n`,
+    );
+    for (const file of files) {
+      const single = NodeChildProcess.spawnSync("vp", ["lint", file, "--format", "json"], {
+        cwd: REPO_ROOT,
+        encoding: "utf8",
+      });
+      const singleReport = parseReport(single.stdout);
+      if (singleReport === undefined || singleReport.number_of_files === 0) {
+        console.error(`  - ${file}`);
+      }
+    }
+    process.exit(1);
+  }
+
+  const diagnostics = report.diagnostics ?? [];
+  if (diagnostics.length > 0) {
+    console.error(
+      `fork-lint: ${diagnostics.length} warning(s) in fork-owned code. The fork owns these ` +
+        `files, so there is no upstream to wait for — fix them.\n`,
+    );
+    for (const diagnostic of diagnostics) {
+      const span = diagnostic.labels?.[0]?.span;
+      const where = span
+        ? `${diagnostic.filename}:${span.line}:${span.column}`
+        : diagnostic.filename;
+      console.error(`  ${where}  ${diagnostic.code}  ${diagnostic.message}`);
+    }
+    process.exit(1);
+  }
+
+  console.log(`fork-lint: ${files.length} fork-owned files, no warnings.`);
+}
+
+if (process.argv[1] && import.meta.url === NodeURL.pathToFileURL(process.argv[1]).href) {
+  main();
+}

.github/workflows/ci.yml

@@ -33,6 +33,16 @@ jobs:
       - name: Check
         run: vp check
 
+      # fork:begin fork-lint-cleanliness — see .fork/customizations.yaml#fork-lint-cleanliness
+      # `vp check` lints the whole repo but exits 0 on warnings, and no
+      # --max-warnings is set anywhere, so fork-authored dead code accumulates
+      # silently — it took three PRs to remove nine dead imports because
+      # noticing was the hard part. This fails on a single warning in
+      # fork-owned code only, so upstream's warnings can never turn it red.
+      - name: Lint fork-owned code
+        run: node .fork/lint-owned.mjs
+      # fork:end fork-lint-cleanliness
+
       - name: Typecheck
         run: vpr typecheck
 

apps/web/src/__fork_guards__/forkLintCleanliness.test.ts

@@ -0,0 +1,88 @@
+// @effect-diagnostics nodeBuiltinImport:off
+/**
+ * Fork guard — see `.fork/customizations.yaml#fork-lint-cleanliness`.
+ *
+ * The gate itself runs in CI, not here: linting 28 files is a multi-second
+ * subprocess and this suite finishes 1600+ tests in about ten seconds. What
+ * this guard covers is the part that fails *quietly* — the path selection. A
+ * gate pointed at the wrong paths still exits 0, so "green" would mean
+ * "inspected nothing" rather than "found nothing".
+ *
+ * So: assert the selection is right (cheap, pure, and the actual risk), and
+ * assert the CI wiring that runs it still exists (the same shape ciOnCustom
+ * uses). The expensive half stays where expensive checks already live.
+ */
+
+import * as NodeFS from "node:fs";
+import * as NodePath from "node:path";
+import * as NodeURL from "node:url";
+import { describe, expect, it } from "vite-plus/test";
+
+// @ts-expect-error — plain .mjs module without type declarations, same as
+// detect-drift.mjs next to it; its selection behaviour is what this exercises.
+import { collectForkOwnedFiles, FORK_OWNED_DIRECTORIES } from "../../../../.fork/lint-owned.mjs";
+
+const selectForkOwnedFiles = (manifest: string, root: string): readonly string[] =>
+  collectForkOwnedFiles(manifest, root) as readonly string[];
+
+const repoRoot = NodePath.resolve(
+  NodeURL.fileURLToPath(new URL(".", import.meta.url)),
+  "../../../..",
+);
+
+const manifestText = NodeFS.readFileSync(
+  NodePath.join(repoRoot, ".fork/customizations.yaml"),
+  "utf8",
+);
+
+describe("fork guard: fork-lint-cleanliness", () => {
+  it("selects every fork-owned directory", () => {
+    const files = selectForkOwnedFiles(manifestText, repoRoot);
+    // `overrides/` holds only a README today, so requiring a hit from all three
+    // would fail for a legitimate reason. Requiring the two that hold code
+    // catches a selector that silently stopped matching.
+    expect(files.some((file) => file.startsWith("apps/web/src/custom/"))).toBe(true);
+    expect(files.some((file) => file.startsWith("apps/web/src/__fork_guards__/"))).toBe(true);
+    expect(FORK_OWNED_DIRECTORIES).toContain("apps/web/src/overrides");
+  });
+
+  it("picks up fork-owned files the directories do not cover", () => {
+    const files = selectForkOwnedFiles(manifestText, repoRoot);
+    // A manifest `files:` entry outside the three directories. If the selector
+    // ever collapsed to "just walk the directories", this is what would be lost.
+    expect(files).toContain("apps/desktop/src/app/DesktopClerkForkSkip.test.ts");
+  });
+
+  it("lints only what can be linted", () => {
+    const files = selectForkOwnedFiles(manifestText, repoRoot);
+    // `files:` also lists .md, .png, .ico, .css, .yml and .sh. Handing oxlint a
+    // PNG is not a hypothetical — nine such entries are in the manifest today.
+    for (const file of files) {
+      expect([".ts", ".tsx", ".mjs"]).toContain(NodePath.extname(file));
+    }
+  });
+
+  it("covers the fork's own tooling", () => {
+    const files = selectForkOwnedFiles(manifestText, repoRoot);
+    // The gate holds itself to the standard it enforces. It did not at first:
+    // a .ts-only filter meant the repo-wide lint caught a rule violation in
+    // this script that the gate itself had passed over.
+    expect(files).toContain(".fork/lint-owned.mjs");
+  });
+
+  it("never hands the same path over twice", () => {
+    const files = selectForkOwnedFiles(manifestText, repoRoot);
+    // Most `files:` entries live under a fork-owned directory, so duplicates
+    // are the default failure. They do not produce a false green — oxlint
+    // counts each occurrence, so the gate's file-count check still agrees —
+    // but they duplicate work and inflate the reported coverage, which is
+    // worth failing on precisely because nothing downstream would notice.
+    expect(new Set(files).size).toBe(files.length);
+  });
+
+  it("keeps the gate wired into CI", () => {
+    const ci = NodeFS.readFileSync(NodePath.join(repoRoot, ".github/workflows/ci.yml"), "utf8");
+    expect(ci).toContain("fork:begin fork-lint-cleanliness");
+    expect(ci).toContain("node .fork/lint-owned.mjs");
+  });
+});