build: add a browser-test exclusion check and a pre-push static-check hook

.github/CONTRIBUTING.md

@@ -28,12 +28,15 @@ The frontend is plain JS served from `src/web/public/` with no bundler in dev: e
 CI runs all of these, so save yourself a round trip:
 
 ```bash
-npm run typecheck              # tsc --noEmit, strict mode
+npm run typecheck               # tsc --noEmit, strict mode
 npm run lint
 npm run format:check
-npm run check:frontend-syntax  # syntax-checks the plain-JS frontend modules
+npm run check:frontend-syntax   # syntax-checks the plain-JS frontend modules
+npm run check:browser-excludes  # every browser-driven test is kept out of `npm test`
 ```
 
+`npm install` also installs a `pre-push` git hook that runs these static checks (about 10-40s, machine-dependent) and blocks the push if one fails. It skips itself when you push something other than the checked-out HEAD, or when the tree has uncommitted changes the checks would read. Skip it once with `CODEMAN_SKIP_PREPUSH=1 git push`; it never replaces a `pre-push` hook of your own.
+
 ### Tests
 
 ```bash

.github/workflows/ci.yml

@@ -34,6 +34,13 @@ jobs:
       - name: Frontend JS syntax check
         run: npm run check:frontend-syntax
 
+      # Asks `vitest list` what CI would actually collect, rather than matching
+      # filenames: a browser-driven test missing from BROWSER_TEST_GLOBS
+      # (config/test-suites.ts) passes locally and dies in the test job with
+      # "browserType.launch: Executable doesn't exist".
+      - name: Browser-test exclusion check
+        run: npm run check:browser-excludes
+
       - name: Format check
         run: npm run format:check
 

CLAUDE.md

@@ -34,6 +34,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 - To land a commit on master **without** switching branches (which would yank the tree out from under the other session): `git push origin HEAD:master` then `git branch -f master HEAD`. Never `git checkout master` to "fix" it.
 - **Never `git add -A`/`git add .`** — stage explicit paths. A sweep will pick up another session's WIP.
 - Another session's broken WIP can block `npm run build`, since `tsc` is the first step and the build gates on it. That is not your bug to fix. ⚠️ `tsc` still EMITS on type errors, so a failed `npm run build` leaves a rebuilt `dist/index.js` compiled from their tree; check what it pulled in before restarting the service. To deploy frontend-only changes past a blocked `tsc`, run the asset stage of `scripts/build.mjs` (everything after the `tsc`/`chmod` lines is independent of it).
+- **A pre-push failure in a file you did not touch is another session's WIP.** Push with `CODEMAN_SKIP_PREPUSH=1 git push` and leave it alone. (The hook already skips itself when the tree has uncommitted changes in a path it checks, so this mostly happens once the other session has committed.)
 
 ## CRITICAL: Always Test Before Deploying
 
@@ -112,6 +113,8 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
 | Gesture playground             | `npm run dev` **in** `packages/gesture-control/` (standalone vite demo, fake tabs)                                                                                                                             |
 | Check public-asset formatting  | `npm run check:public-assets` (prettier-checks `src/web/public/**` text assets; `scripts/check-public-assets.mjs`)                                                                                             |
 | Frontend JS syntax check       | `npm run check:frontend-syntax` (`scripts/check-frontend-syntax.mjs`; runs in CI)                                                                                                                              |
+| Browser-test exclusion check   | `npm run check:browser-excludes` (`scripts/check-browser-test-excludes.mjs`; runs in CI, <1s). Fails if a test importing playwright/puppeteer is still collected by `config/vitest.ci.config.ts`; add it to `BROWSER_TEST_GLOBS` in `config/test-suites.ts` |
+| Pre-push hook                  | Installed by `npm install` (`scripts/git-hooks.mjs`, via postinstall): runs the static CI checks (~10-40s) before `git push`. Skip once: `CODEMAN_SKIP_PREPUSH=1 git push`. Skips itself with a notice when HEAD is not the pushed commit or the tree has uncommitted changes the checks would read. Marker-owned, so a hand-written `pre-push` is never overwritten; installs ONLY into the repo's own `<git-common-dir>/hooks` (worktree-safe; a `core.hooksPath` elsewhere, e.g. a global one, is left alone) |
 | Excluded-suite runners         | `npm run test:browser` · `npm run test:mobile` · `npm run test:perf` · `npm run test:all` (everything, environmental failures included) — see Testing                                                           |
 | Production start               | `npm run start`                                                                                                                                                                                                |
 | Production logs                | `journalctl --user -u codeman-web -f`                                                                                                                                                                          |
@@ -120,7 +123,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
 | Dependency doctor              | `codeman doctor` (alias `check-deps`; `--json`, `--category core\|office\|other`). Probes Node/Claude CLI/tmux/LibreOffice/MS Office against `config/dependency-registry.ts`; engine is pure given an injectable `ProbeHost` |
 | Multi-user accounts            | `codeman users add <name>` / `passwd <name>` / `list` / `rm <name>` (writes `~/.codeman/users.json`, mode 0600; see Multi-user mode)                                                                            |
 
-**CI**: `.github/workflows/ci.yml` (push to master/main + PRs, Node 22) runs two jobs: **(1)** `check:lockfile`, `typecheck`, `lint`, `check:frontend-syntax`, `format:check`, then a **server boot smoke test** (`tsx src/index.ts web --port 3151` must answer `/api/status` within 30s); **(2)** the **unit/integration test suite** via `npm run test:ci` (`config/vitest.ci.config.ts` — excludes the browser-driven `test/mobile/**` suite, `perf-*` benchmarks, and 9 Playwright tests; globs live in `config/test-suites.ts`), followed by the **`packages/xterm-zerolag-input` package tests** (a bare `npx vitest run` in that directory; its vitest is hoisted by the root `npm ci`, so no separate install, and `npm test` at the root does NOT run them). `npm test` runs this same config, so local green == CI green. Tests are tmux-safe in CI: `TmuxManager` no-ops all shell commands under `VITEST` (see Testing). A third workflow, `wiki-sync.yml`, fires only on master pushes touching `docs/wiki/**` and mirrors that directory to the GitHub wiki (browser edits to the wiki are overwritten by the next sync, so fix pages via `docs/wiki/`).
+**CI**: `.github/workflows/ci.yml` (push to master/main + PRs, Node 22) runs two jobs: **(1)** `check:lockfile`, `typecheck`, `lint`, `check:frontend-syntax`, `check:browser-excludes`, `format:check`, then a **server boot smoke test** (`tsx src/index.ts web --port 3151` must answer `/api/status` within 30s); **(2)** the **unit/integration test suite** via `npm run test:ci` (`config/vitest.ci.config.ts` — excludes the browser-driven `test/mobile/**` suite, `perf-*` benchmarks, and 14 Playwright tests; globs live in `config/test-suites.ts`), followed by the **`packages/xterm-zerolag-input` package tests** (a bare `npx vitest run` in that directory; its vitest is hoisted by the root `npm ci`, so no separate install, and `npm test` at the root does NOT run them). `npm test` runs this same config, so local green == CI green. Tests are tmux-safe in CI: `TmuxManager` no-ops all shell commands under `VITEST` (see Testing). A third workflow, `wiki-sync.yml`, fires only on master pushes touching `docs/wiki/**` and mirrors that directory to the GitHub wiki (browser edits to the wiki are overwritten by the next sync, so fix pages via `docs/wiki/`).
 
 **Code style**: Prettier (`singleQuote: true`, `printWidth: 120`, `trailingComma: "es5"`) — config lives in the **`"prettier"` key of `package.json`**, not a `.prettierrc` (keeps the repo root short; editors read it natively). `.prettierignore` stays at the root because Prettier resolves it relative to cwd. ESLint flat config (`config/eslint.config.js`) allows `no-console`, warns on `@typescript-eslint/no-explicit-any`. Ignores: `app.js`, `scripts/**/*.mjs`, `src/web/public/vendor/**`, `scripts/remotion/**`.
 

docs/wiki/Contributing.md

@@ -42,10 +42,17 @@ npm run typecheck
 npm run lint
 npm run format:check
 npm run check:frontend-syntax
+npm run check:browser-excludes
 npm test -- test/<file>.test.ts   # one file, the normal way
 npm run test:ci                    # the full CI sweep
 ```
 
+`npm install` installs a `pre-push` git hook that runs the static checks above (about 10-40s,
+machine-dependent) and blocks a push that would fail them. It skips itself when you push
+something other than the checked-out HEAD, or when the tree has uncommitted changes the
+checks would read. Skip it once with `CODEMAN_SKIP_PREPUSH=1 git push`; a
+`pre-push` hook of your own is never overwritten.
+
 **Never run bare `npm test`.** The default configuration includes browser-driven Playwright
 suites that need a live server, Chromium, and environment-specific baselines; they hang or
 fail on a normal machine. `test:ci` is the honest "run everything".

package.json

@@ -28,6 +28,7 @@
     "pretest:mobile": "node scripts/prepare-test-vendor.mjs",
     "test:mobile": "vitest run --config test/mobile/vitest.config.ts",
     "check:frontend-syntax": "node scripts/check-frontend-syntax.mjs",
+    "check:browser-excludes": "node scripts/check-browser-test-excludes.mjs",
     "fix:node-pty": "node scripts/fix-node-pty.mjs",
     "typecheck": "tsc --noEmit && tsc -p config/tsconfig.scripts.json",
     "lint": "eslint --config config/eslint.config.js 'src/**/*.ts'",

