diff --git a/artifacts/requirements.yaml b/artifacts/requirements.yaml index 396a2c7d..c91d661e 100644 --- a/artifacts/requirements.yaml +++ b/artifacts/requirements.yaml @@ -7579,7 +7579,7 @@ artifacts: - id: REQ-239 type: requirement title: validate <-> commit-name parsing parity - status: proposed + status: verified description: "Sync the validate naming checks with commit-name parsing so naming issues are caught early and consistently. #577. v0.23." provenance: created-by: ai-assisted diff --git a/rivet-core/src/commits.rs b/rivet-core/src/commits.rs index f672da09..b74deea5 100644 --- a/rivet-core/src/commits.rs +++ b/rivet-core/src/commits.rs @@ -322,23 +322,38 @@ pub fn extract_artifact_refs(value: &str) -> (Vec, Vec) { /// but is not itself a valid artifact ID. This deliberately excludes /// ordinary hyphenated prose (no digit) and bare numbers (no hyphen), /// so `rivet commits` flags genuine typos without choking on free text. -fn looks_like_artifact_id_attempt(token: &str) -> bool { +/// +/// `pub` because `rivet validate` reuses it (#577): an artifact whose id +/// looks like a botched numbered id (e.g. a dotted suffix `H-3.2`) validates +/// fine but can't be referenced in a commit trailer — validate warns early so +/// the mismatch isn't discovered only at commit time. +pub fn looks_like_artifact_id_attempt(token: &str) -> bool { !is_artifact_id(token) && token.contains('-') && token.chars().any(|c| c.is_ascii_digit()) } -/// Check whether a string looks like an artifact ID. +/// Check whether a string has the shape rivet recognises as an artifact ID +/// in a **commit trailer** (`Implements: `). This is the single source of +/// truth for that shape — `rivet validate` calls it too, so a project can't +/// have IDs that validate but silently fail to trace through commits (#577). /// -/// Matches simple IDs like `REQ-001` and compound-prefix IDs like -/// `UCA-C-10`. The last hyphen-separated segment must be all digits; -/// all preceding segments must be non-empty uppercase ASCII. -fn is_artifact_id(s: &str) -> bool { +/// Matches simple IDs like `REQ-001` and compound-prefix IDs like `UCA-C-10`. +/// The last hyphen-separated segment must be all digits; every preceding +/// segment must be non-empty, contain at least one uppercase ASCII letter, and +/// consist only of uppercase ASCII letters or digits — so a digit-bearing +/// prefix like `MAD1-101` is accepted (#577), while `123-4` (no letter), +/// `mad1-1` (lowercase), and `H-3.2` (dotted suffix) are not. +pub fn is_artifact_id(s: &str) -> bool { if let Some(pos) = s.rfind('-') { let prefix = &s[..pos]; let suffix = &s[pos + 1..]; !prefix.is_empty() - && prefix - .split('-') - .all(|seg| !seg.is_empty() && seg.chars().all(|c| c.is_ascii_uppercase())) + && prefix.split('-').all(|seg| { + !seg.is_empty() + && seg + .chars() + .all(|c| c.is_ascii_uppercase() || c.is_ascii_digit()) + && seg.chars().any(|c| c.is_ascii_uppercase()) + }) && !suffix.is_empty() && suffix.chars().all(|c| c.is_ascii_digit()) } else { @@ -1271,6 +1286,30 @@ mod tests { assert!(!is_artifact_id("-1")); } + // #577 (REQ-239): a digit-bearing prefix segment (e.g. `MAD1`) is a valid + // commit-trailer ref — the parser used to require letter-only prefixes, + // forcing a rename loop. Segments must still contain at least one letter + // and the suffix must be all digits. + // rivet: verifies REQ-239 + #[test] + fn artifact_id_accepts_digit_bearing_prefix() { + assert!(is_artifact_id("MAD1-101"), "digit in prefix segment is ok"); + assert!(is_artifact_id("A1-B2-3"), "digits across compound segments"); + assert!(is_artifact_id("REQ-001"), "plain case still works"); + // Still rejected: no letter, lowercase, dotted suffix, non-digit suffix. + assert!(!is_artifact_id("123-4"), "prefix segment needs a letter"); + assert!(!is_artifact_id("mad1-1"), "lowercase prefix rejected"); + assert!(!is_artifact_id("H-3.2"), "dotted suffix is not all-digits"); + assert!(!is_artifact_id("REQ-00A"), "non-digit suffix rejected"); + // …and the parity heuristic flags the botched-but-digit-bearing ones. + assert!(looks_like_artifact_id_attempt("H-3.2")); + assert!(!looks_like_artifact_id_attempt("MAD1-101")); + assert!( + !looks_like_artifact_id_attempt("ARCH-CORE-COMMITS"), + "a descriptive (no-digit) id is not a botched numbered id" + ); + } + // -- integration: extract_artifact_ids with ranges -- // rivet: verifies REQ-017 diff --git a/rivet-core/src/validate.rs b/rivet-core/src/validate.rs index 2cf8dbac..3776a405 100644 --- a/rivet-core/src/validate.rs +++ b/rivet-core/src/validate.rs @@ -636,6 +636,32 @@ pub fn validate_structural_with_externals_and_variant( if is_external_artifact(artifact) { continue; } + // #577 (REQ-239): an id that looks like a botched numbered id — a + // hyphen and a digit but not a parseable artifact id, e.g. a dotted + // suffix `H-3.2` — validates fine but cannot be referenced in a commit + // trailer (`Implements: `). Warn early so the mismatch isn't + // discovered only when `rivet commit` rejects the trailer. Shares the + // exact shape rule with the commit parser (`crate::commits`) so the + // two never diverge (the relaxation there now accepts digit-bearing + // prefixes like `MAD1-101`, so only genuinely un-referenceable shapes + // remain flagged here). Externally-prefixed ids are already skipped. + if crate::commits::looks_like_artifact_id_attempt(&artifact.id) { + diagnostics.push(Diagnostic { + source_file: None, + line: None, + column: None, + severity: Severity::Warning, + artifact_id: Some(artifact.id.clone()), + rule: "commit-ref-shape".to_string(), + message: format!( + "id '{}' can't be used as a commit-trailer reference \ + (trailers need an uppercase-alphanumeric prefix and an \ + all-digit suffix, e.g. REQ-001); rename it to trace this \ + artifact through commits", + artifact.id + ), + }); + } let type_def = match lookup_type(artifact, schema, externals) { TypeLookup::Found(td) => td, TypeLookup::Unknown => { @@ -2719,6 +2745,53 @@ then: ); } + /// #577 (REQ-239): validate warns when an artifact id can't be used as a + /// commit-trailer reference — a dotted suffix (`H-3.2`) is flagged, while a + /// now-accepted digit-bearing prefix (`MAD1-101`) is not. Keeps validate + /// and the commit parser in sync so naming issues surface at validate time. + /// + /// rivet: verifies REQ-239 + #[test] + fn validate_warns_on_non_commit_referenceable_id() { + let mut file = minimal_schema("test"); + file.artifact_types = vec![ArtifactTypeDef { + name: "requirement".to_string(), + description: "REQ".to_string(), + fields: vec![], + link_fields: vec![], + aspice_process: None, + common_mistakes: vec![], + example: None, + yaml_section: None, + yaml_sections: vec![], + yaml_section_suffix: None, + shorthand_links: std::collections::BTreeMap::new(), + }]; + file.traceability_rules = vec![]; + let schema = Schema::merge(&[file]); + + let mut store = Store::new(); + store + .insert(minimal_artifact("H-3.2", "requirement")) + .unwrap(); + store + .insert(minimal_artifact("MAD1-101", "requirement")) + .unwrap(); + + let graph = LinkGraph::build(&store, &schema); + let diags = validate(&store, &schema, &graph); + let flagged: Vec<&str> = diags + .iter() + .filter(|d| d.rule == "commit-ref-shape") + .filter_map(|d| d.artifact_id.as_deref()) + .collect(); + assert_eq!( + flagged, + vec!["H-3.2"], + "only the dotted-suffix id is un-referenceable; MAD1-101 is now valid" + ); + } + /// Issue #349: `required-backlink` written as the INVERSE link-type /// name (e.g. `supported-by`, the convention used by /// `schemas/safety-case.yaml`) was never matched against the stored