Skip to content

docs(agents): ADR 0019 unit checklist, owning-seam mock rule, worktree and rebase guidance - #1836

Merged
thymikee merged 4 commits into
mainfrom
docs/adr-0019-unit-checklist
Aug 18, 2026
Merged

docs(agents): ADR 0019 unit checklist, owning-seam mock rule, worktree and rebase guidance#1836
thymikee merged 4 commits into
mainfrom
docs/adr-0019-unit-checklist

Conversation

@thymikee

Copy link
Copy Markdown
Member

Summary

Retro follow-up, item 2 of the agreed list. Four agent-facing doc changes; no code.

  • docs/agents/adr-0019-unit.md (new) — the checklist for one command unit: what to post on refactor(daemon): platform-free daemon — broader migration waves (successor to #1696) #1739 before writing code, one integration owner for the shared spine, the "can this be made impossible at the owning interface?" question before any cutover-row extension, then the unit in order with the declaration site for each step (use → plan → handler admit/bind-once → descriptor flip → platform packages → cutover row → deletions → tests on the runtime seam → facets), the evidence a unit review must carry, and what "done" is not. Linked from the AGENTS.md task table. Everything in it was rediscovered on refactor: migrate snapshot to device runtime #1779; ADR 0019 keeps the why, this keeps the how.
  • docs/agents/testing.md — mock the seam the code under test consumes: fake inspectFacts / bindDevice (fixture, harness, and facts-builder locations named), not vi.mock('.../core/dispatch.ts'). Sixty-odd files still mock dispatch; retiring dispatchCommand('snapshot') surfaced them one failure at a time. Don't add to the set; a migrating command moves its tests off it in the same PR.
  • AGENTS.md — fresh-worktree preflight (pnpm install --frozen-lockfile && pnpm build in the worktree, else @agent-device/*/ai resolve against another checkout and smoke lanes fail with "Missing dist build"; layering scan reads tracked files only) and concurrent-agent hygiene (one full gate per host — parallel suites are the contention; verify subagent edits with git -C; one PR per worktree; PRs by URL not recalled number; per-agent scratch dirs).
  • docs/agents/pull-requests.md — two readiness claims that "don't wait for CI" kept blurring: published and reported vs merge-ready. And the rebase rule: main has no up-to-date protection, so rebase on conflict or when pnpm check:affected --base <merge-base> --head origin/main names a surface yours depends on; evidence is stamped with the commit it was gathered at, so a rebase dates it rather than invalidating it. Merge queue deferred until migration units regularly overlap.

Validation

Docs only; every claim was checked against the tree while writing: defineUse / resolveSnapshotRuntimePlan in packages/contracts/src/platform-runtime-operations.ts, inspectRequiredRuntimeUse in session-runtime-admission.ts, the row model and cutoverRowDefects in scripts/layering/runtime-command-cutover-*.ts, the fixture/harness/facts-builder paths, check:affected --head (verified on d76e0f94e...origin/main, which correctly lists only the #1826 files). Ran into both worktree traps myself in the same session (unresolved ai, "Missing dist build") before writing them down. check:affected fails open on testing.md (selector-owning) by design; no runtime gate applies.

@thymikee

Copy link
Copy Markdown
Member Author

Two documentation corrections are needed before readiness. In docs/agents/adr-0019-unit.md, inspectRequiredRuntimeUse is shown with three positional arguments, but the owning API accepts one object { device, use, inspectFacts }; fix the copy-paste checklist. In docs/agents/pull-requests.md, the unconditional live-evidence merge rule contradicts the preceding device-facing condition and docs-only guidance; qualify it to changed device-facing paths. Docs CI is otherwise green and the PR is mergeable.

@thymikee
thymikee force-pushed the docs/adr-0019-unit-checklist branch from 1c6b855 to d71cd08 Compare August 18, 2026 14:56
@thymikee

Copy link
Copy Markdown
Member Author

Both fixed.

🤖 Addressed by Claude Code

@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact 9b0f4958: the device-facing live-evidence qualification is fixed. One dependency blocker remains: the ADR checklist now teaches admitRuntimePlan({ device, plan, inspectFacts }) / AdmittedRuntimePlan, but that API is not on this PR’s base or current main; it exists only in still-open #1841, which itself has an unresolved exact-device coupling finding. Either stack/merge this docs PR only after a corrected #1841 lands, or document the current object API inspectRequiredRuntimeUse({ device, use: plan.use, inspectFacts }). Checks are otherwise green.

…e 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.
@thymikee
thymikee force-pushed the docs/adr-0019-unit-checklist branch from 9b0f495 to b64ccb5 Compare August 18, 2026 15:59
@thymikee

Copy link
Copy Markdown
Member Author

Decoupled in b64ccb5 (rebased onto current main; the testing.md conflict with the merged #1834 paragraph was resolved keeping both).

The checklist row now documents the API on main today, in the object form the route actually uses: inspectRequiredRuntimeUse({ device, use: plan.use, inspectFacts }) → on admitted, bind once with that plan's use. It notes in-line that #1841 replaces it with admitRuntimePlan({ device, plan, inspectFacts }) and that the row follows when that lands — I'll carry that one-row update inside #1841 after this merges, so neither PR is stacked on the other.

🤖 Addressed by Claude Code

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 18, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact b64ccb56: clean and merge-ready. The ADR checklist now accurately documents current main’s object-form inspectRequiredRuntimeUse({ device, use: plan.use, inspectFacts }), while the forward note matches corrected #1841’s device+plan admission token. Scope is honestly four agent-facing docs with no code, and all exact-head checks are green.

@thymikee
thymikee merged commit 72d421f into main Aug 18, 2026
8 checks passed
@thymikee
thymikee deleted the docs/adr-0019-unit-checklist branch August 18, 2026 16:43
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-18 16:43 UTC

thymikee added a commit that referenced this pull request Aug 19, 2026
…on API

#1836 documented inspectRequiredRuntimeUse with a forward note pointing here;
this PR makes admitRuntimePlan real, so the row now teaches it plus the
identity-keyed unwrap the binder uses, and points at the shared snapshot/diff
owning interface as the model.
thymikee added a commit that referenced this pull request Aug 19, 2026
… the R32 syntax policy (#1841)

* refactor(daemon): admit-before-bind as an identity-keyed admitted-plan token; retire the R32 syntax policy

admitRuntimePlan (was inspectRequiredRuntimeUse) takes the plan and, on
success, mints an AdmittedRuntimePlan: a nominal class instance with nothing
readable on it. Its payload — a frozen copy of the device the facts were read
for, and the plan — lives in a module-private WeakMap keyed by the token's
exact identity, and the only way to read it is unwrapAdmittedRuntimePlan,
which refuses anything not minted here. The snapshot owning interface
(resolveBoundSnapshotCaptureRuntime, #1847) admits through it and its private
binder takes only the token: no bare plan, no separate device, and no
look-alike — a spread lacks the #private member (not assignable), a Proxy
around a real token types as the token but is a different identity (refused
at unwrap), Object.assign/defineProperty throw on the frozen instance, and the
class value is not exported so its constructor is not nameable.

That retires scripts/layering/runtime-command-cutover-snapshot.ts — R32's
per-command AST policy (call-shape recognition of the admission and a text
sniff for a local admission) — and the source-regex test beside the descriptor
tests. The generic row keeps retirement, narrowing, and singular execution;
the manufactured-proof column now also rejects casts to AdmittedRuntimePlan.

Planted reds: token degraded to a plain public shape → 2 unused
@ts-expect-error directives; unwrap reading the token surface via getters →
the Proxy regression fails; getter-based branded literal → the runtime
retarget test fails.

* docs(agents): the ADR 0019 unit checklist teaches the shipped admission API

#1836 documented inspectRequiredRuntimeUse with a forward note pointing here;
this PR makes admitRuntimePlan real, so the row now teaches it plus the
identity-keyed unwrap the binder uses, and points at the shared snapshot/diff
owning interface as the model.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant