Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ This document contains rules and conventions for AI agents working on this codeb
6. **Every FSM `Registry.X.Get(...)` must have a matching preload, declared by the component that proposes the command** — The FSM apply path reads from the in-memory cache; a cache miss turns the read into a silent no-op. Each component that emits a proposal (the metadata converter, the index builder, the cluster-config reconciler, the idempotency-eviction scheduler, the mirror worker, admission) is responsible for declaring its own `preload.Needs` covering every key its apply path will read. There is NO central proposal→needs registry — coupling the preload package to every proposal type creates a single point that easily falls behind. The component knows what it reads; the component declares it. The shared `proposeTechnical` helper takes a `*preload.Needs` parameter the caller fills in (pass nil or an empty `Needs` when the apply path has no cache-keyed reads, e.g. cluster config / idempotency eviction). The preload populates the cache via `MirrorPreload` with a fresh value read at propose time (Pebble fallback on cache miss), and `PredictedIndex` catches mutations between propose and apply.
7. **Never silently skip a "should not happen" branch** — A branch that is reachable only if an invariant is violated (nil where the contract says non-nil, a state we believe unreachable, a cache miss after a guaranteed preload, etc.) MUST surface a loud signal: `return fmt.Errorf("invariant: ...")` so it bubbles up, or `assert.Unreachable(...)` for SUT-level invariants exercised under antithesis. A silent `return nil` / `continue` on these branches hides real bugs — particularly catastrophic in the FSM apply path, where a no-op desyncs nodes from each other. Branches that represent genuine runtime conditions (cache miss as an expected outcome, stale proposal, deleted entity) keep their soft `return nil`. The distinction is whether the case is *expected* (soft skip OK) or *impossible by design* (must fail loudly). The comment must say *why* the case is impossible so a reader can decide whether to add a hard fail or relax the rule.

