docs(specs): design the dedicated native plan_admitted mint control - #306
Conversation
Merge Protections🔴 1 of 2 protections blocking · waiting on 🙋 you
🔴 🚦 Auto-queueWaiting for
This rule is failing.When all merge protections are satisfied and these conditions match, this pull request will be queued automatically.
Show 1 satisfied protection🟢 require green CI on main
|
There was a problem hiding this comment.
Pull request overview
Adds a new design spec describing a dedicated native “mint control” for producing kernel-signed plan_admitted events (as a prerequisite for future PlanForge admission work), including authority model, verification rules, storage API shape, and a proposed slice ladder.
Changes:
- Introduces a detailed spec for a native
plan_admittedmint control, including recommended broker-host placement and SO_PEERCRED-based authority. - Defines a field-by-field verification/derivation rule set and proposes a two-phase
record_*/seal_*SQLite API mirroring existing governed admission patterns. - Documents a multi-slice implementation plan (S1–S6) and enumerates operator decision gates/questions.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| never to a fixture. Separately: that `plan-cycle` fixture is referenced by **zero** tests or | ||
| scripts (`grep` for `plan-cycle` over `test/`, `scripts/`, `.github/`: no matches), so it is dead | ||
| today; `verify-signed-tape.test.ts:41-81` exercises only `valid`, `tampered`, `bad-root`, and two | ||
| in-memory mutations. |
| fixture (§5.6). The verifier is kind-agnostic — `verifyEvent` branches only on signature | ||
| presence/algorithm/hash/key/bytes and never reads `parsed.kind` *(reported)* — so it needs no | ||
| code change; only the tape content changes from `unsigned` to verified. |
| **Recommendation:** establish the linkage *by construction*, not by string comparison — the | ||
| parent link from a per-task admission back to its plan admission must be recorded natively by | ||
| the same control that mints both, and the approval arm must verify that natively-recorded link. | ||
| This is the candidate-transaction spec's own data flow (that spec's §S5, spec:87-127) and it converts the |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1db38cbec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| | Field | Origin today | Obligation | Failure | | ||
| |---|---|---|---| | ||
| | `input_digest` | `preview.ts:62-63` | **Re-derive.** The control loads the compiled plan bytes from broker-owned CAS (§4.4) and digests the bytes *it* loaded. Caller names a content ref, never a digest value. | `PlanInputDigestMismatch` | |
There was a problem hiding this comment.
If S2 follows this instruction literally and hashes the loaded bytes, every minted input_digest will differ from the PlanForge value: packages/planforge/src/preview.ts:62-63 normalizes CRLF and passes a JavaScript string to digest(), which hashes its JSON-quoted representation rather than the raw bytes. That breaks correlation with the preview and makes the proposed mismatch checks compare different digest domains; require the native control to reproduce the normalization and JSON-string serialization byte-for-byte, or introduce a separately named native digest.
Useful? React with 👍 / 👎.
| | Field | Origin today | Obligation | Failure | | ||
| |---|---|---|---| | ||
| | `input_digest` | `preview.ts:62-63` | **Re-derive.** The control loads the compiled plan bytes from broker-owned CAS (§4.4) and digests the bytes *it* loaded. Caller names a content ref, never a digest value. | `PlanInputDigestMismatch` | | ||
| | `plan_id` | `preview.ts` | **Re-derive** from the loaded bytes. | `PlanInputRejected` | |
There was a problem hiding this comment.
The existing plan_id cannot be derived from the content bytes using an unspecified native digest: packages/planforge/src/preview.ts:26-61 constructs it from the first eight hex characters of an insertion-order JSON fingerprint over parsed constraints, evidence refs, goal, remote, trusted base, and worktree policy. The same fingerprint also forms idempotency_key, yet the next rows say reproducing that scheme buys nothing. Without explicitly porting this derivation, the mint can assign an ID different from the previewed plan and break admission/receipt correlation; either require the exact fingerprint algorithm for both fields or define a new versioned identity.
Useful? React with 👍 / 👎.
| 1. **Verify the referenced event — S4.** Load the named `plan_admitted` and verify its own detached | ||
| signature against pinned trusted keys and an expected kernel signer. The activity-claim path | ||
| already does exactly this with `load_verified_authority_event(conn, event_id, trusted_keys, | ||
| expected_signer, label)` (`sqlite.rs:17959-17984`). |
There was a problem hiding this comment.
When the reference names another event signed by the configured kernel signer, load_verified_authority_event still succeeds: the inspected implementation at native/crates/bp-ledger/src/storage/sqlite.rs:17959-17984 checks only existence, signature presence, signer identity, and signature validity, then returns an arbitrary Event. S4 therefore does not establish that the reference names a plan_admitted unless the new resolution also checks EventKind::PlanAdmitted and Payload::PlanAdmittedV1; specify that check and add a signed-wrong-kind negative test.
Useful? React with 👍 / 👎.
| **Decision: S2's round-trip is a Rust integration test in `native/crates/bp-ledger/tests/` that | ||
| drives `record_plan_admission_v1` + `seal_plan_admission_v1` against a temp store, calls | ||
| `export_signed_tape` to write `tape.json`, then invokes `node scripts/verify-signed-tape.mjs | ||
| --fixture <dir>` via `std::process::Command`.** It must be declared to the L0 ceremony reviewers as |
There was a problem hiding this comment.
In the inspected S2 test plan, this test invokes the two SqliteStore methods directly, so it can pass even if the newly added broker resolver/backend/snapshot-verifier composition is broken or never calls them correctly. It also cannot call that crate-private composition from bp-ledger/tests. Add the end-to-end test under bp-authority-broker (or expose an appropriate test seam) and drive the composition before exporting the resulting store; retain a direct ledger test only as lower-level coverage.
Useful? React with 👍 / 👎.
|
Operator gate answers recorded (2026-08-19), §8:
Defaults taken where the spec recommends: Q2 content-addressed bytes + native re-derivation; Q8 stay at V1 (linkage in the projection table); Q9 stand alongside the quarantined TS port; Q11 fail-closed on non-Linux. Still open (asked before S2/S4 start): Q10 (advisory validation status — ratification required), Q12c (CAS staging path), Q13 (S4 checkpoint coverage), Q6 (protocol convergence). Q4/Q7 UNVERIFIED items to be resolved from code. S1 authorized; authored conventionally (no dogfood). |
|
UNVERIFIED items resolved (2026-08-19, read-only adversarial pass over main): Q7 — production Q4 — operator-key signing deferral: literally current for |
…ntrol S1) (#314) # S1 — close the in-process `plan_admitted` append hole First slice of the plan_admitted native mint-control program (design spec: `docs/superpowers/specs/2026-08-17-plan-admitted-native-mint-control-design.md`, PR #306 — §7 S1, §6.1). Authored conventionally, not via dogfood: the control cannot gate its own construction. **Operator gates answered before this slice started** (recorded on #306, 2026-08-19): build the mint control (Q3/Q5); broker placement (Q1); recovery-evidence semantics (Q12b); **Q12a = accept the tier asymmetry** — this slice front-loads the block per the a53519b V5 precedent, so the mint's exclusivity claim is never false for any intervening window. ## What changes - `EventKind::PlanAdmitted` joins `validate_external_append`'s always-blocked set (`bp-ledger/src/storage/sqlite.rs`) — the unsigned generic-ingest lane now rejects it (`CallerSuppliedTrustSpineEvent`, surfaced as `storage_failure`); the signed lane keeps its pre-existing wire-guard rejection (`CallerSuppliedSignedAuthorityEvent` / `caller_supplied_authority_event`). Both lanes are pinned with the two **different** typed errors plus empty-tape assertions (`bp-ledger/tests/plan_lifecycle.rs`, 9 tests). - **Beyond the spec, from the adversarial review:** a payload-variant guard — a `Payload::PlanAdmittedV1` payload is refused whatever `kind` the envelope declares. Without it, `kind: model_request` + `payload: PlanAdmittedV1` sailed through the kind-only check via direct in-process `store.append` (which does not canonicalize), and `bp-replay` dispatches on the **payload** variant, so replay would have applied it as a genuine admission. Not reachable in production today (both serve-lane writers canonicalize first, enforcing kind↔payload agreement) — but S1's claim must not rest on an unenforced caller invariant. Both clauses are independently pinned (mutation-verified: deleting either clause fails exactly one test). - Integration pin test 2 (`test/ledger-integration/planforge-plan-admission.test.ts`) rewritten from "lands unsigned, unverifiable" to "rejected on the unsigned lane too" — the authorized strengthening its own header anticipated (NG3). **Test 1 is byte-unmodified.** The "no third path" property is now total. - `bp-replay/tests/planforge_cycle.rs` repointed onto a new `#[cfg(any(test, feature = "test-support"))]` insert helper (`insert_event_bypassing_external_validation_for_tests`) — the only test that appended `plan_admitted` through the public API. Replay assertions unchanged. The helper hard-asserts against `TapeCheckpoint` in every profile. - Falsified rationales corrected (comment-only, verified mechanically — non-comment diff of `serve.rs` is one panic-string): `serve.rs` tier doc + `kinds_on_the_second_denylist_still_pass_the_unsigned_lane` doc/panic text; `packages/ledger-client/src/emitter.ts`; `apps/cli/src/plan-admission-port.ts`; `packages/ledger-client/test/caller-supplied-trust-spine-kinds-sync.test.ts`. Wire classification does NOT move — `PlanAdmitted` stays `REJECTED_ONLY_WHEN_SIGNED`; array membership, disposition table, and all assertion logic byte-identical. - `plan_admitted` row added to `docs/operations/trust-spine-compatibility-matrix.md`. Between S1 and S2 `plan_admitted` has zero writers outside test support — deliberate; the kind has no production writer today either (standing disclosure #5). ## Ceremony record (L0 — full 4-role) | Role | Actor | Verdict | |---|---|---| | Implementer | opus, TDD (RED evidence captured per change) | self-verify green | | Independent Reviewer (fresh session) | opus | **PASS** ×3 rounds (initial `b29712e`; repair delta `eb8f78b`; final `29f905a`) | | Adversarial reviewer | **sonnet substitute — DISCLOSED DEVIATION**, see below | **HOLDS** | | Acceptance-criteria verifier | sonnet, independently re-ran all gates | **OVERALL: PASS** ×2 (`b29712e`, `eb8f78b`) | **Disclosure — Codex substitution:** the adversarial role is specced as Codex; the ChatGPT quota was exhausted mid-ceremony (resets 2026-08-22 19:01), so a fresh sonnet session ran the identical DEFEAT brief. It produced the payload-smuggling finding (fixed in `2663cca`) — the role earned its seat. Re-running real Codex post-reset is available on request. Review findings → repairs, all in-branch: adversarial smuggling finding → `2663cca` (+ regression test); reviewer MEDIUM×2 (falsified serve.rs/emitter.ts rationales) + LOW×2 (stale port doc, `debug_assert`→`assert`) → `eb8f78b`/`2663cca`; re-review MEDIUM (clause-(b) no longer test-pinned after the payload guard) → `29f905a` (mirror test, mutation-verified). ## Gates (final HEAD) - Whole-workspace `cargo test --manifest-path native/Cargo.toml` (no `-p`): **1277 passed, 102 suites** (independently re-run by the verifier at each round) - Scoped vitest (4 files: plan-admission integration, kinds-sync, admitted-plan-reader, plan-admission-port): **19 passed** - `pnpm typecheck` clean; `cargo fmt --check` clean on touched files; biome clean on touched TS (isolated-dir probe) - No changeset: the only `src/` changes are comment-only; precedent #301 (test-only → none) ## Carried obligations (recorded, not this slice) - Generalize the payload-variant guard to the other always-blocked kinds (`GovernedDispatchV5AdmissionRecordedV1`, `PromotionReconciliationResolved`) — same smuggling shape, same non-exploitability today; S2. - The storage-layer always-blocked set is now a third denylist with no membership drift guard (the two serve.rs arrays have one); S2. - Coverage note: old test 2 was the only end-to-end join of the port's emitted payload with `createDefaultAdmittedPlanReader`; rejoined by S2's mint round trip (§6.2(1)). - Fourth write path (raw `node:sqlite` INSERT in `admitted-plan-reader.test.ts`) remains open — test-only, spec-sanctioned out of scope (§6.1). - The rejection error string does not distinguish "sent a plan_admitted envelope" from "smuggled the payload under another label" (tests distinguish the inputs; S2 owns the mint's error vocabulary). **L0: not auto-merge eligible — operator admin-merge required. Opened as DRAFT.**
Pre-S2 operator gate round (2026-08-23) — answers recordedSecond gate round per the spec's §8, answered via AskUserQuestion in-session (follows the 2026-08-19 round recorded above). All five recommendations ratified:
With this round, every §8 gate blocking S2–S4 is answered. Still open: Q7 (production Next: S2 (storage API + broker-private composition, L0 4-role), authored conventionally per §9. |
…pi and broker composition (mint-control S2) (#315) ## Mint-control S2 — the plan-admission mint: storage API + broker-private composition Implements §7 S2 of `docs/superpowers/specs/2026-08-17-plan-admitted-native-mint-control-design.md` (ratified via #306). Second slice of the dedicated native `plan_admitted` mint control; follows S1 (#314). Built conventionally per §9 — the control cannot gate its own construction. ### What this delivers **Half 1 — `bp-ledger` storage API** (sited beside the V5 admission pair, mirroring its shape): - `record_plan_admission_v1` / `seal_plan_admission_v1` / `PlanAdmissionDispositionV1` — two-phase mint: one `BEGIN IMMEDIATE`, idempotency-identity-first resolution, §4.3 field derivation, self-canonicalized + kernel-signed event via the private insert pair, projection row, `AwaitingCheckpoint` → sealed. - `plan_admissions` projection table + migration: `CHECK`-constrained state vocabulary, `UNIQUE` admission event id, `BEFORE DELETE` no-delete + seal-only `BEFORE UPDATE` triggers (append-and-advance-once). - Every uncertainty (missing projection, concurrent checkpoint moving the prefix, empty prefix) → `ReconciliationRequired`, never authority — stricter than the V5 analogue in one arm (noted in-code). **Half 2 — broker-private composition** (`bp-authority-broker/src/plan_admission.rs`): - Injected seams (content resolver / ledger backend / fresh-snapshot verifier) per the `dispatch_admission.rs` template. **No transport, no socket, no role, no bin, no config loader** — Q1 staged; nothing production-side can reach the mint until S3. - Closed `deny_unknown_fields` request; re-derives `input_digest` (raw bytes it loaded), native `plan_id`, `trusted_base` (injected descriptor), `decided_by` (injected identity), `decided_at` (own clock). Caller-asserted `plan_digest`/`idempotency_key` recorded verbatim, documented non-authoritative (Q10/§4.3); requests without an asserted validation `PASS` are refused (Q10: advisory + required precondition), enforced at both composition and storage layers. - Outward dispositions: `Sealed` | `ReconciliationRequired`; nine individually-pinned refusal paths via a crate-private `admit_detailed`. **Consumer hardening (from the adversarial round):** `packages/kernel` admitted-plan reader now exposes `sealed` from the mint's projection (fail-closed on missing table/row/state/error) and the orchestrator dispatch gate requires it — enforcing the ratified Q12b ruling ("an unsealed admission authorizes nothing") at the only consumer. This lands the consumer half of the S4-scheduled gate one slice early because S2 itself opens the unsealed crash window (the §7-S1 front-loading rationale). **The blanket consequence — every pre-mint tape's `plan_admitted` reads as unsealed — is a deliberate fail-closed choice pending explicit operator confirmation before S3** (see "Operator gates" below). **Carried S1 obligations:** payload-variant guard generalized from `plan_admitted`-only to the whole always-blocked set; behavioral drift-guard test pinning the denylist membership; payload↔reader coverage rejoined via the round trip. ### Acceptance (§7 S2, all seven) 1. Kernel-signed `plan_admitted` on a real store + `sealed` projection row — `plan_admission_mint.rs` (14 tests). 2. `scripts/verify-signed-tape.mjs` **exits 0** over a tape exported from the store the mint wrote — **the first time PlanForge criterion 5 has ever been reachable**. `plan_admission_mint_round_trip.rs`; never touches the committed `plan-cycle` fixture (§5.6 gaming hazard).⚠️ **This is the repo's first Rust-test-shells-to-node pattern** — skips with an explicit message when `node` is absent; CI runs it with node present; mutation-verified that the node path really executes. 3. All §6.2(2) negatives, each its own test with a named disposition (+ the Q10 refusal). 4. §6.2(3) idempotent crash-recovery positive (injected seal failure → retry → identical sealed evidence, exactly one event). 5. §6.2(4) fresh-snapshot postcondition (fresh durable connection re-verifies the seal). 6. `planforge-plan-admission.test.ts` — **zero diff vs origin/main** (test 1 verbatim per NG3/Q9). 7. Whole-workspace `cargo test` (no `-p`): **1316 passed, 105 suites** (baseline 1277 + 39 new). Also: typecheck clean; `payload-variants.json` byte-identical (doc-comment-only regen); changesets: `@buildplane/ledger-client` patch, `@buildplane/kernel` patch. ### Disclosed deviations (full list in the ceremony record; the load-bearing ones) - **`plan_id` is not natively re-derivable** (contra spec §4.3): `preview.ts` fingerprints a parsed projection, not bytes. The mint derives a visibly-distinct `pf-plan-native-<32hex>` under its own domain separator. Verified harmless: the reader keys on tape event id. - **Mint `input_digest` ≠ PlanForge's scheme** (raw-bytes sha256 vs JSON-quoted-string sha256) — deliberate, per §4.2 "digest the bytes *it* loaded". - **Fresh-snapshot verifier** is a fresh-connection re-derivation through the storage verifier, NOT a `bp-replay` `TrustedGovernedRecoverySnapshot` (no plan-admission accessor exists) — weaker guarantee, stated in-code. - Single kernel signer (Q4) ⇒ no admission/checkpoint signer separation; the seal buys prefix verifiability only. - The M2 resume-rule conflict (spec §5.3) is documented, and now *enforced* on the TS consumer path, but `CLAUDE.md`'s M2 crash-recovery contract text itself is unchanged. ### Ceremony record (L0, 4-role) - **Implementer**: Opus, phased TDD; refusal branches and the round-trip node path mutation-verified. - **Independent reviewer** (fresh Opus): round 1 **HOLD** (HIGH: compatibility-matrix row falsified; MEDIUM: verifier vocabulary overclaim; 3 LOW) with five mutation probes (node-path panic, payload-guard arm deletion, Q10 neutralization all discriminate). Round 2 **PASS at `a9bbcb2`**, two further probes (gate neutralization, fail-open reader) discriminate; two new MEDIUMs resolved in the polish commit. - **Adversarial** (real Codex GPT-5.5 xhigh; sessions `01a02f24-4319-7f40-9f4e-ae44982322cc`, re-check `01a02f46-d67d-7ac2-840f-0823414b75f3`): round 1 **DEFEATED** — [HIGH][CONFIRMED] signed-but-unsealed admissions satisfied the TS dispatch gate; [MEDIUM][CONFIRMED] caller-asserted digests signed under canonical-sounding docs. Post-repair re-check: **finding 1 CLOSED [CONFIRMED]** (bypass hunt, SQL-forgery, and second-dispatch-path attacks all held); finding 2 payload/projection/broker surfaces fixed, two stale legacy doc surfaces then annotated in the polish commit. - **Acceptance verifier**: independent 12-row checklist (§7-S2 seven criteria + five ceremony riders) with re-executed gates, at head `3cd5629` — **OVERALL PASS, 12/12**. Full libtest output was re-derived by running the compiled test binaries directly to confirm the round-trip test executed the node path rather than skipping. ### Operator gates opened by this slice (do not silently pass) 1. **Before S3**: ratify (or amend) the pre-mint-tape consequence — every `plan_admitted` on a tape written before the mint reads as **unsealed** and confers no dispatch authority (fail-closed; currently unreachable). Wording at `admitted-plan-reader.ts:35-53`. 2. **Binding ordering constraint** (recorded in the compatibility matrix): the mint must not gain a transport-reachable consumer before S4 lands the load-and-verify + checkpoint-coverage arm (Q13). ### Known follow-ups (recorded, not silent) - S3: remove the module-wide `#[allow(dead_code)]` when the ingress wires the composition. - S3/S6: cross-language drift test binding the TS reader's hardcoded `plan_admissions` schema to the Rust DDL (back-reference comments exist on both sides). - `MAX_PLAN_ADMISSION_INPUT_BYTES` (1 MiB) is a chosen bound; revisit if real plans approach it. **L0 slice — draft PR, NOT auto-merge eligible. Operator admin-merge only.**
|
Gate round 3 — pre-S3 ratification (2026-08-24, operator-answered): The pre-mint-tape reclassification is ratified as coded (
This closes operator gate 2 from the S2 record. S3 (ingress wiring) is unblocked. Standing constraints unchanged: no transport-reachable consumer before S4; Q7 (production DispatchEnvelopeV5 mint) still open, gating S5; OperatorRequested embargo through S5. |
Summary
Design spec (no implementation) for the dedicated native mint control for
plan_admitted— the controlserve.rs's own comment prescribes ("a dedicated native control that replays and verifies the preceding evidence") and the designated start of all future PlanForge admission work after the 2026-08-15/16 quarantine decisions.Produced by an 8-agent investigation workflow (4 parallel evidence readers -> opus synthesis -> 3 adversarial verification lenses, all SOUND-WITH-FIXES) followed by an opus repair pass that re-verified ~50 citations against
mainand surfaced 4 additional findings the reviewers missed. Every claim is file:line-cited; unverifiable claims are marked UNVERIFIED inline.Highlights:
BrokerAuthorityRoleV1::PlanAdmission(the only boundary proven live end to end);decided_by/decided_atbecome derived facts, not caller strings.PreauthorizationRefbinding: signature verification (S4, partial discharge) vs binding direction (S5, deferred on Q7) — with theOperatorRequestedtightening embargo explicitly extended to S5.input_digest/plan_id/trusted_base; record-not-trustplan_digest/idempotency_key; validationPASSdemoted to advisory — operator gate Q10).record_plan_admission_v1/seal_plan_admission_v1SqliteStore API mirroring the V5 admission pair, with full crash/idempotency semantics and the M2 resume-contract conflict surfaced (Q12b).plan_admittedsurvives at all (Q3), authority-vs-recovery-evidence (Q12b), CAS staging (Q12c), and more.This PR decides nothing
It is a decision document for operator review — the design explicitly frames placement, tier-model change, authority semantics, and build-start as operator gates. Do not merge as authorization to build; merge (or amend) it as ratification of the questions.
Verification
Docs-only change; no changeset (per CLAUDE.md changeset policy); CI trivially green expected.