Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 50 additions & 38 deletions .claude/rules/testing.md
Original file line number Diff line number Diff line change
@@ -1,56 +1,68 @@
# Testing

## Commands
Read [docs/testing.md](../../docs/testing.md) for the shared policy and commands.

```bash
turbo test # Run tests across all packages
```
## Test value

Each package's `test` script passes `--parallel=N` (issue #261): test files
run in worker processes and **every file gets a fresh module registry** - a
fresh `db` singleton with its own database file. Two consequences for suite
authors: a suite that touches the DB must migrate it itself (server/api:
`@/__tests__/helpers/test-database.js`) and may assume neither an empty DB
nor a sibling file's boot; and runner-wide cleanup cannot live in a preload
`afterAll` (those fire per file under `--parallel`), it belongs in the
package's `test` script. A bare `bun test` still runs serially - bun ignores
a `parallel` key in `bunfig.toml [test]` (measured on 1.4.0 and 1.4.2), so
the flag lives in the scripts only. `SUBSHELL_TEST_PARALLEL` overrides a
package's width. CI instead sets `SUBSHELL_TEST_SERIAL=1`, which drops the
flag entirely and adds `--retry=1`: the hosted container runs a fraction of
its labelled cores and starves time-budgeted tests at every worker width, so
CI gets the serial regime plus a shard matrix for speed (`--parallel=1` is
NOT serial - it still isolates per file; omit the flag for serial).
Correctness is the goal; test count and coverage percentage are not. Add a test
when it catches a plausible future bug in behavior we own. Name that failure
and the assertion that would catch it before writing the test. A test is also
an executable record of expected behavior: make its scenario and observable
outcome clear, and assert state, data, requests, side effects, or refusal rather
than only that execution completed. Prioritize
failures involving authorization, privacy, data loss, protocol/artifact
compatibility, process lifecycle, cleanup, and asynchronous ordering. Use the
boundary where the regression becomes observable, and test the refusal path
when it can fail independently.

## Guidelines
Do not require a test per function, component, hook, branch, or file. Do not add
tests for types already checked by TypeScript, copied implementation logic,
fixed explanatory copy, CSS classes, trivial prop forwarding, or third-party
behavior. Test text only when it is itself a behavior contract, such as a wire
or CLI format, machine-readable error, or safety warning whose mismatch could
lead to an unsafe action. Review routine wording and visual layout as content.
Keep accessibility and interaction tests when they verify behavior we own. A
second layer must catch a distinct failure; if one test already catches the
regression, another copy of that assertion adds no confidence.

**Always write tests for new features.** Every new service, repository, route, or significant function should have corresponding tests. Tests should cover:
For a bug fix, reproduce the failure at the appropriate boundary when practical
and confirm the regression assertion would fail against the buggy behavior.
Prefer adding a case to an existing suite over creating a new file and preload.
Do not increase test count or coverage percentages as an end in itself.

- Happy path (expected behavior)
- Edge cases (empty inputs, null values, boundaries)
- Error conditions (invalid inputs, not found cases)
Reuse existing boundary coverage. The API's composed-route authorization suite
already rejects anonymous and node-key requests on every guarded route; don't
repeat a bare 401 case in each route file. Keep separate tests for permission
scopes, ownership, conditional authentication, body-dependent behavior, and
refused writes leaving state unchanged.

Test files live in `__tests__/` directories alongside the code they test:
```
src/services/
├── logs.service.ts
└── __tests__/
└── logs.service.test.ts
Drive polling through a controlled scheduler or clock when elapsed wall time
isn't the behavior under test. Make successful fakes supply the response the
real operation waits for, rather than silently exercising its timeout fallback.
Keep real deadline/process tests when they verify cancellation or termination.

src/db/repositories/
├── logs.repository.ts
└── __tests__/
└── logs.repository.test.ts
```
## Running tests

When fixing bugs, add a test that reproduces the bug before fixing it to prevent regressions.
While editing, run the covering files from their package directory. Before
handoff, inspect `bun run test:changed -- --plan`, then run the affected checks
once. Use package `test` scripts for suite runs; they own parallelism and cleanup.
Use `env -u SHELLOPTS -u BASHOPTS` in automation shells, capture the output once,
and inspect failures from that capture. Confirm the expected file count ran.

Bun's `--parallel` isolates each file. DB suites migrate their own database;
runner-wide cleanup belongs in the package runner, not preload `afterAll`.
`SUBSHELL_TEST_PARALLEL` overrides local width. CI routes packages across at most three jobs. The server lane uses
`--parallel=1` for file isolation: its fixtures mutate process-global state.
Other lanes retain serial execution. Tests in release verification
run without retries; a flaky failure needs fixing, not a green retry.

