Skip to content

Commit a7df851

Browse files
committed
docs(agents): ADR 0019 unit checklist, owning-seam mock rule, worktree and rebase guidance
Retro follow-up (item 2). Adds docs/agents/adr-0019-unit.md — the order of operations for one command unit with the declaration site for each step, the evidence a unit review must carry, and what 'done' is not — so the pattern rediscovered during the snapshot unit (#1779) is written down once. testing.md: mock the seam the code under test consumes (fake inspectFacts / bindDevice), not the generic dispatchCommand mock; a migrating command moves its tests off the dispatch mock in the same PR. AGENTS.md: fresh-worktree preflight (pnpm install + build in the worktree; layering scan reads tracked files only) and concurrent-agent hygiene (one full gate per host, verify subagent edits with git -C, one PR per worktree). pull-requests.md: two readiness claims (published-and-reported vs merge-ready) and the rebase rule — main has no up-to-date protection; rebase on conflict or when `check:affected --base <merge-base> --head origin/main` names your surface.
1 parent 4bba404 commit a7df851

4 files changed

Lines changed: 106 additions & 0 deletions

File tree

AGENTS.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ situational lives one hop away — load it when the task calls for it.
1414
| Which gates to run, test speed rules, shared fixtures | `docs/agents/testing.md` |
1515
| Adding or changing a CLI flag | `docs/agents/cli-flags.md` |
1616
| Opening a PR, or reviewing one | `docs/agents/pull-requests.md` |
17+
| Migrating a command onto the request-bound platform runtime (an ADR 0019 unit) | `docs/agents/adr-0019-unit.md` |
1718
| Running commands against a real device | `docs/agents/device-verification.md` |
1819
| Issues, PRDs, triage labels | `docs/agents/issue-tracker.md`, `docs/agents/triage-labels.md` |
1920
| Web automation backend setup/diagnostics | `docs/agents/web-backend.md` |
@@ -160,6 +161,16 @@ one question so `rg` → read-whole-file stays one cheap bounded read.
160161
under `~/.agent-device/dev/<basename-slug>-<hash>`. Inspect with `pnpm daemon:state-dir`, override
161162
with `--state-dir`/`AGENT_DEVICE_STATE_DIR`, prune with `pnpm clean:daemon --prune-dev`. Daemons
162163
are isolated per worktree; **devices are not** — target different devices for concurrent worktrees.
164+
- A fresh worktree is not ready until `pnpm install --frozen-lockfile && pnpm build` ran **in it**.
165+
Until then `@agent-device/*` and optional peers (`ai`) resolve against whichever checkout last
166+
installed — typecheck fails on `src/ai-sdk`, and the smoke lanes fail with "Missing dist build".
167+
Neither is a regression. The layering scan (`check:layering`) reads tracked files only, so an
168+
untracked new module is invisible to it until `git add`.
169+
- Concurrent agents on one host: one full gate (`pnpm check`, `check:unit`) at a time — several
170+
worktrees running unit suites together *are* the contention that produces timeout-shaped
171+
failures. Verify a subagent's edits with `git -C <worktree> status/diff`, never from its report;
172+
keep one PR per worktree and address PRs by URL/branch, not a number recalled from context;
173+
give each agent its own scratch directory.
163174
- Node ≥22. Prefer built-ins (`fetch`, Web Streams, `AbortSignal.timeout`) over compatibility
164175
wrappers unless the surrounding code needs a lower-level transport.
165176
- Emit with `tsdown` (Rolldown), typecheck with TypeScript 7 via `tsc`. Declaration generation uses

docs/agents/adr-0019-unit.md

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# Migrating one command onto the request-bound platform runtime (ADR 0019 unit)
2+
3+
The checklist for one command unit. ADR 0019 §6–10 holds the rules and the why; issue #1739 holds
4+
the wave plan and per-unit budgets; this file is the order of operations, with the seam, fixture,
5+
and gate you touch at each step. Everything here was rediscovered during the `snapshot` unit
6+
(#1779, 134 files) — do not rediscover it again.
7+
8+
## Before writing code
9+
10+
1. **Post the unit on #1739 first**: exact denominator (every inventory source or supported
11+
device-runtime cell), parity source, facet owner, expected deletions, evidence tier
12+
(request-scoped vs durable-resource), and the size budget. A unit without a posted budget has
13+
no acceptance criterion; the late budget negotiation on #1779 is the incident.
14+
2. **One integration owner.** Parallel agents audit (platform parity, tests, size, architecture)
15+
and implement only clearly disjoint lanes — a platform package's operations, a fixture builder,
16+
the test migration. The shared spine — plan → facts → bind in the handler, the descriptor's
17+
`platformExecution`, the cutover row — moves serially under one owner. Two agents editing the
18+
spine produced the rebase churn on #1779.
19+
3. **Ask the owning-interface question before adding any gate**: "can this invariant be made
20+
impossible at the seam (a type only one function can produce, a required parameter, one
21+
construction path)?" If yes, do that; a cutover-row *extension* that recognizes a call shape is
22+
the fallback, not the default, and it is what AGENTS.md's "reconstructing a compiler" principle
23+
forbids once it needs a second omission patch.
24+
25+
## The unit, in order
26+
27+
Each step names its declaration site; read that, not prose.
28+
29+
| Step | Where |
30+
| --- | --- |
31+
| Declare the use(s) with the one neutral `defineUse`; required-only by default, a preferred operation needs a recorded measurement in the unit review (§9) | `packages/contracts/src/platform-runtime-operations.ts` |
32+
| Plan resolution is pure and lives in contracts (`resolve<Command>RuntimePlan`), never in the handler | `packages/contracts/src/*-runtime-plan.ts` |
33+
| Handler: resolve plan → `inspectRequiredRuntimeUse(device, plan.use, inspectFacts)` → bind **once** with that use → operate. No admission-only binds, no `requireCommandSupported`, no capability bucket | `src/daemon/handlers/session-runtime-admission.ts`, the `snapshot` route in `src/daemon/snapshot-runtime.ts` as the model |
34+
| Descriptor flips to `platformExecution: { kind: 'device-runtime', use(s) }` in the same PR; the discriminator is exhaustive, so a forgotten descriptor fails typecheck | `src/core/command-descriptor/registry.ts` |
35+
| Each platform package reports exact-owner facts and implements the operations; provider ownership fails closed (missing behavior never falls through to a local owner) | `packages/platform-*/src/**`, `packages/provider-*/src/**` |
36+
| Add one row to the parametrized cutover table: `legacyRetirement` (what must be gone), `runtimeTypeNames`, `operations`, `singularExecution` with lexical `operationOwners`. The mechanism already carries the planted-red proof; a row that leaves a claim unstated is rejected by `cutoverRowDefects` | `scripts/layering/runtime-command-cutover-table.ts` (rule ids allocate upward; snapshot is R32) |
37+
| Delete: the legacy adapter, the descriptor's capability bucket + `requireCommandSupported` wiring, the retired route names — the row's `legacyRetirement` is the machine-checked list | wherever `legacyRetirement` points |
38+
| Tests: bind a fake runtime at the seam the handler consumes (`inspectFacts` / `bindDevice`), never a `dispatchCommand` mock. Move the command's existing tests off the dispatch mock in this PR | `src/daemon/__tests__/snapshot-runtime-fixture.ts` (fixture shape), `src/daemon/handlers/__tests__/session-command-harness.ts` (`mockInspectDeviceRuntimeFacts`, `mockBindDeviceRuntime`), `src/__tests__/test-utils/runtime-operation-facts.ts` (facts builders) |
39+
| Cross-cutting facets (freshness, system-chrome guard, presentation) land inside the first consuming unit and get one contract owner; later units consume, never fork | ADR 0019 §10 |
40+
41+
## Evidence the unit review must contain
42+
43+
- Request-scoped tier: the typed use declaration, fact coverage for every denominator cell, the
44+
enumerated legacy-parity cell table, and the cutover-gate row. Durable-resource tier adds ADR
45+
§4–5 lifecycle evidence. Importing durable machinery promotes the tier — say so.
46+
- Planted-red for anything new that is *not* a table row (a facet, a package boundary): revert,
47+
run, quote the failing line. A row needs no planted-red of its own.
48+
- Size: root-bytes-removed vs package-bytes-added and the four checkpoint metrics
49+
(`pnpm size --compare` against the base build), against the posted budget. Move-dominated is the
50+
rule; net growth is itemized, not explained away.
51+
- Live evidence for the changed path on at least one real target per family the denominator
52+
claims (`docs/agents/device-verification.md`); fixture-backed parity does not replace it.
53+
- Layering: `pnpm check:layering` green — R3 seam list narrows in the unit that removes an area's
54+
last platform import and never grows; R9/R10 must not grow (`docs/agents/testing.md`).
55+
56+
## What "done" is not
57+
58+
- A migrated command with a legacy fallback, a provider/local fallback, or a `dispatchCommand`
59+
branch left "just in case" — the row's retirement claim rejects it, and so does review.
60+
- Tests green because they mock the old seam. Grep the command's name across
61+
`vi.mock('.../core/dispatch.ts')` users before calling the tail closed.
62+
- A per-command policy file in `scripts/layering/`. If the row's generalized columns cannot
63+
express the invariant, first return to step 3; extensions are the exception and each one names
64+
what the seam could not make impossible.

docs/agents/pull-requests.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,27 @@
2020
bounded arrays in JSON, artifact paths for large raw data, progressive lookup for deeper detail.
2121
- Close every manual `agent-device` session opened during verification
2222
(`docs/agents/device-verification.md`) and report any cleanup that could not be completed.
23+
- Two readiness claims, never blurred: **published and reported** means the branch is pushed, the
24+
PR body carries the evidence gathered at a named commit, and CI on the head is the authority
25+
still to come; **merge-ready** means the required checks are green on the actual head and the
26+
live evidence for the changed path exists. "Don't wait for CI" licenses the first, not the
27+
second — say which one you are claiming.
28+
29+
## Rebasing onto a moving `main`
30+
31+
`main` has no "require branches up to date" rule; a rebase is not owed to GitHub. Rebase when
32+
there is a conflict, or when the commits `main` gained since your base touch a surface your
33+
change depends on or that decides your gates:
34+
35+
```sh
36+
pnpm check:affected --base <your-merge-base> --head origin/main # what main gained, by gate
37+
```
38+
39+
If that plan names only files and gates disjoint from yours, the rebase buys nothing but another
40+
full validation cycle. Evidence in the PR body is stamped with the commit it was gathered at, so a
41+
rebase dates it rather than invalidating it, and CI on the new head re-establishes it. A merge
42+
queue is the answer once independent migration units regularly land against each other; until
43+
then this rule is.
2344

2445
## PR body
2546

docs/agents/testing.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ cleanup for a directory they created — that's the global teardown's job, and p
7878
already existed for other reasons should stay (it's the fallback global sweep that's new, not a
7979
replacement for tests being tidy).
8080

81+
Mock the seam the code under test consumes, not the widest one available. A daemon handler that
82+
binds a device runtime is tested by handing it a fake `inspectFacts` / `bindDevice` (the fixture
83+
shape in `src/daemon/__tests__/snapshot-runtime-fixture.ts`; the shared mocks in
84+
`src/daemon/handlers/__tests__/session-command-harness.ts`; facts builders in
85+
`src/__tests__/test-utils/runtime-operation-facts.ts`) — not by `vi.mock('.../core/dispatch.ts')`.
86+
The generic dispatch mock is for tests *of* dispatch. Sixty-odd files still mock it from before
87+
the runtime seam existed; retiring `dispatchCommand('snapshot')` surfaced them one failure at a
88+
time. Do not add to that set, and when a command migrates (`docs/agents/adr-0019-unit.md`), its
89+
tests move to the runtime seam in the same PR.
90+
8191
Keep tests behavioral. Do not assert shapes or cases TypeScript already proves.
8292

8393
A test added as a regression pin must be shown to fail without the change it pins — vacuity is the

0 commit comments

Comments
 (0)