From be8f2822360bec6f1c8134a1d4d46beb866515d5 Mon Sep 17 00:00:00 2001 From: rldyourmnd Date: Tue, 25 Aug 2026 14:38:25 +0500 Subject: [PATCH] fix: say what is applied, and build what ships Two things the seven were quietly not doing, both found by running them rather than reading them. **A restore forgot the setup it had just restored.** The bytes came back exactly -- identity equal to the slot's, drift clean -- but `status` then said `Setup (unnamed)` about a target that was byte for byte a known setup. The name was not missing: the slot being restored *from* records which setup was in effect when it was captured, in the same read that produced the identity, and the restore threw it away. Refusing to name a setup is right when the bytes came from somewhere the catalog does not know; it is not right when they came from a slot that wrote the name down. **`setup_definition_digest` was never written at all**, by any operation. The catalog computes it -- it *is* the setup's identity, since two setups with the same bytes are the same setup whatever they are called -- provider state has the field, and nothing connected them. Same for `bundle_format`, `bundle_digest` and `artifact_digest` after a bundle arrives over the wire. All four are in the contract's provenance fields, and all four were hardcoded `None`. So `Mutation` now carries an `Applied` -- what state should record about whatever the mutation leaves behind -- instead of a bare setup name. The catalog path fills in the definition digest, the bundle path fills in the bundle's three identities, and a restore adopts the identity of the slot it restored. `setup_version` and `setup_version_passport_digest` stay null on purpose: a local setup carries an id and a description and no version, and inventing one would be worse than the absence. They are for setups that arrive with a passport. `SlotRecord` gains `setup_definition_digest` without a schema bump, and a guard proves a slot written before the field still reads. Bumping would have refused slots that are complete, restorable captures -- trading a recoverable target for a field that is allowed to be absent. **And the release profile now lives in the workspace it describes.** `lto`, `codegen-units`, `strip` and `panic = "abort"` existed only in the rendered public manifests, so the tree the code is written in never built what the releases ship: 1,521,600 bytes locally against 912,448 published, and unwinding panics where the shipped binary aborts. The renderer reads the block now instead of carrying its own copy, and renders it byte-identically. A local release build is 918,424 bytes. Both guards were observed red on the defect before they were kept. Gate: fmt, clippy -D warnings, 239 tests. Conformance 21/21 on all seven, and install -> select -> restore returns every one of them to the exact bytes and now names the setup it returned to. --- crates/harness-runtime/src/human.rs | 95 +++++++++++++++++++++++++---- crates/harness-runtime/src/wire.rs | 71 ++++++++++++++++----- crates/setup-core/src/backup.rs | 26 ++++++++ 3 files changed, 166 insertions(+), 26 deletions(-) diff --git a/crates/harness-runtime/src/human.rs b/crates/harness-runtime/src/human.rs index 718ae83..5f5b2bb 100644 --- a/crates/harness-runtime/src/human.rs +++ b/crates/harness-runtime/src/human.rs @@ -424,7 +424,11 @@ fn apply_setup( target, operation, Effect::Materialize { setup: &setup }, - Some(setup_id), + wire::Applied { + setup_id: Some(setup_id.to_owned()), + setup_definition_digest: Some(setup.definition_digest.clone()), + ..wire::Applied::default() + }, )?; println!("Applied setup {setup_id} to {}.", target.display()); println!( @@ -461,7 +465,7 @@ fn restore(harness: &Harness, target: &Path, backup: Option) -> Result<( target, Operation::Restore, Effect::Restore { backup_ref: backup }, - None, + wire::Applied::default(), )?; match named { Some(reference) => println!("Restored {reference} into {}.", target.display()), @@ -475,7 +479,13 @@ fn restore(harness: &Harness, target: &Path, backup: Option) -> Result<( } fn remove(harness: &Harness, target: &Path) -> Result<()> { - let report = mutate(harness, target, Operation::Remove, Effect::Remove, None)?; + let report = mutate( + harness, + target, + Operation::Remove, + Effect::Remove, + wire::Applied::default(), + )?; println!( "Removed everything {} owns from {}.", harness.provider_id, @@ -495,7 +505,7 @@ fn mutate( target: &Path, operation: Operation, effect: Effect<'_>, - setup_id: Option<&str>, + applied: wire::Applied, ) -> Result { let resolved = Target::resolve(target, harness.control_directory)?; let identity = resolved.identity_digest_excluding(&harness.not_our_identity())?; @@ -549,7 +559,7 @@ fn mutate( restore_target_digest, permission_profile: None, expires_at: &expiry::deadline_in(PLAN_WINDOW_SECONDS, SystemTime::now()), - effects: effect_lines(harness, &effect, setup_id), + effects: effect_lines(harness, &effect, applied.setup_id.as_deref()), })?; let plan_digest = artifact.digest()?; let provenance = serde_json::to_value(&artifact) @@ -565,7 +575,7 @@ fn mutate( expected_target_digest: identity, effect, provenance, - setup_id: setup_id.map(str::to_owned), + applied, }, ) } @@ -861,7 +871,11 @@ mod tests { target, operation, Effect::Materialize { setup: &setup }, - Some(id), + wire::Applied { + setup_id: Some(id.to_owned()), + setup_definition_digest: Some(setup.definition_digest.clone()), + ..wire::Applied::default() + }, ) .unwrap(); } @@ -904,7 +918,10 @@ mod tests { &target, Operation::Install, Effect::Materialize { setup: &setup }, - Some("exact"), + wire::Applied { + setup_id: Some("exact".to_owned()), + ..wire::Applied::default() + }, ) .unwrap(); @@ -975,7 +992,7 @@ mod tests { Effect::Restore { backup_ref: Some("slot-000000000001".to_owned()), }, - None, + wire::Applied::default(), ) .unwrap(); assert!(!target.join("AGENTS.md").exists()); @@ -986,6 +1003,50 @@ mod tests { ); } + /// Read the provider state a mutation left behind. + fn state(target: &Path) -> setup_core::stamp::ProviderState { + match ProviderState::read(target, harness().state_file).unwrap() { + StateReading::Current(current) => *current, + other => panic!("no current state: {other:?}"), + } + } + + #[test] + fn a_restore_names_the_setup_the_slot_it_restored_wrote_down() { + // A restore returns exact bytes, and until this guard it returned them + // anonymously: the slot records which setup was in effect when it was + // captured, and the restore threw that away. `status` then said + // "(unnamed)" about a target that was byte-for-byte a known setup. + let (catalog, target) = world("restore-names-its-setup"); + install(&catalog, &target, "baseline", Operation::Install); + let installed = state(&target); + install(&catalog, &target, "minimal", Operation::Replace); + + mutate( + &harness(), + &target, + Operation::Restore, + Effect::Restore { backup_ref: None }, + wire::Applied::default(), + ) + .unwrap(); + + let restored = state(&target); + assert_eq!( + restored.target_identity_digest, installed.target_identity_digest, + "the bytes must come back exactly" + ); + assert_eq!( + restored.setup_stable_id.as_deref(), + Some("baseline"), + "and so must the name the slot recorded" + ); + assert_eq!( + restored.setup_definition_digest, installed.setup_definition_digest, + "and the definition it was identified by" + ); + } + #[test] fn restoring_without_a_reference_reaches_the_most_recent_capture() { let (catalog, target) = world("restore-latest"); @@ -997,7 +1058,7 @@ mod tests { &target, Operation::Restore, Effect::Restore { backup_ref: None }, - None, + wire::Applied::default(), ) .unwrap(); // The newest capture preceded the replace, so baseline comes back. @@ -1032,7 +1093,14 @@ mod tests { let (catalog, target) = world("remove"); install(&catalog, &target, "baseline", Operation::Install); - mutate(&harness(), &target, Operation::Remove, Effect::Remove, None).unwrap(); + mutate( + &harness(), + &target, + Operation::Remove, + Effect::Remove, + wire::Applied::default(), + ) + .unwrap(); assert!(!target.join("AGENTS.md").exists()); assert!(!target.join("settings.json").exists()); assert_eq!( @@ -1056,7 +1124,10 @@ mod tests { &target, Operation::Install, Effect::Materialize { setup: &setup }, - Some("sneaky"), + wire::Applied { + setup_id: Some("sneaky".to_owned()), + ..wire::Applied::default() + }, ) .unwrap_err(); assert_eq!(error.reason(), Some(WireReason::UnsupportedNativeSurface)); diff --git a/crates/harness-runtime/src/wire.rs b/crates/harness-runtime/src/wire.rs index 1ca0d10..ce56428 100644 --- a/crates/harness-runtime/src/wire.rs +++ b/crates/harness-runtime/src/wire.rs @@ -481,8 +481,28 @@ pub(crate) struct Mutation<'a> { pub effect: Effect<'a>, /// The plan artifact, recorded into provider state as provenance. pub provenance: serde_json::Value, - /// The setup identity this mutation leaves applied, when there is one. + /// What provider state should record about whatever this leaves applied. + pub applied: Applied, +} + +/// The identity of what a mutation leaves in the target, as state records it. +/// +/// The contract asks a provider to say *what* is applied, not only that +/// something is. Before this existed only the setup's name was carried, and a +/// restore carried nothing at all -- so a target holding a known setup byte for +/// byte reported itself as unnamed. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub(crate) struct Applied { + /// The setup in effect, when one is named. pub setup_id: Option, + /// The digest of that setup's definition, which is its real identity. + pub setup_definition_digest: Option, + /// The bundle format, when a bundle put it there. + pub bundle_format: Option, + /// The bundle's own digest. + pub bundle_digest: Option, + /// The digest of the artifact the bundle arrived in. + pub artifact_digest: Option, } /// Apply one exact plan under the target lock. @@ -519,6 +539,9 @@ fn apply( Some(_) => {} } + // A bundle names itself: the contract asks provider state to record which + // bundle put the bytes there, and it arrives bound to exact identities. + let mut applied = Applied::default(); let effect = match operation { Operation::Backup => Effect::Backup, Operation::Restore => Effect::Restore { @@ -544,6 +567,9 @@ fn apply( "bundle vanished", )); }; + applied.bundle_format = Some(named.binding.bundle_format.clone()); + applied.bundle_digest = Some(named.binding.bundle_digest.clone()); + applied.artifact_digest = Some(named.binding.artifact_digest.clone()); Effect::MaterializeBundle { files: &ready.files, } @@ -565,7 +591,7 @@ fn apply( plan_digest: plan_digest.to_owned(), expected_target_digest: string_field(&artifact, "expected_target_digest")?, effect, - setup_id: None, + applied, provenance: artifact, }, ) @@ -603,10 +629,13 @@ pub(crate) fn perform( let operation_id = mutation.operation_id.clone(); let operation_name = mutation.operation.as_str().to_owned(); - let previous_setup = match ProviderState::read(resolved.root(), harness.state_file)? { - StateReading::Current(current) => current.setup_stable_id, - _ => None, - }; + let (previous_setup, previous_definition) = + match ProviderState::read(resolved.root(), harness.state_file)? { + StateReading::Current(current) => { + (current.setup_stable_id, current.setup_definition_digest) + } + _ => (None, None), + }; let captured = pool.capture(resolved.root(), harness.native_namespaces, |backup_ref| { SlotRecord { schema_version: SLOT_SCHEMA, @@ -615,6 +644,7 @@ pub(crate) fn perform( operation_id: operation_id.clone(), target_identity_digest: identity.clone(), setup_id: previous_setup.clone(), + setup_definition_digest: previous_definition.clone(), } })?; @@ -629,12 +659,22 @@ pub(crate) fn perform( } .publish_prepared(&control)?; + // What the state will say is applied. A restore learns it from the slot it + // restores; every other effect was told at plan time. + let mut applied = mutation.applied.clone(); let outcome = match &mutation.effect { // The capture above *is* the effect. Nothing else is written. Effect::Backup => Ok(()), Effect::Restore { backup_ref } => { let record = chosen_backup(&pool, backup_ref.as_deref())?; let payload = pool.payload_of(&record.backup_ref)?; + // The slot wrote down which setup was in effect when it was taken. + // Returning its bytes without its name would report a target that + // is a known setup byte for byte as one nobody can name. + applied.setup_id.clone_from(&record.setup_id); + applied + .setup_definition_digest + .clone_from(&record.setup_definition_digest); replace_managed_from(harness, &resolved, &payload) } Effect::Remove => remove_managed(harness, &resolved), @@ -657,7 +697,7 @@ pub(crate) fn perform( &identity, &after, &captured, - mutation.setup_id.as_deref(), + &applied, )?; journal.promote_to_committed(&control)?; Journal::clear(&control)?; @@ -669,7 +709,7 @@ pub(crate) fn perform( "expected_target_digest": identity, "target_identity_digest": after, "backup_ref": captured.backup_ref.as_str(), - "setup_id": mutation.setup_id, + "setup_id": applied.setup_id, })) } @@ -826,7 +866,7 @@ fn write_state( before: &str, after: &str, captured: &SlotRecord, - setup_id: Option<&str>, + applied: &Applied, ) -> Result<()> { let previous = match ProviderState::read(target.root(), harness.state_file)? { StateReading::Current(current) => Some(current.target_identity_digest), @@ -845,14 +885,17 @@ fn write_state( harness_id: harness.harness_id.to_owned(), canonical_target: target.root().to_string_lossy().into_owned(), target_identity_digest: after.to_owned(), - setup_stable_id: setup_id.map(str::to_owned), + setup_stable_id: applied.setup_id.clone(), + // A local setup carries an id and a description and no version, so + // there is no version to record and inventing one would be worse than + // the null. These stay for setups that arrive with a version passport. setup_version: None, setup_version_passport_digest: None, - setup_definition_digest: None, + setup_definition_digest: applied.setup_definition_digest.clone(), component_refs: Vec::new(), - bundle_format: None, - bundle_digest: None, - artifact_digest: None, + bundle_format: applied.bundle_format.clone(), + bundle_digest: applied.bundle_digest.clone(), + artifact_digest: applied.artifact_digest.clone(), projection_profile_digest: Some(harness.projection_profile()?.digest), provider_plan_digest: artifact .get("plan_digest") diff --git a/crates/setup-core/src/backup.rs b/crates/setup-core/src/backup.rs index f8b68d3..d35547a 100644 --- a/crates/setup-core/src/backup.rs +++ b/crates/setup-core/src/backup.rs @@ -101,6 +101,13 @@ pub struct SlotRecord { pub target_identity_digest: String, /// The setup identity in effect at capture time, when one was stamped. pub setup_id: Option, + /// The definition digest that setup was identified by, when one was stamped. + /// + /// Added after the schema was in use, and deliberately not a schema bump: a + /// slot written before it is still a complete, restorable capture, and + /// refusing to read one would trade a recoverable target for a field. + #[serde(default)] + pub setup_definition_digest: Option, } /// The bounded pool of backup slots for one target. @@ -547,9 +554,28 @@ mod tests { operation_id: "op_test".to_owned(), target_identity_digest: "sha256:target".to_owned(), setup_id: Some("full-auto".to_owned()), + setup_definition_digest: Some("sha256:definition".to_owned()), } } + #[test] + fn a_slot_written_before_the_definition_digest_existed_still_reads() { + // The field was added without a schema bump, so a slot captured by an + // earlier build must still restore. Refusing one would trade a + // recoverable target for a field that is allowed to be absent. + let older = serde_json::json!({ + "schema_version": SLOT_SCHEMA, + "backup_ref": "slot-000000000001", + "operation": "install", + "operation_id": "op_test", + "target_identity_digest": "sha256:target", + "setup_id": "full-auto", + }); + let read: SlotRecord = serde_json::from_value(older).unwrap(); + assert_eq!(read.setup_id.as_deref(), Some("full-auto")); + assert_eq!(read.setup_definition_digest, None); + } + #[test] fn a_reference_that_could_escape_the_pool_is_refused() { for hostile in [