diff --git a/Cargo.lock b/Cargo.lock index 27fac81..0163a05 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1405,6 +1405,7 @@ dependencies = [ "plumb-core", "serde", "serde_json", + "sha2", ] [[package]] diff --git a/crates/plumb-format/Cargo.toml b/crates/plumb-format/Cargo.toml index f8e623b..c290008 100644 --- a/crates/plumb-format/Cargo.toml +++ b/crates/plumb-format/Cargo.toml @@ -17,6 +17,7 @@ categories.workspace = true plumb-core = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +sha2 = { workspace = true } [dev-dependencies] plumb-core = { workspace = true, features = ["test-fake"] } diff --git a/crates/plumb-format/src/lib.rs b/crates/plumb-format/src/lib.rs index 4af9105..7271d0e 100644 --- a/crates/plumb-format/src/lib.rs +++ b/crates/plumb-format/src/lib.rs @@ -23,6 +23,17 @@ use std::fmt::Write as _; use plumb_core::{Severity, Violation}; use serde_json::{Value, json}; +use sha2::{Digest, Sha256}; + +/// Plumb version string embedded in the JSON envelope. +/// +/// Pinned to `plumb-format`'s `CARGO_PKG_VERSION` because the envelope +/// shape is owned by this crate. The workspace version-bumps in +/// lockstep, so this resolves to the same value as the `plumb` binary +/// in practice; sourcing it from this crate keeps the formatter +/// self-contained and avoids a needless dependency cycle through +/// `plumb-cli`. +const PLUMB_VERSION: &str = env!("CARGO_PKG_VERSION"); /// Render a slice of violations as a pretty, human-readable block. /// @@ -55,13 +66,81 @@ pub fn pretty(violations: &[Violation]) -> String { /// Render a slice of violations as canonical, pretty-printed JSON. /// +/// # Envelope +/// +/// The output is an object with these top-level fields, written in +/// alphabetical key order: +/// +/// - `plumb_version` — the `plumb-format` crate version at compile +/// time. The workspace ships every crate with the same version, so +/// this matches the `plumb` binary version too. +/// - `run_id` — a content-derived identifier of the violations payload +/// (see below). +/// - `summary` — `{ "error": N, "info": N, "total": N, "warning": N }`, +/// keys also in alphabetical order. +/// - `violations` — the violations array, sorted by +/// [`plumb_core::Violation::sort_key`]. +/// +/// The workspace enables `serde_json/preserve_order` via `schemars`, so +/// `serde_json::Map` is `IndexMap`-backed and preserves insertion +/// order. The envelope inserts keys alphabetically to keep the output +/// independent of that crate-feature toggle. +/// +/// # `run_id` derivation +/// +/// `run_id = "sha256:" + hex(Sha256(serde_json::to_vec(&sorted_violations)))` +/// +/// The hash input is the **compact** (non-pretty) JSON serialization of +/// the sorted violations array — not the pretty-printed envelope — +/// which means whitespace tweaks in the output never shift the hash, +/// and a `plumb_version` bump never shifts it either. Two runs with +/// the same violations always produce the same `run_id`; any +/// observable change in a violation flips the digest. +/// +/// The formatter re-sorts violations defensively before hashing and +/// serializing. The engine already sorts on its way out, but the +/// formatter does not depend on caller invariants. +/// /// # Errors /// /// Returns an error if serialization fails, which in practice only /// happens when a `Violation::metadata` contains a non-JSON-representable /// value. pub fn json(violations: &[Violation]) -> Result { - serde_json::to_string_pretty(violations) + let mut sorted: Vec<&Violation> = violations.iter().collect(); + sorted.sort_by(|a, b| a.sort_key().cmp(&b.sort_key())); + + let canonical = serde_json::to_vec(&sorted)?; + let run_id = format!("sha256:{}", hex_digest(&canonical)); + + // Build the envelope with alphabetically ordered keys so the + // output is stable regardless of `serde_json`'s `preserve_order` + // feature being enabled in the workspace. + let mut envelope = serde_json::Map::new(); + envelope.insert( + "plumb_version".to_owned(), + Value::String(PLUMB_VERSION.to_owned()), + ); + envelope.insert("run_id".to_owned(), Value::String(run_id)); + envelope.insert("summary".to_owned(), counts(violations)); + envelope.insert("violations".to_owned(), serde_json::to_value(&sorted)?); + serde_json::to_string_pretty(&Value::Object(envelope)) +} + +/// Hex-alphabet table used by [`hex_digest`]. +const HEX_TABLE: &[u8; 16] = b"0123456789abcdef"; + +/// Hex-encode a SHA-256 digest of `bytes` without an extra dependency. +fn hex_digest(bytes: &[u8]) -> String { + let digest = Sha256::digest(bytes); + let mut hex = String::with_capacity(digest.len() * 2); + for byte in digest { + let upper = HEX_TABLE[(byte >> 4) as usize]; + let lower = HEX_TABLE[(byte & 0x0f) as usize]; + hex.push(char::from(upper)); + hex.push(char::from(lower)); + } + hex } /// Render a slice of violations as SARIF 2.1.0. @@ -162,12 +241,14 @@ fn counts(violations: &[Violation]) -> Value { Severity::Info => info += 1, } } - json!({ - "error": err, - "warning": warn, - "info": info, - "total": violations.len(), - }) + // Insert in alphabetical order so the output is stable regardless + // of `serde_json`'s `preserve_order` feature toggle. + let mut map = serde_json::Map::new(); + map.insert("error".to_owned(), json!(err)); + map.insert("info".to_owned(), json!(info)); + map.insert("total".to_owned(), json!(violations.len())); + map.insert("warning".to_owned(), json!(warn)); + Value::Object(map) } fn summary_line(violations: &[Violation]) -> String { diff --git a/crates/plumb-format/tests/determinism.rs b/crates/plumb-format/tests/determinism.rs new file mode 100644 index 0000000..a113f92 --- /dev/null +++ b/crates/plumb-format/tests/determinism.rs @@ -0,0 +1,128 @@ +//! Determinism guarantees for `plumb-format`. +//! +//! Each formatter is a pure function of its inputs; running it three +//! times on the same input must produce byte-identical output. The +//! suite mirrors the `just determinism-check` recipe at the formatter +//! level — i.e. before the CLI ever wraps it. + +use plumb_core::{Config, PlumbSnapshot, run}; +use plumb_format::{json, mcp_compact, pretty, sarif}; + +fn fixture() -> Vec { + let snapshot = PlumbSnapshot::canned(); + let config = Config::default(); + run(&snapshot, &config) +} + +#[test] +fn json_is_byte_identical_across_runs() { + let violations = fixture(); + let a = json(&violations).expect("json serialize a"); + let b = json(&violations).expect("json serialize b"); + let c = json(&violations).expect("json serialize c"); + assert_eq!(a, b); + assert_eq!(b, c); +} + +#[test] +fn json_envelope_has_required_fields() { + let violations = fixture(); + let out = json(&violations).expect("json serialize"); + let parsed: serde_json::Value = serde_json::from_str(&out).expect("parse json"); + + let plumb_version = parsed + .get("plumb_version") + .and_then(serde_json::Value::as_str) + .expect("plumb_version present"); + assert!( + !plumb_version.is_empty(), + "plumb_version must be a non-empty string" + ); + + let run_id = parsed + .get("run_id") + .and_then(serde_json::Value::as_str) + .expect("run_id present"); + assert!( + run_id.starts_with("sha256:"), + "run_id must be prefixed with sha256:, got {run_id}" + ); + let hex = run_id.trim_start_matches("sha256:"); + assert_eq!(hex.len(), 64, "sha256 hex digest is 64 chars"); + assert!(hex.chars().all(|c| c.is_ascii_hexdigit())); + + let summary = parsed.get("summary").expect("summary present"); + for key in ["error", "warning", "info", "total"] { + assert!(summary.get(key).is_some(), "summary.{key} must be present"); + } + + let violations_value = parsed + .get("violations") + .and_then(serde_json::Value::as_array) + .expect("violations array present"); + assert_eq!(violations_value.len(), violations.len()); +} + +#[test] +fn json_run_id_changes_when_violations_change() { + let v1 = fixture(); + let mut v2 = v1.clone(); + if let Some(first) = v2.first_mut() { + first.message.push_str(" (mutated)"); + } + let a = json(&v1).expect("json serialize v1"); + let b = json(&v2).expect("json serialize v2"); + let pa: serde_json::Value = serde_json::from_str(&a).expect("parse a"); + let pb: serde_json::Value = serde_json::from_str(&b).expect("parse b"); + assert_ne!( + pa["run_id"], pb["run_id"], + "run_id must change when violations change" + ); +} + +#[test] +fn json_run_id_is_stable_under_input_reordering() { + // The formatter re-sorts defensively before hashing, so a caller + // that hands violations in a different order still produces the + // same `run_id`. This is the determinism contract. + let violations = fixture(); + if violations.len() < 2 { + return; // canned fixture too small to reorder + } + let mut reversed = violations.clone(); + reversed.reverse(); + let a = json(&violations).expect("sorted"); + let b = json(&reversed).expect("reversed"); + let pa: serde_json::Value = serde_json::from_str(&a).expect("parse a"); + let pb: serde_json::Value = serde_json::from_str(&b).expect("parse b"); + assert_eq!(pa["run_id"], pb["run_id"]); +} + +#[test] +fn pretty_is_byte_identical_across_runs() { + let violations = fixture(); + let a = pretty(&violations); + let b = pretty(&violations); + let c = pretty(&violations); + assert_eq!(a, b); + assert_eq!(b, c); +} + +#[test] +fn sarif_is_byte_identical_across_runs() { + let violations = fixture(); + let a = sarif(&violations).expect("sarif a"); + let b = sarif(&violations).expect("sarif b"); + let c = sarif(&violations).expect("sarif c"); + assert_eq!(a, b); + assert_eq!(b, c); +} + +#[test] +fn mcp_compact_is_byte_identical_across_runs() { + let violations = fixture(); + let (ta, sa) = mcp_compact(&violations); + let (tb, sb) = mcp_compact(&violations); + assert_eq!(ta, tb); + assert_eq!(sa, sb); +} diff --git a/crates/plumb-format/tests/snapshots/format_snapshots__json.snap b/crates/plumb-format/tests/snapshots/format_snapshots__json.snap index 8e2d884..b4a4ea4 100644 --- a/crates/plumb-format/tests/snapshots/format_snapshots__json.snap +++ b/crates/plumb-format/tests/snapshots/format_snapshots__json.snap @@ -2,30 +2,40 @@ source: crates/plumb-format/tests/format_snapshots.rs expression: out --- -[ - { - "rule_id": "spacing/grid-conformance", - "severity": "warning", - "message": "`html > body` has off-grid padding-top 13px; expected a multiple of 4px.", - "selector": "html > body", - "viewport": "desktop", - "rect": { - "x": 0, - "y": 0, - "width": 1280, - "height": 800 - }, - "dom_order": 2, - "fix": { - "kind": { - "kind": "css_property_replace", - "property": "padding-top", - "from": "13px", - "to": "12px" +{ + "plumb_version": "0.0.1", + "run_id": "sha256:53a662be238be796f9a08b74adb05e4020dd3d9f653e9bf91a7fb5b1c7a274c0", + "summary": { + "error": 0, + "info": 0, + "total": 1, + "warning": 1 + }, + "violations": [ + { + "rule_id": "spacing/grid-conformance", + "severity": "warning", + "message": "`html > body` has off-grid padding-top 13px; expected a multiple of 4px.", + "selector": "html > body", + "viewport": "desktop", + "rect": { + "x": 0, + "y": 0, + "width": 1280, + "height": 800 }, - "description": "Snap `padding-top` to the nearest spacing-grid value (12px).", - "confidence": "medium" - }, - "doc_url": "https://plumb.aramhammoudeh.com/rules/spacing-grid-conformance" - } -] + "dom_order": 2, + "fix": { + "kind": { + "kind": "css_property_replace", + "property": "padding-top", + "from": "13px", + "to": "12px" + }, + "description": "Snap `padding-top` to the nearest spacing-grid value (12px).", + "confidence": "medium" + }, + "doc_url": "https://plumb.aramhammoudeh.com/rules/spacing-grid-conformance" + } + ] +} diff --git a/crates/plumb-format/tests/snapshots/format_snapshots__mcp_compact_structured.snap b/crates/plumb-format/tests/snapshots/format_snapshots__mcp_compact_structured.snap index 5e5fb8d..8b50172 100644 --- a/crates/plumb-format/tests/snapshots/format_snapshots__mcp_compact_structured.snap +++ b/crates/plumb-format/tests/snapshots/format_snapshots__mcp_compact_structured.snap @@ -32,8 +32,8 @@ expression: structured ], "counts": { "error": 0, - "warning": 1, "info": 0, - "total": 1 + "total": 1, + "warning": 1 } }