docs(AGENTS): rewrite the root standing orders to the 1,500-word budget
Applies the documentation standard to its biggest offender. Every rule survives as one to three lines plus a link to its durable home; the stories, duplicate statements, and re-narrations go: - Situational clusters evict to new homes: docs/testing.md (tiers, with-key policy, real-over-mock, world-verification, real-entry-path guards), docs/defensive-patterns.md (the bug-class rules), and docs/cookbook/responding-to-pr-review-on-a-stack.md (the stacked-PR review procedure). - Doc-authoring rules consolidate into docs/AGENTS.md § Writing rules (current-state-never-history, md-wrap, ts-block compilation, @mode, catalog same-change, pair same-change). - packages/README.md drops to the group table + the extension-vs-bundle dependency rule; the hand ASCII graph yields to the generated module-graph.md; group READMEs are the canonical per-package map. - packages/AGENTS.md keeps only its packages-specific rules (export shape, ctx.get, real-Loader coverage); examples/AGENTS.md repoints its with-key-policy link; rfc/README.md loses a narrated-history aside; dsh-code-review / dsh-find-simplifications / verify-md-wrap references follow the moved content. - Budget manifest ratchets: AGENTS.md 8200 -> 1500 (now 1,495 words), packages/README.md 1900 -> 600, packages/AGENTS.md 600 -> 450; the two new eviction docs join the budget set (testing 800, defensive 550); docs/AGENTS.md raises 1000 -> 1250 for the absorbed writing rules (the one justified increase). The doc-tiers RFC's deferred list prunes the two items this change ships.
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# AGENTS.md — The documentation standard
|
||||
|
||||
This file is the contract for every Markdown surface in the repo: what each documentation tier is for, what belongs elsewhere, and the word budgets the `verify-doc-budgets` gate enforces. The repo-wide writing rules live in the root [AGENTS.md](../AGENTS.md) § "Type Safety and Documentation" and apply to everything here. The audit/apply workflow is the [dsh-doc-standards](../.agents/skills/dsh-doc-standards/SKILL.md) skill; the decision record is [the doc-tiers-and-budgets RFC](rfc/implemented/process/2026-07-04-doc-tiers-and-budgets.md).
|
||||
This file is the contract for every Markdown surface in the repo: each tier's job, the writing rules, and the word budgets the `verify-doc-budgets` gate enforces. The audit/apply workflow is the [dsh-doc-standards](../.agents/skills/dsh-doc-standards/SKILL.md) skill; the decision record is [the doc-tiers-and-budgets RFC](rfc/implemented/process/2026-07-04-doc-tiers-and-budgets.md).
|
||||
|
||||
## The tier taxonomy: one home per fact
|
||||
|
||||
@@ -22,12 +22,22 @@ Every fact has exactly one home — the tier whose job it is — and every other
|
||||
|
||||
Placement test: a story about a bug → postmortem. Why we chose X → RFC. How to do task Y → cookbook. What type Z looks like → core-data-structures. What package P promises → its README. A rule every agent must always obey → root AGENTS.md, one line, linking the home that holds the why.
|
||||
|
||||
## Writing rules
|
||||
|
||||
- **Document the current state — never the process or history that produced it.** Prose describes what the code IS and why, as if it had always been so: no "previously/now/no longer/used to/renamed/moved here", and never name a change unit the reader cannot see — a PR, commit, or stack position — in comments, JSDoc, or test names; name the mechanism instead. A genuinely clarifying contrast is framed against the live alternative as a standing fact, not against the past. The change story belongs in the commit message, the PR description, or an RFC.
|
||||
- **A decision worth re-litigating gets an RFC in the same PR.** The test: would a maintainer six months out ask "why was it done this way?" and find no answer in the code? If yes, write one ([when to write one](rfc/README.md)); mechanical or self-evident changes need none.
|
||||
- **One physical line per paragraph** (`verify-md-wrap`): the editor soft-wraps; hard breaks make a one-word edit re-diff the whole paragraph. Prose only — code blocks, tables, and list structure stay; code comments stay under the linter's column limit.
|
||||
- **Fenced `ts` blocks must compile** (`doc-typecheck`); a pasted type definition is fenced ` ```ts type-equiv ` and registered in the manifest so it cannot drift ([mechanics](development.md#documenting-types-verbatim-ts-type-equiv)).
|
||||
- **Every new event's JSDoc carries an `@mode` tag** (emit | waterfall | parallel | serial); the catalog generator hard-errors without it. Write the JSDoc to stand alone — it becomes the catalog entry ([catalog RFC](rfc/implemented/process/2026-06-20-generated-cordis-catalog.md)).
|
||||
- **The [core-data-structures catalog](core-data-structures/core.md) updates in the same change** that reshapes a documented type. `verify-type-equiv` catches drifted pastes, not never-documented new types ([what counts as core](core-data-structures/core.md#what-counts-as-core)).
|
||||
- **Bilingual pairs update together**: editing either side obligates the counterpart and a re-record in the same change ([i18n contract](i18n/README.md)).
|
||||
|
||||
## Budgets and the ceiling gate
|
||||
|
||||
Standing docs accrete: every PR has a lesson it wants to append, and without displacement pressure nothing ever leaves. The gate is that pressure. [scripts/doc-budgets.manifest.json](../scripts/doc-budgets.manifest.json) lists the accretion-prone standing docs with a word ceiling each; `pnpm run verify-doc-budgets` (part of `doc-sync`, so CI and pre-push run it) fails when a doc exceeds its ceiling, and fails when a budgeted file is missing so a rename cannot orphan its budget.
|
||||
|
||||
- Ceilings are an enforcement frontier: a ceiling starts at the doc's current size (freezing further growth) and ratchets down as the doc is brought to its target. Target budgets: root `AGENTS.md` ≤ 1,500 words; `architecture.md` ≤ 1,800; each subtree `AGENTS.md` ≤ 600, except this file (which carries the standard) ≤ 1,000; `packages/README.md` ≤ 600.
|
||||
- When the gate goes red, the fix is to relocate or condense per the taxonomy above. Raising a ceiling is the last resort: the PR description must justify it, and the manifest diff is the reviewable act.
|
||||
- Ceilings are an enforcement frontier: a ceiling starts at the doc's current size (freezing further growth) and ratchets down as the doc is brought to its target. Target budgets: root `AGENTS.md` ≤ 1,500 words; `architecture.md` ≤ 1,800; each subtree `AGENTS.md` ≤ 600, except this file (which carries the standard) ≤ 1,250; `packages/README.md` ≤ 600.
|
||||
- When the gate goes red, the fix is to relocate or condense per the taxonomy above. Raising a ceiling is the last resort: the PR must justify it; the manifest diff is the reviewable act.
|
||||
- Unbudgeted tiers (package READMEs, RFCs, reference matrices) have no ceiling — length is legitimate there when every row is a fact. Review and the slop checklist govern them instead.
|
||||
|
||||
## The slop checklist
|
||||
|
||||
24
docs/cookbook/responding-to-pr-review-on-a-stack.md
Normal file
24
docs/cookbook/responding-to-pr-review-on-a-stack.md
Normal file
@@ -0,0 +1,24 @@
|
||||
# Responding to review across a stacked PR chain
|
||||
|
||||
A wave of review comments lands across several PRs in a dependent stack (`A ← B ← C …`). This is the discipline for resolving it without corrupting the stack. The two invariants it rests on are standing orders in the root [AGENTS.md](../../AGENTS.md) § Conventions: merge commits only, and never rewrite a pushed branch.
|
||||
|
||||
## Ground rules
|
||||
|
||||
1. **One worktree per PR branch.** Each PR's fixes happen in that PR's own worktree; parallel fixes never share a checkout.
|
||||
2. **Bring a child up to date by merging the parent down** (`git merge <parent-branch>` into the child, a new merge commit). Never rebase/amend/force-push a pushed branch: rewriting diverges it from what the parent PR and GitHub recorded, breaks the stacked-merge graph, and erases the review-fix history.
|
||||
3. **A fix lands on the PR that INTRODUCED the issue, then flows down.** When a comment on PR `B` points at code `B` introduced, fix it on `B` and merge `B` into `C` — even if `C` also carries the file. Originating the fix downstream leaves `B` shipping the unfixed code and hides the fix from `B`'s reviewer.
|
||||
4. **Each review fix is a separate commit, never an amend.** The "fix review findings" commit documents what the review caught. Amending is fine only for your own not-yet-pushed, not-yet-reviewed work.
|
||||
|
||||
## Working the wave
|
||||
|
||||
1. Triage every comment on the merits before acting: verify the claim against the code — a reviewer flagging the right symptom can still mis-diagnose the cause.
|
||||
2. Map each accepted finding to its originating PR, fix it there, then merge down the chain in order.
|
||||
3. Delegated fixes are trust-but-verify: a sub-agent's report describes intent, not necessarily what landed. Re-run the gates yourself on the actual tree, and for a regression guard, prove it FAILS on the unfixed code (introduce the regression, watch red, revert) — a guard that passes both ways guards nothing. A sub-agent that reframes a problem as already-handled is a signal to dig in personally.
|
||||
4. Reply in the review thread (`gh api repos/{owner}/{repo}/pulls/{pr}/comments/{id}/replies`), not as a top-level comment, stating the fix and the commit that carries it.
|
||||
5. Before merging the stack, check dependents: deleting a PR's base branch auto-closes the dependent PR — `gh pr list --json number,baseRefName` first, and merge without `--delete-branch` where a child still bases on the branch.
|
||||
|
||||
## Verify
|
||||
|
||||
- Every fixed PR shows a new commit (no force-push icon in the PR timeline).
|
||||
- Each child PR's diff against its parent still shows only its own changes.
|
||||
- The gates pass on every PR in the stack, not just the top.
|
||||
27
docs/defensive-patterns.md
Normal file
27
docs/defensive-patterns.md
Normal file
@@ -0,0 +1,27 @@
|
||||
# Defensive patterns
|
||||
|
||||
Hard-won bug-class rules: each pattern below is a class of defect that actually shipped or nearly shipped here, stated as the rule that prevents its recurrence. Read this before writing lifecycle, concurrency, subprocess, or teardown code. Test-tier counterparts (real entry path, world-verification, resource ownership) are in [testing.md](testing.md).
|
||||
|
||||
## Report orthogonal outcomes independently
|
||||
|
||||
A result can be several things at once — a process can time out AND exit 0 because it trapped the signal. Surface each independent fact (`timedOut`, `signal`, `exitCode`) on its own; never nest one flag's report inside another's branch, or a caller reads a cut-short run as a clean success.
|
||||
|
||||
## Honor cross-seam contracts on BOTH sides
|
||||
|
||||
When an interface documents two valid ways to signal something — an adapter may report failure by THROWING from `stream()` or by ending the stream with a `finish {kind:'error'|'aborted'}` chunk — the consumer handles both, not just the one the first implementation used. A library-backed adapter that can't throw mid-stream relies on the in-band path; a loop that only catches throws turns a provider 401 into a normal completed turn. Document the contract where the type is defined; exercise every branch through the real consumer.
|
||||
|
||||
## Async state is not synchronous state
|
||||
|
||||
`agent.send()` does not flip status before returning; a background task's completion races turn boundaries; `reader.close()` fires for both EOF and disposal. Never gate control flow on a status you only just requested — drive lifecycle off the events/promises that actually fire (`agent/status`, `task.done`), and observe the transition (saw `running` THEN `idle`) rather than counting actions you assume map 1:1 to turns (the loop batches queued messages). The guard cuts both ways: if the awaited transition can never occur (EOF with no work submitted → never `running`), the wait hangs — handle the "nothing to wait for" branch explicitly.
|
||||
|
||||
## Dispose must reach quiescence, not just request it
|
||||
|
||||
A teardown that issues kills/aborts but returns before the work stops leaves orphans. Make cleanup async and await the children's exit (kill → await `done`), and close listener/notification registries BEFORE killing so late completions stay silent. Tests prove disposal waited (pid gone right after `await fiber.dispose()`), not merely that the process eventually dies.
|
||||
|
||||
## Contain callback exceptions at the boundary
|
||||
|
||||
A user-supplied listener that throws must not reject the promise it runs inside or starve the listeners after it. Wrap the dispatch loop in try/catch and log; one bad subscriber never breaks core lifecycle.
|
||||
|
||||
## Never hand untrusted output the ambient environment or predictable paths
|
||||
|
||||
Spawned commands get a scrubbed env (drop `*KEY*`/`*SECRET*`/`*TOKEN*`) so harness credentials cannot leak into output, `env`, or spill files. Temp/spill files use a private (0700) dir, random names, and exclusive owner-only opens (`'wx'`, `0o600`) — predictable world-readable paths invite symlink races and disclosure.
|
||||
@@ -1,6 +1,6 @@
|
||||
# RFCs
|
||||
|
||||
One kind of design doc lives here. An **RFC** records a decision or proposal that shapes this codebase — the *why* and *what we gave up*, the parts code and docs can't carry. (Earlier this split into separate "ADR" and "RFC" trees; they were unified, since most ADRs were simply implemented RFCs.)
|
||||
One kind of design doc lives here. An **RFC** records a decision or proposal that shapes this codebase — the *why* and *what we gave up*, the parts code and docs can't carry.
|
||||
|
||||
## Layout and naming
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@ Status: implemented (accepted 2026-06-30)
|
||||
|
||||
The hooks subsystem runs external hook commands the way Claude Code and Codex do: a hook is a shell command that receives its event payload as **JSON on stdin** and reads context from a handful of **environment variables** (`CLAUDE_PROJECT_DIR`, `CLAUDE_PLUGIN_ROOT`, `PLUGIN_ROOT`, …). The harness already has a perfectly good command runner behind the `ctx.bash` capability seam ([dsh-bash](../../../../packages/bash/bash) → [dsh-bash-local](../../../../packages/bash/bash-local)), with process-group kills, output truncation/spill, and a credential scrub. Reusing it for hook execution means a hook bridge does not re-implement subprocess plumbing — but the seam had no way to write stdin or set extra env. This RFC adds those two inputs.
|
||||
|
||||
**These fields are NOT a new security boundary.** It is tempting to frame arbitrary-stdin / arbitrary-env as "dangerous, so gate who may use them" — but that framing is wrong, because a model driving the `bash` tool **already** has equivalent power through ordinary shell syntax: `FOO=bar cmd` sets an env var, a heredoc or `printf … | cmd` feeds arbitrary stdin. Adding `env`/`stdin` as seam fields grants the model no capability it lacks. In particular they cannot exfiltrate the harness's ambient credentials: the real control for that is the **credential scrub** in [dsh-bash-local](../../../../packages/bash/bash-local)'s `childEnv()`, which strips `*KEY*`/`*SECRET*`/`*TOKEN*` from `process.env` before the child sees it (see [AGENTS.md](../../../../AGENTS.md) § Defensive patterns, "Never hand untrusted/model output the ambient environment or predictable paths"). The scrub works regardless of these fields — a model cannot read a value that is not in the environment, and tool-call arguments are static JSON, never shell-evaluated, so a model cannot write `env: {LEAK: $DEEPSEEK_API_KEY}` and have it expand. So the security question is already answered by the scrub; this RFC is only about giving trusted in-process callers a clean way to pass a JSON payload + `CLAUDE_*` vars without routing them through model-visible shell text.
|
||||
**These fields are NOT a new security boundary.** It is tempting to frame arbitrary-stdin / arbitrary-env as "dangerous, so gate who may use them" — but that framing is wrong, because a model driving the `bash` tool **already** has equivalent power through ordinary shell syntax: `FOO=bar cmd` sets an env var, a heredoc or `printf … | cmd` feeds arbitrary stdin. Adding `env`/`stdin` as seam fields grants the model no capability it lacks. In particular they cannot exfiltrate the harness's ambient credentials: the real control for that is the **credential scrub** in [dsh-bash-local](../../../../packages/bash/bash-local)'s `childEnv()`, which strips `*KEY*`/`*SECRET*`/`*TOKEN*` from `process.env` before the child sees it (see [docs/defensive-patterns.md](../../../defensive-patterns.md) § "Never hand untrusted output the ambient environment or predictable paths"). The scrub works regardless of these fields — a model cannot read a value that is not in the environment, and tool-call arguments are static JSON, never shell-evaluated, so a model cannot write `env: {LEAK: $DEEPSEEK_API_KEY}` and have it expand. So the security question is already answered by the scrub; this RFC is only about giving trusted in-process callers a clean way to pass a JSON payload + `CLAUDE_*` vars without routing them through model-visible shell text.
|
||||
|
||||
## Decision
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ The repo's standing docs accrete. Root `AGENTS.md` reached 8,130 words through 5
|
||||
## Decision
|
||||
|
||||
- **A tier taxonomy with one home per fact.** [docs/AGENTS.md](../../../AGENTS.md) is the documentation standard: it assigns every Markdown tier a single job (standing orders, system map, type catalog, decision records, incident stories, how-tos, per-package contracts, generated catalogs, workflows), forbids restating a fact outside its home tier (link instead), and carries the slop checklist used when writing or reviewing any doc.
|
||||
- **A narrow, hard budget gate.** [scripts/verify-doc-budgets.ts](../../../../scripts/verify-doc-budgets.ts) joins `doc-sync`: every doc listed in [scripts/doc-budgets.manifest.json](../../../../scripts/doc-budgets.manifest.json) must stay under its word ceiling (`wc -w` semantics, whole file), and a budgeted file that is missing fails the gate so a rename cannot silently orphan its budget. Scope is deliberately only the accretion-prone standing docs — the root and subtree `AGENTS.md` files, `architecture.md`, `packages/README.md`. Reference docs, RFCs, and package READMEs are unbudgeted: length is legitimate there when every row is a fact, and review plus the slop checklist govern them.
|
||||
- **A narrow, hard budget gate.** [scripts/verify-doc-budgets.ts](../../../../scripts/verify-doc-budgets.ts) joins `doc-sync`: every doc listed in [scripts/doc-budgets.manifest.json](../../../../scripts/doc-budgets.manifest.json) must stay under its word ceiling (`wc -w` semantics, whole file), and a budgeted file that is missing fails the gate so a rename cannot silently orphan its budget. Scope is deliberately only the accretion-prone standing docs — the root and subtree `AGENTS.md` files, `architecture.md`, `packages/README.md`, and the standing policy docs they evict content into (`docs/testing.md`, `docs/defensive-patterns.md`). Reference docs, RFCs, and package READMEs are unbudgeted: length is legitimate there when every row is a fact, and review plus the slop checklist govern them.
|
||||
- **Ceilings are an enforcement frontier that ratchets.** A ceiling starts at the doc's current size, freezing growth from day one, and ratchets down as the doc is brought to its target budget (root `AGENTS.md` ≤ 1,500 words; `architecture.md` ≤ 1,800; subtree `AGENTS.md` ≤ 600; `packages/README.md` ≤ 600) — the same rollout mechanism as the [translation-pairing `required` list](2026-07-02-bilingual-docs-and-pairing-gate.md). When the gate goes red the fix is to relocate or condense per the taxonomy; raising a ceiling is permitted only with explicit justification in the PR description, the manifest diff being the reviewable act.
|
||||
- **A thin workflow skill, contracts in docs.** [.agents/skills/dsh-doc-standards](../../../../.agents/skills/dsh-doc-standards/SKILL.md) carries the placement/audit/red-gate workflow and defers to the standard as its source of truth, the same split as [dsh-translate-docs](../../../../.agents/skills/dsh-translate-docs/SKILL.md) over the i18n contract.
|
||||
|
||||
@@ -27,9 +27,7 @@ The repo's standing docs accrete. Root `AGENTS.md` reached 8,130 words through 5
|
||||
|
||||
The first audit cycle under the standard, in rough priority order (evidence gathered in the survey that motivated this RFC):
|
||||
|
||||
- Root `AGENTS.md` rewrite to the ≤ 1,500-word target: rules stay as one-liners plus links; situational clusters move to `docs/testing.md`, `docs/defensive-patterns.md`, and a cookbook guide for responding to review across a stacked PR chain; doc-authoring rules consolidate into `docs/AGENTS.md`.
|
||||
- `architecture.md` rewrite to the ≤ 1,800-word target: seam narration compressed to pointers, the MVP feature-to-mechanism checklist moved de-statused into [the extension cookbook](../../../cookbook/extension-cookbook.md), the stale layering-diagram row fixed.
|
||||
- `packages/README.md` reduced to the group table plus the dependency rule; the hand-maintained ASCII dependency graph yields to the generated [module-graph.md](../../../module-graph.md); group READMEs become the canonical per-package map.
|
||||
- Package README trims where generated catalogs or JSDoc are restated or history is narrated: `packages/ui/acp`, `packages/core/tools`, `packages/bash/tool-bash`, `packages/core/session`, `packages/compact/compact-basic`, `packages/session-persistence/session-persistence`.
|
||||
- [The web capability seam RFC](../architecture/2026-06-24-web-capability-seam.md) converted from spec-speak to shipped reality (drop the migration plan and test enumeration, "should" → "is").
|
||||
- `docs/core-data-structures/core.md`: drop the JSDoc walls from the `Agent`/`GenerateOptions` type-equiv pastes per that page's own stated rule.
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
Status: implemented (proposed 2026-06-20; accepted in amended form — `whenIdle()` retained)
|
||||
|
||||
> **Implementation note (scope narrowed from the original proposal).** This RFC proposed removing BOTH `abort()` and `whenIdle()` from the public `Agent` handle. Only `abort()` was removed. Validating the premise against the code ([AGENTS.md "RFCs are proposals, not golden truth"](../../../../AGENTS.md)) found `whenIdle()` to be a **load-bearing quiescence primitive**, not dead surface: it is the settle signal in several ACP tests (`packages/ui/acp/tests/{edges,turns,dispose}.spec.ts`) and is backed by a deliberate loop contract (settle waiters without a status transition; handle the replacement-turn race). The RFC's suggested migration — have consumers observe the `running`→`idle` transition by hand — is exactly the brittle hand-rolled path [AGENTS.md § Defensive patterns](../../../../AGENTS.md) warns against ("Async state is not synchronous state"). Deleting a clean primitive to push every consumer onto that is a net loss, so `whenIdle()` stays. `abort()` was genuinely dead public surface (no production caller; the loop aborts its own `AbortController` directly), so it was removed as proposed. The text below is amended to describe what shipped.
|
||||
> **Implementation note (scope narrowed from the original proposal).** This RFC proposed removing BOTH `abort()` and `whenIdle()` from the public `Agent` handle. Only `abort()` was removed. Validating the premise against the code ([AGENTS.md "RFCs are proposals, not golden truth"](../../../../AGENTS.md)) found `whenIdle()` to be a **load-bearing quiescence primitive**, not dead surface: it is the settle signal in several ACP tests (`packages/ui/acp/tests/{edges,turns,dispose}.spec.ts`) and is backed by a deliberate loop contract (settle waiters without a status transition; handle the replacement-turn race). The RFC's suggested migration — have consumers observe the `running`→`idle` transition by hand — is exactly the brittle hand-rolled path [the defensive patterns](../../../defensive-patterns.md) warns against ("Async state is not synchronous state"). Deleting a clean primitive to push every consumer onto that is a net loss, so `whenIdle()` stays. `abort()` was genuinely dead public surface (no production caller; the loop aborts its own `AbortController` directly), so it was removed as proposed. The text below is amended to describe what shipped.
|
||||
|
||||
## Problem
|
||||
|
||||
|
||||
33
docs/testing.md
Normal file
33
docs/testing.md
Normal file
@@ -0,0 +1,33 @@
|
||||
# Testing policy
|
||||
|
||||
How this repo tests, tier by tier, and the rules that keep a green suite meaning something. Commands live in the root [AGENTS.md](../AGENTS.md) § Commands; the RFCs linked per tier carry the design rationale.
|
||||
|
||||
## Tiers
|
||||
|
||||
- **Unit** (`pnpm run test`): vitest, colocated at `packages/<group>/<pkg>/tests/*.spec.ts`. Every registry gets an HMR-safety test (dispose the contributing fiber, assert cleanup). Excessive tests are welcome — err toward covering edge cases, error paths, event ordering, and concurrency races; review findings get regression tests (see `packages/core/agent-loop/tests/review-fixes.spec.ts`).
|
||||
- **Coverage gate** (`pnpm run test:coverage`): the gating run, per-file 100% on `packages/*/*/src`. An uncovered line is often dead code the gate is correctly flagging for deletion, not a missing test to bolt on. Line coverage is necessary, never sufficient — it proves lines ran, not that the feature works as shipped.
|
||||
- **Real-API e2e** (`pnpm run test:e2e`): with-key tests against the live DeepSeek API; self-skip without `DEEPSEEK_API_KEY` so keyless CI stays green ([real-API e2e RFC](rfc/implemented/testing/2026-06-19-real-api-e2e-ci.md)).
|
||||
- **Snapshot** (`pnpm run test:snapshot`): boots the real example subprocess, replays a recorded session keyless, diffs normalized stdout + the re-persisted log against committed goldens ([snapshot RFC](rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md)). Re-record with `pnpm run test:snapshot:record`; reviewing the golden diff is part of the review.
|
||||
|
||||
## The with-key policy: inference is cheap here
|
||||
|
||||
We are DeepSeek — do not ration real-API tests. A no-key test proves the plumbing; only a with-key run proves the agent works against a real model. Write many: real prompts that write files, multi-turn conversations, tool use, cancellation mid-stream. Cheapest and highest-value are **smoke tests** that boot the real example, send one real prompt, and check the world — they catch the "green unit tests, broken product" class that mocks structurally cannot ([postmortem 0001](postmortem/0001-acp-default-export-drops-inject.md)). The self-skip exists only so secretless CI and keyless contributors aren't blocked; it is not a cost signal. Every example ships both a keyless and a with-key smoke ([examples/AGENTS.md](../examples/AGENTS.md)).
|
||||
|
||||
## Prefer the real implementation over a mock
|
||||
|
||||
Mock only the genuinely expensive or non-deterministic boundary (the LLM adapter, the network, the clock); keep everything downstream real. A hand-rolled stand-in proves the bridge moves bytes, not that the shipping tool behaves as asserted — the two drift while the test stays green. Example: bridge tool-call tests run the scripted mock MODEL but the real tool + real executor (`makeBridgeHarness({ withBash: true })` plugs `dsh-bash-local` + `dsh-tool-bash` and runs an actual `echo`).
|
||||
|
||||
## Verify the world, not the self-report
|
||||
|
||||
An e2e assertion re-runs the command or re-reads the file externally; a keyword probe on the agent's own output lets a cheating agent pass. Assert untouched files are byte-identical. e2e tests own their resources: create the harness in the test, dispose in `afterEach` (even on failure/retry/timeout); shared fixtures live in a plain `tests/harness.ts`, never another `*.e2e.ts` (importing a spec re-registers its `describe` and duplicates real API calls).
|
||||
|
||||
## Test the real entry path
|
||||
|
||||
- A plugin shipped via `cordis.yml` needs at least one test through the REAL Loader path: hand-built `ctx.plugin({...})` mounts bypass `unwrapExports` and cannot catch a broken export shape ([postmortem 0001](postmortem/0001-acp-default-export-drops-inject.md); export-shape rules in [packages/AGENTS.md](../packages/AGENTS.md)).
|
||||
- A guard only guards if the regression actually fails it. For a plugin without `inject` (bundle/composition plugins), a Loader smoke stays green under a broken export shape — add an explicit `expect('default' in mod).toBe(false)` plus an `unwrapExports` round-trip assertion, and prove it: introduce the regression, watch red, revert.
|
||||
- "Real entry path" means the published artifact: the package `bin` points at built `lib/bin.js` under plain `node`, which tsx masks (settle races, module resolution, a swallowed load failure exiting 0). Keep the built-bin smokes green (`packages/ui/*/tests/built-bin.e2e.ts`), and assert a genuinely-missing config exits non-zero.
|
||||
- An e2e that spawns an example from a temp cwd sets `TSX_TSCONFIG_PATH` to the repo-root tsconfig, or it silently falls back to stale built `lib/` ([examples/AGENTS.md](../examples/AGENTS.md)).
|
||||
|
||||
## When a snapshot test is required
|
||||
|
||||
Any change affecting the editor-facing transcript or end-to-end agent UX — the ACP bridge, the loop's observable output, tool presentation — adds or updates a scenario under `examples/acp-agent/tests/snapshots/` (or states in the PR why none applies). New capability seams, lifecycle shapes, or transcript surfaces name their coverage at every tier at plan time and verify the harness can express it — a harness gap is scheduled work, not a mid-build surprise.
|
||||
Reference in New Issue
Block a user