8. **The audit log is the only source of truth — every other persisted dataset is a projection and must be verified by the checker** — Only `AuditEntry` (zone `Cold`, sub `Audit`) is cryptographically bound, via the hash chain that `state.BuildHashedHeaderPayload` + `processing.HashGenerator` produce and `checker.verifyAuditHashChain` verifies on every Check() run. Everything else stored in Pebble — `Log`, `AuditItem`, `AppliedProposal`, `LedgerLog.PurgedVolumes`, attribute caches (`Volume`, `Metadata`, `Transaction`, `Reference`, `Boundary`, etc.), reversion bitsets, idempotency keys, mirror cursors, chapters, bloom filters, signing keys, the read-side index — is a *projection* of orders that already live in the audit chain. Projections are rebuildable from the audit on demand, so we deliberately do NOT extend the hash chain to cover them (refactor over hash binding — see `feedback_audit_is_source_of_truth`). In exchange, **`internal/application/check/checker.go` MUST verify every projection it persists**: re-derive the value the projection should hold by replaying the audit (`ReplayLedgerLog`, `SimulateEphemeralPurge`, `partitionVolumes`, etc.) and compare to what is stored, emitting the matching `CHECK_STORE_ERROR_TYPE_*` event on divergence. A projection that the checker does not verify is a tampering vector — adding a new persisted projection without a matching compare* / collect* pass in the checker is the violation. The current passes are `compareVolumes`, `compareMetadata`, `compareTransactions`, `compareExclusionProjections` (AppliedProposal.TransientVolumes + LedgerLog.PurgedVolumes), `checkReversionInvariants`, `verifySealingHash`, `compareIdempotencyOutcomes` (frozen idempotency outcomes in SubIdempKeys vs the hash-chained AuditFailure/AuditSuccess that wrote them — the failure kind is re-derived from the chain-bound reason via `domain.KindForReason`, never stored), and `compareIndexes` (SubAttrIndex registry vs CreateIndex/DropIndex/RemovedMetadataFieldType/DeleteLedger logs — covers presence + identity; BuildStatus is intentionally excluded because it is purely informational on the cluster-wide registry entry — queries gate on the per-replica `IndexVersionState.CurrentVersion`, not on BuildStatus); extend the list as new persisted projections land.
8. **The checker verifies the authoritative store, not every derived projection** — Only `AuditEntry` (zone `Cold`, sub `Audit`) is cryptographically bound, via the hash chain that `state.BuildHashedHeaderPayload` + `processing.HashGenerator` produce and `checker.verifyAuditHashChain` verifies on every Check() run. The checker must be able to validate the main persisted ledger state against the audit-derived state: when data is part of that authoritative store, or can affect correctness independently of the store it is derived from, `internal/application/check/checker.go` must re-derive the expected value from the audit chain or another already-verified source and compare it to what is stored, emitting the matching `CHECK_STORE_ERROR_TYPE_*` event on divergence. Persisted projections derived from the verified store — denormalized counters, rebuildable read models, informational build status, caches, and other refreshable datasets — do not each require their own compare pass merely because they are persisted. A replication/ingestion cursor is *technical* state, not a ledger business invariant: `MirrorCursor` (main store) is only the mirror worker's progress pointer into an external v2 source, and it is written **in the same atomic Raft-applied Pebble batch as the `MirrorIngest` orders it acknowledges** — the worker bundles the `MirrorSync` TU into the data proposal (`mirror/worker.go`), and `applyProposal` drains that proposal's orders + TUs through one `WriteSet.Merge` and one `batch.Commit()` (`machine.go`). A crash lands both or neither, so the persisted cursor can **never lag the ingested data** through any normal or crash path: the "cursor behind → re-emit already-applied logs" replay is *unreachable operationally* (postings can't be replayed without the cursor having advanced with them). The business effect of mirroring — the ingested transactions — is itself audit-bound (each `MirrorIngest` is an audit-chain order) and already checker-verified (`recordMirrorIngestMutations` feeding `compareVolumes`/`compareTransactions`), so the ledger's correctness holds regardless of the cursor. The only way to move the cursor off that value is to tamper the persisted cursor byte directly, and neither direction is a checker integrity concern. A cursor tampered **behind** self-heals and is checker-invisible: on the next tick the worker re-ingests, the cursor **re-advances atomically** to a value consistent with the (now-duplicated) audit, and the doubled volumes are themselves consistent with an audit that now legitimately contains the duplicate `MirrorIngest` orders — so a cursor compare and `compareVolumes`/`compareTransactions` (which replay the *same* audit the FSM applied) all pass; the real mitigation is FSM-level `v2LogId` idempotency in `processMirrorIngest` (skip an ingest whose `v2LogId` is already applied), a functional hardening tracked separately, not a checker pass. A cursor tampered **ahead** is stable and would make the worker silently skip source logs (v2→v3 completeness loss) — but that is a *parity* property against the external v2 source, verifiable only against the worker's tracked source-head (`SubPLMirrorSourceHead`), not against the audit the checker replays; its home is the mirror worker's recovery reconciliation, not an integrity compare pass. Classify such replication/sync cursors as technical state; the checker guards the *business* invariants of the main store (balances, transactions, metadata, reversions, idempotency, index registry), which are audit-derived and independent of these cursors. The rule for new persisted data is to classify it explicitly by **which store holds it**. Data in the *primary FSM Pebble store* — the single store `Check()` opens and walks — is in scope: if it is authoritative, security-sensitive, or can affect ledger business correctness independently of the store it is derived from, add a compare* / collect* pass or an equivalent recovery-side validation; a primary-store projection may be exempted as derived only when it is deterministically rebuildable from still-retained verified state via a real, wired rebuild path, not merely rebuildable in principle. Data in a **distinct peer secondary Pebble store** (the `usagestore` counter cache, the `readstore` inverted index) is out of the checker's scope *by construction* — `Check()` never opens those stores. Their integrity contract is different and already sound: they are pure derived caches of the audit chain, and the audit chain itself is the cryptographically-verified source of truth, so corruption of a peer store is a *rebuild* event (drop the store, replay from the verified audit), never an undetected ledger-integrity violation. Serving a derived value from a peer store through an API (`GetLedgerStats`, `GetTemplateUsage`, list queries off the readstore) does **not** promote it to authoritative primary-store state or pull it into checker scope — API-visibility is not authoritativeness. Automating the drop-and-rebuild lifecycle for a peer store that today rebuilds only via an operator-triggered `Reset()` (e.g. `usagestore`) is tracked as a follow-up PR: that is *operational* hardening, not a gap in checker coverage. The current checker passes include `compareVolumes`, `compareMetadata`, `compareTransactions`, `compareExclusionProjections` (AppliedProposal.TransientVolumes + LedgerLog.PurgedVolumes), `checkReversionInvariants`, `verifySealingHash`, `compareIdempotencyOutcomes` (frozen idempotency outcomes in SubIdempKeys vs the hash-chained AuditFailure/AuditSuccess that wrote them — the failure kind is re-derived from the chain-bound reason via `domain.KindForReason`, never stored), and `compareIndexes` (SubAttrIndex registry vs CreateIndex/DropIndex/RemovedMetadataFieldType/DeleteLedger logs — covers presence + identity; BuildStatus is intentionally excluded because it is purely informational on the cluster-wide registry entry — queries gate on the per-replica `IndexVersionState.CurrentVersion`, not on BuildStatus); extend the checker when new authoritative persisted state lands.
Comment thread
flemzord marked this conversation as resolved.
Outdated
Comment thread
NumaryBot marked this conversation as resolved.
Outdated
Comment thread
NumaryBot marked this conversation as resolved.
Outdated

9. **Never bypass the FSM coverage gate** — Every cache-attribute read on the FSM hot path MUST go through `Scope.GetX(...)` so the per-order `coverage_bits` admit it. Reading the underlying `Registry.X.KeyStore().M` (or any other parent-cache iterator) directly skips the gate and produces non-deterministic FSM behavior: the gate is what binds the order to the admission-declared preload set, and a direct read silently sees keys the proposer never declared. There is NO documented exception — paths that need to iterate (e.g. cascade-on-delete) MUST either declare the relevant `preload.Needs` upfront, defer the work to a lifecycle path (`batch.deleteLedgerData` + `MarkLedgerForCleanup`), or be rejected at design review. New helpers that scan the parent KeyStore from inside an order/TU handler are the violation, even when wrapped in a method on `WriteSet`. The coverage gate exists precisely so admission's declared key set is the FSM's only legitimate read horizon — under no circumstances should the apply path widen it on the fly.

Expand Down
Loading
Loading