Skip to content

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

Merged
thymikee merged 2 commits into
mainfrom
refactor/inspected-use-token
Aug 19, 2026
Merged

refactor(daemon): admit-before-bind as an admitted-plan token; retire the R32 syntax policy#1841
thymikee merged 2 commits into
mainfrom
refactor/inspected-use-token

Conversation

@thymikee

@thymikee thymikee commented Aug 18, 2026

Copy link
Copy Markdown
Member

Summary

Retro follow-up, item 3. The snapshot unit'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({ device, plan, inspectFacts })
        ↓ mints
AdmittedRuntimePlan<Plan>        // opaque: no readable payload on the object at all
        ↓ required by
resolveBoundSnapshotCaptureRuntime → bindSnapshotCaptureRuntime(bindDevice, admission)
        ↓ reads via
unwrapAdmittedRuntimePlan(admission) → { device, plan }   // by exact token identity
  • admitRuntimePlan (renamed from inspectRequiredRuntimeUse; 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 — no device, no plan getters. Its payload (a frozen copy of the device the facts were read for, plus the plan) lives in a module-private WeakMap keyed by the token's own identity.
  • unwrapAdmittedRuntimePlan is the only reader and refuses anything not minted here. So a look-alike literal, a spread, or a Proxy around 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.
  • bindSnapshotCaptureRuntime takes 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.
  • Storing a frozen copy also closes the mutable-alias path: mutating the caller's DeviceInfo after admission does not move the admitted identity.
  • The one escape left is a type assertion, which the parametrized cutover gate's manufactured-proof column (already rejecting as BoundDeviceRuntime/row runtime types in src/daemon/) now also rejects for as 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's extensions entry) and the source-regex test beside the descriptor tests. R32 is now a plain row — retirement, narrowing, singular execution — and nothing snapshot-specific remains in scripts/layering/. That is the shape docs/agents/adr-0019-unit.md tells the next unit to expect, and this PR updates that checklist row to the shipped API (inspectRequiredRuntimeUse now appears nowhere in the repo).