scripts/check-browser-test-excludes.mjs

@@ -0,0 +1,152 @@
+#!/usr/bin/env node
+/**
+ * Browser-test exclusion check.
+ *
+ * `npm run test:ci` must never try to drive a real browser: CI runners (and any
+ * clean checkout) have no chromium, so such a file dies with
+ * `browserType.launch: Executable doesn't exist` and takes the whole suite with
+ * it. `config/vitest.ci.config.ts` therefore excludes every browser-driven test
+ * via `BROWSER_TEST_GLOBS` in `config/test-suites.ts`. That list is maintained
+ * BY HAND, and a new browser test simply does not appear in it unless someone
+ * remembers. The omission is invisible on a developer machine that has run
+ * `npx playwright install`, where the test passes, and only shows up on a clean
+ * runner.
+ *
+ * Two deliberate design choices:
+ *
+ * 1. **Detection is by CONTENT, not filename.** Matching `*.browser.test.ts`
+ *    would miss the browser tests that predate that convention
+ *    (`inline-rename`, `opencode-resize`, `webgl-fallback`,
+ *    `terminal-copy-shortcut`, `codex-predictive-echo`). What actually makes a
+ *    file dangerous is importing a browser driver, so that is what is tested.
+ *    ⚠️ Only a DIRECT import is seen: a test that reaches playwright through a
+ *    helper module (e.g. `test/mobile/helpers/browser.ts`) is not detected, so
+ *    such a test still has to be added to `BROWSER_TEST_GLOBS` by hand.
+ *
+ * 2. **The exclusion side is answered by vitest itself**, via
+ *    `vitest list --filesOnly`, rather than by re-implementing glob matching
+ *    against the config's `exclude` array. Patterns there include `test/mobile/**`
+ *    and `perf-*`; a hand-rolled matcher that disagreed with vitest by even one
+ *    edge case would report a gap that does not exist, or miss one that does.
+ *    Asking the real resolver cannot drift from the real behaviour.
+ *
+ * The pure pieces are exported for test/check-browser-test-excludes.test.ts; the
+ * check itself only runs when this file is executed directly.
+ */
+import { readdirSync, readFileSync } from 'node:fs';
+import { join, dirname, relative, sep, resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { execFileSync } from 'node:child_process';
+
+const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..');
+const CI_CONFIG = join('config', 'vitest.ci.config.ts');
+const SUITES_FILE = join('config', 'test-suites.ts');
+
+/** Importing any one of these means the test needs a real browser binary. */
+const BROWSER_DRIVER =
+  /\bfrom\s+['"](?:playwright|playwright-core|@playwright\/test|puppeteer|puppeteer-core)['"]|\b(?:require|import)\(\s*['"](?:playwright|playwright-core|@playwright\/test|puppeteer|puppeteer-core)['"]\s*\)/;
+
+/** @param {string} source */
+export function importsBrowserDriver(source) {
+  return BROWSER_DRIVER.test(source);
+}
+
+/** @param {string} dir @returns {string[]} */
+function walk(dir) {
+  const out = [];
+  for (const entry of readdirSync(dir, { withFileTypes: true })) {
+    const path = join(dir, entry.name);
+    if (entry.isDirectory()) out.push(...walk(path));
+    else if (entry.isFile() && entry.name.endsWith('.test.ts')) out.push(path);
+  }
+  return out;
+}
+
+/**
+ * Every `*.test.ts` under `<root>/test` that imports a browser driver, as sorted
+ * repo-relative POSIX paths (the form `vitest list` prints).
+ *
+ * @param {string} root
+ * @returns {string[]}
+ */
+export function findBrowserTests(root) {
+  return walk(join(root, 'test'))
+    .filter((file) => importsBrowserDriver(readFileSync(file, 'utf8')))
+    .map((file) => relative(root, file).split(sep).join('/'))
+    .sort();
+}
+
+/**
+ * Parse `vitest list --filesOnly` output into a set of repo-relative paths. Stray
+ * blank or decorative lines are ignored rather than assuming the format is pristine.
+ *
+ * @param {string} output
+ * @returns {Set<string>}
+ */
+export function parseVitestFileList(output) {
+  return new Set(
+    output
+      .split('\n')
+      .map((line) => line.trim())
+      .filter((line) => line.endsWith('.test.ts'))
+      .map((line) => line.replace(/^\.\//, ''))
+  );
+}
+
+/**
+ * @param {string[]} browserTests
+ * @param {Set<string>} ciFiles
+ * @returns {string[]} browser-driven files that the CI config would still collect
+ */
+export function findLeaks(browserTests, ciFiles) {
+  return browserTests.filter((file) => ciFiles.has(file));
+}
+
+function main() {
+  const browserTests = findBrowserTests(ROOT);
+
+  let collected;
+  try {
+    collected = execFileSync('npx', ['vitest', 'list', '--config', CI_CONFIG, '--filesOnly'], {
+      cwd: ROOT,
+      encoding: 'utf8',
+      stdio: ['ignore', 'pipe', 'pipe'],
+    });
+  } catch (err) {
+    console.error('✗ could not enumerate the CI test set via `vitest list`.');
+    console.error(err.stderr ? err.stderr.toString() : String(err));
+    process.exit(1);
+  }
+
+  const ciFiles = parseVitestFileList(collected);
+  if (ciFiles.size === 0) {
+    // An empty list would make every browser test look excluded: fail rather than pass vacuously.
+    console.error('✗ `vitest list` reported no test files; refusing to pass on an empty CI set.');
+    process.exit(1);
+  }
+
+  const leaked = findLeaks(browserTests, ciFiles);
+
+  if (leaked.length > 0) {
+    console.error(`✗ ${leaked.length} browser-driven test file(s) are NOT excluded from ${CI_CONFIG}:\n`);
+    for (const file of leaked) console.error(`    ${file}`);
+    console.error(`
+These import a browser driver, so on a runner with no chromium they fail with
+"browserType.launch: Executable doesn't exist" and take the suite down. Add each
+to BROWSER_TEST_GLOBS in ${SUITES_FILE} (${CI_CONFIG} derives its excludes from
+it, and \`npm run test:browser\` its includes).
+
+They may well pass on this machine; that is the trap. To reproduce a clean
+runner locally:
+  PLAYWRIGHT_BROWSERS_PATH=\$(mktemp -d) PUPPETEER_CACHE_DIR=\$(mktemp -d) npm run test:ci`);
+    process.exit(1);
+  }
+
+  console.log(
+    `✓ all ${browserTests.length} browser-driven test files are excluded from the CI suite (${ciFiles.size} files collected)`
+  );
+}
+
+if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {
+  main();
+}