diff --git a/crates/epiphany-bundle/DECISIONS.md b/crates/epiphany-bundle/DECISIONS.md index 934018d..c9e88c7 100644 --- a/crates/epiphany-bundle/DECISIONS.md +++ b/crates/epiphany-bundle/DECISIONS.md @@ -411,3 +411,20 @@ acceleration full-`Score` snapshot yet, so its role gate waits for a real producer (the core-side seam `decode_canonical_versioned` already handles {0,1,2}). `SchemaVersion::V2` added; beyond-accept-set tests moved to major 3. + +### Follow-up (review): the snapshot role gets its producer + the base gets a role bound + +A post-commit review caught the criterion-4 harness bypassing the versioned +snapshot contract (current-major `Score` bytes stamped V0 in the +`canonical_base` slot, decoded with the unversioned decoder). Fixed the +substantive way: the harness now stages a **properly-roled acceleration +snapshot** — `ChunkKind::Snapshot` stamped `SchemaVersion::for_major(2)`, +referenced from `Manifest::acceleration_snapshots`, decoded through +`Score::decode_canonical_versioned(bytes, root.schema_version.major)` — so +the schema-major snapshot contract is exercised end-to-end through the +bundle. Consequences: `max_supported_major(Snapshot)` → 2 (superseding the +"waits for a producer" note above), and because the per-kind gate no longer +implies it, the canonical-base-stays-major-0 rule is now enforced per ROLE +(`mis_stamped_canonical_base`, consulted at open and commit → read-only + +`UnsupportedCanonicalChunkMajor`, regression-locked). The `SnapshotId` in +the harness remains a hash-truncation stand-in (companion open question). diff --git a/crates/epiphany-bundle/src/bundle.rs b/crates/epiphany-bundle/src/bundle.rs index 6697faf..37a880a 100644 --- a/crates/epiphany-bundle/src/bundle.rs +++ b/crates/epiphany-bundle/src/bundle.rs @@ -53,17 +53,25 @@ pub const SUPPORTED_SCHEMA_MAJOR: u16 = 0; /// `OperationEnvelopeBlock` admits major 2 (schema major 2 fills the /// cross-cutting/staff/metadata bodies its payloads embed; major 1 embedded a /// v1 `CreateRegion`; the reader treats the block bytes opaquely, so it -/// parses a higher-major block without decoding the payload). Every other -/// role stays at -/// [`SUPPORTED_SCHEMA_MAJOR`] until its own versioned path lands — including the -/// payload-polymorphic `Snapshot` (the acceleration form's migrate-on-read is -/// core-side; the canonical base stays major 0) and the manifest (carried -/// opaquely, never grows a v1 layout). A chunk above its role's max is not +/// parses a higher-major block without decoding the payload). `Snapshot` +/// admits major 2 for the acceleration full-`Score` form (decoded through +/// the core versioned seam); the canonical BASE carried under the same kind +/// must stay major 0, enforced per role. Every other role stays at +/// [`SUPPORTED_SCHEMA_MAJOR`] until its own versioned path lands — the +/// layout cache, the operation index, and the manifest (carried opaquely, +/// never grows a versioned layout). A chunk above its role's max is not /// admitted; for a **canonical** role that means the bundle opens read-only -/// (a major-0-only reader meeting a v1 op block), not a hard reject. +/// (a lower-major-only reader meeting a newer op block), not a hard reject. pub fn max_supported_major(kind: ChunkKind) -> u16 { match kind { ChunkKind::OperationEnvelopeBlock => 2, + // The payload-polymorphic Snapshot role: the acceleration + // full-`Score` form is decoded through the core versioned seam + // (`Score::decode_canonical_versioned`, majors {0,1,2}). The + // *canonical base* must stay major 0 regardless — that is enforced + // per ROLE (`mis_stamped_canonical_base`, consulted at open and + // commit), not by this per-kind bound. + ChunkKind::Snapshot => 2, _ => SUPPORTED_SCHEMA_MAJOR, } } @@ -102,9 +110,10 @@ pub struct StagedChunk { impl StagedChunk { /// A staged operation-envelope block at schema major 0 (the baseline: no - /// operation in the block carries a schema-major-1 payload). Use + /// operation in the block carries a versioned payload). Use /// [`StagedChunk::operation_block_versioned`] for a block whose operations - /// may include a v1 `CreateRegion`. + /// may carry a higher-major payload (a v1 `CreateRegion`; a v2 + /// cross-cutting/staff/metadata value under minimal stamping). pub fn operation_block(payload: Vec) -> Self { StagedChunk::operation_block_versioned(payload, SchemaVersion::V0) } @@ -383,6 +392,14 @@ impl Bundle { schema_major: major, }); } + // The canonical base is role-bound to major 0 (see + // `mis_stamped_canonical_base`). + if let Some(major) = mis_stamped_canonical_base(&manifest) { + read_only = true; + anomalies.push(IntegrityAnomaly::UnsupportedCanonicalChunkMajor { + schema_major: major, + }); + } let write_cursor = store.len(); Ok(Bundle { @@ -754,6 +771,13 @@ impl Bundle { schema_major: major, }); } + if let Some(major) = mis_stamped_canonical_base(&manifest) { + self.read_only = true; + self.anomalies + .push(IntegrityAnomaly::UnsupportedCanonicalChunkMajor { + schema_major: major, + }); + } self.manifest = manifest; self.write_cursor = cursor; Ok(()) @@ -830,6 +854,21 @@ fn unsupported_operation_root_major(manifest: &Manifest) -> Option { .find(|&m| m > max_supported_major(ChunkKind::OperationEnvelopeBlock)) } +/// The canonical base MUST stay schema major 0 across the data-model majors +/// (Binary Format §Schema Major 1 / §Schema Major 2: the `MaterializedState` +/// embeds none of the filled values, and re-stamping byte-identical content +/// churns its content address). With the Snapshot *kind* now admitting the +/// major-2 acceleration form, this per-ROLE check keeps the base constraint: +/// a base ref stamped above major 0 forces read-only preservation, exactly +/// like a beyond-accept-set op root. +fn mis_stamped_canonical_base(manifest: &Manifest) -> Option { + manifest + .canonical_base + .as_ref() + .map(|b| b.root.schema_version.major) + .filter(|&m| m > 0) +} + /// Whether this implementation *understands* a profile (can interpret and honor /// its constraints): a built-in `ProfileId` (not a `Custom` registry profile), /// a supported major version, and a block bound within the reader's hard chunk @@ -934,14 +973,15 @@ fn read_and_verify_chunk_impl( }); } // A chunk at a schema major this reader cannot parse for its role. The - // accept-set is `[0, max_supported_major(kind)]`: the op-block role admits - // major 1 (D2), every other role stays exact-0 until its versioned path - // lands (Binary Format companion §"Schema Major 1"). A chunk above its - // role's max reaches this only on a direct read — a canonical root beyond - // the accept-set opens the bundle read-only at `open` instead, before any - // such read (see the operation-root scan there). Commit-time structural - // validation skips this gate (a newer writer's higher-major root is still - // structurally valid). + // accept-set is `[0, max_supported_major(kind)]`: the op-block and + // snapshot roles admit major 2, every other role stays exact-0 until its + // versioned path lands (Binary Format companion §"Schema Major 2"). A + // chunk above its role's max reaches this only on a direct read — a + // canonical root beyond the accept-set opens the bundle read-only at + // `open` instead, before any such read (see the operation-root and + // canonical-base scans there). Commit-time structural validation skips + // this gate (a newer writer's higher-major root is still structurally + // valid). if enforce_accept_set && r.schema_version.major > max_supported_major(r.kind) { return Err(BundleError::UnsupportedSchemaVersion { version: r.schema_version, @@ -1259,7 +1299,8 @@ pub fn manifest_chunk_hash(payload: &[u8]) -> ContentHash { #[cfg(test)] mod tests { use super::*; - use crate::ids::DocumentId; + use crate::ids::{DocumentId, FrontierBytes, ReductionAlgorithmVersion, SnapshotId}; + use crate::manifest::SnapshotRef; #[test] fn schema_major_1_admission_is_raised_per_role_op_blocks_only() { @@ -1274,11 +1315,12 @@ mod tests { assert_eq!(SchemaVersion::V2.major, 2); // The op-block role admits [0, 2]. assert_eq!(max_supported_major(ChunkKind::OperationEnvelopeBlock), 2); - // Every other role stays at the generic baseline (major 0): the - // payload-polymorphic Snapshot (its migrate-on-read is core-side), the - // layout cache, and the operation index. + // The snapshot role admits the major-2 acceleration form (decoded + // through the core versioned seam); the canonical BASE stays major 0 + // per role (`mis_stamped_canonical_base`). The remaining roles stay + // at the generic baseline: the layout cache and the operation index. assert_eq!(SUPPORTED_SCHEMA_MAJOR, 0); - assert_eq!(max_supported_major(ChunkKind::Snapshot), 0); + assert_eq!(max_supported_major(ChunkKind::Snapshot), 2); assert_eq!(max_supported_major(ChunkKind::LayoutCache), 0); assert_eq!(max_supported_major(ChunkKind::OperationIndex), 0); // The manifest gate is exact to the manifest's own major (0), independent @@ -1324,6 +1366,57 @@ mod tests { )); } + #[test] + fn a_canonical_base_stamped_above_major_0_opens_read_only() { + // The canonical base is role-bound to major 0 (Binary Format §Schema + // Major 2: the MaterializedState embeds no data-model-major values, + // and re-stamping byte-identical content churns its address). With + // the Snapshot KIND now admitting the major-2 acceleration form, the + // per-role check must catch a mis-stamped base: read-only + anomaly, + // at commit and again on reopen. + let mut bundle = fresh_bundle(); + let base = StagedChunk { + kind: ChunkKind::Snapshot, + schema_version: SchemaVersion::V1, + payload: vec![7u8, 7, 7], + }; + bundle + .commit(&[base], |ctx| { + let mut m = ctx.previous_manifest.clone(); + let root = ctx.new_chunks[0]; + let mut sid = [0u8; 16]; + sid.copy_from_slice(&root.hash.as_bytes()[..16]); + m.canonical_base = Some(SnapshotRef { + snapshot_id: SnapshotId(sid), + covers_causal_frontier: FrontierBytes::from_bytes(vec![]), + reduction_algorithm_version: ReductionAlgorithmVersion(0), + profile_id: ProfileId::Full, + hash: root.hash, + root, + }); + m + }) + .expect("a structurally-valid mis-stamped base is publishable"); + assert!( + bundle.is_read_only(), + "a mis-stamped canonical base forces read-only at commit" + ); + assert!(bundle.anomalies().iter().any(|a| matches!( + a, + IntegrityAnomaly::UnsupportedCanonicalChunkMajor { schema_major: 1 } + ))); + let image = bundle.into_store().into_bytes(); + let reopened = Bundle::open(MemStore::from_bytes(image)).expect("reopen"); + assert!( + reopened.is_read_only(), + "a mis-stamped canonical base forces read-only at open" + ); + assert!(reopened.anomalies().iter().any(|a| matches!( + a, + IntegrityAnomaly::UnsupportedCanonicalChunkMajor { schema_major: 1 } + ))); + } + fn fresh_bundle() -> Bundle { Bundle::create( MemStore::new(), diff --git a/crates/epiphany-core/src/codec.rs b/crates/epiphany-core/src/codec.rs index 1e44476..b3c514c 100644 --- a/crates/epiphany-core/src/codec.rs +++ b/crates/epiphany-core/src/codec.rs @@ -2334,7 +2334,7 @@ impl Score { /// Decodes the exact inverse of [`Score::canonical_bytes`], validating every /// tag, length, primitive, and type invariant. Trailing bytes are rejected. /// - /// This is the **current (schema major 1)** layout. To decode bytes whose + /// This is the **current (schema major 2)** layout. To decode bytes whose /// schema major is not known to be current, use /// [`Score::decode_canonical_versioned`]. /// diff --git a/crates/epiphany-ops/src/payload.rs b/crates/epiphany-ops/src/payload.rs index 5966400..af72a1d 100644 --- a/crates/epiphany-ops/src/payload.rs +++ b/crates/epiphany-ops/src/payload.rs @@ -1062,10 +1062,12 @@ impl CanonicalEncode for ModifyCrossCuttingOp { // children — the caller deletes contents first). See `DECISIONS.md`. /// Mint an empty region into the canvas (Chapter 6 §6.10 InsertRegion). Carries -/// the full [`Region`] value (schema major 1, including `permits_spanning_slurs`); +/// the full [`Region`] value (`permits_spanning_slurs` since schema major 1); /// the reduction preconditions it carries no staff instances (an empty -/// container). Its canonical encoding puts this op at schema major 1 — see -/// [`CreateRegionOp::encode_canonical`] and [`OperationKind::schema_major`]. +/// container). Minimal stamping ([`OperationKind::schema_major`]): the payload +/// is major 1, or major 2 iff a carried staff instance bears +/// `Some(staff_lines_override)` (wire-representable though reduction refuses +/// non-empty regions). #[derive(Clone, PartialEq, Eq, Debug)] pub struct CreateRegionOp { pub region: Region, diff --git a/crates/epiphany-testkit/src/bundle_harness.rs b/crates/epiphany-testkit/src/bundle_harness.rs index 45da9f3..e9534b4 100644 --- a/crates/epiphany-testkit/src/bundle_harness.rs +++ b/crates/epiphany-testkit/src/bundle_harness.rs @@ -24,10 +24,11 @@ use epiphany_ops::{peek_operation_id, OperationEnvelope}; /// Stages real operation envelopes into a single op-envelope block, **deriving** /// the block's schema version from its operations: the block major is the max -/// over `OperationEnvelope::schema_major` (a v1 `CreateRegion` → major 1), -/// mapped to a version by [`SchemaVersion::for_major`]. This is the writer-side -/// derivation every real-envelope staging path must use so a block carrying a -/// v1 payload is never mis-stamped major 0. +/// over `OperationEnvelope::schema_major` under minimal stamping (a v1 +/// `CreateRegion` → major 1; a v2 cross-cutting/staff/metadata value → +/// major 2), mapped to a version by [`SchemaVersion::for_major`]. This is the +/// writer-side derivation every real-envelope staging path must use so a +/// block carrying a versioned payload is never mis-stamped major 0. pub fn stage_operation_block(envelopes: &[OperationEnvelope]) -> StagedChunk { let payloads: Vec> = envelopes.iter().map(|e| e.to_canonical_bytes()).collect(); let major = envelopes diff --git a/crates/epiphany-testkit/src/roundtrip.rs b/crates/epiphany-testkit/src/roundtrip.rs index 8ebc5a2..6791aa3 100644 --- a/crates/epiphany-testkit/src/roundtrip.rs +++ b/crates/epiphany-testkit/src/roundtrip.rs @@ -336,8 +336,12 @@ pub fn assert_score_serialization_stable(score: &Score, frontier: &[u8], seed: u "re-encoding the same score changed its bytes" ); - // serialize: stage the canonical score as a real Snapshot chunk referenced - // from the manifest's canonical_base. + // serialize: stage the score as a properly-roled ACCELERATION snapshot + // (Binary Format §Schema Major 2): a `ChunkKind::Snapshot` stamped with + // the current schema major and referenced from the manifest's + // `acceleration_snapshots` — NOT the canonical base, which is the + // MaterializedState's role and stays major 0. (The `SnapshotId` here is a + // hash-truncation stand-in; its derivation is a companion open question.) let mut rng = Rng::new(seed); let uuid = FileUuid(rng.array16()); let doc = DocumentId(rng.array16()); @@ -345,7 +349,7 @@ pub fn assert_score_serialization_stable(score: &Score, frontier: &[u8], seed: u Bundle::create(MemStore::new(), uuid, Manifest::empty(doc)).expect("create bundle"); let snapshot = StagedChunk { kind: ChunkKind::Snapshot, - schema_version: SchemaVersion::V0, + schema_version: SchemaVersion::for_major(2), payload: canonical.clone(), }; let frontier = frontier.to_vec(); @@ -355,7 +359,7 @@ pub fn assert_score_serialization_stable(score: &Score, frontier: &[u8], seed: u let root = ctx.new_chunks[0]; let mut sid = [0u8; 16]; sid.copy_from_slice(&root.hash.as_bytes()[..16]); - m.canonical_base = Some(SnapshotRef { + m.acceleration_snapshots.push(SnapshotRef { snapshot_id: SnapshotId(sid), covers_causal_frontier: FrontierBytes::from_bytes(frontier.clone()), reduction_algorithm_version: ReductionAlgorithmVersion(0), @@ -368,30 +372,36 @@ pub fn assert_score_serialization_stable(score: &Score, frontier: &[u8], seed: u .expect("commit snapshot"); let image = bundle.into_store().into_bytes(); - // load: reopen, hash-verify, read back byte-identically. + // load: reopen (read-write — an acceleration snapshot at the current + // major is within the snapshot role's accept-set), hash-verify, read the + // referenced chunk back byte-identically. let reopened = Bundle::open(MemStore::from_bytes(image)).expect("reopen bundle"); + assert!( + !reopened.is_read_only(), + "a current-major acceleration snapshot must not force read-only" + ); reopened .verify_canonical_chunks() .expect("canonical chunks intact"); - let base = reopened + let accel = reopened .manifest() - .canonical_base - .as_ref() - .expect("a canonical base"); + .acceleration_snapshots + .first() + .expect("an acceleration snapshot"); + assert_eq!(accel.root.schema_version, SchemaVersion::for_major(2)); let loaded = reopened - .read_chunk(&base.root) + .read_chunk(&accel.root) .expect("read snapshot chunk back"); assert_eq!( loaded, canonical, "score bytes were not preserved through content-addressed storage" ); - // deserialize → equal → reserialize byte-identically. (The canonical base - // is a major-0 chunk; the schema-version dispatch seam for the acceleration - // full-`Score` snapshot is exercised on its own read path in a later phase, - // where a properly-roled acceleration snapshot exists. This harness's base - // decodes with the current codec.) - let decoded = Score::decode_canonical(&loaded).expect("loaded score must decode"); + // deserialize through the SCHEMA-VERSION DISPATCH SEAM, keyed by the + // chunk's stamped major — the read path a real acceleration-snapshot + // consumer uses — then re-serialize byte-identically. + let decoded = Score::decode_canonical_versioned(&loaded, accel.root.schema_version.major) + .expect("loaded score must decode at its stamped major"); assert_eq!(&decoded, score, "decoded score changed"); assert_eq!( decoded.canonical_bytes(),