## Server-backed search coverage

Test the HTTP boundary, not just the matching helper: maximum and over-limit
lengths (including Unicode), invalid enum/format values, literal SQL/regex-like
text, and private records or nested members that must remain undiscoverable.
Retain anonymous and scoped-bearer authorization checks when adding filters.
Use the composed-route suite for anonymous rejection. Retain filter-specific
scoped-bearer and visibility checks, including private results staying hidden.

UI tests should verify debounced requests, immediate select/clear actions,
stale-response isolation, and a failed search recovering through Retry without
Expand All @@ -61,4 +73,4 @@ handler so they assert the API's actual error status and body.

## After Making Code Changes

After any code change, check if tests need updating due to changed behavior (e.g., different error types, response formats, new error conditions). Then run full verification - see `verification.md`.
After any code change, check if tests need updating due to changed behavior (e.g., different error types, response formats, new error conditions). Run the checks affected by that behavior; see `verification.md`.
104 changes: 30 additions & 74 deletions .claude/rules/verification.md
Original file line number Diff line number Diff line change
@@ -1,84 +1,40 @@
# Verification

**Run verification focused while iterating, and full at task boundaries** (operator ruling 2026-09-24: the full trio on every edit stopped adding signal and started costing minutes).
**Verify the affected behavior, including at task boundaries.** A push, PR,
merge, or handoff does not justify running unrelated packages. The shared
policy is [docs/testing.md](../../docs/testing.md).

**While iterating**, only what your change can break:
While iterating, run covering test files, the package's type check, and Biome
on changed code. Before handoff, inspect and execute the affected plan:

```bash
bun test <the covering test files you touched> # check the FILE COUNT — bun silently skips paths that don't exist
bunx turbo verify-types --filter=<pkg> # the package you changed, not all 52
bunx biome check <changed paths> # read-only; `bun run lint` to fix
bun run test:changed -- --plan
bun run verify:changed
```

**At a task boundary**, before any push, PR, merge, or handoff, run the full set once:

```bash
bun run verify-types
bun run lint:check
bun run lint:prose
bun run test
```

`lint:prose` enforces the voice rule (no U+2014 in authored prose, operator
ruling 2026-09-25) in ~20 ms; it joined this set after PR #269 shipped two em
dashes that only CI's lint workflow caught, because neither the trio above (as
it then read) nor pre-push covered it.

If any of these fail, fix the issues before considering the task complete. Do not proceed to commits or other work until the checks relevant to that step pass. Subagent gate contracts should name the focused files for the work and reserve the full suite for the boundary.

`pre-push` runs the fast static checks (types, `lint:check`, licenses, design
tokens, prose) but NOT the test suite: CI owns that. Run `bun run test`
yourself before pushing anything you expect to be green on the first try.

## Changes under `packages/` need a build

```bash
bunx turbo build
```

`verify-types`, `lint:check` and `test` all run against SOURCE and will pass
while the workspace build is broken. The trap that catches people is
`@internal/subshell-protocol`: **`apps/client/mobile` imports its barrel through
Metro, which cannot resolve `node:*`**, so adding a node-builtin import to
`src/index.ts` (directly or by re-exporting a module that has one) breaks the
mobile build and NOTHING ELSE. All three local commands stay green and CI fails
on a package you never opened. Node-only modules are subpath exports instead
(`release-artifacts`, `service-test-safety`); `src/index.ts` says so at the
export site. Measured 2026-09-15, twice in one run.

## Rust changes need a fourth command

Those three commands do not touch Rust at all. If you changed anything under
`crates/desktop-core` or either `apps/*/desktop/src-tauri`, also run:

```bash
bun run rust:check
```

It runs exactly what CI's Rust jobs run (`cargo fmt --check`, then
`cargo clippy --all-targets -- -D warnings`, then `cargo test`) in all three
crates.

**`cargo check` on its own is not enough, and the two app crates cannot even be
compiled without help.** `tauri-build` refuses to build when an `externalBin`
path is missing, and the staged sidecar is a gitignored ~110 MB build input, so
a plain `cargo clippy` there dies inside the build script instead of reporting a
lint. `rust:check` stages a stub for the host triple first (the same trick
test.yml uses) and removes only what it created, so a real sidecar staged by a
release in progress is left alone.

This was added after a hand-written import block failed `cargo fmt --check` on
both desktop shards in CI, caught by neither `cargo check` nor clippy (a
full CI round trip to discover something a local command finds in seconds).

