diff --git a/CLAUDE.md b/CLAUDE.md index fee6ba1a..bce3b3fc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -740,7 +740,13 @@ covers the full import graph each function deploys with. If you prefer raw pnpm (or mise isn't available - ephemeral sandboxes, first-time checkouts), the manual sequence is -`pnpm install && pnpm test && pnpm check && pnpm lint && pnpm build`. +`pnpm install && pnpm test && pnpm check && pnpm lint && pnpm build +&& pnpm knip`. Cloud sessions cannot run `mise run check` at all: +mise resolves the whole `[tools]` set before any task, and aqua reads +version lists from the GitHub releases API, which a session scoped to +this repo may not reach. The raw sequence is the way through. See +[`docs/dev/testing.md`](docs/dev/testing.md) for the misleading error +it surfaces as. `pnpm build` is in the gate because Vite/Rollup failures (IIFE/code-splitting in worker bundles, PWA manifest injection, dynamic-import graphs tsc is happy with but Rollup chokes on) only @@ -756,6 +762,20 @@ markdown-only changes. The test suite includes a postcss parse of every stylesheet under `src/` (`tests/styles.test.ts`) and a markdownlint pass over the doc tree (`tests/markdownlint.test.ts`). +**Adding a test that mounts a component or touches a DOM global? +Register it in `environmentMatchGlobs` in `vite.config.ts` in the +same change.** vitest defaults to the `node` environment and jsdom +is opt-in per file BY NAME, so a new file that needs a DOM dies on +`ReferenceError: document is not defined`. The list cannot know +about a file that did not exist when it was written, which makes +this invisible on a branch that forked before the split: the test +passes locally under the old jsdom default, the rebase is clean, the +files are disjoint, and it fails only in CI. + +Full details - the other two test islands, the guardrail tests, +integration-test gating, `tests/setup.ts` - are in +[`docs/dev/testing.md`](docs/dev/testing.md). + ### Check exit codes, not piped output Piping a gate command (`mise run check 2>&1 | tail -2`) replaces @@ -778,7 +798,7 @@ bitten us: imported by Y, dynamic import will not move module into another chunk.` Means your code-splitting isn't actually splitting because another module pulls the same target statically. -- `(!) Some chunks are larger than 500 kB after minification.` +- `(!) Some chunks are larger than 750 kB after minification.` Advisory; the asset list tells you which chunk is over. When doing bundle-shape work, grep the build output for `(!)` and diff --git a/docs/dev/README.md b/docs/dev/README.md index c21444a7..64beba44 100644 --- a/docs/dev/README.md +++ b/docs/dev/README.md @@ -170,6 +170,10 @@ changing a contract that other features depend on. - [Local dev stack](./local-stack.md) — a throwaway local Supabase backend (`mise run dev-start`) isolated from the linked cloud project, for schema work without touching prod. +- [Testing](./testing.md) — the three test islands (vitest, + Deno, Playwright), the vitest node/jsdom environment split + and how to register a new DOM test, the CSS + markdown + guardrail tests, and integration-test gating. ### Future work diff --git a/docs/dev/build-deploy.md b/docs/dev/build-deploy.md index 92975053..755f6a6d 100644 --- a/docs/dev/build-deploy.md +++ b/docs/dev/build-deploy.md @@ -170,6 +170,11 @@ docs imported via `import.meta.glob`. data-touching feature depends on. A broken schema migration halts the deploy; a broken build halts the publish. +- **Testing** — the gate (`mise run check`) exists to keep a + green Tests job from landing a change that breaks the + deploy: `pnpm build` runs in the gate, and the postcss and + markdownlint guardrail tests close the CSS and markdown + versions of the same hole. See `./testing.md`. - **Help** — Vite's `import.meta.glob` is what makes the user docs ship with the PWA. Each doc file becomes its own lazy chunk in `dist/assets/`. See `./help.md`. diff --git a/docs/dev/in-progress/mise-in-cloud-sandbox.md b/docs/dev/in-progress/mise-in-cloud-sandbox.md new file mode 100644 index 00000000..0ef22e5e --- /dev/null +++ b/docs/dev/in-progress/mise-in-cloud-sandbox.md @@ -0,0 +1,145 @@ +# Why `mise run check` cannot run in a cloud agent sandbox + +**Status: open. Research only - no remediation is proposed here.** + +Cloud agent sessions (Claude Code on the web) cannot run the gate +through mise and have to fall back to the raw pnpm sequence. This +records what the failure actually is, since the error message names +the wrong cause and two plausible-sounding diagnoses turn out to be +wrong. + +Nothing here argues for a fix. Whether the sandbox *should* be able +to run mise is a separate decision that has not been made. + +## Symptom + +```text +mise ERROR Failed to install tools: aqua:charmbracelet/gum@latest, aqua:cli/cli@latest + +aqua:charmbracelet/gum@latest: no versions found for +aqua:charmbracelet/gum matching date filter +``` + +The gate never starts. No gate task runs, so nothing is checked. + +## Confirmed mechanism + +1. **mise resolves the entire `[tools]` set before running any + task.** The gate needs node, pnpm, and deno. It does not use + `gh` or `gum` at all. That does not matter: an unresolvable entry + anywhere in `[tools]` aborts before task dispatch. + +2. **Exactly two entries fail**, and both float on `latest`: + + ```toml + "aqua:cli/cli" = "latest" # fails + "aqua:charmbracelet/gum" = "latest" # fails + "aqua:supabase/cli" = "2.101.0" # fine + ``` + +3. **`latest` forces a GitHub releases-list API call**, and that + call is refused: + + ```text + 403 Forbidden for https://api.github.com/repos/cli/cli/releases?per_page=100 + 403 Forbidden for https://api.github.com/repos/charmbracelet/gum/releases?per_page=100 + ``` + + The response body is the agent proxy's own message: "GitHub + access to this repository is not enabled for this session." The + session's GitHub access is scoped to `sysread/nak`, and + `charmbracelet/gum` and `cli/cli` are not it. + +4. **An exact pin needs no list call.** `aqua:supabase/cli` is + pinned, never queries the releases endpoint, and **installs + successfully** in this same sandbox. It is the natural control in + the experiment: same backend, same host, different version + spec, opposite outcome. + +5. **Binary downloads are not blocked.** Follows from (4) - the + pinned tool's release asset downloaded fine. Only the + `api.github.com` version-list query is refused, not the release + asset fetch. + +6. **The reported cause does not exist.** mise renders an empty + version list as "no versions found matching date filter." There + is no date filter: `mise settings` has no date, age, cutoff, or + filter key, and there is no global mise config in the sandbox. + The phrase is mise's generic wording for "the list came back + empty," and it sends you looking for a knob that is not there. + +## Dead ends, so nobody re-walks them + +- **"It is a date/version cutoff somewhere."** No. See (6). +- **"The whole aqua backend is broken in the sandbox."** No. See + (4) - a pinned aqua tool installs. +- **"The session cannot reach GitHub."** Too broad. The GitHub MCP + tools work all session; they use a separate credentialed path. + What is unavailable is `api.github.com` to arbitrary CLI tools. +- **"Only `latest` is the problem, so aqua is fine with pins."** + True as far as tested, but see the open questions - nobody has + actually pinned `gh`/`gum` and re-run the gate. + +## A probe that gives a false negative + +Testing resolution by exit code alone reports success on the +failing tools: + +```sh +mise ls-remote "aqua:charmbracelet/gum@latest" >/dev/null 2>&1 && echo resolves +``` + +`ls-remote` exits 0 with an empty list, so this prints `resolves` +for a tool that cannot resolve. It also appears to pass from a +directory with no `.mise.toml`. Read the warnings, or run +`mise run check` itself - the exit code of a subcommand is not +evidence here. + +## Evidence + +Gathered in a cloud session on mise `2026.7.11 linux-x64`: + +- `mise run check` - fails as above; `supabase/cli` absent from the + failure set. +- `mise ls --installed` - shows `aqua:supabase/cli 2.101.0` + installed. +- `curl https://api.github.com/repos/charmbracelet/gum/releases` - + 403 with the proxy's repo-access message. +- `curl https://api.github.com/repos/sysread/nak` - also 403, so + raw `api.github.com` is not usable by CLI tools even for the + in-scope repo. +- `mise settings` - no date/age/cutoff/filter key. +- `mise ls-remote node@20.11.1`, `pnpm@10.33.4`, `deno@2.8.0` - all + resolve; the core backends are unaffected. + +## Open questions + +- **Would pinning `gh` and `gum` unblock the gate?** Suggestive from + (4) but untested. Pinning also has a cost the pin comment on + `supabase/cli` already spells out: a pinned tool stops tracking + upstream and needs deliberate bumps. +- **Can mise scope a tool to specific tasks** in 2026.7.11, so the + gate does not resolve wizard-only tools? Unverified - do not + assume the syntax exists. +- **Is the `api.github.com` denial deliberate egress policy?** + `/root/.ccr/README.md` says a 403 means the host is not allowed + for the session and to report the blocked host rather than route + around it. If the denial is intentional, "make aqua work" is the + wrong frame and the fallback is the answer. +- **Does this affect the local CLI or CI at all?** No evidence + either does; both resolve these tools normally. The blast radius + looks confined to cloud agent sessions, but that has not been + checked against a non-Anthropic sandbox. + +## What to do meanwhile + +Use the raw pnpm sequence, which is what the cloud sessions already +do: + +```sh +pnpm install && pnpm test && pnpm check && pnpm lint && pnpm build && pnpm knip +``` + +It skips the Deno island (`functions-check`, `functions-test`), so a +change under `supabase/functions/` is not fully covered by it. See +[`../testing.md`](../testing.md). diff --git a/docs/dev/testing.md b/docs/dev/testing.md new file mode 100644 index 00000000..45ce1c62 --- /dev/null +++ b/docs/dev/testing.md @@ -0,0 +1,288 @@ +# Testing + +How the test suites are wired, what each one exists to catch, and +the setup details that are easy to trip over. + +## Role in the app + +There are **three independent test islands**, and they do not see +each other: + +1. **vitest** (`tests/*.test.ts`) - the main-thread suite. Pure + logic, UI-behavior primitives, and component mounts. +2. **Deno** (`supabase/functions/tests/`) - the edge functions have + their own toolchain and are invisible to vitest. +3. **Playwright** (`e2e/*.spec.ts`) - browser-driven end-to-end + specs, run on demand rather than in the gate. + +The vitest suite also carries two **guardrail tests** that are not +really unit tests: a postcss parse of every stylesheet, and a +markdownlint pass over the doc tree. Both exist because their +failure modes otherwise surface only at deploy time or as a +silently-wrong render. + +## Files + +- `vite.config.ts` - the `test` block: environment split, setup + file, include/exclude globs, integration-test gating. +- `tests/setup.ts` - `setupFiles` for **every** vitest file, node + and jsdom alike. +- `tests/styles.test.ts` - postcss parse of every stylesheet under + `src/`. +- `tests/markdownlint.test.ts` - markdownlint over the tracked + markdown. +- `playwright.config.ts`, `e2e/` - the browser specs. +- `.mise.toml` - the gate task graph (`check` and its parallel + dependencies). +- `supabase/functions/deno.json` - import map the Deno tasks point + at. + +## The vitest environment split + +**`environment: 'node'` is the default. jsdom is opt-in, per file, +by name.** + +```ts +environment: 'node', +environmentMatchGlobs: [ + ['tests/ascii-spinner.test.ts', 'jsdom'], + // ... +], +``` + +Most test files are pure logic with no DOM dependency, and jsdom +costs roughly **350ms of bootstrap per file**. With ~128 test files +that was tens of seconds of aggregate environment time and 4-5s of +wall clock on every run. Listing the dozen files that genuinely need +a DOM buys that back. + +**When you add a test that mounts a component or touches a DOM +global, add it to `environmentMatchGlobs` in the same change.** +Things that need jsdom: component mounts (`@testing-library/svelte`), +`localStorage` / `sessionStorage`, the history API, DOMPurify, +`fake-indexeddb`. + +### Why this bites on a branch that forked before the split + +The list enumerates files **by name**, so it cannot know about a +file that did not exist when it was written. That makes the failure +invisible to every signal a branch normally trusts: + +- the test passes locally if the branch forked while jsdom was still + the default; +- the rebase is clean, because the two changes touch different + files; +- the gate is green before the rebase, and `git` reports no + conflict after it. + +It then fails in CI with `ReferenceError: document is not defined`. + +The inline comment in `vite.config.ts` tells you to add the file +*once you see that error*, which is the right instruction for a +fresh branch and useless for a rebased one. Treat "did I add a DOM +test?" as a rebase checklist item, not something the tooling will +remind you about. + +To confirm an entry is actually load-bearing rather than +cargo-culted, delete it and run the file - a CLI +`--environment node` flag will **not** override +`environmentMatchGlobs`, so the flag alone proves nothing. + +## `tests/setup.ts` + +Runs for every file in both environments, so everything in it is +either environment-agnostic or guarded: + +- **`@testing-library/jest-dom/vitest`** is imported here rather + than per file, so any component test gets + `expect(el).toBeInTheDocument()` without a local import. +- **`globalThis.crypto`** is polyfilled from `node:crypto` if absent + or missing `subtle`. +- **`Element.prototype.animate`** is shimmed because jsdom does not + implement it and Svelte's `slide` / `fade` transitions call it. + Guarded on `typeof Element !== 'undefined'` so the node + environment skips it. The shim only needs `cancel()` for Svelte's + lifecycle to clean up; the animation is a no-op, which is fine + because tests assert on state, not interpolated styles. + +## Guardrail tests + +Neither of these tests a feature. Both exist because a green Tests +job used to be compatible with a broken deploy. + +- **`tests/styles.test.ts`** runs postcss over every stylesheet + under `src/`. A stray `}` in `src/styles.css` once got through: + Vite's dev server kept rendering, `pnpm check` and `pnpm test` + do not parse CSS, and the error surfaced only at `pnpm build` - + in the deploy workflow, after Tests had gone green. +- **`tests/markdownlint.test.ts`** runs markdownlint-cli2's + programmatic `main()` (not a child process) over the tracked + markdown. Docs render in three places - GitHub, the in-app Help + modal, and Claude sessions reading `CLAUDE.md` - and a broken + fence ladder shows up as a wrong render rather than a loud + failure. + +**Consequence: run the suite for CSS-only and markdown-only +changes too.** There is no such thing as a change too cosmetic for +the gate. + +## Integration tests + +Files matching `tests/**/*.integration.test.ts` hit live Venice and +are **excluded unless `VENICE_INFERENCE_KEY` is set**: + +```sh +VENICE_INFERENCE_KEY= pnpm test tests/web-search.integration.test.ts +``` + +Presence of the key **is** the opt-in - there is no separate flag to +remember. The default `pnpm test` stays hermetic, so CI never +depends on outbound network or a credential. + +## The Deno island + +The edge functions run on their own toolchain and are invisible to +vitest. Two tasks cover them, and the gate runs both: + +- **`mise run functions-test`** - offline unit tests (fake fetch, no + network, no Supabase) over the pure logic across the functions + tree: `_shared`, the `venice/agents/` modules, and the GC + entrypoints. Handler glue is exercised via `dev-start`'s + `functions serve`, not here. (The task's own description says + "pure logic in `_shared`", which undersells it - the suite has + grown well past that.) +- **`mise run functions-check`** - `deno check` over every function + **entrypoint**. + +`functions-check` is the one that is easy to think redundant and +is not. `deno test` only type-checks the graph its tests import, +and `supabase functions deploy` bundles with esbuild and never +type-checks at all. Without the entrypoint check, a type error in a +handler-graph corner no test imports surfaces **nowhere** and ships +latent. + +It `depends = ["bundle-docs"]` because the venice function imports +the gitignored research-docs corpus, which does not exist on a +fresh clone until the bundler runs. + +## Playwright + +`playwright.config.ts` plus `e2e/` (`setup.spec.ts`, +`setup-hash.spec.ts`), run with `pnpm test:e2e`. **Not in the +gate** - they drive a real browser against a running stack. + +Cloud agent sessions cannot run these, or verify anything visual; +see the "Verifying UI changes" section of `CLAUDE.md` for the +posture that replaces them there. + +## Test-only exports + +Production modules expose internals to tests through a single +`__test = { ... }` namespace export at the bottom of the file +rather than widening their real API one symbol at a time. See +`src/lib/session.ts`, `src/lib/routing.svelte.ts`, +`src/lib/offline-sync.svelte.ts`, `src/lib/pdf-pages.ts`. + +Knip runs in the gate, so an export with no external consumer is a +gate failure, not a lint suggestion. + +## Contracts + +- **The gate is `mise run check`**, and it is what + `.github/workflows/tests.yml` runs, so green locally means green + CI. Its components are parallel `depends`, not a serial `run` + list, so all of them report even when one fails. +- **Every gate task `depends = ["deps"]`**, which runs `pnpm + install --frozen-lockfile`. No separate install step from a fresh + clone or worktree. +- **UI-behavior primitives are tested as plain vitest cases** - no + mount, no harness. That is the point of extracting them out of + `.svelte` files; see `./frontend-organization.md`. +- **New DOM-touching test files register in + `environmentMatchGlobs`.** + +## Interactions with other features + +- **Build & deploy** - the gate exists to keep a green Tests job + from landing a change that breaks the deploy. `pnpm build` is in + the gate for exactly that reason, and the two guardrail tests + close the CSS and markdown versions of the same hole. See + `./build-deploy.md`. +- **Frontend organization** - the extract-primitives-to- + `src/lib/ui/` rule is what keeps most of the suite in the fast + node environment; logic that stays in a `.svelte` file can only + be tested by a jsdom mount. See `./frontend-organization.md`. +- **Help** - `tests/markdownlint.test.ts` guards the docs the Help + modal renders. See `./help.md`. +- **Local dev stack** - edge-function handler glue is exercised + against `dev-start`'s `functions serve`, not by + `functions-test`. See `./local-stack.md`. +- **Every feature with a QA walkthrough** - `docs/qa/use-cases/` + covers the seams these suites cannot reach. See + `docs/qa/README.md`. + +## Gotchas + +- **A CLI `--environment` flag does not override + `environmentMatchGlobs`.** Passing + `--environment node` to a file that is on the jsdom list still + gets jsdom. To test what an unlisted file would do, remove the + entry. +- **`mise` may not resolve its tools in a restricted sandbox, and + the error it prints is a red herring.** You get + `aqua:charmbracelet/gum@latest: no versions found for + aqua:charmbracelet/gum matching date filter`, which fails + `mise run check` before any gate task runs. **There is no date + filter.** aqua reads version lists from the GitHub releases API, + and a session whose GitHub access is scoped to this repo alone + gets denied for `charmbracelet/gum` and `cli/cli`; mise reports + the resulting empty list with that phrasing. Do not go looking + for a cutoff setting - `mise settings` has none. + + Note the blast radius is wider than the tools involved: **mise + resolves the entire `[tools]` set before running any task**, so + `gum` and `gh` - which only `supabase-init`, `doctor`, + `bootstrap`, and `setup-pages` use, and which already degrade + gracefully when absent - block the gate, which needs neither. + Removing one would not help while the other remains. Only the two + entries floating on `latest` fail; `aqua:supabase/cli` is pinned to + an exact version, never queries the releases list, and installs + fine. Full investigation in + [`./in-progress/mise-in-cloud-sandbox.md`](./in-progress/mise-in-cloud-sandbox.md). + + The fallback is the raw pnpm sequence (`pnpm install && pnpm test + && pnpm check && pnpm lint && pnpm build && pnpm knip`) - note it + must include `knip`, which is part of the gate. That sequence skips + the Deno island, so a change touching `supabase/functions/` is not + fully covered by it. +- **Piping a gate command replaces its exit code** with the pipe + tail's, so a failed gate reads as success and a chained `git + commit` runs anyway. Capture the status explicitly or run the + gate un-piped. See the `CLAUDE.md` section on this - it has + shipped broken commits twice. +- **Exit 0 is necessary but not sufficient.** `pnpm build` emits + Rollup warnings that do not fail the gate but do signal real + problems - grep the output for `(!)` when doing bundle-shape + work. +- **A script-less `.svelte` file has no inferable component type.** + svelte-check fails its importers with "Could not find a + declaration file for module ... implicitly has an 'any' type". + A component that needs no props still needs an empty + `