Files
callstack__agent-device/docs/agents/testing.md
Michał Pierzchała 1a76344685 docs: restructure AGENTS.md and CONTEXT.md for progressive disclosure (#1402)
* docs: restructure AGENTS.md and CONTEXT.md for progressive disclosure

Apply the Claude 5 context-engineering guidance to the repo's agent docs:
keep the always-loaded file to gotchas and invariants, and move situational
guidance one hop away behind a routing table.

AGENTS.md 315 -> 229 lines. Cut generic agent-behavior boilerplate, three-way
duplication (Common Mistakes restated Hard Rules; Finding Source Owners
restated the registry section), and facts visible from the repo itself.
Kept verbatim: the expensive-lessons principles, enforcement gates, Hard
Rules, and environment traps.

Split out docs/agents/{cli-flags,pull-requests,device-verification}.md and
folded the Testing Matrix into docs/agents/testing.md, reframed around
pnpm check:affected so the prose stops duplicating the selector.

CONTEXT.md keeps all 50 terms, now grouped under a section index so a task
loads one section instead of the whole glossary.

* fix(check-affected): move the selector-owning sentinel to the Testing Matrix

The Testing Matrix moved from AGENTS.md to docs/agents/testing.md, but the
affected-check selector still treated only AGENTS.md as selector-owning. A
later matrix edit would have been classified as inert docs and skipped the
fail-open, so the selector could keep deriving gates from a spec that had
changed underneath it.

Move the sentinel with the prose, as a named SELECTOR_OWNING_DOCS set so the
next move is one line, and fix the two in-code comments plus the testing.md
paragraph that still pointed at the AGENTS.md matrix.

* docs: restore two rules dropped by the AGENTS.md split

Review caught two repo-specific rules that did not survive the move. Both are
prose without any backticked identifier, so the identifier-diff used to verify
the split could not see them.

- "Test through public interfaces; do not add unrelated production exports
  solely to enable tests" returns next to the behavioral-tests rule in
  docs/agents/testing.md, with the reason it exists.
- The guidance-ownership rule (decide whether new guidance/schema/metadata
  belongs to the command surface, CLI grammar, CLI help, MCP projection, or
  daemon runtime) returns to the always-loaded Docs & skills section, since it
  governs all command-surface work and not just the flag case.

Also point the ADR routing row at docs/adr/README.md, which is already the
"read when you touch…" index, rather than at the bare directory.
2026-07-25 12:13:09 +02:00

8.6 KiB
Raw Permalink Blame History

Testing Notes

Which gates a change needs

Default for code changes: pnpm check:affected --base origin/main --run. It derives the gate set from repository sources of truth, so prefer it over interpreting the table below by hand. GitHub CI stays authoritative.

The mapping it encodes, for when you need to run a gate directly or reason about coverage:

Change Gate
Any TypeScript pnpm typecheck or pnpm check:quick
Daemon handler / shared module pnpm check:unit
Tooling/config (package.json, tsconfig*.json, .oxlintrc.json, .oxfmtrc.json) pnpm check:tooling
Platform/device response — anything emitting platform/appleOs on the wire, or shaping a daemon response pnpm test:integration:provider and pnpm test:coverage
Cross-platform behavior pnpm test:integration
iOS runner / Swift pnpm build:xcuitest
CLI help/guidance (src/cli/parser/cli-help.ts, src/cli-schema/) pnpm exec vitest run src/cli/parser/__tests__ src/cli-schema/command-schema-guards.test.ts
SkillGym prompts/assertions pnpm test:skillgym:case <case-id> (broad: pnpm test:skillgym, filter with -- --tag fixture-smoke or -- --tag skill-guidance)
Anything in src/, test/, skills/ pnpm format

Two traps worth naming:

  • The platform/device-response row is the one agents miss. pnpm check:unit does not exercise the provider-integration project, and that project holds the apple-platform-output leak guard. Internal apple must never reach a command response — project through publicPlatformString.
  • Fallow CI failures reproduce with pnpm check:fallow --base origin/main. Do not estimate complexity or dead-code impact by hand.

Docs/skills-only and non-TS changes with no behavior impact need no tests. Test-only DI seam CI failures are enforced by the workflow — do not add optional typeof DI params to production code to satisfy a test.

Shared test utilities

Before writing a new test, inspect src/__tests__/test-utils/index.ts: rg -n "export .*make|export .*DEVICE|withMocked" src/__tests__/test-utils. Import through the barrel and prefer named shared fixtures over inlining new DeviceInfo, SessionState, snapshot, store, or mocked-binary objects. If a helper is missing, add it near the concept it serves and export it through the barrel.

Keep tests behavioral. Do not assert shapes or cases TypeScript already proves.

Test through public interfaces where practical, and do not add unrelated production exports solely to make a test easier — widening the public surface for a test is a product change, and the exports outlive the test that motivated them. If a seam is genuinely missing, add it as a real one rather than as a test affordance (the workflow separately forbids test-only typeof DI params).

Affected-check selector (pnpm check:affected)

pnpm check:affected --base <ref> derives which local checks a diff needs, so agents stop interpreting the testing matrix by hand. It is a fail-open advisory: existing GitHub CI stays authoritative and required, and this only narrows the local feedback loop.

pnpm check:affected --base origin/main --run     # default agent loop: plan + run
pnpm check:affected --base origin/main           # human-readable plan only
pnpm check:affected --base origin/main --json    # machine-readable plan only

The selection is derived from repository sources of truth rather than a hand-maintained path map:

  • Affected Vitest tests are delegated to vitest related --run, using Vitest's own project configuration and static module graph. The selector passes its complete changed-file set instead of reproducing Vitest globs or import ownership. Dynamic-import relationships remain outside Vitest's analysis; GitHub's authoritative full suites still cover that boundary.
  • Non-Vitest suites retain explicit ownership. Root test/integration/*.ts files use the Node integration lane, SkillGym owns its harness and skill guidance, and platform/build tools keep their native gates.
  • Always-on gates (lint, typecheck, layering, fallow, format) fire for their input categories and are never silently skipped. Platform source also selects the provider-integration and coverage gates required by the Testing Matrix.
  • Commands are resolved from real package.json scripts, so a renamed script fails loudly instead of dropping a gate.
  • A small explicit build-ownership layer covers the paths whose owning build cannot be derived: Swift runner, Android helpers, macOS helper, MCP metadata, and the public package surface (itself derived from package.json exports).
  • SkillGym ownership covers skill guidance (skills/) and the SkillGym harness (test/skillgym/) — those changes select the (local-only) SkillGym suite, and their Markdown is treated as skill/harness input, not inert docs.

Changed-file discovery folds working-tree state into the local plan: in the default local mode (--head HEAD) it unions the committed base..HEAD diff with staged, unstaged, and untracked files, and disables rename detection so both sides of a rename are classified (a moved file cannot look docs-only by its destination alone).

Anything the selector cannot classify — unknown, ambiguous, workflow/tooling, or a change to the selector's own sources — fails open to the full check set. That includes this file: the Testing Matrix above is the prose the ownership rules mirror, so docs/agents/testing.md is selector-owning (SELECTOR_OWNING_DOCS in scripts/check-affected/model.ts) and outranks the docs-only short-circuit its path would otherwise take. If the matrix moves again, move that entry with it. The plan documents the rule and changed path behind every selected check.

Model and catalog live under scripts/check-affected/; the derivation is guarded by pnpm check:affected:test (the Affected-check Selector CI job).

Live web smoke

The live web platform smoke runs the public built CLI against a local fixture page through the managed web backend:

AGENT_DEVICE_WEB_E2E=1 pnpm test:smoke:web

The test is skipped unless AGENT_DEVICE_WEB_E2E=1 is set. The test runs agent-device web setup and agent-device web doctor with an isolated state directory before opening the fixture URL, so it verifies the public managed-backend setup path instead of relying on a global agent-browser. CI runs the lane on Node 24 because the managed backend requires Node >= 24. Failure artifacts, daemon state, and browser config are written under test/artifacts/web/.

Speed rules (experiment-backed, 2026-07-04)

Measured on the full unit suite (340 files, 3,210 tests, 48s wall at ~7x parallelism):

  • Wall clock equals the slowest file. The 44.6s android monolith bounded the whole 48s run (Amdahl at file granularity: vitest parallelizes per file). Splitting monolith test files is a wall-clock optimization, not just a navigation one — see the AGENTS.md test-topology mirror rule.
  • Unit tests must not wait real time. The suite's worst tests slept through production budgets: 10.8s to prove "times out" by waiting out the full constant, 8s emulator-boot polls at 1Hz, real retry backoffs. Conversion patterns, in preference order (tracking issue #1098):
    1. Budget-derived cadence (production-legit): poll intervals scale with the caller's timeout — this took devices.test.ts from 25.6s to 2.8s (9x) while making short-budget production calls more responsive.
    2. Budget-wiring assertion: don't re-prove the exec layer's timeout per call site; mock the tool layer and assert the right timeoutMs constant is passed. Exec-layer timeout semantics are proven once, in exec's own tests.
    3. Fake clocks where the code accepts an injected clock. Never add a test-only DI seam for this — the CI gate forbids it; patterns 12 are production improvements and test restructurings respectively.
  • The slow-test ratchet (scripts/vitest-slow-test-reporter.ts) enforces this: unit budget 2.5s, integration 15s, failure at 2x budget (the band between reports without failing — host load legitimately stretches borderline tests, and a flaky gate trains people to ignore it). The pin list only shrinks, or grows in the same PR with a justification.
  • Isolation stays ON; pool stays forks — both measured. --no-isolate: 205s wall vs 48s (module state — timers, memos, singletons — thrashes across files sharing a worker). --pool=threads: no change (50.4s). The ~100s aggregate import overhead is the price of isolation and is paid in parallel; reduce it per file by importing the module under test, not platform barrels.