Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@

### Added

- **REQ-135 — status enum can now be enforced (the mechanism).** The merged
`Schema` dropped `base-fields` entirely, so `status` (a base field) could
never be enum-validated — the root cause behind a 4-issue cluster
(#352/#354/#355/#353). `Schema::merge` now retains `base_fields`, and
`validate` checks `artifact.status` against the `status` base-field's
`allowed-values` when declared (rule `status-allowed-values`, with REQ-124
remediation: "set to one of […]" / "widen the schema"). Non-breaking: with
no `allowed-values` declared (today's `common.yaml`) the check is inert.
Declaring the canonical lifecycle set activates enforcement.

- **Issue #357 — `validate --min-severity` filters display noise.** `rivet
validate` printed every diagnostic; on a clean repo that is ~160 advisory
warnings for 0 errors, burying the actionable ones. `rivet validate
Expand Down
1 change: 1 addition & 0 deletions rivet-core/src/compliance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ mod tests {
traceability_rules: Vec::new(),
conditional_rules: Vec::new(),
validation_rules: Vec::new(),
base_fields: Vec::new(),
}
}

Expand Down
3 changes: 3 additions & 0 deletions rivet-core/src/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1451,6 +1451,7 @@ mod tests {
traceability_rules: vec![],
conditional_rules: vec![],
validation_rules: vec![],
base_fields: Vec::new(),
};
let sw_req = crate::schema::ArtifactTypeDef {
name: "sw-req".into(),
Expand Down Expand Up @@ -1645,6 +1646,7 @@ mod tests {
traceability_rules: vec![],
conditional_rules: vec![],
validation_rules: vec![],
base_fields: Vec::new(),
};
let sw_req = crate::schema::ArtifactTypeDef {
name: "sw-req".into(),
Expand Down Expand Up @@ -1709,6 +1711,7 @@ mod tests {
traceability_rules: vec![],
conditional_rules: vec![],
validation_rules: vec![],
base_fields: Vec::new(),
};
let sw_req = crate::schema::ArtifactTypeDef {
name: "sw-req".into(),
Expand Down
42 changes: 42 additions & 0 deletions rivet-core/src/remediation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,52 @@ pub fn remediation_for(diag: &Diagnostic, schema: &Schema, store: &Store) -> Opt
"cardinality" => cardinality(diag, schema, store),
"unknown-field" => unknown_field(diag, schema, store),
"known-type" => known_type(diag, schema, store),
"status-allowed-values" => status_allowed_values(diag, schema, store),
_ => None,
}
}

/// `status-allowed-values` (REQ-135): the artifact's `status` is outside the
/// `allowed-values` declared on the schema's `status` base-field.
/// A. set status to a declared lifecycle value (usual — a typo);
/// B. add the value to the schema if it is a legitimate new state.
fn status_allowed_values(diag: &Diagnostic, schema: &Schema, store: &Store) -> Option<Remediation> {
let source_id = diag.artifact_id.as_deref()?;
let artifact = store.get(source_id)?;
let status = artifact.status.as_deref()?;
let allowed = schema
.base_fields
.iter()
.find(|f| f.name == "status")
.and_then(|f| f.allowed_values.as_ref())?;
let allowed_list = allowed.join(", ");
Some(Remediation {
summary: format!(
"status '{status}' on '{source_id}' is not a declared lifecycle value \
[{allowed_list}]. Usually a typo; widen the schema only if it is a real state."
),
fix_options: vec![
FixOption {
kind: FixKind::FixArtifact,
detail: format!(
"Set '{source_id}' to one of [{allowed_list}] \
(e.g. `rivet modify {source_id} --set-status <value>`)."
),
targets: allowed.clone(),
},
FixOption {
kind: FixKind::AdjustSchema,
detail: format!(
"If '{status}' is a legitimate lifecycle state, add it to the `status` \
base-field's `allowed-values` in the schema."
),
targets: vec![status.to_string()],
},
],
doc_topic: "diagnostics/allowed-values".to_string(),
})
}

/// `required-field`: a field the schema marks `required: true` is absent.
/// A. add the field to the artifact (usual);
/// B. make the field optional in the schema (if it should not be mandatory).
Expand Down
1 change: 1 addition & 0 deletions rivet-core/src/reqif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2140,6 +2140,7 @@ mod tests {
traceability_rules: vec![],
validation_rules: vec![],
conditional_rules: vec![],
base_fields: vec![],
};

let mut fields: BTreeMap<String, serde_yaml::Value> = BTreeMap::new();
Expand Down
16 changes: 16 additions & 0 deletions rivet-core/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,11 @@ pub struct Schema {
pub traceability_rules: Vec<TraceabilityRule>,
pub conditional_rules: Vec<ConditionalRule>,
pub validation_rules: Vec<ValidationRule>,
/// Schema-wide base fields (`base-fields:`) applied to every artifact —
/// `id`, `title`, `status`, `tags`, … Retained on the merged schema
/// (REQ-135) so the validator can enforce e.g. a `status` enum; before
/// this they were parsed onto `SchemaFile` and dropped by `merge`.
pub base_fields: Vec<FieldDef>,
}

impl Schema {
Expand All @@ -1062,8 +1067,18 @@ impl Schema {
let mut traceability_rules = Vec::new();
let mut conditional_rules = Vec::new();
let mut validation_rules = Vec::new();
// REQ-135: union base-fields across files by name (later-wins on the
// scalar attributes, mirroring artifact-type field merging).
let mut base_fields: Vec<FieldDef> = Vec::new();

for file in files {
for bf in &file.base_fields {
if let Some(existing) = base_fields.iter_mut().find(|f| f.name == bf.name) {
*existing = bf.clone();
} else {
base_fields.push(bf.clone());
}
}
for at in &file.artifact_types {
let mut at = at.clone();
// Populate shorthand_links from link_fields so the YAML
Expand Down Expand Up @@ -1123,6 +1138,7 @@ impl Schema {
traceability_rules,
conditional_rules,
validation_rules,
base_fields,
}
}

Expand Down
109 changes: 109 additions & 0 deletions rivet-core/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,36 @@ pub fn validate_structural_with_externals_and_variant(
}
}

// 3b. REQ-135: validate the `status` base field against its declared
// enum. `status` is a schema base-field (not a per-type field) and
// lives in `artifact.status` (top-level), so the per-type
// allowed-values loop above never sees it. When the schema declares
// `allowed-values` on the `status` base-field, enforce it here; with
// no values declared the check is inert (backward compatible).
if let Some(status) = artifact.status.as_deref() {
if let Some(allowed) = schema
.base_fields
.iter()
.find(|f| f.name == "status")
.and_then(|f| f.allowed_values.as_ref())
{
if !allowed.is_empty() && !allowed.iter().any(|a| a == status) {
diagnostics.push(Diagnostic {
source_file: None,
line: None,
column: None,
severity: Severity::Error,
artifact_id: Some(artifact.id.clone()),
rule: "status-allowed-values".to_string(),
message: format!(
"status '{}' is not an allowed value, allowed: {:?}",
status, allowed
),
});
}
}
}

// 4. Check link field cardinality
for link_field in &type_def.link_fields {
let count = artifact
Expand Down Expand Up @@ -2585,6 +2615,85 @@ then:
);
}

// REQ-135: when a schema declares `allowed-values` on the `status`
// base-field, validate enforces it (a typo'd status flags), reading the
// top-level `artifact.status`. The base-field must survive `Schema::merge`.
// rivet: verifies REQ-135
#[test]
fn status_enum_is_enforced_when_declared() {
let mut file = minimal_schema("requirement");
file.artifact_types = vec![ArtifactTypeDef {
name: "requirement".into(),
..Default::default()
}];
file.base_fields = vec![FieldDef {
name: "status".into(),
field_type: "enum".into(),
allowed_values: Some(vec![
"draft".into(),
"approved".into(),
"implemented".into(),
]),
..Default::default()
}];
let schema = Schema::merge(&[file]);
assert!(
!schema.base_fields.is_empty(),
"merge must retain base_fields"
);

let mut store = Store::new();
let mut good = minimal_artifact("REQ-1", "requirement");
good.status = Some("approved".to_string());
store.insert(good).unwrap();
let mut bad = minimal_artifact("REQ-2", "requirement");
bad.status = Some("implmented".to_string()); // typo
store.insert(bad).unwrap();

let graph = LinkGraph::build(&store, &schema);
let diags = validate_structural(&store, &schema, &graph);
let status_diags: Vec<_> = diags
.iter()
.filter(|d| d.rule == "status-allowed-values")
.collect();
assert_eq!(
status_diags.len(),
1,
"only the typo'd status should flag; got {status_diags:?}"
);
assert_eq!(status_diags[0].artifact_id.as_deref(), Some("REQ-2"));
assert!(status_diags[0].message.contains("implmented"));
}

// Backward-compatibility: with no `allowed-values` declared on `status`
// (today's common.yaml), the check is inert — any status passes.
// rivet: verifies REQ-135
#[test]
fn status_enum_inert_when_no_values_declared() {
let mut file = minimal_schema("requirement");
file.artifact_types = vec![ArtifactTypeDef {
name: "requirement".into(),
..Default::default()
}];
file.base_fields = vec![FieldDef {
name: "status".into(),
field_type: "enum".into(),
..Default::default() // no allowed_values
}];
let schema = Schema::merge(&[file]);

let mut store = Store::new();
let mut a = minimal_artifact("REQ-1", "requirement");
a.status = Some("anything-goes".to_string());
store.insert(a).unwrap();
let graph = LinkGraph::build(&store, &schema);
let diags = validate_structural(&store, &schema, &graph);
assert!(
!diags.iter().any(|d| d.rule == "status-allowed-values"),
"no enum declared → no status enforcement"
);
}

// ── Mutation-pinning tests for link cardinality ────────────────────
//
// Each test pins one or more surviving mutants in
Expand Down
Loading