feat(ledger): close the in-process plan_admitted append hole (mint-control S1) - #314
Merged
Merged
Conversation
`plan_admitted` was refused on the signed generic-ingest lane by `reject_caller_supplied_authority_event`, but the unsigned lane cleared that guard (the kind is signed-only on the wire) and landed the event through `store.append`. Any pipe-controlled caller could therefore persist a PlanForge admission no control had ever authorized. `EventKind::PlanAdmitted` now joins `validate_external_append`'s always-blocked set, so every public append entry point — `append`, `append_signed`, `append_signed_with_checkpoint` — fails closed with `CallerSuppliedTrustSpineEvent`, which the serve loop reports as `storage_failure`. The two lanes keep two distinct typed errors on two distinct layers and both are now pinned. This is a tier-model change, deliberately: the always-blocked set previously held only wire-tier-always-rejected kinds, and `plan_admitted` is the first signed-only-tier kind in it. Its wire tier does not move, so the `serve.rs` disposition table and its three arrays are untouched. The asymmetry is accepted so the dedicated native mint control's exclusivity claim is never false for the window between closing the hole and building the mint — the same ordering `#281` (`a53519b`) used for `governed_dispatch_v5_admission_recorded_v1`. Consequences landing here: - `planforge-plan-admission.test.ts` test 2 is rewritten from "lands unsigned" to "rejected on the unsigned lane too", with the file header updated. Test 1 is byte-unmodified. The rewrite gives up one property the old test held end to end — that the port's emitted payload satisfies the kernel's exact-8-key reader — because no lane can persist that event any more; the two halves stay pinned separately and the header records the loss. - `bp-replay`'s `planforge_cycle` fixture reconstructs a historical tape and so moves onto a new `cfg(test)`/`test-support` gated `insert_event_bypassing_external_validation_for_tests`, mirroring `seal_governed_signed_prefix_for_tests`. Its replay assertions are unchanged. - The rationale comment in `caller-supplied-trust-spine-kinds-sync.test.ts` justified the client-side asymmetry as protecting "the unsigned lane the native side deliberately keeps open". That is no longer true for `plan_admitted`; the comment now states the real reason the guard cannot widen. Its assertions are unchanged.
…ty matrix The matrix had no `plan_admitted` row at all, so an operator reading the compatibility contract could not tell that the kind has no production writer and that neither generic-ingest lane can create one. The new row in "Packets, envelopes, and tape data" states the read/replay position, names both refusals and the code each is reported under, records the quarantined port, points at the mint-control design that must exist before any write path returns, and discloses the one test-only insert helper that can still reconstruct a historical tape.
`validate_external_append` blocked by `event.kind` alone, so an
`Event { kind: ModelRequest, payload: PlanAdmittedV1(..) }` cleared both
the always-blocked set and the canonicalize probe and landed with a
mismatched kind column. `bp-replay`'s `apply_with_verified_signer`
dispatches on the payload variant and never reads `event.kind`, so that
row would replay as a genuine plan admission.
Not reachable from production — both writers (the serve loop's
`Line::Event` handler and `ingest`) canonicalize first, which enforces
`validate_kind_matches_payload` — but S1's exclusivity claim must not
rest on an invariant only well-behaved callers honour. The guard now
refuses a `Payload::PlanAdmittedV1` whatever kind its envelope declares,
naming `plan_admitted` as the refused record.
Deliberately scoped to this one payload. Generalizing to every
trust-spine payload, or enforcing kind/payload agreement in this guard
outright, is wider than closing the hole and is left to the mint slice.
Also hardens the test-support writer: `record_ordinary_append` guards
`TapeCheckpoint` with a `debug_assert!`, which vanishes under
`--release`, so `insert_event_bypassing_external_validation_for_tests`
now asserts it in every profile rather than silently poisoning the
ordinary high-water mark.
Comment-only. Three places still described `plan_admitted` as refused
only when the append is signed, which stopped being true once
`validate_external_append` began always blocking it:
- `serve.rs`'s tier doc claimed the unsigned lane "keeps legacy
compatibility" for every second-list kind, and the panic message in
`kinds_on_the_second_denylist_still_pass_the_unsigned_lane` read as if
clearing the guard meant reaching the tape. Both now adopt the idiom
already used for `TapeCheckpoint` — cleared here, refused downstream —
and the `PlanAdmitted` entry says so at the point of membership. No
array membership, disposition-table entry, or assertion changes: the
wire tier does not move.
- `emitter.ts` described the second list without noting that clearing it
is not acceptance, so a reader could conclude an unsigned
`plan_admitted` emit succeeds.
- `plan-admission-port.ts` described the superseded regression pin ("the
unsigned lane can never produce a verifiable tape"); it now describes
what the rewritten test asserts — two distinct native rejections, one
per lane, each with a fail-closed empty tape.
…ayload check
`validate_external_append` refuses a plan admission twice — once on the
envelope kind, once on the payload variant — and both clauses raise a
byte-identical `CallerSuppliedTrustSpineEvent { kind: "plan_admitted" }`.
Every existing test paired kind=PlanAdmitted with payload=PlanAdmittedV1,
so the payload clause alone satisfied all of them and the kind clause
could have been deleted with the suite still green.
The kind clause is not redundant. It catches the mirror shape: a
`plan_admitted` envelope carrying some other payload. That row is
reachable through direct in-process `store.append`, which does not
canonicalize and so never enforces kind/payload agreement, and it is not
inert once written — the kernel's admitted-plan reader selects rows by
`kind = 'plan_admitted'`, which is exactly what a label-only row
presents.
Adds the mirror regression test. Verified by mutation: with the
`EventKind::PlanAdmitted` arm removed from the kind clause, this new test
is the only one of the nine that fails (`got Ok(())`), and restoring the
arm returns the suite to green — so each clause now has a test that fails
if it alone is deleted. A comment on the pair records why both must stay.
Also scopes the stale claim in `emitter.ts` in place rather than only
correcting it in a later paragraph: the second native denylist blocks
those kinds when signed *within that guard*, which is not a statement
about whether the ledger accepts them.
Contributor
Merge Protections🔴 2 of 2 protections blocking · waiting on 🙋 you
🔴 require green CI on mainWaiting for
This rule is failing.
🔴 🚦 Auto-queueWaiting for
This rule is failing.When all merge protections are satisfied and these conditions match, this pull request will be queued automatically.
|
There was a problem hiding this comment.
Pull request overview
Closes an in-process append hole where plan_admitted could still be persisted via the unsigned generic-ingest lane, by adding a storage-layer block (plus a payload-variant “smuggling” guard) so neither generic-ingest lane can write plan_admitted before the dedicated native mint control exists.
Changes:
- Block
EventKind::PlanAdmittedinSqliteStore::validate_external_append, and also refusePayload::PlanAdmittedV1regardless of the envelope kind to prevent payload smuggling. - Add/adjust Rust + TS integration tests to pin the two distinct rejections (signed wire-guard vs unsigned storage-guard) and the “empty tape” fail-closed property.
- Introduce a test-only insert helper (gated behind
cfg(test)/test-support) to keep historical replay fixtures viable, and update docs/comments to reflect the tier + guard semantics.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/ledger-integration/planforge-plan-admission.test.ts | Updates the end-to-end integration pin to assert unsigned-lane rejection + empty tape (not “lands unsigned but unverifiable”). |
| packages/ledger-client/test/caller-supplied-trust-spine-kinds-sync.test.ts | Clarifies the meaning of the client-side denylist vs native lane behavior now that plan_admitted is storage-blocked too. |
| packages/ledger-client/src/emitter.ts | Tightens documentation explaining why the client cannot mirror the signed-only denylist and why “passes the guard” ≠ “accepted by ledger”. |
| native/crates/bp-replay/tests/planforge_cycle.rs | Switches the fixture writer to a gated test-only insert helper to reconstruct historical tapes containing plan_admitted. |
| native/crates/bp-replay/Cargo.toml | Adds bp-ledger dev-dependency with test-support feature for the replay fixture helper. |
| native/crates/bp-ledger/tests/plan_lifecycle.rs | Adds regression pins for both generic-ingest lane rejections and the payload-smuggling scenario. |
| native/crates/bp-ledger/src/storage/sqlite.rs | Adds the storage-layer always-block for plan_admitted and the payload-variant refusal; introduces the gated test-only insert helper. |
| native/crates/bp-ledger/src/serve.rs | Clarifies comments/tests about what the wire-guard classification means vs downstream storage acceptance. |
| docs/operations/trust-spine-compatibility-matrix.md | Adds a compatibility-matrix row documenting the no-writer/no-lane status for plan_admitted and where it is blocked. |
| apps/cli/src/plan-admission-port.ts | Updates port documentation to reflect that both generic-ingest lanes refuse plan_admitted (with distinct rejection sources). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
SollanSystems
added a commit
that referenced
this pull request
Aug 24, 2026
…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.**
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
S1 — close the in-process
plan_admittedappend holeFirst 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::PlanAdmittedjoinsvalidate_external_append's always-blocked set (bp-ledger/src/storage/sqlite.rs) — the unsigned generic-ingest lane now rejects it (CallerSuppliedTrustSpineEvent, surfaced asstorage_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).Payload::PlanAdmittedV1payload is refused whateverkindthe envelope declares. Without it,kind: model_request+payload: PlanAdmittedV1sailed through the kind-only check via direct in-processstore.append(which does not canonicalize), andbp-replaydispatches 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).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.rsrepointed onto a new#[cfg(any(test, feature = "test-support"))]insert helper (insert_event_bypassing_external_validation_for_tests) — the only test that appendedplan_admittedthrough the public API. Replay assertions unchanged. The helper hard-asserts againstTapeCheckpointin every profile.serve.rsis one panic-string):serve.rstier doc +kinds_on_the_second_denylist_still_pass_the_unsigned_lanedoc/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 —PlanAdmittedstaysREJECTED_ONLY_WHEN_SIGNED; array membership, disposition table, and all assertion logic byte-identical.plan_admittedrow added todocs/operations/trust-spine-compatibility-matrix.md.Between S1 and S2
plan_admittedhas zero writers outside test support — deliberate; the kind has no production writer today either (standing disclosure #5).Ceremony record (L0 — full 4-role)
b29712e; repair deltaeb8f78b; final29f905a)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)
cargo test --manifest-path native/Cargo.toml(no-p): 1277 passed, 102 suites (independently re-run by the verifier at each round)pnpm typecheckclean;cargo fmt --checkclean on touched files; biome clean on touched TS (isolated-dir probe)src/changes are comment-only; precedent test(cli): budget the cli smoke suite for parallel-suite load #301 (test-only → none)Carried obligations (recorded, not this slice)
GovernedDispatchV5AdmissionRecordedV1,PromotionReconciliationResolved) — same smuggling shape, same non-exploitability today; S2.createDefaultAdmittedPlanReader; rejoined by S2's mint round trip (§6.2(1)).node:sqliteINSERT inadmitted-plan-reader.test.ts) remains open — test-only, spec-sanctioned out of scope (§6.1).L0: not auto-merge eligible — operator admin-merge required. Opened as DRAFT.