From dfa7e069a4787a3220beee72de83e1d9ea86df11 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 14:28:24 +0000 Subject: [PATCH 01/18] test(song-curation): preregister Slice 2 core RED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests-only commit — no implementation. Preregisters 43 of the accepted contract's 63 §14 cases against the not-yet-existing apply module: - plan/integrity A1–A8 (incl. the A8 sweep of the five Apply-reachable Slice-1 inventory/replay refusals at the artifact boundary); - chain/index C1–C10 (initial and chained application over the real published output tree, already-applied precedence over the chain law, the exact §7.2 relation refusals, fingerprint-neutral batches, index-internal validation, independent lineages); - labels L1–L9 (four-way authority case table, supersession-evidence consistency, drift excluded by fingerprint); - corpus completeness K1–K9 + K12 (every-chunk-together, raw byte copy of untouched files, laundering guard, no-root-songs law, tree agreement, curated projection, protected curated path, partial vs holdout-ready, recursive reserved-area shape law); - report/index R1, R2, R3, R5 (digest law, record/report binding, publish-neither on pre-publication refusal, curated digest law); - pure filesystem laws F3 (no-trace refusals) and F5 (byte determinism). Plus the shared fixture/builder module (tests/common): corpus trees, serialized plan artifacts, index files, byte-walk comparators, and the contract's staging/lock/temp name derivations — no production behaviour. RED evidence (cargo test --manifest-path song-curation/Cargo.toml): error[E0432]: unresolved import 'griff_song_curation::apply' (both test binaries: apply_core, apply_outputs) error: could not compile 'griff-song-curation' (test "apply_core") error: could not compile 'griff-song-curation' (test "apply_outputs") The tests compile exactly as far as the absent API allows and fail because Slice 2 is not implemented. The frozen Slice-1 lib target still compiles. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/tests/apply_core.rs | 1139 ++++++++++++++++++++++++++ song-curation/tests/apply_outputs.rs | 459 +++++++++++ song-curation/tests/common/mod.rs | 339 ++++++++ 3 files changed, 1937 insertions(+) create mode 100644 song-curation/tests/apply_core.rs create mode 100644 song-curation/tests/apply_outputs.rs create mode 100644 song-curation/tests/common/mod.rs diff --git a/song-curation/tests/apply_core.rs b/song-curation/tests/apply_core.rs new file mode 100644 index 0000000..e62838d --- /dev/null +++ b/song-curation/tests/apply_core.rs @@ -0,0 +1,1139 @@ +//! Slice-2 preregistered acceptance matrix — core cases (ADR-0033 Slice 2 +//! contract §14): plan/integrity A1–A8, chain/index C1–C10, labels L1–L9. +//! +//! Every test is named after its §14 case id. These are the RED tests for the +//! Apply core: they exercise the accepted contract through the serialized +//! artifact boundary (plan file + corpus tree + index file → output tree). + +mod common; + +use common::{ + accept, batch_for, correct, event, layout, lock_path_of, merge, read_manifest, read_value, + reject, split, staging_path_of, tamper_plan, write_corpus, write_empty_index, write_plan, + Chunk, Layout, +}; +use griff_song_curation::apply::{ + apply, ApplicationIndex, ApplyPaths, ApplyRefusal, ApplyRun, AppliedReceipt, + APPLICATIONS_SCHEMA, CURATED_MANIFEST_RELPATH, REPORT_RELPATH, +}; +use griff_song_curation::{decisions_digest, plan_digest, CurationError, DryRunPlan}; +use serde_json::json; +use std::fs; + +fn run(l: &Layout) -> ApplyRun { + apply(&ApplyPaths { + plan: l.plan.clone(), + corpus: l.corpus.clone(), + index: l.index.clone(), + output: l.output.clone(), + }) +} + +fn refuse(l: &Layout) -> ApplyRefusal { + run(l).primary.expect_err("expected a refusal") +} + +fn succeed(l: &Layout) -> AppliedReceipt { + run(l).primary.expect("expected success") +} + +fn read_index(l: &Layout) -> ApplicationIndex { + serde_json::from_str(&fs::read_to_string(&l.index).expect("read index")) + .expect("index parses strictly") +} + +/// Refusal from steps 1–8 must leave nothing behind: no output, no staging, +/// no lockfile, index byte-identical. +fn assert_nothing_written(l: &Layout, index_before: &[u8]) { + assert!(!l.output.exists(), "no output tree may exist"); + assert!( + !staging_path_of(&l.output).exists(), + "no staging dir may remain" + ); + assert!( + !lock_path_of(&l.index).exists(), + "the lockfile must be released" + ); + assert_eq!( + fs::read(&l.index).expect("read index"), + index_before, + "the index must be byte-identical" + ); +} + +fn slice1_errors(refusal: &ApplyRefusal) -> &Vec { + match refusal { + ApplyRefusal::PlanVerification { refusals } => refusals, + other => panic!("expected PlanVerification, got {other:?}"), + } +} + +const UNCURATED_AB: &[Chunk<'static>] = &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: None, + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: None, + }, +]; + +// ── A. plan / integrity ──────────────────────────────────────────────────────── + +#[test] +fn a1_valid_serialized_plan_applies() { + let l = layout("a1"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![ + event("ev0", 0, accept("g1", &["shaA"], "song-000001", &[])), + event("ev1", 1, accept("g2", &["shaB"], "song-000002", &[])), + ], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + + let receipt = succeed(&l); + + // Output tree published with the labels applied. + let out_a = read_value(&l.output.join("a1.chunk.json")); + assert_eq!(out_a["source"]["song_id"], json!("song-000001")); + let out_b = read_value(&l.output.join("b1.chunk.json")); + assert_eq!(out_b["source"]["song_id"], json!("song-000002")); + + // Curated manifest and report live in the reserved area. + assert!(l.output.join(CURATED_MANIFEST_RELPATH).exists()); + assert!(l.output.join(REPORT_RELPATH).exists()); + + // The index gained exactly one record binding to the report. + let index = read_index(&l); + assert_eq!(index.schema, APPLICATIONS_SCHEMA); + assert_eq!(index.applications.len(), 1); + let record = &index.applications[0]; + assert_eq!(record.batch_id, "batch1"); + assert_eq!(record.report_digest, receipt.report.report_digest); + assert_eq!( + record.input_corpus_fingerprint, + receipt.report.input_corpus_fingerprint + ); + assert_eq!( + record.output_corpus_fingerprint, + receipt.report.output_corpus_fingerprint + ); + assert!( + !lock_path_of(&l.index).exists(), + "the lock is released after success" + ); +} + +#[test] +fn a2_plan_with_foreign_field_refuses() { + let l = layout("a2"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let before = fs::read(&l.index).expect("index bytes"); + + tamper_plan(&l.plan, |v| common::plant_rogue(v, &[])); + + let refusal = refuse(&l); + assert!( + matches!(refusal, ApplyRefusal::MalformedPlanArtifact { .. }), + "got {refusal:?}" + ); + assert_nothing_written(&l, &before); +} + +#[test] +fn a3_corrupted_plan_digest_refuses() { + let l = layout("a3"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let before = fs::read(&l.index).expect("index bytes"); + + tamper_plan(&l.plan, |v| { + v["plan_digest"] = json!("deadbeef"); + }); + + let refusal = refuse(&l); + assert!(slice1_errors(&refusal) + .iter() + .any(|e| matches!(e, CurationError::PlanDigestMismatch { .. }))); + assert_nothing_written(&l, &before); +} + +#[test] +fn a4_corrupted_decisions_digest_refuses() { + let l = layout("a4"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + let mut plan = write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + + plan.decisions_digest = "deadbeef".to_owned(); + plan.plan_digest = plan_digest(&plan); + fs::write(&l.plan, serde_json::to_string_pretty(&plan).expect("ser")).expect("write"); + + let refusal = refuse(&l); + assert!(slice1_errors(&refusal) + .iter() + .any(|e| matches!(e, CurationError::DecisionDigestMismatch { .. }))); +} + +#[test] +fn a5_corpus_drift_refuses() { + let l = layout("a5"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let before = fs::read(&l.index).expect("index bytes"); + + // The corpus is relabelled after planning: the fingerprint drifts. + write_corpus( + &l.corpus, + &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-else"), + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: None, + }, + ], + ); + + let refusal = refuse(&l); + assert!(slice1_errors(&refusal) + .iter() + .any(|e| matches!(e, CurationError::PlanCorpusFingerprintMismatch { .. }))); + assert_nothing_written(&l, &before); +} + +#[test] +fn a6_forged_projection_with_self_consistent_digests_refuses() { + let l = layout("a6"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + let mut plan = write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + + plan.assignments[0].song_id = "song-forged".to_owned(); + plan.plan_digest = plan_digest(&plan); + fs::write(&l.plan, serde_json::to_string_pretty(&plan).expect("ser")).expect("write"); + + let refusal = refuse(&l); + assert!(slice1_errors(&refusal) + .iter() + .any(|e| matches!(e, CurationError::DecisionProjectionMismatch { .. }))); +} + +#[test] +fn a7_invalid_embedded_batch_short_circuits_before_digests() { + let l = layout("a7"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + + tamper_plan(&l.plan, |v| { + v["decision_batch"]["events"][0]["ordinal"] = json!(9); + v["decisions_digest"] = json!("corrupt"); + v["plan_digest"] = json!("corrupt"); + }); + + let refusal = refuse(&l); + let errors = slice1_errors(&refusal); + assert!(errors + .iter() + .any(|e| matches!(e, CurationError::InvalidDecisionBatchOrder { .. }))); + for forbidden in [ + errors + .iter() + .any(|e| matches!(e, CurationError::PlanDigestMismatch { .. })), + errors + .iter() + .any(|e| matches!(e, CurationError::DecisionDigestMismatch { .. })), + errors + .iter() + .any(|e| matches!(e, CurationError::DecisionProjectionMismatch { .. })), + ] { + assert!(!forbidden, "digest/projection work must not run: {errors:?}"); + } +} + +/// A8: apply-time corpus/batch faults surfacing unchanged through step 5. +#[test] +fn a8_reachable_slice1_refusals_surface_through_step5() { + // (a) a corpus chunk with no sha256 → UnidentifiedSource. + { + let l = layout("a8a"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + write_corpus( + &l.corpus, + &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: None, + }, + Chunk { + id: "z1", + sha: None, + song: None, + }, + ], + ); + let refusal = refuse(&l); + assert!(slice1_errors(&refusal) + .iter() + .any(|e| matches!(e, CurationError::UnidentifiedSource { .. }))); + assert!(!l.output.exists()); + } + // (b) conflicting existing labels on one source. + { + let l = layout("a8b"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + write_corpus( + &l.corpus, + &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-1"), + }, + Chunk { + id: "a2", + sha: Some("shaA"), + song: Some("song-2"), + }, + ], + ); + let refusal = refuse(&l); + assert!(slice1_errors(&refusal) + .iter() + .any(|e| matches!(e, CurationError::ConflictingExistingSongIds { .. }))); + } + // (c)–(e): tamper the embedded batch with recomputed digests, so the fault + // is the only refusal source. + let tampered = |tag: &str, mutate: fn(&mut DryRunPlan)| -> ApplyRefusal { + let l = layout(tag); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let text = fs::read_to_string(&l.plan).expect("read plan"); + let mut plan: DryRunPlan = serde_json::from_str(&text).expect("parse plan"); + mutate(&mut plan); + plan.decisions_digest = decisions_digest(&plan.decision_batch); + plan.plan_digest = plan_digest(&plan); + fs::write(&l.plan, serde_json::to_string_pretty(&plan).expect("ser")).expect("write"); + refuse(&l) + }; + // (c) a decision naming an unknown source. + let r = tampered("a8c", |plan| { + plan.decision_batch + .events + .push(event("ev1", 1, accept("h", &["shaZ"], "song-2", &[]))); + }); + assert!(slice1_errors(&r) + .iter() + .any(|e| matches!(e, CurationError::UnknownDecisionSource { .. }))); + // (d) a split assigning one source two labels. + let r = tampered("a8d", |plan| { + plan.decision_batch.events.push(event( + "ev1", + 1, + split( + "song-old", + &[("song-1", &["shaB"]), ("song-2", &["shaB"])], + &["song-old"], + ), + )); + }); + assert!(slice1_errors(&r) + .iter() + .any(|e| matches!(e, CurationError::SourceAssignedToMultipleSongs { .. }))); + // (e) a duplicate event_id in the embedded batch (short-circuits). + let r = tampered("a8e", |plan| { + plan.decision_batch + .events + .push(event("ev0", 1, accept("h", &["shaB"], "song-2", &[]))); + }); + let errors = slice1_errors(&r); + assert!(errors + .iter() + .any(|e| matches!(e, CurationError::DuplicateDecisionEventId { .. }))); + assert!(!errors + .iter() + .any(|e| matches!(e, CurationError::DecisionProjectionMismatch { .. }))); +} + +// ── C. chain / index ─────────────────────────────────────────────────────────── + +/// Apply one accept of `shaA` on a fresh A/B corpus; returns the receipt. +fn apply_initial(l: &Layout) -> AppliedReceipt { + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g1", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + succeed(l) +} + +#[test] +fn c1_valid_initial_application() { + let l = layout("c1"); + let receipt = apply_initial(&l); + assert_eq!(receipt.report.batch_id, "batch1"); + assert_eq!(receipt.report.previous_application_report_digest, None); + let index = read_index(&l); + assert_eq!(index.applications.len(), 1); +} + +#[test] +fn c2_valid_second_application_chained_to_first() { + let l = layout("c2"); + let receipt1 = apply_initial(&l); + + // The second input is the first apply's REAL published output tree, + // reserved area included. + let corpus2 = l.output.clone(); + let m2 = read_manifest(&corpus2); + let b2 = batch_for( + &m2, + "batch2", + Some(&receipt1.report.report_digest), + vec![event("ev0", 0, accept("g2", &["shaB"], "song-000002", &[]))], + ); + let plan2 = l.td.path.join("plan2.json"); + write_plan(&corpus2, b2, &plan2); + let output2 = l.td.path.join("out2"); + let run2 = apply(&ApplyPaths { + plan: plan2, + corpus: corpus2, + index: l.index.clone(), + output: output2.clone(), + }); + let receipt2 = run2.primary.expect("second apply succeeds"); + + let index = read_index(&l); + assert_eq!(index.applications.len(), 2, "two ordered records"); + assert_eq!(index.applications[0].batch_id, "batch1"); + assert_eq!(index.applications[1].batch_id, "batch2"); + assert_eq!( + receipt2.report.previous_application_report_digest, + Some(receipt1.report.report_digest.clone()) + ); + + // The second output's reserved area holds exactly the SECOND + // application's artifacts — the first's are superseded, not raw-copied. + let reserved = output2.join("song-curation"); + let mut entries: Vec = fs::read_dir(&reserved) + .expect("reserved area") + .map(|e| e.expect("entry").file_name().to_string_lossy().into_owned()) + .collect(); + entries.sort(); + assert_eq!(entries, vec!["apply-report.json", "manifest.json"]); + let report2 = read_value(&output2.join(REPORT_RELPATH)); + assert_eq!(report2["batch_id"], json!("batch2")); + assert_eq!( + report2["report_digest"], + json!(receipt2.report.report_digest) + ); +} + +#[test] +fn c3_duplicate_batch_id_refuses_as_already_applied_not_chain() { + let l = layout("c3"); + apply_initial(&l); + + // Re-apply the very same plan file to a fresh output path. + let output2 = l.td.path.join("out2"); + let run2 = apply(&ApplyPaths { + plan: l.plan.clone(), + corpus: l.corpus.clone(), + index: l.index.clone(), + output: output2.clone(), + }); + let refusal = run2.primary.expect_err("must refuse"); + assert!( + matches!(&refusal, ApplyRefusal::DecisionBatchAlreadyApplied { batch_id } if batch_id == "batch1"), + "already-applied must win over the chain refusal, got {refusal:?}" + ); + assert!(!output2.exists()); +} + +#[test] +fn c4_wrong_previous_report_digest_refuses() { + let l = layout("c4"); + apply_initial(&l); + let corpus2 = l.output.clone(); + let m2 = read_manifest(&corpus2); + let b2 = batch_for( + &m2, + "batch2", + Some("deadbeef-wrong"), + vec![event("ev0", 0, accept("g2", &["shaB"], "song-000002", &[]))], + ); + let plan2 = l.td.path.join("plan2.json"); + write_plan(&corpus2, b2, &plan2); + let run2 = apply(&ApplyPaths { + plan: plan2, + corpus: corpus2, + index: l.index.clone(), + output: l.td.path.join("out2"), + }); + let refusal = run2.primary.expect_err("must refuse"); + match &refusal { + ApplyRefusal::ApplicationChainMismatch { relation, .. } => { + assert!( + relation.contains("previous_application_report_digest"), + "relation (1) must be named, got {relation}" + ); + } + other => panic!("expected ApplicationChainMismatch, got {other:?}"), + } +} + +#[test] +fn c5_wrong_chained_corpus_fingerprint_refuses() { + let l = layout("c5"); + let receipt1 = apply_initial(&l); + + // A second batch planned against the ORIGINAL corpus (stale head). + let m = read_manifest(&l.corpus); + let b2 = batch_for( + &m, + "batch2", + Some(&receipt1.report.report_digest), + vec![event("ev0", 0, accept("g2", &["shaB"], "song-000002", &[]))], + ); + let plan2 = l.td.path.join("plan2.json"); + write_plan(&l.corpus, b2, &plan2); + let run2 = apply(&ApplyPaths { + plan: plan2, + corpus: l.corpus.clone(), + index: l.index.clone(), + output: l.td.path.join("out2"), + }); + let refusal = run2.primary.expect_err("must refuse"); + assert!( + matches!(refusal, ApplyRefusal::ApplicationChainMismatch { .. }), + "got {refusal:?}" + ); +} + +#[test] +fn c6_fingerprint_neutral_batch_applies_then_refuses_by_id() { + let l = layout("c6"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, reject("g1", &["shaA"]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + + let receipt = succeed(&l); + assert_eq!( + receipt.report.input_corpus_fingerprint, receipt.report.output_corpus_fingerprint, + "reject-only batch is fingerprint-neutral" + ); + assert_eq!(receipt.report.assignments_applied, 0); + assert_eq!(receipt.report.assignments_unchanged, 0); + assert_eq!(receipt.report.sources_reviewed_unassigned, 1); + assert_eq!(receipt.report.sources_untouched, 1); + assert_eq!(read_index(&l).applications.len(), 1); + + // The corpus shows no side effect, but the index still proves application. + let run2 = apply(&ApplyPaths { + plan: l.plan.clone(), + corpus: l.corpus.clone(), + index: l.index.clone(), + output: l.td.path.join("out2"), + }); + let refusal = run2.primary.expect_err("must refuse"); + assert!(matches!( + refusal, + ApplyRefusal::DecisionBatchAlreadyApplied { .. } + )); +} + +#[test] +fn c7_missing_index_and_foreign_field_refuse() { + // (i) missing index file: refused at step 1, before the lock. + let l = layout("c7a"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + // no index written + let refusal = refuse(&l); + assert!( + matches!(refusal, ApplyRefusal::MalformedApplicationIndex { .. }), + "got {refusal:?}" + ); + assert!( + !lock_path_of(&l.index).exists(), + "no lockfile may be created for a missing index" + ); + assert!(!l.output.exists()); + + // (ii) foreign field in the index document (step 2). + let l = layout("c7b"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + fs::write( + &l.index, + "{\"schema\":\"song-curation.applications.v1\",\"applications\":[],\"rogue\":1}", + ) + .expect("write index"); + let refusal = refuse(&l); + assert!(matches!( + refusal, + ApplyRefusal::MalformedApplicationIndex { .. } + )); + assert!(!lock_path_of(&l.index).exists(), "lock released"); +} + +#[test] +fn c8_duplicate_internal_batch_id_refuses() { + let l = layout("c8"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + fs::write( + &l.index, + r#"{"schema":"song-curation.applications.v1","applications":[ + {"batch_id":"dup","report_digest":"r1","input_corpus_fingerprint":"f1","output_corpus_fingerprint":"f2"}, + {"batch_id":"dup","report_digest":"r2","input_corpus_fingerprint":"f2","output_corpus_fingerprint":"f3"}]}"#, + ) + .expect("write index"); + let refusal = refuse(&l); + assert!( + matches!(&refusal, ApplyRefusal::DuplicateAppliedBatchId { batch_id } if batch_id == "dup"), + "got {refusal:?}" + ); +} + +#[test] +fn c9_index_with_broken_internal_chain_refuses() { + let l = layout("c9"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + fs::write( + &l.index, + r#"{"schema":"song-curation.applications.v1","applications":[ + {"batch_id":"b1","report_digest":"r1","input_corpus_fingerprint":"f1","output_corpus_fingerprint":"f2"}, + {"batch_id":"b2","report_digest":"r2","input_corpus_fingerprint":"f9","output_corpus_fingerprint":"f3"}]}"#, + ) + .expect("write index"); + let refusal = refuse(&l); + match &refusal { + ApplyRefusal::ApplicationIndexChainInvalid { position, .. } => { + assert_eq!(*position, 1); + } + other => panic!("expected ApplicationIndexChainInvalid, got {other:?}"), + } +} + +#[test] +fn c10_initial_batch_on_independent_copy_with_own_index_applies() { + let l = layout("c10"); + apply_initial(&l); + + // A byte-identical copy of the corpus with its OWN fresh empty index is + // an independent lineage: the same plan applies there too. + let corpus2 = l.td.path.join("corpus2"); + fs::create_dir_all(&corpus2).expect("mkdir"); + for entry in fs::read_dir(&l.corpus).expect("read corpus") { + let p = entry.expect("entry").path(); + fs::copy(&p, corpus2.join(p.file_name().expect("name"))).expect("copy"); + } + let index2 = l.td.path.join("index2.json"); + write_empty_index(&index2); + let run2 = apply(&ApplyPaths { + plan: l.plan.clone(), + corpus: corpus2, + index: index2, + output: l.td.path.join("out2"), + }); + assert!(run2.primary.is_ok(), "independent lineage must apply"); +} + +// ── L. labels ───────────────────────────────────────────────────────────────── + +#[test] +fn l1_assign_unlabelled_source_updates_every_chunk() { + let l = layout("l1"); + let m = write_corpus( + &l.corpus, + &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: None, + }, + Chunk { + id: "a2", + sha: Some("shaA"), + song: None, + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: None, + }, + ], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let receipt = succeed(&l); + assert_eq!(receipt.report.assignments_applied, 1); + assert_eq!(receipt.report.assignments_unchanged, 0); + for id in ["a1", "a2"] { + let v = read_value(&l.output.join(format!("{id}.chunk.json"))); + assert_eq!(v["source"]["song_id"], json!("song-000001"), "{id}"); + } + let v = read_value(&l.output.join("b1.chunk.json")); + assert_eq!(v["source"].get("song_id"), None, "untouched stays None"); +} + +#[test] +fn l2_already_correct_label_counts_unchanged() { + let l = layout("l2"); + let m = write_corpus( + &l.corpus, + &[Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-1"), + }], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let receipt = succeed(&l); + assert_eq!(receipt.report.assignments_applied, 0); + assert_eq!(receipt.report.assignments_unchanged, 1); + let v = read_value(&l.output.join("a1.chunk.json")); + assert_eq!(v["source"]["song_id"], json!("song-1")); +} + +#[test] +fn l3_authorized_correct_replaces_label() { + let l = layout("l3"); + let m = write_corpus( + &l.corpus, + &[Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-old"), + }], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event( + "ev0", + 0, + correct(&["shaA"], "song-new", &["song-old"]), + )], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let receipt = succeed(&l); + assert_eq!(receipt.report.assignments_applied, 1); + let v = read_value(&l.output.join("a1.chunk.json")); + assert_eq!(v["source"]["song_id"], json!("song-new")); +} + +#[test] +fn l4_authorized_merge_replaces_labels() { + let l = layout("l4"); + let m = write_corpus( + &l.corpus, + &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-1"), + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: Some("song-2"), + }, + ], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event( + "ev0", + 0, + merge( + &["song-1", "song-2"], + "song-1", + &["shaA", "shaB"], + &["song-1", "song-2"], + ), + )], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let receipt = succeed(&l); + assert_eq!(receipt.report.assignments_applied, 1, "shaB changed"); + assert_eq!(receipt.report.assignments_unchanged, 1, "shaA already song-1"); + let v = read_value(&l.output.join("b1.chunk.json")); + assert_eq!(v["source"]["song_id"], json!("song-1")); +} + +#[test] +fn l5_authorized_split_replaces_labels() { + let l = layout("l5"); + let m = write_corpus( + &l.corpus, + &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-1"), + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: Some("song-1"), + }, + ], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event( + "ev0", + 0, + split( + "song-1", + &[("song-2", &["shaA"]), ("song-3", &["shaB"])], + &["song-1"], + ), + )], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let receipt = succeed(&l); + assert_eq!(receipt.report.assignments_applied, 2); + let a = read_value(&l.output.join("a1.chunk.json")); + assert_eq!(a["source"]["song_id"], json!("song-2")); + let b1 = read_value(&l.output.join("b1.chunk.json")); + assert_eq!(b1["source"]["song_id"], json!("song-3")); +} + +#[test] +fn l6_unauthorized_replacement_refuses() { + // (i) accept over an existing label: empty supersession set. + let l = layout("l6a"); + let m = write_corpus( + &l.corpus, + &[Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-old"), + }], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-new", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let before = fs::read(&l.index).expect("index bytes"); + let refusal = refuse(&l); + match &refusal { + ApplyRefusal::ExistingLabelReplacementNotAuthorized { + source_sha256, + on_disk_song_id, + new_song_id, + event_id, + } => { + assert_eq!(source_sha256, "shaA"); + assert_eq!(on_disk_song_id, "song-old"); + assert_eq!(new_song_id, "song-new"); + assert_eq!(event_id, "ev0"); + } + other => panic!("expected ExistingLabelReplacementNotAuthorized, got {other:?}"), + } + assert_nothing_written(&l, &before); + + // (ii) correct whose supersession set does not cover the on-disk label. + let l = layout("l6b"); + let m = write_corpus( + &l.corpus, + &[Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-old"), + }], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event( + "ev0", + 0, + correct(&["shaA"], "song-new", &["song-other"]), + )], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let refusal = refuse(&l); + assert!(matches!( + refusal, + ApplyRefusal::ExistingLabelReplacementNotAuthorized { .. } + )); +} + +#[test] +fn l7_on_disk_label_drift_refuses_as_fingerprint_mismatch() { + let l = layout("l7"); + let m = write_corpus( + &l.corpus, + &[Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-old"), + }], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event( + "ev0", + 0, + correct(&["shaA"], "song-new", &["song-old"]), + )], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + + // The corpus is relabelled between planning and apply. + write_corpus( + &l.corpus, + &[Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-else"), + }], + ); + let refusal = refuse(&l); + assert!( + slice1_errors(&refusal) + .iter() + .any(|e| matches!(e, CurationError::PlanCorpusFingerprintMismatch { .. })), + "drift is excluded by the fingerprint (§7.4 note)" + ); +} + +#[test] +fn l8_merge_and_split_supersession_mismatch_refuse() { + // merge whose supersedes ≠ from_song_ids. + let l = layout("l8a"); + let m = write_corpus( + &l.corpus, + &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-1"), + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: Some("song-2"), + }, + ], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event( + "ev0", + 0, + merge( + &["song-1", "song-2"], + "song-1", + &["shaA", "shaB"], + &["song-1"], + ), + )], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let refusal = refuse(&l); + assert!( + matches!(&refusal, ApplyRefusal::SupersessionEvidenceContradiction { event_id, .. } if event_id == "ev0"), + "got {refusal:?}" + ); + + // split whose supersedes ≠ [from_song_id]. + let l = layout("l8b"); + let m = write_corpus( + &l.corpus, + &[Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-1"), + }], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event( + "ev0", + 0, + split("song-1", &[("song-2", &["shaA"])], &[]), + )], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let refusal = refuse(&l); + assert!(matches!( + refusal, + ApplyRefusal::SupersessionEvidenceContradiction { .. } + )); +} + +#[test] +fn l9_accept_with_nonempty_supersedes_refuses() { + let l = layout("l9"); + let m = write_corpus(&l.corpus, UNCURATED_AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event( + "ev0", + 0, + accept("g", &["shaA"], "song-1", &["song-x"]), + )], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let refusal = refuse(&l); + assert!( + matches!(refusal, ApplyRefusal::SupersessionEvidenceContradiction { .. }), + "got {refusal:?}" + ); +} diff --git a/song-curation/tests/apply_outputs.rs b/song-curation/tests/apply_outputs.rs new file mode 100644 index 0000000..d64f8e7 --- /dev/null +++ b/song-curation/tests/apply_outputs.rs @@ -0,0 +1,459 @@ +//! Slice-2 preregistered acceptance matrix — corpus completeness / +//! preservation (K1–K9, K12) and report/index publication (R1, R2, R3, R5), +//! plus the pure filesystem laws F3 and F5 (ADR-0033 Slice 2 contract §14). + +mod common; + +use common::{ + accept, batch_for, event, layout, lock_path_of, read_value, sha256_of_file, staging_path_of, + walk_bytes, write_corpus, write_corpus_with_songs, write_empty_index, write_plan, Chunk, + Layout, +}; +use griff_core::corpus::{CorpusManifest, SongId}; +use griff_song_curation::apply::{ + apply, report_digest, ApplicationIndex, ApplicationReport, ApplyPaths, ApplyRefusal, ApplyRun, + AppliedReceipt, CURATED_MANIFEST_RELPATH, REPORT_RELPATH, +}; +use serde_json::json; +use std::collections::BTreeMap; +use std::fs; + +fn run(l: &Layout) -> ApplyRun { + apply(&ApplyPaths { + plan: l.plan.clone(), + corpus: l.corpus.clone(), + index: l.index.clone(), + output: l.output.clone(), + }) +} + +fn refuse(l: &Layout) -> ApplyRefusal { + run(l).primary.expect_err("expected a refusal") +} + +fn succeed(l: &Layout) -> AppliedReceipt { + run(l).primary.expect("expected success") +} + +const AB: &[Chunk<'static>] = &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: None, + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: None, + }, +]; + +fn plan_accept_sha_a(l: &Layout) { + let m = write_corpus(&l.corpus, AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); +} + +// ── K. corpus completeness / preservation ────────────────────────────────────── + +#[test] +fn k1_every_chunk_of_one_sha_updated_together() { + let l = layout("k1"); + let m = write_corpus( + &l.corpus, + &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: None, + }, + Chunk { + id: "a2", + sha: Some("shaA"), + song: None, + }, + ], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + succeed(&l); + // Both chunk files AND both manifest records carry the label. + for id in ["a1", "a2"] { + let v = read_value(&l.output.join(format!("{id}.chunk.json"))); + assert_eq!(v["source"]["song_id"], json!("song-000001"), "{id} file"); + } + let manifest = read_value(&l.output.join("manifest.json")); + for record in manifest["chunks"].as_array().expect("chunks") { + assert_eq!( + record["source"]["song_id"], + json!("song-000001"), + "manifest record" + ); + } +} + +#[test] +fn k2_untouched_files_are_raw_byte_copies() { + let l = layout("k2"); + plan_accept_sha_a(&l); + // Extra corpus content the tool does not interpret. + fs::write(l.corpus.join("g.group.json"), "{\"weird\": [1,2 , 3]}").expect("group"); + fs::write(l.corpus.join("notes.txt"), b"\x00\xffraw bytes\n").expect("notes"); + // Non-canonical whitespace in the UNTOUCHED chunk file must survive. + let b_text = fs::read_to_string(l.corpus.join("b1.chunk.json")).expect("read b1"); + let b_text = format!("{b_text}\n\n"); + fs::write(l.corpus.join("b1.chunk.json"), &b_text).expect("write b1"); + + succeed(&l); + + for name in ["b1.chunk.json", "g.group.json", "notes.txt"] { + assert_eq!( + fs::read(l.corpus.join(name)).expect("in"), + fs::read(l.output.join(name)).expect("out"), + "{name} must be byte-identical" + ); + } +} + +#[test] +fn k3_touched_file_with_unknown_member_refuses() { + let l = layout("k3"); + plan_accept_sha_a(&l); + // Plant a member the core schema does not know into the TOUCHED file. + let path = l.corpus.join("a1.chunk.json"); + let mut v = read_value(&path); + v.as_object_mut() + .expect("object") + .insert("rogue_member".to_owned(), json!("smuggled")); + fs::write(&path, v.to_string()).expect("write"); + + let refusal = refuse(&l); + assert!( + matches!(&refusal, ApplyRefusal::NonCanonicalCorpusFile { path, .. } + if path.contains("a1.chunk.json")), + "got {refusal:?}" + ); + assert!(!l.output.exists(), "nothing may be published"); +} + +#[test] +fn k4_root_manifest_with_songs_refuses() { + let l = layout("k4"); + let mut songs = BTreeMap::new(); + songs.insert(SongId("song-1".to_owned()), vec!["shaA".to_owned()]); + let m = write_corpus_with_songs( + &l.corpus, + &[Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-1"), + }], + Some(songs), + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-1", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let refusal = refuse(&l); + assert!( + matches!(refusal, ApplyRefusal::OrdinaryManifestCarriesSongs), + "got {refusal:?}" + ); +} + +#[test] +fn k5_tree_disagreement_and_missing_manifest_refuse() { + // (i) the manifest lists a chunk that has no on-disk file. + let l = layout("k5a"); + plan_accept_sha_a(&l); + let path = l.corpus.join("manifest.json"); + let mut v = read_value(&path); + let extra = read_value(&l.corpus.join("a1.chunk.json")); + let mut extra = extra; + extra["id"] = json!("ghost"); + v["chunks"].as_array_mut().expect("chunks").push(extra); + fs::write(&path, v.to_string()).expect("write manifest"); + let refusal = refuse(&l); + assert!( + matches!(refusal, ApplyRefusal::CorpusTreeDisagreement { .. }), + "got {refusal:?}" + ); + + // (ii) the root manifest is missing entirely. + let l = layout("k5b"); + plan_accept_sha_a(&l); + fs::remove_file(l.corpus.join("manifest.json")).expect("rm manifest"); + let refusal = refuse(&l); + assert!(matches!(refusal, ApplyRefusal::CorpusTreeDisagreement { .. })); +} + +#[test] +fn k6_curated_songs_map_matches_labels_exactly() { + let l = layout("k6"); + let m = write_corpus( + &l.corpus, + &[ + Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-old"), + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: None, + }, + ], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaB"], "song-new", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + succeed(&l); + + let curated: CorpusManifest = serde_json::from_str( + &fs::read_to_string(l.output.join(CURATED_MANIFEST_RELPATH)).expect("read curated"), + ) + .expect("curated parses"); + let songs = curated.songs.expect("curated carries songs"); + let mut expected = BTreeMap::new(); + expected.insert(SongId("song-old".to_owned()), vec!["shaA".to_owned()]); + expected.insert(SongId("song-new".to_owned()), vec!["shaB".to_owned()]); + assert_eq!(songs, expected, "exact projection of the applied labels"); +} + +#[test] +fn k7_curated_manifest_at_protected_path_root_manifest_stays_songless() { + let l = layout("k7"); + plan_accept_sha_a(&l); + succeed(&l); + assert!(l.output.join(CURATED_MANIFEST_RELPATH).exists()); + let root = read_value(&l.output.join("manifest.json")); + assert!( + root.get("songs").is_none(), + "ordinary root manifest must never gain songs" + ); + let curated = read_value(&l.output.join(CURATED_MANIFEST_RELPATH)); + assert!(curated.get("songs").is_some()); +} + +#[test] +fn k8_partial_curation_reports_not_holdout_ready() { + let l = layout("k8"); + plan_accept_sha_a(&l); // labels only shaA; shaB stays uncurated + let receipt = succeed(&l); + assert!(!receipt.report.holdout_ready); + assert_eq!(receipt.report.holdout_refusals.len(), 1); + let refusal = &receipt.report.holdout_refusals[0]; + assert_eq!(refusal.kind, "uncurated_source"); + assert_eq!(refusal.sha256.as_deref(), Some("shaB")); +} + +#[test] +fn k9_fully_curated_fixture_is_holdout_ready() { + let l = layout("k9"); + let m = write_corpus(&l.corpus, AB); + let b = batch_for( + &m, + "batch1", + None, + vec![ + event("ev0", 0, accept("g1", &["shaA"], "song-000001", &[])), + event("ev1", 1, accept("g2", &["shaB"], "song-000002", &[])), + ], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let receipt = succeed(&l); + assert!(receipt.report.holdout_ready, "the real core preflight passes"); + assert!(receipt.report.holdout_refusals.is_empty()); +} + +#[test] +fn k12_foreign_reserved_area_entry_refuses() { + // (i) a foreign file directly at the reserved root. + let l = layout("k12a"); + plan_accept_sha_a(&l); + fs::create_dir_all(l.corpus.join("song-curation")).expect("mkdir"); + fs::write(l.corpus.join("song-curation/notes.txt"), "junk").expect("write"); + let refusal = refuse(&l); + match &refusal { + ApplyRefusal::CorpusTreeDisagreement { detail } => { + assert!(detail.contains("notes.txt"), "must name the path: {detail}"); + } + other => panic!("expected CorpusTreeDisagreement, got {other:?}"), + } + + // (ii) a nested foreign entry — "only two files" is recursive. + let l = layout("k12b"); + plan_accept_sha_a(&l); + fs::create_dir_all(l.corpus.join("song-curation/extra")).expect("mkdir"); + fs::write(l.corpus.join("song-curation/extra/x.json"), "{}").expect("write"); + let refusal = refuse(&l); + match &refusal { + ApplyRefusal::CorpusTreeDisagreement { detail } => { + assert!(detail.contains("extra"), "must name the path: {detail}"); + } + other => panic!("expected CorpusTreeDisagreement, got {other:?}"), + } +} + +// ── R. report / index publication ────────────────────────────────────────────── + +#[test] +fn r1_report_digest_recomputes_identically() { + let l = layout("r1"); + plan_accept_sha_a(&l); + let receipt = succeed(&l); + assert_eq!( + report_digest(&receipt.report), + receipt.report.report_digest, + "in-memory report digest law" + ); + // The published artifact obeys the same law after a strict re-parse. + let published: ApplicationReport = serde_json::from_str( + &fs::read_to_string(l.output.join(REPORT_RELPATH)).expect("read report"), + ) + .expect("report parses strictly"); + assert_eq!(published, receipt.report); + assert_eq!(report_digest(&published), published.report_digest); +} + +#[test] +fn r2_index_record_matches_report() { + let l = layout("r2"); + plan_accept_sha_a(&l); + let receipt = succeed(&l); + let index: ApplicationIndex = + serde_json::from_str(&fs::read_to_string(&l.index).expect("read index")) + .expect("index parses"); + let record = &index.applications[0]; + assert_eq!(record.batch_id, receipt.report.batch_id); + assert_eq!(record.report_digest, receipt.report.report_digest); + assert_eq!( + record.input_corpus_fingerprint, + receipt.report.input_corpus_fingerprint + ); + assert_eq!( + record.output_corpus_fingerprint, + receipt.report.output_corpus_fingerprint + ); +} + +#[test] +fn r3_success_publishes_report_and_record_refusal_publishes_neither() { + // Success half. + let l = layout("r3a"); + plan_accept_sha_a(&l); + let receipt = succeed(&l); + assert!(l.output.join(REPORT_RELPATH).exists()); + let index: ApplicationIndex = + serde_json::from_str(&fs::read_to_string(&l.index).expect("read")).expect("parse"); + assert_eq!(index.applications[0].report_digest, receipt.report.report_digest); + + // Pre-publication refusal half: neither a record nor a published tree. + let l = layout("r3b"); + plan_accept_sha_a(&l); + common::tamper_plan(&l.plan, |v| { + v["plan_digest"] = json!("deadbeef"); + }); + let before = fs::read(&l.index).expect("index"); + let _ = refuse(&l); + assert!(!l.output.exists()); + assert_eq!(fs::read(&l.index).expect("index"), before); +} + +#[test] +fn r5_curated_manifest_digest_is_sha256_of_published_bytes() { + let l = layout("r5"); + plan_accept_sha_a(&l); + let receipt = succeed(&l); + assert_eq!( + sha256_of_file(&l.output.join(CURATED_MANIFEST_RELPATH)), + receipt.report.curated_manifest_digest + ); +} + +// ── F. pure filesystem laws ──────────────────────────────────────────────────── + +#[test] +fn f3_pre_staging_refusal_leaves_no_trace() { + let l = layout("f3"); + let m = write_corpus( + &l.corpus, + &[Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-old"), + }], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-new", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let before = fs::read(&l.index).expect("index"); + let refusal = refuse(&l); + assert!(matches!( + refusal, + ApplyRefusal::ExistingLabelReplacementNotAuthorized { .. } + )); + assert!(!l.output.exists(), "no output"); + assert!(!staging_path_of(&l.output).exists(), "no staging"); + assert!(!lock_path_of(&l.index).exists(), "no lock left"); + assert_eq!(fs::read(&l.index).expect("index"), before, "index identical"); +} + +#[test] +fn f5_repeated_execution_is_byte_identical() { + let make = |tag: &str| -> Layout { + let l = layout(tag); + let m = write_corpus(&l.corpus, AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + l + }; + let l1 = make("f5x"); + let l2 = make("f5y"); + succeed(&l1); + succeed(&l2); + assert_eq!( + walk_bytes(&l1.output), + walk_bytes(&l2.output), + "output trees must be byte-identical" + ); + assert_eq!( + fs::read(&l1.index).expect("i1"), + fs::read(&l2.index).expect("i2"), + "updated indexes must be byte-identical" + ); +} diff --git a/song-curation/tests/common/mod.rs b/song-curation/tests/common/mod.rs new file mode 100644 index 0000000..e29df70 --- /dev/null +++ b/song-curation/tests/common/mod.rs @@ -0,0 +1,339 @@ +//! Shared test support for the Slice-2 transactional-Apply suite (ADR-0033). +//! +//! Everything here is fixture/builder machinery over the frozen Slice-1 +//! public API and `griff_core` types: corpus trees on disk, serialized plan +//! artifacts, application-index files, and byte-level comparison helpers. +//! No production behaviour lives here. + +#![allow(dead_code)] // each integration-test binary uses a subset + +use griff_core::corpus::{ + source_sha256, ChunkId, ChunkMeta, CorpusManifest, SongId, SourceFormat, SCHEMA_VERSION, +}; +use griff_song_curation::{ + build_plan, corpus_fingerprint, Action, DecisionBatch, DecisionEvent, DecisionsLedger, + DryRunPlan, SplitTarget, +}; +use serde_json::{json, Value}; +use std::collections::BTreeMap; +use std::fs; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; + +// ── temp dirs without new dependencies ───────────────────────────────────────── + +static COUNTER: AtomicU64 = AtomicU64::new(0); + +/// A process-unique temporary directory, removed on drop. Uniqueness comes +/// from pid + an atomic counter, so parallel tests never collide and no +/// randomness or extra dependency is needed. +pub struct TempDir { + pub path: PathBuf, +} + +impl TempDir { + pub fn new(tag: &str) -> Self { + let path = std::env::temp_dir().join(format!( + "griff-slice2-{}-{}-{}", + std::process::id(), + tag, + COUNTER.fetch_add(1, Ordering::SeqCst) + )); + fs::create_dir_all(&path).expect("create temp dir"); + TempDir { path } + } +} + +impl Drop for TempDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.path); + } +} + +// ── corpus fixtures ──────────────────────────────────────────────────────────── + +const V10_CHUNK: &str = r#"{ + "id": "x", "title": "T", + "source": { "filename": "f.gp5", "format": "gp5", "bar_range": null }, + "tempo_bpm": 120.0, "ticks_per_quarter": 960, "time_signature": [4, 4], + "tuning": "standard_e", "tags": [], "boundaries": [], "techniques": [], + "quality_flags": [], "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" +}"#; + +/// One corpus chunk spec: `(chunk id, sha256, existing song_id)`. +#[derive(Clone, Copy)] +pub struct Chunk<'a> { + pub id: &'a str, + pub sha: Option<&'a str>, + pub song: Option<&'a str>, +} + +pub fn chunk(id: &str, sha: Option<&str>, song: Option<&str>) -> ChunkMeta { + let mut c: ChunkMeta = serde_json::from_str(V10_CHUNK).expect("fixture parses"); + c.id = ChunkId(id.to_owned()); + c.title = format!("Title {id}"); + c.source.filename = format!("{id}.gp5"); + c.source.format = SourceFormat::Gp5; + c.source.sha256 = sha.map(ToOwned::to_owned); + c.source.song_id = song.map(|s| SongId(s.to_owned())); + c +} + +pub fn manifest_of( + chunks: &[Chunk<'_>], + songs: Option>>, +) -> CorpusManifest { + CorpusManifest { + schema_version: SCHEMA_VERSION, + chunks: chunks.iter().map(|c| chunk(c.id, c.sha, c.song)).collect(), + groups: Vec::new(), + songs, + } +} + +/// Write a corpus snapshot tree: one `.chunk.json` per chunk plus the +/// root `manifest.json`, all pretty-printed (the corpus rendering convention). +pub fn write_corpus(dir: &Path, chunks: &[Chunk<'_>]) -> CorpusManifest { + write_corpus_with_songs(dir, chunks, None) +} + +pub fn write_corpus_with_songs( + dir: &Path, + chunks: &[Chunk<'_>], + songs: Option>>, +) -> CorpusManifest { + fs::create_dir_all(dir).expect("create corpus dir"); + let manifest = manifest_of(chunks, songs); + for meta in &manifest.chunks { + let text = serde_json::to_string_pretty(meta).expect("serialize chunk"); + fs::write(dir.join(format!("{}.chunk.json", meta.id.0)), text).expect("write chunk"); + } + let text = serde_json::to_string_pretty(&manifest).expect("serialize manifest"); + fs::write(dir.join("manifest.json"), text).expect("write manifest"); + manifest +} + +pub fn read_manifest(dir: &Path) -> CorpusManifest { + let text = fs::read_to_string(dir.join("manifest.json")).expect("read manifest"); + serde_json::from_str(&text).expect("parse manifest") +} + +// ── decision events / batches / plans ────────────────────────────────────────── + +pub fn event(id: &str, ordinal: u64, action: Action) -> DecisionEvent { + DecisionEvent { + event_id: id.to_owned(), + ordinal, + curator: "curator".to_owned(), + occurred_at: "2026-08-20T00:00:00Z".to_owned(), + note: None, + action, + } +} + +pub fn accept(candidate: &str, shas: &[&str], song: &str, supersedes: &[&str]) -> Action { + Action::AcceptSuggestion { + candidate_id: candidate.to_owned(), + source_sha256s: shas.iter().map(|s| (*s).to_owned()).collect(), + assign_song_id: song.to_owned(), + supersedes_song_ids: supersedes.iter().map(|s| (*s).to_owned()).collect(), + } +} + +pub fn reject(candidate: &str, shas: &[&str]) -> Action { + Action::RejectSuggestion { + candidate_id: candidate.to_owned(), + reviewed_source_sha256s: shas.iter().map(|s| (*s).to_owned()).collect(), + reason: None, + } +} + +pub fn manual(shas: &[&str], song: &str) -> Action { + Action::ManualDefine { + source_sha256s: shas.iter().map(|s| (*s).to_owned()).collect(), + assign_song_id: song.to_owned(), + } +} + +pub fn correct(shas: &[&str], new: &str, supersedes: &[&str]) -> Action { + Action::Correct { + source_sha256s: shas.iter().map(|s| (*s).to_owned()).collect(), + new_song_id: new.to_owned(), + supersedes_song_ids: supersedes.iter().map(|s| (*s).to_owned()).collect(), + } +} + +pub fn merge(from: &[&str], into: &str, shas: &[&str], supersedes: &[&str]) -> Action { + Action::Merge { + from_song_ids: from.iter().map(|s| (*s).to_owned()).collect(), + into_song_id: into.to_owned(), + source_sha256s: shas.iter().map(|s| (*s).to_owned()).collect(), + supersedes_song_ids: supersedes.iter().map(|s| (*s).to_owned()).collect(), + } +} + +pub fn split(from: &str, into: &[(&str, &[&str])], supersedes: &[&str]) -> Action { + Action::Split { + from_song_id: from.to_owned(), + into: into + .iter() + .map(|(song, shas)| SplitTarget { + assign_song_id: (*song).to_owned(), + source_sha256s: shas.iter().map(|s| (*s).to_owned()).collect(), + }) + .collect(), + supersedes_song_ids: supersedes.iter().map(|s| (*s).to_owned()).collect(), + } +} + +pub fn batch_for( + manifest: &CorpusManifest, + id: &str, + prev_report_digest: Option<&str>, + events: Vec, +) -> DecisionBatch { + DecisionBatch { + batch_id: id.to_owned(), + input_corpus_fingerprint: corpus_fingerprint(manifest), + previous_application_report_digest: prev_report_digest.map(ToOwned::to_owned), + events, + } +} + +pub fn ledger_of(batches: Vec) -> DecisionsLedger { + DecisionsLedger { + schema: "song-curation.decisions.v1".to_owned(), + next_song_seq: 1, + batches, + } +} + +/// Build a Slice-1 plan for `batch` against the corpus at `corpus_dir` and +/// serialize it to `plan_path` (the artifact boundary Apply consumes). +pub fn write_plan(corpus_dir: &Path, batch: DecisionBatch, plan_path: &Path) -> DryRunPlan { + let manifest = read_manifest(corpus_dir); + let id = batch.batch_id.clone(); + let plan = build_plan(&manifest, &ledger_of(vec![batch]), &id).expect("plan builds"); + fs::write( + plan_path, + serde_json::to_string_pretty(&plan).expect("serialize plan"), + ) + .expect("write plan"); + plan +} + +/// Load the plan artifact, mutate its JSON value, and write it back. +pub fn tamper_plan(plan_path: &Path, f: impl FnOnce(&mut Value)) { + let text = fs::read_to_string(plan_path).expect("read plan"); + let mut value: Value = serde_json::from_str(&text).expect("parse plan"); + f(&mut value); + fs::write(plan_path, value.to_string()).expect("write plan"); +} + +// ── application index files ──────────────────────────────────────────────────── + +pub const EMPTY_INDEX: &str = "{\n \"schema\": \"song-curation.applications.v1\",\n \"applications\": []\n}"; + +pub fn write_empty_index(path: &Path) { + fs::write(path, EMPTY_INDEX).expect("write index"); +} + +// ── filesystem comparison helpers ────────────────────────────────────────────── + +/// Every file under `dir` as `(relative path, bytes)`, sorted by path. +pub fn walk_bytes(dir: &Path) -> Vec<(String, Vec)> { + fn walk(root: &Path, dir: &Path, out: &mut Vec<(String, Vec)>) { + for entry in fs::read_dir(dir).expect("read_dir") { + let path = entry.expect("entry").path(); + if path.is_dir() { + walk(root, &path, out); + } else { + let rel = path + .strip_prefix(root) + .expect("under root") + .to_string_lossy() + .replace('\\', "/"); + out.push((rel, fs::read(&path).expect("read file"))); + } + } + } + let mut out = Vec::new(); + walk(dir, dir, &mut out); + out.sort(); + out +} + +pub fn read_value(path: &Path) -> Value { + serde_json::from_str(&fs::read_to_string(path).expect("read file")).expect("parse json") +} + +pub fn sha256_of_file(path: &Path) -> String { + source_sha256(&fs::read(path).expect("read file")) +} + +/// The staging sibling the contract derives for `output` (§8.1). +pub fn staging_path_of(output: &Path) -> PathBuf { + let name = output.file_name().expect("output name").to_string_lossy(); + output + .parent() + .expect("output parent") + .join(format!(".{name}.apply-staging")) +} + +/// The lock / temp coordination names for a (supplied) index path (§8.1). +pub fn lock_path_of(index: &Path) -> PathBuf { + coordination_path(index, "lock") +} + +pub fn temp_path_of(index: &Path) -> PathBuf { + coordination_path(index, "tmp") +} + +fn coordination_path(index: &Path, ext: &str) -> PathBuf { + let name = index.file_name().expect("index name").to_string_lossy(); + index + .parent() + .expect("index parent") + .join(format!(".{name}.{ext}")) +} + +/// Standard four-path apply layout inside one temp root: +/// `corpus/`, `plan.json`, `index.json`, `out`. +pub struct Layout { + pub td: TempDir, + pub corpus: PathBuf, + pub plan: PathBuf, + pub index: PathBuf, + pub output: PathBuf, +} + +pub fn layout(tag: &str) -> Layout { + let td = TempDir::new(tag); + let corpus = td.path.join("corpus"); + let plan = td.path.join("plan.json"); + let index = td.path.join("index.json"); + let output = td.path.join("out"); + Layout { + td, + corpus, + plan, + index, + output, + } +} + +/// Inject a rogue member into a JSON object located by `path` steps. +pub fn plant_rogue(root: &mut Value, path: &[&str]) { + let mut cursor = root; + for step in path { + cursor = match step.parse::() { + Ok(i) => cursor.get_mut(i).expect("array index exists"), + Err(_) => cursor.get_mut(*step).expect("object key exists"), + }; + } + cursor + .as_object_mut() + .expect("target is object") + .insert("rogue_field".to_owned(), json!("smuggled")); +} From 4d311f41514db9b34989d4125ee28404823c5a4e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 14:37:41 +0000 Subject: [PATCH 02/18] test(song-curation): A8 fixtures replace the corpus tree wholesale Fixture-only correction, no law change: the A8(a)/A8(b) drift scenarios rewrote the corpus with a different chunk set but left the previous chunk files on disk, so the honest step-3 tree-agreement refusal fired before the step-5 refusal the case actually preregisters. The fixtures now clear the corpus directory before writing the replacement snapshot. Also applies cargo fmt to the test tree. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/tests/apply_core.rs | 19 +++++++++++++++---- song-curation/tests/apply_outputs.rs | 25 +++++++++++++++++++------ song-curation/tests/common/mod.rs | 3 ++- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/song-curation/tests/apply_core.rs b/song-curation/tests/apply_core.rs index e62838d..1f022a8 100644 --- a/song-curation/tests/apply_core.rs +++ b/song-curation/tests/apply_core.rs @@ -13,7 +13,7 @@ use common::{ Chunk, Layout, }; use griff_song_curation::apply::{ - apply, ApplicationIndex, ApplyPaths, ApplyRefusal, ApplyRun, AppliedReceipt, + apply, ApplicationIndex, AppliedReceipt, ApplyPaths, ApplyRefusal, ApplyRun, APPLICATIONS_SCHEMA, CURATED_MANIFEST_RELPATH, REPORT_RELPATH, }; use griff_song_curation::{decisions_digest, plan_digest, CurationError, DryRunPlan}; @@ -300,7 +300,10 @@ fn a7_invalid_embedded_batch_short_circuits_before_digests() { .iter() .any(|e| matches!(e, CurationError::DecisionProjectionMismatch { .. })), ] { - assert!(!forbidden, "digest/projection work must not run: {errors:?}"); + assert!( + !forbidden, + "digest/projection work must not run: {errors:?}" + ); } } @@ -319,6 +322,7 @@ fn a8_reachable_slice1_refusals_surface_through_step5() { ); write_plan(&l.corpus, b, &l.plan); write_empty_index(&l.index); + fs::remove_dir_all(&l.corpus).expect("clear corpus"); write_corpus( &l.corpus, &[ @@ -352,6 +356,7 @@ fn a8_reachable_slice1_refusals_surface_through_step5() { ); write_plan(&l.corpus, b, &l.plan); write_empty_index(&l.index); + fs::remove_dir_all(&l.corpus).expect("clear corpus"); write_corpus( &l.corpus, &[ @@ -888,7 +893,10 @@ fn l4_authorized_merge_replaces_labels() { write_empty_index(&l.index); let receipt = succeed(&l); assert_eq!(receipt.report.assignments_applied, 1, "shaB changed"); - assert_eq!(receipt.report.assignments_unchanged, 1, "shaA already song-1"); + assert_eq!( + receipt.report.assignments_unchanged, 1, + "shaA already song-1" + ); let v = read_value(&l.output.join("b1.chunk.json")); assert_eq!(v["source"]["song_id"], json!("song-1")); } @@ -1133,7 +1141,10 @@ fn l9_accept_with_nonempty_supersedes_refuses() { write_empty_index(&l.index); let refusal = refuse(&l); assert!( - matches!(refusal, ApplyRefusal::SupersessionEvidenceContradiction { .. }), + matches!( + refusal, + ApplyRefusal::SupersessionEvidenceContradiction { .. } + ), "got {refusal:?}" ); } diff --git a/song-curation/tests/apply_outputs.rs b/song-curation/tests/apply_outputs.rs index d64f8e7..45d35b5 100644 --- a/song-curation/tests/apply_outputs.rs +++ b/song-curation/tests/apply_outputs.rs @@ -11,8 +11,8 @@ use common::{ }; use griff_core::corpus::{CorpusManifest, SongId}; use griff_song_curation::apply::{ - apply, report_digest, ApplicationIndex, ApplicationReport, ApplyPaths, ApplyRefusal, ApplyRun, - AppliedReceipt, CURATED_MANIFEST_RELPATH, REPORT_RELPATH, + apply, report_digest, ApplicationIndex, ApplicationReport, AppliedReceipt, ApplyPaths, + ApplyRefusal, ApplyRun, CURATED_MANIFEST_RELPATH, REPORT_RELPATH, }; use serde_json::json; use std::collections::BTreeMap; @@ -200,7 +200,10 @@ fn k5_tree_disagreement_and_missing_manifest_refuse() { plan_accept_sha_a(&l); fs::remove_file(l.corpus.join("manifest.json")).expect("rm manifest"); let refusal = refuse(&l); - assert!(matches!(refusal, ApplyRefusal::CorpusTreeDisagreement { .. })); + assert!(matches!( + refusal, + ApplyRefusal::CorpusTreeDisagreement { .. } + )); } #[test] @@ -285,7 +288,10 @@ fn k9_fully_curated_fixture_is_holdout_ready() { write_plan(&l.corpus, b, &l.plan); write_empty_index(&l.index); let receipt = succeed(&l); - assert!(receipt.report.holdout_ready, "the real core preflight passes"); + assert!( + receipt.report.holdout_ready, + "the real core preflight passes" + ); assert!(receipt.report.holdout_refusals.is_empty()); } @@ -369,7 +375,10 @@ fn r3_success_publishes_report_and_record_refusal_publishes_neither() { assert!(l.output.join(REPORT_RELPATH).exists()); let index: ApplicationIndex = serde_json::from_str(&fs::read_to_string(&l.index).expect("read")).expect("parse"); - assert_eq!(index.applications[0].report_digest, receipt.report.report_digest); + assert_eq!( + index.applications[0].report_digest, + receipt.report.report_digest + ); // Pre-publication refusal half: neither a record nor a published tree. let l = layout("r3b"); @@ -424,7 +433,11 @@ fn f3_pre_staging_refusal_leaves_no_trace() { assert!(!l.output.exists(), "no output"); assert!(!staging_path_of(&l.output).exists(), "no staging"); assert!(!lock_path_of(&l.index).exists(), "no lock left"); - assert_eq!(fs::read(&l.index).expect("index"), before, "index identical"); + assert_eq!( + fs::read(&l.index).expect("index"), + before, + "index identical" + ); } #[test] diff --git a/song-curation/tests/common/mod.rs b/song-curation/tests/common/mod.rs index e29df70..d4e838b 100644 --- a/song-curation/tests/common/mod.rs +++ b/song-curation/tests/common/mod.rs @@ -233,7 +233,8 @@ pub fn tamper_plan(plan_path: &Path, f: impl FnOnce(&mut Value)) { // ── application index files ──────────────────────────────────────────────────── -pub const EMPTY_INDEX: &str = "{\n \"schema\": \"song-curation.applications.v1\",\n \"applications\": []\n}"; +pub const EMPTY_INDEX: &str = + "{\n \"schema\": \"song-curation.applications.v1\",\n \"applications\": []\n}"; pub fn write_empty_index(path: &Path) { fs::write(path, EMPTY_INDEX).expect("write index"); From f8c9f1c2a98f6c9f2059fb4859f53c843b242047 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 14:37:41 +0000 Subject: [PATCH 03/18] feat(song-curation): implement Slice 2 apply core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GREEN for the 43 preregistered RED-A cases (A1–A8, C1–C10, L1–L9, K1–K9, K12, R1–R3, R5, F3, F5): the new public apply module executes the accepted contract's pipeline over the serialized artifact boundary — - §5 wire contracts: strict (deny_unknown_fields) application-index v1 and apply-report v1 with the four-way partition, total-order holdout-refusal records, no wall-clock timestamp, and report_digest over the Slice-1 shared canonical encoding (canonical_json made pub(crate), not copied); - §6 order for the implemented checks: index existence resolved before the lock; lock create_new + marker; strict plan/index parsing before any mutation; snapshot load with recursive sorted walk, reserved-area exclusion + recursive shape law, multiset tree agreement, and the no-root-songs law; index schema/uniqueness/internal-chain validation; verify_plan reused literally; already-applied before the chain equations; the three exact §7.2 relations (and the §7.1 null relation); supersession -evidence consistency; replacement authority via the single shared replay primitive carrying acting-event attribution (§9 — the one permitted internal Slice-1 accommodation; observable Slice-1 behaviour unchanged, frozen suite green); - §8 happy-path protocol: fixed-name staging via create_dir, preservation law (§10) with raw byte copy for untouched files and the round-trip laundering guard for touched ones, curated manifest at the protected path, step-10 re-read-from-staged-bytes self-check + the single real song_holdout_preflight over the curated view, report written last into staging, one publication rename, temp+sync+rename commit point, lock released on every exit with the §8.2 release-warning result shape. Deliberately NOT yet implemented (their §14 cases stay RED for the adversarial phase): hardlink refusal, coordination-path collisions, reserved output namespace, output/staging pre-existence refusals, containment laws, lock-content classification, under-lock temp inspection, no-clobber temp creation, duplicate-key rejection, staged tree-agreement re-run. Evidence: song-curation suite 45 (frozen Slice 1) + 27 + 16 green; clippy --all-targets clean under the crate's deny(all) lints; fmt --check clean. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/src/apply.rs | 1155 ++++++++++++++++++++++++++++++++++++ song-curation/src/lib.rs | 44 +- 2 files changed, 1190 insertions(+), 9 deletions(-) create mode 100644 song-curation/src/apply.rs diff --git a/song-curation/src/apply.rs b/song-curation/src/apply.rs new file mode 100644 index 0000000..98feced --- /dev/null +++ b/song-curation/src/apply.rs @@ -0,0 +1,1155 @@ +//! ADR-0033 **Slice 2** — transactional Apply. +//! +//! Executable implementation of the independently accepted Slice-2 contract +//! (`docs/proposals/song-curation-slice-2-transactional-apply.md`, normative +//! reviewed artifact `47e734cf…`, acceptance recorded in +//! `docs/decisions.log.md`). Apply consumes a serialized Slice-1 +//! [`DryRunPlan`], the current corpus snapshot, and the application index, +//! and — under the contract's 12-step fail-closed verification order — +//! publishes the curated snapshot with its proof artifacts: the curated +//! manifest, the application report, and the appended index record. +//! +//! The batch is applied **iff** its record is in the application index +//! (§8.2): one publication `rename` makes the snapshot visible, and the +//! index temp+`rename` is the single commit point. Every refusal — I/O +//! included — is returned only by a run that did not reach that commit. + +use crate::{ + corpus_fingerprint, inventory, replay, verify_plan, Action, CurationError, DryRunPlan, +}; +use griff_core::corpus::{ + song_holdout_preflight, source_sha256, CorpusManifest, SongHoldoutRefusal, SongId, +}; +use serde::{Deserialize, Serialize}; +use serde_json::Value; +use std::collections::{BTreeMap, BTreeSet}; +use std::fs; +use std::io::Write; +use std::path::{Path, PathBuf}; + +// ── artifact schemas (§5) ────────────────────────────────────────────────────── + +/// Schema id of the application index (§5.1). +pub const APPLICATIONS_SCHEMA: &str = "song-curation.applications.v1"; +/// Schema id of the application report (§5.2). +pub const REPORT_SCHEMA: &str = "song-curation.apply-report.v1"; +/// The lockfile ownership marker (§8.1) — one fixed line. +pub const LOCK_MARKER: &str = "{\"schema\":\"song-curation.lock.v1\"}"; +/// Fixed curated-manifest location inside the published snapshot (§4.3). +pub const CURATED_MANIFEST_RELPATH: &str = "song-curation/manifest.json"; +/// Fixed application-report location inside the published snapshot (§4.3). +pub const REPORT_RELPATH: &str = "song-curation/apply-report.json"; +/// The reserved area name inside every snapshot (§4.2). +pub const RESERVED_DIR: &str = "song-curation"; + +/// The append-only applied-batch registry (§5.1). Strict on the wire. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ApplicationIndex { + pub schema: String, + pub applications: Vec, +} + +/// One applied batch (§5.1). Content-addressed; deliberately no paths. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ApplicationRecord { + pub batch_id: String, + pub report_digest: String, + pub input_corpus_fingerprint: String, + pub output_corpus_fingerprint: String, +} + +/// The application report v1 (§5.2). Proof-bearing; no wall-clock timestamp. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ApplicationReport { + pub schema: String, + pub batch_id: String, + pub applied_event_ids: Vec, + pub input_corpus_fingerprint: String, + pub output_corpus_fingerprint: String, + pub decisions_digest: String, + pub plan_digest: String, + pub previous_application_report_digest: Option, + pub curated_manifest_path: String, + pub curated_manifest_digest: String, + pub assignments_applied: u64, + pub assignments_unchanged: u64, + pub sources_reviewed_unassigned: u64, + pub sources_untouched: u64, + pub coverage: Coverage, + pub holdout_ready: bool, + pub holdout_refusals: Vec, + pub report_digest: String, +} + +/// Post-apply totals over the output snapshot (§5.2). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Coverage { + pub unique_sources: u64, + pub labelled: u64, + pub unlabelled: u64, + pub songs: u64, +} + +/// One recorded holdout-preflight refusal on the curated view (§5.2), sorted +/// by the total tuple `(kind, sha256, song_id, chunk_id)`. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct HoldoutRefusalRecord { + pub kind: String, + pub sha256: Option, + pub song_id: Option, + pub chunk_id: Option, +} + +/// `report_digest` (§5.4): the shared Slice-1 canonical encoding over the +/// complete report with the `report_digest` field omitted. +#[must_use] +pub fn report_digest(report: &ApplicationReport) -> String { + let mut value = serde_json::to_value(report).unwrap_or(Value::Null); + if let Value::Object(map) = &mut value { + map.remove("report_digest"); + } + source_sha256(crate::canonical_json(&value).as_bytes()) +} + +// ── the Apply refusal surface (§12) ──────────────────────────────────────────── + +/// The closed Slice-2 refusal surface (§12): 24 new typed refusals plus the +/// Slice-1 refusals reused verbatim through step 5 — [`ApplyRefusal::PlanVerification`] +/// is transport for those, not a new refusal kind. +#[derive(Debug, Clone, PartialEq, Eq)] +#[allow(clippy::module_name_repetitions)] +pub enum ApplyRefusal { + OutputAlreadyExists { + path: String, + }, + OutputWouldModifyInput { + detail: String, + }, + ApplicationIndexInsideTree { + path: String, + }, + OutputCollidesWithIndexArtifacts { + path: String, + artifact: String, + }, + OutputNameReserved { + path: String, + }, + ApplicationIndexHardLinked { + path: String, + nlink: u64, + }, + ApplicationIndexLocked { + path: String, + }, + ApplicationIndexLockPathOccupied { + path: String, + }, + ApplicationIndexTempExists { + path: String, + }, + CuratedManifestPathNotDistinct { + path: String, + }, + MalformedPlanArtifact { + detail: String, + }, + MalformedApplicationIndex { + detail: String, + }, + CorpusTreeDisagreement { + detail: String, + }, + OrdinaryManifestCarriesSongs, + UnsupportedApplicationIndexSchema { + schema: String, + }, + DuplicateAppliedBatchId { + batch_id: String, + }, + ApplicationIndexChainInvalid { + position: usize, + detail: String, + }, + /// The Slice-1 refusal surface, reused verbatim through step 5 (§12). + PlanVerification { + refusals: Vec, + }, + DecisionBatchAlreadyApplied { + batch_id: String, + }, + ApplicationChainMismatch { + relation: String, + expected: String, + actual: String, + }, + SupersessionEvidenceContradiction { + event_id: String, + detail: String, + }, + ExistingLabelReplacementNotAuthorized { + source_sha256: String, + on_disk_song_id: String, + new_song_id: String, + event_id: String, + }, + NonCanonicalCorpusFile { + path: String, + detail: String, + }, + ApplyIoError { + path: String, + op: String, + detail: String, + }, + OutputPreflightInconsistent { + detail: String, + }, +} + +// ── the observable result shape (§8.2) ───────────────────────────────────────── + +/// The four declared Apply inputs (§4.1). +#[derive(Debug, Clone)] +pub struct ApplyPaths { + pub plan: PathBuf, + pub corpus: PathBuf, + pub index: PathBuf, + pub output: PathBuf, +} + +/// A committed application: the published report is the receipt. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AppliedReceipt { + pub report: ApplicationReport, +} + +/// A failed best-effort lock release — orthogonal to the primary outcome and +/// never a refusal (§8.2 result shape). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LockReleaseWarning { + pub lockfile: String, + pub detail: String, +} + +/// Exactly one primary outcome plus the optional release warning (§8.2). +#[derive(Debug)] +pub struct ApplyRun { + pub primary: Result, + pub lock_release_warning: Option, +} + +// ── entry point ──────────────────────────────────────────────────────────────── + +/// Run transactional Apply under the contract's 12-step verification order. +#[must_use] +pub fn apply(paths: &ApplyPaths) -> ApplyRun { + // Step 1 (pre-lock): resolve the index identity and coordination names. + let ctx = match preflight(paths) { + Ok(ctx) => ctx, + Err(refusal) => { + return ApplyRun { + primary: Err(refusal), + lock_release_warning: None, + } + } + }; + // Step 1 (lock): acquire the single-writer lock; held through step 12. + if let Err(refusal) = acquire_lock(&ctx) { + return ApplyRun { + primary: Err(refusal), + lock_release_warning: None, + }; + } + let primary = locked_apply(paths, &ctx); + let lock_release_warning = release_lock(&ctx.lock_path); + ApplyRun { + primary, + lock_release_warning, + } +} + +/// Everything step 1 resolves before any parsing. +struct Ctx { + canonical_index: PathBuf, + lock_path: PathBuf, + temp_path: PathBuf, + staging: PathBuf, +} + +fn io_refusal(path: &Path, op: &str, err: &std::io::Error) -> ApplyRefusal { + ApplyRefusal::ApplyIoError { + path: path.display().to_string(), + op: op.to_owned(), + detail: err.to_string(), + } +} + +fn preflight(paths: &ApplyPaths) -> Result { + // The supplied index path must resolve to an existing regular file — + // §4.1's missing-index refusal, raised before canonicalization and the + // lock because `canonicalize` needs an existing target. + let canonical_index = + paths + .index + .canonicalize() + .map_err(|e| ApplyRefusal::MalformedApplicationIndex { + detail: format!( + "index {} does not resolve to an existing file: {e}", + paths.index.display() + ), + })?; + let meta = + fs::metadata(&canonical_index).map_err(|e| io_refusal(&canonical_index, "metadata", &e))?; + if !meta.is_file() { + return Err(ApplyRefusal::MalformedApplicationIndex { + detail: format!("index {} is not a regular file", canonical_index.display()), + }); + } + let lock_path = coordination_path(&canonical_index, "lock"); + let temp_path = coordination_path(&canonical_index, "tmp"); + let staging = staging_path(&paths.output); + Ok(Ctx { + canonical_index, + lock_path, + temp_path, + staging, + }) +} + +/// `..` next to the canonical index file (§8.1). +fn coordination_path(canonical_index: &Path, ext: &str) -> PathBuf { + let name = canonical_index + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_default(); + canonical_index + .parent() + .unwrap_or_else(|| Path::new(".")) + .join(format!(".{name}.{ext}")) +} + +/// `..apply-staging` as a sibling of the output path (§8.1). +fn staging_path(output: &Path) -> PathBuf { + let name = output + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_default(); + output + .parent() + .unwrap_or_else(|| Path::new(".")) + .join(format!(".{name}.apply-staging")) +} + +fn acquire_lock(ctx: &Ctx) -> Result<(), ApplyRefusal> { + let mut file = match fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&ctx.lock_path) + { + Ok(file) => file, + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + return Err(ApplyRefusal::ApplicationIndexLocked { + path: ctx.lock_path.display().to_string(), + }); + } + Err(e) => return Err(io_refusal(&ctx.lock_path, "lock create_new", &e)), + }; + if let Err(e) = file.write_all(LOCK_MARKER.as_bytes()) { + let refusal = io_refusal(&ctx.lock_path, "lock marker write", &e); + let _ = fs::remove_file(&ctx.lock_path); + return Err(refusal); + } + Ok(()) +} + +fn release_lock(lock_path: &Path) -> Option { + match fs::remove_file(lock_path) { + Ok(()) => None, + Err(e) => Some(LockReleaseWarning { + lockfile: lock_path.display().to_string(), + detail: format!( + "lock release failed ({e}); the stale lock refuses future applies until \ + the §8.2 recovery removes it" + ), + }), + } +} + +// ── the locked phase: steps 2–12 ─────────────────────────────────────────────── + +#[allow(clippy::too_many_lines)] +fn locked_apply(paths: &ApplyPaths, ctx: &Ctx) -> Result { + // Step 2: strict artifact parsing, before any filesystem mutation. + let plan_text = + fs::read_to_string(&paths.plan).map_err(|e| ApplyRefusal::MalformedPlanArtifact { + detail: format!("{}: {e}", paths.plan.display()), + })?; + let plan: DryRunPlan = + serde_json::from_str(&plan_text).map_err(|e| ApplyRefusal::MalformedPlanArtifact { + detail: format!("{}: {e}", paths.plan.display()), + })?; + let index_text = fs::read_to_string(&ctx.canonical_index).map_err(|e| { + ApplyRefusal::MalformedApplicationIndex { + detail: format!("{}: {e}", ctx.canonical_index.display()), + } + })?; + let index: ApplicationIndex = + serde_json::from_str(&index_text).map_err(|e| ApplyRefusal::MalformedApplicationIndex { + detail: format!("{}: {e}", ctx.canonical_index.display()), + })?; + + // Step 3: snapshot load, tree agreement, reserved-area shape, no-root-songs. + let snapshot = load_snapshot(&paths.corpus)?; + if snapshot.manifest.songs.is_some() { + return Err(ApplyRefusal::OrdinaryManifestCarriesSongs); + } + + // Step 4: application-index validation (§5.1). + validate_index(&index)?; + + // Step 5: plan verification — the Slice-1 `verify_plan`, reused verbatim. + verify_plan(&plan, &snapshot.manifest) + .map_err(|refusals| ApplyRefusal::PlanVerification { refusals })?; + + // Step 6: already-applied — before the chain equations (§7.3). + let batch = &plan.decision_batch; + if index + .applications + .iter() + .any(|r| r.batch_id == batch.batch_id) + { + return Err(ApplyRefusal::DecisionBatchAlreadyApplied { + batch_id: batch.batch_id.clone(), + }); + } + + // Step 7: the chain law (§7.1–§7.2). + let current_fp = corpus_fingerprint(&snapshot.manifest); + check_chain( + batch.previous_application_report_digest.as_deref(), + &batch.input_corpus_fingerprint, + &index, + ¤t_fp, + )?; + + // Step 8: supersession-evidence consistency, then replacement authority. + check_supersession_consistency(&plan)?; + let counts = check_replacement_authority(&plan, &snapshot.manifest)?; + + // Steps 9–12: the filesystem protocol (§8). + stage_publish_commit( + paths, + ctx, + &StageInput { + plan: &plan, + index: &index, + snapshot: &snapshot, + counts: &counts, + }, + ) +} + +/// Everything steps 2–8 verified, handed to the filesystem protocol. +struct StageInput<'a> { + plan: &'a DryRunPlan, + index: &'a ApplicationIndex, + snapshot: &'a Snapshot, + counts: &'a AuthorityCounts, +} + +// ── step 3: the snapshot ─────────────────────────────────────────────────────── + +struct Snapshot { + manifest: CorpusManifest, + /// Every non-reserved file, as (relative path, absolute path), sorted. + files: Vec<(String, PathBuf)>, +} + +fn tree_disagreement(detail: String) -> ApplyRefusal { + ApplyRefusal::CorpusTreeDisagreement { detail } +} + +fn load_snapshot(corpus: &Path) -> Result { + let manifest_path = corpus.join("manifest.json"); + let manifest_text = fs::read_to_string(&manifest_path) + .map_err(|e| tree_disagreement(format!("root manifest.json: {e}")))?; + let manifest: CorpusManifest = serde_json::from_str(&manifest_text) + .map_err(|e| tree_disagreement(format!("root manifest.json: {e}")))?; + + // Recursive sorted walk, the migrate discipline; the reserved area is + // excluded from corpus-content enumeration and shape-checked instead. + let mut files = Vec::new(); + walk(corpus, corpus, &mut files).map_err(|(p, e)| io_refusal(&p, "walk", &e))?; + files.sort(); + let reserved_root = corpus.join(RESERVED_DIR); + if reserved_root.exists() { + check_reserved_shape(corpus, &files)?; + } + let files: Vec<(String, PathBuf)> = files + .into_iter() + .filter(|(rel, _)| !is_reserved(rel)) + .collect(); + + // Tree agreement: manifest chunk records == on-disk chunk records, as + // multisets of canonical encodings (§4.2). + let mut disk_chunks = Vec::new(); + for (rel, abs) in &files { + if rel.ends_with(".chunk.json") { + let text = + fs::read_to_string(abs).map_err(|e| tree_disagreement(format!("{rel}: {e}")))?; + let meta: griff_core::corpus::ChunkMeta = serde_json::from_str(&text) + .map_err(|e| tree_disagreement(format!("{rel}: {e}")))?; + disk_chunks.push(canonical_chunk(&meta)); + } + } + let mut manifest_chunks: Vec = manifest.chunks.iter().map(canonical_chunk).collect(); + disk_chunks.sort(); + manifest_chunks.sort(); + if disk_chunks != manifest_chunks { + let first = manifest_chunks + .iter() + .find(|c| !disk_chunks.contains(c)) + .or_else(|| disk_chunks.iter().find(|c| !manifest_chunks.contains(c))); + return Err(tree_disagreement(format!( + "manifest chunk records and on-disk chunk files disagree ({} records vs {} files); \ + first divergence: {}", + manifest_chunks.len(), + disk_chunks.len(), + first.map_or_else(String::new, |c| truncate(c, 200)), + ))); + } + + Ok(Snapshot { manifest, files }) +} + +fn is_reserved(rel: &str) -> bool { + rel == RESERVED_DIR || rel.starts_with(&format!("{RESERVED_DIR}/")) +} + +fn canonical_chunk(meta: &griff_core::corpus::ChunkMeta) -> String { + crate::canonical_json(&serde_json::to_value(meta).unwrap_or(Value::Null)) +} + +fn truncate(s: &str, n: usize) -> String { + s.chars().take(n).collect() +} + +fn walk( + root: &Path, + dir: &Path, + out: &mut Vec<(String, PathBuf)>, +) -> Result<(), (PathBuf, std::io::Error)> { + let entries = fs::read_dir(dir).map_err(|e| (dir.to_path_buf(), e))?; + for entry in entries { + let path = entry.map_err(|e| (dir.to_path_buf(), e))?.path(); + if path.is_dir() { + walk(root, &path, out)?; + } else { + let rel = path + .strip_prefix(root) + .unwrap_or(&path) + .to_string_lossy() + .replace('\\', "/"); + out.push((rel, path)); + } + } + Ok(()) +} + +/// The reserved-area shape law (§4.2): recursively, at most the two tool-owned +/// proof artifacts as regular files at the reserved root. +fn check_reserved_shape(_corpus: &Path, files: &[(String, PathBuf)]) -> Result<(), ApplyRefusal> { + let allowed = [ + format!("{RESERVED_DIR}/manifest.json"), + format!("{RESERVED_DIR}/apply-report.json"), + ]; + for (rel, _) in files.iter().filter(|(rel, _)| is_reserved(rel)) { + if !allowed.contains(rel) { + return Err(tree_disagreement(format!( + "foreign reserved-area entry: {rel}" + ))); + } + } + Ok(()) +} + +// ── step 4: index validation (§5.1) ──────────────────────────────────────────── + +fn validate_index(index: &ApplicationIndex) -> Result<(), ApplyRefusal> { + if index.schema != APPLICATIONS_SCHEMA { + return Err(ApplyRefusal::UnsupportedApplicationIndexSchema { + schema: index.schema.clone(), + }); + } + let mut seen: BTreeSet<&str> = BTreeSet::new(); + for record in &index.applications { + if !seen.insert(record.batch_id.as_str()) { + return Err(ApplyRefusal::DuplicateAppliedBatchId { + batch_id: record.batch_id.clone(), + }); + } + } + for (i, pair) in index.applications.windows(2).enumerate() { + if pair[1].input_corpus_fingerprint != pair[0].output_corpus_fingerprint { + return Err(ApplyRefusal::ApplicationIndexChainInvalid { + position: i + 1, + detail: format!( + "applications[{}].input_corpus_fingerprint {} != applications[{}].output_corpus_fingerprint {}", + i + 1, + pair[1].input_corpus_fingerprint, + i, + pair[0].output_corpus_fingerprint + ), + }); + } + } + Ok(()) +} + +// ── step 7: the chain law (§7) ───────────────────────────────────────────────── + +fn chain_mismatch(relation: &str, expected: String, actual: String) -> ApplyRefusal { + ApplyRefusal::ApplicationChainMismatch { + relation: relation.to_owned(), + expected, + actual, + } +} + +fn check_chain( + prev: Option<&str>, + batch_input_fp: &str, + index: &ApplicationIndex, + current_fp: &str, +) -> Result<(), ApplyRefusal> { + match index.applications.last() { + None => { + // §7.1: exactly one relation for the initial batch. + if let Some(actual) = prev { + return Err(chain_mismatch( + "(initial) previous_application_report_digest == null", + "null".to_owned(), + actual.to_owned(), + )); + } + } + Some(head) => { + // §7.2 relation (1). + match prev { + Some(p) if p == head.report_digest => {} + other => { + return Err(chain_mismatch( + "previous_application_report_digest == head report_digest", + head.report_digest.clone(), + other.map_or_else(|| "null".to_owned(), ToOwned::to_owned), + )); + } + } + // §7.2 relation (2). + if batch_input_fp != head.output_corpus_fingerprint { + return Err(chain_mismatch( + "batch input_corpus_fingerprint == head output_corpus_fingerprint", + head.output_corpus_fingerprint.clone(), + batch_input_fp.to_owned(), + )); + } + // §7.2 relation (3). + if head.output_corpus_fingerprint != current_fp { + return Err(chain_mismatch( + "head output_corpus_fingerprint == current corpus fingerprint", + current_fp.to_owned(), + head.output_corpus_fingerprint.clone(), + )); + } + } + } + Ok(()) +} + +// ── step 8: supersession consistency + replacement authority (§7.4) ──────────── + +fn sorted_unique(items: &[String]) -> Vec { + let mut v = items.to_vec(); + v.sort(); + v.dedup(); + v +} + +fn check_supersession_consistency(plan: &DryRunPlan) -> Result<(), ApplyRefusal> { + for event in &plan.decision_batch.events { + let contradiction = |detail: String| ApplyRefusal::SupersessionEvidenceContradiction { + event_id: event.event_id.clone(), + detail, + }; + match &event.action { + Action::AcceptSuggestion { + supersedes_song_ids, + .. + } => { + if !supersedes_song_ids.is_empty() { + return Err(contradiction(format!( + "accept_suggestion must carry an empty supersedes_song_ids, got {supersedes_song_ids:?}" + ))); + } + } + Action::Merge { + from_song_ids, + supersedes_song_ids, + .. + } => { + if sorted_unique(supersedes_song_ids) != sorted_unique(from_song_ids) { + return Err(contradiction(format!( + "merge supersedes_song_ids {supersedes_song_ids:?} != from_song_ids {from_song_ids:?}" + ))); + } + } + Action::Split { + from_song_id, + supersedes_song_ids, + .. + } => { + if supersedes_song_ids != &vec![from_song_id.clone()] { + return Err(contradiction(format!( + "split supersedes_song_ids {supersedes_song_ids:?} != [{from_song_id:?}]" + ))); + } + } + Action::ManualDefine { .. } + | Action::Correct { .. } + | Action::RejectSuggestion { .. } => {} + } + } + Ok(()) +} + +/// The four-way partition of the snapshot's sources (§5.2). +struct AuthorityCounts { + applied: u64, + unchanged: u64, + reviewed_unassigned: u64, + untouched: u64, +} + +/// The authorized supersession set of the acting event (§7.4). +fn authorized_set(action: &Action) -> BTreeSet<&str> { + match action { + Action::Correct { + supersedes_song_ids, + .. + } => supersedes_song_ids.iter().map(String::as_str).collect(), + Action::Merge { from_song_ids, .. } => from_song_ids.iter().map(String::as_str).collect(), + Action::Split { from_song_id, .. } => std::iter::once(from_song_id.as_str()).collect(), + Action::AcceptSuggestion { .. } + | Action::ManualDefine { .. } + | Action::RejectSuggestion { .. } => BTreeSet::new(), + } +} + +fn check_replacement_authority( + plan: &DryRunPlan, + manifest: &CorpusManifest, +) -> Result { + // verify_plan (step 5) has proven the plan projection is reproducible, + // so inventory and replay cannot fail here; the single shared replay + // primitive also carries the acting-event attribution (§9). + let inv = + inventory(manifest).map_err(|refusals| ApplyRefusal::PlanVerification { refusals })?; + let state = replay(&inv, &plan.decision_batch) + .map_err(|refusals| ApplyRefusal::PlanVerification { refusals })?; + let on_disk: BTreeMap<&str, Option<&str>> = inv + .sources + .iter() + .map(|s| { + ( + s.source_sha256.as_str(), + s.existing_song_ids.first().map(String::as_str), + ) + }) + .collect(); + let events: BTreeMap<&str, &Action> = plan + .decision_batch + .events + .iter() + .map(|e| (e.event_id.as_str(), &e.action)) + .collect(); + + let mut counts = AuthorityCounts { + applied: 0, + unchanged: 0, + reviewed_unassigned: 0, + untouched: 0, + }; + for (sha, effect) in &state { + let Some(new_song) = &effect.song else { + counts.reviewed_unassigned += 1; + continue; + }; + let disk = on_disk.get(sha.as_str()).copied().flatten(); + match disk { + None => counts.applied += 1, + Some(existing) if existing == new_song => counts.unchanged += 1, + Some(existing) => { + let action = events.get(effect.event_id.as_str()).copied(); + let authorized = action.is_some_and(|a| authorized_set(a).contains(existing)); + if !authorized { + return Err(ApplyRefusal::ExistingLabelReplacementNotAuthorized { + source_sha256: sha.clone(), + on_disk_song_id: existing.to_owned(), + new_song_id: new_song.clone(), + event_id: effect.event_id.clone(), + }); + } + counts.applied += 1; + } + } + } + let total = u64::try_from(inv.sources.len()).unwrap_or(u64::MAX); + counts.untouched = total - counts.applied - counts.unchanged - counts.reviewed_unassigned; + Ok(counts) +} + +// ── steps 9–12: the filesystem protocol (§8) ─────────────────────────────────── + +fn cleanup_staging(staging: &Path) { + let _ = fs::remove_dir_all(staging); +} + +#[allow(clippy::too_many_lines)] +fn stage_publish_commit( + paths: &ApplyPaths, + ctx: &Ctx, + input: &StageInput<'_>, +) -> Result { + let StageInput { + plan, + index, + snapshot, + counts, + } = *input; + let assigned: BTreeMap<&str, &str> = plan + .assignments + .iter() + .map(|a| (a.source_sha256.as_str(), a.song_id.as_str())) + .collect(); + + // Step 9: stage the snapshot — corpus files under the preservation law + // (§10) plus the curated manifest. The report is written only in step 10. + let staged = stage_snapshot(ctx, plan, snapshot, &assigned); + let staged_manifest = match staged { + Ok(m) => m, + Err(refusal) => { + cleanup_staging(&ctx.staging); + return Err(refusal); + } + }; + + // Step 10: staged self-check from bytes, the single preflight, then the + // report. + let step10 = staged_selfcheck_and_report(ctx, plan, counts, &staged_manifest); + let report = match step10 { + Ok(report) => report, + Err(refusal) => { + cleanup_staging(&ctx.staging); + return Err(refusal); + } + }; + + // Step 11: publish the snapshot with one rename. + if let Err(e) = fs::rename(&ctx.staging, &paths.output) { + let refusal = io_refusal(&paths.output, "publish rename", &e); + cleanup_staging(&ctx.staging); + return Err(refusal); + } + + // Step 12: commit — temp write + rename over the canonical index. Only + // after this rename is the batch applied. + let mut updated = index.clone(); + updated.applications.push(ApplicationRecord { + batch_id: report.batch_id.clone(), + report_digest: report.report_digest.clone(), + input_corpus_fingerprint: report.input_corpus_fingerprint.clone(), + output_corpus_fingerprint: report.output_corpus_fingerprint.clone(), + }); + let index_json = + serde_json::to_string_pretty(&updated).map_err(|e| ApplyRefusal::ApplyIoError { + path: ctx.temp_path.display().to_string(), + op: "serialize index".to_owned(), + detail: e.to_string(), + })?; + let mut temp = fs::File::create(&ctx.temp_path) + .map_err(|e| io_refusal(&ctx.temp_path, "temp create", &e))?; + temp.write_all(index_json.as_bytes()) + .map_err(|e| io_refusal(&ctx.temp_path, "temp write", &e))?; + temp.sync_all() + .map_err(|e| io_refusal(&ctx.temp_path, "temp sync", &e))?; + drop(temp); + fs::rename(&ctx.temp_path, &ctx.canonical_index).map_err(|e| { + let refusal = io_refusal(&ctx.canonical_index, "commit rename", &e); + let _ = fs::remove_file(&ctx.temp_path); + refusal + })?; + + Ok(AppliedReceipt { report }) +} + +/// Step 9: write the staged output tree. Returns the modified manifest. +fn stage_snapshot( + ctx: &Ctx, + plan: &DryRunPlan, + snapshot: &Snapshot, + assigned: &BTreeMap<&str, &str>, +) -> Result { + fs::create_dir(&ctx.staging).map_err(|e| io_refusal(&ctx.staging, "staging create_dir", &e))?; + + let manifest_touched = !assigned.is_empty(); + let mut modified_manifest = snapshot.manifest.clone(); + for chunk in &mut modified_manifest.chunks { + if let Some(sha) = &chunk.source.sha256 { + if let Some(song) = assigned.get(sha.as_str()) { + chunk.source.song_id = Some(SongId((*song).to_owned())); + } + } + } + + for (rel, abs) in &snapshot.files { + let dest = ctx.staging.join(rel); + if let Some(parent) = dest.parent() { + fs::create_dir_all(parent).map_err(|e| io_refusal(parent, "staging mkdir", &e))?; + } + let is_manifest = rel == "manifest.json"; + let is_chunk = rel.ends_with(".chunk.json"); + let touched = if is_manifest { + manifest_touched + } else if is_chunk { + let text = fs::read_to_string(abs).map_err(|e| io_refusal(abs, "read chunk", &e))?; + let meta: griff_core::corpus::ChunkMeta = serde_json::from_str(&text) + .map_err(|e| tree_disagreement(format!("{rel}: {e}")))?; + meta.source + .sha256 + .as_deref() + .is_some_and(|sha| assigned.contains_key(sha)) + } else { + false + }; + if touched { + write_touched(rel, abs, &dest, assigned, &modified_manifest)?; + } else { + let bytes = fs::read(abs).map_err(|e| io_refusal(abs, "read", &e))?; + fs::write(&dest, bytes).map_err(|e| io_refusal(&dest, "write", &e))?; + } + } + + // The curated manifest (§5.3): the output manifest with the exact songs + // projection at the protected distinct path. + let curated_dir = ctx.staging.join(RESERVED_DIR); + fs::create_dir_all(&curated_dir).map_err(|e| io_refusal(&curated_dir, "mkdir", &e))?; + let mut curated = modified_manifest.clone(); + curated.songs = Some( + plan.generated_songs_map + .iter() + .map(|(song, shas)| (SongId(song.clone()), shas.clone())) + .collect(), + ); + let curated_json = + serde_json::to_string_pretty(&curated).map_err(|e| ApplyRefusal::ApplyIoError { + path: CURATED_MANIFEST_RELPATH.to_owned(), + op: "serialize curated manifest".to_owned(), + detail: e.to_string(), + })?; + let curated_path = ctx.staging.join(CURATED_MANIFEST_RELPATH); + fs::write(&curated_path, curated_json).map_err(|e| io_refusal(&curated_path, "write", &e))?; + + Ok(modified_manifest) +} + +/// Rewrite one touched JSON file under the §10 preservation law: semantic +/// identity outside the assigned `source.song_id` members, canonical +/// rendering, fail-closed laundering guard. +fn write_touched( + rel: &str, + abs: &Path, + dest: &Path, + assigned: &BTreeMap<&str, &str>, + modified_manifest: &CorpusManifest, +) -> Result<(), ApplyRefusal> { + let text = fs::read_to_string(abs).map_err(|e| io_refusal(abs, "read", &e))?; + let raw: Value = + serde_json::from_str(&text).map_err(|e| tree_disagreement(format!("{rel}: {e}")))?; + let rendered = if rel == "manifest.json" { + // Guard the unmodified round-trip first (§10.3). + let reparsed: CorpusManifest = + serde_json::from_str(&text).map_err(|e| tree_disagreement(format!("{rel}: {e}")))?; + guard_round_trip(rel, &raw, &serde_json::to_value(&reparsed))?; + serde_json::to_string_pretty(modified_manifest) + } else { + let meta: griff_core::corpus::ChunkMeta = + serde_json::from_str(&text).map_err(|e| tree_disagreement(format!("{rel}: {e}")))?; + guard_round_trip(rel, &raw, &serde_json::to_value(&meta))?; + let mut meta = meta; + if let Some(sha) = meta.source.sha256.clone() { + if let Some(song) = assigned.get(sha.as_str()) { + meta.source.song_id = Some(SongId((*song).to_owned())); + } + } + serde_json::to_string_pretty(&meta) + }; + let rendered = rendered.map_err(|e| ApplyRefusal::ApplyIoError { + path: rel.to_owned(), + op: "serialize".to_owned(), + detail: e.to_string(), + })?; + fs::write(dest, rendered).map_err(|e| io_refusal(dest, "write", &e)) +} + +fn guard_round_trip( + rel: &str, + raw: &Value, + reserialized: &Result, +) -> Result<(), ApplyRefusal> { + let non_canonical = |detail: String| ApplyRefusal::NonCanonicalCorpusFile { + path: rel.to_owned(), + detail, + }; + let reserialized = reserialized + .as_ref() + .map_err(|e| non_canonical(format!("re-serialization failed: {e}")))?; + if reserialized != raw { + return Err(non_canonical( + "parse→serialize round-trip diverges from the raw JSON value — the file would \ + be laundered by a rewrite (unknown member, or a value the parse re-renders \ + differently)" + .to_owned(), + )); + } + Ok(()) +} + +/// Step 10: re-read staged bytes, recompute the proofs, run the single +/// preflight over the curated view, then build and write the report. +fn staged_selfcheck_and_report( + ctx: &Ctx, + plan: &DryRunPlan, + counts: &AuthorityCounts, + expected_manifest: &CorpusManifest, +) -> Result { + let inconsistent = |detail: String| ApplyRefusal::OutputPreflightInconsistent { detail }; + + // Re-read the staged root manifest from bytes. + let staged_manifest_text = fs::read_to_string(ctx.staging.join("manifest.json")) + .map_err(|e| inconsistent(format!("staged manifest.json unreadable: {e}")))?; + let staged_manifest: CorpusManifest = serde_json::from_str(&staged_manifest_text) + .map_err(|e| inconsistent(format!("staged manifest.json unparseable: {e}")))?; + let expected_value = serde_json::to_value(expected_manifest).unwrap_or(Value::Null); + let staged_value = serde_json::to_value(&staged_manifest).unwrap_or(Value::Null); + if expected_value != staged_value { + return Err(inconsistent( + "staged root manifest diverges from the derived output manifest".to_owned(), + )); + } + let output_fp = corpus_fingerprint(&staged_manifest); + + // Re-read the staged curated manifest from bytes; its digest is over the + // exact published bytes (§5.2). + let curated_path = ctx.staging.join(CURATED_MANIFEST_RELPATH); + let curated_bytes = fs::read(&curated_path) + .map_err(|e| inconsistent(format!("staged curated manifest: {e}")))?; + let curated_digest = source_sha256(&curated_bytes); + let curated: CorpusManifest = serde_json::from_slice(&curated_bytes) + .map_err(|e| inconsistent(format!("staged curated manifest unparseable: {e}")))?; + + // The single execution of the real core preflight (§11). + let (holdout_ready, holdout_refusals) = match song_holdout_preflight(&curated) { + Ok(()) => (true, Vec::new()), + Err(refusals) => { + let mut records = Vec::new(); + for refusal in refusals { + match refusal { + SongHoldoutRefusal::UncuratedSource { sha256, example } => { + records.push(HoldoutRefusalRecord { + kind: "uncurated_source".to_owned(), + sha256: Some(sha256), + song_id: None, + chunk_id: Some(example.0), + }); + } + other => { + return Err(inconsistent(format!( + "non-partiality preflight refusal on the staged curated view: {other:?}" + ))); + } + } + } + records.sort(); + (false, records) + } + }; + + // Coverage over the curated output view. + let inv = inventory(&curated) + .map_err(|e| inconsistent(format!("staged curated view fails inventory: {e:?}")))?; + let unique = u64::try_from(inv.sources.len()).unwrap_or(u64::MAX); + let labelled = u64::try_from( + inv.sources + .iter() + .filter(|s| !s.existing_song_ids.is_empty()) + .count(), + ) + .unwrap_or(u64::MAX); + let songs: BTreeSet<&String> = inv + .sources + .iter() + .flat_map(|s| s.existing_song_ids.iter()) + .collect(); + + let mut report = ApplicationReport { + schema: REPORT_SCHEMA.to_owned(), + batch_id: plan.decision_batch.batch_id.clone(), + applied_event_ids: plan + .decision_batch + .events + .iter() + .map(|e| e.event_id.clone()) + .collect(), + input_corpus_fingerprint: plan.input_corpus_fingerprint.clone(), + output_corpus_fingerprint: output_fp, + decisions_digest: plan.decisions_digest.clone(), + plan_digest: plan.plan_digest.clone(), + previous_application_report_digest: plan + .decision_batch + .previous_application_report_digest + .clone(), + curated_manifest_path: CURATED_MANIFEST_RELPATH.to_owned(), + curated_manifest_digest: curated_digest, + assignments_applied: counts.applied, + assignments_unchanged: counts.unchanged, + sources_reviewed_unassigned: counts.reviewed_unassigned, + sources_untouched: counts.untouched, + coverage: Coverage { + unique_sources: unique, + labelled, + unlabelled: unique - labelled, + songs: u64::try_from(songs.len()).unwrap_or(u64::MAX), + }, + holdout_ready, + holdout_refusals, + report_digest: String::new(), + }; + report.report_digest = report_digest(&report); + + let report_json = + serde_json::to_string_pretty(&report).map_err(|e| ApplyRefusal::ApplyIoError { + path: REPORT_RELPATH.to_owned(), + op: "serialize report".to_owned(), + detail: e.to_string(), + })?; + let report_path = ctx.staging.join(REPORT_RELPATH); + fs::write(&report_path, report_json).map_err(|e| io_refusal(&report_path, "write", &e))?; + + Ok(report) +} diff --git a/song-curation/src/lib.rs b/song-curation/src/lib.rs index 7898172..6c73441 100644 --- a/song-curation/src/lib.rs +++ b/song-curation/src/lib.rs @@ -20,6 +20,8 @@ use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use std::collections::{BTreeMap, BTreeSet}; +pub mod apply; + // ── inventory ───────────────────────────────────────────────────────────────── /// One source file (by exact `sha256`) with its deterministic, sorted/unique @@ -176,10 +178,21 @@ const POLICY_VERSION: &str = "1"; /// The one decisions-ledger schema this slice reads; anything else is refused. const DECISIONS_SCHEMA: &str = "song-curation.decisions.v1"; -/// Per-source batch state after replay: `Some(song)` = the batch assigns this -/// label; `None` = the batch reviewed the source without assigning (a `reject` -/// that supersedes any earlier pending assignment). -type BatchState = BTreeMap>; +/// Per-source batch effect after replay: `song = Some(..)` = the batch assigns +/// this label; `None` = the batch reviewed the source without assigning (a +/// `reject` that supersedes any earlier pending assignment). `event_id` names +/// the acting event — the latest event whose effect set this state — which is +/// the §9-permitted internal attribution the Slice-2 authority law consumes. +/// Attribution is carried alongside the effect and never changes what Slice 1 +/// derives from it. +#[derive(Debug, Clone)] +pub(crate) struct BatchEffect { + pub(crate) song: Option, + pub(crate) event_id: String, +} + +/// Per-source batch state after replay (latest event wins). +pub(crate) type BatchState = BTreeMap; // ── typed refusals (Slice-1 subset) ───────────────────────────────────────────── @@ -247,7 +260,9 @@ pub enum CurationError { /// Emit `value` as compact UTF-8 JSON with object keys sorted lexicographically /// and array order preserved — the shared canonical encoding for every digest. -fn canonical_json(value: &Value) -> String { +/// Crate-visible so Slice 2's `report_digest` uses this one encoding rather +/// than a near-copy (contract §5.4). +pub(crate) fn canonical_json(value: &Value) -> String { let mut out = String::new(); write_canonical(value, &mut out); out @@ -583,7 +598,10 @@ fn action_effects(action: &Action) -> Vec<(String, Option)> { /// the source without assigning (a reject that supersedes any earlier pending /// assignment). Every referenced source is validated; a source assigned two /// distinct labels within one event refuses. -fn replay(inventory: &Inventory, batch: &DecisionBatch) -> Result> { +pub(crate) fn replay( + inventory: &Inventory, + batch: &DecisionBatch, +) -> Result> { let known: BTreeSet<&str> = inventory .sources .iter() @@ -609,7 +627,13 @@ fn replay(inventory: &Inventory, batch: &DecisionBatch) -> Result = state .iter() .filter_map(|(sha, effect)| { - let song = effect.clone()?; + let song = effect.song.clone()?; let record = by_sha.get(sha.as_str()); Some(Assignment { source_sha256: sha.clone(), @@ -673,7 +697,9 @@ fn derive( // untouched labels survive (the map is the Slice-2 manifest projection). let mut generated_songs_map: BTreeMap> = BTreeMap::new(); for source in &inventory.sources { - let batch_label = state.get(&source.source_sha256).and_then(Clone::clone); + let batch_label = state + .get(&source.source_sha256) + .and_then(|effect| effect.song.clone()); if let Some(song) = batch_label.or_else(|| source.existing_label()) { generated_songs_map .entry(song) From 6ce1a69572508712894ad66510ae6df960b702b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 14:39:22 +0000 Subject: [PATCH 04/18] test(song-curation): add Slice 2 fault-injection fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixture/harness commit — no production behaviour. Adds the deterministic fault-injection registry (apply::fault): thread-local, one-shot hooks that a test registers at named points; a production run registers none, so every point is an inert no-op. One-shot consumption makes a nested Apply inside a hook safe (a point cannot re-fire recursively), which is what lets the concurrency cases (C14, C16 live window) run a second applier inline on the same thread instead of relying on scheduler timing. Named points wired into the protocol as inert pass-throughs: lock:after_create — between lock create_new and marker publication lock:release — best-effort release failure (F9) stage:write — staged-write failure (F4) stage:before_selfcheck — staged-corruption window before step 10 (K11) publish:rename — failure of the step-11 publication rename (F4) commit:before_temp — late temp-occupant window (F12) commit:temp_write — temp-write failure inside step 12 (F4/R4) commit:rename — commit-rename failure after the temp exists (F4/R4; also the C14 live step-12 temp window) Full suite stays green (45 + 27 + 16); clippy/fmt clean. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/src/apply.rs | 34 ++++++++++++++++++++++++++ song-curation/src/apply/fault.rs | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 song-curation/src/apply/fault.rs diff --git a/song-curation/src/apply.rs b/song-curation/src/apply.rs index 98feced..9ca5abf 100644 --- a/song-curation/src/apply.rs +++ b/song-curation/src/apply.rs @@ -14,6 +14,8 @@ //! index temp+`rename` is the single commit point. Every refusal — I/O //! included — is returned only by a run that did not reach that commit. +pub mod fault; + use crate::{ corpus_fingerprint, inventory, replay, verify_plan, Action, CurationError, DryRunPlan, }; @@ -360,6 +362,11 @@ fn acquire_lock(ctx: &Ctx) -> Result<(), ApplyRefusal> { } Err(e) => return Err(io_refusal(&ctx.lock_path, "lock create_new", &e)), }; + if let Err(e) = fault::hit("lock:after_create") { + let refusal = io_refusal(&ctx.lock_path, "lock marker write", &e); + let _ = fs::remove_file(&ctx.lock_path); + return Err(refusal); + } if let Err(e) = file.write_all(LOCK_MARKER.as_bytes()) { let refusal = io_refusal(&ctx.lock_path, "lock marker write", &e); let _ = fs::remove_file(&ctx.lock_path); @@ -369,6 +376,15 @@ fn acquire_lock(ctx: &Ctx) -> Result<(), ApplyRefusal> { } fn release_lock(lock_path: &Path) -> Option { + if let Err(e) = fault::hit("lock:release") { + return Some(LockReleaseWarning { + lockfile: lock_path.display().to_string(), + detail: format!( + "lock release failed ({e}); the stale lock refuses future applies until \ + the §8.2 recovery removes it" + ), + }); + } match fs::remove_file(lock_path) { Ok(()) => None, Err(e) => Some(LockReleaseWarning { @@ -851,6 +867,11 @@ fn stage_publish_commit( // Step 10: staged self-check from bytes, the single preflight, then the // report. + if let Err(e) = fault::hit("stage:before_selfcheck") { + let refusal = io_refusal(&ctx.staging, "staging", &e); + cleanup_staging(&ctx.staging); + return Err(refusal); + } let step10 = staged_selfcheck_and_report(ctx, plan, counts, &staged_manifest); let report = match step10 { Ok(report) => report, @@ -861,6 +882,11 @@ fn stage_publish_commit( }; // Step 11: publish the snapshot with one rename. + if let Err(e) = fault::hit("publish:rename") { + let refusal = io_refusal(&paths.output, "publish rename", &e); + cleanup_staging(&ctx.staging); + return Err(refusal); + } if let Err(e) = fs::rename(&ctx.staging, &paths.output) { let refusal = io_refusal(&paths.output, "publish rename", &e); cleanup_staging(&ctx.staging); @@ -882,13 +908,20 @@ fn stage_publish_commit( op: "serialize index".to_owned(), detail: e.to_string(), })?; + fault::hit("commit:before_temp").map_err(|e| io_refusal(&ctx.temp_path, "temp create", &e))?; let mut temp = fs::File::create(&ctx.temp_path) .map_err(|e| io_refusal(&ctx.temp_path, "temp create", &e))?; + fault::hit("commit:temp_write").map_err(|e| io_refusal(&ctx.temp_path, "temp write", &e))?; temp.write_all(index_json.as_bytes()) .map_err(|e| io_refusal(&ctx.temp_path, "temp write", &e))?; temp.sync_all() .map_err(|e| io_refusal(&ctx.temp_path, "temp sync", &e))?; drop(temp); + if let Err(e) = fault::hit("commit:rename") { + let refusal = io_refusal(&ctx.canonical_index, "commit rename", &e); + let _ = fs::remove_file(&ctx.temp_path); + return Err(refusal); + } fs::rename(&ctx.temp_path, &ctx.canonical_index).map_err(|e| { let refusal = io_refusal(&ctx.canonical_index, "commit rename", &e); let _ = fs::remove_file(&ctx.temp_path); @@ -918,6 +951,7 @@ fn stage_snapshot( } for (rel, abs) in &snapshot.files { + fault::hit("stage:write").map_err(|e| io_refusal(&ctx.staging, "staging write", &e))?; let dest = ctx.staging.join(rel); if let Some(parent) = dest.parent() { fs::create_dir_all(parent).map_err(|e| io_refusal(parent, "staging mkdir", &e))?; diff --git a/song-curation/src/apply/fault.rs b/song-curation/src/apply/fault.rs new file mode 100644 index 0000000..8f670b8 --- /dev/null +++ b/song-curation/src/apply/fault.rs @@ -0,0 +1,41 @@ +//! Deterministic fault-injection points for the §14 adversarial matrix. +//! +//! Pure test harness: hooks are thread-local and one-shot; a production run +//! registers none, so every point is an inert no-op. A hook may perform side +//! effects (corrupt a staged file, run a second Apply inline — same thread, +//! no scheduler timing) and/or return an `Err` that the surrounding +//! operation treats as its own I/O failure. One-shot removal makes nested +//! Apply calls inside a hook safe (the point cannot re-fire recursively). + +use std::cell::RefCell; +use std::collections::BTreeMap; +use std::io; + +type Hook = Box io::Result<()>>; + +thread_local! { + static HOOKS: RefCell> = RefCell::new(BTreeMap::new()); +} + +/// Register a one-shot hook at a named point (test harness only). +pub fn set(point: &'static str, hook: F) +where + F: FnMut() -> io::Result<()> + 'static, +{ + HOOKS.with(|h| h.borrow_mut().insert(point, Box::new(hook))); +} + +/// Remove every registered hook (call between test scenarios). +pub fn clear() { + HOOKS.with(|h| h.borrow_mut().clear()); +} + +/// Fire the hook at `point`, if any. The hook is consumed before it runs, so +/// a nested Apply inside it never re-enters the same point. +pub(crate) fn hit(point: &'static str) -> io::Result<()> { + let hook = HOOKS.with(|h| h.borrow_mut().remove(point)); + match hook { + None => Ok(()), + Some(mut hook) => hook(), + } +} From bd2a339a8ef86601be72de5c5c039a434d515b5c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 14:43:23 +0000 Subject: [PATCH 05/18] test(song-curation): preregister transactional Apply adversarial RED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests-only commit — no production changes. Preregisters the remaining 20 §14 cases: F1, F2, F4, F6–F13, C11–C16, K10, K11, R4. Concurrency cases run the second applier inline inside a one-shot fault hook on the same thread (C14 inside the live step-12 temp window; C16(a) inside the create_new→marker window), so nothing depends on scheduler timing. RED evidence (cargo test --test apply_adversarial): 8 passed; 12 FAILED. Genuinely RED — the §8/§6 adversarial hardening does not exist yet: f1 (containment + symlink aliases → OutputWouldModifyInput) f2 (pre-existing output/staging → OutputAlreadyExists) f6 (index inside tree → ApplicationIndexInsideTree) f10 (output vs lock/tmp/index collisions → OutputCollidesWithIndexArtifacts) f11 (hardlinked index → ApplicationIndexHardLinked) f12 (late temp occupant → pre-commit ApplyIoError, occupant untouched) f13 (reserved staging namespace → OutputNameReserved) c13 (real second index at the temp name → ApplicationIndexTempExists, never unlinked; today it is silently truncated) c15 (real second index at the lock name → ApplicationIndexLockPathOccupied) k10 (duplicate JSON keys → NonCanonicalCorpusFile via a distinct duplicate-rejecting pass; Value comparison cannot see them) k11 (staged tree corruption → OutputPreflightInconsistent via the staged tree-agreement re-run) r4 (orphan output after a commit failure must make a retry refuse OutputAlreadyExists, not fail as a late I/O error) Already green — characterization of behaviour the GREEN-A core plus the fault fixtures already provide (no new public API; passing before commit per the repo's characterization rule): f4, f7, f8, f9, c11, c12, c14, c16. They are committed here to pin those §14 laws against regression while the RED cases above are closed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/tests/apply_adversarial.rs | 668 +++++++++++++++++++++++ 1 file changed, 668 insertions(+) create mode 100644 song-curation/tests/apply_adversarial.rs diff --git a/song-curation/tests/apply_adversarial.rs b/song-curation/tests/apply_adversarial.rs new file mode 100644 index 0000000..5b1389c --- /dev/null +++ b/song-curation/tests/apply_adversarial.rs @@ -0,0 +1,668 @@ +//! Slice-2 preregistered acceptance matrix — filesystem / transactional +//! adversarial cases (ADR-0033 Slice 2 contract §14): F1, F2, F4, F6–F13, +//! C11–C16, K10, K11, R4. +//! +//! Fault points come from the deterministic `apply::fault` harness; the +//! concurrency cases run the second applier inline inside a hook on the same +//! thread, so nothing depends on scheduler timing. + +mod common; + +use common::{ + accept, batch_for, event, layout, lock_path_of, read_value, staging_path_of, temp_path_of, + write_corpus, write_empty_index, write_plan, Chunk, Layout, EMPTY_INDEX, +}; +use griff_song_curation::apply::{ + apply, fault, ApplyPaths, ApplyRefusal, ApplyRun, LOCK_MARKER, +}; +use std::cell::RefCell; +use std::fs; +use std::io; +use std::path::Path; +use std::rc::Rc; + +fn run(l: &Layout) -> ApplyRun { + run_paths(&l.plan, &l.corpus, &l.index, &l.output) +} + +fn run_paths(plan: &Path, corpus: &Path, index: &Path, output: &Path) -> ApplyRun { + apply(&ApplyPaths { + plan: plan.to_path_buf(), + corpus: corpus.to_path_buf(), + index: index.to_path_buf(), + output: output.to_path_buf(), + }) +} + +fn refuse(l: &Layout) -> ApplyRefusal { + run(l).primary.expect_err("expected a refusal") +} + +/// Standard valid setup: A/B corpus, one accept of shaA, empty index. +fn valid_setup(l: &Layout) { + let m = write_corpus(&l.corpus, &AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); +} + +const AB: [Chunk<'static>; 2] = [ + Chunk { + id: "a1", + sha: Some("shaA"), + song: None, + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: None, + }, +]; + +fn io_fail() -> io::Result<()> { + Err(io::Error::other("injected fault")) +} + +// ── F. filesystem preflight and protocol ─────────────────────────────────────── + +#[test] +fn f1_output_inside_input_refuses_including_symlink_aliases() { + // (a) output textually inside the corpus root. + let l = layout("f1a"); + valid_setup(&l); + let output = l.corpus.join("out"); + let result = run_paths(&l.plan, &l.corpus, &l.index, &output); + let refusal = result.primary.expect_err("must refuse"); + assert!( + matches!(refusal, ApplyRefusal::OutputWouldModifyInput { .. }), + "got {refusal:?}" + ); + assert!(!output.exists()); + + // (b) the same containment reached only through a symlink alias of the + // corpus root. + let l = layout("f1b"); + valid_setup(&l); + let alias = l.td.path.join("corpus-alias"); + std::os::unix::fs::symlink(&l.corpus, &alias).expect("symlink"); + let output = l.corpus.join("out2"); + let result = run_paths(&l.plan, &alias, &l.index, &output); + let refusal = result.primary.expect_err("must refuse"); + assert!( + matches!(refusal, ApplyRefusal::OutputWouldModifyInput { .. }), + "canonicalization must see through the alias, got {refusal:?}" + ); +} + +#[test] +fn f2_pre_existing_output_or_staging_refuses() { + // (a) pre-existing (non-empty) output. + let l = layout("f2a"); + valid_setup(&l); + fs::create_dir_all(l.output.join("keep")).expect("mk output"); + fs::write(l.output.join("keep/x"), "x").expect("occupant"); + let refusal = refuse(&l); + assert!( + matches!(refusal, ApplyRefusal::OutputAlreadyExists { .. }), + "got {refusal:?}" + ); + assert!(l.output.join("keep/x").exists(), "occupant untouched"); + + // (b) pre-existing staging directory. + let l = layout("f2b"); + valid_setup(&l); + fs::create_dir_all(staging_path_of(&l.output)).expect("mk staging"); + let refusal = refuse(&l); + match &refusal { + ApplyRefusal::OutputAlreadyExists { path } => { + assert!(path.contains(".apply-staging"), "carries the staging path"); + } + other => panic!("expected OutputAlreadyExists, got {other:?}"), + } +} + +#[test] +fn f4_injected_failures_land_in_enumerated_states() { + // (a) staging write failure: nothing published, staging cleaned. + let l = layout("f4a"); + valid_setup(&l); + let before = fs::read(&l.index).expect("index"); + fault::set("stage:write", io_fail); + let refusal = refuse(&l); + fault::clear(); + assert!(matches!(refusal, ApplyRefusal::ApplyIoError { .. })); + assert!(!l.output.exists()); + assert!(!staging_path_of(&l.output).exists(), "cleanup ran"); + assert_eq!(fs::read(&l.index).expect("index"), before); + + // (b) failure of the step-11 publication rename itself: output absent, + // index unchanged; staging may remain only if cleanup also failed. + let l = layout("f4b"); + valid_setup(&l); + let before = fs::read(&l.index).expect("index"); + fault::set("publish:rename", io_fail); + let refusal = refuse(&l); + fault::clear(); + assert!(matches!(refusal, ApplyRefusal::ApplyIoError { .. })); + assert!(!l.output.exists()); + assert_eq!(fs::read(&l.index).expect("index"), before); + + // (c) temp-write failure inside step 12: the output is already + // published, the old index is unchanged — provably not applied. + let l = layout("f4c"); + valid_setup(&l); + let before = fs::read(&l.index).expect("index"); + fault::set("commit:temp_write", io_fail); + let refusal = refuse(&l); + fault::clear(); + assert!(matches!(refusal, ApplyRefusal::ApplyIoError { .. })); + assert!(l.output.exists(), "published orphan (§8.2)"); + assert_eq!(fs::read(&l.index).expect("index"), before, "not applied"); + + // (d) commit-rename failure: same state; the run's own temp is removed. + let l = layout("f4d"); + valid_setup(&l); + let before = fs::read(&l.index).expect("index"); + fault::set("commit:rename", io_fail); + let refusal = refuse(&l); + fault::clear(); + assert!(matches!(refusal, ApplyRefusal::ApplyIoError { .. })); + assert!(l.output.exists(), "published orphan (§8.2)"); + assert_eq!(fs::read(&l.index).expect("index"), before, "not applied"); + assert!(!temp_path_of(&l.index).exists(), "own temp removed"); +} + +#[test] +fn f6_index_inside_corpus_tree_refuses() { + let l = layout("f6"); + let m = write_corpus(&l.corpus, &AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + let index = l.corpus.join("index.json"); + write_empty_index(&index); + let result = run_paths(&l.plan, &l.corpus, &index, &l.output); + let refusal = result.primary.expect_err("must refuse"); + assert!( + matches!(refusal, ApplyRefusal::ApplicationIndexInsideTree { .. }), + "got {refusal:?}" + ); + assert!(!l.output.exists()); +} + +#[test] +fn f7_path_like_batch_id_influences_no_filesystem_path() { + let l = layout("f7"); + let m = write_corpus(&l.corpus, &AB); + let b = batch_for( + &m, + "../griff-f7-escape", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + let result = run(&l); + assert!( + result.primary.is_ok(), + "the apply proceeds purely by contract law" + ); + for escape in [ + l.td.path.join("griff-f7-escape"), + l.td.path.parent().expect("parent").join("griff-f7-escape"), + ] { + assert!(!escape.exists(), "no path derived from batch_id: {escape:?}"); + } +} + +#[test] +fn f8_index_symlink_aliases_converge_on_the_canonical_file() { + let l = layout("f8"); + valid_setup(&l); + let alias = l.td.path.join("alias.json"); + std::os::unix::fs::symlink(&l.index, &alias).expect("symlink"); + + // Contention through another alias contends on the canonical lock. + fs::write(lock_path_of(&l.index), LOCK_MARKER).expect("hold canonical lock"); + let held = run_paths(&l.plan, &l.corpus, &alias, &l.output); + let refusal = held.primary.expect_err("must contend"); + assert!( + matches!(refusal, ApplyRefusal::ApplicationIndexLocked { .. }), + "alias must derive the canonical lock, got {refusal:?}" + ); + fs::remove_file(lock_path_of(&l.index)).expect("release"); + + // Applying through the alias commits over the REAL file; the alias still + // resolves to the updated index. + let result = run_paths(&l.plan, &l.corpus, &alias, &l.output); + assert!(result.primary.is_ok(), "{:?}", result.primary); + let real = read_value(&l.index); + assert_eq!(real["applications"].as_array().expect("apps").len(), 1); + assert!( + !alias.symlink_metadata().expect("alias meta").is_file(), + "the alias entry remains a symlink — the rename replaced the real file" + ); + let via_alias = read_value(&alias); + assert_eq!(via_alias, real); +} + +#[test] +fn f9_lock_release_failure_is_a_warning_never_the_primary_outcome() { + // (a) after a committed apply: success stays success. + let l = layout("f9a"); + valid_setup(&l); + fault::set("lock:release", io_fail); + let result = run(&l); + fault::clear(); + assert!(result.primary.is_ok(), "committed apply stays applied"); + let warning = result.lock_release_warning.expect("warning attached"); + assert!(warning.lockfile.contains(".lock")); + assert!( + lock_path_of(&l.index).exists(), + "the stale lock remains for §8.2 recovery" + ); + fs::remove_file(lock_path_of(&l.index)).expect("operator recovery"); + + // (b) after a refusal: the original refusal stays primary. + let l = layout("f9b"); + let m = write_corpus( + &l.corpus, + &[Chunk { + id: "a1", + sha: Some("shaA"), + song: Some("song-old"), + }], + ); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-new", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); + fault::set("lock:release", io_fail); + let result = run(&l); + fault::clear(); + let refusal = result.primary.expect_err("refusal preserved"); + assert!( + matches!(refusal, ApplyRefusal::ExistingLabelReplacementNotAuthorized { .. }), + "never masked by the release failure, got {refusal:?}" + ); + assert!(result.lock_release_warning.is_some()); + let _ = fs::remove_file(lock_path_of(&l.index)); +} + +#[test] +fn f10_output_colliding_with_index_artifacts_refuses_before_the_lock() { + // (a) output equal to the lock coordination path. + let l = layout("f10a"); + valid_setup(&l); + let output = lock_path_of(&l.index); + let result = run_paths(&l.plan, &l.corpus, &l.index, &output); + let refusal = result.primary.expect_err("must refuse"); + assert!( + matches!(refusal, ApplyRefusal::OutputCollidesWithIndexArtifacts { .. }), + "got {refusal:?}" + ); + assert!( + !output.exists(), + "the lock must never be created at a declared output path" + ); + + // (b) output equal to the temp coordination path. + let l = layout("f10b"); + valid_setup(&l); + let output = temp_path_of(&l.index); + let result = run_paths(&l.plan, &l.corpus, &l.index, &output); + let refusal = result.primary.expect_err("must refuse"); + assert!(matches!( + refusal, + ApplyRefusal::OutputCollidesWithIndexArtifacts { .. } + )); + + // (c) staging equal to the canonical index file: an index deliberately + // named like a staging sibling. + let l = layout("f10c"); + let m = write_corpus(&l.corpus, &AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + let index = l.td.path.join(".out.apply-staging"); + write_empty_index(&index); + let before = fs::read(&index).expect("index"); + let result = run_paths(&l.plan, &l.corpus, &index, &l.output); + let refusal = result.primary.expect_err("must refuse"); + assert!(matches!( + refusal, + ApplyRefusal::OutputCollidesWithIndexArtifacts { .. } + )); + assert_eq!(fs::read(&index).expect("index"), before, "index untouched"); +} + +#[test] +fn f11_hardlinked_index_refuses_through_either_entry() { + let l = layout("f11"); + valid_setup(&l); + let link = l.td.path.join("link2.json"); + fs::hard_link(&l.index, &link).expect("hard link"); + for entry in [&l.index, &link] { + let result = run_paths(&l.plan, &l.corpus, entry, &l.output); + let refusal = result.primary.expect_err("must refuse"); + match &refusal { + ApplyRefusal::ApplicationIndexHardLinked { nlink, .. } => { + assert_eq!(*nlink, 2); + } + other => panic!("expected ApplicationIndexHardLinked, got {other:?}"), + } + assert!( + !lock_path_of(entry).exists() && !lock_path_of(&l.index).exists(), + "no lock may be created" + ); + } + assert!(!l.output.exists()); +} + +#[test] +fn f12_late_temp_occupant_makes_commit_refuse_and_stays_untouched() { + let l = layout("f12"); + valid_setup(&l); + let temp = temp_path_of(&l.index); + let before = fs::read(&l.index).expect("index"); + { + let temp = temp.clone(); + fault::set("commit:before_temp", move || { + fs::write(&temp, b"foreign occupant").expect("plant occupant"); + Ok(()) + }); + } + let refusal = refuse(&l); + fault::clear(); + assert!( + matches!(refusal, ApplyRefusal::ApplyIoError { .. }), + "pre-commit ApplyIoError, got {refusal:?}" + ); + assert_eq!( + fs::read(&temp).expect("occupant"), + b"foreign occupant", + "the late occupant is left untouched" + ); + assert_eq!(fs::read(&l.index).expect("index"), before, "not applied"); +} + +#[test] +fn f13_output_in_reserved_staging_namespace_refuses() { + let l = layout("f13"); + valid_setup(&l); + let output = l.td.path.join(".x.apply-staging"); + let result = run_paths(&l.plan, &l.corpus, &l.index, &output); + let refusal = result.primary.expect_err("must refuse"); + assert!( + matches!(refusal, ApplyRefusal::OutputNameReserved { .. }), + "got {refusal:?}" + ); + assert!(!output.exists(), "nothing created"); +} + +// ── C. lock / temp coordination ──────────────────────────────────────────────── + +#[test] +fn c11_existing_lockfile_refuses_locked() { + let l = layout("c11"); + valid_setup(&l); + fs::write(lock_path_of(&l.index), LOCK_MARKER).expect("hold lock"); + let before = fs::read(&l.index).expect("index"); + let refusal = refuse(&l); + assert!( + matches!(refusal, ApplyRefusal::ApplicationIndexLocked { .. }), + "got {refusal:?}" + ); + assert!(!l.output.exists()); + assert_eq!(fs::read(&l.index).expect("index"), before); + assert_eq!( + fs::read(lock_path_of(&l.index)).expect("lock"), + LOCK_MARKER.as_bytes(), + "the held lock is untouched" + ); +} + +#[test] +fn c12_stale_lock_blocks_until_validated_recovery() { + let l = layout("c12"); + valid_setup(&l); + fs::write(lock_path_of(&l.index), LOCK_MARKER).expect("stale lock"); + let refusal = refuse(&l); + assert!(matches!(refusal, ApplyRefusal::ApplicationIndexLocked { .. })); + + // §8.2 recovery: only an exact complete marker may be auto-deleted. + let content = fs::read(lock_path_of(&l.index)).expect("read lock"); + assert_eq!(content, LOCK_MARKER.as_bytes(), "validated before deletion"); + fs::remove_file(lock_path_of(&l.index)).expect("recovery"); + let result = run(&l); + assert!(result.primary.is_ok(), "apply succeeds after recovery"); +} + +#[test] +fn c13_real_second_index_at_temp_name_is_never_unlinked() { + let l = layout("c13"); + valid_setup(&l); + // A REAL second index whose filename equals this index's temp name. + let second = temp_path_of(&l.index); + fs::write(&second, EMPTY_INDEX).expect("second index"); + let refusal = refuse(&l); + assert!( + matches!(refusal, ApplyRefusal::ApplicationIndexTempExists { .. }), + "got {refusal:?}" + ); + assert_eq!( + fs::read(&second).expect("second"), + EMPTY_INDEX.as_bytes(), + "the second index is never unlinked or overwritten" + ); + assert!(!lock_path_of(&l.index).exists(), "lock released"); + assert!(!l.output.exists()); +} + +#[test] +fn c14_second_applier_during_live_step12_temp_loses_at_the_lock() { + let l = layout("c14"); + valid_setup(&l); + let second_result: Rc>> = Rc::new(RefCell::new(None)); + { + let second_result = Rc::clone(&second_result); + let plan = l.plan.clone(); + let corpus = l.corpus.clone(); + let index = l.index.clone(); + let output2 = l.td.path.join("out2"); + let temp = temp_path_of(&l.index); + fault::set("commit:rename", move || { + // Inside the first applier's step 12: temp exists, lock held. + assert!(temp.exists(), "the live temp exists in this window"); + let second = run_paths(&plan, &corpus, &index, &output2); + *second_result.borrow_mut() = + Some(second.primary.expect_err("second applier must refuse")); + Ok(()) + }); + } + let first = run(&l); + fault::clear(); + assert!(first.primary.is_ok(), "the live commit completes"); + let second = second_result.borrow_mut().take().expect("second ran"); + assert!( + matches!(second, ApplyRefusal::ApplicationIndexLocked { .. }), + "a non-holder always loses at the lock boundary and can never \ + observe the live temp as ApplicationIndexTempExists, got {second:?}" + ); +} + +#[test] +fn c15_real_second_index_at_lock_name_is_occupied_not_locked() { + let l = layout("c15"); + valid_setup(&l); + // A REAL second index whose filename equals this index's lock name. + let second = lock_path_of(&l.index); + fs::write(&second, EMPTY_INDEX).expect("second index"); + let refusal = refuse(&l); + assert!( + matches!(refusal, ApplyRefusal::ApplicationIndexLockPathOccupied { .. }), + "an index document is not a byte-prefix of the marker, got {refusal:?}" + ); + assert_eq!( + fs::read(&second).expect("second"), + EMPTY_INDEX.as_bytes(), + "the second index is never deleted" + ); +} + +#[test] +fn c16_partial_lock_marker_classifies_locked_and_recovery_is_gated() { + // (a) live window: a concurrent applier observing the lock between + // create_new and marker publication (empty prefix) refuses Locked. + let l = layout("c16a"); + valid_setup(&l); + let second_result: Rc>> = Rc::new(RefCell::new(None)); + { + let second_result = Rc::clone(&second_result); + let plan = l.plan.clone(); + let corpus = l.corpus.clone(); + let index = l.index.clone(); + let output2 = l.td.path.join("out2"); + fault::set("lock:after_create", move || { + let second = run_paths(&plan, &corpus, &index, &output2); + *second_result.borrow_mut() = + Some(second.primary.expect_err("second applier must refuse")); + Ok(()) + }); + } + let first = run(&l); + fault::clear(); + assert!(first.primary.is_ok(), "{:?}", first.primary); + let second = second_result.borrow_mut().take().expect("second ran"); + assert!( + matches!(second, ApplyRefusal::ApplicationIndexLocked { .. }), + "an empty prefix is a Griff lock, never foreign occupancy, got {second:?}" + ); + + // (b) crashed non-empty partial marker: classified Locked; never + // auto-deleted; the operator-proven relocation unblocks. + let l = layout("c16b"); + valid_setup(&l); + let partial = &LOCK_MARKER.as_bytes()[..10]; + fs::write(lock_path_of(&l.index), partial).expect("crash debris"); + let refusal = refuse(&l); + assert!( + matches!(refusal, ApplyRefusal::ApplicationIndexLocked { .. }), + "a non-empty partial prefix is never LockPathOccupied, got {refusal:?}" + ); + assert_eq!( + fs::read(lock_path_of(&l.index)).expect("lock"), + partial, + "the partial marker is not auto-deleted by any apply" + ); + // Operator-proven recovery: non-destructive relocation out of the + // coordination namespace (§8.2), then the apply succeeds. + fs::rename( + lock_path_of(&l.index), + l.td.path.join("quarantined-lock-debris"), + ) + .expect("relocate"); + let result = run(&l); + assert!(result.primary.is_ok(), "unblocked after relocation"); + assert!( + l.td.path.join("quarantined-lock-debris").exists(), + "relocation preserved the evidence" + ); +} + +// ── K. preservation under adversarial bytes ──────────────────────────────────── + +#[test] +fn k10_duplicate_json_keys_in_touched_file_refuse() { + let l = layout("k10"); + valid_setup(&l); + // Raw-bytes fixture: duplicate the "title" member with the same value — + // Value comparison cannot see it; only a duplicate-rejecting pass can. + let path = l.corpus.join("a1.chunk.json"); + let text = fs::read_to_string(&path).expect("read"); + let needle = "\"title\": \"Title a1\","; + assert!(text.contains(needle), "fixture shape changed"); + let dup = format!("\"title\": \"Title a1\",\n {needle}"); + fs::write(&path, text.replacen(needle, &dup, 1)).expect("write"); + + let refusal = refuse(&l); + assert!( + matches!(&refusal, ApplyRefusal::NonCanonicalCorpusFile { path, .. } + if path.contains("a1.chunk.json")), + "got {refusal:?}" + ); + assert!(!l.output.exists(), "nothing published"); +} + +#[test] +fn k11_staged_tree_corruption_aborts_before_publication() { + let l = layout("k11"); + valid_setup(&l); + let original = fs::read(l.corpus.join("a1.chunk.json")).expect("original"); + { + let staged_chunk = staging_path_of(&l.output).join("a1.chunk.json"); + fault::set("stage:before_selfcheck", move || { + // A buggy write left one affected chunk file stale. + fs::write(&staged_chunk, &original).expect("corrupt staged chunk"); + Ok(()) + }); + } + let refusal = refuse(&l); + fault::clear(); + assert!( + matches!(refusal, ApplyRefusal::OutputPreflightInconsistent { .. }), + "the staged tree-agreement re-run must abort, got {refusal:?}" + ); + assert!(!l.output.exists(), "nothing published"); + assert!(!staging_path_of(&l.output).exists(), "staging cleaned"); +} + +// ── R. commit failure can never surface as success ───────────────────────────── + +#[test] +fn r4_commit_failure_is_never_success_and_orphan_blocks_retry() { + let l = layout("r4"); + valid_setup(&l); + let before = fs::read(&l.index).expect("index"); + fault::set("commit:rename", io_fail); + let result = run(&l); + fault::clear(); + let refusal = result.primary.expect_err("commit failure is a refusal"); + assert!(matches!(refusal, ApplyRefusal::ApplyIoError { .. })); + assert_eq!( + fs::read(&l.index).expect("index"), + before, + "no record ⇒ provably not applied" + ); + assert!(l.output.exists(), "published orphan per §8.2"); + + // A retry must refuse OutputAlreadyExists until the operator removes the + // orphan — never stack a second tree, never claim success. + let retry = run(&l); + let refusal = retry.primary.expect_err("retry must refuse"); + assert!( + matches!(refusal, ApplyRefusal::OutputAlreadyExists { .. }), + "got {refusal:?}" + ); + // Mandated recovery: delete the orphan, then the apply succeeds. + fs::remove_dir_all(&l.output).expect("operator removes orphan"); + let after_recovery = run(&l); + assert!(after_recovery.primary.is_ok(), "{:?}", after_recovery.primary); +} From 5cc34b38d1f8dbbd5636702afdbcfa0e22316d25 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 14:50:18 +0000 Subject: [PATCH 06/18] test(song-curation): K10 exercises the residual the derive parse cannot see MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixture-only correction discovered while closing RED-B: serde's derived ChunkMeta deserializer already rejects a duplicated KNOWN struct field at parse time, so that variant refuses even earlier than §10.3 — at step-3 tree agreement, as CorpusTreeDisagreement, before the rewrite path exists. The distinct duplicate-rejecting pass therefore has exactly one residual to guard: a duplicate inside an UNKNOWN member, which the tolerant derive skips wholesale and Value comparison cannot see (last wins). K10 is split accordingly: (i) a duplicate inside an unknown member must refuse NonCanonicalCorpusFile with the duplicate named — proving the distinct pass runs and runs before the round-trip guard; (ii) a duplicated known field is pinned as the even-earlier step-3 refusal. Fail-closed both ways; no duplicate is ever silently laundered. No contract law changes. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/tests/apply_adversarial.rs | 74 +++++++++++++++++++----- 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/song-curation/tests/apply_adversarial.rs b/song-curation/tests/apply_adversarial.rs index 5b1389c..c4967eb 100644 --- a/song-curation/tests/apply_adversarial.rs +++ b/song-curation/tests/apply_adversarial.rs @@ -12,9 +12,7 @@ use common::{ accept, batch_for, event, layout, lock_path_of, read_value, staging_path_of, temp_path_of, write_corpus, write_empty_index, write_plan, Chunk, Layout, EMPTY_INDEX, }; -use griff_song_curation::apply::{ - apply, fault, ApplyPaths, ApplyRefusal, ApplyRun, LOCK_MARKER, -}; +use griff_song_curation::apply::{apply, fault, ApplyPaths, ApplyRefusal, ApplyRun, LOCK_MARKER}; use std::cell::RefCell; use std::fs; use std::io; @@ -220,7 +218,10 @@ fn f7_path_like_batch_id_influences_no_filesystem_path() { l.td.path.join("griff-f7-escape"), l.td.path.parent().expect("parent").join("griff-f7-escape"), ] { - assert!(!escape.exists(), "no path derived from batch_id: {escape:?}"); + assert!( + !escape.exists(), + "no path derived from batch_id: {escape:?}" + ); } } @@ -295,7 +296,10 @@ fn f9_lock_release_failure_is_a_warning_never_the_primary_outcome() { fault::clear(); let refusal = result.primary.expect_err("refusal preserved"); assert!( - matches!(refusal, ApplyRefusal::ExistingLabelReplacementNotAuthorized { .. }), + matches!( + refusal, + ApplyRefusal::ExistingLabelReplacementNotAuthorized { .. } + ), "never masked by the release failure, got {refusal:?}" ); assert!(result.lock_release_warning.is_some()); @@ -311,7 +315,10 @@ fn f10_output_colliding_with_index_artifacts_refuses_before_the_lock() { let result = run_paths(&l.plan, &l.corpus, &l.index, &output); let refusal = result.primary.expect_err("must refuse"); assert!( - matches!(refusal, ApplyRefusal::OutputCollidesWithIndexArtifacts { .. }), + matches!( + refusal, + ApplyRefusal::OutputCollidesWithIndexArtifacts { .. } + ), "got {refusal:?}" ); assert!( @@ -445,7 +452,10 @@ fn c12_stale_lock_blocks_until_validated_recovery() { valid_setup(&l); fs::write(lock_path_of(&l.index), LOCK_MARKER).expect("stale lock"); let refusal = refuse(&l); - assert!(matches!(refusal, ApplyRefusal::ApplicationIndexLocked { .. })); + assert!(matches!( + refusal, + ApplyRefusal::ApplicationIndexLocked { .. } + )); // §8.2 recovery: only an exact complete marker may be auto-deleted. let content = fs::read(lock_path_of(&l.index)).expect("read lock"); @@ -517,7 +527,10 @@ fn c15_real_second_index_at_lock_name_is_occupied_not_locked() { fs::write(&second, EMPTY_INDEX).expect("second index"); let refusal = refuse(&l); assert!( - matches!(refusal, ApplyRefusal::ApplicationIndexLockPathOccupied { .. }), + matches!( + refusal, + ApplyRefusal::ApplicationIndexLockPathOccupied { .. } + ), "an index document is not a byte-prefix of the marker, got {refusal:?}" ); assert_eq!( @@ -591,21 +604,48 @@ fn c16_partial_lock_marker_classifies_locked_and_recovery_is_gated() { #[test] fn k10_duplicate_json_keys_in_touched_file_refuse() { - let l = layout("k10"); + // (i) A duplicate key the tolerant derive parse cannot see: inside an + // unknown member, which serde's derived deserializer skips wholesale. + // Value comparison cannot prove duplicates absent either (last wins), so + // only the distinct §10.3 duplicate-rejecting pass can catch it — and it + // must run before the round-trip guard, so the refusal names the + // duplicate, not the unknown member. + let l = layout("k10a"); + valid_setup(&l); + let path = l.corpus.join("a1.chunk.json"); + let text = fs::read_to_string(&path).expect("read"); + let with_rogue = text.replacen("{\n", "{\n \"rogue\": {\"k\": 1, \"k\": 1},\n", 1); + assert_ne!(with_rogue, text, "fixture shape changed"); + fs::write(&path, with_rogue).expect("write"); + let refusal = refuse(&l); + match &refusal { + ApplyRefusal::NonCanonicalCorpusFile { path, detail } => { + assert!(path.contains("a1.chunk.json")); + assert!( + detail.contains("duplicate"), + "the duplicate-rejecting pass must fire first: {detail}" + ); + } + other => panic!("expected NonCanonicalCorpusFile, got {other:?}"), + } + assert!(!l.output.exists(), "nothing published"); + + // (ii) A duplicated KNOWN field is refused even earlier: the derived + // parse itself rejects it during step-3 tree agreement, so the file + // never reaches the rewrite path at all. Fail-closed both ways — no + // duplicate is ever silently laundered. + let l = layout("k10b"); valid_setup(&l); - // Raw-bytes fixture: duplicate the "title" member with the same value — - // Value comparison cannot see it; only a duplicate-rejecting pass can. let path = l.corpus.join("a1.chunk.json"); let text = fs::read_to_string(&path).expect("read"); let needle = "\"title\": \"Title a1\","; assert!(text.contains(needle), "fixture shape changed"); let dup = format!("\"title\": \"Title a1\",\n {needle}"); fs::write(&path, text.replacen(needle, &dup, 1)).expect("write"); - let refusal = refuse(&l); assert!( - matches!(&refusal, ApplyRefusal::NonCanonicalCorpusFile { path, .. } - if path.contains("a1.chunk.json")), + matches!(&refusal, ApplyRefusal::CorpusTreeDisagreement { detail } + if detail.contains("duplicate field")), "got {refusal:?}" ); assert!(!l.output.exists(), "nothing published"); @@ -664,5 +704,9 @@ fn r4_commit_failure_is_never_success_and_orphan_blocks_retry() { // Mandated recovery: delete the orphan, then the apply succeeds. fs::remove_dir_all(&l.output).expect("operator removes orphan"); let after_recovery = run(&l); - assert!(after_recovery.primary.is_ok(), "{:?}", after_recovery.primary); + assert!( + after_recovery.primary.is_ok(), + "{:?}", + after_recovery.primary + ); } From c9489e16eaa26ce85fd189752ed180d7e3e09cec Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 14:50:18 +0000 Subject: [PATCH 07/18] feat(song-curation): implement transactional Apply filesystem protocol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GREEN for the 12 RED-B cases (plus the 8 pinned characterizations), and exactly the accepted §6 step-1 order and §8 semantics: Step 1, in order: index resolves to an existing regular file (before canonicalization and the lock); nlink == 1 or ApplicationIndexHardLinked; coordination names derived from the canonical index only; output/staging resolved as canonical parent + final component; collisions with the canonical index / lock / temp paths refused before lock acquisition (OutputCollidesWithIndexArtifacts); the reserved ..apply-staging namespace refused for outputs (OutputNameReserved); output/staging pre-existence (OutputAlreadyExists); containment against the resolved corpus root both ways (OutputWouldModifyInput); canonical index equals-or- inside any tree root (ApplicationIndexInsideTree); the distinct curated- manifest hard guard; then lock acquisition; then — only under the held lock — the temp inspection (ApplicationIndexTempExists), so a live writer's step-12 temp is unreachable for a non-holder. Lock: create_new + ownership marker; contention classification is prefix-closed (empty, partial, or complete marker → ApplicationIndexLocked; anything else → ApplicationIndexLockPathOccupied) so live ownership is never misclassified and a real second index named .foo.lock is never deletable by recovery. Commit: the temp write is an atomic no-clobber create_new — the step-1 absence check is a fail-fast courtesy, not the safety argument; a late occupant yields a pre-commit ApplyIoError and is left untouched. Step 10: the §4.2 tree-agreement law is re-run over the staged tree (staged root manifest ↔ staged chunk files) before fingerprinting, the single preflight, and the report, so a write that missed one affected chunk file can never publish (K11). §10.3: duplicate-key rejection as a distinct native serde visitor pass (NoDupKeys) over touched files, run before the round-trip guard — Value comparison cannot prove duplicates absent, and the derive parse already refuses duplicated known fields at step 3. Publication stays plain rename (honestly non-no-clobber, §8.1): compliant overlap is impossible via the reserved namespace + atomic staging create_dir; the external empty-directory residual remains exactly as the contract scopes it. Evidence: full crate suite green — 45 (frozen Slice 1) + 27 + 16 + 20; clippy --all-targets clean under deny(all); fmt --check clean. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/src/apply.rs | 255 ++++++++++++++++++++++++++++++++++--- 1 file changed, 237 insertions(+), 18 deletions(-) diff --git a/song-curation/src/apply.rs b/song-curation/src/apply.rs index 9ca5abf..21b7c98 100644 --- a/song-curation/src/apply.rs +++ b/song-curation/src/apply.rs @@ -281,6 +281,8 @@ struct Ctx { canonical_index: PathBuf, lock_path: PathBuf, temp_path: PathBuf, + /// The resolved output path (canonical parent + final component). + output: PathBuf, staging: PathBuf, } @@ -313,17 +315,126 @@ fn preflight(paths: &ApplyPaths) -> Result { detail: format!("index {} is not a regular file", canonical_index.display()), }); } + // The canonical index file must have a link count of exactly one: + // hardlink aliases defeat path-derived locking and split under + // commit-by-rename (§8.1). + let nlink = std::os::unix::fs::MetadataExt::nlink(&meta); + if nlink != 1 { + return Err(ApplyRefusal::ApplicationIndexHardLinked { + path: canonical_index.display().to_string(), + nlink, + }); + } let lock_path = coordination_path(&canonical_index, "lock"); let temp_path = coordination_path(&canonical_index, "tmp"); - let staging = staging_path(&paths.output); + + // Resolve the not-yet-existing output and staging paths as canonical + // parent + final component (the migrate discipline). + let output = resolve_fresh(&paths.output)?; + let staging = staging_path(&output); + + // Output/staging must not equal any coordination artifact — checked + // BEFORE lock acquisition, so the lock is never created at a declared + // output path and step 12 can never collide with the output. + for target in [&output, &staging] { + for (artifact, path) in [ + ("index", &canonical_index), + ("lock", &lock_path), + ("temp", &temp_path), + ] { + if target == path { + return Err(ApplyRefusal::OutputCollidesWithIndexArtifacts { + path: target.display().to_string(), + artifact: artifact.to_owned(), + }); + } + } + } + + // The reserved staging namespace: no compliant output may lie in it — + // this is what makes compliant-applier publication overlap impossible. + let output_name = output + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_default(); + if output_name.starts_with('.') && output_name.ends_with(".apply-staging") { + return Err(ApplyRefusal::OutputNameReserved { + path: output.display().to_string(), + }); + } + + // Pre-existence refusals for output and staging. + for target in [&output, &staging] { + if target.symlink_metadata().is_ok() { + return Err(ApplyRefusal::OutputAlreadyExists { + path: target.display().to_string(), + }); + } + } + + // Containment laws against the resolved input corpus root. + let corpus = paths + .corpus + .canonicalize() + .map_err(|e| io_refusal(&paths.corpus, "canonicalize corpus", &e))?; + for target in [&output, &staging] { + if target == &corpus || target.starts_with(&corpus) || corpus.starts_with(target) { + return Err(ApplyRefusal::OutputWouldModifyInput { + detail: format!( + "output/staging {} equals, contains, or is contained by the input corpus root {}", + target.display(), + corpus.display() + ), + }); + } + } + + // The canonical index must not equal or lie inside any tree root. + for root in [&corpus, &output, &staging] { + if &canonical_index == root || canonical_index.starts_with(root) { + return Err(ApplyRefusal::ApplicationIndexInsideTree { + path: canonical_index.display().to_string(), + }); + } + } + + // Distinct curated-manifest guard (§4.3): structurally unreachable under + // the fixed v1 path, kept as a hard guard. + let curated_target = output.join(CURATED_MANIFEST_RELPATH); + for root in [&corpus, &output] { + if curated_target == root.join("manifest.json") { + return Err(ApplyRefusal::CuratedManifestPathNotDistinct { + path: curated_target.display().to_string(), + }); + } + } + Ok(Ctx { canonical_index, lock_path, temp_path, + output, staging, }) } +/// Resolve a not-yet-existing path as canonical parent + final component. +fn resolve_fresh(path: &Path) -> Result { + let parent = match path.parent() { + Some(p) if !p.as_os_str().is_empty() => p, + _ => Path::new("."), + }; + let parent = parent + .canonicalize() + .map_err(|e| io_refusal(parent, "canonicalize parent", &e))?; + let name = path.file_name().ok_or_else(|| ApplyRefusal::ApplyIoError { + path: path.display().to_string(), + op: "resolve".to_owned(), + detail: "path has no final component".to_owned(), + })?; + Ok(parent.join(name)) +} + /// `..` next to the canonical index file (§8.1). fn coordination_path(canonical_index: &Path, ext: &str) -> PathBuf { let name = canonical_index @@ -356,7 +467,19 @@ fn acquire_lock(ctx: &Ctx) -> Result<(), ApplyRefusal> { { Ok(file) => file, Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { - return Err(ApplyRefusal::ApplicationIndexLocked { + // Contention classification is prefix-closed (§8.1): any byte + // prefix of the canonical marker — empty, partial, or complete — + // is a Griff lock (a live writer can only ever be observed in a + // prefix state); anything else is an unproven occupant that no + // mandated recovery may delete or relocate. + let content = fs::read(&ctx.lock_path) + .map_err(|e| io_refusal(&ctx.lock_path, "lock read", &e))?; + if LOCK_MARKER.as_bytes().starts_with(content.as_slice()) { + return Err(ApplyRefusal::ApplicationIndexLocked { + path: ctx.lock_path.display().to_string(), + }); + } + return Err(ApplyRefusal::ApplicationIndexLockPathOccupied { path: ctx.lock_path.display().to_string(), }); } @@ -401,6 +524,19 @@ fn release_lock(lock_path: &Path) -> Option { #[allow(clippy::too_many_lines)] fn locked_apply(paths: &ApplyPaths, ctx: &Ctx) -> Result { + // Step 1 (end), only now under the held lock: inspect the temp + // coordination path. The ordering is load-bearing — during a live + // writer's step 12 the temp legitimately exists while the lock is held, + // so a non-holder always loses at the lock boundary first and can never + // misclassify a live commit's temp. Nothing pre-existing there is ever + // unlinked: the lock proves no live writer, not ownership of those + // bytes (§8.1); recovery is explicit operator inspection (§8.2). + if ctx.temp_path.symlink_metadata().is_ok() { + return Err(ApplyRefusal::ApplicationIndexTempExists { + path: ctx.temp_path.display().to_string(), + }); + } + // Step 2: strict artifact parsing, before any filesystem mutation. let plan_text = fs::read_to_string(&paths.plan).map_err(|e| ApplyRefusal::MalformedPlanArtifact { @@ -460,7 +596,6 @@ fn locked_apply(paths: &ApplyPaths, ctx: &Ctx) -> Result, -) -> Result { +fn stage_publish_commit(ctx: &Ctx, input: &StageInput<'_>) -> Result { let StageInput { plan, index, @@ -883,12 +1014,12 @@ fn stage_publish_commit( // Step 11: publish the snapshot with one rename. if let Err(e) = fault::hit("publish:rename") { - let refusal = io_refusal(&paths.output, "publish rename", &e); + let refusal = io_refusal(&ctx.output, "publish rename", &e); cleanup_staging(&ctx.staging); return Err(refusal); } - if let Err(e) = fs::rename(&ctx.staging, &paths.output) { - let refusal = io_refusal(&paths.output, "publish rename", &e); + if let Err(e) = fs::rename(&ctx.staging, &ctx.output) { + let refusal = io_refusal(&ctx.output, "publish rename", &e); cleanup_staging(&ctx.staging); return Err(refusal); } @@ -909,8 +1040,15 @@ fn stage_publish_commit( detail: e.to_string(), })?; fault::hit("commit:before_temp").map_err(|e| io_refusal(&ctx.temp_path, "temp create", &e))?; - let mut temp = fs::File::create(&ctx.temp_path) - .map_err(|e| io_refusal(&ctx.temp_path, "temp create", &e))?; + // Atomic no-clobber creation (std's answer to exactly this TOCTOU): the + // step-1 under-lock absence check was a fail-fast courtesy, not the + // safety argument. AlreadyExists here is a pre-commit ApplyIoError and + // the late-appearing occupant is left untouched (§8.1). + let mut temp = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&ctx.temp_path) + .map_err(|e| io_refusal(&ctx.temp_path, "temp create_new", &e))?; fault::hit("commit:temp_write").map_err(|e| io_refusal(&ctx.temp_path, "temp write", &e))?; temp.write_all(index_json.as_bytes()) .map_err(|e| io_refusal(&ctx.temp_path, "temp write", &e))?; @@ -1013,6 +1151,15 @@ fn write_touched( modified_manifest: &CorpusManifest, ) -> Result<(), ApplyRefusal> { let text = fs::read_to_string(abs).map_err(|e| io_refusal(abs, "read", &e))?; + // Duplicate-key rejection is a DISTINCT pass (§10.3): serde_json::Value + // is already a map that silently keeps the last duplicate, so no + // comparison of Values can ever prove duplicates were absent. + if let Err(detail) = reject_duplicate_keys(&text) { + return Err(ApplyRefusal::NonCanonicalCorpusFile { + path: rel.to_owned(), + detail, + }); + } let raw: Value = serde_json::from_str(&text).map_err(|e| tree_disagreement(format!("{rel}: {e}")))?; let rendered = if rel == "manifest.json" { @@ -1074,11 +1221,14 @@ fn staged_selfcheck_and_report( ) -> Result { let inconsistent = |detail: String| ApplyRefusal::OutputPreflightInconsistent { detail }; - // Re-read the staged root manifest from bytes. - let staged_manifest_text = fs::read_to_string(ctx.staging.join("manifest.json")) - .map_err(|e| inconsistent(format!("staged manifest.json unreadable: {e}")))?; - let staged_manifest: CorpusManifest = serde_json::from_str(&staged_manifest_text) - .map_err(|e| inconsistent(format!("staged manifest.json unparseable: {e}")))?; + // Re-read the staged tree from bytes and RE-RUN the §4.2 tree-agreement + // law over it (staged root manifest ↔ staged chunk files — the same + // multiset check as step 3, not a second preflight), so a write that + // updated the manifests but missed an affected chunk file, or the + // reverse, can never publish. + let staged_snapshot = load_snapshot(&ctx.staging) + .map_err(|e| inconsistent(format!("staged tree agreement failed: {e:?}")))?; + let staged_manifest = staged_snapshot.manifest; let expected_value = serde_json::to_value(expected_manifest).unwrap_or(Value::Null); let staged_value = serde_json::to_value(&staged_manifest).unwrap_or(Value::Null); if expected_value != staged_value { @@ -1187,3 +1337,72 @@ fn staged_selfcheck_and_report( Ok(report) } + +// ── duplicate-key rejection (§10.3) ──────────────────────────────────────────── + +/// A deserialization target whose only job is to refuse a repeated object key +/// at any depth. Native `serde` visitor; no new dependency. +struct NoDupKeys; + +impl<'de> serde::Deserialize<'de> for NoDupKeys { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct V; + impl<'de> serde::de::Visitor<'de> for V { + type Value = NoDupKeys; + fn expecting(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("any JSON value without duplicate object keys") + } + fn visit_map(self, mut map: A) -> Result + where + A: serde::de::MapAccess<'de>, + { + let mut seen: BTreeSet = BTreeSet::new(); + while let Some(key) = map.next_key::()? { + if !seen.insert(key.clone()) { + return Err(serde::de::Error::custom(format!( + "duplicate object key {key:?}" + ))); + } + let NoDupKeys = map.next_value::()?; + } + Ok(NoDupKeys) + } + fn visit_seq(self, mut seq: A) -> Result + where + A: serde::de::SeqAccess<'de>, + { + while seq.next_element::()?.is_some() {} + Ok(NoDupKeys) + } + fn visit_bool(self, _: bool) -> Result { + Ok(NoDupKeys) + } + fn visit_i64(self, _: i64) -> Result { + Ok(NoDupKeys) + } + fn visit_u64(self, _: u64) -> Result { + Ok(NoDupKeys) + } + fn visit_f64(self, _: f64) -> Result { + Ok(NoDupKeys) + } + fn visit_str(self, _: &str) -> Result { + Ok(NoDupKeys) + } + fn visit_unit(self) -> Result { + Ok(NoDupKeys) + } + } + deserializer.deserialize_any(V) + } +} + +/// Prove `text` contains no repeated object key at any depth. +fn reject_duplicate_keys(text: &str) -> Result<(), String> { + serde_json::from_str::(text) + .map(|NoDupKeys| ()) + .map_err(|e| e.to_string()) +} From 5bb78fbceafe963256572fcfc78b1ecf17e7ffa3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 14:53:25 +0000 Subject: [PATCH 08/18] test(song-curation): adversarial self-review witnesses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase-8 falsification pass over the implementation. Fourteen mutation targets were probed against the 63-case matrix; four survived with no killing test, and each gets a dedicated witness here (characterization — green before commit, no new public API): 1. removing the §5.1 index schema-identity check → witness_unsupported_index_schema_refuses; 2. letting prev == null pass §7.2 relation (1) against a non-empty head → witness_null_prev_digest_against_nonempty_head_refuses; 3. dropping deny_unknown_fields from the index RECORD type → witness_index_record_foreign_field_refuses; 4. dropping deny_unknown_fields from the report type → witness_published_report_is_strict_on_the_wire. Probed-and-already-killed (killer in parentheses): already-applied precedence over the chain (C3); temp inspection under the lock (C14); prefix classification both ways (C15/C16); no auto-deletion of stale locks (C11/C12); create_new temp no-clobber (F12); hardlink refusal (F11); reserved output namespace (F13); coordination collisions (F10); commit-point result semantics (R4, F4c/d, F9a); raw-copy preservation (K2); duplicate-key guard (K10); staged tree re-check (K11); report/index digest binding (R1/R2). Survived-by-unreachability, documented rather than tested: §7.2 relation (3) (entailed by relation (2) plus the step-5 fingerprint proof — the contract itself notes it is stated for attribution); the CuratedManifestPathNotDistinct hard guard (structurally unreachable under the fixed v1 path, per §12); corpus-inside-output containment (an output that contains an existing corpus necessarily exists itself and refuses OutputAlreadyExists first, per the §6 order). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/tests/apply_witnesses.rs | 149 +++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 song-curation/tests/apply_witnesses.rs diff --git a/song-curation/tests/apply_witnesses.rs b/song-curation/tests/apply_witnesses.rs new file mode 100644 index 0000000..e83c5af --- /dev/null +++ b/song-curation/tests/apply_witnesses.rs @@ -0,0 +1,149 @@ +//! Adversarial self-review witnesses (implementation Phase 8). +//! +//! Each test kills one specific mutation that the preregistered §14 matrix +//! alone would let survive. They characterize behaviour the implementation +//! already provides (no new public API; green before commit), pinning it +//! against exactly the mutations named in their doc comments. + +mod common; + +use common::{ + accept, batch_for, event, layout, read_manifest, write_corpus, write_empty_index, write_plan, + Chunk, Layout, +}; +use griff_song_curation::apply::{ + apply, ApplicationReport, ApplyPaths, ApplyRefusal, ApplyRun, REPORT_RELPATH, +}; +use serde_json::json; +use std::fs; + +fn run(l: &Layout) -> ApplyRun { + apply(&ApplyPaths { + plan: l.plan.clone(), + corpus: l.corpus.clone(), + index: l.index.clone(), + output: l.output.clone(), + }) +} + +const AB: [Chunk<'static>; 2] = [ + Chunk { + id: "a1", + sha: Some("shaA"), + song: None, + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: None, + }, +]; + +fn valid_setup(l: &Layout) { + let m = write_corpus(&l.corpus, &AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); +} + +/// Kills: removing the §5.1 schema-identity check from index validation. +#[test] +fn witness_unsupported_index_schema_refuses() { + let l = layout("w-schema"); + valid_setup(&l); + fs::write( + &l.index, + "{\"schema\":\"song-curation.applications.v99\",\"applications\":[]}", + ) + .expect("write index"); + let refusal = run(&l).primary.expect_err("must refuse"); + assert!( + matches!(&refusal, ApplyRefusal::UnsupportedApplicationIndexSchema { schema } + if schema == "song-curation.applications.v99"), + "got {refusal:?}" + ); +} + +/// Kills: letting a `null` previous_application_report_digest pass §7.2 +/// relation (1) against a non-empty index head. +#[test] +fn witness_null_prev_digest_against_nonempty_head_refuses() { + let l = layout("w-nullprev"); + valid_setup(&l); + let receipt = run(&l).primary.expect("initial apply"); + let corpus2 = l.output.clone(); + let m2 = read_manifest(&corpus2); + // batch2 correctly fingerprinted against the head output, but prev=None. + let b2 = batch_for( + &m2, + "batch2", + None, + vec![event("ev0", 0, accept("g2", &["shaB"], "song-000002", &[]))], + ); + let plan2 = l.td.path.join("plan2.json"); + write_plan(&corpus2, b2, &plan2); + let result = apply(&ApplyPaths { + plan: plan2, + corpus: corpus2, + index: l.index.clone(), + output: l.td.path.join("out2"), + }); + let refusal = result.primary.expect_err("must refuse"); + match &refusal { + ApplyRefusal::ApplicationChainMismatch { + relation, + expected, + actual, + } => { + assert!(relation.contains("previous_application_report_digest")); + assert_eq!(expected, &receipt.report.report_digest); + assert_eq!(actual, "null"); + } + other => panic!("expected ApplicationChainMismatch, got {other:?}"), + } +} + +/// Kills: dropping `deny_unknown_fields` from the index RECORD type — a +/// foreign field nested inside a record must refuse, not launder. +#[test] +fn witness_index_record_foreign_field_refuses() { + let l = layout("w-recfield"); + valid_setup(&l); + fs::write( + &l.index, + r#"{"schema":"song-curation.applications.v1","applications":[ + {"batch_id":"b1","report_digest":"r","input_corpus_fingerprint":"f1", + "output_corpus_fingerprint":"f2","rogue":true}]}"#, + ) + .expect("write index"); + let refusal = run(&l).primary.expect_err("must refuse"); + assert!( + matches!(refusal, ApplyRefusal::MalformedApplicationIndex { .. }), + "got {refusal:?}" + ); +} + +/// Kills: dropping `deny_unknown_fields` from the report type — the +/// published artifact must stay strict for every future reader. +#[test] +fn witness_published_report_is_strict_on_the_wire() { + let l = layout("w-report"); + valid_setup(&l); + run(&l).primary.expect("apply"); + let text = fs::read_to_string(l.output.join(REPORT_RELPATH)).expect("read report"); + let mut value: serde_json::Value = serde_json::from_str(&text).expect("parse"); + value + .as_object_mut() + .expect("object") + .insert("rogue".to_owned(), json!(1)); + let laundered = serde_json::from_value::(value); + assert!( + laundered.is_err(), + "a foreign field in the report must be rejected" + ); +} From bbc79288f66e8633a3f3887eeface6632e04ac55 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 15:08:07 +0000 Subject: [PATCH 09/18] docs(song-curation): Slice 2 implementation evidence and README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docs-only closure candidate for the Slice-2 implementation: - docs/audit/2026-08-slice2-apply-implementation.md — the implementation evidence: implemented API; the full unsquashed RED→GREEN commit sequence with kinds; the 63/63 §14 matrix (case → test → RED commit → result, characterizations marked); refusal coverage (23/24 new typed refusals exercised + the structurally unreachable hard guard documented, 11/11 Apply-reachable Slice-1 refusals exercised, 3 ledger-side members proven intentionally unreachable); adversarial/fault-injection results; the three implementation-time findings; and the validation matrix (isolated crate 112/0, workspace 1535/0, clippy -D warnings, fmt, doc, MSRV 1.92). - song-curation/README.md — truthful lifecycle update: title covers Slices 1–2, a Slice-2 section describes the apply module, and the out-of-scope list now names what actually remains gated (Slice 3, the pilot, real-corpus labeling). Slice-1 sections untouched. Explicitly preserved distinctions: contract acceptance (recorded at bad7b44 against 47e734c) is NOT implementation acceptance — this branch is a review candidate and marks nothing ACCEPTED/CLOSED/FROZEN; Slice 3, the controlled pilot, and any real-/full-corpus labeling remain BLOCKED. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- .../2026-08-slice2-apply-implementation.md | 232 ++++++++++++++++++ song-curation/README.md | 43 +++- 2 files changed, 265 insertions(+), 10 deletions(-) create mode 100644 docs/audit/2026-08-slice2-apply-implementation.md diff --git a/docs/audit/2026-08-slice2-apply-implementation.md b/docs/audit/2026-08-slice2-apply-implementation.md new file mode 100644 index 0000000..30876f3 --- /dev/null +++ b/docs/audit/2026-08-slice2-apply-implementation.md @@ -0,0 +1,232 @@ +# ADR-0033 Slice 2 — transactional Apply: implementation evidence + +Status: **implementation review candidate — awaiting independent +implementation acceptance**. The contract itself was independently accepted +(normative reviewed artifact +`47e734cfbf1a6bd90c1bd2a035cdc68692378e96`, acceptance recorded in +[`../decisions.log.md`](../decisions.log.md) @ `bad7b44`); this document is +evidence for the *implementation*, which is a separate acceptance under the +contract's §16. Nothing here marks anything ACCEPTED / CLOSED / FROZEN. + +Slice 3 (suggestions), the controlled pilot, and any real- / production- / +full-corpus labeling remain **BLOCKED** behind their own gates (ADR-0033 +Decision 10). No real corpus file or label was read or modified by this +work: every test runs over synthetic fixtures in process-unique temp +directories. + +## Implemented API + +New public module `griff_song_curation::apply` in the isolated +non-workspace `song-curation/` crate (posture unchanged): + +- `apply(&ApplyPaths) -> ApplyRun` — the §6 12-step transactional Apply + over the serialized artifact boundary (plan file + corpus tree + index + file → published output tree). `ApplyRun` is the §8.2 observable result + shape: exactly one primary outcome (`AppliedReceipt` with the published + `ApplicationReport`, or one typed `ApplyRefusal`) plus an optional + orthogonal `LockReleaseWarning`. +- Wire contracts (§5, all `deny_unknown_fields`): `ApplicationIndex` / + `ApplicationRecord` (`song-curation.applications.v1`), + `ApplicationReport` / `Coverage` / `HoldoutRefusalRecord` + (`song-curation.apply-report.v1`), `report_digest()` over the Slice-1 + shared canonical encoding (§5.4), and the constants + `APPLICATIONS_SCHEMA`, `REPORT_SCHEMA`, `LOCK_MARKER`, + `CURATED_MANIFEST_RELPATH`, `REPORT_RELPATH`, `RESERVED_DIR`. +- `ApplyRefusal` — the closed §12 surface: the 24 new typed refusals, plus + `PlanVerification { refusals: Vec }` as *transport* for + the Slice-1 refusals reused verbatim through step 5 (an encoding + artifact, not a new refusal kind). +- `apply::fault` — the deterministic fault-injection registry + (thread-local, one-shot named hooks; inert in any run that registers + none). Test harness, not contract surface. + +Frozen Slice 1: public API, semantics, and all 45 tests untouched and +green. The §9-permitted internal accommodation is exactly one: the shared +`replay` primitive now records, per source, the acting `event_id` +(`BatchEffect`), consumed by the §7.4 authority law; `canonical_json` was +made `pub(crate)` so `report_digest` uses the one shared encoding instead +of a near-copy. Both are crate-internal; no observable Slice-1 behaviour +changed. + +## Commit sequence (RED→GREEN per commit, unsquashed) + +| # | Commit | Kind | Content | +|---|---|---|---| +| 1 | `dfa7e06` | RED | 43 preregistered cases (A1–A8, C1–C10, L1–L9, K1–K9, K12, R1–R3, R5, F3, F5) + the fixture/builder module. Evidence in the commit message: both test binaries fail with `E0432: unresolved import griff_song_curation::apply` — as far as the absent API allows. | +| 2 | `4d311f4` | TEST-FIX | A8 drift fixtures clear the corpus tree before rewriting it (stale chunk files made the honest step-3 refusal fire before the preregistered step-5 one). | +| 3 | `f8c9f1c` | GREEN | Apply core: full §5 wire contracts, §6 ordering for the implemented checks, §7 laws, §8 happy path, §10 preservation with the round-trip guard, §11 single preflight. Adversarial hardening deliberately absent (kept RED). | +| 4 | `6ce1a69` | FIXTURE | `apply::fault` registry + eight inert named points (§14-exempt fixture work). | +| 5 | `bd2a339` | RED | 20 adversarial cases; evidence in the commit message: 12 FAILED (F1, F2, F6, F10–F13, C13, C15, K10, K11, R4), 8 declared as passing characterizations (F4, F7–F9, C11, C12, C14, C16). | +| 6 | `5cc34b3` | TEST-FIX | K10 split: serde's derived parse already refuses a duplicated *known* field at step 3, so the distinct §10.3 pass is exercised on its actual residual — a duplicate inside an unknown member (finding 1 below). | +| 7 | `c9489e1` | GREEN | The §8 filesystem protocol: full step-1 order, prefix-closed lock classification, under-lock temp inspection, `create_new` temp, reserved namespace, collisions, hardlink refusal, staged tree re-agreement, duplicate-key pass. | +| 8 | `5bb78fb` | WITNESS | Phase-8 falsification pass: 4 surviving mutations got dedicated killing witnesses (index schema check, null-prev against a non-empty head, record-level and report-level wire strictness). | + +## §14 matrix — 63/63 + +Test names are the case ids; files: `apply_core.rs` (A, C1–C10, L), +`apply_outputs.rs` (K1–K9, K12, R1–R3, R5, F3, F5), `apply_adversarial.rs` +(F1, F2, F4, F6–F13, C11–C16, K10, K11, R4). "RED commit" is where the +case was preregistered; ⊙ marks the eight cases that were declared passing +characterizations at preregistration time (pinning behaviour the core + +fixtures already provided). + +| Case | Test | RED commit | Result | +|---|---|---|---| +| A1 | `a1_valid_serialized_plan_applies` | `dfa7e06` | green | +| A2 | `a2_plan_with_foreign_field_refuses` | `dfa7e06` | green | +| A3 | `a3_corrupted_plan_digest_refuses` | `dfa7e06` | green | +| A4 | `a4_corrupted_decisions_digest_refuses` | `dfa7e06` | green | +| A5 | `a5_corpus_drift_refuses` | `dfa7e06` | green | +| A6 | `a6_forged_projection_with_self_consistent_digests_refuses` | `dfa7e06` | green | +| A7 | `a7_invalid_embedded_batch_short_circuits_before_digests` | `dfa7e06` | green | +| A8 | `a8_reachable_slice1_refusals_surface_through_step5` | `dfa7e06` (+`4d311f4`) | green | +| C1 | `c1_valid_initial_application` | `dfa7e06` | green | +| C2 | `c2_valid_second_application_chained_to_first` | `dfa7e06` | green | +| C3 | `c3_duplicate_batch_id_refuses_as_already_applied_not_chain` | `dfa7e06` | green | +| C4 | `c4_wrong_previous_report_digest_refuses` | `dfa7e06` | green | +| C5 | `c5_wrong_chained_corpus_fingerprint_refuses` | `dfa7e06` | green | +| C6 | `c6_fingerprint_neutral_batch_applies_then_refuses_by_id` | `dfa7e06` | green | +| C7 | `c7_missing_index_and_foreign_field_refuse` | `dfa7e06` | green | +| C8 | `c8_duplicate_internal_batch_id_refuses` | `dfa7e06` | green | +| C9 | `c9_index_with_broken_internal_chain_refuses` | `dfa7e06` | green | +| C10 | `c10_initial_batch_on_independent_copy_with_own_index_applies` | `dfa7e06` | green | +| C11 | `c11_existing_lockfile_refuses_locked` | `bd2a339` ⊙ | green | +| C12 | `c12_stale_lock_blocks_until_validated_recovery` | `bd2a339` ⊙ | green | +| C13 | `c13_real_second_index_at_temp_name_is_never_unlinked` | `bd2a339` | green | +| C14 | `c14_second_applier_during_live_step12_temp_loses_at_the_lock` | `bd2a339` ⊙ | green | +| C15 | `c15_real_second_index_at_lock_name_is_occupied_not_locked` | `bd2a339` | green | +| C16 | `c16_partial_lock_marker_classifies_locked_and_recovery_is_gated` | `bd2a339` ⊙ | green | +| L1 | `l1_assign_unlabelled_source_updates_every_chunk` | `dfa7e06` | green | +| L2 | `l2_already_correct_label_counts_unchanged` | `dfa7e06` | green | +| L3 | `l3_authorized_correct_replaces_label` | `dfa7e06` | green | +| L4 | `l4_authorized_merge_replaces_labels` | `dfa7e06` | green | +| L5 | `l5_authorized_split_replaces_labels` | `dfa7e06` | green | +| L6 | `l6_unauthorized_replacement_refuses` | `dfa7e06` | green | +| L7 | `l7_on_disk_label_drift_refuses_as_fingerprint_mismatch` | `dfa7e06` | green | +| L8 | `l8_merge_and_split_supersession_mismatch_refuse` | `dfa7e06` | green | +| L9 | `l9_accept_with_nonempty_supersedes_refuses` | `dfa7e06` | green | +| F1 | `f1_output_inside_input_refuses_including_symlink_aliases` | `bd2a339` | green | +| F2 | `f2_pre_existing_output_or_staging_refuses` | `bd2a339` | green | +| F3 | `f3_pre_staging_refusal_leaves_no_trace` | `dfa7e06` | green | +| F4 | `f4_injected_failures_land_in_enumerated_states` | `bd2a339` ⊙ | green | +| F5 | `f5_repeated_execution_is_byte_identical` | `dfa7e06` | green | +| F6 | `f6_index_inside_corpus_tree_refuses` | `bd2a339` | green | +| F7 | `f7_path_like_batch_id_influences_no_filesystem_path` | `bd2a339` ⊙ | green | +| F8 | `f8_index_symlink_aliases_converge_on_the_canonical_file` | `bd2a339` ⊙ | green | +| F9 | `f9_lock_release_failure_is_a_warning_never_the_primary_outcome` | `bd2a339` ⊙ | green | +| F10 | `f10_output_colliding_with_index_artifacts_refuses_before_the_lock` | `bd2a339` | green | +| F11 | `f11_hardlinked_index_refuses_through_either_entry` | `bd2a339` | green | +| F12 | `f12_late_temp_occupant_makes_commit_refuse_and_stays_untouched` | `bd2a339` | green | +| F13 | `f13_output_in_reserved_staging_namespace_refuses` | `bd2a339` | green | +| K1 | `k1_every_chunk_of_one_sha_updated_together` | `dfa7e06` | green | +| K2 | `k2_untouched_files_are_raw_byte_copies` | `dfa7e06` | green | +| K3 | `k3_touched_file_with_unknown_member_refuses` | `dfa7e06` | green | +| K4 | `k4_root_manifest_with_songs_refuses` | `dfa7e06` | green | +| K5 | `k5_tree_disagreement_and_missing_manifest_refuse` | `dfa7e06` | green | +| K6 | `k6_curated_songs_map_matches_labels_exactly` | `dfa7e06` | green | +| K7 | `k7_curated_manifest_at_protected_path_root_manifest_stays_songless` | `dfa7e06` | green | +| K8 | `k8_partial_curation_reports_not_holdout_ready` | `dfa7e06` | green | +| K9 | `k9_fully_curated_fixture_is_holdout_ready` | `dfa7e06` | green | +| K10 | `k10_duplicate_json_keys_in_touched_file_refuse` | `bd2a339` (+`5cc34b3`) | green | +| K11 | `k11_staged_tree_corruption_aborts_before_publication` | `bd2a339` | green | +| K12 | `k12_foreign_reserved_area_entry_refuses` | `dfa7e06` | green | +| R1 | `r1_report_digest_recomputes_identically` | `dfa7e06` | green | +| R2 | `r2_index_record_matches_report` | `dfa7e06` | green | +| R3 | `r3_success_publishes_report_and_record_refusal_publishes_neither` | `dfa7e06` | green | +| R4 | `r4_commit_failure_is_never_success_and_orphan_blocks_retry` | `bd2a339` | green | +| R5 | `r5_curated_manifest_digest_is_sha256_of_published_bytes` | `dfa7e06` | green | + +Plus 4 Phase-8 witnesses (`apply_witnesses.rs`, `5bb78fb`) beyond the +preregistered 63. + +## Refusal coverage + +**24 new typed refusals** — 23 exercised by at least one case: +`OutputAlreadyExists` (F2, R4), `OutputWouldModifyInput` (F1), +`ApplicationIndexInsideTree` (F6), `OutputCollidesWithIndexArtifacts` +(F10), `OutputNameReserved` (F13), `ApplicationIndexHardLinked` (F11), +`ApplicationIndexLocked` (C11, C12, C14, C16, F8), +`ApplicationIndexLockPathOccupied` (C15), `ApplicationIndexTempExists` +(C13), `MalformedPlanArtifact` (A2), `MalformedApplicationIndex` (C7, +witness), `CorpusTreeDisagreement` (K5, K10ii, K12), +`OrdinaryManifestCarriesSongs` (K4), `UnsupportedApplicationIndexSchema` +(witness), `DuplicateAppliedBatchId` (C8), `ApplicationIndexChainInvalid` +(C9), `DecisionBatchAlreadyApplied` (C3, C6), `ApplicationChainMismatch` +(C4, C5, witness), `SupersessionEvidenceContradiction` (L8, L9), +`ExistingLabelReplacementNotAuthorized` (L6, F9b), +`NonCanonicalCorpusFile` (K3, K10i), `ApplyIoError` (F4, F12), +`OutputPreflightInconsistent` (K11). The 24th — +`CuratedManifestPathNotDistinct` — is implemented as the §12 hard guard +and is, exactly as §12 itself states, structurally unreachable under the +fixed v1 curated path; no behavioural case can reach it (finding 3). + +**11 Apply-reachable Slice-1 refusals** — all exercised through the reused +`verify_plan`: `UnidentifiedSource` (A8a), `ConflictingExistingSongIds` +(A8b), `UnknownDecisionSource` (A8c), `SourceAssignedToMultipleSongs` +(A8d), `InvalidDecisionBatchOrder` (A7), `DuplicateDecisionEventId` (A8e), +`PlanCorpusFingerprintMismatch` (A5, L7), +`DecisionBatchFingerprintMismatch` (A5 — corpus drift breaks both +bindings), `DecisionDigestMismatch` (A4), `PlanDigestMismatch` (A3), +`DecisionProjectionMismatch` (A6). + +**3 ledger-side Slice-1 members stay intentionally unreachable** +(`UnsupportedDecisionsLedgerSchema`, `DuplicateDecisionBatchId`, +`BatchNotInLedger`): Apply consumes a plan, never a ledger — `grep` proof: +`apply.rs` calls `verify_plan` and `replay` only; `validate_ledger` / +`build_plan` are not referenced anywhere in the module. + +## Adversarial / fault-injection results + +Concurrency is deterministic: the second applier runs *inline inside a +one-shot fault hook on the same thread* (C14 inside the live step-12 temp +window; C16(a) inside the `create_new`→marker window) — no scheduler +timing anywhere. Injected failures land in exactly the enumerated §8.2 +states (F4 a–d), a commit failure can never surface as success and its +orphan blocks a retry until the mandated recovery (R4), and a lock-release +failure never changes the primary outcome in either direction (F9). + +The Phase-8 falsification pass probed 14 mutation targets; 10 already had +killers in the matrix, 4 got dedicated witnesses (`5bb78fb`). Three +mutations survive **by unreachability**, documented rather than tested: +§7.2 relation (3) (entailed by relation (2) + the step-5 fingerprint +proof; the contract states it for attribution), the +`CuratedManifestPathNotDistinct` guard (see above), and corpus-inside- +output containment (such an output necessarily exists and refuses +`OutputAlreadyExists` first under the §6 order). + +## Findings (implementation-time, no contract law changed) + +1. **serde's derived parse already refuses duplicated *known* struct + fields** ("duplicate field" at step-3 tree agreement), i.e. earlier and + stricter than the §10.3 pass. The distinct duplicate-rejecting pass + therefore guards its actual residual: duplicates inside *unknown* + members, which the tolerant derive skips wholesale. K10 was split to + prove both branches (`5cc34b3`). Fail-closed both ways; no law changed. +2. **Unresolvable non-index input paths** (missing corpus dir, missing + output parent) surface as `ApplyIoError { op: "canonicalize…" }` — the + contract's single typed I/O boundary; §6 enumerates no dedicated + refusal for them and none was invented. +3. **§14's coverage sentence vs the `CuratedManifestPathNotDistinct` + guard**: the contract simultaneously declares the guard structurally + unreachable (§12) and claims every Apply-reachable refusal appears in a + case (§14). Read together, the guard belongs with the documented- + unreachable set; recorded here so the implementation reviewer can + confirm that reading rather than inherit it silently. + +## Validation matrix + +| Check | Result | +|---|---| +| `cargo test --manifest-path song-curation/Cargo.toml` (isolated crate) | 112 passed / 0 failed (45 frozen Slice-1 + 27 + 16 + 20 + 4) | +| frozen Slice-1 suite | 45/45 green, untouched | +| `cargo test --workspace` | 1535 passed / 0 failed | +| `cargo clippy --workspace --all-targets -- -D warnings` | clean | +| `cargo clippy --manifest-path song-curation/Cargo.toml --all-targets` (crate `deny(all)`) | clean | +| `cargo fmt --all -- --check` | clean | +| `cargo doc --no-deps --workspace` | 1 pre-existing swang warning (ambiguous `format` link), untouched by this work | +| `cargo doc --no-deps --manifest-path song-curation/Cargo.toml` | 1 pre-existing warning in a frozen Slice-1 doc comment (private-item link), untouched | +| MSRV: `cargo +1.92 check --manifest-path song-curation/Cargo.toml --all-targets` | clean | + +The isolated crate remains a non-workspace member (root `Cargo.toml` +`exclude` unchanged) and is verified by the dedicated commands above, per +the ADR-0010 isolation posture. diff --git a/song-curation/README.md b/song-curation/README.md index 3263f1a..d9eb4a6 100644 --- a/song-curation/README.md +++ b/song-curation/README.md @@ -1,13 +1,18 @@ -# griff-song-curation — Slice 1: decision & validation core (ADR-0033) +# griff-song-curation — ADR-0033 Slices 1–2 An **isolated** offline tool (ADR-0010 / ADR-0033 isolation posture): deliberately **not** a workspace member, so production builds, CI, `--workspace` clippy, the CLI, and the cockpit never acquire curation policy. `griff-core` supplies only reusable schema/validation contracts. -This crate implements **Slice 1** of the accepted ADR-0033 workflow: the -**read-only decision & validation core**. It reads synthetic/fixture inputs and -**writes no corpus** — there is no apply path here. +This crate implements **Slice 1** of the accepted ADR-0033 workflow — the +**read-only decision & validation core** (accepted and frozen) — and +**Slice 2**, the **transactional Apply** (`apply` module), implemented +against the independently accepted Slice-2 contract +([`../docs/proposals/song-curation-slice-2-transactional-apply.md`](../docs/proposals/song-curation-slice-2-transactional-apply.md), +normative reviewed artifact `47e734c`; acceptance recorded in +`docs/decisions.log.md` @ `bad7b44`). Implementation evidence: +[`../docs/audit/2026-08-slice2-apply-implementation.md`](../docs/audit/2026-08-slice2-apply-implementation.md). ## What Slice 1 does @@ -95,14 +100,32 @@ curator decisions*, not asserted. `PlanCorpusFingerprintMismatch`, `DecisionBatchFingerprintMismatch`, `DecisionDigestMismatch`, `PlanDigestMismatch`, `DecisionProjectionMismatch`. +## Slice 2: transactional Apply (`apply` module) + +`apply(&ApplyPaths) -> ApplyRun` consumes a **serialized** Slice-1 plan, +the current corpus snapshot (a directory tree), and the application index, +and — under the contract's 12-step fail-closed verification order — +publishes the curated snapshot with its proof artifacts: the curated +manifest at the protected `song-curation/manifest.json` path, the +application report, and the appended `song-curation.applications.v1` index +record. **The batch is applied iff its record is in the index**: one +publication `rename` makes the snapshot visible; the index temp+`rename` +under a marker-bearing single-writer lock is the single commit point, and +every refusal — the closed 24-member typed surface plus the Slice-1 +refusals reused verbatim through `verify_plan` — proves the run did not +commit. Untouched files are raw byte copies; touched JSON is rewritten +under a fail-closed laundering guard (duplicate-key pass + round-trip +equality); the real core `song_holdout_preflight` runs exactly once, over +the staged curated view. + ## Out of scope (later, separately-accepted slices) -No corpus writes, output tree, transactional apply, application report/index, -manifest generation, holdout-readiness execution over a changed corpus, -suggestion generation or metadata normalization, interactive confirmation UI, -CLI/cockpit integration, or any real corpus files/labels. Slices 2–4 each need -separate independent acceptance; corpus labeling stays prohibited until the -controlled pilot is independently accepted (ADR-0033 Decision 10). +No suggestion generation or metadata normalization, no interactive +confirmation UI, no CLI/cockpit integration, and **no real corpus +files/labels** — every input this crate has ever touched is a synthetic +fixture. Slices 3–4 each need separate independent acceptance; corpus +labeling stays prohibited until the controlled pilot is independently +accepted (ADR-0033 Decision 10). ## Run From 8de362af2bcda98b787d5ae9cc5294b53ac6048c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 16:05:11 +0000 Subject: [PATCH 10/18] test(song-curation): RED witnesses for review blockers 1 and 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests-only. Preregisters the first two defects from the hostile implementation review of bbc7928 (PR #191), both fixable inside the accepted law — the expected refusals are exactly the ones §12 already assigns: - review1_empty_foreign_reserved_subdir_refuses: an EMPTY foreign subdirectory in the reserved area is invisible to the file-only walk and currently passes; §4.2 admits only the two tool-owned proof artifacts as regular files at the reserved root, so a directory entry is foreign even when empty → CorpusTreeDisagreement naming the entry. - review1_reserved_allowed_name_must_be_a_regular_file: an allowed NAME is not enough — a symlink at song-curation/manifest.json currently passes the name-only test → CorpusTreeDisagreement naming the entry. - review2_directory_lock_occupant_classifies_occupied: a pre-existing NON-REGULAR lock-path occupant (a directory) currently falls through fs::read into ApplyIoError; pre-existence must be classified at the lock boundary → ApplicationIndexLockPathOccupied, occupant untouched, and no read of unproven non-regular occupants (a FIFO could block the no-wait protocol). RED evidence: 0 passed; 3 failed (review1 ×2 currently APPLY SUCCEEDS where the law refuses; review2 returns ApplyIoError). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/tests/apply_review_repairs.rs | 133 ++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 song-curation/tests/apply_review_repairs.rs diff --git a/song-curation/tests/apply_review_repairs.rs b/song-curation/tests/apply_review_repairs.rs new file mode 100644 index 0000000..770faf2 --- /dev/null +++ b/song-curation/tests/apply_review_repairs.rs @@ -0,0 +1,133 @@ +//! Implementation-review repair witnesses (PR #191 hostile review round 1). +//! +//! Each test preregisters one defect the review found in the implementation +//! relative to the accepted contract `47e734c`. Committed RED, tests-only; +//! the GREEN repair follows in a separate commit. No contract law changes: +//! every expected refusal is the one the accepted taxonomy already assigns. + +mod common; + +use common::{ + accept, batch_for, event, layout, lock_path_of, write_corpus, write_empty_index, write_plan, + Chunk, Layout, +}; +use griff_song_curation::apply::{apply, fault, ApplyPaths, ApplyRefusal, ApplyRun}; +use std::fs; +use std::io; + +fn run(l: &Layout) -> ApplyRun { + apply(&ApplyPaths { + plan: l.plan.clone(), + corpus: l.corpus.clone(), + index: l.index.clone(), + output: l.output.clone(), + }) +} + +fn refuse(l: &Layout) -> ApplyRefusal { + run(l).primary.expect_err("expected a refusal") +} + +const AB: [Chunk<'static>; 2] = [ + Chunk { + id: "a1", + sha: Some("shaA"), + song: None, + }, + Chunk { + id: "b1", + sha: Some("shaB"), + song: None, + }, +]; + +fn valid_setup(l: &Layout) { + let m = write_corpus(&l.corpus, &AB); + let b = batch_for( + &m, + "batch1", + None, + vec![event("ev0", 0, accept("g", &["shaA"], "song-000001", &[]))], + ); + write_plan(&l.corpus, b, &l.plan); + write_empty_index(&l.index); +} + +/// Review blocker 1a: an EMPTY foreign subdirectory inside the reserved area +/// is invisible to a file-only walk, yet the accepted §4.2 shape law admits +/// only the two tool-owned proof artifacts as regular files at the reserved +/// root — a directory entry is foreign even when it contains nothing. +#[test] +fn review1_empty_foreign_reserved_subdir_refuses() { + let l = layout("rev1-emptydir"); + valid_setup(&l); + fs::create_dir_all(l.corpus.join("song-curation/extra")).expect("mk empty foreign dir"); + let refusal = refuse(&l); + match &refusal { + ApplyRefusal::CorpusTreeDisagreement { detail } => { + assert!(detail.contains("extra"), "must name the entry: {detail}"); + } + other => panic!("expected CorpusTreeDisagreement, got {other:?}"), + } + assert!(!l.output.exists(), "nothing written"); +} + +/// Review blocker 1b: an allowed NAME does not satisfy the shape law unless +/// it is a regular file — a symlink at `song-curation/manifest.json` must +/// refuse, not pass a name-only test. +#[test] +fn review1_reserved_allowed_name_must_be_a_regular_file() { + let l = layout("rev1-symlink"); + valid_setup(&l); + fs::create_dir_all(l.corpus.join("song-curation")).expect("mk reserved"); + // Symlink to a real file elsewhere, so a follow-based test would even + // find readable JSON behind it. + std::os::unix::fs::symlink( + l.corpus.join("manifest.json"), + l.corpus.join("song-curation/manifest.json"), + ) + .expect("symlink"); + let refusal = refuse(&l); + match &refusal { + ApplyRefusal::CorpusTreeDisagreement { detail } => { + assert!( + detail.contains("song-curation/manifest.json"), + "must name the entry: {detail}" + ); + } + other => panic!("expected CorpusTreeDisagreement, got {other:?}"), + } + assert!(!l.output.exists(), "nothing written"); +} + +/// Review blocker 2: a pre-existing NON-REGULAR occupant of the lock path +/// (here: a directory) must be classified at the lock boundary as +/// `ApplicationIndexLockPathOccupied` — §12 reserves `ApplyIoError` for lock +/// -acquisition causes other than pre-existence, and reading an arbitrary +/// occupant (a FIFO could block) contradicts the no-wait protocol. The +/// occupant stays untouched. +#[test] +fn review2_directory_lock_occupant_classifies_occupied() { + let l = layout("rev2-dirlock"); + valid_setup(&l); + let lock = lock_path_of(&l.index); + fs::create_dir(&lock).expect("directory occupant"); + let refusal = refuse(&l); + assert!( + matches!( + refusal, + ApplyRefusal::ApplicationIndexLockPathOccupied { .. } + ), + "pre-existence is classified, never ApplyIoError: got {refusal:?}" + ); + assert!(lock.is_dir(), "the occupant is left untouched"); + assert!(!l.output.exists(), "nothing written"); +} + +// Keep the fault import referenced so the harness stays linked for the +// blocker-3 witness added in the next tests-only commit. +#[allow(dead_code)] +fn _fault_link() { + fault::clear(); + let _ = io::Error::other("unused"); +} From 9d505cd691b895c997608e62f8c62c7c65dbed3d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 16:05:52 +0000 Subject: [PATCH 11/18] =?UTF-8?q?test(song-curation):=20RED=20witness=20fo?= =?UTF-8?q?r=20review=20blocker=203=20=E2=80=94=20release=20warning=20surv?= =?UTF-8?q?ives=20marker=20failure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests-only. After create_new succeeds the lock IS acquired, so a marker- publication failure is a post-acquisition exit and §8.2's observable result shape applies in full: the I/O refusal stays the primary outcome and a failed release attaches as the orthogonal LockReleaseWarning — never silently discarded by a best-effort remove. The witness drives both manifestations with the existing fault points (no new fixture needed): - double fault (lock:after_create + lock:release both fail): primary ApplyIoError, warning Some, stale lock remains for §8.2 recovery; - single fault (marker fails, release succeeds): warning None, no stale lock. RED evidence: 0 passed; 1 failed — current code returns warning: None on the double fault and removes the lock through an untracked cleanup path. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/tests/apply_review_repairs.rs | 53 +++++++++++++++++++-- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/song-curation/tests/apply_review_repairs.rs b/song-curation/tests/apply_review_repairs.rs index 770faf2..850bc37 100644 --- a/song-curation/tests/apply_review_repairs.rs +++ b/song-curation/tests/apply_review_repairs.rs @@ -124,10 +124,53 @@ fn review2_directory_lock_occupant_classifies_occupied() { assert!(!l.output.exists(), "nothing written"); } -// Keep the fault import referenced so the harness stays linked for the -// blocker-3 witness added in the next tests-only commit. -#[allow(dead_code)] -fn _fault_link() { +fn io_fail() -> io::Result<()> { + Err(io::Error::other("injected fault")) +} + +/// Review blocker 3: after `create_new` succeeds the lock IS acquired, so a +/// marker-publication failure is a post-acquisition exit — §8.2's result +/// shape applies in full: the I/O refusal stays the primary outcome, and a +/// failed release is attached as the orthogonal warning, never silently +/// discarded by a `let _ = remove_file`. +#[test] +fn review3_marker_failure_with_release_failure_keeps_the_warning() { + // (a) double fault: marker publication fails AND the release fails — + // the stale lock remains and the warning must say so. + let l = layout("rev3-double"); + valid_setup(&l); + fault::set("lock:after_create", io_fail); + fault::set("lock:release", io_fail); + let result = run(&l); fault::clear(); - let _ = io::Error::other("unused"); + let refusal = result + .primary + .expect_err("marker failure is the primary refusal"); + assert!( + matches!(refusal, ApplyRefusal::ApplyIoError { .. }), + "got {refusal:?}" + ); + let warning = result + .lock_release_warning + .expect("the failed release must surface as the orthogonal warning"); + assert!(warning.lockfile.contains(".lock")); + assert!( + lock_path_of(&l.index).exists(), + "the stale lock remains for §8.2 recovery" + ); + fs::remove_file(lock_path_of(&l.index)).expect("operator recovery"); + + // (b) single fault: marker publication fails, release succeeds — no + // warning, no stale lock. + let l = layout("rev3-single"); + valid_setup(&l); + fault::set("lock:after_create", io_fail); + let result = run(&l); + fault::clear(); + assert!(result.primary.is_err()); + assert!( + result.lock_release_warning.is_none(), + "a successful release attaches no warning" + ); + assert!(!lock_path_of(&l.index).exists(), "lock released"); } From 987d0a346a0c471119647cada0675fea9f64a4a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 16:08:19 +0000 Subject: [PATCH 12/18] fix(song-curation): repair the three review blockers inside the accepted law MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GREEN for the four RED review witnesses. No refusal added, no ordering changed, no contract law touched. 1. Reserved-area shape (§4.2 / §6 step 3) is now inspected at the directory-entry level: check_reserved_shape reads the reserved root itself, so an EMPTY foreign subdirectory is foreign (a file-only walk could not see it), and the two allowed names must be REGULAR FILES — a symlink or any other non-regular type at song-curation/manifest.json or apply-report.json refuses. Both are CorpusTreeDisagreement exactly as the accepted taxonomy assigns. 2. Lock-boundary classification no longer assumes a readable regular file: classify_lock_occupant stats without following first; a non-regular occupant (directory; FIFO — which a read could block on, breaking the no-wait protocol), an unstatable or unreadable occupant, or non-prefix content all fail closed as ApplicationIndexLockPathOccupied and are never read or touched. Prefix classification is reached only for regular readable files; ApplyIoError stays reserved for non-pre-existence causes, per §12. 3. Lock acquisition is split at the true acquisition point: create_lock (create_new + occupant classification — a failure here means the lock was never acquired, warning channel n/a) and publish_marker (the marker write into the already-acquired lock). Every post-create exit — marker-publication failure included — now releases through the one release_lock channel, so a failed release always surfaces as the orthogonal §8.2 LockReleaseWarning instead of vanishing in a best-effort remove; the primary refusal is preserved unchanged. Evidence: crate suite 116/0 (45 frozen Slice-1 + 71 Slice-2, including the four review witnesses now green); clippy deny(all) clean; fmt clean. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/src/apply.rs | 134 ++++++++++++++++++++++++------------- 1 file changed, 87 insertions(+), 47 deletions(-) diff --git a/song-curation/src/apply.rs b/song-curation/src/apply.rs index 21b7c98..fbedc6f 100644 --- a/song-curation/src/apply.rs +++ b/song-curation/src/apply.rs @@ -261,14 +261,21 @@ pub fn apply(paths: &ApplyPaths) -> ApplyRun { } } }; - // Step 1 (lock): acquire the single-writer lock; held through step 12. - if let Err(refusal) = acquire_lock(&ctx) { - return ApplyRun { - primary: Err(refusal), - lock_release_warning: None, - }; - } - let primary = locked_apply(paths, &ctx); + // Step 1 (lock): create the lockfile. A failure here means the lock was + // never acquired, so no release (and no warning channel) applies. + let lock_file = match create_lock(&ctx) { + Ok(file) => file, + Err(refusal) => { + return ApplyRun { + primary: Err(refusal), + lock_release_warning: None, + } + } + }; + // From this point the lock IS acquired: every exit — marker-publication + // failure included — releases through the one §8.2 channel, so a failed + // release always surfaces as the orthogonal warning. + let primary = publish_marker(lock_file, &ctx).and_then(|()| locked_apply(paths, &ctx)); let lock_release_warning = release_lock(&ctx.lock_path); ApplyRun { primary, @@ -459,43 +466,59 @@ fn staging_path(output: &Path) -> PathBuf { .join(format!(".{name}.apply-staging")) } -fn acquire_lock(ctx: &Ctx) -> Result<(), ApplyRefusal> { - let mut file = match fs::OpenOptions::new() +fn create_lock(ctx: &Ctx) -> Result { + match fs::OpenOptions::new() .write(true) .create_new(true) .open(&ctx.lock_path) { - Ok(file) => file, + Ok(file) => Ok(file), Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { - // Contention classification is prefix-closed (§8.1): any byte - // prefix of the canonical marker — empty, partial, or complete — - // is a Griff lock (a live writer can only ever be observed in a - // prefix state); anything else is an unproven occupant that no - // mandated recovery may delete or relocate. - let content = fs::read(&ctx.lock_path) - .map_err(|e| io_refusal(&ctx.lock_path, "lock read", &e))?; - if LOCK_MARKER.as_bytes().starts_with(content.as_slice()) { - return Err(ApplyRefusal::ApplicationIndexLocked { - path: ctx.lock_path.display().to_string(), - }); - } - return Err(ApplyRefusal::ApplicationIndexLockPathOccupied { - path: ctx.lock_path.display().to_string(), - }); + Err(classify_lock_occupant(&ctx.lock_path)) } - Err(e) => return Err(io_refusal(&ctx.lock_path, "lock create_new", &e)), + Err(e) => Err(io_refusal(&ctx.lock_path, "lock create_new", &e)), + } +} + +/// Pre-existence at the lock path is CLASSIFIED, never an I/O error (§12 +/// reserves `ApplyIoError` for lock-acquisition causes other than +/// pre-existence). Contention classification is prefix-closed (§8.1): any +/// byte prefix of the canonical marker — empty, partial, or complete — is a +/// Griff lock (a live writer can only ever be observed in a prefix state). +/// Anything else — a non-regular occupant (a directory; a FIFO, which a read +/// could even block on, breaking the no-wait protocol), an unreadable or +/// unstatable occupant, or non-prefix content — is unproven and fails closed +/// as `ApplicationIndexLockPathOccupied`; no mandated recovery may delete or +/// relocate it, and it is never read unless it is a regular file. +fn classify_lock_occupant(lock_path: &Path) -> ApplyRefusal { + let occupied = ApplyRefusal::ApplicationIndexLockPathOccupied { + path: lock_path.display().to_string(), }; - if let Err(e) = fault::hit("lock:after_create") { - let refusal = io_refusal(&ctx.lock_path, "lock marker write", &e); - let _ = fs::remove_file(&ctx.lock_path); - return Err(refusal); + let Ok(meta) = lock_path.symlink_metadata() else { + return occupied; + }; + if !meta.is_file() { + return occupied; } - if let Err(e) = file.write_all(LOCK_MARKER.as_bytes()) { - let refusal = io_refusal(&ctx.lock_path, "lock marker write", &e); - let _ = fs::remove_file(&ctx.lock_path); - return Err(refusal); + let Ok(content) = fs::read(lock_path) else { + return occupied; + }; + if LOCK_MARKER.as_bytes().starts_with(content.as_slice()) { + return ApplyRefusal::ApplicationIndexLocked { + path: lock_path.display().to_string(), + }; } - Ok(()) + occupied +} + +/// Publish the ownership marker into the already-acquired lock. A failure is +/// a post-acquisition refusal: the caller releases through [`release_lock`], +/// so a failed cleanup surfaces as the §8.2 warning instead of vanishing. +fn publish_marker(mut file: fs::File, ctx: &Ctx) -> Result<(), ApplyRefusal> { + fault::hit("lock:after_create") + .map_err(|e| io_refusal(&ctx.lock_path, "lock marker write", &e))?; + file.write_all(LOCK_MARKER.as_bytes()) + .map_err(|e| io_refusal(&ctx.lock_path, "lock marker write", &e)) } fn release_lock(lock_path: &Path) -> Option { @@ -639,8 +662,8 @@ fn load_snapshot(corpus: &Path) -> Result { walk(corpus, corpus, &mut files).map_err(|(p, e)| io_refusal(&p, "walk", &e))?; files.sort(); let reserved_root = corpus.join(RESERVED_DIR); - if reserved_root.exists() { - check_reserved_shape(corpus, &files)?; + if reserved_root.symlink_metadata().is_ok() { + check_reserved_shape(&reserved_root)?; } let files: Vec<(String, PathBuf)> = files .into_iter() @@ -713,19 +736,36 @@ fn walk( Ok(()) } -/// The reserved-area shape law (§4.2): recursively, at most the two tool-owned -/// proof artifacts as regular files at the reserved root. -fn check_reserved_shape(_corpus: &Path, files: &[(String, PathBuf)]) -> Result<(), ApplyRefusal> { - let allowed = [ - format!("{RESERVED_DIR}/manifest.json"), - format!("{RESERVED_DIR}/apply-report.json"), - ]; - for (rel, _) in files.iter().filter(|(rel, _)| is_reserved(rel)) { - if !allowed.contains(rel) { +/// The reserved-area shape law (§4.2): the reserved directory holds exactly +/// at most the two tool-owned proof artifacts as REGULAR FILES directly at +/// its root. Inspected at the directory-entry level — a file-only walk would +/// miss an empty foreign subdirectory, and an allowed NAME is not enough +/// when the entry is a symlink or any other non-regular type. +fn check_reserved_shape(reserved_root: &Path) -> Result<(), ApplyRefusal> { + if !reserved_root.symlink_metadata().is_ok_and(|m| m.is_dir()) { + return Err(tree_disagreement(format!( + "reserved area {RESERVED_DIR} is not a directory" + ))); + } + let allowed = ["manifest.json", "apply-report.json"]; + let entries = fs::read_dir(reserved_root) + .map_err(|e| tree_disagreement(format!("reserved area {RESERVED_DIR}: {e}")))?; + for entry in entries { + let entry = + entry.map_err(|e| tree_disagreement(format!("reserved area {RESERVED_DIR}: {e}")))?; + let name = entry.file_name().to_string_lossy().into_owned(); + let rel = format!("{RESERVED_DIR}/{name}"); + if !allowed.contains(&name.as_str()) { return Err(tree_disagreement(format!( "foreign reserved-area entry: {rel}" ))); } + let is_regular = entry.path().symlink_metadata().is_ok_and(|m| m.is_file()); + if !is_regular { + return Err(tree_disagreement(format!( + "reserved-area entry is not a regular file: {rel}" + ))); + } } Ok(()) } From 92c97d6bb1e5c0c8968ee657c17ae4c77fe56439 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 16:09:11 +0000 Subject: [PATCH 13/18] test(song-curation): C16(a) is the exact preregistered live partial-marker case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Evidence correction (review blocker 4): the accepted C16 requires a NON-EMPTY partial marker in BOTH manifestations, and the live half previously observed only the empty state right after create_new. The lock:after_create hook now materializes a non-empty strict prefix of the marker at the held lock path, asserts the observed bytes really are that prefix, and only then runs the second applier inline — which must classify ApplicationIndexLocked, never LockPathOccupied. The first writer then completes its marker publication over the same handle and commits. The crash half (permanent non-empty partial + provenance-gated recovery) was already exact and is unchanged. Green on the repaired implementation; the 63/63 claim becomes true only as of this commit and is restated in the evidence update that follows. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/tests/apply_adversarial.rs | 25 ++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/song-curation/tests/apply_adversarial.rs b/song-curation/tests/apply_adversarial.rs index c4967eb..306a91b 100644 --- a/song-curation/tests/apply_adversarial.rs +++ b/song-curation/tests/apply_adversarial.rs @@ -542,8 +542,12 @@ fn c15_real_second_index_at_lock_name_is_occupied_not_locked() { #[test] fn c16_partial_lock_marker_classifies_locked_and_recovery_is_gated() { - // (a) live window: a concurrent applier observing the lock between - // create_new and marker publication (empty prefix) refuses Locked. + // (a) live window, the exact preregistered case: a concurrent applier + // observes a NON-EMPTY strict prefix of the marker while the first + // writer's marker publication is still incomplete. The hook fires + // between create_new and the marker write; it materializes the partial + // state at the held lock path, proves it is really non-empty and + // partial, and only then runs the second applier inline. let l = layout("c16a"); valid_setup(&l); let second_result: Rc>> = Rc::new(RefCell::new(None)); @@ -553,7 +557,15 @@ fn c16_partial_lock_marker_classifies_locked_and_recovery_is_gated() { let corpus = l.corpus.clone(); let index = l.index.clone(); let output2 = l.td.path.join("out2"); + let lock = lock_path_of(&l.index); fault::set("lock:after_create", move || { + let partial = &LOCK_MARKER.as_bytes()[..10]; + fs::write(&lock, partial).expect("materialize live partial marker"); + let observed = fs::read(&lock).expect("read live lock"); + assert_eq!( + observed, partial, + "the live state is a non-empty strict prefix" + ); let second = run_paths(&plan, &corpus, &index, &output2); *second_result.borrow_mut() = Some(second.primary.expect_err("second applier must refuse")); @@ -562,11 +574,16 @@ fn c16_partial_lock_marker_classifies_locked_and_recovery_is_gated() { } let first = run(&l); fault::clear(); - assert!(first.primary.is_ok(), "{:?}", first.primary); + assert!( + first.primary.is_ok(), + "the first writer completes: {:?}", + first.primary + ); let second = second_result.borrow_mut().take().expect("second ran"); assert!( matches!(second, ApplyRefusal::ApplicationIndexLocked { .. }), - "an empty prefix is a Griff lock, never foreign occupancy, got {second:?}" + "a live NON-EMPTY partial marker is a Griff lock, never foreign \ + occupancy, got {second:?}" ); // (b) crashed non-empty partial marker: classified Locked; never From 5aca93ad6f7b8b9137d278d5ce95caa7786f306c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 16:10:52 +0000 Subject: [PATCH 14/18] docs(song-curation): record review repair round 1 in the implementation evidence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Appends the repair-round section: the four review blockers, the RED→GREEN repair commits (8de362a, 9d505cd, 987d0a3, 92c97d6), the corrected matrix statement (63/63 holds only as of 92c97d6 — the prior claim was overstated, as the review found), updated totals (crate 116/0, clippy, fmt, MSRV re-verified), and the unchanged acceptance state: implementation acceptance pending independent re-review; contract acceptance at 47e734c unaffected; Slice 3 / pilot / corpus labeling still BLOCKED. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- .../2026-08-slice2-apply-implementation.md | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/docs/audit/2026-08-slice2-apply-implementation.md b/docs/audit/2026-08-slice2-apply-implementation.md index 30876f3..75a33ac 100644 --- a/docs/audit/2026-08-slice2-apply-implementation.md +++ b/docs/audit/2026-08-slice2-apply-implementation.md @@ -230,3 +230,41 @@ output containment (such an output necessarily exists and refuses The isolated crate remains a non-workspace member (root `Cargo.toml` `exclude` unchanged) and is verified by the dedicated commands above, per the ADR-0010 isolation posture. + +## Repair round 1 — hostile implementation review of `bbc7928` + +The independent implementation review (PR #191) FAILED acceptance at +`bbc7928` with three implementation blockers and one acceptance-evidence +blocker — all repairable inside the accepted law; none reopened `47e734c`. +Repairs follow the mandated history discipline; the original nine commits +are untouched. + +| # | Commit | Kind | Content | +|---|---|---|---| +| 10 | `8de362a` | RED | Witnesses for blockers 1–2: empty foreign reserved subdirectory; allowed reserved name that is a symlink; directory occupant at the lock path. Evidence in the message: 0 passed / 3 failed (the two shape cases wrongly *applied*; the lock case returned `ApplyIoError`). | +| 11 | `9d505cd` | RED | Witness for blocker 3: marker-publication failure is a post-acquisition exit — primary `ApplyIoError` + orthogonal release warning on a double fault; no warning and no stale lock on a single fault. 0 passed / 1 failed. | +| 12 | `987d0a3` | GREEN | `check_reserved_shape` inspects the reserved root at the directory-entry level (empty subdirectory = foreign; allowed names must be regular files); `classify_lock_occupant` stats before ever reading (non-regular / unstatable / unreadable occupants fail closed as `ApplicationIndexLockPathOccupied`, never read — a FIFO can no longer block the no-wait protocol); lock acquisition split at the true acquisition point (`create_lock` / `publish_marker`) so every post-create exit releases through the single §8.2 warning channel. | +| 13 | `92c97d6` | TEST-FIX | C16(a) made exact: the live window now materializes a **non-empty strict prefix** of the marker at the held lock path (asserted as such) before the inline second applier observes it — the preregistered case as accepted, not the empty-state approximation. | +| 14 | (this commit) | DOCS | This evidence update. | + +Corrected matrix statement: **63/63 holds as of `92c97d6`** — before it, +C16's live half was an approximation and the claim was overstated, exactly +as the review found. The four review witnesses in +`tests/apply_review_repairs.rs` are additional to the preregistered 63. + +Updated totals: isolated crate suite **116 passed / 0 failed** (45 frozen +Slice 1 + 71 Slice 2); crate clippy `deny(all)` clean; fmt clean; +`cargo +1.92 check --all-targets` clean. The repairs touch only the +isolated crate, so the workspace results recorded above are unaffected +(the workspace does not build this crate). + +What the review confirmed intact is recorded in its comment on PR #191: +the 12-step ordering, literal `verify_plan` reuse, the single replay +attribution, hardlink refusal, under-lock temp inspection, `create_new` +temp, the preservation split, the duplicate-key pass, the staged re-read, +the single real preflight, and the commit-point semantics. + +Acceptance state: implementation acceptance remains **pending independent +re-review** at the new head. Contract acceptance at `47e734c` is +unaffected; Slice 3, the controlled pilot, and real-/full-corpus labeling +remain BLOCKED. From 94e5c38d47e6a55674eade39abbfcf1627460063 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 02:06:39 +0000 Subject: [PATCH 15/18] fixture(song-curation): walk:descend trace point for non-traversal witnesses Re-review of repair round 1 found one remaining blocker: the reserved subtree is excluded from enumeration only AFTER the recursive walk has already traversed it (Path::is_dir() even follows a directory symlink at the reserved root). To witness that defect deterministically, the walk gains a trace point that fires before every descent below the corpus root. Pure test harness, same posture as the fault registry itself: production registers no hook, so the point is an inert no-op. Fixture-only: no behavioural change; isolated crate suite remains 116 passed / 0 failed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/src/apply.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/song-curation/src/apply.rs b/song-curation/src/apply.rs index fbedc6f..c350ea3 100644 --- a/song-curation/src/apply.rs +++ b/song-curation/src/apply.rs @@ -723,6 +723,10 @@ fn walk( for entry in entries { let path = entry.map_err(|e| (dir.to_path_buf(), e))?.path(); if path.is_dir() { + // Trace point: fires before every descent below the corpus root, + // so a witness can prove a subtree was NOT traversed. Inert in + // production (no hook registered). + fault::hit("walk:descend").map_err(|e| (path.clone(), e))?; walk(root, &path, out)?; } else { let rel = path From 60fc2286fd2b8e816c6c1bdca29807bdfba5bc6b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 02:08:07 +0000 Subject: [PATCH 16/18] =?UTF-8?q?test(song-curation):=20RED=20witnesses=20?= =?UTF-8?q?=E2=80=94=20reserved=20subtree=20must=20refuse=20without=20trav?= =?UTF-8?q?ersal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-review blocker on 5aca93a: load_snapshot runs the full recursive walk BEFORE check_reserved_shape, and the walk's Path::is_dir() follows directory symlinks, so a foreign reserved subtree (or a reserved root symlinked to an external directory) is traversed before the §4.2 / §6 step-3 shape law can refuse it. The accepted contract excludes the reserved subtree from corpus-content enumeration; the immediate reserved-root entry is already sufficient to refuse. Two witnesses, both armed with the walk:descend trace hook so traversal is observable (the fixture corpus has no legitimate subdirectory): - rereview_foreign_reserved_subtree_refused_without_traversal: song-curation/extra/deep/foreign.json must refuse as CorpusTreeDisagreement naming the immediate entry, never descending. - rereview_reserved_root_symlink_refused_without_traversal: song-curation -> must be classified no-follow as 'not a directory', never entering the external target. RED evidence (tests-only; cargo test --test apply_review_repairs): both FAILED with the identical defect signature — ApplyIoError { path: ".../corpus/song-curation", op: "walk", detail: "injected fault" } i.e. the walk descended into / followed the symlinked reserved root. Expected instead: the shape law's own CorpusTreeDisagreement with the trace hook never reached. 2 failed / 0 of the new passing; the prior 116 tests unaffected. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/tests/apply_review_repairs.rs | 70 +++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/song-curation/tests/apply_review_repairs.rs b/song-curation/tests/apply_review_repairs.rs index 850bc37..572bf32 100644 --- a/song-curation/tests/apply_review_repairs.rs +++ b/song-curation/tests/apply_review_repairs.rs @@ -128,6 +128,76 @@ fn io_fail() -> io::Result<()> { Err(io::Error::other("injected fault")) } +/// Re-review blocker (round 2): the reserved subtree must be excluded from +/// corpus-content enumeration BEFORE any descent (§4.2 / §6 step 3) — shape +/// classification of the immediate reserved-root entry is already sufficient +/// to refuse, so a foreign subtree is never traversed. The armed +/// `walk:descend` trace hook makes traversal observable: if the walk +/// descends anywhere (the fixture corpus has no legitimate subdirectory), +/// the run degrades to `ApplyIoError`; the required outcome is the shape +/// law's own `CorpusTreeDisagreement` with the hook never reached. +#[test] +fn rereview_foreign_reserved_subtree_refused_without_traversal() { + let l = layout("rerev-subtree"); + valid_setup(&l); + let deep = l.corpus.join("song-curation/extra/deep"); + fs::create_dir_all(&deep).expect("mk foreign subtree"); + fs::write(deep.join("foreign.json"), "{}").expect("foreign content"); + fault::set("walk:descend", io_fail); + let result = run(&l); + fault::clear(); + let refusal = result.primary.expect_err("expected a refusal"); + match &refusal { + ApplyRefusal::CorpusTreeDisagreement { detail } => { + assert!( + detail.contains("extra"), + "the immediate foreign entry is named without traversal: {detail}" + ); + } + other => panic!( + "expected CorpusTreeDisagreement without traversal, got {other:?} \ + (ApplyIoError here means the walk descended)" + ), + } + assert!(!l.output.exists(), "nothing written"); +} + +/// Re-review blocker (round 2), symlink half: `Path::is_dir()` follows a +/// directory symlink, so a reserved root that is a symlink to an external +/// directory was traversed OUTSIDE the corpus before the shape law ran. The +/// no-follow shape classification of the reserved root itself must refuse +/// first; the external target is never entered. +#[test] +fn rereview_reserved_root_symlink_refused_without_traversal() { + let l = layout("rerev-symlink"); + valid_setup(&l); + let outside = l.td.path.join("outside"); + fs::create_dir_all(outside.join("inner")).expect("mk external tree"); + fs::write(outside.join("inner/marker.json"), "{}").expect("external content"); + std::os::unix::fs::symlink(&outside, l.corpus.join("song-curation")).expect("symlink"); + fault::set("walk:descend", io_fail); + let result = run(&l); + fault::clear(); + let refusal = result.primary.expect_err("expected a refusal"); + match &refusal { + ApplyRefusal::CorpusTreeDisagreement { detail } => { + assert!( + detail.contains("not a directory"), + "the symlinked reserved root is classified no-follow: {detail}" + ); + } + other => panic!( + "expected CorpusTreeDisagreement without traversal, got {other:?} \ + (ApplyIoError here means the walk followed the symlink)" + ), + } + assert!(!l.output.exists(), "nothing written"); + assert!( + outside.join("inner/marker.json").exists(), + "the external tree is untouched" + ); +} + /// Review blocker 3: after `create_new` succeeds the lock IS acquired, so a /// marker-publication failure is a post-acquisition exit — §8.2's result /// shape applies in full: the I/O refusal stays the primary outcome, and a From 96ca7556e0432388f378ee2768e48e12af1378b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 02:09:58 +0000 Subject: [PATCH 17/18] fix(song-curation): exclude the reserved root from the walk before any descent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GREEN for the round-2 re-review blocker. The recursive corpus walk now computes each entry's relative name FIRST and skips the reserved root before any type check — Path::is_dir() follows directory symlinks, so even asking the type would traverse a symlinked reserved root. The reserved subtree is therefore excluded from corpus-content enumeration exactly as §4.2 / §6 step 3 require, and check_reserved_shape stays the single, no-follow, directory-entry-level classifier of that root: - a foreign subtree under song-curation/ refuses on its immediate root entry (CorpusTreeDisagreement) without one descent into it; - song-curation -> refuses as 'not a directory' via symlink_metadata without ever entering the external target — no enumeration outside the corpus, no alias loops, no path-length degradation into ApplyIoError; - the post-walk is_reserved filter remains only as a redundant invariant. No refusal, schema, ordering, or contract change; both round-2 witnesses flip to the shape law's own refusal with the walk:descend trace hook proving zero descents (the fixture corpora have no other subdirectory). Isolated crate suite: 118 passed / 0 failed (45 frozen Slice-1 + 73 Slice-2). Crate clippy deny(all) clean; fmt clean; cargo +1.92 check --all-targets clean. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- song-curation/src/apply.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/song-curation/src/apply.rs b/song-curation/src/apply.rs index c350ea3..acea1aa 100644 --- a/song-curation/src/apply.rs +++ b/song-curation/src/apply.rs @@ -656,8 +656,10 @@ fn load_snapshot(corpus: &Path) -> Result { let manifest: CorpusManifest = serde_json::from_str(&manifest_text) .map_err(|e| tree_disagreement(format!("root manifest.json: {e}")))?; - // Recursive sorted walk, the migrate discipline; the reserved area is - // excluded from corpus-content enumeration and shape-checked instead. + // Recursive sorted walk, the migrate discipline. The walk itself skips + // the reserved root before any descent (§4.2) and the shape law below + // classifies it independently; the post-walk filter stays only as a + // redundant invariant. let mut files = Vec::new(); walk(corpus, corpus, &mut files).map_err(|(p, e)| io_refusal(&p, "walk", &e))?; files.sort(); @@ -722,6 +724,19 @@ fn walk( let entries = fs::read_dir(dir).map_err(|e| (dir.to_path_buf(), e))?; for entry in entries { let path = entry.map_err(|e| (dir.to_path_buf(), e))?.path(); + let rel = path + .strip_prefix(root) + .unwrap_or(&path) + .to_string_lossy() + .replace('\\', "/"); + // §4.2: the reserved subtree is excluded from corpus-content + // enumeration BEFORE any type check or descent — `is_dir` follows + // directory symlinks, so even asking would traverse a symlinked + // reserved root. Its shape is classified independently, no-follow, + // at the directory-entry level. + if rel == RESERVED_DIR { + continue; + } if path.is_dir() { // Trace point: fires before every descent below the corpus root, // so a witness can prove a subtree was NOT traversed. Inert in @@ -729,11 +744,6 @@ fn walk( fault::hit("walk:descend").map_err(|e| (path.clone(), e))?; walk(root, &path, out)?; } else { - let rel = path - .strip_prefix(root) - .unwrap_or(&path) - .to_string_lossy() - .replace('\\', "/"); out.push((rel, path)); } } From 54639fc8bb2c993211ca8ca13e8e7fee4765f6e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 02:10:32 +0000 Subject: [PATCH 18/18] docs(song-curation): record review repair round 2 in the implementation evidence Appends the round-2 section: the re-review confirmed the four round-1 repairs and failed on one remaining blocker (reserved subtree traversed before the shape law); commits 15-17 repair it as evidence (FIXTURE trace point, tests-only RED, GREEN walk exclusion). 63/63 unchanged; crate suite 118/0. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn --- .../2026-08-slice2-apply-implementation.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/docs/audit/2026-08-slice2-apply-implementation.md b/docs/audit/2026-08-slice2-apply-implementation.md index 75a33ac..099071b 100644 --- a/docs/audit/2026-08-slice2-apply-implementation.md +++ b/docs/audit/2026-08-slice2-apply-implementation.md @@ -268,3 +268,40 @@ Acceptance state: implementation acceptance remains **pending independent re-review** at the new head. Contract acceptance at `47e734c` is unaffected; Slice 3, the controlled pilot, and real-/full-corpus labeling remain BLOCKED. + +## Repair round 2 — re-review of `5aca93a` + +The independent re-review confirmed all four round-1 repairs and the +validity of the repair history, then FAILED acceptance on one remaining +blocker: the reserved subtree was excluded from corpus-content +enumeration only *after* the recursive walk — `load_snapshot` ran +`walk()` over the whole tree (and `Path::is_dir()` follows directory +symlinks) before `check_reserved_shape`, so a foreign reserved subtree +was traversed before the §4.2 / §6 step-3 shape law could refuse it. A +reserved root symlinked to an external directory was even enumerated +*outside* the corpus, and alias loops could degrade into `ApplyIoError` +or path-length failures instead of the immediate `CorpusTreeDisagreement` +the accepted law assigns. Repairable inside the accepted law; `47e734c` +not reopened. + +| # | Commit | Kind | Content | +|---|---|---|---| +| 15 | `94e5c38` | FIXTURE | `walk:descend` trace point — fires before every descent below the corpus root, making "this subtree was never traversed" observable; inert in production like every other fault point. | +| 16 | `60fc228` | RED | Two witnesses, both armed with the trace hook over fixture corpora that have no legitimate subdirectory: a foreign subtree `song-curation/extra/deep/…` and a reserved root symlinked to an external directory. Evidence in the message: both FAILED with the identical defect signature `ApplyIoError { path: ".../corpus/song-curation", op: "walk", detail: "injected fault" }` — the walk descended into / followed the symlinked reserved root. | +| 17 | `96ca755` | GREEN | The walk computes each entry's relative name first and skips the reserved root before any type check or descent (`is_dir` would follow a symlink merely to answer). Both witnesses flip to the shape law's own `CorpusTreeDisagreement` with the hook never reached; the post-walk `is_reserved` filter remains only as a redundant invariant. No refusal, schema, or ordering change. | +| 18 | (this commit) | DOCS | This evidence update + PR-body refresh (the review's non-blocking cleanup: the body still described the pre-repair head). | + +The §14 matrix is unchanged by this round: **63/63 continues to hold** +(the round-2 witnesses are additional, like the round-1 ones), and the +refusals involved are the ones the taxonomy already assigned — no new +refusal, no law change, no stop condition triggered. + +Updated totals: isolated crate suite **118 passed / 0 failed** (45 frozen +Slice 1 + 73 Slice 2); crate clippy `deny(all)` clean; fmt clean; +`cargo +1.92 check --all-targets` clean. Only the isolated crate is +touched; the workspace results recorded above are unaffected. + +Acceptance state: implementation acceptance remains **pending independent +re-review** of this round. Contract acceptance at `47e734c` is +unaffected; Slice 3, the controlled pilot, and real-/full-corpus labeling +remain BLOCKED.