diff --git a/crates/context/src/component_review/history.rs b/crates/context/src/component_review/history.rs index 1c8c7170c..a747e6d1f 100644 --- a/crates/context/src/component_review/history.rs +++ b/crates/context/src/component_review/history.rs @@ -72,7 +72,10 @@ pub fn between(previous: &Value, current: &Value) -> Value { .filter(|p| before.get(*p) != after.get(*p)) .cloned() .collect(); - let unchanged = prior["revision"] == component["revision"]; + let source_changed = prior["revision"] != component["revision"]; + let unchanged = !source_changed || (current["visualApprovalCarry"][id]["basis"] == "identical-native-captures-v1" + && current["visualApprovalCarry"][id]["fromPacketRevision"] == previous["packet"]["revision"]); + let mut reasons = Vec::new(); if !files.is_empty() { reasons.push("files"); @@ -83,7 +86,7 @@ pub fn between(previous: &Value, current: &Value) -> Value { if !unchanged && reasons.is_empty() { reasons.push("definition"); } - json!({"kind":if unchanged{"unchanged"}else{"changed"},"files":files,"reasons":reasons}) + json!({"kind":if unchanged{"unchanged"}else{"changed"},"files":files,"reasons":reasons,"sourceChanged":source_changed}) } else { json!({"kind":"added","files":[],"reasons":[]}) }; diff --git a/crates/context/src/component_review/mod.rs b/crates/context/src/component_review/mod.rs index a1ca5d82c..6b9823a72 100644 --- a/crates/context/src/component_review/mod.rs +++ b/crates/context/src/component_review/mod.rs @@ -4,9 +4,10 @@ mod history; mod manifest; mod server; mod store; -pub mod verify; #[cfg(test)] mod tests; +pub mod verify; +mod visual_approval; use impeccable_common::Io; use serde_json::json; use std::path::PathBuf; @@ -78,13 +79,17 @@ pub fn run_with_capturer( io.out(&format!("{}\n", receipt)); Ok(()) } - Some("serve") | Some("status") => { + Some("serve") | Some("status") | Some("refresh-approvals") => { let id = arg(args, "--session").ok_or("needs --session ")?; if id.len() != 64 || !id.bytes().all(|b| b.is_ascii_hexdigit()) { return Err("invalid session id".into()); } let dir = store.join(id); - if args[0] == "serve" { + if args[0] == "refresh-approvals" { + let count=store::refresh_approvals(&dir)?; + io.out(&format!("{}\n",json!({"carried":count}))); + Ok(()) + } else if args[0] == "serve" { let port = arg(args, "--port") .unwrap_or_else(|| "0".into()) .parse::() @@ -102,7 +107,7 @@ pub fn run_with_capturer( Ok(()) } } - _ => Err("usage: impeccable component-review prepare|capture|verify --manifest | serve --session [--port 0] | status --session [--store ]".into()) + _ => Err("usage: impeccable component-review prepare|capture|verify --manifest | serve --session [--port 0] | status|refresh-approvals --session [--store ]".into()) } })(); match result { diff --git a/crates/context/src/component_review/store.rs b/crates/context/src/component_review/store.rs index 9c0eced6a..2ffe922ff 100644 --- a/crates/context/src/component_review/store.rs +++ b/crates/context/src/component_review/store.rs @@ -1,5 +1,5 @@ use super::manifest::{digest, freeze, relative, string, valid_box}; -use serde_json::{Value, json}; +use serde_json::{json, Value}; use std::{ fs, io::Write, @@ -216,12 +216,10 @@ fn prepare_bound( packet["round"] = json!(round); // URLs include the frozen revision, so a new round cannot silently replace old preview pixels. let prefix = format!("/files/{rev}/"); - packet["comp"]["url"] = json!( - packet["comp"]["url"] - .as_str() - .unwrap() - .replacen("/files/", &prefix, 1) - ); + packet["comp"]["url"] = json!(packet["comp"]["url"] + .as_str() + .unwrap() + .replacen("/files/", &prefix, 1)); for c in packet["components"].as_array_mut().unwrap() { for key in ["preview", "context", "thumbnail"] { if let Some(url) = c[key]["url"].as_str() { @@ -257,6 +255,9 @@ fn prepare_bound( } } let mut state = json!({"schemaVersion":1,"contentRevision":content_revision,"project":project,"packet":packet,"files":hashes,"sources":sources,"capture":capture,"draft":draft,"receipt":null}); + if let Some(previous) = &old { + super::visual_approval::carry(previous, &mut state, &blobs); + } state["history"] = old .as_ref() .map(|previous| super::history::between(previous, &state)) @@ -375,3 +376,30 @@ pub fn submit(dir: &Path, body: &Value) -> Result { // current.json is the authoritative atomic commit; a receipt export is not approval authority. Ok(receipt) } + +/// Update only an unsubmitted draft, retaining packet/source identity and old receipts. +pub fn refresh_approvals(dir: &Path) -> Result { + let _guard = lock(dir)?; + let mut state = read(&dir.join("current.json"))?; + if !state["receipt"].is_null() { + return Ok(0); + } + sources_current(&state)?; + let Some(rev) = state["history"]["packet"]["revision"].as_str() else { + return Ok(0); + }; + if rev.len() != 64 || !rev.bytes().all(|b| b.is_ascii_hexdigit()) { + return Err("invalid previous revision".into()); + } + let previous = read(&dir.join(format!("revisions/{rev}.json")))?; + if previous["packet"]["revision"] != rev { + return Err("previous revision mismatch".into()); + } + let count = super::visual_approval::carry(&previous, &mut state, &dir.join("blobs")); + let history = super::history::between(&previous, &state); + if count > 0 || state["history"] != history { + state["history"] = history; + write(&dir.join("current.json"), &state)?; + } + Ok(count) +} diff --git a/crates/context/src/component_review/tests.rs b/crates/context/src/component_review/tests.rs index 3a22ce86c..36dbac04c 100644 --- a/crates/context/src/component_review/tests.rs +++ b/crates/context/src/component_review/tests.rs @@ -525,3 +525,119 @@ fn shared_target_changes_invalidate_other_isolated_components() { input["stage"] = Value::Null; assert!(manifest::freeze(&f.project,&input).is_err()); } + +#[test] +fn visual_approvals_survive_shared_source_edits_but_not_changed_scope_or_pixels() { + struct Renderer(&'static [u8]); + impl super::capture::ComponentCapturer for Renderer { + fn capture( + &mut self, + packet: &mut Value, + _: &std::collections::BTreeMap>, + ) -> Result { + packet["components"][1]["preview"] = json!({"kind":"image","sourceKind":"page","url":"/files/_review_captures/control.png"}); + Ok(super::capture::CapturedPreviews { + files: std::collections::BTreeMap::from([( + "_review_captures/control.png".into(), + self.0.to_vec(), + )]), + evidence: json!({"schema":"native-component-previews-v1","components":[{"id":"art"},{"id":"control","views":{"preview":{"kind":"static-code","entry":"control.html","screenshotSha256":manifest::digest(self.0),"viewport":{"width":100,"height":100,"dpr":1}}}}]}), + }) + } + } + let f = Fixture::new(); + let dir = store::prepare_captured( + &f.store, + &f.project, + &f.manifest(), + Some(&mut Renderer(b"same pixels")), + ) + .unwrap(); + let before = store::read(&dir.join("current.json")).unwrap(); + store::submit(&dir, &approve(&before)).unwrap(); + fs::write( + f.project.join("shared.css"), + b"button{color:red} .unrelated{color:blue}", + ) + .unwrap(); + assert!(store::sources_current(&before).is_err()); + store::prepare_captured( + &f.store, + &f.project, + &f.manifest(), + Some(&mut Renderer(b"same pixels")), + ) + .unwrap(); + let mut after = store::read(&dir.join("current.json")).unwrap(); + assert_ne!( + before["packet"]["components"][1]["revision"], + after["packet"]["components"][1]["revision"] + ); + assert_eq!(after["draft"]["decisions"]["control"]["action"], "approve"); + assert_eq!( + after["visualApprovalCarry"]["control"]["basis"], + "identical-native-captures-v1" + ); + assert!(after["receipt"].is_null()); + assert_eq!(after["history"]["changes"]["control"]["kind"], "unchanged"); + assert_eq!(after["history"]["changes"]["control"]["sourceChanged"], true); + assert_eq!(after["history"]["changes"]["control"]["carried"], true); + // Existing pending packets can gain carry-forward without changing their revision. + after["draft"]["decisions"] + .as_object_mut() + .unwrap() + .remove("control"); + store::write(&dir.join("current.json"), &after).unwrap(); + assert_eq!(store::refresh_approvals(&dir).unwrap(), 1); + assert_eq!(store::refresh_approvals(&dir).unwrap(), 0); + let carried = store::read(&dir.join("current.json")).unwrap(); + assert_eq!(carried["packet"], after["packet"]); + assert_eq!(carried["sources"], after["sources"]); + // Changed scope, comp, preview bytes, absent proof and corrupt blobs fail closed. + let previous = store::read(&dir.join(format!( + "revisions/{}.json", + before["packet"]["revision"].as_str().unwrap() + ))) + .unwrap(); + for field in ["box", "medium", "note", "context"] { + let mut changed = after.clone(); + changed["packet"]["components"][1][field] = json!("changed"); + assert_eq!( + super::visual_approval::carry(&previous, &mut changed, &dir.join("blobs")), + 0, + "{field}" + ); + } + let mut changed = after.clone(); + changed["capture"] = Value::Null; + assert_eq!( + super::visual_approval::carry(&previous, &mut changed, &dir.join("blobs")), + 0 + ); + let mut changed = after.clone(); + changed["draft"]["decisions"]["control"] = json!({"action":"revise"}); + assert_eq!( + super::visual_approval::carry(&previous, &mut changed, &dir.join("blobs")), + 0 + ); + let mut changed = after.clone(); + changed["capture"]["components"][1]["views"]["preview"]["viewport"]["width"] = json!(200); + assert_eq!(super::visual_approval::carry(&previous, &mut changed, &dir.join("blobs")), 0); + let mut unsubmitted = previous.clone(); + unsubmitted["receipt"] = Value::Null; + assert_eq!(super::visual_approval::carry(&unsubmitted, &mut after.clone(), &dir.join("blobs")), 0); + let pixel_path = dir.join("blobs").join(manifest::digest(b"same pixels")); + fs::write(&pixel_path, b"corrupted capture").unwrap(); + assert_eq!(super::visual_approval::carry(&previous, &mut after.clone(), &dir.join("blobs")), 0); + fs::write(&pixel_path, b"same pixels").unwrap(); + store::submit(&dir, &approve(&carried)).unwrap(); + store::prepare_captured( + &f.store, + &f.project, + &f.manifest(), + Some(&mut Renderer(b"different pixels")), + ) + .unwrap(); + let changed = store::read(&dir.join("current.json")).unwrap(); + assert!(changed["draft"]["decisions"]["control"].is_null()); +} diff --git a/crates/context/src/component_review/visual_approval.rs b/crates/context/src/component_review/visual_approval.rs new file mode 100644 index 000000000..48719d39a --- /dev/null +++ b/crates/context/src/component_review/visual_approval.rs @@ -0,0 +1,116 @@ +//! Visual decisions bind reviewed pixels and scope; source integrity remains separate. +use super::manifest::digest; +use serde_json::{json, Value}; +use std::path::Path; + +fn identity(state: &Value, component: &Value, blobs: &Path) -> Option { + if state["capture"]["schema"] != "native-component-previews-v1" { + return None; + } + let evidence = state["capture"]["components"] + .as_array()? + .iter() + .find(|c| c["id"] == component["id"])?; + let prefix = format!("/files/{}/", state["packet"]["revision"].as_str()?); + let mut definition = component.clone(); + definition.as_object_mut()?.remove("revision"); + // Dependency paths remain part of scope; their bytes are checked by sources_current. + let mut views = serde_json::Map::new(); + for key in ["preview", "context", "thumbnail", "comp"] { + let view = if key == "comp" { + &state["packet"]["comp"] + } else { + &component[key] + }; + if view.is_null() { + continue; + } + let path = view["url"].as_str()?.strip_prefix(&prefix)?; + let hash = state["files"][path].as_str()?; + if hash.len() != 64 || !hash.bytes().all(|b| b.is_ascii_hexdigit()) { + return None; + } + if digest(&std::fs::read(blobs.join(hash)).ok()?) != hash { + return None; + } + let mut scoped = view.clone(); + scoped.as_object_mut()?.remove("url"); + scoped["sha256"] = json!(hash); + if key != "comp" { + definition[key] = scoped.clone(); + } + let proof = &evidence["views"][key]; + if view["sourceKind"] == "page" { + if proof["kind"] != "static-code" + || proof["screenshotSha256"] != hash + || !proof["viewport"].is_object() + { + return None; + } + // Keep capture geometry and representation. Whole-document hashes and + // resource hashes are deliberately not visual approval identities. + for field in [ + "kind", + "entry", + "viewport", + "box", + "cropMethod", + "reducedMotion", + "svgElements", + "rasterElements", + "semanticControls", + ] { + scoped[field] = proof[field].clone(); + } + } + views.insert(key.into(), scoped); + } + Some( + json!({"component":definition,"views":views,"stage":state["packet"]["stage"],"request":state["packet"]["id"]}), + ) +} + +/// Rebind a submitted visual approval to an unchanged captured presentation. +/// Never manufacture a receipt or suppress a source change. +pub fn carry(previous: &Value, current: &mut Value, blobs: &Path) -> usize { + if previous["receipt"]["captureVerified"] != true + || previous["receipt"]["submission"]["packetRevision"] != previous["packet"]["revision"] + { + return 0; + } + let Some(components) = current["packet"]["components"].as_array().cloned() else { + return 0; + }; + let mut count = 0; + for component in components { + let Some(id) = component["id"].as_str() else { + continue; + }; + // Preserve all new human work, including needs-work decisions. + if !current["draft"]["decisions"][id].is_null() { + continue; + } + let Some(prior) = previous["packet"]["components"] + .as_array() + .and_then(|cs| cs.iter().find(|c| c["id"] == id)) + else { + continue; + }; + let decision = &previous["receipt"]["submission"]["decisions"][id]; + if decision["action"] != "approve" || decision["revision"] != prior["revision"] { + continue; + } + let Some(before) = identity(previous, prior, blobs) else { + continue; + }; + if identity(current, &component, blobs).as_ref() != Some(&before) { + continue; + } + let mut carried = decision.clone(); + carried["revision"] = component["revision"].clone(); + current["draft"]["decisions"][id] = carried; + current["visualApprovalCarry"][id] = json!({"fromPacketRevision":previous["packet"]["revision"],"fromComponentRevision":prior["revision"],"basis":"identical-native-captures-v1"}); + count += 1; + } + count +} diff --git a/docs/CLI-CONTRACT.md b/docs/CLI-CONTRACT.md index a87b4ba9f..13c91c56a 100644 --- a/docs/CLI-CONTRACT.md +++ b/docs/CLI-CONTRACT.md @@ -1868,3 +1868,18 @@ The packet and evidence contract is documented in For native comp-led runs, a verified assembled-page review can resolve a text-style contradiction against the original comp. The renderer reads the private native review session, binds the approved screenshot and source bytes, and compares the disputed text region against that human-approved rendering at the current desktop breakpoint. The original comp scores remain in the report; accepted region IDs and the review revision are recorded separately under `humanTextReview`. This does not waive missing regions, overall comp fidelity, controls, raster production/placement, source integrity, or native capture checks. Changed reviewed sources, an unreviewed current dependency, a stale receipt, a partial component capture, or a changed approved image cannot provide this evidence. A host capture service may select its private native review session with `IMPECCABLE_CAPTURE_REVIEW_SESSION` at service startup; this is not a caller-supplied capture parameter. Standalone native builds use the matching local component-review session. + + +### Visual approval carry-forward + +Native component review carries a submitted approval when all displayed image +bytes and review scope are identical, even if shared source files changed. +The comparison includes the approved comp, region, representation, declared +component definition, and native capture geometry. Changed or unverified views +remain unapproved. Source revisions and integrity validation remain independent; +carrying a visual decision never creates a submitted receipt or accepts stale files. + +`component-review refresh-approvals --session --store ` applies +this rule to an existing unsubmitted draft. It verifies stored image blobs and +current source integrity, preserves any new reviewer decisions, records the +prior approval revision, and leaves packet revisions and old receipts unchanged.