diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 057ed302..1b54b7e4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,3 +64,13 @@ jobs: - name: Validate generated feeds run: pnpm run validate-feeds + + - name: Validate generated content contracts + run: pnpm run validate-generated-content + + # After the build: these journeys drive the real dist/client output. + - name: Install Chromium for browser journeys + run: pnpm exec playwright install --with-deps chromium + + - name: Run browser regression journeys + run: pnpm run test:browser diff --git a/README.md b/README.md index f393192e..86a2b617 100644 --- a/README.md +++ b/README.md @@ -47,15 +47,25 @@ English is the listing language. Spanish is a twin: language toggle, title searc ## Toolchain -| Tool | Version | Source of truth | -| ------- | --------------------- | ------------------------------------------ | -| Node.js | 22.12.0 (`>=22.12.0`) | `.nvmrc`, `engines.node` in `package.json` | -| pnpm | 12.3.4 | `packageManager` in `package.json` | +| Tool | Version | Source of truth | +| ------- | ---------------- | ------------------------------------------ | +| Node.js | 22.12.0 (`22.x`) | `.nvmrc`, `engines.node` in `package.json` | +| pnpm | 12.3.4 | `packageManager` in `package.json` | CI reads both from those files — `actions/setup-node` uses `node-version-file: .nvmrc` and `pnpm/action-setup` uses the `packageManager` field — so there is no separate version to keep in sync in `.github/workflows/ci.yml`. +`engines.node` is capped to a single major on purpose. **Vercel reads +`engines.node` and it overrides the Node version in Project Settings**, so an +open range such as `>=22.12.0` silently promotes production to the newest major +Node — and keeps doing so as new ones ship. `22.x` keeps production, CI, and +`.nvmrc` on the same major. + +pnpm does not enforce `engines` unless `engine-strict` is set, so a newer local +Node still installs and runs. It is not the tested target, though: run `nvm use` +in this directory to pick up `.nvmrc` before trusting a local result. + Locally, `corepack enable` makes `pnpm` in this directory resolve to the pinned version. Without corepack, install pnpm 12 yourself; older majors may not understand every key in `pnpm-workspace.yaml`. @@ -93,32 +103,101 @@ Remark42 uses `PUBLIC_REMARK42_HOST` and `PUBLIC_REMARK42_SITE_ID` when you turn ## Scripts -| Command | What it does | -| ----------------------------------- | --------------------------------------------------------------------------------------------- | -| `pnpm run dev` | Dev server | -| `pnpm run build` | Builds social JPG/PNG from AVIF (skipped when fingerprints + files match), then `astro build` | -| `pnpm run preview` | Serves prerendered `dist/client` on :4321 (no Vercel CLI needed; see caveat below) | -| `pnpm run preview:vercel` | `astro preview` — needs the Vercel CLI installed | -| `pnpm test` | Vitest unit tests (publish filters, SEO routing, feed HTML, quotes helpers) | -| `pnpm run test:watch` | Vitest in watch mode | -| `pnpm changelog:since` | Commits + file groups since the previous version (for CHANGELOG drafts) | -| `pnpm run check` | `astro check` (TypeScript / Astro diagnostics) | -| `pnpm run lint` | ESLint | -| `pnpm run lint:fix` | ESLint with `--fix` | -| `pnpm run format` | Prettier write | -| `pnpm run format:check` | Prettier check (CI verifier; does not rewrite files) | -| `pnpm run audit-frontmatter` | Required-field / language sanity check on `src/content/p` | -| `pnpm run validate-feeds` | Validates `dist/rss.xml` + `dist/feed.json` (run after build) | -| `pnpm run validate-structured-data` | Smoke-checks structured-data module exports | -| `pnpm run generate-social-images` | AVIF → JPEG/PNG under `public/social/` only (same logic as the start of `pnpm run build`) | -| `pnpm run generate-favicons` | Favicon assets | -| `pnpm run sync-remark42-rewrite` | Regenerates the Remark42 rewrite in `vercel.json` from `REMARK42_UPSTREAM_ORIGIN` | -| `pnpm run check-remark42-rewrite` | CI check that `vercel.json`'s Remark42 rewrite matches `REMARK42_UPSTREAM_ORIGIN` | -| `pnpm run lighthouse` | Lighthouse HTML report against `dist/`; starts and stops its own preview server | -| `pnpm run audit-performance` | Same, performance category only, JSON output | +| Command | What it does | +| ------------------------------------- | --------------------------------------------------------------------------------------------- | +| `pnpm run dev` | Dev server | +| `pnpm run build` | Builds social JPG/PNG from AVIF (skipped when fingerprints + files match), then `astro build` | +| `pnpm run preview` | Serves prerendered `dist/client` on :4321 (no Vercel CLI needed; see caveat below) | +| `pnpm run preview:vercel` | `astro preview` — needs the Vercel CLI installed | +| `pnpm test` | Vitest unit tests (publish filters, SEO routing, feed HTML, quotes helpers) | +| `pnpm run test:watch` | Vitest in watch mode | +| `pnpm run test:browser` | Browser regression journeys against `dist/client` (build first; see below) | +| `pnpm changelog:since` | Commits + file groups since the previous version (for CHANGELOG drafts) | +| `pnpm run check` | `astro check` (TypeScript / Astro diagnostics) | +| `pnpm run lint` | ESLint | +| `pnpm run lint:fix` | ESLint with `--fix` | +| `pnpm run format` | Prettier write | +| `pnpm run format:check` | Prettier check (CI verifier; does not rewrite files) | +| `pnpm run audit-frontmatter` | Required-field / language sanity check on `src/content/p` | +| `pnpm run validate-feeds` | Validates `dist/rss.xml` + `dist/feed.json` (run after build) | +| `pnpm run validate-structured-data` | Smoke-checks structured-data module exports (source only) | +| `pnpm run validate-generated-content` | Parses the built site: JSON-LD, RSS XML, feed ids, canonicals, local assets, drafts | +| `pnpm run generate-social-images` | AVIF → JPEG/PNG under `public/social/` only (same logic as the start of `pnpm run build`) | +| `pnpm run generate-favicons` | Favicon assets | +| `pnpm run sync-remark42-rewrite` | Regenerates the Remark42 rewrite in `vercel.json` from `REMARK42_UPSTREAM_ORIGIN` | +| `pnpm run check-remark42-rewrite` | CI check that `vercel.json`'s Remark42 rewrite matches `REMARK42_UPSTREAM_ORIGIN` | +| `pnpm run lighthouse` | Lighthouse HTML report against `dist/`; starts and stops its own preview server | +| `pnpm run audit-performance` | Same, performance category only, JSON output | CI’s format step **checks**; it does not rewrite or open a follow-up commit. After `pnpm install`, a pre-commit hook runs Prettier on staged files so commits already match that check. +### Generated content contracts + +`pnpm run validate-generated-content` runs after a build and parses the emitted +bytes rather than the source: + +- every representative surface's JSON-LD parses and carries `@context` / `@type` +- `rss.xml` is well-formed XML by a real parser — a substring check passes on an + unescaped `&` or an unbalanced tag, which are the faults that break readers +- feed ids are unique, and RSS and JSON Feed carry the same posts +- canonical URL, feed entry URL, and JSON-LD identity agree +- site-local `og:image` targets and internal links resolve to real files +- drafts, `published: false`, and future-dated posts appear nowhere; public + English posts appear everywhere they should; Spanish twins stay out of feeds + +Surfaces covered: homepage, `/everything`, `/writing-insights`, an English essay, +a Spanish twin, and a recipe. External URLs are never fetched — a release must +not fail because someone else's site is down. Remote link health, if wanted, is a +separate optional report. + +The rules live in `scripts/lib/generated-content-checks.mjs` as pure functions +with unit tests in `pnpm test`, because the repo has no draft or future-dated +post to exercise them against. + +### Offline behaviour + +`public/sw.js` makes one promise: **everything you have already opened stays +readable offline** — article pages, their images, the browse pages you navigated +through, and the site's CSS, JS, and fonts. Anything never opened is not +available offline, and `/offline.html` says so. + +It does not pre-download the site. Measured on this build, reader-facing output +is about 118 MB: 25.9 MB of article HTML across 129 posts (~205 KB per page), +67.3 MB of images, and ~24 MB of listing pages. Precaching that on a first visit +would cost more data than most phone plans enjoy and be re-fetched on every +deploy. Caching as you read reaches the same practical result for a few hundred +KB per article. + +Never cached: `/api/*` (quotes, the Remark42 proxy) is network-only, `/social/*` +(30.3 MB of Open Graph cards a reader never displays) is skipped entirely, and +cross-origin requests are left alone. + +Caches are named `notes--v`, and an upgrade deletes only caches +carrying that prefix, so a cache belonging to anything else on the origin +survives. Page, asset, and image caches have entry ceilings (120/120/250), +trimmed FIFO. `pnpm run test:browser` covers activation, upgrade, the offline +journeys, and what must never be stored. + +### Browser regression journeys + +`pnpm run test:browser` drives headless Chromium over the real build output — +lightbox keyboard journey, search announcements and rapid clear, carousel pause +including reduced motion, mobile navigation focus return, theme persistence, and +EN/ES routing — at 390, 768, and 1440 pixels. + +It needs a completed `pnpm run build` and does not build for you: building inside +the test run would make a failure ambiguous between the build and the behavior. +First run also needs the browser binary: + +```bash +pnpm exec playwright install chromium +``` + +These use the `playwright` library through Vitest rather than `@playwright/test`, +so the repo keeps one test runner. They are a separate command from `pnpm test` +so the unit suite stays fast. Service workers are blocked in the test context — +offline behavior needs its own suite with explicit lifecycle steps. + ### Previewing and measuring a build locally `pnpm run preview` serves `dist/client` from `scripts/serve-dist.mjs`. It shows the diff --git a/docs/TECHNICAL-AUDIT.md b/docs/TECHNICAL-AUDIT.md index 90ee3501..de6da18e 100644 --- a/docs/TECHNICAL-AUDIT.md +++ b/docs/TECHNICAL-AUDIT.md @@ -119,18 +119,40 @@ No accounts. No server-side reading progress. Constitution principle IV applies. ## 7. Quality gates (actual vs claimed) -| Gate | Status | -| ----------------------------------- | -------------------------------------------------------- | -| `pnpm run format:check` | CI | -| `pnpm run check` | CI | -| `pnpm run lint` | CI | -| `pnpm run build` | CI (+ social image step) | -| `pnpm run validate-feeds` | CI, after the build (needs `dist/`) | -| `pnpm run audit-frontmatter` | CI, before the build — walks `src/content/p` | -| `pnpm run validate-structured-data` | CI — smoke-checks structured-data module surface only | -| `pnpm run check-remark42-rewrite` | CI — `vercel.json` rewrite vs `REMARK42_UPSTREAM_ORIGIN` | -| Unit tests (`pnpm test`) | CI — see §9 for coverage | -| Browser / e2e tests | **None** — Playwright is installed but unconfigured | +| Gate | Status | +| ----------------------------------- | ------------------------------------------------------------- | +| `pnpm run format:check` | CI | +| `pnpm run check` | CI — also owns unused locals/params (see below) | +| `pnpm run lint` | CI — coverage detailed below | +| `pnpm run build` | CI (+ social image step) | +| `pnpm run validate-feeds` | CI, after the build (needs `dist/`) | +| `pnpm run audit-frontmatter` | CI, before the build — walks `src/content/p` | +| `pnpm run validate-structured-data` | CI — smoke-checks structured-data module surface only | +| `pnpm run check-remark42-rewrite` | CI — `vercel.json` rewrite vs `REMARK42_UPSTREAM_ORIGIN` | +| Unit tests (`pnpm test`) | CI — see §9 for coverage | +| Browser / e2e tests | CI — 22 journeys over `dist/client` (`pnpm run test:browser`) | + +**Lint coverage (corrected 2026-09-10).** Before this pass `eslint .` reached 131 +files and applied exactly 8 rules to each — all `eslint-plugin-astro` deprecated-API +checks. `.ts` files matched no config block at all and were skipped silently, plain +`.js`/`.mjs` got the Astro rules and no JavaScript rules, and +`src/pages/writing-insights/**` was excluded outright. A green `pnpm run lint` +therefore established very little. + +It now reaches 214 files with a correctness rule set applied to `.js`, `.mjs`, +`.cjs`, `.ts`, and `.astro`, and no source directory is excluded. Removing the +Writing Insights exclusion surfaced one real defect: an unescaped `>` in +`insights.astro` that the Astro compiler tolerates but `astro-eslint-parser` +cannot parse. + +Unused symbols are split by language on purpose. Core `no-unused-vars` cannot read +TypeScript type positions — it reports every parameter name in a function type as +an unused argument — so it runs on plain JavaScript only, and `noUnusedLocals` / +`noUnusedParameters` in `tsconfig.json` cover TypeScript through `astro check`. +Both halves were confirmed by planting a deliberate violation in each file class. + +Still uncovered: type-aware lint rules. Adding `typescript-eslint` would bring +them, at the cost of a new devDependency. --- diff --git a/eslint.config.mjs b/eslint.config.mjs index e84bd586..800dd604 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -1,17 +1,103 @@ import eslintPluginAstro from 'eslint-plugin-astro'; +import tsParser from '@typescript-eslint/parser'; + +/** + * Correctness-focused core rules. These are ESLint built-ins, so they need no + * extra plugin package. + * + * Two rules are deliberately absent from this shared set: + * + * - `no-undef` needs a per-environment globals list to be accurate, and the + * `globals` package is not a direct dependency here. TypeScript already + * reports undefined identifiers across `.ts` and `.astro` via `astro check`. + * - `no-unused-vars` is applied to plain JavaScript only, further down. The core + * rule cannot read TypeScript type positions, so it reports every parameter + * name in a function *type* — `warn: (message: string) => void` — as an unused + * argument: 45 false positives and no true ones in this repo. `noUnusedLocals` + * and `noUnusedParameters` in `tsconfig.json` cover TypeScript correctly and + * run in CI through `astro check`. + */ +const correctnessRules = { + // Default 'except-parens' on purpose: it still catches an accidental + // `if (a = b)` while permitting the parenthesized `while ((m = re.exec(s)))` + // iteration idiom the social-image script uses. + 'no-cond-assign': 'error', + 'no-constant-binary-expression': 'error', + 'no-constant-condition': ['error', { checkLoops: false }], + 'no-dupe-args': 'error', + 'no-dupe-else-if': 'error', + 'no-dupe-keys': 'error', + 'no-duplicate-case': 'error', + 'no-empty': ['error', { allowEmptyCatch: true }], + 'no-fallthrough': 'error', + 'no-func-assign': 'error', + 'no-irregular-whitespace': 'error', + 'no-self-assign': 'error', + 'no-self-compare': 'error', + 'no-sparse-arrays': 'error', + 'no-template-curly-in-string': 'error', + // 'no-unmodified-loop-condition' is left off: it cannot see a binding mutated + // through a method, so `while (d <= end) { d.setMonth(...) }` in + // brainScience/data.ts reads as an infinite loop to it. Not in ESLint's + // recommended set for the same reason. + 'no-unreachable': 'error', + 'no-unsafe-finally': 'error', + 'no-unsafe-negation': 'error', + 'no-unsafe-optional-chaining': 'error', + 'no-async-promise-executor': 'error', + 'no-compare-neg-zero': 'error', + 'use-isnan': 'error', + 'valid-typeof': 'error', +}; export default [ { - // Ignore build output, dependencies, and complex Writing Insights dashboards - ignores: [ - 'dist/**', - 'build/**', - 'coverage/**', - '**/*.min.js', - 'node_modules/**', - 'src/pages/writing-insights/**', - ], + // Build output and dependencies only. Source directories are not excluded: + // an ignored directory makes a green lint run mean less than it appears to. + ignores: ['dist/**', 'build/**', 'coverage/**', '.vercel/**', '**/*.min.js', 'node_modules/**'], }, - // Astro + JavaScript/TypeScript recommended rules for .astro files and scripts + + // Astro's own rules for .astro files. ...eslintPluginAstro.configs.recommended, + + // Plain JavaScript: scripts/, public/, and config files. TypeScript does not + // check these (`checkJs` is off), so ESLint owns unused symbols here. + { + files: ['**/*.js', '**/*.mjs', '**/*.cjs'], + languageOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + }, + rules: { + ...correctnessRules, + 'no-unused-vars': [ + 'error', + { + args: 'after-used', + argsIgnorePattern: '^_', + varsIgnorePattern: '^_', + caughtErrors: 'none', + ignoreRestSiblings: true, + }, + ], + }, + }, + + // TypeScript. Flat config lints no .ts file unless a config block claims it, + // so without this entry every .ts file in src/ was silently skipped. + { + files: ['**/*.ts', '**/*.mts', '**/*.cts'], + languageOptions: { + parser: tsParser, + ecmaVersion: 'latest', + sourceType: 'module', + }, + rules: correctnessRules, + }, + + // The same rules inside .astro frontmatter and