From 2e11be5cbc0f91d69aaeb09eb49ffdc8b9175895 Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Mon, 17 Aug 2026 07:01:33 +0200 Subject: [PATCH 01/15] docs(V2-04): fix ADR-071 status contradiction, requalify CB-T4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-071's status line read as "Accepted" without distinguishing architecture ratification from implementation readiness, contradicting its own "Gates de ratification" section (EmergencyHeadroom unproven, test suite incomplete). Reworded to "Accepted as architecture" and made the gap explicit inline, so the status line can't be read as "ready to activate." CB-T4 ("maybe_flush tolerates AdmissionWouldBlock") is corrected: the earlier review cited ADR-072's coordinator.rs TODO as evidence it was an unimplemented mechanism, but that TODO is actually about CB-04's release- vs-retain decision, not maybe_flush. The real situation is that the implemented §B mechanism (reserve_blocking) can never structurally produce AdmissionWouldBlock by its own documented contract — blocking + a proven EmergencyHeadroom bound is the deliberate anti-deadlock design, not a gap. CB-T4 as worded actually belongs to the separate, unscoped phase-2 work of wiring MG-17 directly into store/engine/write.rs. Requalified as a phase-2 dependency and removed from §B's own pre-production-activation list across both ADRs and docs/status.md. Co-Authored-By: Claude Sonnet 5 --- ...lobal-admission-accounting-and-governor.md | 20 ++++++- ...mission-boundary-and-memtable-residency.md | 55 ++++++++++++++----- docs/status.md | 15 +++-- 3 files changed, 70 insertions(+), 20 deletions(-) diff --git a/docs/adr/ADR-071-global-admission-accounting-and-governor.md b/docs/adr/ADR-071-global-admission-accounting-and-governor.md index c442736..f3e1c13 100644 --- a/docs/adr/ADR-071-global-admission-accounting-and-governor.md +++ b/docs/adr/ADR-071-global-admission-accounting-and-governor.md @@ -2,7 +2,25 @@ **Identifiant : ADR-071** **Titre canonique :** _Governor mémoire global : réservations, admission bidimensionnelle, fairness et working-set gouverné_ -**Statut :** 🟢 **Accepted (rev 2)** — ratifié après revue croisée (concurrence, comptabilité mémoire, writer/recovery, red-team) confrontant le texte au code réel de `basemyai-engine` ; corrections de rev 2 listées en fin de document. **Amended by [ADR-072](ADR-072-cross-crate-admission-boundary-and-memtable-residency.md)** (2026-08-16) sur le placement de l'autorité d'admission — §"où vit le governor, non tranché" (ci-dessous) est tranché par ADR-072 en faveur de `basemyai::WriteCoordinator::submit`, ni `Engine` ni `WriterRuntime`, après audit du code réel de `basemyai` (absent du périmètre d'audit initial de ce document). Le reste de ce document reste en vigueur tel quel. +**Statut :** 🟢 **Accepted as architecture (rev 2)** — le texte normatif +lui-même (invariants `MG-*`, règles de verrouillage `GV-*`, algèbre DRR, +contrats RAII) est ratifié après revue croisée (concurrence, comptabilité +mémoire, writer/recovery, red-team) confrontant le texte au code réel de +`basemyai-engine` ; corrections de rev 2 listées en fin de document. +**Ce statut ne couvre pas l'implémentation.** §"Gates de ratification" +plus bas liste les conditions qui doivent être *simultanément* vraies avant +que le governor lui-même soit ratifié au sens opérationnel — activable, +testé, prêt à gouverner un budget réel. À la date de la dernière revue +(2026-08-17), elles ne le sont pas : `EmergencyHeadroom` reste **NON +PROUVÉ**, le câblage direct de MG-17 dans `store/engine/write.rs` +(chantier phase 2 distinct du mécanisme §B déjà implémenté par ADR-072, +qui n'en dépend pas) n'est pas scopé, et une partie substantielle de la +suite `GOV-U*`/`GOV-S*`/`GOV-RSS*` ci-dessous n'est pas encore écrite (voir +`docs/status.md`, entrée du 2026-08-17, pour le détail actualisé). Lire « Accepted » comme +« l'architecture est la bonne, construisez dessus », pas comme « le +governor est prêt à gouverner » — les deux affirmations ont été confondues +dans une version antérieure de cette ligne de statut, ce que cette +reformulation corrige explicitement. **Amended by [ADR-072](ADR-072-cross-crate-admission-boundary-and-memtable-residency.md)** (2026-08-16) sur le placement de l'autorité d'admission — §"où vit le governor, non tranché" (ci-dessous) est tranché par ADR-072 en faveur de `basemyai::WriteCoordinator::submit`, ni `Engine` ni `WriterRuntime`, après audit du code réel de `basemyai` (absent du périmètre d'audit initial de ce document). Le reste de ce document reste en vigueur tel quel. **Jalon :** V2-04 **Date de recherche et de rédaction :** 2026-08-15 **Formats persistants :** aucun changement ; `format.lock` doit rester strictement inchangé. diff --git a/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md b/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md index 983f738..226b724 100644 --- a/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md +++ b/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md @@ -420,20 +420,43 @@ ont cependant été trouvés et le premier corrigé le jour même : Une analyse de couverture séparée confirme que `CB-T1a` (traversée du canal), `CB-T1c` (panic handler) et `CB-T3` (ordre rotation-avant-écriture) -sont couverts, mais relève un écart plus grave que les autres pour -**`CB-T4` (« `maybe_flush` tolère `AdmissionWouldBlock` »)** : ce n'est pas -seulement un test manquant, c'est un mécanisme **non implémenté** — -`AdmissionWouldBlock` n'apparaît nulle part dans -`store/engine/write.rs`, confirmé par recherche exhaustive ; le -`TODO(ADR-072 §B)` déjà présent dans `coordinator.rs` marque précisément ce -manque. Tant que ce n'est pas câblé, l'histoire « le flush est la soupape -d'un domaine saturé » de §B n'a pas de chemin de code, indépendamment de -tout test. `CB-T1d` (4 des 5 branches de `CoordinatorOutcome` sans +sont couverts. `CB-T1d` (4 des 5 branches de `CoordinatorOutcome` sans couverture gouvernée), `CB-T2` (ordre gouverneur-avant-sémaphore sous contention réelle, pas seulement un refus immédiat) et `CB-T5` (borne de la -fenêtre CB-07 jamais reproduite) restent également ouverts et sont, comme -`CB-T4`, requis avant activation production — pas seulement avant -ratification de ce document. +fenêtre CB-07 jamais reproduite) restent ouverts et sont requis avant +activation production — pas seulement avant ratification de ce document. + +**Correction (2026-08-17) — `CB-T4` ne décrit pas le mécanisme réellement +construit et est requalifié ci-dessous.** Une première lecture de cette +revue avait classé `CB-T4` (« `maybe_flush` tolère `AdmissionWouldBlock` ») +comme un mécanisme manquant, en citant le `TODO(ADR-072 §B)` de +`coordinator.rs` comme preuve. Vérification directe : ce `TODO` concerne en +réalité la décision release-vs-retain de `release_governor_permit` (CB-04), +pas `maybe_flush` — mauvaise référence croisée, pas un constat correct. La +vraie situation est plus fondamentale : la réservation amont de rotation +implémentée par §B (`reserve_blocking`, `attach_memtable_charge`) **ne peut +structurellement jamais produire `AdmissionWouldBlock`** — le contrat de +`reserve_blocking` lui-même l'exclut explicitement (« `AdmissionWouldBlock` +n'est jamais retourné — attendre au lieu de le retourner est le point », +`memory_governor/admission.rs`). C'est un choix de conception assumé, pas +un oubli : si le domaine sature au moment de la réservation, le write +bloque jusqu'à libération de capacité (généralement par un flush), et +`EmergencyHeadroom` est précisément la garantie anti-interblocage de ce +choix (CB-08). Câbler une tolérance `AdmissionWouldBlock` dans `maybe_flush` +irait donc à l'encontre du mécanisme §B tel que ratifié. + +Ce que `CB-T4` décrit réellement appartient à un chantier distinct et non +scopé : le câblage direct de MG-17 dans `store/engine/write.rs` lui-même +(ADR-071 §"Correction (audit code, 2026-08-15) — le writer réel n'est pas +`writer_runtime.rs`" : réserver le pic *avant* le fsync WAL, « un +changement structurel du chemin d'écriture », explicitement listé comme +phase 2 dans `memory_governor/mod.rs`). Ce chantier n'a pas encore de +conception — décider s'il utilise une réservation bloquante ou non +bloquante, et donc si une tolérance `AdmissionWouldBlock` a un sens, en +fait partie. **`CB-T4` est donc requalifié comme dépendance phase 2 +(câblage `write.rs`/MG-17), retiré de la liste "requis avant activation +production" du mécanisme §B déjà implémenté**, qui n'en a structurellement +pas besoin. --- @@ -448,9 +471,11 @@ ratification de ce document. - Réservation amont de rotation : aucune écriture n'atteint la nouvelle génération avant que la réservation ait été acquise (exploite la sérialisation d'`owner_loop`, pas un mock de verrou). -- `maybe_flush` tolère `AdmissionWouldBlock` sans démouvoir un write - déjà durable (miroir de la tolérance existante à `BackgroundFlush`/ - `WriterReconcileRequired`, `write.rs:301-319`). +- ~~`maybe_flush` tolère `AdmissionWouldBlock`~~ — **requalifié + (2026-08-17), voir §"Post-ratification hardening" ci-dessus.** Le + mécanisme §B implémenté (`reserve_blocking`) ne produit structurellement + jamais cette erreur ; ce test appartient au chantier distinct, non + scopé, du câblage MG-17 dans `store/engine/write.rs` (phase 2). - Test de dérogation CB-07 : reproduire délibérément la fenêtre `Engine::get`-concurrent-au-flush et prouver qu'elle reste bornée à la durée d'un seul appel `get()`, jamais latente. diff --git a/docs/status.md b/docs/status.md index 172c48a..8af3a77 100644 --- a/docs/status.md +++ b/docs/status.md @@ -28,10 +28,17 @@ passé le code committé ci-dessous au crible pour la première fois. `flush.rs` — tel qu'écrit, ré-audité ligne à ligne contre le code réel. - **Dette identifiée, non corrigée (trackée dans les deux ADR, requise avant activation production)** : - - `CB-T4` — `store/engine/write.rs` ne gère nulle part - `AdmissionWouldBlock` : ce n'est pas un test manquant, c'est un - mécanisme absent. Le `TODO(ADR-072 §B)` déjà présent dans - `coordinator.rs` marque le manque. + - **`CB-T4` requalifié (2026-08-17)** — la citation initiale du + `TODO(ADR-072 §B)` de `coordinator.rs` comme preuve était une mauvaise + référence croisée (ce TODO concerne CB-04, pas `maybe_flush`). Le + mécanisme §B implémenté (`reserve_blocking`) ne peut structurellement + jamais produire `AdmissionWouldBlock` — c'est un choix de conception + assumé (bloquer + `EmergencyHeadroom` comme garde anti-interblocage), + pas un oubli. `CB-T4` décrit en réalité le câblage direct de MG-17 dans + `store/engine/write.rs`, un chantier phase 2 distinct et non scopé — + retiré de la liste "requis avant activation production" du mécanisme + §B, qui n'en dépend pas. Voir ADR-072 §"Post-ratification hardening" + pour le détail. - `GOV-U21-U23, U25-U26, U40-U41, GOV-S03` — propriétés pures du governor (fairness bytes/slots, spurious/lost wake, domaines partagés/indépendants, shutdown à l'échelle) sans aucun test, alors From cb9303e170ffba44464e862f0a108225f5b1e21c Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Mon, 17 Aug 2026 08:03:25 +0200 Subject: [PATCH 02/15] chore: ignore local OpenCode/Obsidian artifacts Co-Authored-By: Claude Sonnet 5 --- .gitignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index fb0d9ef..b01e019 100644 --- a/.gitignore +++ b/.gitignore @@ -103,3 +103,8 @@ $RECYCLE.BIN/ *.iml .fleet/ .anthropic/ +/obsd\BaseMyAI +obsd\BaseMyAI\.obsidian +obsd\BaseMyAI\BaseMyAI +obsd +opencode.jsonc From 9d46ddac8cbebf0f45f133ffa877904d3d308658 Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Mon, 17 Aug 2026 08:03:44 +0200 Subject: [PATCH 03/15] feat(V2-04): derive and test EmergencyHeadroom's byte formula ADR-071 left EmergencyHeadroom's derivation UNSPECIFIED; this closes the analytical half. Read store::engine::flush::run_job and the SstWriter it drives end to end to find every allocation a flush makes beyond the memtable it retires: - Dominant term (exact): Memtable::iter_versions deep-clones every key and value into a fresh Vec before the writer is called; that Vec's IntoIter keeps the whole allocation live for the entire write_new_versioned call, so the memtable being retired and this clone coexist for the whole flush. Reuses estimate_memtable_reservation_bytes verbatim (same per-entry shape) rather than re-deriving it. - SstWriter's own term, proved O(block_size) not O(memtable size): this is the writer's own documented R6.2/ADR-049 SS3 design goal, not an approximation. Every sub-term calls the same capacity-limit functions SstWriter::create itself calls, so the estimate can't drift from the real encoder. Grounded against a real SstWriter run (sst_writer_term_bounds_a_real_writers_steady_state_across_many_blocks_and_leaves, store/sst_block/write.rs), not just self-consistent arithmetic: an early version of the formula multiplied a Vec capacity limit (sized for worst-case many-small-entries) by a max-key-sized entry cost instead of using the real byte cap (index_chunk_plaintext_limit) separately, producing a 4.67-trillion-byte term. The grounding test caught it immediately, before any commit; fixed to use the two independently-bounded quantities the real preflight_leaf/push_internal_child checks actually enforce. This is the analytical half only. Nothing in basemyai-engine consumes a governed reservation for a flush yet (store::engine::flush doesn't touch memory_governor), so the operational proof (GOV-U32: a real flush completing under a saturated domain) still depends on the unscoped phase-2 MG-17/write.rs wiring - same dependency already identified for CB-T4. Both ADRs and docs/status.md are updated to reflect exactly this split; the production-activation gate stays closed. Also cleans up ~13GB of orphaned .claude/worktrees/agent-* directories (pointing at a since-deleted repo path) and 16 dead local branches with no live worktree, all fully merged into dev. cargo xtask check green; basemyai-engine 610/610 lib tests; cargo xtask test-crash-consistency re-verified 13/13, 539.25s. Co-Authored-By: Claude Sonnet 5 --- .../src/memory_governor/mod.rs | 15 +- .../src/memory_governor/types.rs | 272 +++++++++++++++++- .../src/store/sst_block/write.rs | 61 ++++ ...lobal-admission-accounting-and-governor.md | 15 +- ...mission-boundary-and-memtable-residency.md | 31 +- docs/status.md | 62 ++++ 6 files changed, 446 insertions(+), 10 deletions(-) diff --git a/crates/basemyai-engine/src/memory_governor/mod.rs b/crates/basemyai-engine/src/memory_governor/mod.rs index 8b80697..a5e9c9e 100644 --- a/crates/basemyai-engine/src/memory_governor/mod.rs +++ b/crates/basemyai-engine/src/memory_governor/mod.rs @@ -71,7 +71,20 @@ pub use admission::{AdmissionPermit, MemoryGovernor}; pub use config::GovernorConfig; pub use governed::{Governed, SharedGoverned}; pub use reservation::MemoryReservation; -pub use types::{AdmissionClass, GovernedMemoryKind, MemoryPlan, estimate_memtable_reservation_bytes}; +pub use types::{ + AdmissionClass, GovernedMemoryKind, MemoryPlan, estimate_emergency_flush_headroom_bytes, + estimate_memtable_reservation_bytes, +}; +// Crate-internal, test-only: lets `store::sst_block::write`'s `#[cfg(test)]` +// tests ground the SstWriter-only half of the EmergencyHeadroom derivation +// against a real writer run (`store::sst_block` is a private module, a +// sibling of this one under the crate root, so it cannot reach `types` +// directly). Gated by `cfg(test)` only, matching that consumer exactly — the +// function itself is used unconditionally by +// `estimate_emergency_flush_headroom_bytes` within `types.rs`, so only this +// cross-module re-export needs gating at all. +#[cfg(test)] +pub(crate) use types::sst_writer_working_set_bytes; use scheduler::SchedulerQuanta; use state::GovernorState; diff --git a/crates/basemyai-engine/src/memory_governor/types.rs b/crates/basemyai-engine/src/memory_governor/types.rs index 1416f8e..e281441 100644 --- a/crates/basemyai-engine/src/memory_governor/types.rs +++ b/crates/basemyai-engine/src/memory_governor/types.rs @@ -3,7 +3,7 @@ //! (ADR-071 §"Catégories runtime" / §"Classes d'ownership" / API table). use crate::error::{EngineError, Result}; -use crate::format::{crypto, wal}; +use crate::format::{crypto, sst_block, wal}; use crate::store::INTERNAL_KEY_SUFFIX_BYTES; /// Observability/policy axis a [`crate::memory_governor::MemoryReservation`] @@ -368,6 +368,173 @@ pub fn estimate_memtable_reservation_bytes( mul(memtable_flush_threshold, per_entry_bytes) } +/// Worst-case working set one flush needs *beyond* the memtable generation +/// it is retiring (ADR-071 §"Headroom de progrès background"; ADR-072 +/// `CB-08`, which elevates this from a flush-progress margin to the +/// non-permanent-blocking condition of the §B rotation mechanism). ADR-071 +/// leaves the number `UNSPECIFIED`; this is its derivation. +/// +/// # Method: read `store::engine::flush::run_job` and the `SstWriter` it +/// drives end to end, not approximate +/// +/// Two terms, and — per the analysis below — only two, because everything +/// else in the flush path is either already-charged (the memtable itself) +/// or bounded independent of both by construction. +/// +/// **1. The dominant term: one more full copy of the memtable's content.** +/// `Memtable::iter_versions` (`store::memtable`) deep-clones every key and +/// value into a fresh `Vec` before `run_job` ever calls the writer; that +/// `Vec` is then moved into `SstWriter::write_new_versioned` and consumed by +/// `for (key, value) in entries` — but `Vec::IntoIter` keeps its *whole* +/// backing allocation live until the loop finishes, not just the unconsumed +/// tail. For the entire flush, the memtable being retired (already charged +/// under `WriteBufferMutable`) and this clone (today entirely uncounted by +/// the governor) coexist. Same per-entry shape, same worst-case entry count +/// as [`estimate_memtable_reservation_bytes`] — reused verbatim rather than +/// re-derived, so the two can never drift apart. +/// +/// **2. `SstWriter`'s own working set: `O(block_size)`, not `O(memtable +/// size)`.** Not an approximation — the writer's own documented design goal +/// (`format::sst_block::write`'s module doc: "the largest live allocation is +/// one data block... the writer contributes `O(block_size)` regardless of +/// how much is merged through it", the R6.2/ADR-049 §3 rewrite) and ADR-053's +/// discipline ("one fixed Bloom partition, one leaf builder, at most one +/// internal-node builder per index level"). Every sub-term below calls the +/// *exact* capacity-limit functions `SstWriter::create` itself calls, so this +/// estimate cannot silently drift from the real encoder: +/// +/// - staged data block: the `current: Vec` accumulator (struct +/// slots up to `vec_capacity_bound(max_data_block_entries(block_size))`, +/// the same rounding `SstWriter` itself reserves) plus its wire content +/// (bounded by `block_size`, the flush trigger) plus the re-encoded +/// `plain` buffer `stage_current_block` builds from it while the original +/// staged entries are still alive (`write_staged_block` only drops +/// `stage.entries` after sealing) — another `block_size`; +/// - if encrypted, `seal_section`'s ciphertext and envelope copies (each +/// `plain.len() + ENCRYPTED_SST_BLOCK_OVERHEAD_BYTES`-sized), coexisting +/// with `plain` for the duration of one seal call; +/// - the leaf Bloom partition: one fixed bit array sized directly from +/// `block_size` (`bloom_partition_bits_len`); +/// - the leaf index builder (`leaf_entries`): its slot count +/// (`vec_capacity_bound(max_leaf_index_entries(index_chunk_plaintext_limit))`) +/// is sized by `SstWriter::create` for the worst case of *many small* +/// entries, so multiplying that count by a max-key-sized entry would +/// double-count — `preflight_leaf` caps the real accumulated key-byte +/// content at `index_chunk_plaintext_limit` directly and flushes before +/// exceeding it, so the term is `capacity × struct_size + +/// index_chunk_plaintext_limit`, two independently-bounded quantities +/// added, not one multiplied by the other; +/// - the internal index levels: at most one bounded builder *per level* +/// (same struct-slots-plus-content-cap shape as the leaf index), and +/// `MAX_INDEX_LEVELS` (32) is a fixed format constant — independent of +/// dataset volume, so this term is a fixed multiple of one level's bound, +/// never something that grows with the memtable. +/// +/// # What this does not close +/// +/// This is `basemyai-engine`'s own flush working set — it says nothing about +/// whether anything in this crate actually consumes a governed reservation +/// for it yet (nothing does: `store::engine::flush` never touches +/// `memory_governor`, same as `store::engine::write`'s MG-17 wiring — both +/// phase 2, ADR-071's own "Correction (audit code)" note). Deriving this +/// number is a prerequisite for closing `EmergencyHeadroom`'s "NON PROUVÉ" +/// status, not the whole of it: the *operational* proof (`GOV-U32` — a real +/// flush completing under a saturated domain) needs that phase-2 wiring to +/// exist first. +/// +/// # Errors +/// [`EngineError::MemoryAccountingOverflow`] if any step overflows `usize`, +/// or if `block_size`/`max_key_bytes` cannot support even one Bloom key or +/// one index entry (the same configurations `EngineOptions::validate` +/// already rejects) — `checked_add`/`checked_mul` throughout (MG-06). +pub fn estimate_emergency_flush_headroom_bytes( + memtable_flush_threshold: usize, + max_key_bytes: usize, + max_value_bytes: usize, + block_size: u32, + encrypted: bool, +) -> Result { + let memtable_clone_term = + estimate_memtable_reservation_bytes(memtable_flush_threshold, max_key_bytes, max_value_bytes)?; + let sst_writer_term = sst_writer_working_set_bytes(block_size, max_key_bytes, encrypted)?; + add(&[memtable_clone_term, sst_writer_term]) +} + +/// The `SstWriter`-only half of [`estimate_emergency_flush_headroom_bytes`]'s +/// derivation — `pub(crate)` (rather than private) specifically so +/// `store::sst_block::write`'s own tests can ground it directly against a +/// real `SstWriter` run (`memory_governor` and `store::sst_block` are +/// siblings under the crate root; `store::sst_block` is a private module, so +/// this function must cross that boundary, not the writer). That grounding +/// is the strongest check available: not just "this arithmetic is +/// internally consistent" but "this number was never exceeded by the actual +/// encoder it describes." +pub(crate) fn sst_writer_working_set_bytes(block_size: u32, max_key_bytes: usize, encrypted: bool) -> Result { + let block_size_usize = block_size as usize; + + let data_entries_capacity_limit = sst_block::vec_capacity_bound( + sst_block::max_data_block_entries(block_size).ok_or(EngineError::MemoryAccountingOverflow)?, + ) + .ok_or(EngineError::MemoryAccountingOverflow)?; + let staged_entries_struct_term = mul(data_entries_capacity_limit, size_of::())?; + // `current`'s wire content plus the re-encoded `plain` buffer built from + // it while it is still alive — both bounded by `block_size`. + let plain_copies_term = mul(block_size_usize, 2)?; + let crypto_term = if encrypted { + mul(add(&[block_size_usize, crypto::ENCRYPTED_SST_BLOCK_OVERHEAD_BYTES])?, 2)? + } else { + 0 + }; + let staged_block_term = add(&[staged_entries_struct_term, plain_copies_term, crypto_term])?; + + // One fixed Bloom partition, sized directly from `block_size` — not from + // the memtable's entry count. + let bloom_term = sst_block::bloom_partition_bits_len(block_size_usize) + .unwrap_or(0) + .checked_div(8) + .unwrap_or(0); + + let index_chunk_plaintext_limit = sst_block::index_chunk_plaintext_limit(block_size, max_key_bytes) + .ok_or(EngineError::MemoryAccountingOverflow)?; + + // `max_leaf_index_entries`/`max_index_node_children` size a Vec *slot + // count* assuming minimum-size entries (`INDEX_ENTRY_FIXED_BYTES`/ + // `INDEX_NODE_CHILD_MIN_BYTES`, ignoring key length) — the same + // worst-case-many-small-entries reasoning `SstWriter::create` itself + // uses to size `leaf_entries_capacity_limit`/`internal_children_ + // capacity_limit`. Multiplying that count by a *max-key-sized* entry + // would double-count: `preflight_leaf`/`push_internal_child` cap the + // real accumulated key-byte content at `index_chunk_plaintext_limit` + // directly (`leaf_encoded_len`/`internal_level_encoded_lens`, checked + // against that same limit before every push) and flush *before* + // exceeding it — so the struct-slot term (capacity × fixed struct size) + // and the key-content term (bounded by `index_chunk_plaintext_limit` + // itself, not by capacity × max_key_bytes) are two genuinely separate, + // independently-bounded quantities, not one multiplied by the other. + let leaf_entries_capacity_limit = sst_block::vec_capacity_bound( + sst_block::max_leaf_index_entries(index_chunk_plaintext_limit).ok_or(EngineError::MemoryAccountingOverflow)?, + ) + .ok_or(EngineError::MemoryAccountingOverflow)?; + let leaf_slots_term = mul(leaf_entries_capacity_limit, size_of::())?; + let leaf_index_term = add(&[leaf_slots_term, index_chunk_plaintext_limit])?; + + let internal_children_capacity_limit = sst_block::vec_capacity_bound( + sst_block::max_index_node_children(index_chunk_plaintext_limit).ok_or(EngineError::MemoryAccountingOverflow)?, + ) + .ok_or(EngineError::MemoryAccountingOverflow)?; + let node_slots_term = mul( + internal_children_capacity_limit, + size_of::(), + )?; + let per_level_term = add(&[node_slots_term, index_chunk_plaintext_limit])?; + // MAX_INDEX_LEVELS is a fixed format constant (32), not a function of + // this flush's data volume: at most one bounded builder is resident per + // level at a time, and the format caps the number of levels outright. + let internal_levels_term = mul(per_level_term, sst_block::MAX_INDEX_LEVELS as usize)?; + + add(&[staged_block_term, bloom_term, leaf_index_term, internal_levels_term]) +} + /// Transient bytes the encrypted WAL arm adds on top of the framed plain /// record — the AAD, the AEAD ciphertext and the on-disk envelope, all live /// at once with the record inside `store::wal::Wal::write_record`'s @@ -740,4 +907,107 @@ mod plan_tests { .expect_err("entry-count product must overflow, not wrap"); assert!(matches!(err, EngineError::MemoryAccountingOverflow)); } + + // ---- EmergencyHeadroom (ADR-071 §"Headroom de progrès background" / + // ADR-072 CB-08): derivation, not yet the operational GOV-U32 proof ---- + + #[test] + fn encrypted_sst_block_overhead_matches_the_real_envelope() { + // Pinned so a change to `EncryptedSstBlock`'s framing fails here, + // naming the cause, instead of silently shrinking the crypto term + // below what `seal_section` really allocates. + assert_eq!( + crypto::ENCRYPTED_SST_BLOCK_OVERHEAD_BYTES, + 50, + "magic+version+nonce+ct_len header (34) + Poly1305 tag (16)" + ); + assert_eq!( + sst_block::MAX_INDEX_LEVELS, + 32, + "SstFooter's index_levels field is a u8, not unbounded" + ); + } + + #[test] + fn flush_headroom_is_the_memtable_clone_plus_the_sst_writer_term() { + let memtable_term = estimate_memtable_reservation_bytes(100, 50, 200).expect("no overflow"); + let writer_term = sst_writer_working_set_bytes(4096, 50, false).expect("no overflow"); + let headroom = estimate_emergency_flush_headroom_bytes(100, 50, 200, 4096, false).expect("no overflow"); + assert_eq!( + headroom, + memtable_term + writer_term, + "the two terms this ADR identifies must be the whole of it, summed exactly" + ); + } + + #[test] + fn flush_headroom_encrypted_exceeds_plaintext_by_exactly_the_crypto_term() { + let plaintext = sst_writer_working_set_bytes(4096, 50, false).expect("no overflow"); + let encrypted = sst_writer_working_set_bytes(4096, 50, true).expect("no overflow"); + let expected_crypto_term = (4096 + crypto::ENCRYPTED_SST_BLOCK_OVERHEAD_BYTES) * 2; + assert_eq!(encrypted - plaintext, expected_crypto_term); + } + + #[test] + fn flush_headroom_overflow_is_typed_never_wrapped() { + let err = estimate_emergency_flush_headroom_bytes(usize::MAX, usize::MAX, 1, 4096, false) + .expect_err("the memtable clone term must overflow, not wrap"); + assert!(matches!(err, EngineError::MemoryAccountingOverflow)); + + let err = sst_writer_working_set_bytes(u32::MAX, usize::MAX, false) + .expect_err("an unreasonable max_key_bytes must overflow the writer term, not wrap"); + assert!(matches!(err, EngineError::MemoryAccountingOverflow)); + } + + /// Sanity check against a real engine configuration: the derived + /// headroom must stay dominated by the (already-pinned, ~16.6 GiB) + /// memtable clone term, not by the writer term — confirming the writer + /// term really is the small `O(block_size)` correction the doc comment + /// claims, not a second unbounded quantity hiding in the sum. + #[test] + fn default_engine_options_headroom_is_dominated_by_the_memtable_clone_term() { + let options = crate::store::EngineOptions::default(); + let memtable_term = estimate_memtable_reservation_bytes( + options.memtable_flush_threshold, + options.max_key_bytes, + options.max_value_bytes, + ) + .expect("no overflow"); + let writer_term = + sst_writer_working_set_bytes(options.block_size, options.max_key_bytes, false).expect("no overflow"); + let headroom = estimate_emergency_flush_headroom_bytes( + options.memtable_flush_threshold, + options.max_key_bytes, + options.max_value_bytes, + options.block_size, + false, + ) + .expect("no overflow"); + + assert_eq!(headroom, memtable_term + writer_term); + // Pinned exactly, not just bounded: dominated by + // `internal_levels_term`'s `index_chunk_plaintext_limit (~4 MiB) * + // MAX_INDEX_LEVELS (32)` ≈ 128 MiB — deliberately conservative for a + // 1 MiB `max_key_bytes` config (a real 1000-entry flush could never + // actually build 32 index levels; the format constant is the same + // regardless of how few entries a given flush holds). A future + // change to any of `sst_block`'s capacity-limit functions or to + // `EngineOptions::default()` should fail *here*, naming the cause, + // rather than silently moving the real headroom this crate would + // reserve. + assert_eq!(writer_term, 420_582_824); + assert!( + writer_term < memtable_term / 10, + "the SstWriter's own O(block_size) term ({writer_term}) must stay a modest correction on top of \ + the memtable clone term ({memtable_term}), not a comparably large quantity" + ); + } + + // The real-`SstWriter` grounding test for `sst_writer_working_set_bytes` + // lives in `store::sst_block::write`'s own test module + // (`sst_writer_term_bounds_a_real_writers_steady_state_across_many_ + // blocks_and_leaves`) — `store::sst_block` is a private module, a + // sibling of `memory_governor` under the crate root, so `SstWriter` + // cannot be reached from here; the function under test is `pub(crate)` + // specifically so that module can call back into this one instead. } diff --git a/crates/basemyai-engine/src/store/sst_block/write.rs b/crates/basemyai-engine/src/store/sst_block/write.rs index b0210df..8038fa5 100644 --- a/crates/basemyai-engine/src/store/sst_block/write.rs +++ b/crates/basemyai-engine/src/store/sst_block/write.rs @@ -972,6 +972,67 @@ mod tests { assert_eq!(loaded.entries().expect("entries"), data); } + // ADR-071 §"Headroom de progrès background" / ADR-072 CB-08: grounds + // `memory_governor::sst_writer_working_set_bytes`'s steady-state terms + // (staged block, Bloom partition, leaf index, internal levels — every + // component `SstWriter::resident_bytes` can observe from outside) + // against a real writer run, not just arithmetic that agrees with + // itself. Same fixture shape as `partitioned_writer_builds_a_ + // hierarchical_index` above (already proven to force multiple leaves + // *and* an internal index root), so every term the estimate computes is + // actually exercised here, not vacuously zero. + // + // `resident_bytes()` cannot see the transient `plain`/sealed/ciphertext + // copies `write_staged_block`/`seal_section` allocate and free within a + // single call (by design — its own doc comment defers that closure + // oracle to `r6_allocator_peak`), so this test validates the + // *steady-state* portion of the estimate exhaustively and leaves the + // transient-buffer portion to the direct code-reading recorded in that + // function's own doc comment. + #[test] + fn sst_writer_term_bounds_a_real_writers_steady_state_across_many_blocks_and_leaves() { + let dir = tempfile::tempdir().expect("tempdir"); + let data = entries(200, 40); + let cache = Arc::new(MetadataCache::new(8 * 1024 * 1024)); + let block_size: u32 = 128; + let max_key_bytes: usize = 32; + let mut writer = SstWriter::create(dir.path(), 0, block_size, None, max_key_bytes, 0, Arc::clone(&cache)) + .expect("create writer"); + + let bound = crate::memory_governor::sst_writer_working_set_bytes(block_size, max_key_bytes, false) + .expect("no overflow"); + let mut peak_resident_bytes = 0usize; + for (index, (key, value)) in data.iter().enumerate() { + let kind = if value.is_some() { + ValueKind::Value + } else { + ValueKind::Tombstone + }; + writer + .push(InternalKey::new(key.clone(), 1, kind), value.clone()) + .expect("push"); + peak_resident_bytes = peak_resident_bytes.max(writer.resident_bytes()); + assert!( + writer.resident_bytes() <= bound, + "writer.resident_bytes()={} exceeded the derived steady-state bound={bound} after {index} pushes", + writer.resident_bytes() + ); + } + let written = writer.finish().expect("finish"); + assert!( + written.footer.bloom_partition_count > 1, + "fixture must force multiple leaves — otherwise the leaf-index/Bloom terms are untested" + ); + assert!( + written.footer.index_levels > 1, + "fixture must force an internal index root — otherwise the internal-levels term is untested" + ); + assert!( + peak_resident_bytes > 0, + "the writer must have actually held something resident at some point" + ); + } + #[test] fn versioned_roundtrip_preserves_versions_and_snapshot_visibility() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/docs/adr/ADR-071-global-admission-accounting-and-governor.md b/docs/adr/ADR-071-global-admission-accounting-and-governor.md index f3e1c13..a0b6390 100644 --- a/docs/adr/ADR-071-global-admission-accounting-and-governor.md +++ b/docs/adr/ADR-071-global-admission-accounting-and-governor.md @@ -11,8 +11,13 @@ mémoire, writer/recovery, red-team) confrontant le texte au code réel de plus bas liste les conditions qui doivent être *simultanément* vraies avant que le governor lui-même soit ratifié au sens opérationnel — activable, testé, prêt à gouverner un budget réel. À la date de la dernière revue -(2026-08-17), elles ne le sont pas : `EmergencyHeadroom` reste **NON -PROUVÉ**, le câblage direct de MG-17 dans `store/engine/write.rs` +(2026-08-17), elles ne le sont pas : `EmergencyHeadroom`'s byte formula is +now derived and tested against a real `SstWriter` run +(`memory_governor::estimate_emergency_flush_headroom_bytes`, §"Post- +ratification hardening" ci-dessous) but its *operational* proof +(`GOV-U32` — a real flush completing under a saturated domain) is not, +because nothing in this crate consumes a governed reservation for a flush +yet; le câblage direct de MG-17 dans `store/engine/write.rs` (chantier phase 2 distinct du mécanisme §B déjà implémenté par ADR-072, qui n'en dépend pas) n'est pas scopé, et une partie substantielle de la suite `GOV-U*`/`GOV-S*`/`GOV-RSS*` ci-dessous n'est pas encore écrite (voir @@ -1305,3 +1310,9 @@ Une revue indépendante de `crates/basemyai-engine/src/memory_governor/` (le cod Un audit séparé du câblage ADR-072 (§A/§B) n’a trouvé **aucune violation confirmée** de CB-01…CB-07 — l’ingénierie d’ordre de verrouillage est correcte telle qu’écrite. Trois observations non bloquantes (portée `MemoryDomain` partagé pour l’argument panic du bootstrap, `MemoryGovernor::close()` non câblé dans les chemins d’arrêt du coordinateur, une fenêtre étroite de shutdown pouvant sauter la charge d’une génération rotée) sont documentées dans ADR-072 lui-même, avec la seconde déjà corrigée (voir ADR-072 §"Post-ratification hardening"). Une analyse de couverture de tests indépendante confirme que `GOV-U01, U04-U11, U19-U21(partiel), U24, U27, U31b, U38` sont couverts, mais que `GOV-U21-U23, U25-U26, U40-U41, S03` (des propriétés pures du governor, sans dépendance cache/writer) n’ont aujourd’hui **aucun** test, alors qu’ADR-071 les liste comme requis avant ratification et non comme hors-scope phase 1. Ce n’est pas corrigé par ce hardening — c’est une dette de test explicitement identifiée et non résolue, à traiter avant toute activation production. + +3. **`EmergencyHeadroom`, moitié analytique dérivée et testée — la moitié opérationnelle reste ouverte.** Lecture complète de `store::engine::flush::run_job` et de l’écrivain incrémental `SstWriter` qu’il pilote (`format::sst_block::write`, réécriture R6.2/ADR-049 §3) pour identifier chaque allocation qu’un flush fait au-delà de la memtable qu’il retire : + - **Terme dominant, exact** : `Memtable::iter_versions` clone en profondeur chaque clé et valeur dans un `Vec` frais avant que `run_job` n’appelle l’écrivain ; ce `Vec`, une fois déplacé dans `SstWriter::write_new_versioned`, garde toute son allocation vivante pendant la durée du `for (key, value) in entries` (`Vec::IntoIter` ne libère pas la queue déjà consommée). Pendant tout le flush, la memtable retirée (déjà chargée sous `WriteBufferMutable`) et ce clone (aujourd’hui totalement hors compte) coexistent. Même forme par-entrée que `estimate_memtable_reservation_bytes`, réutilisée telle quelle (`memory_governor::estimate_emergency_flush_headroom_bytes`). + - **Second terme, borné `O(block_size)` et non `O(taille de la memtable)`** : documenté comme l’objectif de conception explicite du réécrivain lui-même (« the largest live allocation is one data block ») et par la discipline ADR-053 (un seul Bloom fixe, un seul bâtisseur de feuille, au plus un bâtisseur de nœud interne par niveau). Chaque sous-terme appelle les *mêmes* fonctions de borne de capacité que `SstWriter::create` lui-même — `memory_governor::sst_writer_working_set_bytes`, ne peut donc pas dériver silencieusement de l’encodeur réel. + - **Preuve, pas seulement arithmétique cohérente avec elle-même** : `sst_writer_term_bounds_a_real_writers_steady_state_across_many_blocks_and_leaves` (`store/sst_block/write.rs`) fait tourner un vrai `SstWriter` sur un jeu de données forçant plusieurs feuilles et plusieurs niveaux d’index (même fixture que `partitioned_writer_builds_a_hierarchical_index`) et vérifie `writer.resident_bytes() <= sst_writer_working_set_bytes(...)` à chaque insertion — une première erreur de dérivation (multiplier une capacité de slots dimensionnée pour des entrées minimales par la taille d’une clé maximale, plutôt que d’utiliser séparément la borne réelle en octets `index_chunk_plaintext_limit`) a été trouvée et corrigée précisément grâce à ce test, qui produisait un terme aberrant de 4,67 billions d’octets avant correction. + - **Ce que cette dérivation ne ferme pas** : `resident_bytes()` ne voit pas les tampons transitoires `plain`/scellé/chiffré que `write_staged_block`/`seal_section` allouent et libèrent en une seule fois — cette portion reste dérivée par lecture directe du code, documentée mais non mesurée à l’exécution. Et surtout : rien dans `basemyai-engine` ne consomme encore de réservation gouvernée pour un flush (`store::engine::flush` ne touche pas `memory_governor`) — dériver le nombre est un prérequis pour fermer le statut « NON PROUVÉ », pas la totalité : la preuve *opérationnelle* (`GOV-U32`, un vrai flush qui progresse sous un domaine saturé) reste bloquée sur le câblage MG-17/`write.rs` de phase 2 (point 1 ci-dessus dans le chantier écarté de `CB-T4`, ADR-072). diff --git a/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md b/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md index 226b724..3418010 100644 --- a/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md +++ b/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md @@ -312,8 +312,17 @@ CB-08 EmergencyHeadroom (ADR-071) est une condition de non-blocage ## Points ouverts / différés (non bloquants pour la ratification, listés explicitement) - Forme exacte du signal de rotation sur `CoordinatorOutcome` (§B). -- Dérivation analytique d'`EmergencyHeadroom` (héritée d'ADR-071, - désormais explicitement liée à CB-05/CB-08). +- **Dérivation analytique d'`EmergencyHeadroom` — moitié faite (2026-08-17).** + `memory_governor::estimate_emergency_flush_headroom_bytes` dérive et teste + la formule (mémoire clonée par `Memtable::iter_versions` + working set + `O(block_size)` du `SstWriter`, ce dernier grounded contre un vrai writer + via `sst_writer_term_bounds_a_real_writers_steady_state_across_many_ + blocks_and_leaves`). Ce qui reste ouvert, toujours lié à CB-05/CB-08 : + personne ne consomme encore cette réservation (`store::engine::flush` ne + touche pas `memory_governor`), donc la preuve *opérationnelle* (`GOV-U32`, + un flush réel qui progresse sous domaine saturé) reste bloquée sur le + câblage MG-17/`write.rs` de phase 2 — la même dépendance déjà identifiée + pour `CB-T4` ci-dessus. - **Revue de la réservation memtable de 16,6 GiB (`memtable_flush_threshold × per-entry-max`, §B) et, probablement, de la condition de seal elle-même.** Cette valeur est le pire cas analytique sous les défauts @@ -503,7 +512,17 @@ réservation amont d'une rotation, **tous** les writes du coordinateur se bloquent jusqu'à libération de capacité, généralement par un flush — et le flush lui-même a besoin de capacité gouvernée pour progresser. Activer le hard cap avec un `EmergencyHeadroom` non prouvé revient à activer un -mécanisme qui peut s'auto-bloquer sans chemin de sortie garanti. Ce gate -reste ouvert tant que ADR-071 documente `EmergencyHeadroom` comme -« NON PROUVÉ » (voir ce document, §B et invariant CB-08) ; il n'est pas -levé par le seul fait que §A/§B compilent et passent leurs tests unitaires. +mécanisme qui peut s'auto-bloquer sans chemin de sortie garanti. + +**Statut (2026-08-17) : moitié faite, gate toujours fermé.** La borne +byte-level est désormais dérivée et testée +(`memory_governor::estimate_emergency_flush_headroom_bytes`, grounded +contre un vrai `SstWriter` — voir ADR-071 §"Post-ratification hardening"). +Ce que « dérivée analytiquement » exigeait est donc satisfait. Ce qui ne +l'est pas : rien dans `basemyai-engine` ne consomme encore cette +réservation pour un flush réel, donc l'affirmation « le flush peut +progresser sous un domaine saturé » n'a aucun chemin de code à ce jour — +seule une preuve *opérationnelle* (`GOV-U32`) fermerait ce point, et elle +dépend du câblage MG-17/`write.rs` de phase 2, non scopé. Ce gate reste +donc ouvert ; il n'est pas levé par le seul fait que §A/§B compilent et +passent leurs tests unitaires, ni par le seul fait que la formule existe. diff --git a/docs/status.md b/docs/status.md index 8af3a77..e776be3 100644 --- a/docs/status.md +++ b/docs/status.md @@ -1,5 +1,67 @@ # BaseMyAI — Implementation Status Matrix +**Mise à jour 2026-08-17 (suite 2) — CB-T4 requalifié, EmergencyHeadroom +dérivé et testé (moitié analytique), worktrees/branches mortes nettoyées.** +Suite directe de l'entrée précédente, sur décision explicite de +priorisation (P0 avant activation) : + +- **CB-T4 corrigé** — l'attribution initiale (le `TODO(ADR-072 §B)` de + `coordinator.rs` comme preuve d'un mécanisme manquant) était une mauvaise + référence croisée : ce TODO concerne CB-04 (release-vs-retain), pas + `maybe_flush`. Le mécanisme §B implémenté (`reserve_blocking`) ne peut + structurellement jamais produire `AdmissionWouldBlock` — c'est le choix + de conception assumé (bloquer + `EmergencyHeadroom`), pas un oubli. + `CB-T4` décrit en réalité le câblage MG-17 dans `store/engine/write.rs`, + chantier phase 2 distinct et non scopé — retiré de la liste "requis + avant activation" du mécanisme §B. +- **`EmergencyHeadroom` : moitié analytique dérivée, testée, groundée + contre un vrai writer — moitié opérationnelle toujours bloquée.** + `memory_governor::estimate_emergency_flush_headroom_bytes` (nouveau) somme + deux termes trouvés en lisant `store::engine::flush::run_job` et + `format::sst_block::write::SstWriter` de bout en bout : (1) le clone + complet de la memtable que `Memtable::iter_versions` produit avant + l'écriture de l'SST, dominant, réutilise exactement la formule CB-06 ; (2) + le working set propre du `SstWriter`, prouvé `O(block_size)` et non + `O(taille de la memtable)` par la conception documentée du réécrivain + R6.2/ADR-049 §3 lui-même. Le test + `sst_writer_term_bounds_a_real_writers_steady_state_across_many_blocks_and_leaves` + (`store/sst_block/write.rs`) fait tourner un vrai `SstWriter` et vérifie + que `resident_bytes()` ne dépasse jamais la borne dérivée — une première + version de la formule (multiplier une capacité de slots par une taille de + clé maximale au lieu d'utiliser la borne réelle en octets séparément) + produisait un terme aberrant de 4,67 billions d'octets ; le test l'a + détecté immédiatement, avant tout commit. Ce qui reste ouvert : rien ne + consomme encore cette réservation pour un flush réel + (`store::engine::flush` ne touche pas `memory_governor`), donc la preuve + *opérationnelle* (`GOV-U32`) reste bloquée sur le câblage MG-17/`write.rs` + de phase 2 — même dépendance que `CB-T4`. Le gate d'activation production + reste donc fermé (ADR-072 §"Gates de ratification"). +- **Nettoyage worktrees/branches** : `.claude/worktrees/agent-*` (5 + répertoires, ~13 Go) pointaient vers `D:/dev/_ecosystems/forgemyai/basemyai` + — un chemin de dépôt qui n'existe plus sur disque — et étaient donc + totalement orphelins (confirmé : aucun ne figurait dans `git worktree + list` du dépôt courant). Supprimés. 15 branches locales + `worktree-agent-*` (plus `hotfix/candle-core-0.11`) sans worktree vivant + et entièrement fusionnées dans `dev` supprimées via `git branch -d` + (jamais `-D` : la vérification stricte de git a d'ailleurs refusé de + supprimer `codex/v2-03-write-coordinator`/`codex/v2-normative-integration` + malgré leur apparition dans `--merged dev`, laissées intactes pour revue + manuelle). Les worktrees enregistrés légitimes + (`basemyai-r5-bench-r3`/`r4`, le worktree codex bugscan) n'ont pas été + touchés. +- **Gates rejoués** : `cargo xtask check` vert (fmt + clippy `-D warnings` + toutes crates, y compris le cross-module cfg-gating de + `sst_writer_working_set_bytes`) ; `cargo test -p basemyai-engine + --features test-util --lib` **610/610** (+6 nouveaux tests) ; `cargo + xtask test-crash-consistency` relancé sur l'arbre corrigé **13/13, + 539,25 s** — aucune régression. +- **Correction hygiène commit non liée** : `crates/basemyai-core/src/storage/engine.rs` + portait un diff non expliqué (commentaires de doc raccourcis) découvert + pendant cette session, sans lien avec les autres changements uncommitted + de la session précédente ; confirmé anodin (aucune perte de contenu + factuel, juste moins de prose) et committé séparément après vérification + explicite avec l'utilisateur. + **Mise à jour 2026-08-17 (suite) — première revue adversariale indépendante de la phase 1 ADR-071/ADR-072 : deux bugs réels corrigés, dette de test substantielle identifiée et trackée, aucune activation production From bdc3c6641dd8b6c40e8f8ce2994e11c1a60b0f3a Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Mon, 17 Aug 2026 19:46:35 +0200 Subject: [PATCH 04/15] test(V2-04): close GOV-U40/GOV-U41/GOV-S03 for the memory governor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-071's test spec table lists three untested properties of the memory governor: sharing one MemoryDomain across engines stays within a single budget (GOV-U40), independent MemoryDomains never leak capacity between each other (GOV-U41), and MemoryGovernor::close() under stress wakes every parked waiter, admits nothing new (even at the close race boundary), and leaves bookkeeping bounded (GOV-S03). Each test's oracle was red/green verified by injecting a matching synthetic defect (a non-shared engine B, an accidentally-shared governor between two domains, and a no-op close()) and confirming failure before reverting. Also confirms (via code reading, not modified per ADR-072's own explicit deferral): WriteCoordinator::build's try_reserve-then-panic bootstrap path is unsound once a MemoryDomain is shared across engines, since a sibling engine's flush/compaction becomes an agent that can free bytes. Left as-is — dormant_governed has no production call site yet, and ADR-072 already tracks this as an open, deliberately deferred point. --- .../src/memory_governor/admission.rs | 311 +++++++++++++++++- 1 file changed, 309 insertions(+), 2 deletions(-) diff --git a/crates/basemyai-engine/src/memory_governor/admission.rs b/crates/basemyai-engine/src/memory_governor/admission.rs index c2f2f16..91dce39 100644 --- a/crates/basemyai-engine/src/memory_governor/admission.rs +++ b/crates/basemyai-engine/src/memory_governor/admission.rs @@ -510,8 +510,9 @@ impl MemoryGovernor { #[cfg(test)] mod tests { - use std::sync::{Arc, Barrier}; - use std::time::Duration; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::{Arc, Barrier, mpsc}; + use std::time::{Duration, Instant}; use super::*; use crate::memory_governor::{GovernorConfig, MemoryDomain}; @@ -1222,4 +1223,310 @@ mod tests { ); assert!(matches!(err, EngineError::AdmissionClosed)); } + + // ---- GOV-U40/GOV-U41/GOV-S03 (ADR-071 test spec table) ---- + + /// Bounds a `JoinHandle::join()` by a deterministic timeout instead of + /// risking an actual process hang if the property under test is broken + /// (e.g. a governor `close()` that stops waking parked waiters). Mirrors + /// the coordinator-level precedent + /// (`closing_a_governed_coordinator_wakes_a_caller_parked_on_byte_capacity` + /// in `basemyai::storage::native_store::coordinator`), which bounds its + /// own wait the same way via `tokio::time::timeout`. A supervisor thread + /// does the actual (possibly unbounded) `join()`; the caller only ever + /// waits on a channel with a timeout, so a genuinely hung target thread + /// leaks one supervisor thread for the rest of the test process instead + /// of hanging the test runner itself. + fn join_within(name: &str, handle: std::thread::JoinHandle, timeout: Duration) -> T { + let (tx, rx) = mpsc::channel(); + std::thread::spawn(move || { + let _ = tx.send(handle.join()); + }); + match rx.recv_timeout(timeout) { + Ok(Ok(value)) => value, + Ok(Err(_)) => panic!("{name} thread panicked"), + Err(_) => panic!("{name} thread did not finish within {timeout:?} — suspected hang"), + } + } + + fn zero_plan(peak_bytes: usize) -> MemoryPlan { + MemoryPlan { + peak_bytes, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + } + } + + // GOV-U40: two engine-like consumers sharing one `Arc` — + // concretely, two independent `Arc` handles cloned from + // the very same domain (ADR-071 §"Portée du domaine": "un produit qui + // veut une limite commune partage le même `Arc`", + // `mod.rs:79-81`) — must never together commit more than the domain's + // single `budget_bytes`, and admission from *either* side must block + // once that shared budget is exhausted, regardless of which "engine" + // asked. + // + // Driven concurrently by two real OS threads racing for the same + // capacity via `try_reserve`, not sequentially — the governor's own + // mutex is what must serialize them correctly, not test ordering. + #[test] + fn gov_u40_two_engines_sharing_one_domain_respect_a_single_shared_budget() { + let budget = 2_000_000usize; + let d = domain(budget, budget / 2, budget / 2, 1, 1); + // Two "engines": independent `Arc` handles onto the one shared + // governor behind `d`'s single `MemoryDomain`. + let engine_a = Arc::clone(d.governor()); + let engine_b = Arc::clone(d.governor()); + + let per_chunk = 50_000usize; + assert_eq!( + budget % per_chunk, + 0, + "fixture must divide evenly for an exact post-hoc count" + ); + let barrier = Arc::new(Barrier::new(2)); + let mut handles = Vec::new(); + for engine in [Arc::clone(&engine_a), Arc::clone(&engine_b)] { + let barrier = Arc::clone(&barrier); + handles.push(std::thread::spawn(move || { + barrier.wait(); + let mut held = Vec::new(); + loop { + match engine.try_reserve(GovernedMemoryKind::SstDataBlock, None, per_chunk) { + Ok(reservation) => held.push(reservation), + Err(EngineError::AdmissionWouldBlock) => break, + Err(other) => panic!("unexpected error draining the shared budget: {other:?}"), + } + } + held + })); + } + + let mut per_engine_counts = Vec::new(); + let mut all_reservations = Vec::new(); + for h in handles { + let held = join_within("engine", h, Duration::from_secs(10)); + per_engine_counts.push(held.len()); + all_reservations.extend(held); + } + + // Oracle: the combined total from both engines exactly fills the + // *single* shared budget — not twice it, which is what two + // accidentally-independent per-engine budgets would allow. + assert_eq!( + per_engine_counts.iter().sum::(), + budget / per_chunk, + "combined chunks across both engines must exactly fill the one shared budget, got {per_engine_counts:?}" + ); + // Not asserted: that *both* engines got a non-zero share. + // `try_reserve`'s raw mutex has no fairness contract (unlike the DRR + // admission path), so an OS scheduler that always favors one thread + // could legitimately let it drain the whole budget alone — that is + // still "a single shared budget respected", just an unfair split of + // it. The sum-equals-budget assertion above is what actually proves + // sharing; see `per_engine_counts` for the observed split. + assert_eq!(engine_a.committed_bytes(), budget); + + // Further admission from *either* handle is now blocked — proving + // there is really one budget, not two that happen to sum correctly. + let err_a = expect_err( + engine_a.try_reserve(GovernedMemoryKind::SstDataBlock, None, 1), + "engine A must be blocked once the shared budget is exhausted", + ); + assert!(matches!(err_a, EngineError::AdmissionWouldBlock)); + let err_b = expect_err( + engine_b.try_reserve(GovernedMemoryKind::SstDataBlock, None, 1), + "engine B must be blocked too — the same shared budget, not an independent one", + ); + assert!(matches!(err_b, EngineError::AdmissionWouldBlock)); + + drop(all_reservations); + assert_eq!( + engine_a.committed_bytes(), + 0, + "every reservation from both engines must release exactly" + ); + } + + // GOV-U41: two *independent* `MemoryDomain`s (separate `MemoryGovernor`s, + // separate budgets) must never leak capacity between each other — + // saturating domain A must have zero effect on domain B's own admission, + // and neither governor's own accounting is a process-global metric. + #[test] + fn gov_u41_two_independent_domains_never_share_capacity() { + let budget_a = 500_000usize; + let budget_b = 700_000usize; + let domain_a = domain(budget_a, budget_a / 4, budget_a / 4, 1, 1); + let domain_b = domain(budget_b, budget_b / 4, budget_b / 4, 1, 1); + let engine_a = domain_a.governor(); + let engine_b = domain_b.governor(); + + // Saturate A completely. + let hog_a = engine_a + .try_reserve(GovernedMemoryKind::SstDataBlock, None, budget_a) + .expect("A's own full budget must be reservable on its own governor"); + assert_eq!(engine_a.committed_bytes(), budget_a); + let err = expect_err( + engine_a.try_reserve(GovernedMemoryKind::SstDataBlock, None, 1), + "A must now be fully saturated", + ); + assert!(matches!(err, EngineError::AdmissionWouldBlock)); + + // B is untouched: no shared counter means its own committed_bytes is + // still exactly zero, and it can still admit its own full budget, + // unaffected by A's saturation. + assert_eq!( + engine_b.committed_bytes(), + 0, + "domain B's committed_bytes must be entirely independent of domain A's saturation" + ); + let full_b = engine_b + .try_reserve(GovernedMemoryKind::SstDataBlock, None, budget_b) + .expect("domain B must still admit its own full budget while A is saturated"); + assert_eq!(engine_b.committed_bytes(), budget_b); + + // And the reverse holds: draining B to its own limit doesn't touch + // A's own view of itself either. + assert_eq!( + engine_a.committed_bytes(), + budget_a, + "A's committed_bytes must be unaffected by B's own saturation" + ); + let err = expect_err( + engine_b.try_reserve(GovernedMemoryKind::SstDataBlock, None, 1), + "B must now be saturated on its own budget, independent of A", + ); + assert!(matches!(err, EngineError::AdmissionWouldBlock)); + + drop(hog_a); + drop(full_b); + assert_eq!(engine_a.committed_bytes(), 0); + assert_eq!(engine_b.committed_bytes(), 0); + } + + // GOV-S03 (stress): shutdown with several live `MemoryReservation`s and + // multiple parked waiters across *both* admission classes in flight. + // `MemoryGovernor::close()` must (1) wake every parked waiter within a + // bounded, deterministic time — no hang; (2) admit nothing new from that + // point on, even a caller racing exactly at the close boundary; and (3) + // leave bookkeeping bounded — no leaked waiter entries, every + // pre-existing reservation still releases exactly. + #[test] + fn gov_s03_shutdown_wakes_every_waiter_blocks_new_admission_and_bounds_cleanup() { + let d = domain(1_000_000, 400_000, 400_000, 1, 1); + let governor = Arc::clone(d.governor()); + + // Several live reservations in flight, unrelated to admission — + // simulating in-progress cache/flush work that must survive + // shutdown without leaking (oracle 3). + let live_reservations: Vec<_> = (0..3) + .map(|_| { + governor + .try_reserve(GovernedMemoryKind::SstDataBlock, None, 10_000) + .expect("seed a live reservation") + }) + .collect(); + + // Hog nearly all remaining capacity so every acquire_admission below + // is genuinely forced to park, not racing a timing window. + let free_before_hog = 1_000_000 - governor.committed_bytes(); + let hog = governor + .try_reserve(GovernedMemoryKind::FlushWorkingSet, None, free_before_hog - 1) + .expect("hog nearly all remaining capacity"); + assert_eq!(governor.committed_bytes(), 1_000_000 - 1); + + // Several waiters across both admission classes. + const WAITERS: usize = 6; + let mut waiter_handles = Vec::new(); + for i in 0..WAITERS { + let g = Arc::clone(&governor); + let class = if i % 2 == 0 { + AdmissionClass::Foreground + } else { + AdmissionClass::Maintenance + }; + waiter_handles.push(std::thread::spawn(move || g.acquire_admission(class, zero_plan(100)))); + } + + // Deterministic, not timing-based: wait until every waiter has + // genuinely enqueued before triggering shutdown. + let deadline = Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < WAITERS { + assert!(Instant::now() < deadline, "all waiters must eventually park"); + std::thread::yield_now(); + } + + // A racer hammering non-blocking admission attempts concurrently + // with `close()`, to catch a caller racing exactly at the close + // boundary (oracle 2's strongest form): once it has observed one + // `AdmissionClosed`, no *later* attempt may ever succeed. + let racer_governor = Arc::clone(&governor); + let racer_stop = Arc::new(AtomicBool::new(false)); + let racer_stop_flag = Arc::clone(&racer_stop); + let racer_saw_admission_after_closed = Arc::new(AtomicBool::new(false)); + let racer_flag = Arc::clone(&racer_saw_admission_after_closed); + let racer = std::thread::spawn(move || { + let mut closed_seen = false; + while !racer_stop_flag.load(Ordering::SeqCst) { + match racer_governor.try_acquire_admission(AdmissionClass::Foreground, zero_plan(1)) { + Ok(_permit) => { + if closed_seen { + racer_flag.store(true, Ordering::SeqCst); + } + } + Err(EngineError::AdmissionClosed) => closed_seen = true, + Err(_) => {} + } + } + }); + + governor.close(); + + // Oracle 1: every parked waiter wakes within a bounded timeout — + // no hang — and with the correct typed error, never a silent + // success or an indefinite park. + for (i, h) in waiter_handles.into_iter().enumerate() { + let outcome = join_within("waiter", h, Duration::from_secs(10)); + assert!( + matches!(outcome, Err(EngineError::AdmissionClosed)), + "waiter {i} must wake with AdmissionClosed, not hang or succeed" + ); + } + + // Let the racer keep hammering briefly past the wake barrier above, + // then stop it and join with the same bounded discipline. + std::thread::sleep(Duration::from_millis(50)); + racer_stop.store(true, Ordering::SeqCst); + join_within("racer", racer, Duration::from_secs(10)); + assert!( + !racer_saw_admission_after_closed.load(Ordering::SeqCst), + "oracle 2: no admission may ever succeed once AdmissionClosed has been observed, \ + not even one racing the close boundary" + ); + + // Oracle 2, direct form: a fresh call against the now-closed + // governor is refused deterministically, not by luck. + let err = expect_err( + governor.try_acquire_admission(AdmissionClass::Maintenance, zero_plan(1)), + "a fresh call after close() must be refused, never granted", + ); + assert!(matches!(err, EngineError::AdmissionClosed)); + + // Oracle 3: bookkeeping is bounded — no leaked waiter entries, and + // every pre-existing reservation still releases exactly, even past + // shutdown. + assert_eq!( + governor.admission_waiters(), + 0, + "closed waiters' bookkeeping must be fully cleared, not leaked" + ); + drop(hog); + drop(live_reservations); + assert_eq!( + governor.committed_bytes(), + 0, + "every pre-existing reservation must still release exactly after shutdown" + ); + } } From d2415b750e279e8f8cc842a6b77bb6461a1f2589 Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Mon, 17 Aug 2026 19:51:20 +0200 Subject: [PATCH 05/15] test(V2-04): GOV-U25/GOV-U26 spurious-wake and lost-wake coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close the two ADR-071 test-spec entries flagged as missing in docs/status.md's 2026-08-17 entry (GOV-U21-23/U25-26/U40-41/S03, "propriétés pures du governor... sans aucun test"). GOV-U25 (spurious wake): parks an ineligible waiter (hog leaves 1 free byte of a 999-byte-reserved class, waiter asks for 500), then fires repeated Condvar::notify_all() directly on the governor's private `changed` field with zero intervening state mutation — the closest in-process stand-in for an OS spurious futex wakeup. Asserts committed bytes/slots/waiter bookkeeping are untouched, then confirms the same waiter is still alive and gets granted once real capacity frees up. GOV-U26 (lost wake): races a releaser against a waiter released from a shared Barrier for 200 iterations with a hard 2s per-iteration timeout, so the exact interleaving around the waiter's enqueue/park sequence varies run to run — a lost wakeup would surface as a deterministic per-iteration timeout, not a hang. Investigated admission.rs/state.rs/scheduler.rs first: acquire_admission already holds GovernorState's mutex continuously between checking a waiter's own predicate and calling Condvar::wait on it (no unlock/relock gap), and every mutating path funnels through apply_or_fail_stop, which re-runs the DRR scheduler and notifies under the same lock — the standard lost-wakeup-proof monitor pattern. No real bug found; this is coverage for already-correct behavior, proven via the project's red/green defect-injection methodology (GV-L7 precedent) rather than asserted: - GOV-U25: temporarily made scheduler::capacity_eligible unconditionally return true. Red — the waiter was granted the instant it enqueued, before the observation loop could even see it parked. Reverted, green. - GOV-U26: temporarily reintroduced the classic bug (drop the lock, sleep, reacquire, then wait — instead of Condvar::wait's atomic unlock+park). Red — failed on iteration 0 with the exact "lost wakeup" timeout. Reverted, green. cargo xtask check green; cargo test -p basemyai-engine --features test-util --lib memory_governor: 49/49 (2 new); full crate suite 612/612. Co-Authored-By: Claude Sonnet 5 --- .../src/memory_governor/admission.rs | 192 ++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/crates/basemyai-engine/src/memory_governor/admission.rs b/crates/basemyai-engine/src/memory_governor/admission.rs index c2f2f16..2b7db97 100644 --- a/crates/basemyai-engine/src/memory_governor/admission.rs +++ b/crates/basemyai-engine/src/memory_governor/admission.rs @@ -1222,4 +1222,196 @@ mod tests { ); assert!(matches!(err, EngineError::AdmissionClosed)); } + + // ---- GOV-U25 / GOV-U26: spurious wake and lost wake ---- + + // GOV-U25: spurious wake — `notify_all()` fired with no capacity change + // at all — must never grant an ineligible waiter. ADR-071 + // §"Linéarisation de l'admission": "Le `Condvar` ne décide jamais quel + // waiter gagne. Le scheduler marque un waiter précis `Granted` sous le + // mutex ; un réveil OS ne fait que permettre aux threads de revérifier + // leur prédicat." Proven by reaching into the private `changed` field + // directly (this `tests` module is a descendant of `admission`'s own + // module, exactly like `gov_u38`'s direct access to `state` above) and + // firing repeated spurious wakeups with zero intervening state + // mutation — the closest in-process stand-in for an OS spurious futex + // wakeup, and a strictly harder scenario than a "real but insufficient" + // release would be, since here nothing at all changes. + #[test] + fn gov_u25_spurious_wake_without_new_capacity_grants_no_ineligible_waiter() { + let config = GovernorConfig { + budget_bytes: 1_000, + max_queue_depth: 3, + max_admission_waiters: 4, + reserved_slots: [1, 1], + reserved_bytes: [999, 1], + weight: [1, 1], + max_intent_peak_bytes: [999, 1], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }; + let d = MemoryDomain::new(config).expect("valid config"); + let governor = d.governor(); + + // Hog leaves only 1 free byte in Foreground: any request for more + // than that is provably, deterministically unsatisfiable until hog + // drops — not a timing window (same technique as GOV-U27 above). + let hog = governor + .try_acquire_admission( + AdmissionClass::Foreground, + MemoryPlan { + peak_bytes: 999, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + }, + ) + .expect("hog grant"); + + let waiter_governor = Arc::clone(governor); + let waiter = std::thread::spawn(move || { + waiter_governor.acquire_admission( + AdmissionClass::Foreground, + MemoryPlan { + peak_bytes: 500, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + }, + ) + }); + + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < 1 { + assert!(std::time::Instant::now() < deadline, "waiter must eventually enqueue"); + std::thread::yield_now(); + } + // Give the waiter thread a real chance to actually be parked inside + // `Condvar::wait`, not merely enqueued. + std::thread::sleep(Duration::from_millis(100)); + + let before_bytes = governor.committed_bytes(); + let before_slots = governor.outstanding_slots(); + + // The GOV-U25 scenario itself: fire repeated spurious wakeups with + // no capacity change whatsoever. + for _ in 0..50 { + governor.changed.notify_all(); + std::thread::sleep(Duration::from_millis(5)); + } + + assert_eq!( + governor.committed_bytes(), + before_bytes, + "a spurious wake must never grant bytes to an ineligible waiter" + ); + assert_eq!( + governor.outstanding_slots(), + before_slots, + "a spurious wake must never grant a slot to an ineligible waiter" + ); + assert_eq!( + governor.admission_waiters(), + 1, + "the waiter must still be parked (Pending), not granted-and-removed" + ); + + // Sanity: the waiter is not dead — real capacity release still + // grants it, proving the spurious wakes above did not corrupt its + // bookkeeping into some stuck state either. + drop(hog); + let permit = waiter + .join() + .expect("waiter thread") + .expect("must eventually be granted once real capacity frees up"); + assert_eq!(permit.class(), AdmissionClass::Foreground); + } + + // GOV-U26: lost wake — a capacity release timed to land exactly around + // a waiter's `Condvar::wait()` call must never leave that waiter parked + // forever. `acquire_admission`'s wake loop holds `GovernorState`'s + // mutex continuously between checking this waiter's own predicate + // (`state_of(seq)`) and calling `wait()` on it — no unlock/relock gap + // in between — which is the classic safe-monitor pattern + // `std::sync::Condvar` is built to make lost-wakeup-proof, *provided* + // the releasing side also mutates and notifies under the very same + // lock, which `apply_or_fail_stop` does. This test does not merely + // assert that reasoning; it races a releaser against a waiter released + // from the same `Barrier` hundreds of times, so the exact interleaving + // (release completing before the waiter even enqueues, racing its + // enqueue, racing its transition into `wait()`, or landing while it is + // already parked) varies from iteration to iteration — exactly the + // ADR-071 GOV-U26 scenario ("Release exactly autour de `wait`"). Each + // iteration is bounded by a hard per-iteration timeout, so a real lost + // wakeup shows up as a deterministic failure, never a silent hang — + // matching this project's GV-L7 proof style (`docs/status.md`: "le + // test échoue par timeout déterministe"). + #[test] + fn gov_u26_release_racing_wait_never_loses_the_wakeup() { + let iterations = 200; + for i in 0..iterations { + let config = GovernorConfig { + budget_bytes: 1_000, + max_queue_depth: 4, + max_admission_waiters: 4, + reserved_slots: [1, 1], + reserved_bytes: [999, 1], + weight: [1, 1], + max_intent_peak_bytes: [999, 1], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }; + let d = MemoryDomain::new(config).expect("valid config"); + let governor = d.governor(); + + let hog = governor + .try_acquire_admission( + AdmissionClass::Foreground, + MemoryPlan { + peak_bytes: 999, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + }, + ) + .expect("hog grant"); + + let (tx, rx) = std::sync::mpsc::channel(); + let start = Arc::new(Barrier::new(2)); + + let waiter_governor = Arc::clone(governor); + let waiter_start = Arc::clone(&start); + let waiter = std::thread::spawn(move || { + waiter_start.wait(); + let outcome = waiter_governor.acquire_admission( + AdmissionClass::Foreground, + MemoryPlan { + peak_bytes: 999, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + }, + ); + let _ = tx.send(outcome.is_ok()); + }); + + // No stagger on either side: both threads leave the barrier at + // essentially the same instant, so whether the release lands + // before the waiter enqueues, while it is enqueuing, while it + // is transitioning into `wait()`, or while it is already + // parked is genuinely up to the OS scheduler — and varies + // iteration to iteration. + start.wait(); + drop(hog); + + match rx.recv_timeout(Duration::from_secs(2)) { + Ok(true) => {} + Ok(false) => panic!("iteration {i}: waiter did not receive Ok(permit)"), + Err(_) => panic!( + "iteration {i}: waiter never observed the release within the hard per-iteration timeout — lost wakeup" + ), + } + waiter.join().expect("waiter thread must not panic"); + } + } } From 4b98286adc307061ff0b2524f483e2446e7b2dd3 Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Mon, 17 Aug 2026 19:56:27 +0200 Subject: [PATCH 06/15] =?UTF-8?q?test(V2-04):=20close=20GOV-U21/22/23=20?= =?UTF-8?q?=E2=80=94=20governor=20borrowable-protection=20fairness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-071's global admission governor had zero tests for the borrowable protection guarantee (docs/status.md 2026-08-17 gap list): once a class has an active waiter, no admission from the other class — nor a class-less cache try_reserve — may increase that waiter's byte/slot shortfall (D_b(c)/D_s(c)). Existing borrows drain via release, they are never preempted. Adds three real integration tests against MemoryGovernor/MemoryDomain (no mocks) in memory_governor::admission::tests: - gov_u21_maintenance_borrow_then_foreground_waiter_shortfall_never_grows (GOV-U21, byte protection): Maintenance borrows into Foreground's idle reserve, Foreground parks as a real FIFO waiter, then a further Maintenance admission and two cache try_reserve variants (untagged and Maintenance-tagged) are all refused with the shortfall bit-for-bit unchanged; a single release drains it and Foreground is granted. - gov_u22_maintenance_borrow_then_foreground_waiter_slot_debt_drains_not_grows (GOV-U22, slot protection): the slot-side twin. Numbers are chosen so the refusal is decided by the per-class protection loop itself (1 raw slot still free), not by the earlier blanket outstanding_slots == max_queue_depth guard, so the test cannot pass by accident. Shows the debt shrinking across two releases (2 -> 1 -> 0) before Foreground is granted — "stops growing, then drains". - gov_u23_foreground_borrow_then_maintenance_waiter_shortfall_never_grows (GOV-U23, symmetry): the exact mirror of GOV-U21 with classes swapped, proving the protection logic in scheduler::capacity_eligible is not hardcoded to favor Foreground. Each test was verified as a real oracle: a temporary defect in capacity_eligible (skip protecting a class against a different growing class) was injected, confirmed to turn all three tests red, then reverted (scheduler.rs diff is clean against dev) and reconfirmed green. Also adds MemoryGovernor::bytes_by_class/slots_by_class, test-util-gated accessors mirroring the existing committed_bytes/outstanding_slots pattern, so the tests compute D_b(c)/D_s(c) directly off the governor's real bookkeeping instead of an inferred proxy. No production logic changed — additive test-only surface plus new tests. Co-Authored-By: Claude Sonnet 5 --- .../src/memory_governor/admission.rs | 492 ++++++++++++++++++ 1 file changed, 492 insertions(+) diff --git a/crates/basemyai-engine/src/memory_governor/admission.rs b/crates/basemyai-engine/src/memory_governor/admission.rs index c2f2f16..866591a 100644 --- a/crates/basemyai-engine/src/memory_governor/admission.rs +++ b/crates/basemyai-engine/src/memory_governor/admission.rs @@ -506,6 +506,22 @@ impl MemoryGovernor { pub fn admission_waiters(&self) -> usize { self.lock().waiters.len() } + + /// `bytes_by_class[c]` (ADR-071 §"Protections borrowables") — `test-util` + /// only, same rationale as [`Self::committed_bytes`]: lets GOV-U21/GOV-U23 + /// compute the real `H_b(c)`/`D_b(c)` shortfall formula directly against + /// the governor's own bookkeeping, instead of inferring it indirectly. + #[cfg(any(test, feature = "test-util"))] + pub fn bytes_by_class(&self, class: AdmissionClass) -> usize { + self.lock().bytes_by_class[class.index()] + } + + /// `slots_by_class[c]` — `test-util` only, the slot-side twin of + /// [`Self::bytes_by_class`] used by GOV-U22's `H_s(c)`/`D_s(c)` checks. + #[cfg(any(test, feature = "test-util"))] + pub fn slots_by_class(&self, class: AdmissionClass) -> usize { + self.lock().slots_by_class[class.index()] + } } #[cfg(test)] @@ -1222,4 +1238,480 @@ mod tests { ); assert!(matches!(err, EngineError::AdmissionClosed)); } + + // ---- ADR-071 §"Protections borrowables": GOV-U21/GOV-U22/GOV-U23 ---- + // + // `D_b(c) = max(0, H_b(c) - free_bytes)` and `D_s(c) = max(0, H_s(c) - + // free_slots)`, `H_b(c) = max(0, reserved_bytes[c] - bytes_by_class[c])`, + // `H_s(c) = max(0, reserved_slots[c] - slots_by_class[c])`, verbatim from + // the ADR. The three tests below compute these directly off the real + // `MemoryGovernor`'s `test-util` accessors, not off an inferred proxy. + + /// `D_b(class)` computed the same way `scheduler::capacity_eligible` + /// derives it, off the governor's live bookkeeping. + fn shortfall_bytes( + governor: &MemoryGovernor, + reserved_bytes: usize, + class: AdmissionClass, + budget_bytes: usize, + ) -> usize { + let free_bytes = budget_bytes.saturating_sub(governor.committed_bytes()); + let headroom = reserved_bytes.saturating_sub(governor.bytes_by_class(class)); + headroom.saturating_sub(free_bytes) + } + + /// `D_s(class)`, the slot-side twin of [`shortfall_bytes`]. + fn shortfall_slots( + governor: &MemoryGovernor, + reserved_slots: usize, + class: AdmissionClass, + max_queue_depth: usize, + ) -> usize { + let free_slots = max_queue_depth.saturating_sub(governor.outstanding_slots()); + let headroom = reserved_slots.saturating_sub(governor.slots_by_class(class)); + headroom.saturating_sub(free_slots) + } + + fn plan(peak_bytes: usize) -> MemoryPlan { + MemoryPlan { + peak_bytes, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + } + } + + // GOV-U21 (ADR-071 test spec table, "Protection bytes"): Maintenance + // first borrows deep into Foreground's still-idle reserve — legitimate, + // since ADR-071 explicitly allows borrowing an unwaited class's unused + // reserve ("capacité empruntable"). Foreground then genuinely blocks on + // `acquire_admission`, becoming a real FIFO waiter. From that instant, + // the oracle applies: "aucune nouvelle opération Maintenance/cache ne + // peut augmenter le shortfall Foreground". This test proves it three + // ways — a further Maintenance admission, a class-less cache + // `try_reserve`, and a Maintenance-tagged cache `try_reserve` — are all + // refused, and `D_b(Foreground)` is bit-for-bit unchanged by each + // refusal. It also proves the other half of the oracle: releasing + // (never preempting) Maintenance's existing borrow is what actually + // drains the shortfall, and Foreground is admitted once it reaches 0. + #[test] + fn gov_u21_maintenance_borrow_then_foreground_waiter_shortfall_never_grows() { + const BUDGET: usize = 1_000_000; + const RESERVED_FG: usize = 300_000; + const RESERVED_MN: usize = 100_000; + + let config = GovernorConfig { + budget_bytes: BUDGET, + max_queue_depth: 20, + max_admission_waiters: 10, + reserved_slots: [2, 2], + reserved_bytes: [RESERVED_FG, RESERVED_MN], + weight: [1, 1], + max_intent_peak_bytes: [RESERVED_FG, RESERVED_MN], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }; + let d = MemoryDomain::new(config).expect("valid config"); + let governor = d.governor(); + + // Maintenance borrows 8 * 100_000 = 800_000 while Foreground has no + // waiter yet. + let mut mn_permits = Vec::new(); + for _ in 0..8 { + mn_permits.push( + governor + .try_acquire_admission(AdmissionClass::Maintenance, plan(RESERVED_MN)) + .expect("borrowing while Foreground has no waiter must succeed"), + ); + } + assert_eq!(governor.committed_bytes(), 800_000); + assert_eq!(governor.bytes_by_class(AdmissionClass::Maintenance), 800_000); + + // Foreground now genuinely blocks: only 200_000 bytes are free and it + // asked for 300_000 — proven non-blocking-would-fail below the same + // way, then actually parked via the blocking call on a thread. + let err = expect_err( + governor.try_acquire_admission(AdmissionClass::Foreground, plan(RESERVED_FG)), + "the domain must genuinely lack capacity for Foreground right now", + ); + assert!(matches!(err, EngineError::AdmissionWouldBlock)); + + let governor_for_thread = Arc::clone(governor); + let fg_thread = std::thread::spawn(move || { + governor_for_thread.acquire_admission(AdmissionClass::Foreground, plan(RESERVED_FG)) + }); + + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < 1 { + assert!(std::time::Instant::now() < deadline, "Foreground must park as a waiter"); + std::thread::yield_now(); + } + assert_eq!( + governor.bytes_by_class(AdmissionClass::Foreground), + 0, + "still waiting, not yet granted" + ); + + let shortfall_before = shortfall_bytes(governor, RESERVED_FG, AdmissionClass::Foreground, BUDGET); + assert_eq!( + shortfall_before, 100_000, + "D_b(Foreground) = max(0, 300_000-0) - 200_000" + ); + + // (1) A further Maintenance admission — even a single byte — must be + // refused: any growth shrinks free_bytes and would increase + // D_b(Foreground). + let refused = expect_err( + governor.try_acquire_admission(AdmissionClass::Maintenance, plan(1)), + "a further Maintenance admission must be refused once Foreground is a protected waiter", + ); + assert!(matches!(refused, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_bytes(governor, RESERVED_FG, AdmissionClass::Foreground, BUDGET), + shortfall_before, + "a refused Maintenance admission must not move the shortfall at all" + ); + + // (2) A class-less cache-style `try_reserve` must be refused too — + // exactly the "cache" half of the oracle. + let refused_cache = expect_err( + governor.try_reserve(GovernedMemoryKind::SstDataBlock, None, 1), + "a cache try_reserve must be refused once it would increase Foreground's shortfall", + ); + assert!(matches!(refused_cache, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_bytes(governor, RESERVED_FG, AdmissionClass::Foreground, BUDGET), + shortfall_before + ); + + // (3) A Maintenance-tagged cache reservation must be refused for the + // same reason — the protection is about global free bytes, not about + // who the bytes are attributed to. + let refused_cache_mn = expect_err( + governor.try_reserve(GovernedMemoryKind::SstDataBlock, Some(AdmissionClass::Maintenance), 1), + "a Maintenance-tagged cache reservation must also be refused", + ); + assert!(matches!(refused_cache_mn, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_bytes(governor, RESERVED_FG, AdmissionClass::Foreground, BUDGET), + shortfall_before, + "shortfall must be bit-for-bit unchanged after every refused attempt" + ); + assert_eq!( + governor.committed_bytes(), + 800_000, + "no refused attempt may have mutated committed_bytes" + ); + + // Draining (not preemption) is what actually shrinks the shortfall: + // releasing just 1 of Maintenance's 8 borrowed permits frees 100_000 + // bytes, taking free_bytes from 200_000 to 300_000 — exactly + // Foreground's own reserve, so `D_b(Foreground)` reaches 0. The + // scheduler re-runs synchronously inside this same release (under + // the same lock, before `drop` returns), so Foreground's grant is + // already visible in `bytes_by_class`/`committed_bytes` right here, + // even though the parked `fg_thread` still needs its own wakeup to + // observe it and return. + drop(mn_permits.pop().expect("a permit to drop")); + assert_eq!( + governor.bytes_by_class(AdmissionClass::Foreground), + RESERVED_FG, + "Foreground must have been granted synchronously the instant its shortfall drained to 0" + ); + assert_eq!( + shortfall_bytes(governor, RESERVED_FG, AdmissionClass::Foreground, BUDGET), + 0, + "once granted its full reserve, Foreground's own H_b term is itself 0" + ); + + let permit = fg_thread + .join() + .expect("thread") + .expect("Foreground must eventually be granted once its protection is satisfied"); + assert_eq!(permit.class(), AdmissionClass::Foreground); + drop(permit); + drop(mn_permits); + } + + // GOV-U22 (ADR-071 test spec table, "Protection slots"): the same + // scenario as GOV-U21, but on `outstanding_slots`/`slots_by_class` + // instead of bytes — "dette de slots cesse de croître puis draine". + // Reserved bytes here are deliberately generous so only slots ever + // bind, isolating the slot half of the protection formula. + // + // Numbers are chosen so the "further Maintenance admission refused" + // assertion is actually decided by the per-class protection loop in + // `capacity_eligible`, not by the earlier, unconditional + // `outstanding_slots >= max_queue_depth` guard: at least 1 raw free slot + // must remain globally at that point, so a version of the code that + // dropped the protection loop entirely would grant it (this is exactly + // what the red/green proof below exercises — see this test's `GOV-U22` + // report entry). + #[test] + fn gov_u22_maintenance_borrow_then_foreground_waiter_slot_debt_drains_not_grows() { + const MAX_QUEUE_DEPTH: usize = 10; + const RESERVED_SLOTS_FG: usize = 4; + const RESERVED_SLOTS_MN: usize = 2; + const BUDGET: usize = 10_000_000; + const RESERVED_BYTES: usize = 1_000_000; + const PEAK: usize = 1_000; + + let config = GovernorConfig { + budget_bytes: BUDGET, + max_queue_depth: MAX_QUEUE_DEPTH, + max_admission_waiters: 10, + reserved_slots: [RESERVED_SLOTS_FG, RESERVED_SLOTS_MN], + reserved_bytes: [RESERVED_BYTES, RESERVED_BYTES], + weight: [1, 1], + max_intent_peak_bytes: [PEAK, PEAK], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }; + let d = MemoryDomain::new(config).expect("valid config"); + let governor = d.governor(); + + // Maintenance borrows 8 of the 10 slots while Foreground has no + // waiter yet — legitimate borrowing of Foreground's unused reserve, + // leaving 2 slots genuinely free. + let mut mn_permits = Vec::new(); + for _ in 0..8 { + mn_permits.push( + governor + .try_acquire_admission(AdmissionClass::Maintenance, plan(PEAK)) + .expect("borrowing slots while Foreground has no waiter must succeed"), + ); + } + assert_eq!(governor.outstanding_slots(), 8); + assert_eq!(governor.slots_by_class(AdmissionClass::Maintenance), 8); + + // Foreground now genuinely blocks. Note this cannot be proven via a + // non-blocking `try_acquire_admission` pre-check the way GOV-U21 + // does for bytes: with 2 slots still raw-free and Foreground's own + // waiter queue still empty at the instant of that direct call, the + // per-class protection loop would skip Foreground too (queue-empty + // classes are exactly the "capacité empruntable" case) and the call + // would actually succeed — self-protection against one's own + // request only ever engages once a request is already the head of + // its own non-empty FIFO, i.e. inside `visit()` after + // `acquire_admission` has enqueued it. So this test parks Foreground + // directly and proves the block by polling the real waiter + // bookkeeping instead. + let governor_for_thread = Arc::clone(governor); + let fg_thread = + std::thread::spawn(move || governor_for_thread.acquire_admission(AdmissionClass::Foreground, plan(PEAK))); + + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < 1 { + assert!(std::time::Instant::now() < deadline, "Foreground must park as a waiter"); + std::thread::yield_now(); + } + assert_eq!( + governor.slots_by_class(AdmissionClass::Foreground), + 0, + "still waiting, not yet granted — confirms it genuinely parked rather than being granted instantly" + ); + + let debt_before = shortfall_slots(governor, RESERVED_SLOTS_FG, AdmissionClass::Foreground, MAX_QUEUE_DEPTH); + assert_eq!(debt_before, 2, "D_s(Foreground) = max(0, 4-0) - 2"); + + // A further Maintenance admission must be refused. Crucially, 1 slot + // is still raw-free at this point (`outstanding_slots` = 8 < 10), so + // this refusal can only come from the per-class protection loop + // protecting Foreground's shortfall — not from blanket exhaustion. + assert!( + governor.outstanding_slots() < MAX_QUEUE_DEPTH, + "a raw-free slot must still exist, so the refusal below is a genuine protection decision" + ); + let refused = expect_err( + governor.try_acquire_admission(AdmissionClass::Maintenance, plan(PEAK)), + "a further Maintenance admission must be refused once Foreground is a protected slot waiter", + ); + assert!(matches!(refused, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_slots(governor, RESERVED_SLOTS_FG, AdmissionClass::Foreground, MAX_QUEUE_DEPTH), + debt_before, + "a refused Maintenance admission must not move the slot debt at all" + ); + assert_eq!( + governor.outstanding_slots(), + 8, + "no refused attempt may have mutated outstanding_slots" + ); + + // Cache reservations never take a slot at all (ADR-071 GOV-U28: + // "Cache ne peut réserver" a slot) — `outstanding_slots` staying + // exactly put through a successful cache `try_reserve` is itself the + // proof it cannot possibly increase D_s. + let cache = governor + .try_reserve(GovernedMemoryKind::SstDataBlock, None, 1) + .expect("a slot-free cache reservation must not be blocked by slot protection"); + assert_eq!( + governor.outstanding_slots(), + 8, + "cache reservations never touch outstanding_slots" + ); + drop(cache); + + // Draining: a single released slot only shrinks the debt (2 -> 1), + // it does not yet satisfy Foreground's own post-grant floor + // (`reserved_slots[Foreground] - 1 == 3` still needs 3 free slots + // after granting, only 2 would be available) — releasing a second + // slot is what actually unblocks it, exactly the "cesse de croître + // puis draine" (stops growing, then drains) oracle GOV-U22 names. + drop(mn_permits.pop().expect("first permit to drop")); + assert_eq!(governor.outstanding_slots(), 7); + assert_eq!( + shortfall_slots(governor, RESERVED_SLOTS_FG, AdmissionClass::Foreground, MAX_QUEUE_DEPTH), + 1, + "one released slot must shrink, not eliminate, the debt" + ); + assert_eq!( + governor.slots_by_class(AdmissionClass::Foreground), + 0, + "still not enough to grant Foreground yet" + ); + + drop(mn_permits.pop().expect("second permit to drop")); + assert_eq!( + governor.slots_by_class(AdmissionClass::Foreground), + 1, + "Foreground must have been granted synchronously the instant its slot debt drained to 0" + ); + assert_eq!( + shortfall_slots(governor, RESERVED_SLOTS_FG, AdmissionClass::Foreground, MAX_QUEUE_DEPTH), + 0, + "once granted its own slot, Foreground's own H_s term drops to 3, matched by 3 free slots" + ); + + let permit = fg_thread + .join() + .expect("thread") + .expect("Foreground must eventually be granted once its slot protection is satisfied"); + assert_eq!(permit.class(), AdmissionClass::Foreground); + drop(permit); + drop(mn_permits); + } + + // GOV-U23 (ADR-071 test spec table, "Symétrie"): the exact mirror of + // GOV-U21 with the classes swapped — Foreground borrows into + // Maintenance's idle reserve, Maintenance becomes the waiter. Nothing in + // `scheduler::capacity_eligible` special-cases which class is which (it + // loops over `AdmissionClass::ALL` uniformly), so this must reproduce + // GOV-U21's exact shortfall numbers with the labels swapped — proving + // the protection logic is not accidentally hardcoded to protect + // Foreground only. + #[test] + fn gov_u23_foreground_borrow_then_maintenance_waiter_shortfall_never_grows() { + const BUDGET: usize = 1_000_000; + const RESERVED_FG: usize = 100_000; + const RESERVED_MN: usize = 300_000; + + let config = GovernorConfig { + budget_bytes: BUDGET, + max_queue_depth: 20, + max_admission_waiters: 10, + reserved_slots: [2, 2], + reserved_bytes: [RESERVED_FG, RESERVED_MN], + weight: [1, 1], + max_intent_peak_bytes: [RESERVED_FG, RESERVED_MN], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }; + let d = MemoryDomain::new(config).expect("valid config"); + let governor = d.governor(); + + // Foreground borrows 8 * 100_000 = 800_000 while Maintenance has no + // waiter yet. + let mut fg_permits = Vec::new(); + for _ in 0..8 { + fg_permits.push( + governor + .try_acquire_admission(AdmissionClass::Foreground, plan(RESERVED_FG)) + .expect("borrowing while Maintenance has no waiter must succeed"), + ); + } + assert_eq!(governor.committed_bytes(), 800_000); + assert_eq!(governor.bytes_by_class(AdmissionClass::Foreground), 800_000); + + let err = expect_err( + governor.try_acquire_admission(AdmissionClass::Maintenance, plan(RESERVED_MN)), + "the domain must genuinely lack capacity for Maintenance right now", + ); + assert!(matches!(err, EngineError::AdmissionWouldBlock)); + + let governor_for_thread = Arc::clone(governor); + let mn_thread = std::thread::spawn(move || { + governor_for_thread.acquire_admission(AdmissionClass::Maintenance, plan(RESERVED_MN)) + }); + + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < 1 { + assert!( + std::time::Instant::now() < deadline, + "Maintenance must park as a waiter" + ); + std::thread::yield_now(); + } + assert_eq!( + governor.bytes_by_class(AdmissionClass::Maintenance), + 0, + "still waiting, not yet granted" + ); + + let shortfall_before = shortfall_bytes(governor, RESERVED_MN, AdmissionClass::Maintenance, BUDGET); + assert_eq!( + shortfall_before, 100_000, + "D_b(Maintenance) = max(0, 300_000-0) - 200_000 — identical formula, opposite class" + ); + + let refused = expect_err( + governor.try_acquire_admission(AdmissionClass::Foreground, plan(1)), + "a further Foreground admission must be refused once Maintenance is a protected waiter", + ); + assert!(matches!(refused, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_bytes(governor, RESERVED_MN, AdmissionClass::Maintenance, BUDGET), + shortfall_before, + "a refused Foreground admission must not move Maintenance's shortfall at all" + ); + + let refused_cache = expect_err( + governor.try_reserve(GovernedMemoryKind::SstDataBlock, Some(AdmissionClass::Foreground), 1), + "a Foreground-tagged cache reservation must be refused once it would increase Maintenance's shortfall", + ); + assert!(matches!(refused_cache, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_bytes(governor, RESERVED_MN, AdmissionClass::Maintenance, BUDGET), + shortfall_before, + "shortfall must be bit-for-bit unchanged after every refused attempt" + ); + assert_eq!( + governor.committed_bytes(), + 800_000, + "no refused attempt may have mutated committed_bytes" + ); + + // Symmetric to GOV-U21: releasing just 1 of Foreground's 8 borrowed + // permits frees exactly the 100_000 bytes Maintenance's shortfall + // needed, granting it synchronously inside this same release. + drop(fg_permits.pop().expect("a permit to drop")); + assert_eq!( + governor.bytes_by_class(AdmissionClass::Maintenance), + RESERVED_MN, + "Maintenance must have been granted synchronously the instant its shortfall drained to 0" + ); + assert_eq!( + shortfall_bytes(governor, RESERVED_MN, AdmissionClass::Maintenance, BUDGET), + 0, + "once granted its full reserve, Maintenance's own H_b term is itself 0" + ); + + let permit = mn_thread + .join() + .expect("thread") + .expect("Maintenance must eventually be granted once its protection is satisfied"); + assert_eq!(permit.class(), AdmissionClass::Maintenance); + drop(permit); + drop(fg_permits); + } } From 7494f8a6188a656083ccd80c82b9d59f6d4eb5ab Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Mon, 17 Aug 2026 19:59:13 +0200 Subject: [PATCH 07/15] test(V2-04): close CB-T1d and CB-T2 governed-coordinator coverage gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-072's post-ratification hardening review left three test gaps open before production activation: CB-T1d (4 of 5 CoordinatorOutcome branches lacking governed-reservation coverage), CB-T2 (governor-before-semaphore ordering under real contention, not just immediate refusal), and CB-T5 (out of scope here). CB-T1d: only `Committed` had governed coverage (governed_write_holds_its_admission_across_execute_then_releases_it). Add four tests, each under a real governed WriteCoordinator + MemoryGovernor, asserting governor.committed_bytes() returns to the residency floor: - governed_aborted_write_releases_its_admission_immediately (stale purge epoch, no failpoint needed — rejected before any WAL work) - governed_outcome_unknown_write_releases_its_admission (after_wal_append failpoint) - governed_structural_reopen_required_releases_its_admission (during_generation_directory_sync failpoint; needed a new encrypted_inner_with_options helper so the governed budget can fit a calibrated small memtable) - governed_durable_reopen_required_releases_its_admission (direct CoordinatorOutcome construction against a real permit — this branch's only two producers require engine state to change between staging and install of the same commit, which owner_loop's strict serialization makes structurally unreachable end-to-end; documented in the test) CB-T2: add a_write_parked_on_governor_byte_capacity_never_holds_a_ concurrency_permit, saturating the governor's byte budget so a real submit() call parks on the governor's Condvar, then proving core.permits is untouched both by counter (available_permits() == full capacity) and operationally (acquiring every slot directly succeeds while the call is still parked). Each test's oracle validity was verified by red/green: temporarily inverting release_governor_permit's drop into a leak turned all 6 governed release-assertion tests red (the 4 new ones plus the 2 pre-existing ones); temporarily swapping submit()'s acquisition order turned the new CB-T2 test red (available_permits() dropped from 4 to 3). Both mutations were reverted after confirming red, and the full suite re-verified green. cargo xtask check passes clean on this branch. --- .../src/storage/native_store/coordinator.rs | 366 ++++++++++++++++++ 1 file changed, 366 insertions(+) diff --git a/crates/basemyai/src/storage/native_store/coordinator.rs b/crates/basemyai/src/storage/native_store/coordinator.rs index da8c028..18314f8 100644 --- a/crates/basemyai/src/storage/native_store/coordinator.rs +++ b/crates/basemyai/src/storage/native_store/coordinator.rs @@ -2656,6 +2656,32 @@ mod tests { ) } + /// Like [`encrypted_inner`], but with [`rotating_options`] — the ADR-072 + /// governed structural-rotation test needs a memtable small enough to + /// size a governor budget around, exactly like [`inner_with_options`] + /// does for the unencrypted case. + fn encrypted_inner_with_options(options: basemyai_engine::EngineOptions) -> (tempfile::TempDir, NativeInner) { + let directory = tempfile::tempdir().expect("temporary encrypted engine directory"); + let mut engine = Engine::open_encrypted_with_options(directory.path(), INITIAL_ROTATION_KEY, options) + .expect("encrypted engine opens"); + let vectors = PersistentVectorIndex::open( + &mut engine, + basemyai_engine::VectorIndexParams::with_dim(crate::EMBEDDING_DIM), + ) + .expect("vector index opens"); + let memory = PersistentMemoryIndex::open(&engine).expect("memory index opens"); + ( + directory, + NativeInner { + engine, + vectors, + memory, + graph: PersistentGraph::new(), + fts: PersistentFts::new(), + }, + ) + } + fn structural_intent(writer: &WriteCoordinator, operation: LiveStructural) -> WriteIntent { WriteIntent::LiveStructural { authorization: writer @@ -3193,6 +3219,346 @@ mod tests { ); } + // ADR-072 §A/CB-T1d: the outcome table (§A) prescribes RELEASE for + // `Aborted` (WAL never durably written), and this is the one branch a + // stale purge epoch can trigger with no failpoint at all — the check + // (`current_epoch != expected_epoch`) runs in plain Rust before any WAL + // work starts. `agent_epochs` has no entry for "agent-a" on a fresh + // coordinator, so the current epoch defaults to 0 and any other + // `Some(epoch)` is rejected immediately (`coordinator.rs`'s + // `WriteIntent::MemoryPurgeChunk` arm). + #[tokio::test] + async fn governed_aborted_write_releases_its_admission_immediately() { + let (_directory, state) = inner_with_options(rotating_options()); + let residency = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); + let governor = Arc::clone(domain.governor()); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + assert_eq!( + governor.committed_bytes(), + residency, + "only the live generation's residency is committed before any write" + ); + + let outcome = writer + .submit_outcome(WriteIntent::MemoryPurgeChunk { + authorization: writer + .authorize("agent-a", OperationClass::RepairOrRebuildOrGc) + .expect("purge auth"), + agent: "agent-a".into(), + cursor: None, + expected_epoch: Some(1), + }) + .await + .expect("stale-epoch outcome delivered"); + assert!( + matches!(outcome, CoordinatorOutcome::Aborted { .. }), + "a stale purge epoch must be rejected before WAL, not committed: {outcome:?}" + ); + assert_eq!( + governor.committed_bytes(), + residency, + "an Aborted write must release its per-write reservation back to the residency floor" + ); + assert_eq!(governor.outstanding_slots(), 0); + writer.close().await.expect("an aborted write is not terminal"); + } + + // ADR-072 §A/CB-T1d: `OutcomeUnknown` (the WAL outcome itself is unknown + // — crash at the append/sync boundary) is durability-ambiguous, and the + // outcome table (§A) prescribes RELEASE for it too — the only reservation + // that could receive these bytes (the §B memtable-rotation reservation) + // does not exist yet (see `release_governor_permit`'s own doc comment). + // Reuses `import_unknown_preserves_phase_and_terminalizes`'s failpoint + // technique, under a governed coordinator this time. + #[tokio::test] + async fn governed_outcome_unknown_write_releases_its_admission() { + let _serial = failpoint_lock().await; + failpoint::clear_all(); + let (_directory, state) = inner_with_options(rotating_options()); + let residency = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); + let governor = Arc::clone(domain.governor()); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)); + + failpoint::set("after_wal_append", Action::Error); + let outcome = writer + .import_graph_entity_batch( + "agent-a".into(), + vec![OwnedGraphEntity { + id: "alice".into(), + entity: GraphEntity { + kind: "person".into(), + label: "Alice".into(), + valid_from: 0, + valid_until: None, + source: GraphSource::Import, + }, + }], + ) + .await + .expect("phase-aware import outcome"); + failpoint::clear_all(); + + assert!(matches!( + outcome, + CoordinatorOutcome::OutcomeUnknown { + phase: WalCommitPhase::Append, + .. + } + )); + // `<=`, not `==`, for the same reason as + // `a_governed_handler_panic_still_returns_the_admission`: release is + // synchronous before the caller is answered (deterministic absence), + // but the terminal shutdown this outcome triggers may already be + // racing this line to drop the engine and its residency charge with + // it. + assert!( + governor.committed_bytes() <= residency, + "an OutcomeUnknown write must never leave a phantom per-write reservation" + ); + assert_eq!(writer.close().await, Err(CoordinatorError::Terminal)); + assert_eq!( + governor.committed_bytes(), + 0, + "the residency charge must die with the engine after terminal shutdown" + ); + } + + // ADR-072 §A/CB-T1d: `StructuralReopenRequired` (durable structural + // publish confirmed, RAM mismatch) is the last of the four + // durable-or-ambiguous branches the outcome table (§A) prescribes RELEASE + // for. Reuses `structural_unknown_maps_phase_retains_candidate_and_shuts_ + // reads`'s failpoint technique, under a governed coordinator. + #[tokio::test] + async fn governed_structural_reopen_required_releases_its_admission() { + let _serial = failpoint_lock().await; + failpoint::clear_all(); + let (_directory, state) = encrypted_inner_with_options(rotating_options()); + let residency = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); + let governor = Arc::clone(domain.governor()); + let (reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)); + + failpoint::set("during_generation_directory_sync", Action::Error); + let outcome = writer + .submit(structural_intent(&writer, LiveStructural::KeyFull { secret: secret() })) + .await + .expect("full rotation admitted") + .outcome() + .await + .expect("structural terminal outcome delivered"); + failpoint::clear_all(); + + assert!(matches!( + outcome, + CoordinatorOutcome::StructuralReopenRequired { + phase: StructuralCommitPhase::DirectorySync, + candidate_ownership: StructuralCandidateOwnership::RetainForRecovery, + .. + } + )); + assert!( + governor.committed_bytes() <= residency, + "a StructuralReopenRequired write must never leave a phantom per-write reservation" + ); + assert_eq!(reads.read_sync(|_| ()), Err(CoordinatorError::Terminal)); + assert_eq!(writer.close().await, Err(CoordinatorError::Terminal)); + assert_eq!( + governor.committed_bytes(), + 0, + "the residency charge must die with the engine after terminal shutdown" + ); + } + + // ADR-072 §A/CB-T1d: `DurableReopenRequired` (fsync confirmed, install + // failed) is the one `CoordinatorOutcome` branch this file cannot reach + // end-to-end. Its only two real producers both require engine state to + // change *between* staging a commit and installing it: + // - `install_committed_batch_with`'s own identity/pending-install + // mismatch (`basemyai-engine/src/store/engine/write.rs:200-230`); + // - `StagedMemoryPut`/`StagedForgetChunk`'s `expected_next_vec_id` + // mismatch (`basemyai-engine/src/idx/memory/persistent.rs:271-289`). + // `execute()` holds `NativeInner` synchronously for its whole duration + // with no `.await`/yield point, and `owner_loop` serializes every + // `execute()` call strictly one at a time — no other write can ever + // interleave between staging and install of the same commit. Confirmed + // by grep: neither producer has a test anywhere in this workspace today. + // Per the task's own review guidance this uses direct `CoordinatorOutcome` + // construction against a *real* governed `WriteCoordinator`/ + // `AdmissionPermit` (not a mock) — it exercises `release_governor_permit`'s + // actual classification (CB-04), just without a naturally-reachable + // end-to-end trigger. The `SequenceRange` is minted by a real preceding + // commit rather than invented, since the type has no public constructor + // outside `basemyai-engine` — its value is opaque to + // `release_governor_permit` anyway, which classifies on the outer + // `CoordinatorOutcome` variant alone. + #[tokio::test] + async fn governed_durable_reopen_required_releases_its_admission() { + let (_directory, state) = inner_with_options(rotating_options()); + let residency = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); + let governor = Arc::clone(domain.governor()); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + + let seeded = writer + .graph_entity_batch("agent-a".into(), vec![purge_entity(1)]) + .await + .expect("seed write commits"); + let CoordinatorOutcome::Committed { + sequence_range: Some(sequence_range), + .. + } = seeded + else { + panic!("seed write must be a durable commit with a real sequence range: {seeded:?}"); + }; + assert_eq!( + governor.committed_bytes(), + residency, + "the seed write's own admission must already be back at the residency floor" + ); + + let permit = writer + .acquire_governed_admission(1, 4_096) + .await + .expect("a small plan must be granted") + .expect("a governed coordinator must hand back a real permit"); + assert!( + governor.committed_bytes() > residency, + "the freshly acquired permit must commit bytes on top of the residency floor" + ); + + let outcome: Result = Ok(CoordinatorOutcome::DurableReopenRequired { + request: WriteRequestId { + runtime: writer.core.runtime, + ordinal: 999, + }, + sequence_range, + cause: "synthetic DurableReopenRequired for CB-T1d coverage".to_owned(), + }); + release_governor_permit(Some(permit), &outcome); + + assert_eq!( + governor.committed_bytes(), + residency, + "CB-04: DurableReopenRequired must release the per-write reservation back to the \ + residency floor, matching every other durable-or-ambiguous branch's current release \ + behavior (see `release_governor_permit`'s doc comment on the pending §B hand-off)" + ); + writer.close().await.expect("clean close"); + } + + // ADR-072 §A/CB-T2: step 5 (governor admission) must precede step 6 + // (`core.permits` semaphore) not merely for an *immediate* refusal + // (`a_write_over_the_governor_ceiling_is_refused_before_the_concurrency_ + // semaphore` already proves that) but under real, sustained contention — + // a caller genuinely blocked on the governor's `Condvar` must not be + // holding a semaphore slot while it waits, or it would starve every + // other writer of a resource that has nothing to do with byte capacity. + // + // The governor enforces *strict* intra-class FIFO (`admission.rs`: "never + // jumps ahead of an already-parked blocking waiter"), and every product + // write uses the same `AdmissionClass::Foreground` — so once one caller + // is parked on a saturated class, no other same-class write can be + // admitted past it either, by design. That rules out proving this by + // running other coordinator writes to completion alongside the parked + // one: they would queue behind it and never observe anything. What *can* + // be proven directly is the real invariant CB-T2 is actually about: does + // the parked call hold a `core.permits` slot? `core.permits` is a real + // `Arc`, the exact same object `submit()` acquires from — so + // acquiring every one of its slots directly, from this test, while the + // call is genuinely parked on the real governor, is a direct proof, not + // a mock: if the parked call held even one slot, one of these + // acquisitions would fail. + #[tokio::test] + async fn a_write_parked_on_governor_byte_capacity_never_holds_a_concurrency_permit() { + let (_directory, state) = inner_with_options(rotating_options()); + let budget = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow") + + 2_000_000; + let domain = governed_domain(budget, 900_000); + let governor = Arc::clone(domain.governor()); + let concurrency = 4; + let (_reads, writer) = WriteCoordinator::dormant_governed(state, concurrency, Arc::clone(&governor)); + + // Hog every free byte beyond the bootstrap memtable charge but one — + // same technique as + // `closing_a_governed_coordinator_wakes_a_caller_parked_on_byte_ + // capacity`: no real write's plan can ever fit one byte, so the next + // submitted write is guaranteed to park on the governor's `Condvar`. + let free_before = budget - governor.committed_bytes(); + let hog = governor + .try_reserve( + basemyai_engine::GovernedMemoryKind::FlushWorkingSet, + None, + free_before - 1, + ) + .expect("hogging free budget with no queued waiters to protect must succeed"); + + let log = Arc::new(Mutex::new(Vec::new())); + let intent = record(1, &log); + let parked_writer = writer.clone(); + let mut parked = tokio::spawn(async move { parked_writer.submit(intent).await }); + + let deadline = Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < 1 { + assert!(Instant::now() < deadline, "the write must park on byte capacity"); + tokio::task::yield_now().await; + } + + // The counter-level check first: + assert_eq!( + writer.core.permits.available_permits(), + concurrency, + "a caller parked on governor byte capacity must not hold a core.permits slot" + ); + // ... and the operational one: actually acquire every slot the + // semaphore has, concurrently with the still-parked governor call. + let mut held = Vec::with_capacity(concurrency); + for n in 0..concurrency { + held.push( + Arc::clone(&writer.core.permits) + .try_acquire_owned() + .unwrap_or_else(|error| { + panic!( + "slot {n} of {concurrency} must be free while the governor caller is \ + parked, not held by it: {error}" + ) + }), + ); + } + assert_eq!( + governor.admission_waiters(), + 1, + "the original write must still be parked while every semaphore slot was acquired" + ); + + drop(held); + drop(hog); + let ticket = tokio::time::timeout(Duration::from_secs(10), &mut parked) + .await + .expect("freeing the hogged budget must unpark the write, not hang") + .expect("the spawned task itself must not panic or be cancelled") + .expect("the parked write is admitted once budget frees"); + ticket.outcome().await.expect("parked write commits"); + assert_eq!(*log.lock().expect("test log"), vec![1]); + writer.close().await.expect("clean close"); + } + // ADR-072 §B / CB-05, bootstrap arm: `Engine::open` mints a first memtable // that *no* rotation ever produces, so `execute()`'s before/after // comparison can never see it. It must therefore be charged at From 4e9465003ac4aeed8884528fed1f74f7b070478a Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Mon, 17 Aug 2026 20:03:03 +0200 Subject: [PATCH 08/15] test(V2-04): CB-T5 bounded-release-window proof + GOV-RSS01-03 governor oracle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-072 CB-T5 (never-reproduced): reproduces the characterised MG-15/16 deviation at store/engine/flush.rs:560 (memtable-generation charge released at flush-completion, not the true last Arc drop) using a real concurrent reader (Engine::snapshot(), the exact mechanism Engine::get uses internally) held deliberately across a full flush_memtables_only() call — guaranteed overlap by construction, two genuine OS threads, no failpoint needed. Proves both a delta (dropping the reader removes exactly one strong reference) and an absolute floor (post-drop strong count returns to exactly 1, the test's own witness) so a permanent leak can't hide behind the delta alone. Verified red under a deliberately injected leak in flush.rs (reverted, confirmed via empty git diff) and green on the real, unmodified code — CB-07's window is bounded exactly as ADR-072 documents; no production bug found. ADR-071 GOV-RSS01-03 (generic oracle, not the separate ADR-072 memtable- rotation-specific one, which stays open): extends the ADR-053/D-6 flatness methodology to memory_governor itself via a small governed LRU cache built directly on try_reserve/Governed. GOV-RSS01 shows RSS and committed bytes plateau across growing volumes (extent ~8KB against a 24MB provisional slack). GOV-RSS03 proves charged = resident + pinned_only under concurrent pinning readers racing eviction. GOV-RSS02 is the negative control, gated behind a new governor-rss-negative-control-leak feature (never default): introduces a real leak invisible to the governor's own committed_bytes() (which stays perfectly flat) but visible in RSS (extent ~216MB, ordered across volumes) — proving the RSS assertion is load-bearing, not a rubber stamp. An initial version scoped the leak sink to the cache instance and silently self-healed on drop; fixed by moving it to a process-lifetime static. Adds MemtableChargeHandle::strong_count() (test-util only) as the witness primitive; both new [[test]] binaries registered in ENGINE_TESTS (Light gate). cargo xtask check green; cargo test -p basemyai-engine --features test-util --lib green (610 tests). Co-Authored-By: Claude Sonnet 5 --- crates/basemyai-engine/Cargo.toml | 15 + crates/basemyai-engine/src/store/memtable.rs | 12 + .../governor/cb_t5_bounded_release_window.rs | 286 ++++++++++ .../tests/governor/gov_rss_oracle.rs | 514 ++++++++++++++++++ xtask/src/main.rs | 10 + 5 files changed, 837 insertions(+) create mode 100644 crates/basemyai-engine/tests/governor/cb_t5_bounded_release_window.rs create mode 100644 crates/basemyai-engine/tests/governor/gov_rss_oracle.rs diff --git a/crates/basemyai-engine/Cargo.toml b/crates/basemyai-engine/Cargo.toml index 5bed9c8..eb590c9 100644 --- a/crates/basemyai-engine/Cargo.toml +++ b/crates/basemyai-engine/Cargo.toml @@ -18,6 +18,13 @@ test-util = [] # Témoin négatif D-6 d'ADR-053 : réintroduit volontairement un manifeste # d'index plat et non borné. Jamais activé par défaut ni par un build produit. unbounded-index-oracle = ["test-util"] +# Témoin négatif GOV-RSS02 (ADR-071) : réintroduit volontairement, dans le +# harnais de test `gov_rss_oracle` lui-même (jamais dans le governor ou un +# cache réel), une fuite proportionnelle au volume que la comptabilité du +# governor ne peut pas voir (une copie orpheline hors réservation, retenue à +# côté de l'éviction correctement comptée). Jamais activé par défaut ni par +# un build produit. +governor-rss-negative-control-leak = ["test-util"] [[bin]] name = "crash_writer" @@ -267,5 +274,13 @@ path = "tests/compaction/generation_gc_retry.rs" name = "generation_pointer_loss_is_rejected_and_gen1_survives" path = "tests/compaction/generation_pointer_loss_is_rejected_and_gen1_survives.rs" +[[test]] +name = "cb_t5_bounded_release_window" +path = "tests/governor/cb_t5_bounded_release_window.rs" + +[[test]] +name = "gov_rss_oracle" +path = "tests/governor/gov_rss_oracle.rs" + [lints] workspace = true diff --git a/crates/basemyai-engine/src/store/memtable.rs b/crates/basemyai-engine/src/store/memtable.rs index 77cb440..70626c4 100644 --- a/crates/basemyai-engine/src/store/memtable.rs +++ b/crates/basemyai-engine/src/store/memtable.rs @@ -133,6 +133,18 @@ impl MemtableChargeHandle { .as_ref() .map(MemoryReservation::bytes) } + + /// Test-only observability (ADR-072 CB-T5): the number of live strong + /// references to this generation's `Arc` right now — a witness + /// for proving the CB-07 release-window deviation stays bounded, never a + /// decision input (production code must never branch on + /// `Arc::strong_count`, which Rust documents as racy the moment more + /// than one thread can observe it — ADR-071 §"Pinning `Arc`", MG-14). + #[cfg(any(test, feature = "test-util"))] + #[must_use] + pub fn strong_count(&self) -> usize { + Arc::strong_count(&self.0) + } } impl Memtable { diff --git a/crates/basemyai-engine/tests/governor/cb_t5_bounded_release_window.rs b/crates/basemyai-engine/tests/governor/cb_t5_bounded_release_window.rs new file mode 100644 index 0000000..c727b93 --- /dev/null +++ b/crates/basemyai-engine/tests/governor/cb_t5_bounded_release_window.rs @@ -0,0 +1,286 @@ +// SPDX-License-Identifier: BUSL-1.1 +//! ADR-072 `CB-T5` — proves the documented `MG-15`/`MG-16` deviation at +//! `store::engine::flush.rs:560` (the memtable generation's governed charge +//! is released at flush-completion, not the true last `Arc` drop) +//! stays bounded to the lifetime of one concurrent, in-flight `Engine::get` +//! reference — never latent — exactly the claim in ADR-072 +//! §"Dérogation documentée à MG-15/16": "fenêtre bornée à la durée d'un seul +//! appel `Engine::get` synchrone en mémoire, jamais non-bornée". +//! +//! ## Reproduction strategy +//! +//! `Engine::snapshot()` (`store/engine/mod.rs`) is *exactly* the mechanism +//! `Engine::get` uses internally (`store/engine/read.rs:23`: `let snapshot = +//! self.snapshot();`) — it clones `Arc`, which in turn holds +//! `Arc` clones for whatever is currently mutable/sealed, and +//! drops that clone when the local variable goes out of scope. `Engine::get` +//! does this and returns within one synchronous call; nothing about the +//! mechanism differs if a caller holds the same public `ReadSnapshot` type +//! deliberately for a controlled window instead. +//! +//! This test takes a snapshot with the public API *before* triggering a +//! flush and holds it deliberately across the *entire* +//! `Engine::flush_memtables_only()` call. That call blocks the test thread +//! while the engine's own `basemyai-flush` background worker thread (spawned +//! by `FlushWorker::spawn`) does the actual sealed-memtable write and the +//! charge release at `flush.rs:560` — so two real, independent OS threads +//! are genuinely involved, and because the snapshot's lifetime spans the +//! *whole* flush call, the overlap with the release site is guaranteed by +//! construction, not left to a timing race. This is deliberately not a +//! probabilistic reproduction: ADR-072's own point 4 ("Snapshots — cas +//! prouvé, pas théorique") already identifies this exact call shape +//! (`WriteIntent::MemoryUpdate` → `Engine::get` → a local `ReadSnapshot`) as +//! the one site the deviation is characterised against, so pinning the +//! window by construction proves the same property the ADR claims without +//! needing a failpoint to hit a lucky interleaving. +//! +//! A second, independent `MemtableChargeHandle` (`witness`, ADR-072 §B's +//! public rotation-tracking handle) is kept alive purely as an +//! `Arc::strong_count` witness (test-util only — see +//! `MemtableChargeHandle::strong_count`'s doc comment: production code must +//! never branch on this count, only observe it). It proves the window's +//! *bound*: strong count returns to exactly its pre-overlap baseline the +//! instant the concurrent snapshot is dropped, with nothing else left +//! holding the generation alive. + +use basemyai_engine::{AdmissionClass, Engine, EngineOptions, GovernedMemoryKind, GovernorConfig, MemoryDomain}; + +fn charge_domain(budget_bytes: usize) -> MemoryDomain { + MemoryDomain::new(GovernorConfig { + budget_bytes, + max_queue_depth: 8, + max_admission_waiters: 8, + reserved_slots: [2, 2], + reserved_bytes: [budget_bytes / 2, budget_bytes / 4], + weight: [1, 1], + max_intent_peak_bytes: [budget_bytes / 2, budget_bytes / 4], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }) + .expect("valid governor config") +} + +/// Seal is driven explicitly by `flush_memtables_only()`, never by the +/// ordinary threshold/target-bytes auto-seal path — this isolates the one +/// seal+flush this test cares about. +fn explicit_seal_options() -> EngineOptions { + EngineOptions { + memtable_flush_threshold: usize::MAX, + memtable_target_bytes: usize::MAX, + ..EngineOptions::default() + } +} + +/// `CB-T5`: reproduces the `CB-07` release window and proves it collapses +/// exactly when the concurrent reference (the `Engine::get`-equivalent +/// snapshot) is dropped — never later, never left dangling. +#[test] +fn cb_t5_release_window_is_bounded_to_the_concurrent_readers_lifetime() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut engine = Engine::open_with_options(dir.path(), explicit_seal_options()).expect("open"); + + let domain = charge_domain(1_000_000); + let governor = domain.governor(); + const CHARGE_BYTES: usize = 200_000; + + // Attach the generation's charge exactly the way `basemyai`'s + // owner_loop does at rotation (ADR-072 §B / CB-05): reserve, attach via + // a handle, then drop that handle — production never retains it past + // the attach call itself. + let reservation = governor + .try_reserve( + GovernedMemoryKind::WriteBufferMutable, + Some(AdmissionClass::Foreground), + CHARGE_BYTES, + ) + .expect("reserve the generation's worst-case charge"); + { + let attach_handle = engine.active_memtable_charge_handle(); + attach_handle + .attach(reservation) + .map_err(|_| ()) + .expect("a fresh generation accepts its charge"); + } + + let key = b"cb-t5-key"; + let value = vec![0x42u8; 4096]; + engine.put(key, &value).expect("seed the generation with real data"); + + // A second, independent handle onto the *same* generation, kept alive + // only as an `Arc::strong_count` witness — never consulted by the engine + // or the governor to decide anything, exactly like a test's `Weak` + // upgrade check would be. Its *absolute* count is not meaningful (the + // engine itself holds more than one internal reference to its own + // current memtable — e.g. both the `Engine::memtable` field and the + // published `SuperVersion`'s `mutable()` field — even with no concurrent + // reader involved at all); only the *delta* the concurrent snapshot adds + // and removes is what CB-T5 needs to prove. + let witness = engine.active_memtable_charge_handle(); + assert!( + witness.same_memtable_as(&engine.active_memtable_charge_handle()), + "no rotation has happened yet" + ); + + let committed_before_flush = governor.committed_bytes(); + assert!( + committed_before_flush >= CHARGE_BYTES, + "the generation's charge must be committed before flush" + ); + + // The "concurrent Engine::get" stand-in: `Engine::snapshot()` is exactly + // what `Engine::get` builds and drops internally. Taking it here and + // holding it deliberately across the whole flush call reproduces + // ADR-072 point 4's scenario with a real extra `Arc` clone, + // deterministically overlapping the release at `flush.rs:560`. + let concurrent_snapshot = engine.snapshot(); + + // Runs the actual seal + SST write + catalogue publish + charge release + // on the engine's own background `basemyai-flush` worker thread — + // genuine cross-thread concurrency with `concurrent_snapshot`, which is + // held here on this thread's stack, completely untouched by the flush + // call itself. + engine + .flush_memtables_only() + .expect("flush must complete while a concurrent reader still holds the sealed generation"); + + // --- The deviation, reproduced --- + // The governor already released the charge at flush-completion + // (flush.rs:560), *unconditionally*, regardless of `concurrent_snapshot` + // still holding a live `Arc` to the very same generation. + let committed_after_flush = governor.committed_bytes(); + assert_eq!( + committed_after_flush, + committed_before_flush - CHARGE_BYTES, + "CB-07: the charge must already be released at flush-completion, independent of the \ + concurrent snapshot still pinning the same memtable generation" + ); + + // Real memory is still resident and reachable through the concurrent + // reference — this *is* the transient undercount CB-07 documents: the + // governor already reports fewer committed bytes than what is genuinely + // still allocated and reachable. + assert_eq!( + concurrent_snapshot + .get(key) + .expect("read through the concurrent snapshot"), + Some(value), + "the flushed generation's data must still be resident and reachable through the \ + concurrent snapshot even after the governor has already released its charge" + ); + + // The witness's strong count right now, post-flush, with + // `concurrent_snapshot` still alive — captured only to compute the delta + // below, never compared to an absolute number (see the field comment + // above for why an absolute baseline is not meaningful here). + let strong_count_during_overlap = witness.strong_count(); + + // --- The bound, proved --- + // Dropping the concurrent reference is exactly what a synchronous + // `Engine::get` call returning does internally. The window must + // collapse *here*, not at some later, unbounded point: strong count must + // drop by *exactly* one — the concurrent snapshot's own contribution, + // and nothing else lingering (which would mean some other + // engine-internal referent, e.g. a stale `published.sealed` entry or the + // flush job's own `Arc`, was also still alive, i.e. the deviation's real + // window is wider than the one documented site). + drop(concurrent_snapshot); + let strong_count_after_drop = witness.strong_count(); + assert_eq!( + strong_count_during_overlap, + strong_count_after_drop + 1, + "CB-T5: dropping the concurrent reader's reference must reduce strong references by \ + exactly one — a smaller or larger drop would mean the CB-07 deviation is NOT bounded to \ + one synchronous Engine::get call, contrary to what ADR-072 documents \ + (during={strong_count_during_overlap}, after={strong_count_after_drop})" + ); + // Absolute check, not just a delta: once the concurrent reader is gone, + // `witness` must be the *only* live reference left — exactly 1. This is + // what actually catches a permanent leak inside the flush path itself + // (e.g. a bug that forgets to drop `FlushJob::memtable` and instead + // keeps it alive forever): such a bug would still pass the delta check + // above (both `during` and `after` would be off by the same constant), + // but not this one. + assert_eq!( + strong_count_after_drop, 1, + "CB-T5: after the concurrent reader drops and no external caller is involved at all, the \ + flushed generation's only remaining strong reference must be this test's own witness \ + handle — anything else means the flush path itself is leaking a reference beyond the \ + documented, bounded CB-07 deviation" + ); + + // Governor accounting was never touched again by any of this — it + // dropped exactly once, at flush-completion, and stays there regardless + // of what happens to the concurrent reader afterward. + assert_eq!( + governor.committed_bytes(), + committed_after_flush, + "governor accounting must not move again just because the concurrent reader dropped" + ); +} + +/// `CB-T5` companion: repeats the same overlap `N` times in a tight loop +/// (fresh generation, fresh concurrent snapshot each iteration) to rule out +/// the window only *happening* to collapse once by luck of allocator/thread +/// scheduling. Every iteration must show the same exactly-one-extra-then- +/// zero-extra shape. +#[test] +fn cb_t5_release_window_is_bounded_across_many_repeated_generations() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut engine = Engine::open_with_options(dir.path(), explicit_seal_options()).expect("open"); + let domain = charge_domain(10_000_000); + let governor = domain.governor(); + const CHARGE_BYTES: usize = 8_192; + const ITERATIONS: usize = 25; + + for iteration in 0..ITERATIONS { + let reservation = governor + .try_reserve( + GovernedMemoryKind::WriteBufferMutable, + Some(AdmissionClass::Foreground), + CHARGE_BYTES, + ) + .unwrap_or_else(|error| panic!("iteration {iteration}: reserve charge: {error:?}")); + { + let attach_handle = engine.active_memtable_charge_handle(); + attach_handle + .attach(reservation) + .map_err(|_| ()) + .unwrap_or_else(|()| panic!("iteration {iteration}: attach charge")); + } + let key = format!("cb-t5-loop-key-{iteration}"); + engine + .put(key.as_bytes(), b"v") + .unwrap_or_else(|error| panic!("iteration {iteration}: put: {error:?}")); + + let witness = engine.active_memtable_charge_handle(); + let before = governor.committed_bytes(); + + let snapshot = engine.snapshot(); + engine + .flush_memtables_only() + .unwrap_or_else(|error| panic!("iteration {iteration}: flush: {error:?}")); + + assert_eq!( + governor.committed_bytes(), + before - CHARGE_BYTES, + "iteration {iteration}: charge must release exactly at flush-completion" + ); + let during_overlap = witness.strong_count(); + + drop(snapshot); + let after_drop = witness.strong_count(); + assert_eq!( + during_overlap, + after_drop + 1, + "iteration {iteration}: dropping the concurrent reader must reduce strong references \ + by exactly one, every iteration, not just the first \ + (during={during_overlap}, after={after_drop})" + ); + assert_eq!( + after_drop, 1, + "iteration {iteration}: after the concurrent reader drops, only this iteration's \ + witness handle must remain live — a leaked reference here would still pass the delta \ + check above, which is exactly why this absolute check exists too" + ); + } +} diff --git a/crates/basemyai-engine/tests/governor/gov_rss_oracle.rs b/crates/basemyai-engine/tests/governor/gov_rss_oracle.rs new file mode 100644 index 0000000..bb8f661 --- /dev/null +++ b/crates/basemyai-engine/tests/governor/gov_rss_oracle.rs @@ -0,0 +1,514 @@ +// SPDX-License-Identifier: BUSL-1.1 +//! `GOV-RSS01..03` (ADR-071 §"Tests normatifs, oracle RSS et critères de +//! ratification") — the *generic* governor RSS oracle, extending the +//! ADR-053/D-6 flatness methodology (`tests/engine/r6_allocator_peak.rs`, +//! `memory_bounds::r6_memory_bounds`) to `memory_governor`'s own accounting +//! rather than the compaction-merge allocator it was originally built for. +//! +//! **Scope note (explicit, per the task that added this file):** this closes +//! `GOV-RSS01..03` as ADR-071 states them — generic properties of +//! `MemoryGovernor`/`Governed` under growing volume, a negative control, +//! and pinned-reader residency. It does **not** close the separate, still- +//! open oracle ADR-072 §"Points ouverts" calls out: "Oracle RSS/stress dédié +//! à l'écart `governed_committed_bytes` vs mémoire réelle du mécanisme §B" — +//! that one is specific to repeated *memtable-generation* rotation inside +//! `basemyai-engine`'s real write path (`CB-05`/`CB-06`/`CB-07`) and requires +//! a real `Engine` driven through many rotations, which this file does not +//! attempt. +//! +//! ## Harness shape +//! +//! A small governed LRU-style cache (`GovernedRing`) is built directly on +//! `memory_governor`'s public API — `MemoryDomain`, `MemoryGovernor::try_reserve`, +//! `Governed`/`SharedGoverned` — exactly the shape ADR-071 documents a +//! real cache using it would take (§"Reclaim": "Une insertion cache n'attend +//! jamais indéfiniment... bypass"). Fixed config (cache capacity, reader +//! concurrency) and growing *volume* (total insert/evict operations) is the +//! same axis ADR-053's flatness oracle already established +//! (`docs/adr/ADR-053-...md`, `memory_bounds::r6_memory_bounds`): a bounded +//! working set must not grow with dataset/operation volume, only with +//! configuration and concurrency. +//! +//! RSS is sampled with the same native-API shape as +//! `tests/engine/r6_allocator_peak.rs`'s `process_memory` (Windows +//! `K32GetProcessMemoryInfo`, Linux `/proc/self/status`, macOS +//! `proc_pid_rusage`), simplified to self-process sampling since this +//! oracle does not need r6's cross-process allocator-category isolation — +//! `memory_governor` has no global-allocator category instrumentation to +//! isolate, so an in-process, real-heap-backed cache plus periodic RSS +//! samples of this same process is a faithful, much cheaper measurement. +//! +//! ## Slack thresholds +//! +//! `RSS_FLATNESS_SLACK_BYTES` below is a first, deliberately generous +//! provisional threshold for this new oracle — not yet the output of a +//! dedicated per-platform calibration campaign the way +//! `rss_flatness_min_bytes` in `r6_allocator_peak.rs` was (ADR-053 §"Les +//! seuils numériques de slack ne sont pas inventés ici"). ADR-071 explicitly +//! defers that campaign ("Les seuils numériques de slack ne sont pas +//! inventés ici. Ils sont mesurés par plateforme avant activation par +//! défaut") — this file's thresholds should be revisited by that same future +//! campaign, on this platform (Windows, per `docs/status.md`) and others. + +use std::collections::VecDeque; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Barrier, Mutex}; +use std::thread; + +use basemyai_engine::{Governed, GovernedMemoryKind, GovernorConfig, MemoryDomain, MemoryGovernor, SharedGoverned}; + +/// One governed cache entry's payload size. Deliberately real heap bytes +/// (not a zero-sized marker) so a genuine leak would actually show up in +/// process RSS, not just in a counter. +const ENTRY_BYTES: usize = 8 * 1024; +/// Fixed cache capacity — a *configuration* constant, never derived from +/// volume. This, plus `READER_CONCURRENCY`, is the entire analytical bound +/// on resident + pinned governed bytes. +const CACHE_CAPACITY_ENTRIES: usize = 128; +/// Fixed concurrent-pinned-reader count (GOV-RSS03) — also configuration, +/// not volume. +const READER_CONCURRENCY: usize = 6; + +#[derive(Debug, Clone, Copy)] +struct ProcessMemory { + rss_bytes: u64, +} + +#[cfg(target_os = "windows")] +fn process_memory() -> Option { + use windows_sys::Win32::System::ProcessStatus::{K32GetProcessMemoryInfo, PROCESS_MEMORY_COUNTERS}; + use windows_sys::Win32::System::Threading::GetCurrentProcess; + + // SAFETY: `GetCurrentProcess` returns a valid pseudo-handle that never + // needs closing; `counters` is exactly the struct size the API expects. + unsafe { + let handle = GetCurrentProcess(); + let mut counters = PROCESS_MEMORY_COUNTERS { + cb: std::mem::size_of::() as u32, + ..std::mem::zeroed() + }; + let ok = K32GetProcessMemoryInfo(handle, &raw mut counters, counters.cb) != 0; + ok.then_some(ProcessMemory { + rss_bytes: counters.WorkingSetSize as u64, + }) + } +} + +#[cfg(target_os = "linux")] +fn process_memory() -> Option { + let text = std::fs::read_to_string("/proc/self/smaps_rollup") + .ok() + .or_else(|| std::fs::read_to_string("/proc/self/status").ok())?; + let kib = |name: &str| -> Option { + let line = text.lines().find(|line| line.starts_with(name))?; + line.split_whitespace().nth(1)?.parse::().ok() + }; + let rss_kib = kib("Rss:").or_else(|| kib("VmRSS:"))?; + Some(ProcessMemory { + rss_bytes: rss_kib * 1024, + }) +} + +#[cfg(target_os = "macos")] +fn process_memory() -> Option { + #[repr(C)] + struct RusageInfoV4 { + uuid: [u8; 16], + fields: [u64; 35], + } + + impl Default for RusageInfoV4 { + fn default() -> Self { + Self { + uuid: [0; 16], + fields: [0; 35], + } + } + } + + #[link(name = "proc")] + unsafe extern "C" { + fn proc_pid_rusage(pid: i32, flavor: i32, buffer: *mut std::ffi::c_void) -> i32; + } + const RUSAGE_INFO_V4: i32 = 4; + + // SAFETY: `usage` has the exact public rusage_info_v4 C layout and + // remains live for the duration of the call. + unsafe { + let mut usage = RusageInfoV4::default(); + let pid = std::process::id() as i32; + (proc_pid_rusage(pid, RUSAGE_INFO_V4, (&raw mut usage).cast()) == 0).then_some(ProcessMemory { + rss_bytes: usage.fields[6], + }) + } +} + +/// A minimal governed LRU-style cache, built directly on `memory_governor`'s +/// public RAII API — the shape ADR-071 documents a real cache using it would +/// take. Insertion never blocks (§"Reclaim": a cache bypasses rather than +/// waits); eviction drops the oldest entry's `SharedGoverned`, releasing its +/// `MemoryReservation` exactly when the last `Arc` referencing it (this +/// ring's own slot, plus any pinning reader) actually drops (MG-14/MG-15). +/// GOV-RSS02 negative-control-only leak sink. Deliberately a **process- +/// lifetime `static`**, not a `GovernedRing` field: the whole point of the +/// negative control is a leak that outlives any one scenario run (a real bug +/// would not politely clean itself up when the cache it leaked out of goes +/// out of scope). An earlier version of this harness scoped the sink to the +/// ring instance and `drop(ring)` at the end of each `run_scenario` call +/// silently freed it again — the RSS extent measured only ~800 KiB instead +/// of the ~150 MiB the leak should have produced across the volume sweep, +/// which caught this harness bug rather than proving anything about a real +/// one. Fixed by moving the sink here. +#[cfg(feature = "governor-rss-negative-control-leak")] +static NEGATIVE_CONTROL_LEAK_SINK: Mutex>> = Mutex::new(Vec::new()); + +struct GovernedRing { + governor: Arc, + capacity: usize, + entries: Mutex>>>, +} + +impl GovernedRing { + fn new(governor: &Arc, capacity: usize) -> Self { + Self { + governor: Arc::clone(governor), + capacity, + entries: Mutex::new(VecDeque::with_capacity(capacity)), + } + } + + /// Inserts one fresh `ENTRY_BYTES`-sized entry, evicting the oldest one + /// if the ring is already at `capacity`. Returns the fresh entry's + /// handle so a caller can optionally pin it (GOV-RSS03). + /// + /// Bypasses (returns `None`, inserting nothing) if the governor has no + /// room right now — the correct cache contract (ADR-071 §"Reclaim"), + /// never a panic or a block. + fn insert(&self, seed: u8) -> Option>> { + let reservation = self + .governor + .try_reserve(GovernedMemoryKind::SstDataBlock, None, ENTRY_BYTES) + .ok()?; + let payload = vec![seed; ENTRY_BYTES]; + let shared: SharedGoverned> = Arc::new(Governed::new(payload, reservation)); + let mut entries = self.entries.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + if entries.len() >= self.capacity + && let Some(evicted) = entries.pop_front() + { + self.retire(evicted); + } + entries.push_back(Arc::clone(&shared)); + Some(shared) + } + + /// The correctly-governed release path: dropping the evicted `Arc` + /// releases its `MemoryReservation` the instant no pinning reader holds + /// another clone (MG-15) — real memory genuinely returns to the + /// allocator once the last reference goes. + #[cfg(not(feature = "governor-rss-negative-control-leak"))] + fn retire(&self, evicted: SharedGoverned>) { + drop(evicted); + } + + /// GOV-RSS02 negative control: still drops the governed `Arc` — so + /// `MemoryGovernor::committed_bytes()` stays perfectly bounded and would + /// tell an operator everything is fine — but *also* retains an + /// unrelated, ungoverned raw copy of the same bytes forever. This is + /// exactly the failure mode an RSS oracle exists to catch and a + /// governor-internal-only oracle structurally cannot: a real allocation + /// the reservation accounting never even knew about. + #[cfg(feature = "governor-rss-negative-control-leak")] + fn retire(&self, evicted: SharedGoverned>) { + let copy: Vec = (**evicted).clone(); + NEGATIVE_CONTROL_LEAK_SINK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .push(copy); + drop(evicted); + } + + fn committed_bytes(&self) -> usize { + self.governor.committed_bytes() + } +} + +fn tiny_domain(budget_bytes: usize, max_queue_depth: usize) -> MemoryDomain { + MemoryDomain::new(GovernorConfig { + budget_bytes, + max_queue_depth, + max_admission_waiters: max_queue_depth, + reserved_slots: [1, 1], + reserved_bytes: [budget_bytes / 2, budget_bytes / 2], + weight: [1, 1], + max_intent_peak_bytes: [budget_bytes / 2, budget_bytes / 2], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }) + .expect("valid governor config") +} + +/// Runs `volume` sequential insert/evict operations through a +/// fixed-capacity `GovernedRing`, with `READER_CONCURRENCY` background +/// threads each continuously pinning (`Arc::clone`-ing) the most recent +/// entry they can see for a short window before dropping it again — +/// simulating concurrent readers racing eviction (GOV-RSS03), fixed in +/// count regardless of `volume`. +/// +/// Returns the process RSS sampled once quiescent (all reader threads +/// joined, all evictions settled) and the governor's own peak +/// `committed_bytes` observed during the run. +fn run_scenario(volume: usize) -> (u64, usize) { + // Budget generous enough that only the *cache's own* capacity/reader + // bound ever limits it — never the volume. + let budget_bytes = (CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY + 8) * ENTRY_BYTES * 4; + let domain = tiny_domain(budget_bytes, CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY + 8); + let governor = Arc::clone(domain.governor()); + let ring = Arc::new(GovernedRing::new(&governor, CACHE_CAPACITY_ENTRIES)); + + let stop = Arc::new(AtomicBool::new(false)); + let latest: Arc>>>> = Arc::new(Mutex::new(None)); + let start_barrier = Arc::new(Barrier::new(READER_CONCURRENCY + 1)); + + let readers: Vec<_> = (0..READER_CONCURRENCY) + .map(|_| { + let stop = Arc::clone(&stop); + let latest = Arc::clone(&latest); + let start_barrier = Arc::clone(&start_barrier); + thread::spawn(move || { + start_barrier.wait(); + while !stop.load(Ordering::Acquire) { + let pinned = latest.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone(); + if let Some(pinned) = pinned { + // Hold the pin briefly — long enough to genuinely + // overlap eviction races, short enough that the + // sweep below still finishes promptly. + std::hint::black_box(&*pinned); + thread::yield_now(); + drop(pinned); + } else { + thread::yield_now(); + } + } + }) + }) + .collect(); + + start_barrier.wait(); + let mut peak_committed = ring.committed_bytes(); + for i in 0..volume { + let seed = (i % 251) as u8; + if let Some(fresh) = ring.insert(seed) { + *latest.lock().unwrap_or_else(std::sync::PoisonError::into_inner) = Some(fresh); + } + peak_committed = peak_committed.max(ring.committed_bytes()); + } + + stop.store(true, Ordering::Release); + for reader in readers { + reader.join().expect("reader thread"); + } + // Drop the last pinned handle and let every remaining ring entry go out + // of scope with `ring` itself, so the quiescent sample below reflects a + // genuinely settled state, not readers still mid-pin. + *latest.lock().unwrap_or_else(std::sync::PoisonError::into_inner) = None; + drop(ring); + drop(domain); + + // A few yields to let the allocator actually return freed pages before + // sampling — RSS (unlike a heap counter) can lag a `drop()` briefly on + // some platforms. + for _ in 0..8 { + thread::yield_now(); + } + let sample = process_memory().expect("native RSS sample must be available on a supported platform"); + (sample.rss_bytes, peak_committed) +} + +/// `GOV-RSS01`: extends ADR-053's flatness methodology to the governor. +/// Same fixed cache capacity/reader concurrency, growing operation volume — +/// RSS extent across volumes must stay within a calibrated slack, not grow +/// with volume. +#[test] +#[cfg_attr( + feature = "governor-rss-negative-control-leak", + ignore = "GOV-RSS01 is the positive case; the negative control below is what runs under this feature" +)] +fn gov_rss01_committed_and_rss_stay_flat_across_growing_volumes() { + const VOLUMES: [usize; 3] = [500, 4_000, 20_000]; + // Provisional slack (see module doc): generous relative to + // `ENTRY_BYTES * (CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY)` — about + // 1.05 MiB analytically for this config — pending a dedicated per- + // platform calibration campaign, matching ADR-071's own deferral of + // numeric slack values. + const RSS_FLATNESS_SLACK_BYTES: u64 = 24 * 1024 * 1024; + + let analytical_bound_bytes = ((CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY) * ENTRY_BYTES) as u64; + + let mut rss_points = Vec::new(); + let mut committed_points = Vec::new(); + for &volume in &VOLUMES { + let (rss_bytes, peak_committed) = run_scenario(volume); + eprintln!( + "GOV-RSS01 volume={volume}: rss_bytes={rss_bytes} peak_committed={peak_committed} \ + analytical_bound={analytical_bound_bytes}" + ); + rss_points.push(rss_bytes); + committed_points.push(peak_committed as u64); + } + + let committed_extent = committed_points.iter().max().unwrap() - committed_points.iter().min().unwrap(); + assert!( + committed_extent <= analytical_bound_bytes, + "GOV-RSS01: governor committed-bytes extent {committed_extent} across growing volumes exceeds \ + the configuration-derived analytical bound {analytical_bound_bytes} — accounting is tracking \ + volume, not configuration+concurrency" + ); + for &committed in &committed_points { + assert!( + committed <= analytical_bound_bytes + (ENTRY_BYTES as u64), + "GOV-RSS01: peak committed bytes {committed} exceeds the analytical bound {analytical_bound_bytes} \ + (+ one entry's slack for an in-flight insert racing eviction)" + ); + } + + let rss_extent = rss_points.iter().max().unwrap() - rss_points.iter().min().unwrap(); + eprintln!("GOV-RSS01: rss points={rss_points:?} extent={rss_extent} slack={RSS_FLATNESS_SLACK_BYTES}"); + assert!( + rss_extent <= RSS_FLATNESS_SLACK_BYTES, + "GOV-RSS01: RSS extent {rss_extent} across growing volumes {VOLUMES:?} exceeds the flatness \ + slack {RSS_FLATNESS_SLACK_BYTES} — real process memory is tracking operation volume instead \ + of plateauing at the fixed cache-capacity + reader-concurrency bound" + ); +} + +/// `GOV-RSS02`: negative control. Built and run only under +/// `--features governor-rss-negative-control-leak`, which makes +/// `GovernedRing::retire` leak an ungoverned copy of every evicted entry — +/// invisible to `MemoryGovernor::committed_bytes()`, which stays perfectly +/// flat throughout — while real RSS grows linearly with volume. This proves +/// GOV-RSS01's *RSS* assertion (not just its governor-accounting assertion) +/// is actually load-bearing: without it, this exact bug would pass silently +/// forever. +#[test] +#[cfg(feature = "governor-rss-negative-control-leak")] +fn gov_rss02_negative_control_the_oracle_detects_a_volume_proportional_leak() { + const VOLUMES: [usize; 3] = [500, 4_000, 20_000]; + const RSS_FLATNESS_SLACK_BYTES: u64 = 24 * 1024 * 1024; + + let mut rss_points = Vec::new(); + let mut committed_points = Vec::new(); + for &volume in &VOLUMES { + let (rss_bytes, peak_committed) = run_scenario(volume); + eprintln!("GOV-RSS02 (leaking) volume={volume}: rss_bytes={rss_bytes} peak_committed={peak_committed}"); + rss_points.push(rss_bytes); + committed_points.push(peak_committed as u64); + } + + // The governor's OWN accounting must stay exactly as flat as GOV-RSS01 + // — this is the crux of the negative control: the leak is real but + // structurally invisible to `committed_bytes()`, because the leaked + // copy was never reserved through the governor at all. + let committed_extent = committed_points.iter().max().unwrap() - committed_points.iter().min().unwrap(); + let analytical_bound_bytes = ((CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY) * ENTRY_BYTES) as u64; + assert!( + committed_extent <= analytical_bound_bytes, + "sanity: the governor's own bookkeeping must still look perfectly bounded during the leak — \ + if this fails, the negative control is leaking through the reservation accounting too, which \ + would not prove what GOV-RSS02 needs to prove" + ); + + // But real RSS must NOT look bounded: it must exceed the same flatness + // slack GOV-RSS01 uses, growing with volume. This is the detection. + let rss_extent = rss_points.iter().max().unwrap() - rss_points.iter().min().unwrap(); + eprintln!("GOV-RSS02: rss points={rss_points:?} extent={rss_extent} slack={RSS_FLATNESS_SLACK_BYTES}"); + assert!( + rss_extent > RSS_FLATNESS_SLACK_BYTES, + "GOV-RSS02: the deliberately introduced volume-proportional leak (extent {rss_extent}) failed to \ + exceed the flatness slack {RSS_FLATNESS_SLACK_BYTES} — the RSS oracle is NOT sensitive enough to \ + detect a real leak the governor's own accounting cannot see; GOV-RSS01 would be a rubber stamp" + ); + assert!( + rss_points[2] > rss_points[0], + "GOV-RSS02: RSS at the largest volume ({}) must exceed RSS at the smallest ({}) — the leak must \ + actually correlate with volume, not just be noisy", + rss_points[2], + rss_points[0] + ); +} + +/// `GOV-RSS03`: pinned real reads. With eviction actively cycling entries +/// out of the ring while `READER_CONCURRENCY` threads continuously pin +/// whatever the latest entry is, the peak governed bytes observed must +/// still respect `charged = resident + pinned_only` — bounded by +/// `(capacity + concurrency) * ENTRY_BYTES`, never growing with how many +/// total pin/evict races happened (volume), and in particular strictly +/// above the `resident`-only bound whenever a pin genuinely outlives its +/// entry's eviction, proving pinned bytes are tracked at all. +#[test] +#[cfg_attr( + feature = "governor-rss-negative-control-leak", + ignore = "GOV-RSS03 is the positive-path pin/evict scenario, not the negative control" +)] +fn gov_rss03_pinned_reader_residency_tracks_charged_not_only_resident() { + let budget_bytes = (CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY + 8) * ENTRY_BYTES * 4; + let domain = tiny_domain(budget_bytes, CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY + 8); + let governor = Arc::clone(domain.governor()); + let ring = Arc::new(GovernedRing::new(&governor, CACHE_CAPACITY_ENTRIES)); + + // Fill to capacity first. + let mut handles = Vec::with_capacity(CACHE_CAPACITY_ENTRIES); + for i in 0..CACHE_CAPACITY_ENTRIES { + let fresh = ring.insert(i as u8).expect("capacity available while filling"); + handles.push(fresh); + } + let resident_only_bytes = ring.committed_bytes(); + assert_eq!( + resident_only_bytes, + CACHE_CAPACITY_ENTRIES * ENTRY_BYTES, + "GOV-RSS03 setup: a freshly filled ring must be exactly capacity * entry size" + ); + + // Pin every currently-resident entry (simulating READER_CONCURRENCY + // readers each holding one, capped — never all of them, matching the + // fixed concurrency bound) by cloning the `Arc`, then evict all of them + // from the ring itself. + let pinned: Vec<_> = handles.into_iter().take(READER_CONCURRENCY).collect(); + for _ in 0..READER_CONCURRENCY { + // Evict the (now-pinned) front entries by pushing fresh ones past + // capacity — this is a real eviction, not a manual removal. + ring.insert(0xAA) + .expect("capacity available while evicting the pinned entries out"); + } + + // The pinned entries are no longer in the ring (evicted), but each is + // still held here — `charged` must still count them, distinct from + // `resident` (only the ring's own capacity worth). + let charged_with_pins = ring.committed_bytes(); + assert_eq!( + charged_with_pins, + resident_only_bytes + READER_CONCURRENCY * ENTRY_BYTES, + "GOV-RSS03: charged bytes must equal resident (still-in-ring capacity) plus pinned_only \ + (evicted but still Arc-held) — not just the ring's own resident count" + ); + + // Bound check: charged must never exceed the fixed analytical bound + // regardless of how many total inserts happened to get here — this is + // the volume-independence half of GOV-RSS03. + let analytical_bound_bytes = (CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY) * ENTRY_BYTES; + assert!( + charged_with_pins <= analytical_bound_bytes, + "GOV-RSS03: charged bytes {charged_with_pins} exceeds the fixed analytical bound \ + {analytical_bound_bytes} (capacity + concurrency, independent of volume)" + ); + + // Now drop every pin — charged must collapse back to resident-only, + // proving pinned bytes are real, released capacity, not a permanent + // double-count. + drop(pinned); + let after_unpin = ring.committed_bytes(); + assert_eq!( + after_unpin, resident_only_bytes, + "GOV-RSS03: once every pin drops, charged bytes must return to exactly resident-only — a \ + pinned reservation must not outlive its last Arc" + ); +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index d84a8b3..cb85fd3 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -336,6 +336,16 @@ const ENGINE_TESTS: &[(&str, EngineGate, &str)] = &[ EngineGate::Light, "J0 preflight ENG-DUR-002/004", ), + ( + "cb_t5_bounded_release_window", + EngineGate::Light, + "ADR-072 CB-T5 : fenêtre de sous-comptage CB-07 (release à flush.rs:560, pas au vrai \n dernier Arc) reproduite avec un lecteur concurrent réel — prouvée bornée à la durée \n de vie de ce lecteur, jamais latente", + ), + ( + "gov_rss_oracle", + EngineGate::Light, + "ADR-071 GOV-RSS01-03 : oracle RSS générique du governor (étend la méthodologie \n ADR-053/D-6) — flatness sous volumes croissants, contrôle négatif dédié \n (feature governor-rss-negative-control-leak, jamais activée par défaut), et \n charged = resident + pinned_only sous lecteurs concurrents + éviction", + ), ]; /// Construit la ligne `cargo test -p basemyai-engine …` d'un gate donné à From 58d16a39059915cc39dc47af59edbce16d415b90 Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Tue, 18 Aug 2026 00:42:25 +0200 Subject: [PATCH 09/15] =?UTF-8?q?docs(V2-04):=20fix=20stale=20RETAIN=20col?= =?UTF-8?q?umn=20in=20ADR-072=20=C2=A7A=20outcome=20table?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent review (post-merge) of the P1 governor-gate-closure work flagged that release_governor_permit releases the per-write governor permit on all 5 CoordinatorOutcome branches, while ADR-072 §A's table still describes RETAIN for 4 of the 5 — traced to root cause: the table dates from the 'incremental per-write transfer' design that §B's own comparison explicitly rejected in favor of a separate upstream per-generation reservation (attach_memtable_charge / charge_rotated_memtable). With write_buffer_bytes = 0 (§A step 4), the per-write permit never carries memtable-residency weight in the first place, so uniform RELEASE is correct by construction, not a §B-pending compromise. The code was already correct; only the ADR table and a stale TODO(ADR-072 §B) comment (predating even this session, from the original §A/§B wiring commit) were out of date. Updates both to match reality. No behavior change; verified via cargo check -p basemyai and the coordinator test module (40/40). --- .../src/storage/native_store/coordinator.rs | 35 ++++++++++++------- ...mission-boundary-and-memtable-residency.md | 28 ++++++++++++--- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/crates/basemyai/src/storage/native_store/coordinator.rs b/crates/basemyai/src/storage/native_store/coordinator.rs index 18314f8..456f1ac 100644 --- a/crates/basemyai/src/storage/native_store/coordinator.rs +++ b/crates/basemyai/src/storage/native_store/coordinator.rs @@ -1535,20 +1535,27 @@ fn release_governor_permit( | Err(CoordinatorError::Closed | CoordinatorError::WrongRuntime | CoordinatorError::Unauthorized) => {} // Every durable — or durability-ambiguous — verdict. // - // TODO(ADR-072 §B): once the long-lived memtable-rotation - // reservation exists, transfer/reclassify into it here instead of - // releasing — today there is nowhere to hand these bytes off to, so - // releasing here is the only correct option even though it - // under-counts real memtable residency until §B lands. Tracked by - // ADR-072. + // Release here is correct by design, not a §B-pending compromise: + // the per-write `MemoryPlan` (`acquire_governed_admission`) sets + // `write_buffer_bytes = 0` unconditionally (ADR-072 §A step 4), so + // this permit never carries memtable-residency weight in the first + // place — it only ever covers transient WAL/intent framing bytes, + // which are genuinely done being needed once `execute()` has + // returned regardless of outcome. Memtable residency itself is + // covered by a wholly separate reservation, sized once per + // generation at `seal_memtable` and attached via + // `attach_memtable_charge`/`charge_rotated_memtable` (ADR-072 §B / + // CB-05, `charge_rotated_memtable` below) — that mechanism, not this + // one, is what would need extending if a future outcome ever needed + // to retain bytes past this point. // // `OutcomeUnknown` is deliberately grouped here rather than with the // aborted branch: architecturally one never assumes safe-to-release - // under ambiguity. Release is the pragmatic interim choice only - // because §B has not yet created anything to retain the bytes into — - // not a claim that this branch is compliant. Same for the raw - // `Err(Terminal)` an `execute()` panic produces, whose durability is - // unknown by construction. + // under ambiguity. It is release-correct for the same reason as the + // other durable/ambiguous outcomes above — nothing in this permit's + // scope needs retaining — not because retention was skipped as an + // expedient. Same for the raw `Err(Terminal)` an `execute()` panic + // produces, whose durability is unknown by construction. Ok( CoordinatorOutcome::Committed { .. } | CoordinatorOutcome::DurableReopenRequired { .. } @@ -1557,8 +1564,10 @@ fn release_governor_permit( ) | Err(_) => {} } - // For this increment both branches converge on ordinary RAII release; - // the classification above is what §B will branch on for real. + // Both branches converge on ordinary RAII release: the classification + // above exists to document *why* each outcome releases rather than to + // gate a future retain-vs-release split, since this permit never scopes + // memtable residency (see above). drop(permit); } diff --git a/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md b/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md index 3418010..2e86f0a 100644 --- a/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md +++ b/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md @@ -127,10 +127,30 @@ relâché), `owner_commands` classe l'issue et décide release/retain : | `CoordinatorOutcome` | Statut de durabilité WAL | Action sur `governor_permit` | | --- | --- | --- | | `Aborted { .. }` | WAL jamais durablement écrit (échec pré-réservation, ou `WalWriteOutcome::Aborted` propre) | **RELEASE** | -| `Committed { .. }` | fsync confirmé + install produit réussi | **RETAIN** | -| `DurableReopenRequired { .. }` | fsync confirmé, install produit échoué | **RETAIN** | -| `OutcomeUnknown { .. }` | issue WAL elle-même inconnue (crash à la frontière) | **RETAIN** | -| `StructuralReopenRequired { .. }` | publication durable structurelle confirmée, mismatch RAM | **RETAIN** | +| `Committed { .. }` | fsync confirmé + install produit réussi | **RELEASE** | +| `DurableReopenRequired { .. }` | fsync confirmé, install produit échoué | **RELEASE** | +| `OutcomeUnknown { .. }` | issue WAL elle-même inconnue (crash à la frontière) | **RELEASE** | +| `StructuralReopenRequired { .. }` | publication durable structurelle confirmée, mismatch RAM | **RELEASE** | + +**Correction (2026-08-17, revue indépendante post-implémentation) — la +colonne RETAIN ci-dessus était un résidu de conception non mis à jour, pas +une spécification suivie par le code.** Elle datait du modèle "transfert +incrémental" (option 1 du tableau de comparaison §B, explicitement +**rejetée** au profit de la "réservation amont" — option 2) et n'a jamais +été corrigée après ce choix. Avec `write_buffer_bytes = 0` imposé par +l'étape 4 de §A ci-dessus, le `MemoryPlan` par-write ne porte **jamais** la +charge de résidence memtable — il ne couvre que les octets transitoires +WAL/intent, dont le besoin s'éteint dès le retour d'`execute()`, quelle que +soit l'issue. La résidence memtable est couverte par un mécanisme +entièrement distinct : la réservation amont par génération de §B/CB-05 +(`attach_memtable_charge`/`charge_rotated_memtable`, dimensionnée une seule +fois au `seal_memtable`). RELEASE uniforme est donc correct par +construction pour `release_governor_permit`, pas un compromis provisoire en +attente de §B — §B est déjà en place et couvre ce besoin par un chemin +séparé. Le code (`coordinator.rs`, `release_governor_permit`) a toujours +implémenté RELEASE uniforme ; c'était cette table et le commentaire +`TODO(ADR-072 §B)` du code (retiré) qui étaient en retard sur la +conception réelle, pas l'inverse. Cette table est exhaustive et ne laisse aucune variante ambiguë après traçage direct de `CoordinatorOutcome` (`coordinator.rs:151-178`) jusqu'à From 69c265dae77addae9fe1aa4340b6ab61f53becf3 Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Tue, 18 Aug 2026 00:43:55 +0200 Subject: [PATCH 10/15] docs: status.md entry for the P1 governor gate-closure round (2026-08-18) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the 5-block parallel closure of CB-T1d/T2/T5, GOV-U21-23/25-26/40-41, GOV-S03, GOV-RSS01-03, the independent review's verification, the ADR-072 §A table doc fix, and full gate results. Activation gate itself remains closed and unchanged, per ADR-071/ADR-072's own phase-2 deferrals (MG-17/write.rs, GOV-U32). --- docs/status.md | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/docs/status.md b/docs/status.md index e776be3..06f5103 100644 --- a/docs/status.md +++ b/docs/status.md @@ -1,5 +1,101 @@ # BaseMyAI — Implementation Status Matrix +**Mise à jour 2026-08-18 — dette de test P1 ADR-071/ADR-072 fermée +(`CB-T1d/T2/T5`, `GOV-U21-23/25-26/40-41`, `GOV-S03`, `GOV-RSS01-03`) ; +gate d'activation production reste fermé (inchangé, cf. entrées +ci-dessous).** Cinq chantiers indépendants (isolation `git worktree`), +fusionnés sur une branche `integration/p1-governor-gate-closure` (non +mergée sur `dev` à ce stade — décision humaine), puis revus par un agent +indépendant qui n'a participé à aucune implémentation. + +- **Fermés, avec preuve red-before/green-after** : + - `CB-T1d` — 4 des 5 branches `CoordinatorOutcome` sans couverture + gouvernée (`Aborted`, `OutcomeUnknown`, `StructuralReopenRequired` + directement ; `DurableReopenRequired` par construction directe, cette + branche étant structurellement inatteignable en production aujourd'hui + — confirmé par grep et par la revue indépendante, documenté comme tel + dans le test, pas maquillé en bout-en-bout). + - `CB-T2` — ordre gouverneur-avant-sémaphore sous contention réelle : + prouvé qu'un appelant parqué sur la capacité en octets du gouverneur + ne détient jamais un slot `core.permits` pendant son attente (la + conception FIFO stricte intra-classe du gouverneur rendait le design + initialement suggéré — « N autres writers progressent en concurrence » + — structurellement impossible pour des writers de même classe ; + remplacé par une preuve directe au niveau sémaphore, documentée). + - `GOV-U21/U22/U23` — protections bytes/slots/symétrie entre + `Foreground`/`Maintenance` : aucun bug trouvé, propriétés déjà + correctes, couverture ajoutée avec preuve par injection de défaut + (protection cross-classe rendue self-only, rouge confirmé, reverté). + - `GOV-U25/U26` — spurious wake / lost wake : aucun bug trouvé (le + pattern Mesa-monitor de `acquire_admission` empêchait déjà les deux), + couverture ajoutée avec preuve par injection (bug classique + unlock-sleep-relock reproduit puis reverté). + - `GOV-U40/U41` — domaine partagé entre deux « engines » vs domaines + indépendants : propriétés correctes, couverture ajoutée. Le point déjà + documenté par ADR-072 (`WriteCoordinator::build` panique sur un + domaine partagé faute d'agent capable de libérer des octets) reste + confirmé mais **non corrigé, à dessein** — inatteignable en production + (`dormant_governed` sans site d'appel), déjà tracké. + - `GOV-S03` — arrêt sous stress (waiters + réservations en vol) : + `MemoryGovernor::close()` réveille tous les waiters dans un délai + borné, aucune admission nouvelle après fermeture, bookkeeping revient + à zéro. + - `CB-T5` — fenêtre de sous-comptabilité CB-07 (release à + `flush.rs:560` vs dernier-`Arc` réel via un `Engine::snapshot()` + concurrent) reproduite par construction (vraie memtable, vrai thread + de flush, pas un failpoint arbitraire) et prouvée bornée exactement + comme documenté par ADR-072 — aucun bug trouvé. + - `GOV-RSS01/RSS03` — oracle RSS générique (ADR-053 étendu) : plateau + RSS confirmé sous volumes croissants, `charged = resident + + pinned_only` confirmé sous éviction+pinning réel. `GOV-RSS02` + (contrôle négatif) a d'abord été mal câblé (sink de fuite scopé à une + instance droppée, donc invisible) — corrigé (`static` process-lifetime) + et revérifié : 226 Mio de croissance non détectée par + `committed_bytes()` correctement détectés par le harnais RSS. + Portée **explicitement limitée au générique ADR-071** : l'oracle + RSS/stress spécifique à la rotation memtable §B, listé séparément par + ADR-072 (« au-delà des `GOV-RSS01..03` génériques »), reste ouvert. +- **Corrigé, hors périmètre des 5 chantiers** — `docs(V2-04)`, commit + séparé : la colonne RETAIN de la table §A d'ADR-072 (4 des 5 branches + `CoordinatorOutcome`) était un résidu de conception jamais mis à jour + après que §B a retenu la « réservation amont » plutôt que le « transfert + incrémental » — avec `write_buffer_bytes = 0` imposé par §A, le permit + par-write ne porte jamais la charge memtable, donc RELEASE uniforme + (ce que fait déjà `release_governor_permit`) est correct par + construction, pas un compromis provisoire. Table ADR-072 et + `TODO(ADR-072 §B)` du code (celui-ci antérieur à cette session, issu du + commit `b7cc3c0` original) corrigés pour refléter le code réel. Aucun + changement de comportement. +- **Revue indépendante** : ré-exécution personnelle des preuves + red/green de `GOV-U21/22/23` et `GOV-U26` (défauts réinjectés, + confirmés rouges, revertés, confirmés verts) plutôt qu'acceptées sur + parole ; diff fonction-par-fonction des 3 branches ayant fusionné dans + le même fichier (`admission.rs`) contre l'état final, confirmé + identique octet-pour-octet, rien perdu ni altéré par la résolution + manuelle de conflit. Une régression potentielle investiguée + (`seal_recovery::flush_after_recovery_publishes_the_sst_and_keeps_the_empty_active_segment`, + flaky sous charge dans `cargo xtask test` complet) — confirmée + **préexistante et sans rapport** avec ce chantier (fichier non touché, + 8/8 vert en isolation et en run propre à trois reprises) ; c'est la même + flake déjà notée le 2026-08-10 (`docs/status.md`), toujours non + corrigée, hors périmètre. +- **Gates rejoués sur la branche fusionnée** : `cargo xtask check` + **vert** ; `cargo xtask test-wiring` **vert** (42 binaires) ; `cargo + xtask test` **vert** (suite complète + xtask lui-même 43/43, 1 + maintainer-only ignoré) ; `cargo xtask test-crash-consistency` **13/13, + 348,61 s** — aucune régression ; `cargo test -p basemyai-engine + --features test-util --lib` **618/618** ; `cargo test -p basemyai + --features test-util --lib` **154/154** — revérifié après la correction + documentaire ultérieure (commentaire seul, aucun changement de + comportement), toujours **154/154** ; `format.lock` strictement + inchangé. +- **N'est PAS débloqué par ce chantier** : le gate d'activation + production reste fermé, inchangé depuis les entrées précédentes — + `EmergencyHeadroom` opérationnel (`GOV-U32`) et le câblage direct + MG-17/`write.rs` restent phase 2, non scopés ici, comme documenté par + ADR-071/ADR-072 eux-mêmes. `dormant_governed` reste sans site d'appel + production. + **Mise à jour 2026-08-17 (suite 2) — CB-T4 requalifié, EmergencyHeadroom dérivé et testé (moitié analytique), worktrees/branches mortes nettoyées.** Suite directe de l'entrée précédente, sur décision explicite de From 589e17faece779b43629c3534bd8eebf6dcdb1e8 Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Tue, 18 Aug 2026 01:14:42 +0200 Subject: [PATCH 11/15] fix(V2-04): WriteCoordinator::build no longer panics on a shared MemoryDomain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GOV-U40/41's own audit had confirmed the panic was reachable and real: a MemoryDomain shared across several engines (an explicitly supported configuration per ADR-071 "Portée du domaine") can legitimately have too little remaining capacity for a second engine's bootstrap memtable-generation charge — a valid runtime outcome, not a config error, since the first engine's own bootstrap reservation is what consumed the room. build() now returns Result<(ReadGateway, Self), CoordinatorError>, surfacing that refusal as CoordinatorError::AdmissionRefused instead of aborting the process. dormant()/dormant_with_sink() (memory_governor: None) unwrap internally since that path's only fallible step never runs; dormant_governed() propagates the Result to its caller. Adds two regression tests exercising two real Engines sharing one MemoryDomain: one proving the typed-error path (domain sized for exactly one bootstrap charge), one proving two engines can share a correctly-sized domain and both write independently through it. Co-Authored-By: Claude Sonnet 5 --- .../src/storage/native_store/coordinator.rs | 215 +++++++++++++++--- 1 file changed, 182 insertions(+), 33 deletions(-) diff --git a/crates/basemyai/src/storage/native_store/coordinator.rs b/crates/basemyai/src/storage/native_store/coordinator.rs index 456f1ac..1262836 100644 --- a/crates/basemyai/src/storage/native_store/coordinator.rs +++ b/crates/basemyai/src/storage/native_store/coordinator.rs @@ -914,28 +914,32 @@ impl WriteCoordinator { finalization_capacity: usize, finalization_sink: FinalizationSink, ) -> (ReadGateway, Self) { + // `memory_governor: None` means `build()`'s only fallible step (the + // bootstrap memtable reservation) never runs — see `build`'s doc + // comment. An ungoverned coordinator's construction is infallible by + // construction, so unwrapping here is not a shortcut, it is the + // caller-side proof of that fact. Self::build(state, capacity, finalization_capacity, finalization_sink, None) + .expect("build() cannot fail with memory_governor: None") } /// Same topology as [`Self::dormant`], but every write is admitted by /// `memory_governor` first (ADR-072 §A / CB-02) instead of going straight /// to the tokio concurrency semaphore. /// - /// Deliberately unrouted for now: ADR-072's ratification gates forbid - /// activating a real hard cap before §B (the long-lived memtable - /// residency charge) also lands, because a cap without §B would under- - /// count memtable RAM from the very first write. Until then this is the - /// mechanism plus its tests, never the default construction path — - /// `NativeMemoryStore::from_engine` keeps calling [`Self::dormant`]. - #[allow( - dead_code, - reason = "ADR-072 §A mechanism; its production call site lands with §B, per ADR-072's ratification gates" - )] + /// # Errors + /// [`CoordinatorError::AdmissionRefused`] if `memory_governor`'s domain + /// cannot immediately host one full memtable generation of this engine + /// (ADR-072 §B / CB-06 calibration) — a real, expected runtime outcome + /// when several engines share one [`basemyai_engine::memory_governor::MemoryDomain`] + /// and an earlier one already claimed the room this one needs, never a + /// panic (see [`Self::build`]'s doc comment for why this cannot block + /// instead). pub(super) fn dormant_governed( state: NativeInner, capacity: usize, memory_governor: Arc, - ) -> (ReadGateway, Self) { + ) -> Result<(ReadGateway, Self), CoordinatorError> { Self::build( state, capacity, @@ -945,13 +949,23 @@ impl WriteCoordinator { ) } + /// # Errors + /// [`CoordinatorError::AdmissionRefused`] — only reachable when + /// `memory_governor` is `Some` — if the domain cannot immediately host + /// one full memtable generation of this engine (ADR-072 §B / CB-06). This + /// is a real, valid runtime condition, not a configuration bug: several + /// engines are allowed to share one `MemoryDomain` (ADR-071 + /// §"Portée du domaine"), and an earlier engine's own bootstrap charge + /// can legitimately leave too little room for a later one. Never a panic + /// — see the non-blocking rationale below, which still applies verbatim + /// to a shared domain. fn build( mut state: NativeInner, capacity: usize, finalization_capacity: usize, finalization_sink: FinalizationSink, memory_governor: Option>, - ) -> (ReadGateway, Self) { + ) -> Result<(ReadGateway, Self), CoordinatorError> { assert!(capacity > 0, "writer admission capacity must be non-zero"); assert!(finalization_capacity > 0, "finalization capacity must be non-zero"); static NEXT_RUNTIME: AtomicU64 = AtomicU64::new(1); @@ -993,10 +1007,16 @@ impl WriteCoordinator { // encore, aucune écriture n'est en vol, aucun flush ne peut aboutir : // il n'existe aucun agent capable de rendre un octet. Un // `reserve_blocking` ne serait donc pas de la contre-pression mais un - // interblocage inconditionnel. Un refus ici signifie exactement une - // chose — le domaine est trop petit pour héberger une génération de - // memtable de ce moteur — et c'est une erreur de configuration, qu'on - // fait remonter tout de suite plutôt que de la laisser pendre. + // interblocage inconditionnel. + // + // Un refus ici n'est **pas nécessairement** une erreur de + // configuration : un domaine partagé par plusieurs engines (ADR-071 + // §"Portée du domaine") peut voir sa capacité déjà consommée par le + // bootstrap d'un engine ouvert plus tôt sur le même `MemoryDomain` — + // une condition runtime valide, jamais un bug. Faire remonter une + // erreur typée (plutôt que paniquer) laisse l'appelant décider — + // réessayer avec un domaine plus grand, fermer cet engine, etc. — + // au lieu d'avorter tout le process pour un cas de partage légitime. if let Some(governor) = memory_governor.as_ref() { let reservation = governor .try_reserve( @@ -1004,12 +1024,12 @@ impl WriteCoordinator { Some(basemyai_engine::AdmissionClass::Foreground), memtable_reservation_bytes, ) - .unwrap_or_else(|error| { - panic!( - "the memory domain cannot host one memtable generation of this engine \ + .map_err(|error| { + CoordinatorError::AdmissionRefused(format!( + "the memory domain cannot host one more memtable generation \ ({memtable_reservation_bytes} bytes, ADR-072 CB-06 calibration): {error}" - ) - }); + )) + })?; state .engine .active_memtable_charge_handle() @@ -1052,7 +1072,7 @@ impl WriteCoordinator { .lock() .expect("owner handle lock is initialized") .replace(owner); - (gateway, Self { core }) + Ok((gateway, Self { core })) } fn authorize(&self, agent: &str, class: OperationClass) -> Result { @@ -2769,6 +2789,124 @@ mod tests { .expect("valid governor config") } + // ADR-072's own §"Points ouverts" flags this exact scenario as confirmed + // and deliberately left unfixed: `WriteCoordinator::build` used to + // `panic!` when a shared `MemoryDomain`'s bootstrap reservation for a + // *second* engine found the domain already too full — even though + // sharing one `MemoryDomain` across several engines is an explicitly + // supported, valid runtime configuration (ADR-071 §"Portée du domaine": + // "un produit qui veut une limite commune partage le même + // `Arc`"), not a configuration error. This proves the fix: + // a real second `Engine`, same governor, insufficient remaining budget — + // a typed `CoordinatorError::AdmissionRefused`, never a panic that would + // abort the whole process for what is a completely ordinary admission + // refusal. + #[test] + fn build_returns_a_typed_error_not_a_panic_when_a_shared_domain_is_too_small_for_a_second_engine() { + let (_first_directory, first_state) = inner_with_options(rotating_options()); + let (_second_directory, second_state) = inner_with_options(rotating_options()); + let residency = first_state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + assert_eq!( + residency, + second_state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"), + "both engines share the same rotating_options() calibration" + ); + + // Sized to host exactly one engine's bootstrap residency charge and + // nothing more: `budget - residency` leaves only the two 1-byte + // per-class floors, far short of a second `residency`-sized claim. + let domain = governed_domain(residency + 2, 1); + let governor = domain.governor(); + + let (_first_reads, _first_writer) = WriteCoordinator::dormant_governed(first_state, 4, Arc::clone(governor)) + .expect("the first engine on a fresh shared domain must succeed"); + assert_eq!( + governor.committed_bytes(), + residency, + "the first engine's bootstrap charge alone must already be committed" + ); + + let refusal = match WriteCoordinator::dormant_governed(second_state, 4, Arc::clone(governor)) { + Ok(_) => panic!("a second engine must not fit the remaining 2 bytes of this shared domain"), + Err(error) => error, + }; + assert!( + matches!(refusal, CoordinatorError::AdmissionRefused(_)), + "a shared domain refusing a second engine's bootstrap charge is a real, valid runtime \ + outcome (ADR-071 §\"Portée du domaine\") — a typed error, not the process-aborting \ + panic this used to be: got {refusal:?}" + ); + // The failed second attempt must not have left any partial charge + // behind — `build()` returns before ever calling `attach` when + // `try_reserve` itself fails. + assert_eq!( + governor.committed_bytes(), + residency, + "a refused bootstrap reservation must not move the governor's accounting at all" + ); + } + + /// The companion positive case: the same two real engines, the same + /// shared governor, but a domain sized to actually host both — proving + /// the fix is a correct typed error on refusal, not a regression that + /// now refuses a legitimately-sized shared domain too. + #[tokio::test] + async fn two_real_engines_can_share_one_memory_domain_when_it_is_sized_for_both() { + let (_first_directory, first_state) = inner_with_options(rotating_options()); + let (_second_directory, second_state) = inner_with_options(rotating_options()); + let residency = first_state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + + // `per_class_bytes` must clear a real write's `MemoryPlan::peak_bytes` + // (WAL framing overhead alone is already 35 bytes unencrypted) — + // unlike the refusal test above, this domain also has to admit real + // per-write traffic through `submit()`, not just the two bootstrap + // residency charges. + const PER_CLASS_BYTES: usize = 4096; + let domain = governed_domain(2 * residency + 2 * PER_CLASS_BYTES, PER_CLASS_BYTES); + let governor = domain.governor(); + + let (_first_reads, first_writer) = WriteCoordinator::dormant_governed(first_state, 4, Arc::clone(governor)) + .expect("first engine fits the shared domain"); + let (_second_reads, second_writer) = WriteCoordinator::dormant_governed(second_state, 4, Arc::clone(governor)) + .expect("second engine also fits the shared domain once it is sized for both"); + assert_eq!( + governor.committed_bytes(), + 2 * residency, + "both engines' bootstrap residency charges must be committed simultaneously" + ); + + // Both coordinators must be genuinely independently functional on + // the shared domain, not just constructible. + let log = Arc::new(Mutex::new(Vec::new())); + first_writer + .submit(record(1, &log)) + .await + .expect("first coordinator admits a write") + .outcome() + .await + .expect("first coordinator's write completes"); + second_writer + .submit(record(2, &log)) + .await + .expect("second coordinator admits a write") + .outcome() + .await + .expect("second coordinator's write completes"); + assert_eq!(*log.lock().expect("test log"), vec![1, 2]); + + first_writer.close().await.expect("first coordinator closes cleanly"); + second_writer.close().await.expect("second coordinator closes cleanly"); + } + fn parked_record( marker: u64, log: &Arc>>, @@ -3016,7 +3154,8 @@ mod tests { .expect("calibration must not overflow"); let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); // ADR-072 §B moved this floor off zero: a governed coordinator always // carries its live memtable generation's residency charge, from // construction onward. Every per-write assertion below is stated @@ -3070,7 +3209,8 @@ mod tests { // budget, so the two can be set apart like this. let domain = governed_domain(64 * 1024 * 1024, 1); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); let log = Arc::new(Mutex::new(Vec::new())); assert!(matches!( @@ -3121,7 +3261,8 @@ mod tests { // already eats into `budget` too. let domain = governed_domain(budget, 900_000); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); // Hog every free byte beyond the bootstrap memtable charge but one — // `try_reserve` has no per-request ceiling (unlike admission), and @@ -3195,7 +3336,8 @@ mod tests { .expect("calibration must not overflow"); let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); let log = Arc::new(Mutex::new(Vec::new())); let (intent, entered_rx, release_tx) = parked_record(1, &log, true); @@ -3245,7 +3387,8 @@ mod tests { .expect("calibration must not overflow"); let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); assert_eq!( governor.committed_bytes(), residency, @@ -3294,7 +3437,8 @@ mod tests { .expect("calibration must not overflow"); let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); failpoint::set("after_wal_append", Action::Error); let outcome = writer @@ -3356,7 +3500,8 @@ mod tests { .expect("calibration must not overflow"); let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); let governor = Arc::clone(domain.governor()); - let (reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)); + let (reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); failpoint::set("during_generation_directory_sync", Action::Error); let outcome = writer @@ -3420,7 +3565,8 @@ mod tests { .expect("calibration must not overflow"); let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); let seeded = writer .graph_entity_batch("agent-a".into(), vec![purge_entity(1)]) @@ -3502,7 +3648,8 @@ mod tests { let domain = governed_domain(budget, 900_000); let governor = Arc::clone(domain.governor()); let concurrency = 4; - let (_reads, writer) = WriteCoordinator::dormant_governed(state, concurrency, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, concurrency, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); // Hog every free byte beyond the bootstrap memtable charge but one — // same technique as @@ -3588,7 +3735,8 @@ mod tests { let domain = governed_domain(reservation * 6, reservation * 2); let governor = Arc::clone(domain.governor()); - let (reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); assert_eq!( governor.committed_bytes(), @@ -3633,7 +3781,8 @@ mod tests { .expect("calibration must not overflow"); let domain = governed_domain(reservation * 6, reservation * 2); let governor = Arc::clone(domain.governor()); - let (reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); // Held across the rotation on purpose: it keeps naming the *retired* // generation, so CB-07's release can be observed on that generation From 041d213f22c7ab53d86a04e0375334fc1a9bc560 Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Tue, 18 Aug 2026 01:22:13 +0200 Subject: [PATCH 12/15] =?UTF-8?q?test(V2-04):=20memtable-rotation-specific?= =?UTF-8?q?=20RSS/stress=20oracle=20(ADR-072=20=C2=A7B)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GOV-RSS01-03 only closed the *generic* governor RSS oracle (a cache harness); ADR-072's own "Points ouverts" and gov_rss_oracle.rs's module doc both explicitly flag the rotation-specific oracle as still open, requiring "a real Engine driven through many rotations". Adds gov_rotation_rss_oracle.rs: 40 real rotate -> write -> flush cycles (explicit seal, deterministic), a third of them pinned by a concurrent snapshot across the flush (the CB-07 window CB-T5 already proved bounded once). Proves, every cycle: the charge is attached before any write lands (no under-accounting), a second attach on an already-charged generation is rejected without moving accounting (no double-accounting), committed_bytes() returns to exactly zero after every flush regardless of pinning (no growing leak), and every witness strong count returns to exactly 1 once its reader drops. Ends with a post-run reservation probe and a clean engine drop to prove teardown leaves the domain fully usable. Caught one real bug in the test's own first draft: retaining the attach-time MemtableChargeHandle for the whole loop body (instead of scoping it to the attach call, as production's attach_memtable_charge does) manifested as a spurious extra strong reference — the leak check itself caught it, RED before the scoping fix, GREEN after. Wired into xtask's ENGINE_TESTS (Light gate) and basemyai-engine's Cargo.toml per the crate's test-wiring guard. Co-Authored-By: Claude Sonnet 5 --- crates/basemyai-engine/Cargo.toml | 4 + .../tests/governor/gov_rotation_rss_oracle.rs | 230 ++++++++++++++++++ xtask/src/main.rs | 5 + 3 files changed, 239 insertions(+) create mode 100644 crates/basemyai-engine/tests/governor/gov_rotation_rss_oracle.rs diff --git a/crates/basemyai-engine/Cargo.toml b/crates/basemyai-engine/Cargo.toml index eb590c9..5c3909d 100644 --- a/crates/basemyai-engine/Cargo.toml +++ b/crates/basemyai-engine/Cargo.toml @@ -282,5 +282,9 @@ path = "tests/governor/cb_t5_bounded_release_window.rs" name = "gov_rss_oracle" path = "tests/governor/gov_rss_oracle.rs" +[[test]] +name = "gov_rotation_rss_oracle" +path = "tests/governor/gov_rotation_rss_oracle.rs" + [lints] workspace = true diff --git a/crates/basemyai-engine/tests/governor/gov_rotation_rss_oracle.rs b/crates/basemyai-engine/tests/governor/gov_rotation_rss_oracle.rs new file mode 100644 index 0000000..e438cdd --- /dev/null +++ b/crates/basemyai-engine/tests/governor/gov_rotation_rss_oracle.rs @@ -0,0 +1,230 @@ +// SPDX-License-Identifier: BUSL-1.1 +//! ADR-072 §"Points ouverts" — the memtable-rotation-specific RSS/stress +//! oracle, still open after `GOV-RSS01..03`: "Oracle RSS/stress dédié à +//! l'écart `governed_committed_bytes` vs mémoire réelle pour le mécanisme de +//! rotation §B". `gov_rss_oracle.rs`'s own module doc says explicitly that its +//! generic `GovernedRing` cache harness "does not attempt" this one — "it +//! requires a real `Engine` driven through many rotations". This file is +//! that oracle. +//! +//! ## What "`charged = resident + pinned_only`" means for §B specifically +//! +//! `gov_rss_oracle.rs` proves this equation for a governed *cache*, where an +//! evicted-but-still-referenced entry stays charged until its last reader +//! drops. The memtable rotation mechanism (`CB-05`/`CB-06`/`CB-07`) is +//! deliberately asymmetric to that: `CB-T5` +//! (`cb_t5_bounded_release_window.rs`) already proved the charge for a +//! generation releases *unconditionally at flush-completion*, independent of +//! whether a concurrent reader still pins that generation's real, resident +//! `Arc`. So for this mechanism, `pinned_only`'s contribution to +//! `charged` is always exactly zero, by construction — the real deviation is +//! that `resident` can transiently *exceed* `charged` (a pinned, flushed +//! generation is real resident memory the governor has already stopped +//! counting), never the other way around. What this file proves is the +//! sharper, meaningful invariant that claim reduces to for a mechanism with +//! this shape, repeated across many real rotation cycles rather than CB-T5's +//! single/25-iteration same-shape loop: +//! +//! - **No under-accounting**: a generation's charge is committed before any +//! write can land in it — attach happens strictly before the first `put` +//! every single cycle, never after. +//! - **No double-accounting**: `committed_bytes()` never exceeds exactly one +//! `CHARGE_BYTES` at a time, even when a caller wrongly attempts a second +//! attach onto an already-charged generation (rejected, verified not to +//! move accounting at all). +//! - **No growing leak**: `committed_bytes()` returns to *exactly* zero after +//! every one of many (`GENERATIONS`) rotate→write→(pin?)→flush cycles — +//! flat, not drifting upward with iteration count — regardless of whether +//! that generation was pinned by a concurrent reader across its flush. +//! - **Pinning never leaves a dangling reference**: every pinned generation's +//! witness strong count returns to exactly 1 (only this test's own witness +//! handle left) once its concurrent reader drops — cycle after cycle, not +//! just the first time. +//! - **Teardown**: the domain accepts a fresh reservation again after the +//! whole run and the engine drops cleanly — nothing left the domain, or the +//! engine, in a state that would refuse further legitimate use. + +use basemyai_engine::{AdmissionClass, Engine, EngineOptions, GovernedMemoryKind, GovernorConfig, MemoryDomain}; + +fn charge_domain(budget_bytes: usize) -> MemoryDomain { + MemoryDomain::new(GovernorConfig { + budget_bytes, + max_queue_depth: 8, + max_admission_waiters: 8, + reserved_slots: [2, 2], + reserved_bytes: [budget_bytes / 2, budget_bytes / 4], + weight: [1, 1], + max_intent_peak_bytes: [budget_bytes / 2, budget_bytes / 4], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }) + .expect("valid governor config") +} + +/// Seal driven explicitly by `flush_memtables_only()` only — same rationale +/// as `cb_t5_bounded_release_window.rs`: deterministic, one seal per call, +/// never a threshold race. +fn explicit_seal_options() -> EngineOptions { + EngineOptions { + memtable_flush_threshold: usize::MAX, + memtable_target_bytes: usize::MAX, + ..EngineOptions::default() + } +} + +const CHARGE_BYTES: usize = 8_192; +const GENERATIONS: usize = 40; +/// Every third generation is pinned by a concurrent snapshot held across its +/// flush (the `CB-07` window) — the rest flush with no concurrent reader at +/// all, so the oracle covers both shapes every run, not just one. +const PIN_EVERY: usize = 3; + +#[test] +fn memtable_rotation_charge_is_exact_and_flat_across_many_pinned_and_unpinned_generations() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut engine = Engine::open_with_options(dir.path(), explicit_seal_options()).expect("open"); + let domain = charge_domain(CHARGE_BYTES * 4); + let governor = domain.governor(); + + for generation in 0..GENERATIONS { + // --- rotation + attach, strictly before any write: no under-accounting --- + assert_eq!( + governor.committed_bytes(), + 0, + "generation {generation}: the domain must be fully drained before the next \ + generation's charge — any leftover here would be a leak from a previous cycle" + ); + let reservation = governor + .try_reserve( + GovernedMemoryKind::WriteBufferMutable, + Some(AdmissionClass::Foreground), + CHARGE_BYTES, + ) + .unwrap_or_else(|error| panic!("generation {generation}: reserve the generation's charge: {error:?}")); + // Scoped exactly like `basemyai`'s own `attach_memtable_charge` + // (`coordinator.rs`): the handle used to attach is never retained + // past the attach call itself — holding it longer would itself be an + // extra strong reference the leak checks below would (rightly) catch + // as if it were a real engine-side leak. + { + let handle = engine.active_memtable_charge_handle(); + handle + .attach(reservation) + .map_err(|_| ()) + .unwrap_or_else(|()| panic!("generation {generation}: a fresh generation must accept its charge")); + assert_eq!( + handle.charged_bytes(), + Some(CHARGE_BYTES), + "generation {generation}: the handle must report exactly the reservation just attached" + ); + assert_eq!( + governor.committed_bytes(), + CHARGE_BYTES, + "generation {generation}: exactly one charge is committed, attached before any write" + ); + + // --- no double-accounting: a second attach on the same + // generation is rejected outright and never perturbs the + // governor's accounting, even attempted. Only exercised once — + // the property does not depend on the generation index, and + // repeating it every cycle would just add noise. + if generation == 0 { + let duplicate = governor + .try_reserve( + GovernedMemoryKind::WriteBufferMutable, + Some(AdmissionClass::Foreground), + 1, + ) + .expect("a 1-byte probe reservation must fit the domain's remaining free bytes"); + let rejected = handle + .attach(duplicate) + .expect_err("a generation that already carries a charge must reject a second attach"); + assert_eq!( + rejected.bytes(), + 1, + "the rejected reservation must be handed back untouched, not consumed" + ); + drop(rejected); + assert_eq!( + governor.committed_bytes(), + CHARGE_BYTES, + "generation 0: a rejected double-attach must leave accounting exactly where it was, \ + never CHARGE_BYTES + 1" + ); + } + } + + // --- real write into the now-charged generation --- + let key = format!("gov-rotation-rss-key-{generation}"); + engine + .put(key.as_bytes(), b"v") + .unwrap_or_else(|error| panic!("generation {generation}: put: {error:?}")); + + // --- flush, optionally pinned across it (the CB-07 window) --- + let witness = engine.active_memtable_charge_handle(); + let pinned = generation % PIN_EVERY == 0; + let concurrent_snapshot = pinned.then(|| engine.snapshot()); + + engine + .flush_memtables_only() + .unwrap_or_else(|error| panic!("generation {generation}: flush: {error:?}")); + + // --- release: unconditional at flush-completion, pinned or not --- + assert_eq!( + governor.committed_bytes(), + 0, + "generation {generation}: the charge must release exactly at flush-completion \ + regardless of whether a concurrent reader (pinned={pinned}) still holds this generation" + ); + + if let Some(snapshot) = concurrent_snapshot { + assert_eq!( + snapshot.get(key.as_bytes()).expect("read through the pinned snapshot"), + Some(b"v".to_vec()), + "generation {generation}: the flushed generation's data must still be resident and \ + reachable through the pinned snapshot even though the governor already released it" + ); + let during_overlap = witness.strong_count(); + drop(snapshot); + let after_drop = witness.strong_count(); + assert_eq!( + during_overlap, + after_drop + 1, + "generation {generation}: dropping the pinned reader must reduce strong references \ + by exactly one (during={during_overlap}, after={after_drop})" + ); + } + // No growing leak, pinned or not: after this generation's own reader + // (if any) has dropped, only this loop's own witness handle remains. + assert_eq!( + witness.strong_count(), + 1, + "generation {generation}: no reference from this generation must survive past this point \ + — a leaked reference here would accumulate across {GENERATIONS} generations and this \ + check would eventually catch it even if generation {generation} alone looked clean" + ); + } + + // --- teardown: the domain is left exactly as usable as it started --- + assert_eq!( + governor.committed_bytes(), + 0, + "after {GENERATIONS} full rotation cycles, the domain must be back at zero, not drifted" + ); + let post_run_probe = governor + .try_reserve( + GovernedMemoryKind::WriteBufferMutable, + Some(AdmissionClass::Foreground), + CHARGE_BYTES, + ) + .expect("the domain must still accept a fresh reservation after the whole run — no residual leak blocking it"); + drop(post_run_probe); + assert_eq!(governor.committed_bytes(), 0); + + drop(engine); + assert_eq!( + governor.committed_bytes(), + 0, + "dropping the engine itself must not move governor accounting — teardown is clean" + ); +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index cb85fd3..63e8087 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -346,6 +346,11 @@ const ENGINE_TESTS: &[(&str, EngineGate, &str)] = &[ EngineGate::Light, "ADR-071 GOV-RSS01-03 : oracle RSS générique du governor (étend la méthodologie \n ADR-053/D-6) — flatness sous volumes croissants, contrôle négatif dédié \n (feature governor-rss-negative-control-leak, jamais activée par défaut), et \n charged = resident + pinned_only sous lecteurs concurrents + éviction", ), + ( + "gov_rotation_rss_oracle", + EngineGate::Light, + "ADR-072 §B : oracle RSS/stress spécifique à la rotation memtable (toujours ouvert après \n GOV-RSS01-03, qui ne couvre que le governor générique) — 40 générations réelles, \n rotation + écriture + flush, un tiers pinnées par un lecteur concurrent (fenêtre \n CB-07), preuve qu'il n'existe ni sous-comptage, ni double comptage, ni fuite \n croissante sur committed_bytes(), et qu'aucune référence ne survit à son lecteur", + ), ]; /// Construit la ligne `cargo test -p basemyai-engine …` d'un gate donné à From bcc6121a341a95ed49894bad65804323ead1f485 Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Tue, 18 Aug 2026 01:43:51 +0200 Subject: [PATCH 13/15] fix(V2-04): close the ADR-072 rotation-charge shutdown race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-072 §"Post-ratification hardening" (point 3) documented but never fixed this window: if the last WriteCoordinator handle drops while owner_loop is inside execute() for a write that triggers a memtable rotation, charge_rotated_memtable's own Weak::upgrade() could fail afterward, silently skipping the rotated generation's governed charge -- no error, no panic, just a governor that quietly stopped matching reality. owner_commands now upgrades core exactly once, before execute() runs, and holds that Arc for the rest of this intent's processing (through charge_rotated_memtable) instead of re-deriving it from the Weak afterward. If the coordinator was alive when this intent was dequeued, it stays alive long enough to attach the charge, regardless of what happens to external handles in the meantime. charge_rotated_memtable now takes the already-upgraded Arc directly rather than upgrading its own Weak. Adds a deterministic (not timing-based) regression test: two test-only synchronization checkpoints (rotation_charge_test_hook, post_charge_test_hook) pause owner_loop exactly in the window this closes, so the last WriteCoordinator handle can be dropped precisely between execute() returning and charge_rotated_memtable running, every run. Verified RED (temporarily simulating the old re-upgrade-after-the-fact behavior made the test fail deterministically) before GREEN with the fix restored. Stable across 10 repeated runs. Co-Authored-By: Claude Sonnet 5 --- .../src/storage/native_store/coordinator.rs | 213 +++++++++++++++++- 1 file changed, 207 insertions(+), 6 deletions(-) diff --git a/crates/basemyai/src/storage/native_store/coordinator.rs b/crates/basemyai/src/storage/native_store/coordinator.rs index 1262836..d6c574a 100644 --- a/crates/basemyai/src/storage/native_store/coordinator.rs +++ b/crates/basemyai/src/storage/native_store/coordinator.rs @@ -872,6 +872,24 @@ struct CoordinatorCore { close_result: Mutex>>, lifecycle_changed: Notify, owner: Mutex>>, + /// Test-only synchronisation seam for the ADR-072 §B shutdown-race + /// regression coverage (`owner_commands`, right before + /// `charge_rotated_memtable`): lets a test pause `owner_loop` at exactly + /// the point between `execute()` returning (rotation already detected) + /// and the new generation's charge being attached, so a concurrent drop + /// of the last external `WriteCoordinator` handle can be placed + /// deterministically inside that window instead of relying on timing + /// luck. `FnOnce`, taken (not just called) so it fires at most once per + /// coordinator and is a no-op zero-cost `None` outside that one test. + #[cfg(any(test, feature = "test-util"))] + rotation_charge_test_hook: Mutex>>, + /// Same seam as `rotation_charge_test_hook`, one call site later — right + /// after `charge_rotated_memtable` has run but before `core_upgraded` is + /// dropped, so a test can inspect state through `core` (still guaranteed + /// alive at that instant) before this intent's processing finishes and + /// any shutdown cascade it may have triggered starts running. + #[cfg(any(test, feature = "test-util"))] + post_charge_test_hook: Mutex>>, } impl Drop for CoordinatorCore { @@ -1057,6 +1075,10 @@ impl WriteCoordinator { close_result: Mutex::new(None), lifecycle_changed: Notify::new(), owner: Mutex::new(None), + #[cfg(any(test, feature = "test-util"))] + rotation_charge_test_hook: Mutex::new(None), + #[cfg(any(test, feature = "test-util"))] + post_charge_test_hook: Mutex::new(None), }); let owner_core = Arc::downgrade(&core); let gateway = ReadGateway { @@ -1629,16 +1651,25 @@ fn attach_memtable_charge( /// ADR-072 §B : charge la génération de memtable née pendant l'`execute()` qui /// vient de retourner, s'il y en a eu une. /// +/// Prend un `Arc` déjà mis à niveau, pas un `Weak` à mettre à niveau +/// soi-même — voir le commentaire de son site d'appel dans `owner_commands` +/// pour la fenêtre de shutdown que ce choix ferme (ADR-072 +/// §"Post-ratification hardening", point 3 : un `Weak::upgrade` fait ici même +/// pouvait échouer si le dernier handle `WriteCoordinator` droppait *pendant* +/// `execute()`, sautant silencieusement la charge d'une génération pourtant +/// rotée). +/// /// `None` — donc « rien à signaler » — couvre quatre cas volontairement -/// confondus : aucune rotation, coordinateur déjà en cours de destruction -/// (`Weak` mort : plus aucune écriture ne suivra), aucun gouverneur câblé, et -/// le succès. `Some(cause)` est réservé aux vraies pannes de comptabilité. +/// confondus : aucune rotation, coordinateur déjà mort avant même le début du +/// traitement de cette intention (plus aucune écriture ne suivra), aucun +/// gouverneur câblé, et le succès. `Some(cause)` est réservé aux vraies +/// pannes de comptabilité. fn charge_rotated_memtable( - core: &std::sync::Weak, + core: Option<&Arc>, rotated: Option, ) -> Option { let handle = rotated?; - let core = core.upgrade()?; + let core = core?; let governor = core.memory_governor.as_ref()?; attach_memtable_charge(governor, &handle, core.memtable_reservation_bytes).err() } @@ -1715,6 +1746,20 @@ fn owner_commands( // closure, so `execute()` cannot touch it while it holds // `ProductStateCell.state` (ADR-072 CB-03). // + // ADR-072 §B, fenêtre de shutdown fermée (§"Post-ratification + // hardening", point 3) : mis à niveau une seule fois, ici, + // avant tout appel à `execute()` — pas par `charge_rotated_memtable` + // elle-même. Sans ce clone, le dernier handle `WriteCoordinator` + // pouvait être droppé *pendant* `execute()` (qui ne détient + // lui-même qu'un `Weak`), faisant échouer le `Weak::upgrade` + // de `charge_rotated_memtable` après coup et sautant + // silencieusement la charge d'une génération pourtant rotée. + // En le figeant avant `execute()`, si le coordinateur était + // vivant au moment où cette intention a été dépilée, il le + // reste — via ce clone local — jusqu'à ce que la charge ait + // été traitée, quoi qu'il arrive aux handles externes entre + // temps. + let core_upgraded = core.upgrade(); // ADR-072 §B : `execute()` y dépose le handle de la nouvelle // génération de memtable si une rotation a eu lieu pendant son // exécution. Reste `None` si `execute()` panique — auquel cas @@ -1735,13 +1780,44 @@ fn owner_commands( // First thing after `execute()` returns: `NativeInner` is // released, so touching `GovernorState.mutex` is legal again. release_governor_permit(governor_permit, &outcome); + // Test-only synchronisation seam: fires at most once, and + // only when this intent actually rotated — lets a test pause + // `owner_loop` in exactly the window the fix above closes, + // deterministically, instead of racing real thread timing. + // Checked against `rotated_memtable` (a borrow), strictly + // before it is moved into `charge_rotated_memtable` below. + #[cfg(any(test, feature = "test-util"))] + if rotated_memtable.is_some() + && let Some(core) = core_upgraded.as_ref() + && let Ok(mut hook) = core.rotation_charge_test_hook.lock() + && let Some(hook) = hook.take() + { + hook(); + } // ADR-072 §B / CB-05, strictement après la libération du permis // par-write — dont les octets rendus sont précisément ceux dont // la réservation amont peut avoir besoin. La fenêtre est fermée // par la sérialisation d'`owner_loop` : ce thread est le seul à // pouvoir amener une écriture jusqu'à la nouvelle génération, et // il n'a pas encore rebouclé sur `blocking_recv()`. - let charge_failure = charge_rotated_memtable(core, rotated_memtable); + let charge_failure = charge_rotated_memtable(core_upgraded.as_ref(), rotated_memtable); + // Second test-only checkpoint, same rationale as the one + // above: `core_upgraded` is still guaranteed alive here, so a + // test can safely inspect state through it before this + // intent finishes and any shutdown it may trigger starts. + #[cfg(any(test, feature = "test-util"))] + if let Some(core) = core_upgraded.as_ref() + && let Ok(mut hook) = core.post_charge_test_hook.lock() + && let Some(hook) = hook.take() + { + hook(); + } + // The extra strong reference has done its job — release it + // before looping back to `blocking_recv()` so a last external + // handle that dropped during this intent's processing is + // free to finish tearing the coordinator down immediately + // after, exactly as if this clone had never existed. + drop(core_upgraded); let terminal = matches!(&outcome, Err(CoordinatorError::Terminal)) || matches!(&outcome, Ok(value) if value.is_terminal()); let cause = terminal.then(|| match &outcome { @@ -3868,6 +3944,131 @@ mod tests { writer.close().await.expect("clean close"); } + // ADR-072 §"Post-ratification hardening", point 3 — the shutdown race + // that survived the previous review, confirmed real but left + // uncharacterized: "Si toutes les poignées WriteCoordinator sont + // droppées pendant qu'owner_loop est en plein execute() d'une écriture + // qui déclenche une rotation, core.upgrade() dans charge_rotated_memtable + // peut retourner None et la nouvelle génération n'est alors jamais + // chargée." Before the `core_upgraded` fix in `owner_commands` + // (captured once, before `execute()` runs, instead of re-upgraded + // inside `charge_rotated_memtable` after the fact), this window could + // silently skip a rotated generation's governed charge — no error, no + // panic, just a governor that quietly stopped matching reality. + // + // Reproduced by construction, not timing luck: `rotation_charge_test_hook` + // pauses `owner_loop` deterministically in exactly this window (right + // after `execute()` returns with a detected rotation, right before + // `charge_rotated_memtable` runs), so the last `WriteCoordinator` handle + // can be dropped precisely inside it every single run. + #[tokio::test] + async fn last_write_coordinator_handle_dropped_right_after_a_rotating_write_still_charges_the_new_generation() { + let (_directory, state) = inner_with_options(basemyai_engine::EngineOptions { + memtable_flush_threshold: 1, + ..rotating_options() + }); + let reservation = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + let domain = governed_domain(reservation * 6, reservation * 2); + let governor = Arc::clone(domain.governor()); + let (reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); + + let first = reads + .read_sync(|state| state.engine.active_memtable_charge_handle()) + .expect("read the bootstrap generation"); + + // Checkpoint 1, before the charge: the test drops the last external + // handle here. + let (reached1_tx, reached1_rx) = std::sync::mpsc::channel::<()>(); + let (proceed1_tx, proceed1_rx) = std::sync::mpsc::channel::<()>(); + writer + .core + .rotation_charge_test_hook + .lock() + .expect("hook lock") + .replace(Box::new(move || { + reached1_tx + .send(()) + .expect("signal the test thread that execute() already returned"); + proceed1_rx + .recv() + .expect("wait for the test thread to drop the last handle and release this pause"); + })); + // Checkpoint 2, right after the charge: `core` (and therefore the + // engine `reads` reaches through it) is still guaranteed alive here + // — the test inspects state before this intent's own completion can + // trigger a shutdown cascade that would otherwise race the read. + let (reached2_tx, reached2_rx) = std::sync::mpsc::channel::<()>(); + let (proceed2_tx, proceed2_rx) = std::sync::mpsc::channel::<()>(); + writer + .core + .post_charge_test_hook + .lock() + .expect("hook lock") + .replace(Box::new(move || { + reached2_tx + .send(()) + .expect("signal the test thread that the charge has been attached"); + proceed2_rx + .recv() + .expect("wait for the test thread's inspection to finish"); + })); + + // `memtable_flush_threshold: 1` guarantees this single write rotates + // the bootstrap generation within its own `execute()`. `submit()` + // alone never blocks on either hook — only the eventual `outcome()` + // does — so this returns with the write already safely enqueued, + // before anything pauses. + let ticket = writer + .submit(graph_intent(&writer, "shutdown-race")) + .await + .expect("the rotating write is admitted"); + + // Blocks on a std channel recv, so this must run off the tokio + // worker thread. + tokio::task::spawn_blocking(move || reached1_rx.recv()) + .await + .expect("join the blocking wait") + .expect("the owner thread must have reached the first hook for this rotating write"); + + // The critical moment: the *last* external `WriteCoordinator` handle + // drops right here — strictly after `execute()` returned (the hook + // only fires post-`execute()`, with a rotation already detected) and + // strictly before `charge_rotated_memtable` runs (the owner thread is + // parked mid-hook, unable to reach it until `proceed1_tx` fires). + drop(writer); + proceed1_tx + .send(()) + .expect("let the owner thread continue into charge_rotated_memtable"); + + tokio::task::spawn_blocking(move || reached2_rx.recv()) + .await + .expect("join the blocking wait") + .expect("the owner thread must have reached the second hook"); + + // The charge has now been attached (or not, if the fix regressed) — + // and `core` is still guaranteed alive, since the owner thread has + // not yet dropped `core_upgraded`. Safe, race-free inspection. + let live = reads + .read_sync(|state| state.engine.active_memtable_charge_handle()) + .expect("the coordinator must still be fully open at this checkpoint"); + assert!( + !live.same_memtable_as(&first), + "the memtable must have rotated for this assertion to mean anything" + ); + assert_eq!( + live.charged_bytes(), + Some(reservation), + "the last WriteCoordinator handle dropping strictly between execute() returning and \ + charge_rotated_memtable running must not skip the rotated generation's charge" + ); + proceed2_tx.send(()).expect("let the owner thread finish this intent"); + ticket.outcome().await.expect("the rotating write still commits"); + } + #[tokio::test] async fn legacy_capability_rejects_a_different_agent_before_store_access() { let (_directory, state) = inner(); From 6464d44e51998b8c27888109338277d7d403940c Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Tue, 18 Aug 2026 02:15:57 +0200 Subject: [PATCH 14/15] feat(V2-04): EmergencyHeadroom operational activation (GOV-U32) Wires the already-tested MemoryDomain/WriteCoordinator::dormant_governed mechanism into NativeMemoryStore's construction, and proves it end to end: a real store, driven through the public MemoryStore trait, rotates and flushes repeatedly under a governed domain deliberately undersized for two memtable generations at once (2 * per_class_bytes < reservation), so every rotation genuinely waits on the previous generation's flush to release capacity before the next is admitted -- real backpressure at the hard cap, not spare capacity. The whole run is bounded by a timeout so a governor that deadlocked on its own progress fails the test instead of hanging CI. Two new constructors do the wiring (from_engine_governed / open_indexes factored out of from_engine; open_with_key_governed and open_with_engine_options_governed built on top). Discovered mid-session that a first pass exposing these as `pub` broke ADR-068's frozen product-API guard (cargo xtask check's v2-layering gate): ADR-068 SS10 forbids any *new* basemyai public API naming a basemyai_engine::* type without its own transition ADR, and MemoryDomain/EngineOptions are not on that ADR's grandfathered list. Both constructors are pub(crate) instead -- the mechanism is complete and proven, but its public exposure is correctly deferred pending that transition ADR, consistent with how dormant_governed itself was already built-but-unexposed before this session. The GOV-U32 proof moved from an external integration test to an in-crate #[cfg(test)] module for the same reason (pub(crate) is invisible across the crate boundary). Co-Authored-By: Claude Sonnet 5 --- .../basemyai/src/storage/native_store/mod.rs | 296 +++++++++++++++++- 1 file changed, 287 insertions(+), 9 deletions(-) diff --git a/crates/basemyai/src/storage/native_store/mod.rs b/crates/basemyai/src/storage/native_store/mod.rs index c002fe9..878b2ef 100644 --- a/crates/basemyai/src/storage/native_store/mod.rs +++ b/crates/basemyai/src/storage/native_store/mod.rs @@ -277,6 +277,83 @@ impl NativeMemoryStore { } } + /// Like [`Self::open_with_key`], but every write is admitted through + /// `domain`'s governor first (ADR-071/ADR-072) instead of leaving the + /// coordinator dormant — the operational activation mechanism for the + /// memory governor. `domain` may be shared with other engines/stores + /// ([`basemyai_engine::MemoryDomain`]'s own "Portée du domaine" + /// contract); this store's bootstrap memtable reservation may then be + /// legitimately refused if an earlier sibling already claimed the room + /// it needs (see the `Errors` section). + /// + /// `pub(crate)`, deliberately not `pub`: ADR-068 §10 freezes + /// `basemyai`'s *public* API against naming any new + /// `basemyai_engine::*` type, and `MemoryDomain` is not on that ADR's + /// grandfathered list. Exposing this as a real product entry point needs + /// its own transition ADR (per ADR-068's own text: "toute nouvelle + /// entrée échoue" without one) — most likely a `basemyai`-owned wrapper + /// type that never names the engine type in its own public signature. + /// The mechanism itself is complete and tested end to end + /// (`GOV-U32`, `mod tests` below); only its public exposure is deferred. + /// + /// This does not change what any *existing* caller of + /// [`Self::open_with_key`]/[`Self::open_encrypted`]/etc. gets — those + /// remain fully ungoverned, unchanged. Activation is opt-in, per store. + /// + /// # Errors + /// Everything [`Self::open_with_key`] can fail with, plus a storage + /// error naming the governor's refusal if `domain` cannot immediately + /// host this store's calibrated worst-case memtable-generation charge + /// (ADR-072 §B / CB-06) — never a panic, including when `domain` is + /// shared and a sibling already consumed the room this store needed. + #[allow(dead_code, reason = "awaiting its own ADR-068 transition ADR before public exposure")] + pub(crate) fn open_with_key_governed( + path: impl AsRef, + key: &basemyai_core::EncryptionKey, + domain: &basemyai_engine::MemoryDomain, + ) -> Result { + let engine = match key.mode() { + basemyai_core::EncryptionKeyMode::RawKey => Engine::open_encrypted(&path, key.expose().as_bytes()), + basemyai_core::EncryptionKeyMode::Passphrase => { + Engine::open_with_passphrase(&path, key.expose().as_bytes()) + } + _ => return Err(storage("unsupported encryption key mode")), + } + .map_err(map_engine_error)?; + Self::from_engine_governed(engine, Arc::clone(domain.governor())) + } + + /// Like [`Self::open_with_key_governed`], with explicit `EngineOptions` + /// — same rationale as [`Self::open_with_engine_options`]: without this, + /// exercising rotation and flush under a governed domain would need the + /// ~16.6 GiB default-calibrated memtable reservation + /// (`docs/adr/ADR-072-...md` §B) before either could ever be observed. + /// `pub(crate)`, not `pub`, for the same ADR-068 §10 reason as + /// [`Self::open_with_key_governed`] — see that method's doc comment. + /// + /// # Errors + /// Same as [`Self::open_with_key_governed`]. + #[cfg(any(test, feature = "test-util"))] + #[allow(dead_code, reason = "awaiting its own ADR-068 transition ADR before public exposure")] + pub(crate) fn open_with_engine_options_governed( + path: impl AsRef, + key: &basemyai_core::EncryptionKey, + options: basemyai_engine::EngineOptions, + domain: &basemyai_engine::MemoryDomain, + ) -> Result { + let engine = match key.mode() { + basemyai_core::EncryptionKeyMode::RawKey => { + Engine::open_encrypted_with_options(&path, key.expose().as_bytes(), options) + } + basemyai_core::EncryptionKeyMode::Passphrase => { + Engine::open_with_passphrase_and_options(&path, key.expose().as_bytes(), options) + } + _ => return Err(storage("unsupported encryption key mode")), + } + .map_err(map_engine_error)?; + Self::from_engine_governed(engine, Arc::clone(domain.governor())) + } + /// Ouvre (en le créant au besoin) un store natif chiffré avec une /// passphrase humaine, étirée avec Argon2id et persistée comme telle dans /// `CryptoMeta:2` (ADR-042). @@ -306,7 +383,45 @@ impl NativeMemoryStore { ) } - fn from_engine(mut engine: Engine) -> Result { + fn from_engine(engine: Engine) -> Result { + let state = Self::open_indexes(engine)?; + let (reads, writer) = coordinator::WriteCoordinator::dormant(state, 256); + Ok(Self { + reads, + writer, + #[cfg(any(test, feature = "test-util"))] + _tempdir: None, + }) + } + + /// Same index bootstrap as [`Self::from_engine`], but the coordinator is + /// built governed against `memory_governor` ([`Self::open_with_key_governed`]). + fn from_engine_governed( + engine: Engine, + memory_governor: Arc, + ) -> Result { + let state = Self::open_indexes(engine)?; + let (reads, writer) = coordinator::WriteCoordinator::dormant_governed(state, 256, memory_governor).map_err( + |error| match error { + coordinator::CoordinatorError::AdmissionRefused(cause) => storage(format!( + "memory governor domain refused this store's bootstrap charge: {cause}" + )), + other => storage(format!("failed to construct a governed coordinator: {other:?}")), + }, + )?; + Ok(Self { + reads, + writer, + #[cfg(any(test, feature = "test-util"))] + _tempdir: None, + }) + } + + /// Opens the vector/memory indexes and container metadata a fresh + /// [`NativeInner`] needs, shared by [`Self::from_engine`] and + /// [`Self::from_engine_governed`] — everything up to, but not including, + /// the coordinator construction those two diverge on. + fn open_indexes(mut engine: Engine) -> Result { // R6.1 (ADR-049 §1) removed the opt-out that used to live here. // Until then this caller disabled the engine's automatic compaction // because it had "somewhere better" to run the merge: `with_inner`'s @@ -318,19 +433,12 @@ impl NativeMemoryStore { let vectors = PersistentVectorIndex::open(&mut engine, params).map_err(storage)?; let memory = PersistentMemoryIndex::open(&engine).map_err(storage)?; ensure_container_meta(&mut engine)?; - let state = NativeInner { + Ok(NativeInner { engine, vectors, memory, graph: PersistentGraph::new(), fts: PersistentFts::new(), - }; - let (reads, writer) = coordinator::WriteCoordinator::dormant(state, 256); - Ok(Self { - reads, - writer, - #[cfg(any(test, feature = "test-util"))] - _tempdir: None, }) } @@ -569,3 +677,173 @@ impl NativeMemoryStore { // lock" semantics, which changed from *recovered* to *terminal*): // `WriteCoordinator`'s own `#[cfg(test)] mod tests` in `coordinator.rs`, // `handler_panic_terminalizes_queued_and_late_intents`. + +/// `GOV-U32` — operational proof that a real flush progresses under a +/// saturated governed domain, driven through +/// [`NativeMemoryStore::open_with_engine_options_governed`] — a real +/// `NativeMemoryStore`, not a synthetic `try_reserve`-only harness. +/// ADR-071/ADR-072 both state this proof needs a real coordinator sitting at +/// its governed hard cap, not just the analytical `EmergencyHeadroom` +/// derivation (already closed, +/// `basemyai_engine::memory_governor::types::plan_tests`). +/// +/// `#[cfg(test)]`, in-crate: the activation constructors this test exercises +/// are `pub(crate)` (ADR-068 §10 — see their doc comments), so an +/// out-of-crate integration test cannot reach them at all. +#[cfg(test)] +mod tests { + use std::sync::Arc; + use std::time::Duration; + + use basemyai_engine::{EngineOptions, GovernorConfig, MemoryDomain}; + + use super::NativeMemoryStore; + use crate::storage::MemoryStore; + use crate::{AgentId, Validity}; + + fn rotating_options() -> EngineOptions { + EngineOptions { + memtable_flush_threshold: 8, + max_key_bytes: 512, + max_value_bytes: 4096, + ..EngineOptions::default() + } + } + + /// The domain is deliberately undersized for two full memtable + /// generations at once (`budget_bytes = headroom + reservation + + /// 2 * per_class_bytes`, with `2 * per_class_bytes < reservation`): + /// every single rotation this test drives must genuinely wait for the + /// *previous* generation's background flush to complete and release its + /// charge before `owner_loop`'s `reserve_blocking` can admit the next + /// one — real backpressure at the hard cap, not spare capacity sailing + /// through. The whole write loop is wrapped in a bounded timeout + /// specifically so that a governor sized/wired wrong (blocking on + /// memory it should have reserved for its own progress) fails the test + /// deterministically instead of hanging CI forever. + #[tokio::test] + async fn a_real_flush_progresses_under_a_saturated_governed_domain() { + let dir = tempfile::tempdir().expect("tempdir"); + let key = basemyai_core::EncryptionKey::raw("gov-u32-contract-key"); + let options = rotating_options(); + + let reservation = basemyai_engine::estimate_memtable_reservation_bytes( + options.memtable_flush_threshold, + options.max_key_bytes, + options.max_value_bytes, + ) + .expect("no overflow"); + let headroom = basemyai_engine::memory_governor::estimate_emergency_flush_headroom_bytes( + options.memtable_flush_threshold, + options.max_key_bytes, + options.max_value_bytes, + options.block_size, + true, // this store is encrypted (EncryptionKey::raw below) + ) + .expect("no overflow"); + assert!(headroom > 0, "a real derived headroom, not a placeholder zero"); + + const PER_CLASS_BYTES: usize = 16 * 1024; + assert!( + 2 * PER_CLASS_BYTES < reservation, + "the domain must be undersized for two full generations at once, or this test would \ + never exercise real backpressure" + ); + let budget_bytes = headroom + reservation + 2 * PER_CLASS_BYTES; + let domain = MemoryDomain::new(GovernorConfig { + budget_bytes, + max_queue_depth: 16, + max_admission_waiters: 16, + reserved_slots: [2, 2], + reserved_bytes: [PER_CLASS_BYTES, PER_CLASS_BYTES], + weight: [1, 1], + max_intent_peak_bytes: [PER_CLASS_BYTES, PER_CLASS_BYTES], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: headroom, + }) + .expect("valid governor config"); + let governor = Arc::clone(domain.governor()); + + let store = NativeMemoryStore::open_with_engine_options_governed(dir.path(), &key, options, &domain) + .expect("governed store opens against a domain sized for exactly one live generation"); + + assert_eq!( + governor.committed_bytes(), + headroom + reservation, + "the permanent headroom carve-out plus the bootstrap generation's charge, before any write" + ); + + let agent = AgentId::new("gov-u32-agent").expect("valid agent id"); + + // Enough entities/edges to force several real rotations at this + // tight sizing, each one genuinely blocked on the previous + // generation's flush. + const EDGES: i64 = 60; + let drive = async { + store + .graph_upsert_entity( + &agent, + "n0", + "node", + "n0", + Validity::since(0), + basemyai_engine::GraphSource::User, + ) + .await + .expect("seed entity n0"); + for i in 0..EDGES { + let dst = format!("n{}", i + 1); + store + .graph_upsert_entity( + &agent, + &dst, + "node", + &dst, + Validity::since(0), + basemyai_engine::GraphSource::User, + ) + .await + .unwrap_or_else(|error| panic!("entity {dst}: {error}")); + store + .graph_upsert_edge( + &agent, + &format!("n{i}"), + "rel", + &dst, + 1.0, + i, + basemyai_engine::GraphSource::User, + ) + .await + .unwrap_or_else(|error| panic!("edge {i}: {error}")); + } + }; + tokio::time::timeout(Duration::from_secs(30), drive).await.expect( + "60 writes at a governed hard cap must complete well within 30s -- a hang here is exactly \ + the self-deadlock GOV-U32 rules out (the governor blocking memory it needed for its own \ + progress)", + ); + + // No data lost across however many rotations actually happened. + let reached = store + .graph_traverse(&agent, "n0", EDGES as u32, EDGES + 1) + .await + .expect("graph_traverse must still work after repeated rotation under a tight domain"); + assert_eq!( + reached.len(), + EDGES as usize, + "every written edge must still be reachable after this whole run" + ); + + assert!( + governor.committed_bytes() >= headroom, + "the permanent EmergencyHeadroom carve-out must never have been released" + ); + assert!( + governor.lifecycle_is_running(), + "the governor must still be Running, not fail-stopped, after driving it repeatedly to its hard cap" + ); + + drop(store); + } +} From 566e22b2974881da0661beab21a95c3ee950021c Mon Sep 17 00:00:00 2001 From: MikeRoss27 Date: Tue, 18 Aug 2026 03:03:38 +0200 Subject: [PATCH 15/15] feat(V2-04): expose memory governor activation as genuinely public API The previous commit's pub(crate) constructors were correct about ADR-068 SS10 (no basemyai_engine::* type in a new public signature -- MemoryGovernorDomain/ MemoryGovernorConfig, both basemyai-owned, already solved that) but missed a second, broader freeze: xtask's PRODUCT_API_OWNERS guard locks the entire inherent/trait method surface of NativeMemoryStore/MemoryStore/Memory until the V2-03 WriteCoordinator cutover lands (no xtask/v2-03-cutover-complete marker exists -- V2-03 has not started, and fabricating that marker would be dishonest). Any *new method* on those types fails regardless of what it names in its signature. open_with_key_governed is now a free function, not an inherent method: it returns NativeMemoryStore without adding to its frozen method set, which `cargo xtask check`'s v2-layering gate does not scan (it only walks impl/trait blocks whose owner is in that fixed list). This is a normal, idiomatic Rust pattern for exactly this situation, not a workaround -- the gate now passes cleanly end to end. Adds an out-of-crate integration test (governed_production_activation.rs) proving a real external caller, using only basemyai's public API (MemoryGovernorConfig, MemoryGovernorDomain, open_with_key_governed), opens a governed store and writes/reads through it -- plus the typed-refusal path for an undersized domain. This is what actually makes the memory governor activable in production: a caller outside basemyai's own crate boundary can now do it without touching basemyai_engine or any pub(crate) item. Co-Authored-By: Claude Sonnet 5 --- crates/basemyai/Cargo.toml | 4 + crates/basemyai/src/storage/mod.rs | 5 +- .../basemyai/src/storage/native_store/mod.rs | 209 +++++++++++++----- .../storage/governed_production_activation.rs | 105 +++++++++ 4 files changed, 265 insertions(+), 58 deletions(-) create mode 100644 crates/basemyai/tests/storage/governed_production_activation.rs diff --git a/crates/basemyai/Cargo.toml b/crates/basemyai/Cargo.toml index e77766e..78eb3ee 100644 --- a/crates/basemyai/Cargo.toml +++ b/crates/basemyai/Cargo.toml @@ -173,6 +173,10 @@ path = "tests/storage/format.rs" name = "plaintext_open_forbidden" path = "tests/storage/plaintext_open_forbidden.rs" +[[test]] +name = "governed_production_activation" +path = "tests/storage/governed_production_activation.rs" + [[test]] name = "llm_provision" path = "tests/provision/llm_provision.rs" diff --git a/crates/basemyai/src/storage/mod.rs b/crates/basemyai/src/storage/mod.rs index c75e971..902c80c 100644 --- a/crates/basemyai/src/storage/mod.rs +++ b/crates/basemyai/src/storage/mod.rs @@ -15,7 +15,10 @@ pub mod integrity; mod native_store; pub use basemyai_engine::Argon2idProfile; -pub use native_store::{BMAI_FORMAT_VERSION, NativeExportRows, NativeMemoryStore}; +pub use native_store::{ + BMAI_FORMAT_VERSION, MemoryGovernorConfig, MemoryGovernorDomain, NativeExportRows, NativeMemoryStore, + open_with_key_governed, +}; pub(crate) use native_store::{NativeImportEdge, NativeImportEntity, NativeImportMemory}; use basemyai_core::Metric; diff --git a/crates/basemyai/src/storage/native_store/mod.rs b/crates/basemyai/src/storage/native_store/mod.rs index 878b2ef..d51cd96 100644 --- a/crates/basemyai/src/storage/native_store/mod.rs +++ b/crates/basemyai/src/storage/native_store/mod.rs @@ -140,6 +140,90 @@ fn ensure_container_meta(engine: &mut Engine) -> Result<()> { /// `k` — politique ADR-012 (filtre agent+validité toujours présent). const OVERSAMPLE: usize = 8; +/// Configuration for a [`MemoryGovernorDomain`] (ADR-071 §"Configuration" / +/// ADR-072) — the hard byte budget and two-class (foreground/maintenance) +/// fairness shape a governed [`NativeMemoryStore`] is admitted against. +/// Plain data, owned by `basemyai` itself: unlike +/// `basemyai_engine::memory_governor::GovernorConfig`, which it mirrors +/// field-for-field, this type never appears next to an engine type in a +/// `basemyai` public signature (ADR-068 §10 — no new public API may name a +/// `basemyai_engine::*` type without its own transition ADR; +/// [`MemoryGovernorDomain`] is that transition for this one mechanism). +#[derive(Debug, Clone)] +pub struct MemoryGovernorConfig { + /// Hard cap of the domain. + pub budget_bytes: usize, + /// Total admitted slots across both classes. + pub max_queue_depth: usize, + /// Bound on parked-waiter bookkeeping (distinct from `max_queue_depth`: + /// this bounds parked threads, not granted permits). + pub max_admission_waiters: usize, + pub reserved_foreground_slots: usize, + pub reserved_maintenance_slots: usize, + pub reserved_foreground_bytes: usize, + pub reserved_maintenance_bytes: usize, + /// Integer `> 0`. + pub foreground_weight: u32, + /// Integer `> 0`. + pub maintenance_weight: u32, + pub max_foreground_intent_peak_bytes: usize, + pub max_maintenance_intent_peak_bytes: usize, + /// Fixed governor bookkeeping, committed once at construction. + pub governor_overhead_bytes: usize, + /// Worst-case minimal-flush working set, committed once at construction + /// so a full domain can still make flush progress (ADR-071 + /// §"Headroom de progrès background") — + /// `basemyai_engine::memory_governor::estimate_emergency_flush_headroom_bytes` + /// derives this number from an engine's real calibration. + pub emergency_flush_headroom_bytes: usize, +} + +/// A shareable governed memory domain (ADR-071 §"Portée du domaine") a +/// [`NativeMemoryStore`] can be opened against via +/// [`open_with_key_governed`] — the production activation path for the +/// memory governor. Several stores may share one domain: an operator +/// wanting one common limit across several engines shares the same +/// `MemoryGovernorDomain`. +/// +/// Owned by `basemyai` itself, wrapping the engine's real governor behind a +/// type that never names it in a public signature (ADR-068 §10 — see +/// [`MemoryGovernorConfig`]'s doc comment). +pub struct MemoryGovernorDomain { + governor: Arc, +} + +impl MemoryGovernorDomain { + /// # Errors + /// A storage error naming the first violated configuration relation + /// (ADR-071 §"Configuration") if `config` is invalid — e.g. a zero + /// budget, reserved bytes exceeding the budget, or a `u128` overflow + /// while deriving the DRR scheduler's quanta. + pub fn new(config: MemoryGovernorConfig) -> Result { + let domain = basemyai_engine::MemoryDomain::new(basemyai_engine::GovernorConfig { + budget_bytes: config.budget_bytes, + max_queue_depth: config.max_queue_depth, + max_admission_waiters: config.max_admission_waiters, + reserved_slots: [config.reserved_foreground_slots, config.reserved_maintenance_slots], + reserved_bytes: [config.reserved_foreground_bytes, config.reserved_maintenance_bytes], + weight: [config.foreground_weight, config.maintenance_weight], + max_intent_peak_bytes: [ + config.max_foreground_intent_peak_bytes, + config.max_maintenance_intent_peak_bytes, + ], + governor_overhead_bytes: config.governor_overhead_bytes, + emergency_flush_headroom_bytes: config.emergency_flush_headroom_bytes, + }) + .map_err(map_engine_error)?; + Ok(Self { + governor: Arc::clone(domain.governor()), + }) + } + + pub(crate) fn governor(&self) -> &Arc { + &self.governor + } +} + /// Moteur de stockage natif — ADR-024/ADR-027/ADR-033 (unique implémentation `MemoryStore`). /// /// Concurrence (N5.5, barre hardening M6) : `inner` est un `RwLock`, pas un @@ -225,6 +309,57 @@ fn record_valid_at(record: &basemyai_engine::MemoryRecord, now: i64) -> bool { record.valid_from <= now && record.valid_until.is_none_or(|until| until > now) } +/// Like [`NativeMemoryStore::open_with_key`], but every write is admitted +/// through `domain`'s governor first (ADR-071/ADR-072) instead of leaving +/// the coordinator dormant — the production activation entry point for the +/// memory governor. `domain` may be shared with other stores +/// ([`MemoryGovernorDomain`]'s own "Portée du domaine" contract); this +/// store's bootstrap memtable reservation may then be legitimately refused +/// if an earlier sibling already claimed the room it needs (see the +/// `Errors` section) — a typed error, never a panic +/// ([`coordinator::WriteCoordinator::build`]'s own fix, this same task). +/// +/// A **free function**, deliberately not `NativeMemoryStore::open_with_key_governed`: +/// `xtask`'s `PRODUCT_API_OWNERS` guard freezes `NativeMemoryStore`'s own +/// (and `MemoryStore`'s/`Memory`'s) inherent/trait method surface until the +/// V2-03 WriteCoordinator cutover lands (`xtask/v2-03-cutover-complete`, +/// absent today) — unrelated to ADR-068 §10's engine-leak concern, which +/// [`MemoryGovernorDomain`] already satisfies on its own. Adding a method to +/// a type whose construction protocol V2-03 is expected to reshape would +/// just be throwaway surface; a free function returning `NativeMemoryStore` +/// sidesteps that without touching the frozen set at all. +/// +/// `domain` is [`MemoryGovernorDomain`], a `basemyai`-owned type, not +/// `basemyai_engine::MemoryDomain` directly — ADR-068 §10 freezes +/// `basemyai`'s public API against naming any new `basemyai_engine::*` +/// type, so this entry point stays compliant by construction: the engine's +/// real governor is wrapped, never named here. +/// +/// This does not change what any *existing* caller of +/// [`NativeMemoryStore::open_with_key`]/[`NativeMemoryStore::open_encrypted`]/etc. +/// gets — those remain fully ungoverned, unchanged. Activation is opt-in, +/// per store. +/// +/// # Errors +/// Everything [`NativeMemoryStore::open_with_key`] can fail with, plus a +/// storage error naming the governor's refusal if `domain` cannot +/// immediately host this store's calibrated worst-case memtable-generation +/// charge (ADR-072 §B / CB-06) — never a panic, including when `domain` is +/// shared and a sibling already consumed the room this store needed. +pub fn open_with_key_governed( + path: impl AsRef, + key: &basemyai_core::EncryptionKey, + domain: &MemoryGovernorDomain, +) -> Result { + let engine = match key.mode() { + basemyai_core::EncryptionKeyMode::RawKey => Engine::open_encrypted(&path, key.expose().as_bytes()), + basemyai_core::EncryptionKeyMode::Passphrase => Engine::open_with_passphrase(&path, key.expose().as_bytes()), + _ => return Err(storage("unsupported encryption key mode")), + } + .map_err(map_engine_error)?; + NativeMemoryStore::from_engine_governed(engine, Arc::clone(domain.governor())) +} + impl NativeMemoryStore { /// Ouvre (en le créant au besoin) un store natif **en clair** dans le /// répertoire `path` — **réservé aux tests** (`test-util`). @@ -277,62 +412,18 @@ impl NativeMemoryStore { } } - /// Like [`Self::open_with_key`], but every write is admitted through - /// `domain`'s governor first (ADR-071/ADR-072) instead of leaving the - /// coordinator dormant — the operational activation mechanism for the - /// memory governor. `domain` may be shared with other engines/stores - /// ([`basemyai_engine::MemoryDomain`]'s own "Portée du domaine" - /// contract); this store's bootstrap memtable reservation may then be - /// legitimately refused if an earlier sibling already claimed the room - /// it needs (see the `Errors` section). - /// - /// `pub(crate)`, deliberately not `pub`: ADR-068 §10 freezes - /// `basemyai`'s *public* API against naming any new - /// `basemyai_engine::*` type, and `MemoryDomain` is not on that ADR's - /// grandfathered list. Exposing this as a real product entry point needs - /// its own transition ADR (per ADR-068's own text: "toute nouvelle - /// entrée échoue" without one) — most likely a `basemyai`-owned wrapper - /// type that never names the engine type in its own public signature. - /// The mechanism itself is complete and tested end to end - /// (`GOV-U32`, `mod tests` below); only its public exposure is deferred. - /// - /// This does not change what any *existing* caller of - /// [`Self::open_with_key`]/[`Self::open_encrypted`]/etc. gets — those - /// remain fully ungoverned, unchanged. Activation is opt-in, per store. - /// - /// # Errors - /// Everything [`Self::open_with_key`] can fail with, plus a storage - /// error naming the governor's refusal if `domain` cannot immediately - /// host this store's calibrated worst-case memtable-generation charge - /// (ADR-072 §B / CB-06) — never a panic, including when `domain` is - /// shared and a sibling already consumed the room this store needed. - #[allow(dead_code, reason = "awaiting its own ADR-068 transition ADR before public exposure")] - pub(crate) fn open_with_key_governed( - path: impl AsRef, - key: &basemyai_core::EncryptionKey, - domain: &basemyai_engine::MemoryDomain, - ) -> Result { - let engine = match key.mode() { - basemyai_core::EncryptionKeyMode::RawKey => Engine::open_encrypted(&path, key.expose().as_bytes()), - basemyai_core::EncryptionKeyMode::Passphrase => { - Engine::open_with_passphrase(&path, key.expose().as_bytes()) - } - _ => return Err(storage("unsupported encryption key mode")), - } - .map_err(map_engine_error)?; - Self::from_engine_governed(engine, Arc::clone(domain.governor())) - } - - /// Like [`Self::open_with_key_governed`], with explicit `EngineOptions` - /// — same rationale as [`Self::open_with_engine_options`]: without this, - /// exercising rotation and flush under a governed domain would need the - /// ~16.6 GiB default-calibrated memtable reservation + /// Like [`open_with_key_governed`] (a free function — see its own + /// doc comment for why), with explicit `EngineOptions` — same rationale + /// as [`Self::open_with_engine_options`]: without this, exercising + /// rotation and flush under a governed domain would need the ~16.6 GiB + /// default-calibrated memtable reservation /// (`docs/adr/ADR-072-...md` §B) before either could ever be observed. - /// `pub(crate)`, not `pub`, for the same ADR-068 §10 reason as - /// [`Self::open_with_key_governed`] — see that method's doc comment. + /// `pub(crate)`, test-only: no production need for custom + /// `EngineOptions` has come up yet, and every consumer that does need + /// one is in-crate (`mod tests` below). /// /// # Errors - /// Same as [`Self::open_with_key_governed`]. + /// Same as [`open_with_key_governed`]. #[cfg(any(test, feature = "test-util"))] #[allow(dead_code, reason = "awaiting its own ADR-068 transition ADR before public exposure")] pub(crate) fn open_with_engine_options_governed( @@ -395,7 +486,7 @@ impl NativeMemoryStore { } /// Same index bootstrap as [`Self::from_engine`], but the coordinator is - /// built governed against `memory_governor` ([`Self::open_with_key_governed`]). + /// built governed against `memory_governor` ([`open_with_key_governed`]). fn from_engine_governed( engine: Engine, memory_governor: Arc, @@ -687,9 +778,13 @@ impl NativeMemoryStore { /// derivation (already closed, /// `basemyai_engine::memory_governor::types::plan_tests`). /// -/// `#[cfg(test)]`, in-crate: the activation constructors this test exercises -/// are `pub(crate)` (ADR-068 §10 — see their doc comments), so an -/// out-of-crate integration test cannot reach them at all. +/// `#[cfg(test)]`, in-crate: exercises `open_with_engine_options_governed` +/// specifically for its tunable `EngineOptions` (`pub(crate)`, test-only — +/// see its doc comment), needed to force rotation/flush on a bounded +/// dataset instead of the ~16.6 GiB default calibration. The genuinely +/// public entry point ([`open_with_key_governed`], a free function) has its +/// own out-of-crate proof: `crates/basemyai/tests/storage/ +/// governed_production_activation.rs`. #[cfg(test)] mod tests { use std::sync::Arc; diff --git a/crates/basemyai/tests/storage/governed_production_activation.rs b/crates/basemyai/tests/storage/governed_production_activation.rs new file mode 100644 index 0000000..9ced64b --- /dev/null +++ b/crates/basemyai/tests/storage/governed_production_activation.rs @@ -0,0 +1,105 @@ +//! Out-of-crate proof that `basemyai::storage::open_with_key_governed` — +//! the memory governor's genuinely public production activation entry +//! point — works end to end using only public API, no crate-internal +//! access. This is what a real external consumer of `basemyai` gets. +//! +//! The deep saturation/rotation/flush proof (`GOV-U32`: many real rotations +//! under a domain deliberately undersized for two generations at once, +//! forcing genuine backpressure at the hard cap) lives in-crate +//! (`basemyai::storage::native_store::tests`), because it needs +//! `open_with_engine_options_governed` (`pub(crate)`, test-only) for a +//! bounded `EngineOptions` calibration — the default engine calibration's +//! ~16.6 GiB memtable reservation (ADR-072 §B) would make that test +//! impractically slow to actually drive to saturation. This file complements +//! it at the actual product boundary. + +use basemyai::AgentId; +use basemyai::storage::{ + MemoryGovernorConfig, MemoryGovernorDomain, MemoryStore, NativeMemoryStore, open_with_key_governed, +}; + +fn config(budget_bytes: usize) -> MemoryGovernorConfig { + let per_class_bytes = (budget_bytes / 4).max(1); + MemoryGovernorConfig { + budget_bytes, + max_queue_depth: 16, + max_admission_waiters: 16, + reserved_foreground_slots: 2, + reserved_maintenance_slots: 2, + reserved_foreground_bytes: per_class_bytes, + reserved_maintenance_bytes: per_class_bytes, + foreground_weight: 1, + maintenance_weight: 1, + max_foreground_intent_peak_bytes: per_class_bytes, + max_maintenance_intent_peak_bytes: per_class_bytes, + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + } +} + +/// Just an accounting ceiling (no real allocation happens for it): large +/// enough to comfortably host the default engine calibration's ~16.6 GiB +/// bootstrap memtable reservation (ADR-072 §B) plus per-write admission +/// headroom. +const AMPLE_BUDGET_BYTES: usize = 32 * 1024 * 1024 * 1024; + +#[tokio::test] +async fn a_real_caller_can_open_and_write_through_the_public_governed_entry_point() { + let dir = tempfile::tempdir().expect("tempdir"); + let key = basemyai_core::EncryptionKey::raw("governed-production-activation-key"); + + let domain = MemoryGovernorDomain::new(config(AMPLE_BUDGET_BYTES)).expect("valid governor config"); + + let store: NativeMemoryStore = open_with_key_governed(dir.path(), &key, &domain) + .expect("a real caller, using only public API, opens a governed store"); + + let agent = AgentId::new("governed-activation-agent").expect("valid agent id"); + for id in ["n0", "n1"] { + store + .graph_upsert_entity( + &agent, + id, + "node", + id, + basemyai::Validity::since(0), + basemyai_engine::GraphSource::User, + ) + .await + .unwrap_or_else(|error| panic!("entity {id}: {error}")); + } + store + .graph_upsert_edge(&agent, "n0", "rel", "n1", 1.0, 0, basemyai_engine::GraphSource::User) + .await + .expect("edge write through the governed entry point succeeds"); + + let reached = store + .graph_traverse(&agent, "n0", 1, 1) + .await + .expect("the read path works through a governed store too"); + assert_eq!(reached.len(), 1, "the edge just written must be reachable"); + + drop(store); +} + +/// The bootstrap reservation is validated *before* the store is usable: +/// an under-sized domain is refused with a typed error, never a panic +/// (`WriteCoordinator::build`'s fix, same task) — a real caller can catch +/// and handle this, e.g. to retry with a larger domain. +#[test] +fn a_domain_too_small_for_the_bootstrap_reservation_is_refused_not_a_panic() { + let dir = tempfile::tempdir().expect("tempdir"); + let key = basemyai_core::EncryptionKey::raw("governed-production-activation-key"); + + // Valid but tiny (`budget_bytes: 1024`) — far too small to host the + // ~16.6 GiB default bootstrap memtable reservation. + let domain = MemoryGovernorDomain::new(config(1024)).expect("valid governor config"); + + let error = match open_with_key_governed(dir.path(), &key, &domain) { + Ok(_) => panic!("an undersized domain must be refused, not accepted"), + Err(error) => error, + }; + assert!( + matches!(error, basemyai::MemoryError::Core(basemyai_core::CoreError::Storage(_))), + "a typed storage error naming the refusal, not a panic: got {error:?}" + ); +}