diff --git a/AGENTS.md b/AGENTS.md index e2e7d5e06..ae6422df9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,6 +14,7 @@ situational lives one hop away — load it when the task calls for it. | Which gates to run, test speed rules, shared fixtures | `docs/agents/testing.md` | | Adding or changing a CLI flag | `docs/agents/cli-flags.md` | | Opening a PR, or reviewing one | `docs/agents/pull-requests.md` | +| Migrating a command onto the request-bound platform runtime (an ADR 0019 unit) | `docs/agents/adr-0019-unit.md` | | Running commands against a real device | `docs/agents/device-verification.md` | | Issues, PRDs, triage labels | `docs/agents/issue-tracker.md`, `docs/agents/triage-labels.md` | | 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. under `~/.agent-device/dev/-`. Inspect with `pnpm daemon:state-dir`, override with `--state-dir`/`AGENT_DEVICE_STATE_DIR`, prune with `pnpm clean:daemon --prune-dev`. Daemons are isolated per worktree; **devices are not** — target different devices for concurrent worktrees. +- A fresh worktree is not ready until `pnpm install --frozen-lockfile && pnpm build` ran **in it**. + Until then `@agent-device/*` and optional peers (`ai`) resolve against whichever checkout last + installed — typecheck fails on `src/ai-sdk`, and the smoke lanes fail with "Missing dist build". + Neither is a regression. The layering scan (`check:layering`) reads tracked files only, so an + untracked new module is invisible to it until `git add`. +- Concurrent agents on one host: one full gate (`pnpm check`, `check:unit`) at a time — several + worktrees running unit suites together *are* the contention that produces timeout-shaped + failures. Verify a subagent's edits with `git -C status/diff`, never from its report; + keep one PR per worktree and address PRs by URL/branch, not a number recalled from context; + give each agent its own scratch directory. - Node ≥22. Prefer built-ins (`fetch`, Web Streams, `AbortSignal.timeout`) over compatibility wrappers unless the surrounding code needs a lower-level transport. - Emit with `tsdown` (Rolldown), typecheck with TypeScript 7 via `tsc`. Declaration generation uses diff --git a/docs/agents/adr-0019-unit.md b/docs/agents/adr-0019-unit.md new file mode 100644 index 000000000..8a3ae7c6a --- /dev/null +++ b/docs/agents/adr-0019-unit.md @@ -0,0 +1,64 @@ +# Migrating one command onto the request-bound platform runtime (ADR 0019 unit) + +The checklist for one command unit. ADR 0019 §6–10 holds the rules and the why; issue #1739 holds +the wave plan and per-unit budgets; this file is the order of operations, with the seam, fixture, +and gate you touch at each step. Everything here was rediscovered during the `snapshot` unit +(#1779, 134 files) — do not rediscover it again. + +## Before writing code + +1. **Post the unit on #1739 first**: exact denominator (every inventory source or supported + device-runtime cell), parity source, facet owner, expected deletions, evidence tier + (request-scoped vs durable-resource), and the size budget. A unit without a posted budget has + no acceptance criterion; the late budget negotiation on #1779 is the incident. +2. **One integration owner.** Parallel agents audit (platform parity, tests, size, architecture) + and implement only clearly disjoint lanes — a platform package's operations, a fixture builder, + the test migration. The shared spine — plan → facts → bind in the handler, the descriptor's + `platformExecution`, the cutover row — moves serially under one owner. Two agents editing the + spine produced the rebase churn on #1779. +3. **Ask the owning-interface question before adding any gate**: "can this invariant be made + impossible at the seam (a type only one function can produce, a required parameter, one + construction path)?" If yes, do that; a cutover-row *extension* that recognizes a call shape is + the fallback, not the default, and it is what AGENTS.md's "reconstructing a compiler" principle + forbids once it needs a second omission patch. + +## The unit, in order + +Each step names its declaration site; read that, not prose. + +| Step | Where | +| --- | --- | +| 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` | +| Plan resolution is pure and lives in contracts (`resolveRuntimePlan`), never in the handler | `packages/contracts/src/*-runtime-plan.ts` | +| Handler: resolve plan → `inspectRequiredRuntimeUse({ device, use: plan.use, inspectFacts })` → on `admitted`, bind **once** with that plan's use → operate. No admission-only binds, no `requireCommandSupported`, no capability bucket. (#1841 replaces this with `admitRuntimePlan({ device, plan, inspectFacts })`, whose returned token names both device and plan and is what the binder requires; this row follows it when it lands) | `src/daemon/handlers/session-runtime-admission.ts`; the `snapshot` route in `src/daemon/snapshot-runtime.ts` + `snapshot-runtime-binding.ts` as the model | +| 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` | +| 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/**` | +| 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) | +| 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 | +| 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) | +| 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 | + +## Evidence the unit review must contain + +- Request-scoped tier: the typed use declaration, fact coverage for every denominator cell, the + enumerated legacy-parity cell table, and the cutover-gate row. Durable-resource tier adds ADR + §4–5 lifecycle evidence. Importing durable machinery promotes the tier — say so. +- Planted-red for anything new that is *not* a table row (a facet, a package boundary): revert, + run, quote the failing line. A row needs no planted-red of its own. +- Size: root-bytes-removed vs package-bytes-added and the four checkpoint metrics + (`pnpm size --compare` against the base build), against the posted budget. Move-dominated is the + rule; net growth is itemized, not explained away. +- Live evidence for the changed path on at least one real target per family the denominator + claims (`docs/agents/device-verification.md`); fixture-backed parity does not replace it. +- Layering: `pnpm check:layering` green — R3 seam list narrows in the unit that removes an area's + last platform import and never grows; R9/R10 must not grow (`docs/agents/testing.md`). + +## What "done" is not + +- A migrated command with a legacy fallback, a provider/local fallback, or a `dispatchCommand` + branch left "just in case" — the row's retirement claim rejects it, and so does review. +- Tests green because they mock the old seam. Grep the command's name across + `vi.mock('.../core/dispatch.ts')` users before calling the tail closed. +- A per-command policy file in `scripts/layering/`. If the row's generalized columns cannot + express the invariant, first return to step 3; extensions are the exception and each one names + what the seam could not make impossible. diff --git a/docs/agents/pull-requests.md b/docs/agents/pull-requests.md index 270392917..285ac3d7e 100644 --- a/docs/agents/pull-requests.md +++ b/docs/agents/pull-requests.md @@ -20,6 +20,28 @@ bounded arrays in JSON, artifact paths for large raw data, progressive lookup for deeper detail. - Close every manual `agent-device` session opened during verification (`docs/agents/device-verification.md`) and report any cleanup that could not be completed. +- Two readiness claims, never blurred: **published and reported** means the branch is pushed, the + PR body carries the evidence gathered at a named commit, and CI on the head is the authority + still to come; **merge-ready** means the required checks are green on the actual head and, + where the change touches a device-facing path, the live evidence for that path exists (the + device-facing bullet above; docs-only and pure-tooling changes owe none). "Don't wait for CI" + licenses the first, not the second — say which one you are claiming. + +## Rebasing onto a moving `main` + +`main` has no "require branches up to date" rule; a rebase is not owed to GitHub. Rebase when +there is a conflict, or when the commits `main` gained since your base touch a surface your +change depends on or that decides your gates: + +```sh +pnpm check:affected --base --head origin/main # what main gained, by gate +``` + +If that plan names only files and gates disjoint from yours, the rebase buys nothing but another +full validation cycle. Evidence in the PR body is stamped with the commit it was gathered at, so a +rebase dates it rather than invalidating it, and CI on the new head re-establishes it. A merge +queue is the answer once independent migration units regularly land against each other; until +then this rule is. ## PR body diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 067c287d0..80aa7ae0c 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -90,6 +90,16 @@ forked workers, and any daemon a test spawned running with that `TMPDIR` — the until the last of them exits. A concurrent run in another worktree is live by both tests. If the check fails, the leak is this run's: a teardown that did not execute, not history. +Mock the seam the code under test consumes, not the widest one available. A daemon handler that +binds a device runtime is tested by handing it a fake `inspectFacts` / `bindDevice` (the fixture +shape in `src/daemon/__tests__/snapshot-runtime-fixture.ts`; the shared mocks in +`src/daemon/handlers/__tests__/session-command-harness.ts`; facts builders in +`src/__tests__/test-utils/runtime-operation-facts.ts`) — not by `vi.mock('.../core/dispatch.ts')`. +The generic dispatch mock is for tests *of* dispatch. Sixty-odd files still mock it from before +the runtime seam existed; retiring `dispatchCommand('snapshot')` surfaced them one failure at a +time. Do not add to that set, and when a command migrates (`docs/agents/adr-0019-unit.md`), its +tests move to the runtime seam in the same PR. + Keep tests behavioral. Do not assert shapes or cases TypeScript already proves. A test added as a regression pin must be shown to fail without the change it pins — vacuity is the