refactor(daemon): admit-before-bind as an admitted-plan token; retire the R32 syntax policy - #1841
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. |
21170a5 to
12c0fe4
Compare
|
P1 architecture blocker: the admitted token proves only the plan, not the device. |
12c0fe4 to
2b9d1da
Compare
|
Addressed in 2b9d1da (also rebased onto current main).
The token proves plan and device now; ownership qualification beyond identity comes from the facts the binder re-reads at bind time ( 🤖 Addressed by Claude Code |
…1841 updates the row when it lands
|
Re-reviewed exact Replace it with a genuinely non-copyable nominal token or private payload store, and add compile-time and behavioral regressions for spread/retargeting. Exact-head iOS Smoke is also red, and Linux Smoke was cancelled independently. |
…e and rebase guidance (#1836) * 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. * docs(agents): name the admitted-plan token in the ADR 0019 unit checklist (#1841) * docs(agents): merge-ready owes live evidence only for changed device-facing paths * docs(agents): the unit checklist documents the admission API on main; #1841 updates the row when it lands
2b9d1da to
bb0f894
Compare
|
Addressed in bb0f894 (rebased onto current main).
Layering OK; daemon/core units 2394; provider-integration 157; 🤖 Addressed by Claude Code |
|
Re-reviewed exact Consume admissions through module-private exact-identity storage/unwrap (for example, a Android Smoke failed with |
bb0f894 to
ddc103e
Compare
|
Addressed in ddc103e — rebased onto current main (squashed to one commit; #1847's Proxy / getter trapping — the token now carries nothing readable. Regressions ( Everything else as before: R32's per-command AST policy (main's grown version too — its delegation-chain checks are covered by the row's pinned operation owners: any second bind path would need its own operation calls, which the singular-execution column rejects) and the source-regex test are gone; manufactured-proof column rejects casts to Android Smoke on the previous head: 🤖 Addressed by Claude Code |
|
Re-reviewed exact Not ready to label or merge until the inherited #1860 Coverage failure is fixed/rebased, the timeout-shaped iOS failure is rerun and classified, and cancelled Linux is rerun. Please also correct the body disclosure from 12 touched files to the actual 11. |
…n 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.
…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.
ddc103e to
13fb32b
Compare
|
Re-reviewed exact Before merge, refresh the Summary: it still describes the superseded unique-symbol/public |
|
Body refreshed at Also folded in: this PR now carries the 🤖 Addressed by Claude Code |
|
Summary
Retro follow-up, item 3. The
snapshotunit's R32 gate recognized a particular AST call shape (inspectRequiredRuntimeUse({device, use: plan.use, inspectFacts: params.inspectFacts}), exactly once) and text-sniffed the binding file for a locally reimplemented admission — improved over the 300-line flow tracker, not solved. This makes the invariant impossible at the owning interface instead:admitRuntimePlan(renamed frominspectRequiredRuntimeUse; same facts-first, side-effect-free semantics) returns either a refusal{admitted:false, operation, fact}or a token. The token is a nominal class instance with nothing readable on it — nodevice, noplangetters. Its payload (a frozen copy of the device the facts were read for, plus the plan) lives in a module-privateWeakMapkeyed by the token's own identity.unwrapAdmittedRuntimePlanis the only reader and refuses anything not minted here. So a look-alike literal, a spread, or aProxyaround a real token — which types as the token with no assertion, and could otherwise trap a public getter to return a different device — has no entry in the store and is rejected. There is nothing left on the token's surface to trap.bindSnapshotCaptureRuntimetakes only the token (no separate device argument) and binds the device and use it unwraps. A route therefore cannot bind what it did not admit, cannot bind a different device than it admitted, and cannot admit through anything but the shared seam.DeviceInfoafter admission does not move the admitted identity.as BoundDeviceRuntime/row runtime types insrc/daemon/) now also rejects foras AdmittedRuntimePlan<…>— a mechanism-level change, proven red in the planted-row test, not a snapshot-specific policy.Deleted:
scripts/layering/runtime-command-cutover-snapshot.ts+ test (R32'sextensionsentry) and the source-regex test beside the descriptor tests. R32 is now a plain row — retirement, narrowing, singular execution — and nothing snapshot-specific remains inscripts/layering/. That is the shapedocs/agents/adr-0019-unit.mdtells the next unit to expect, and this PR updates that checklist row to the shipped API (inspectRequiredRuntimeUsenow appears nowhere in the repo).Dropped without replacement, on purpose: the old regex asserting the binding file contains no
isIosFamily/isIosSimulator/providerOwned. The narrowedBoundDeviceRuntime<Use>exposes only the admitted plan's required operations, so a platform branch there can no longer change which operation runs — the type already makes it harmless at the level the sniff guarded.Validation
@ts-expect-errordirectives unused → 2 tsc errors; restored clean.expected [Function] to throw /not one minted by admitRuntimePlan/; the previous branded-literal token fails the runtime retarget test.AdmittedRuntimePlanand expects the manufactured-proof violation; 43 layering cutover tests pass andcheck:layeringis OK with no R32 violation.captureSnapshotWithCustomActionson Android →unsupported-platform-leaf); throws without a facts seam; aProxy/spread/literal is refused; alias mutation ignored; and the owning interface, driven with a recordingBindDeviceRuntime, binds exactly the session's device.test:integration:provider52/157;pnpm typecheck, layering, lint, fallow clean. All 28 checks green on exact head13fb32bbd.dist, cleaned the dev daemon, iPhone 17 simulator:open com.apple.Preferences→snapshot→snapshot --actions→close, allsuccess: true(the first attempt ran against a stale pre-changedist; redone on the fresh build).12 files, +257/−299 — the snapshot unit's seam,
scripts/layering/, and the one checklist row. Scope did not expand beyond the snapshot/diff admission seam.