From 238d0ecaa8f75bcda0ddc43f50d2a5850e769086 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Wed, 1 Jul 2026 06:15:57 +0200 Subject: [PATCH] fix(validate): make validate and validate --direct produce identical output (REQ-241, #620) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user reported `rivet validate` and `rivet validate --direct` returning different results on the same project — one flagging issues the other didn't. The substantive divergence (self-satisfying links counted as closing a traceability rule) was fixed in #627; this closes the last residual difference. The default (salsa) and --direct (library) paths collect artifacts in different orders, so `check_lifecycle_completeness` returned the lifecycle gaps in different order and the `--explain ` hint (which used `.first()`) named a different example artifact per path. Sort the gaps by artifact id so the printed gap list AND the hint are deterministic and identical across both paths. Verified byte-identical output between the two paths on both the reporter's self-link repro and rivet's own 940-artifact project. Regression test validate_and_direct_produce_identical_output (proven to fail without the sort). Implements: REQ-241 Verifies: REQ-241 Refs: REQ-089 Co-Authored-By: Claude Opus 4.8 --- artifacts/requirements.yaml | 2 +- rivet-cli/src/main.rs | 9 ++++- rivet-cli/tests/cli_commands.rs | 62 +++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/artifacts/requirements.yaml b/artifacts/requirements.yaml index 48954181..52e9f97b 100644 --- a/artifacts/requirements.yaml +++ b/artifacts/requirements.yaml @@ -7601,7 +7601,7 @@ artifacts: - id: REQ-241 type: requirement title: validate and validate --direct must produce identical results - status: proposed + status: verified description: "A user reported `rivet validate` and `rivet validate --direct` returning different results on the same project (one with errors, one without). The self-link case was fixed in #627, but the reporter's broader example (salsa path vs library path divergence) may remain. Reproduce and close the remaining divergence so the two paths never disagree. #620, relates to REQ-089." provenance: created-by: ai-assisted diff --git a/rivet-cli/src/main.rs b/rivet-cli/src/main.rs index 86819c39..8d2f0a60 100644 --- a/rivet-cli/src/main.rs +++ b/rivet-cli/src/main.rs @@ -5810,8 +5810,15 @@ fn cmd_validate( .filter(|a| !a.id.contains(':')) .cloned() .collect(); - let lifecycle_gaps = + let mut lifecycle_gaps = rivet_core::lifecycle::check_lifecycle_completeness(&all_artifacts, &schema, &graph); + // Sort by artifact id so the printed gap list AND the `--explain ` + // hint (which uses `.first()`) are deterministic regardless of the + // upstream artifact iteration order. Without this the default (salsa) + // and `--direct` paths, which collect artifacts in different orders, + // name a different example artifact in the hint — the last observable + // divergence between the two validate paths (#620, REQ-241). + lifecycle_gaps.sort_by(|a, b| a.artifact_id.cmp(&b.artifact_id)); // REQ-082: external (prefixed `prefix:ID`) artifacts are loaded into // the store only so the consumer's `prefix:ID` cross-links resolve. diff --git a/rivet-cli/tests/cli_commands.rs b/rivet-cli/tests/cli_commands.rs index d1387f5f..f766be33 100644 --- a/rivet-cli/tests/cli_commands.rs +++ b/rivet-cli/tests/cli_commands.rs @@ -679,6 +679,68 @@ fn validate_surfaces_parse_error_on_malformed_artifact_file() { ); } +/// #620 (REQ-241): `rivet validate` (default salsa path) and +/// `rivet validate --direct` (library path) must produce IDENTICAL results +/// on the same project. A user reported them disagreeing — one flagging +/// errors the other didn't. The substantive divergence (self-links counted +/// as closing a rule) was fixed in #627; the last residual difference was +/// the `--explain ` hint naming a different example artifact because the +/// two paths collect artifacts in different orders. Both are now covered. +/// +/// rivet: verifies REQ-241 +#[test] +fn validate_and_direct_produce_identical_output() { + let tmp = tempfile::tempdir().expect("create temp dir"); + let dir = tmp.path(); + let dirs = dir.to_str().unwrap(); + assert!( + Command::new(rivet_bin()) + .args(["init", "--preset", "dev", "--dir", dirs]) + .output() + .expect("init") + .status + .success() + ); + // A project that exercises coverage gaps, a self-satisfying link (the + // #627 case), and multiple artifacts (so any order-dependent hint or + // list would diverge between the two paths). + std::fs::write( + dir.join("artifacts").join("reqs.yaml"), + "artifacts:\n \ + - id: REQ-001\n type: requirement\n title: self-sat\n status: approved\n \ + links:\n - type: satisfies\n target: REQ-001\n \ + - id: REQ-002\n type: requirement\n title: second\n status: approved\n \ + - id: REQ-003\n type: requirement\n title: third\n status: approved\n", + ) + .unwrap(); + + let run = |extra: &[&str]| -> String { + let mut args = vec!["--project", dirs, "validate"]; + args.extend_from_slice(extra); + let out = Command::new(rivet_bin()) + .args(&args) + .output() + .expect("validate"); + // Drop the `Schemas:` provenance footer (identical here, but not the + // subject of the test) and sort so ordering of independent diagnostic + // lines is not itself the assertion — the content must match. + let mut lines: Vec = String::from_utf8_lossy(&out.stdout) + .lines() + .filter(|l| !l.starts_with("Schemas:")) + .map(str::to_string) + .collect(); + lines.sort(); + lines.join("\n") + }; + + let salsa = run(&[]); + let direct = run(&["--direct"]); + assert_eq!( + salsa, direct, + "`validate` and `validate --direct` must produce identical results (#620)" + ); +} + /// REQ-064: a `derives-from-external` link (the cross-org variant of /// `derives-from`, terminating at an `external-anchor`) must satisfy a /// required `derives-from` link-field. Before the fix the cardinality