Dropped without replacement, on purpose: the old regex asserting the binding file contains no isIosFamily/isIosSimulator/providerOwned. The narrowed BoundDeviceRuntime<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

  • Planted red, type level: degrading the token to a plain public shape makes both @ts-expect-error directives unused → 2 tsc errors; restored clean.
  • Planted red, behavior level: an unwrap that reads the token's surface instead of the identity store (i.e. the earlier getter-based design) fails the Proxy regression — expected [Function] to throw /not one minted by admitRuntimePlan/; the previous branded-literal token fails the runtime retarget test.
  • Planted red, gate level: the mechanism test's planted row includes a cast to AdmittedRuntimePlan and expects the manufactured-proof violation; 43 layering cutover tests pass and check:layering is OK with no R32 violation.
  • Behavioral tests: admits and carries the exact plan + a frozen device copy; refuses at the first unavailable required op (captureSnapshotWithCustomActions on Android → unsupported-platform-leaf); throws without a facts seam; a Proxy/spread/literal is refused; alias mutation ignored; and the owning interface, driven with a recording BindDeviceRuntime, binds exactly the session's device.
  • Full unit suite 920 files / 6,989 tests; test:integration:provider 52/157; pnpm typecheck, layering, lint, fallow clean. All 28 checks green on exact head 13fb32bbd.
  • Live: rebuilt dist, cleaned the dev daemon, iPhone 17 simulator: open com.apple.Preferencessnapshotsnapshot --actionsclose, all success: true (the first attempt ran against a stale pre-change dist; 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.

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.31 MB 2.31 MB +399 B
JS gzip 759.3 kB 759.5 kB +177 B
npm tarball 882.6 kB 882.8 kB +176 B
npm unpacked 3.08 MB 3.08 MB +399 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.3 ms 28.0 ms +0.7 ms
CLI --help 68.3 ms 68.6 ms +0.3 ms

Top changed chunks: no changes in the largest emitted chunks.

@thymikee

Copy link
Copy Markdown
Member Author

P1 architecture blocker: the admitted token proves only the plan, not the device. bindSnapshotCaptureRuntime(bindDevice, device, admission) accepts the bind target separately, so a caller can admit facts for device A and use that token to bind device B. Carry the admitted device/ownership-qualified identity inside the opaque token, remove the binder’s separate device argument, and add a type/behavior regression showing mismatched-device binding cannot be expressed. CI/live evidence are otherwise green, but the structural goal is not yet achieved.

@thymikee
thymikee force-pushed the refactor/inspected-use-token branch from 12c0fe4 to 2b9d1da Compare August 18, 2026 15:58
@thymikee

Copy link
Copy Markdown
Member Author

Addressed in 2b9d1da (also rebased onto current main).

  • AdmittedRuntimePlan<Plan> now carries device: DeviceInfo — the device whose owner facts were read — alongside the plan and the module-private proof key; only admitRuntimePlan sets it.
  • bindSnapshotCaptureRuntime(bindDevice, admission): the separate device argument is gone; the binder binds admission.device with admission.plan.use. The route passes the token alone.
  • Regressions: @ts-expect-error on bindSnapshotCaptureRuntime(undefined, ANDROID_EMULATOR, admission) — mismatched-device binding is not expressible; planted red by re-adding the device parameter → 2 tsc errors (unused directive + old call), restored clean. Behavior: a recording BindDeviceRuntime sees exactly [IOS_SIMULATOR] after admitting for IOS_SIMULATOR, and admission.device is that device.
  • Layering guard OK, daemon/core units 2383, provider-integration 157, fallow/lint clean.

The token proves plan and device now; ownership qualification beyond identity comes from the facts the binder re-reads at bind time (narrowDeviceBinding on the owner's facts), which is unchanged.

🤖 Addressed by Claude Code

thymikee added a commit that referenced this pull request Aug 18, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact 2b9d1dac: P1 remains. Although the binder no longer accepts a separate device, AdmittedRuntimePlan is still a structurally forgeable object: spreading a valid admission and overriding its public device or plan preserves the enumerable unique-symbol brand, remains assignable without an assertion, and lets the binder use facts admitted for A to bind B.

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.

thymikee added a commit that referenced this pull request Aug 18, 2026
…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
@thymikee
thymikee force-pushed the refactor/inspected-use-token branch from 2b9d1da to bb0f894 Compare August 18, 2026 17:09
@thymikee

Copy link
Copy Markdown
Member Author

Addressed in bb0f894 (rebased onto current main).

AdmittedRuntimePlan is now a genuinely nominal, non-copyable token: a class whose payload lives in #device / #plan behind getter-only accessors, minted only through a closure the class sets in its static {} block; the class value is not exported (only its type), so no module can name the constructor at runtime — admitRuntimePlan is the sole producer.

  • Compile-time: { ...admission, device: ANDROID_EMULATOR } is a plain object without the private members and is not assignable (@ts-expect-error); a literal likewise. Planted red: degrading the token to a plain public shape makes both directives unused → 2 tsc errors, restored clean.
  • Behavioral: Object.keys({ ...admission }) is [] (spread carries nothing), Object.assign(admission, { device }) and defineProperty throw TypeError (getter-only, frozen), and admission.device stays the admitted device. That test fails against the previous branded-literal token (- Expected [] + Received ["admitted","device","plan"]), so it is the regression for this finding.
  • The manufactured-proof column of the cutover gate still rejects as AdmittedRuntimePlan<…> in src/daemon/ (name unchanged).

Layering OK; daemon/core units 2394; provider-integration 157; pnpm typecheck clean; lint/fallow clean. Re-ran the iOS Smoke job on the previous head; will re-run on this head once the workflow finishes.

🤖 Addressed by Claude Code

@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact bb0f8940: P1 remains. A Proxy around a valid token is inferred as the same nominal type without an assertion and can trap the public device / plan getters to return device B or another plan. The binder consumes those getters, so this bypasses the facts admitted for device A despite the private fields and frozen target.

Consume admissions through module-private exact-identity storage/unwrap (for example, a WeakMap) and add a compile-time/runtime proxy-retarget regression. Also prevent mutation through the original mutable DeviceInfo alias by storing an immutable identity or copy as appropriate.

Android Smoke failed with wait_capture_stalled and zero readable captures; that looks flake-prone and unrelated to this snapshot binder, but it needs a rerun/classification. Deterministic gates are green.

@thymikee
thymikee force-pushed the refactor/inspected-use-token branch from bb0f894 to ddc103e Compare August 18, 2026 18:01
@thymikee

Copy link
Copy Markdown
Member Author

Addressed in ddc103e — rebased onto current main (squashed to one commit; #1847's diff migration restructured the seam, so the token now sits in main's resolveBoundSnapshotCaptureRuntime owning interface and its module-private bindSnapshotCaptureRuntime).

Proxy / getter trapping — the token now carries nothing readable. AdmittedRuntimePlan is a nominal class (one #private member keeps literals and spreads from being assignable; class value not exported) whose payload — a frozen copy of the device the facts were read for, plus the plan — lives in a module-private WeakMap keyed by the token's exact identity. The only reader is unwrapAdmittedRuntimePlan, which refuses anything not minted here; the private binder unwraps through it and binds that device with that plan. A Proxy around a real token still types as the token (no assertion needed — the finding), but it is a different identity: refused at unwrap, and there is no device/plan getter left on the token for it to trap. Storing a frozen copy also closes the mutable-DeviceInfo-alias path (test: mutate the caller's object after admission → the admitted device does not move).

Regressions (session-runtime-admission.test.ts, snapshot-runtime-binding.test.ts): compile-time — literal and spread not assignable (@ts-expect-error; planted red: degrade the token to a plain public shape → both directives unused); runtime — a Proxy trapping device to return ANDROID_EMULATOR is refused by unwrapAdmittedRuntimePlan (planted red: a getter-surface unwrap, i.e. the previous design, fails this test), spread has no own keys, defineProperty throws, alias mutation ignored; and the owning interface, with a recording BindDeviceRuntime, binds exactly the session's device.

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 AdmittedRuntimePlan. pnpm typecheck clean, layering OK, daemon/core units 2403, provider-integration 157, lint/fallow clean.

Android Smoke on the previous head: wait_capture_stalled right after an orientation change, only Android smoke failure in the last 30 runs, re-run green — classified as an emulator rotation-time capture stall, unrelated to this seam.

🤖 Addressed by Claude Code

@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact ddc103e3: the prior admission-token blocker is fixed. Exact-token WeakMap identity, a private nominal token and binder, frozen device copy, no readable payload surface, plus Proxy/spread/alias regressions close the retargeting paths. I found no remaining code finding.

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.
@thymikee
thymikee force-pushed the refactor/inspected-use-token branch from ddc103e to 13fb32b Compare August 19, 2026 05:54
@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact 13fb32bb: the identity-keyed admission implementation remains code-clean and every exact-head check is green.

Before merge, refresh the Summary: it still describes the superseded unique-symbol/public admission.plan + admission.device shape and a separate device binder argument. The shipped design is a nominal token with no readable payload, an identity-keyed private WeakMap payload, and a private binder that unwraps the exact minted token.

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

Copy link
Copy Markdown
Member Author

Body refreshed at 13fb32bbd. The Summary now describes the shipped design rather than the superseded one: an opaque nominal token with no readable payload (no device/plan getters), a module-private identity-keyed WeakMap holding a frozen device copy plus the plan, unwrapAdmittedRuntimePlan as the only reader — which is what makes a Proxy around a real token refusable — and a private binder that takes the token alone with no separate device argument. The old diagram showing unique symbol + public admission.plan/admission.device and bindSnapshotCaptureRuntime(bindDevice, device, admission) is gone, and the Validation section now lists the three planted reds that actually pin this design (plain-shape token, surface-reading unwrap, branded-literal token).

Also folded in: this PR now carries the docs/agents/adr-0019-unit.md row update to the shipped API, so inspectRequiredRuntimeUse appears nowhere in the repo.

🤖 Addressed by Claude Code

@thymikee
thymikee merged commit f3d5b3d into main Aug 19, 2026
28 checks passed
@thymikee
thymikee deleted the refactor/inspected-use-token branch August 19, 2026 08:46
@github-actions

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

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