**On macOS it refuses early when `cc` cannot link** (`scripts/macos-toolchain-preflight.sh`,
shared with the `tauri dev` launchers). An unaccepted Xcode licence stops every
cargo build, and cargo reports that as a `note:` about a hundred lines above an
error naming a SOURCE FILE, so a machine that cannot link anything reads as one
broken doctest in a file you just edited. Measured on 2026-09-15. The remedy is
`sudo xcodebuild -license`, or `export DEVELOPER_DIR=/Library/Developer/CommandLineTools`,
whose toolchain the licence gate does not cover. A link failure the probe does
not recognise is reported verbatim rather than blamed on the licence.
The planner includes committed changes since `origin/main`, staged changes,
unstaged changes, and untracked files. `--base <ref>` selects another base.
Unknown inputs or missing history select full verification and explain why.
Do not silently narrow that fallback; fetch the correct base if it is missing.
For a documentation-only change, run prose/content validation instead of product
suites. Do not repeat checks that already passed unless new changes affect them.

`verify:changed` builds and type-checks changed packages and their transitive
consumers. This catches failures source-only tests miss, including a shared
protocol import that Metro cannot bundle. It also runs the selected tests,
compiled/browser checks, and Rust crates. A Rust app edit checks that app;
a shared desktop-core change checks both apps too. `rust:check` still stages
missing sidecar stubs and removes only the stubs it created.

Fix relevant failures before handoff. Report the checks actually run and any
blocked tier. Never describe a partial run as the full suite. `bun run test`
is workspace unit tests only; `bun run test:full` includes the other tiers.
Full verification belongs to release cuts and explicit broad verification,
not every task boundary. Release CI requires a fresh full run on the release
revision before tags or artifacts are published, on Linux and macOS.

The pre-push hook runs affected static checks plus the fast repository contract
checks. CI owns the affected test tiers; the hook does not duplicate them.

## The CLI end-to-end suite

Expand Down
30 changes: 30 additions & 0 deletions .github/actions/setup-ci/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
name: Set up verification
description: Install the pinned JS toolchain and restore dependency/build caches.
inputs:
cache-suffix:
description: Keep independently populated Turbo caches from racing to save the same key.
default: checks
runs:
using: composite
steps:
- name: Trust the workspace
shell: bash
run: git config --global --add safe.directory "$GITHUB_WORKSPACE"
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0
with:
bun-version: 1.4.4
- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
with:
node-version: 24
- uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: ~/.bun/install/cache
key: ${{ runner.os }}-bun-${{ hashFiles('bun.lock') }}
restore-keys: ${{ runner.os }}-bun-
- uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: .turbo/cache
key: ${{ runner.os }}-turbo-${{ github.sha }}-${{ inputs.cache-suffix }}
restore-keys: ${{ runner.os }}-turbo-
- shell: bash
run: bun install --frozen-lockfile
21 changes: 13 additions & 8 deletions .github/workflows/docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,8 @@
# release — and the retry semantics differ: a docs tag already on this commit
# means "redeploy" (below), where for release.yml it would mean "skip".
#
# No CI gate, deliberately. There was one: a `ci-gate` job waited for this
# commit's push-triggered Test + Lint runs and refused anything but success.
# The operator removed it on 2026-09-25, the day a flaky job in an unrelated
# package's Test run blocked BOTH site deploys until a human re-ran it. What
# proves a static site is `lint.yml` building this export on every push (and
# PR CI before that); the gate only coupled deploy health to server/node test
# luck, which for a docs site is borrowing someone else's red light.
# Every release cut requires a fresh full verification of this revision,
# including site releases. A failure blocks tag creation and deployment.
#
# Flow:
# plan — refuse any run not on refs/heads/main, resolve the version
Expand Down Expand Up @@ -49,7 +44,17 @@ concurrency:
cancel-in-progress: false

jobs:
verify:
name: Full release verification
uses: ./.github/workflows/test.yml
with:
full: true
permissions:
contents: read
packages: read

plan:
needs: verify
name: Plan the deploy
runs-on: ubuntu-24.04
timeout-minutes: 10
Expand Down Expand Up @@ -124,7 +129,7 @@ jobs:
bun-version: 1.4.4

# next's bin carries a `#!/usr/bin/env node` shebang — same reason
# lint.yml pins node: a runner image floats its own node, and the
# test.yml pins node: a runner image floats its own node, and the
# pinned version (the root package.json's `engines.node`) is the
# contract, not the image's mood.
- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
Expand Down
Loading
Loading