From 66db9545827115c3176bbb8b71c32f8e042d5872 Mon Sep 17 00:00:00 2001 From: Dieter Baier Date: Mon, 21 Sep 2026 18:47:47 +0200 Subject: [PATCH 1/5] issue_101: Implement the artifact validity axis ADR-009 separated review maturity from temporal validity. The schema and the validator only knew the review axis, so an author following the metamodel document broke validation. The artifact schema now carries validity, retired_on, retired_reason and retired_note, and the validator reads the vocabulary from it exactly as it already reads relation types from the relation schema. Drift between the two representations is prevented by construction rather than by a test; the constants remain only as a fallback for a checkout without the schema, and a parity test holds them to the schema's values. The invariants the axis needs are enforced where they can actually fail: a retired artifact must record retired_on, an active one must not carry retirement fields, retired_on cannot precede created, and a risk-specific reason cannot appear on another artifact type. An absent validity is read as active in code, because a JSON Schema default annotates without materializing anything. A relation is inactive when either endpoint is retired, not only its target, so the matrices mark both the ADR's outgoing row and the retired risk's incoming row. The relation itself is never rewritten. A new relation to a retired artifact warns instead of failing: documents and supersedes legitimately point backwards, while a silent accept would hide stale references. Ten scenarios, ten bridged tests. Breaking the marker and the retirement branch fails seven of them, so the guards have a real failure path. --- example/metamodel/artifact.schema.yaml | 53 +++- features/documentation-generation.feature | 15 ++ features/metamodel-validation.feature | 35 +++ metamodel/artifact.schema.yaml | 53 +++- scripts/validate-metamodel.rb | 172 ++++++++++++- test/validate_metamodel_test.rb | 291 ++++++++++++++++++++++ 6 files changed, 605 insertions(+), 14 deletions(-) diff --git a/example/metamodel/artifact.schema.yaml b/example/metamodel/artifact.schema.yaml index 779a447..efd5eef 100644 --- a/example/metamodel/artifact.schema.yaml +++ b/example/metamodel/artifact.schema.yaml @@ -42,7 +42,9 @@ properties: minLength: 1 status: type: string - description: Lifecycle state of the artifact. + description: >- + Review axis: how confirmed the artifact is. Whether it still holds is the + separate validity axis. enum: - draft - proposed @@ -51,6 +53,39 @@ properties: - rejected - superseded - deprecated + validity: + type: string + description: >- + Temporal axis: whether the artifact still holds. An absent value means + active. The default below is a schema annotation and materializes + nothing, so validator and generator implement that reading explicitly. + enum: + - active + - retired + default: active + retired_on: + type: string + format: date + description: >- + Date of retirement. Required while validity is retired, rejected while + the artifact is active, and never earlier than created. + retired_reason: + type: string + description: >- + Machine-readable retirement reason, for filtering and generated views. + 'no-longer-applicable' and 'removed' apply to every artifact type; + 'mitigated' and 'materialized' apply to Risk artifacts only, which the + validator enforces. + enum: + - no-longer-applicable + - removed + - mitigated + - materialized + retired_note: + type: string + description: >- + Free text explaining the retirement to a human reader. The reason is for + machines, the note is for people; neither replaces the other. owner: type: string minLength: 1 @@ -134,6 +169,22 @@ properties: metadata_version: type: string default: "1.0" +allOf: + - if: + properties: + validity: + const: retired + required: + - validity + then: + required: + - retired_on + else: + properties: + retired_on: false + retired_reason: false + retired_note: false + examples: - id: DOC-09001-adr-index type: Document diff --git a/features/documentation-generation.feature b/features/documentation-generation.feature index 5b6665a..2b128d3 100644 --- a/features/documentation-generation.feature +++ b/features/documentation-generation.feature @@ -44,6 +44,21 @@ Feature: Documentation generation When the metadata attribute fragment is rendered Then it exposes the artifact id, status, and derived-from description as AsciiDoc attributes + Scenario: Impact fragment marks a relation to a retired artifact as inactive + Given an ADR whose outgoing relation points at a retired risk + When the impact fragment is rendered + Then the relation is still listed and marked inactive + + Scenario: Traceability fragment marks the relations of a retired artifact as inactive + Given a retired risk with an incoming relation from an active ADR + When the traceability fragment is rendered for the retired risk + Then the incoming relation is still listed and marked inactive + + Scenario: Traceability matrix marks an inactive relation + Given an ADR whose outgoing relation points at a retired risk + When the traceability matrix is rendered + Then the outgoing relation cell carries the inactive marker + Scenario: Chapter include fragment output is deterministic and sorted by artifact id Given an arc42 chapter with two detail documents When the chapter include fragment is rendered twice diff --git a/features/metamodel-validation.feature b/features/metamodel-validation.feature index 5c466fc..b706700 100644 --- a/features/metamodel-validation.feature +++ b/features/metamodel-validation.feature @@ -90,6 +90,41 @@ Feature: Metamodel validation When the validator runs Then it warns that a bidirectional relation was detected + Scenario: Artifact without validity is treated as active + Given an artifact that records no validity + When the validator runs + Then it reports no errors and the artifact counts as active + + Scenario: Unknown validity value reports an error + Given an artifact whose validity is neither active nor retired + When the validator runs + Then it reports the unknown validity + + Scenario: Retired artifact without a retirement date reports an error + Given a retired artifact that records no retirement date + When the validator runs + Then it reports that a retired artifact must record retired_on + + Scenario: Retirement date before creation reports an error + Given a retired artifact whose retirement date precedes its creation date + When the validator runs + Then it reports that retired_on is earlier than created + + Scenario: Active artifact carrying retirement fields reports an error + Given an active artifact that records a retirement date, reason, and note + When the validator runs + Then it reports that an active artifact must not record retirement fields + + Scenario: Risk retirement reason on another artifact type reports an error + Given a retired document that claims the risk-specific reason mitigated + When the validator runs + Then it reports that the reason applies to Risk artifacts only + + Scenario: Relation to a retired artifact warns and stays valid + Given an accepted relation pointing at a retired risk + When the validator runs + Then it reports no errors and warns that the relation points at a retired artifact + Scenario: Report states validation passed for valid artifacts Given a directory of well-formed architecture artifacts When the validator prints its report diff --git a/metamodel/artifact.schema.yaml b/metamodel/artifact.schema.yaml index 375e159..16669c4 100644 --- a/metamodel/artifact.schema.yaml +++ b/metamodel/artifact.schema.yaml @@ -42,7 +42,9 @@ properties: minLength: 1 status: type: string - description: Lifecycle state of the artifact. + description: >- + Review axis: how confirmed the artifact is. Whether it still holds is the + separate validity axis. enum: - draft - proposed @@ -51,6 +53,39 @@ properties: - rejected - superseded - deprecated + validity: + type: string + description: >- + Temporal axis: whether the artifact still holds. An absent value means + active. The default below is a schema annotation and materializes + nothing, so validator and generator implement that reading explicitly. + enum: + - active + - retired + default: active + retired_on: + type: string + format: date + description: >- + Date of retirement. Required while validity is retired, rejected while + the artifact is active, and never earlier than created. + retired_reason: + type: string + description: >- + Machine-readable retirement reason, for filtering and generated views. + 'no-longer-applicable' and 'removed' apply to every artifact type; + 'mitigated' and 'materialized' apply to Risk artifacts only, which the + validator enforces. + enum: + - no-longer-applicable + - removed + - mitigated + - materialized + retired_note: + type: string + description: >- + Free text explaining the retirement to a human reader. The reason is for + machines, the note is for people; neither replaces the other. owner: type: string minLength: 1 @@ -134,6 +169,22 @@ properties: metadata_version: type: string default: "1.0" +allOf: + - if: + properties: + validity: + const: retired + required: + - validity + then: + required: + - retired_on + else: + properties: + retired_on: false + retired_reason: false + retired_note: false + examples: - id: DOC-09001-adr-index type: Document diff --git a/scripts/validate-metamodel.rb b/scripts/validate-metamodel.rb index c236f7f..cd438a0 100755 --- a/scripts/validate-metamodel.rb +++ b/scripts/validate-metamodel.rb @@ -10,16 +10,28 @@ class MetamodelValidator REQUIRED_FIELDS = %w[id type title status created].freeze + # Fallbacks for the validity vocabulary. The artifact schema owns the values; + # these apply only when it cannot be read, and a parity test keeps the two + # in step. + VALIDITY_VALUES = %w[active retired].freeze + RETIREMENT_FIELDS = %w[retired_on retired_reason retired_note].freeze + RETIREMENT_REASONS = %w[no-longer-applicable removed mitigated materialized].freeze + # Reasons that only make sense for one artifact type; every other reason in + # RETIREMENT_REASONS is universal. A risk is mitigated or materializes, an + # interface does neither. The schema carries the full value list, so only the + # type mapping lives here. + TYPE_RETIREMENT_REASONS = { 'Risk' => %w[mitigated materialized] }.freeze Artifact = Struct.new(:path, :metadata, :document_id, keyword_init: true) attr_reader :errors, :warnings, :root, :docs_dir - def initialize(root:, docs_dir:, relations_schema:) + def initialize(root:, docs_dir:, relations_schema:, artifact_schema: nil) @root = Pathname.new(root).expand_path @docs_paths = Array(docs_dir).map { |path| Pathname.new(path).expand_path } @docs_dir = @docs_paths.length == 1 ? @docs_paths.first : @docs_paths @relations_schema = Pathname.new(relations_schema).expand_path + @artifact_schema = Pathname.new(artifact_schema || @root.join('metamodel/artifact.schema.yaml')).expand_path @errors = [] @warnings = [] end @@ -37,6 +49,7 @@ def validate validate_filename_matches_id(artifacts) validate_decimal_classification(artifacts) validate_unique_ids(artifacts) + validate_validity(artifacts, artifact_vocabulary) validate_relations(artifacts, relation_types, relation_keys) detect_bidirectional_relations(artifacts) @@ -171,8 +184,101 @@ def validate_decimal_classification(artifacts) end end + # The validity axis is orthogonal to `status`: `status` says how confirmed an + # artifact is, `validity` says whether it still holds. An absent value means + # active. The schema carries a `default`, but a JSON Schema default is an + # annotation and materializes nothing, so the reading is implemented here. + def validate_validity(artifacts, vocabulary) + artifacts.each do |artifact| + metadata = artifact.metadata + next unless metadata + + location = relative(artifact.path) + validity = metadata['validity'] + + if !blank?(validity) && !vocabulary.fetch(:validity).include?(validity) + @errors << "#{location} uses unknown validity '#{validity}'" + next + end + + if retired?(metadata) + validate_retirement(metadata, location, vocabulary) + else + recorded = RETIREMENT_FIELDS.select { |field| metadata.key?(field) && !blank?(metadata[field]) } + next if recorded.empty? + + @errors << "#{location} is active but records retirement field(s): #{recorded.sort.join(', ')}" + end + end + end + + def validate_retirement(metadata, location, vocabulary) + validate_retirement_date(metadata, location) + + reason = metadata['retired_reason'] + return if blank?(reason) + + unless vocabulary.fetch(:reasons).include?(reason) + @errors << "#{location} uses unknown retirement reason '#{reason}'" + return + end + + owner, = TYPE_RETIREMENT_REASONS.find { |_type, reasons| reasons.include?(reason) } + return if owner.nil? || owner == metadata['type'] + + @errors << "#{location} uses retirement reason '#{reason}', which applies to #{owner} artifacts only" + end + + def validate_retirement_date(metadata, location) + retired_on = metadata['retired_on'] + if blank?(retired_on) + @errors << "#{location} is retired and must record 'retired_on'" + return + end + + retired_date = parse_date(retired_on) + if retired_date.nil? + @errors << "#{location} has an invalid 'retired_on' date '#{retired_on}'" + return + end + + created = parse_date(metadata['created']) + return if created.nil? || retired_date >= created + + @errors << "#{location} has 'retired_on' #{retired_date} earlier than 'created' #{created}" + end + + def retired?(metadata) + metadata.is_a?(Hash) && metadata['validity'].to_s == 'retired' + end + + def parse_date(value) + return nil if blank?(value) + return value if value.is_a?(Date) + + Date.parse(value.to_s) + rescue ArgumentError, TypeError + nil + end + + # The artifact schema owns the validity vocabulary, exactly as the relation + # schema owns the relation types. Reading it here means the two cannot drift; + # the constants are only a fallback for a checkout without the schema. + def artifact_vocabulary + properties = load_yaml(@artifact_schema).fetch('properties') + + { + validity: properties.fetch('validity').fetch('enum'), + reasons: properties.fetch('retired_reason').fetch('enum') + } + rescue StandardError + { validity: VALIDITY_VALUES, reasons: RETIREMENT_REASONS } + end + def validate_relations(artifacts, relation_types, relation_keys) known_ids = artifacts.map(&:document_id).compact.to_set + retired_ids = artifacts.select { |candidate| retired?(candidate.metadata) } + .map(&:document_id).compact.to_set artifacts.each do |artifact| metadata = artifact.metadata @@ -211,6 +317,15 @@ def validate_relations(artifacts, relation_types, relation_keys) if target && !known_ids.include?(target) @errors << "#{location} references unknown artifact id '#{target}'" end + + # A relation to retired knowledge is allowed: `documents` and + # `supersedes` legitimately point backwards. It is warned about so an + # accidental reference to something that no longer exists is visible + # instead of silent. + if target && retired_ids.include?(target) + @warnings << "Relation to retired artifact: #{artifact.document_id} -> #{target}. " \ + 'It stays valid as history; check that it is not an accidental reference.' + end end end end @@ -326,7 +441,29 @@ def docs_target_label end end +# A relation is inactive for the current architecture when either endpoint is +# retired. It is still rendered, because it remains a true record of what was +# decided and reviewed; the marker says only that it no longer describes the +# system as it is now. +module RelationValidity + INACTIVE_MARKER = '[.relation-inactive]#(inactive)#' + + def retired_metadata?(metadata) + metadata.is_a?(Hash) && metadata['validity'].to_s == 'retired' + end + + def inactive_relation?(source_metadata, target_metadata) + retired_metadata?(source_metadata) || retired_metadata?(target_metadata) + end + + def mark_inactive(text, inactive) + inactive ? "#{text} #{INACTIVE_MARKER}" : text + end +end + class TraceabilityMatrixGenerator + include RelationValidity + DEFAULT_OUTPUT = 'generated/traceability-matrix.adoc' def initialize(root:, docs_dir:, output_path: nil) @@ -370,8 +507,8 @@ def render(artifacts) lines << "| #{cell(metadata['type'])}" lines << "| #{cell(metadata['title'])}" lines << "| #{cell(metadata['status'])}" - lines << "| #{relations_cell(metadata['relations'] || [], artifacts_by_id, :outgoing)}" - lines << "| #{relations_cell(incoming.fetch(id, []), artifacts_by_id, :incoming)}" + lines << "| #{relations_cell(metadata['relations'] || [], artifacts_by_id, :outgoing, metadata)}" + lines << "| #{relations_cell(incoming.fetch(id, []), artifacts_by_id, :incoming, metadata)}" lines << '' end @@ -391,7 +528,7 @@ def incoming_relations(artifacts) end end - def relations_cell(relations, artifacts_by_id, direction) + def relations_cell(relations, artifacts_by_id, direction, self_metadata = nil) return '-' if relations.empty? sorted = relations.sort_by do |relation| @@ -400,11 +537,15 @@ def relations_cell(relations, artifacts_by_id, direction) end sorted.map do |relation| - if direction == :outgoing - "#{cell(relation['type'])} -> #{artifact_ref(relation['target'], artifacts_by_id)}" - else - "#{artifact_ref(relation['source'], artifacts_by_id)} -> #{cell(relation['type'])}" - end + other_id = direction == :outgoing ? relation['target'] : relation['source'] + inactive = inactive_relation?(self_metadata, artifacts_by_id[other_id]&.metadata) + + text = if direction == :outgoing + "#{cell(relation['type'])} -> #{artifact_ref(relation['target'], artifacts_by_id)}" + else + "#{artifact_ref(relation['source'], artifacts_by_id)} -> #{cell(relation['type'])}" + end + mark_inactive(text, inactive) end.join(" +\n") end @@ -663,6 +804,8 @@ def cell(value) end class TraceabilityFragmentGenerator + include RelationValidity + attr_reader :output_paths def initialize(root:, docs_dir:) @@ -708,15 +851,17 @@ def render(artifact, artifacts_by_id, incoming, output_path) lines << '' else outgoing.sort_by { |relation| [relation['type'].to_s, relation['target'].to_s] }.each do |relation| + inactive = inactive_relation?(metadata, artifacts_by_id[relation['target']]&.metadata) lines << '| outgoing' - lines << "| #{helper.cell(relation['type'])}" + lines << "| #{mark_inactive(helper.cell(relation['type']), inactive)}" lines << "| #{helper.artifact_ref(relation['target'], artifacts_by_id)}" lines << '' end incoming.sort_by { |relation| [relation['type'].to_s, relation['source'].to_s] }.each do |relation| + inactive = inactive_relation?(metadata, artifacts_by_id[relation['source']]&.metadata) lines << '| incoming' - lines << "| #{helper.cell(relation['type'])}" + lines << "| #{mark_inactive(helper.cell(relation['type']), inactive)}" lines << "| #{helper.artifact_ref(relation['source'], artifacts_by_id)}" lines << '' end @@ -756,6 +901,8 @@ def traceability_output_path(artifact) end class ImpactFragmentGenerator + include RelationValidity + attr_reader :output_paths def initialize(root:, docs_dir:) @@ -800,8 +947,9 @@ def render(artifact, artifacts_by_id, output_path) lines << '' else outgoing.sort_by { |relation| [relation['type'].to_s, relation['target'].to_s] }.each do |relation| + inactive = inactive_relation?(metadata, artifacts_by_id[relation['target']]&.metadata) lines << "| #{helper.artifact_ref(relation['target'], artifacts_by_id)}" - lines << "| #{helper.cell(relation['type'])}" + lines << "| #{mark_inactive(helper.cell(relation['type']), inactive)}" lines << "| #{helper.cell(relation['rationale'])}" lines << '' end diff --git a/test/validate_metamodel_test.rb b/test/validate_metamodel_test.rb index 9262009..723dd68 100644 --- a/test/validate_metamodel_test.rb +++ b/test/validate_metamodel_test.rb @@ -830,8 +830,299 @@ def test_chapter_include_fragment_write_skips_generated_artifacts FileUtils.rm_rf(temp_dir) if temp_dir end + # Feature: Metamodel validation -- the validity axis + + def test_artifact_without_validity_is_treated_as_active + # Given: an artifact that records no validity + temp_dir = ROOT.join('tmp/test-validity-absent') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('r-100-open.adoc'), risk_metadata('R-100-open')) + write_metadata_artifact(docs_dir.join('adr-100-decision.adoc'), adr_metadata('ADR-100-decision', 'R-100-open')) + + # When: the validator runs + validator = build_validator(temp_dir, docs_dir) + artifacts = validator.validate + artifacts_by_id = artifacts.each_with_object({}) { |artifact, index| index[artifact.metadata['id']] = artifact } + content = ImpactFragmentGenerator.new(root: temp_dir, docs_dir: docs_dir) + .render(artifacts_by_id.fetch('ADR-100-decision'), artifacts_by_id, + docs_dir.join('generated/adr-100-decision-impact.adoc')) + + # Then: it reports no errors and the artifact counts as active + assert_empty validator.errors + assert_includes content, '| mitigates' + refute_includes content, RelationValidity::INACTIVE_MARKER + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + + def test_unknown_validity_value_reports_an_error + # Given: an artifact whose validity is neither active nor retired + temp_dir = ROOT.join('tmp/test-validity-unknown') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('r-100-open.adoc'), + risk_metadata('R-100-open').merge('validity' => 'closed')) + + # When: the validator runs + validator = build_validator(temp_dir, docs_dir) + validator.validate + + # Then: it reports the unknown validity + assert_includes validator.errors.join("\n"), "uses unknown validity 'closed'" + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + + def test_retired_artifact_without_a_retirement_date_reports_an_error + # Given: a retired artifact that records no retirement date + temp_dir = ROOT.join('tmp/test-retired-without-date') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('r-100-gone.adoc'), + risk_metadata('R-100-gone').merge('validity' => 'retired')) + + # When: the validator runs + validator = build_validator(temp_dir, docs_dir) + validator.validate + + # Then: it reports that a retired artifact must record retired_on + assert_includes validator.errors.join("\n"), "is retired and must record 'retired_on'" + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + + def test_retirement_date_before_creation_reports_an_error + # Given: a retired artifact whose retirement date precedes its creation date + temp_dir = ROOT.join('tmp/test-retired-before-created') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('r-100-gone.adoc'), + risk_metadata('R-100-gone').merge('validity' => 'retired', + 'retired_on' => '2026-07-02')) + + # When: the validator runs + validator = build_validator(temp_dir, docs_dir) + validator.validate + + # Then: it reports that retired_on is earlier than created + assert_includes validator.errors.join("\n"), "has 'retired_on' 2026-07-02 earlier than 'created' 2026-07-03" + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + + def test_active_artifact_carrying_retirement_fields_reports_an_error + # Given: an active artifact that records a retirement date, reason, and note + temp_dir = ROOT.join('tmp/test-active-with-retirement-fields') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('r-100-open.adoc'), + risk_metadata('R-100-open').merge('retired_on' => '2026-09-21', + 'retired_reason' => 'mitigated', + 'retired_note' => 'Handled.')) + + # When: the validator runs + validator = build_validator(temp_dir, docs_dir) + validator.validate + + # Then: it reports that an active artifact must not record retirement fields + assert_includes validator.errors.join("\n"), + 'is active but records retirement field(s): retired_note, retired_on, retired_reason' + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + + def test_risk_retirement_reason_on_another_artifact_type_reports_an_error + # Given: a retired document that claims the risk-specific reason mitigated + temp_dir = ROOT.join('tmp/test-reason-wrong-type') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('doc-100-note.adoc'), + risk_metadata('DOC-100-note').merge('type' => 'Document', + 'validity' => 'retired', + 'retired_on' => '2026-09-21', + 'retired_reason' => 'mitigated')) + + # When: the validator runs + validator = build_validator(temp_dir, docs_dir) + validator.validate + + # Then: it reports that the reason applies to Risk artifacts only + assert_includes validator.errors.join("\n"), + "uses retirement reason 'mitigated', which applies to Risk artifacts only" + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + + def test_relation_to_a_retired_artifact_warns_and_stays_valid + # Given: an accepted relation pointing at a retired risk + temp_dir = ROOT.join('tmp/test-relation-to-retired') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('r-100-gone.adoc'), retired_risk_metadata('R-100-gone')) + write_metadata_artifact(docs_dir.join('adr-100-decision.adoc'), adr_metadata('ADR-100-decision', 'R-100-gone')) + + # When: the validator runs + validator = build_validator(temp_dir, docs_dir) + validator.validate + + # Then: it reports no errors and warns that the relation points at a retired + # artifact + assert_empty validator.errors + assert_includes validator.warnings.join("\n"), + 'Relation to retired artifact: ADR-100-decision -> R-100-gone' + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + + # Feature: Documentation generation -- inactive relations + + def test_impact_fragment_marks_a_relation_to_a_retired_artifact_as_inactive + # Given: an ADR whose outgoing relation points at a retired risk + temp_dir = ROOT.join('tmp/test-impact-inactive') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('r-100-gone.adoc'), retired_risk_metadata('R-100-gone')) + write_metadata_artifact(docs_dir.join('adr-100-decision.adoc'), adr_metadata('ADR-100-decision', 'R-100-gone')) + artifacts = build_validator(temp_dir, docs_dir).validate + artifacts_by_id = artifacts.each_with_object({}) { |artifact, index| index[artifact.metadata['id']] = artifact } + + # When: the impact fragment is rendered + content = ImpactFragmentGenerator.new(root: temp_dir, docs_dir: docs_dir) + .render(artifacts_by_id.fetch('ADR-100-decision'), artifacts_by_id, + docs_dir.join('generated/adr-100-decision-impact.adoc')) + + # Then: the relation is still listed and marked inactive + assert_includes content, 'xref:r-100-gone[R-100-gone]' + assert_includes content, "| mitigates #{RelationValidity::INACTIVE_MARKER}" + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + + def test_traceability_fragment_marks_the_relations_of_a_retired_artifact_as_inactive + # Given: a retired risk with an incoming relation from an active ADR + temp_dir = ROOT.join('tmp/test-traceability-inactive') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('r-100-gone.adoc'), retired_risk_metadata('R-100-gone')) + write_metadata_artifact(docs_dir.join('adr-100-decision.adoc'), adr_metadata('ADR-100-decision', 'R-100-gone')) + artifacts = build_validator(temp_dir, docs_dir).validate + + # When: the traceability fragment is rendered for the retired risk + generator = TraceabilityFragmentGenerator.new(root: temp_dir, docs_dir: docs_dir) + generator.write(artifacts) + content = docs_dir.join('generated/r-100-gone-traceability.adoc').read + + # Then: the incoming relation is still listed and marked inactive + assert_includes content, '| incoming' + assert_includes content, "| mitigates #{RelationValidity::INACTIVE_MARKER}" + assert_includes content, 'xref:adr-100-decision[ADR-100-decision]' + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + + def test_traceability_matrix_marks_an_inactive_relation + # Given: an ADR whose outgoing relation points at a retired risk + temp_dir = ROOT.join('tmp/test-matrix-inactive') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('r-100-gone.adoc'), retired_risk_metadata('R-100-gone')) + write_metadata_artifact(docs_dir.join('adr-100-decision.adoc'), adr_metadata('ADR-100-decision', 'R-100-gone')) + artifacts = build_validator(temp_dir, docs_dir).validate + + # When: the traceability matrix is rendered + content = TraceabilityMatrixGenerator.new( + root: temp_dir, + docs_dir: docs_dir, + output_path: docs_dir.join('generated/traceability-matrix.adoc') + ).render(artifacts) + + # Then: the outgoing relation cell carries the inactive marker + assert_includes content, "mitigates -> xref:../r-100-gone.adoc#r-100-gone[R-100-gone] " \ + "#{RelationValidity::INACTIVE_MARKER}" + # The mirror row: the retired risk's incoming relation is inactive too, + # which is the "either endpoint" rule seen from the other side. + assert_includes content, "xref:../adr-100-decision.adoc#adr-100-decision[ADR-100-decision] -> mitigates " \ + "#{RelationValidity::INACTIVE_MARKER}" + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + + # Supporting parity checks. They carry no Gherkin scenario of their own: they + # verify that two representations of one rule cannot drift, which is technical + # decomposition rather than observable behaviour. + + def test_validity_vocabulary_matches_the_artifact_schema + schema = YAML.load_file(ROOT.join('metamodel/artifact.schema.yaml')).fetch('properties') + + assert_equal MetamodelValidator::VALIDITY_VALUES, schema.fetch('validity').fetch('enum') + assert_equal MetamodelValidator::RETIREMENT_REASONS, schema.fetch('retired_reason').fetch('enum') + assert_equal MetamodelValidator::RETIREMENT_FIELDS.sort, + (MetamodelValidator::RETIREMENT_FIELDS & schema.keys).sort + MetamodelValidator::TYPE_RETIREMENT_REASONS.each_value do |reasons| + reasons.each { |reason| assert_includes schema.fetch('retired_reason').fetch('enum'), reason } + end + end + + def test_example_artifact_schema_matches_the_metamodel_schema + canonical = YAML.load_file(ROOT.join('metamodel/artifact.schema.yaml')) + example = YAML.load_file(ROOT.join('example/metamodel/artifact.schema.yaml')) + + assert_equal canonical.fetch('properties'), example.fetch('properties') + assert_equal canonical.fetch('allOf'), example.fetch('allOf') + end + private + def build_validator(temp_dir, docs_dir) + MetamodelValidator.new( + root: temp_dir, + docs_dir: docs_dir, + relations_schema: SCHEMA, + artifact_schema: ROOT.join('metamodel/artifact.schema.yaml') + ) + end + + def risk_metadata(id) + { + 'id' => id, + 'type' => 'Risk', + 'title' => 'Retirement fixture', + 'status' => 'accepted', + 'owner' => 'test', + 'created' => '2026-07-03' + } + end + + def retired_risk_metadata(id) + risk_metadata(id).merge( + 'validity' => 'retired', + 'retired_on' => '2026-09-21', + 'retired_reason' => 'mitigated', + 'retired_note' => 'Validation now prevents the original failure mode.' + ) + end + + def adr_metadata(id, target) + { + 'id' => id, + 'type' => 'ADR', + 'title' => 'Retirement fixture decision', + 'status' => 'accepted', + 'owner' => 'test', + 'created' => '2026-07-03', + 'relations' => [ + { + 'type' => 'mitigates', + 'target' => target, + 'status' => 'accepted', + 'rationale' => 'Fixture relation.' + } + ] + } + end + + def write_metadata_artifact(path, metadata) + FileUtils.mkdir_p(path.dirname) + anchor = metadata.fetch('id').downcase.gsub(/[^a-z0-9]+/, '-').gsub(/\A-+|-+\z/, '') + path.write(<<~ADOC) + #{metadata.to_yaml.strip} + --- + [[#{anchor}]] + = #{metadata.fetch('title')} + ADOC + end + def write_artifact(path, id, title, status: 'draft', owner: 'test', created: '2026-07-03') FileUtils.mkdir_p(path.dirname) path.write(<<~ADOC) From bc048e0e1b88e3e287e0fc0544d98babbb21150e Mon Sep 17 00:00:00 2001 From: Dieter Baier Date: Mon, 21 Sep 2026 18:47:47 +0200 Subject: [PATCH 2/5] issue_101: Teach authors the validity axis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mechanism is useless if the guidance still describes one axis. The metadata rules gain the validity section, the relation rules state that a retired endpoint never changes a relation's status, and the risk template shows retirement as commented metadata rather than an example that would retire every new risk. Both skills now say that retirement is a human decision an agent may only propose, and that a residual risk the team knowingly carries stays active — the case where the domain word and the metamodel word pull apart. The metamodel drops its not-yet-implemented note and names the marker the generator writes. The README gains the migration note for projects that vendored the schema, including the one combination to avoid: a new schema beside an old validator accepts the fields and enforces nothing. --- README.md | 23 ++++++++++++++++++ skills/adr/SKILL.md | 4 ++++ skills/references/metadata-rules.md | 24 +++++++++++++++++++ skills/references/relation-rules.md | 12 ++++++++++ skills/risk/SKILL.md | 8 +++++++ .../doc-04001-metamodel.adoc | 16 +++++-------- templates/risk.adoc | 6 +++++ 7 files changed, 83 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 99e7280..9e6ff42 100644 --- a/README.md +++ b/README.md @@ -232,6 +232,29 @@ the lookup order in the project's `AGENTS.md`. `skills/**/SKILL.md`, `features/`, and the toolkit's own contract text. Agents resolve these from the toolkit at need. +### Schema change: the artifact validity axis + +`metamodel/artifact.schema.yaml` gained a second lifecycle axis. `status` stays +the review axis — how confirmed an artifact is — and the new optional +`validity` (`active` | `retired`), with `retired_on`, `retired_reason`, and +`retired_note`, says whether the artifact still holds. A relation touching a +retired artifact keeps its reviewed status and is rendered as inactive rather +than dropped. The decision is ADR-009; the model is described in the metamodel +document. + +For a project that vendored the schema: + +- Re-copy `metamodel/artifact.schema.yaml` **and** + `scripts/validate-metamodel.rb` together. The validator reads the validity + vocabulary from the schema, exactly as it reads relation types from + `relations.schema.yaml`. +- The change is additive. Existing artifacts stay valid: an absent `validity` + means active, and nothing has to be backfilled. +- A new schema beside an old validator is the one combination to avoid: the + fields validate but nothing enforces the invariants, so a retired artifact + without `retired_on` passes unnoticed. An old schema beside a new validator is + safe — the validator falls back to its built-in vocabulary. + ### Local skills and contracts extend, they do not duplicate A consuming project may add its own `skills/**/SKILL.md` and task contracts for diff --git a/skills/adr/SKILL.md b/skills/adr/SKILL.md index f5c9b82..9471ef4 100644 --- a/skills/adr/SKILL.md +++ b/skills/adr/SKILL.md @@ -93,6 +93,10 @@ Use these templates: evidence that they changed. - Mark uncertain values as assumptions or open questions. - Do not invent accepted relationships. +- Do not change a relation's status because its target was retired. `rejected` + is a verdict about the claim, not about the passage of time. Retirement is a + human decision recorded on the target artifact's validity axis; propose it, + do not record it as done. - Do not create new risk or quality scenario artifacts when a justified relation to an existing artifact is sufficient. - Keep metadata relation targets as stable artifact IDs; render visible diff --git a/skills/references/metadata-rules.md b/skills/references/metadata-rules.md index 8721011..d3be65b 100644 --- a/skills/references/metadata-rules.md +++ b/skills/references/metadata-rules.md @@ -30,13 +30,37 @@ Use YAML front matter for every source artifact created by this skill. ## Status Rules +`status` is the review axis: how confirmed the artifact is. Whether it still +holds is the separate validity axis below. The two are independent, so a +retired risk can still be `status: accepted`. + - Use `proposed` for AI-created ADRs, risks, quality scenarios, and relations. - Use `draft` for incomplete notes or impact reports that are not source architecture truth. - Keep `accepted`, `reviewed`, `rejected`, `superseded`, and `deprecated` only when the repository already records that lifecycle state. +- Use `deprecated` for something being phased out that can still be + encountered, not for an artifact whose subject no longer exists. That is + retirement. - Do not mark AI output as reviewed. +## Validity Rules + +- Omit `validity` while the artifact holds. An absent value means `active`. +- Set `validity: retired` when the subject no longer exists or no longer + applies, and record `retired_on`. `retired_reason` and the free-text + `retired_note` are optional but expected. +- Reasons are `no-longer-applicable` and `removed` for every artifact type, + plus `mitigated` and `materialized` for risks. +- A residual risk the team knowingly carries stays `active`. Its acceptance + belongs in the risk artifact, not on the validity axis. +- Never record a retirement field on an active artifact, and never set + `retired_on` earlier than `created`. The validator rejects both. +- Retirement is a human decision recorded in the repository. Propose it; do not + mark an artifact retired on your own judgement. +- Do not delete a retired artifact and do not rewrite the relations that point + at it. Their review status stands, and generated matrices mark them inactive. + ## ID Rules - Follow the repository's existing ID pattern. diff --git a/skills/references/relation-rules.md b/skills/references/relation-rules.md index 21f486f..dc17c03 100644 --- a/skills/references/relation-rules.md +++ b/skills/references/relation-rules.md @@ -63,6 +63,18 @@ hierarchical or dependency relationships, not as a reciprocal for other relation - **Never add reciprocal relations manually.** Incoming relations appear only in generated documentation, not in source metadata. +## Retired Endpoints + +- A relation is inactive for the current architecture when either endpoint is + retired. It stays in the metadata with the status it was reviewed with, and + the generator marks it inactive. +- Never change a relation's status because its target stopped existing. + `rejected` means the claim was considered and turned down, not that time has + passed. +- Pointing a new relation at a retired artifact is allowed, because + `documents` and `supersedes` legitimately point backwards. The validator + warns, so check that the reference is deliberate rather than accidental. + ## Impact Rules - Link a proposed ADR to quality scenarios it addresses or constrains. diff --git a/skills/risk/SKILL.md b/skills/risk/SKILL.md index 9d775b5..dd53ef1 100644 --- a/skills/risk/SKILL.md +++ b/skills/risk/SKILL.md @@ -100,6 +100,14 @@ Use these templates: - Preserve existing accepted statuses unless there is explicit repository evidence that they changed. - Do not mark a risk accepted unless human acceptance is already recorded. +- Do not retire a risk on your own judgement. When the risk no longer exists, + propose `validity: retired` with `retired_on`, a `retired_reason`, and a + `retired_note`, and leave the decision to the risk owner. A residual risk the + team knowingly carries stays active; its acceptance belongs in the risk + artifact, not on the validity axis. +- Never delete a risk that stopped applying, and never change the status of a + relation that points at it. The relation stays valid history and the + generator marks it inactive. - Avoid false precision in likelihood, impact, priority, or confidence. - Mark uncertainty explicitly instead of pretending evidence is complete. - Prefer actionable mitigations with owners. diff --git a/src/docs/arc42/04-solution-strategy/doc-04001-metamodel.adoc b/src/docs/arc42/04-solution-strategy/doc-04001-metamodel.adoc index df784c4..68f0471 100644 --- a/src/docs/arc42/04-solution-strategy/doc-04001-metamodel.adoc +++ b/src/docs/arc42/04-solution-strategy/doc-04001-metamodel.adoc @@ -257,16 +257,12 @@ express that time has passed. A relation is inactive for the current architecture when either endpoint is retired, not only its target. Generated impact and traceability matrices keep -rendering an inactive relation and mark it as such. Authoring a new relation to -a retired artifact stays allowed, because a historical link such as `documents` -or `supersedes` is legitimate; the validator warns about it so an accidental -reference to obsolete knowledge becomes visible. - -NOTE: The validity axis is decided in -xref:adr-009-artifact-retirement-and-relation-validity[] but not yet -implemented. `metamodel/artifact.schema.yaml` rejects unknown metadata fields, -so do not add `validity`, `retired_on`, `retired_reason`, or `retired_note` to -artifact metadata before the schema and the validator accept them. +rendering an inactive relation and mark it with `(inactive)` in the relation +type cell, carrying the `relation-inactive` role so a renderer can style it. +Authoring a new relation to a retired artifact stays allowed, because a +historical link such as `documents` or `supersedes` is legitimate; the +validator warns about it so an accidental reference to obsolete knowledge +becomes visible. === Relationship Types and Semantics diff --git a/templates/risk.adoc b/templates/risk.adoc index a19e95a..7186fcf 100644 --- a/templates/risk.adoc +++ b/templates/risk.adoc @@ -9,6 +9,12 @@ reviewed: false summary: Proposed risk summary. tags: - risk +# When the risk no longer exists, retire it instead of deleting it. Keep the +# status it was reviewed with; a risk the team knowingly carries stays active. +# validity: retired +# retired_on: YYYY-MM-DD +# retired_reason: mitigated +# retired_note: One sentence for a human reader. relations: - type: depends_on target: ADR-000-short-title From 1e89af42c56e938a81359296938766b56973fa11 Mon Sep 17 00:00:00 2001 From: Dieter Baier Date: Mon, 21 Sep 2026 18:48:46 +0200 Subject: [PATCH 3/5] issue_101: Resolve what the implementation answered The ADR listed the matrix marker as open; this slice chose one, so the bullet now names it instead of still asking. The suppression mechanism for deliberately historical relations stays open and points at its follow-up, because no artifact is retired yet and the warning volume it would be weighed against does not exist. --- .../adr-009-artifact-retirement-and-relation-validity.adoc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/docs/arc42/09-architecture-decisions/adr-009-artifact-retirement-and-relation-validity.adoc b/src/docs/arc42/09-architecture-decisions/adr-009-artifact-retirement-and-relation-validity.adoc index a8fc2bd..2ae1f5a 100644 --- a/src/docs/arc42/09-architecture-decisions/adr-009-artifact-retirement-and-relation-validity.adoc +++ b/src/docs/arc42/09-architecture-decisions/adr-009-artifact-retirement-and-relation-validity.adoc @@ -231,9 +231,12 @@ separation. outgoing `supersedes` relation. * How does an author record that a relation to a retired artifact is deliberately historical, so its warning does not become permanent build - noise? + noise? Deferred until the first retirements make the warning volume + observable. * How does a generated matrix mark an inactive relation, for example a marker - column, a footnote, or struck-through text? + column, a footnote, or struck-through text? Answered by the implementation: + an `(inactive)` marker on the relation type cell, carrying the + `relation-inactive` role. See xref:artifact-lifecycle-status[]. === Review Notes From 5d87be0236275a7919b1c32f3deb9590f6b4de8f Mon Sep 17 00:00:00 2001 From: Dieter Baier Date: Mon, 21 Sep 2026 18:58:49 +0200 Subject: [PATCH 4/5] issue_101: Carry the transitional caveat into the schema The review on the metamodel applies to every place that states the split. The schema's status description and the authoring rules made the same clean claim, so both now name superseded and deprecated as the two temporal values that stayed behind. A vendoring project reads the schema, not the arc42 chapter, so the caveat has to live there too or the exception is invisible where it is most likely to be met. --- example/metamodel/artifact.schema.yaml | 4 +++- metamodel/artifact.schema.yaml | 4 +++- skills/references/metadata-rules.md | 8 ++++++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/example/metamodel/artifact.schema.yaml b/example/metamodel/artifact.schema.yaml index efd5eef..3694e2e 100644 --- a/example/metamodel/artifact.schema.yaml +++ b/example/metamodel/artifact.schema.yaml @@ -44,7 +44,9 @@ properties: type: string description: >- Review axis: how confirmed the artifact is. Whether it still holds is the - separate validity axis. + separate validity axis. 'superseded' and 'deprecated' are transitional + exceptions that still carry temporal meaning here; ADR-009 defers their + migration. enum: - draft - proposed diff --git a/metamodel/artifact.schema.yaml b/metamodel/artifact.schema.yaml index 16669c4..2241aa9 100644 --- a/metamodel/artifact.schema.yaml +++ b/metamodel/artifact.schema.yaml @@ -44,7 +44,9 @@ properties: type: string description: >- Review axis: how confirmed the artifact is. Whether it still holds is the - separate validity axis. + separate validity axis. 'superseded' and 'deprecated' are transitional + exceptions that still carry temporal meaning here; ADR-009 defers their + migration. enum: - draft - proposed diff --git a/skills/references/metadata-rules.md b/skills/references/metadata-rules.md index d3be65b..e23062f 100644 --- a/skills/references/metadata-rules.md +++ b/skills/references/metadata-rules.md @@ -31,8 +31,12 @@ Use YAML front matter for every source artifact created by this skill. ## Status Rules `status` is the review axis: how confirmed the artifact is. Whether it still -holds is the separate validity axis below. The two are independent, so a -retired risk can still be `status: accepted`. +holds is the separate validity axis below, so a retired risk can still be +`status: accepted`. + +Two values are transitional exceptions: `superseded` and `deprecated` are +temporal statements that still sit on `status`. ADR-009 defers their migration, +so keep using them as they are and put new temporal information on `validity`. - Use `proposed` for AI-created ADRs, risks, quality scenarios, and relations. - Use `draft` for incomplete notes or impact reports that are not source From 11176ab9b15322c5b02abdd6ec4ccd64780c6591 Mon Sep 17 00:00:00 2001 From: Dieter Baier Date: Mon, 21 Sep 2026 19:25:31 +0200 Subject: [PATCH 5/5] issue_101: Decide by presence, as the schema does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the review. The validator used blank? in the three places where the schema decides by property presence, so validity: "" read as active, an active artifact with retired_note: "" passed, and a retired one with an empty reason passed. The build runs the Ruby path, not a JSON Schema engine, so metadata could pass the build while violating the schema that consumers vendor — the drift the parity criterion exists to prevent. The validator now rejects present-but-blank values the way the schema does, and names null and empty string separately because they are different YAML mistakes. retired_note gains minLength: 1 so the schema states the same rule instead of accepting an empty explanation. The Ruby check mirrors that exactly and does not strip whitespace, which would have been drift in the other direction. The existing parity tests compared vocabulary, not verdicts. A table now runs fourteen present, empty, and null cases through the validator and asserts the verdict the schema prescribes, naming the deciding schema rule. Restoring the old blank? logic fails it and the new scenario. --- example/metamodel/artifact.schema.yaml | 4 +- features/metamodel-validation.feature | 5 ++ metamodel/artifact.schema.yaml | 4 +- scripts/validate-metamodel.rb | 34 ++++++++++--- test/validate_metamodel_test.rb | 69 ++++++++++++++++++++++++++ 5 files changed, 107 insertions(+), 9 deletions(-) diff --git a/example/metamodel/artifact.schema.yaml b/example/metamodel/artifact.schema.yaml index 3694e2e..8e97007 100644 --- a/example/metamodel/artifact.schema.yaml +++ b/example/metamodel/artifact.schema.yaml @@ -85,9 +85,11 @@ properties: - materialized retired_note: type: string + minLength: 1 description: >- Free text explaining the retirement to a human reader. The reason is for - machines, the note is for people; neither replaces the other. + machines, the note is for people; neither replaces the other. Omit it + rather than leaving it empty. owner: type: string minLength: 1 diff --git a/features/metamodel-validation.feature b/features/metamodel-validation.feature index b706700..369c73e 100644 --- a/features/metamodel-validation.feature +++ b/features/metamodel-validation.feature @@ -115,6 +115,11 @@ Feature: Metamodel validation When the validator runs Then it reports that an active artifact must not record retirement fields + Scenario: Present but empty validity metadata reports an error + Given artifacts that declare validity or retirement fields with an empty or null value + When the validator runs + Then it reports each one instead of reading it as omitted + Scenario: Risk retirement reason on another artifact type reports an error Given a retired document that claims the risk-specific reason mitigated When the validator runs diff --git a/metamodel/artifact.schema.yaml b/metamodel/artifact.schema.yaml index 2241aa9..3dd343e 100644 --- a/metamodel/artifact.schema.yaml +++ b/metamodel/artifact.schema.yaml @@ -85,9 +85,11 @@ properties: - materialized retired_note: type: string + minLength: 1 description: >- Free text explaining the retirement to a human reader. The reason is for - machines, the note is for people; neither replaces the other. + machines, the note is for people; neither replaces the other. Omit it + rather than leaving it empty. owner: type: string minLength: 1 diff --git a/scripts/validate-metamodel.rb b/scripts/validate-metamodel.rb index cd438a0..9051d51 100755 --- a/scripts/validate-metamodel.rb +++ b/scripts/validate-metamodel.rb @@ -188,23 +188,28 @@ def validate_decimal_classification(artifacts) # artifact is, `validity` says whether it still holds. An absent value means # active. The schema carries a `default`, but a JSON Schema default is an # annotation and materializes nothing, so the reading is implemented here. + # + # Presence decides, not blankness -- exactly as in the schema, whose `else` + # branch forbids the retirement properties outright. `validity: ""` or + # `retired_note: ~` is malformed metadata, not an omission; reading it as + # absent would let the build pass what the distributed schema rejects. def validate_validity(artifacts, vocabulary) artifacts.each do |artifact| metadata = artifact.metadata next unless metadata location = relative(artifact.path) - validity = metadata['validity'] - if !blank?(validity) && !vocabulary.fetch(:validity).include?(validity) - @errors << "#{location} uses unknown validity '#{validity}'" + if metadata.key?('validity') && !vocabulary.fetch(:validity).include?(metadata['validity']) + @errors << "#{location} uses unknown validity #{quoted(metadata['validity'])}; " \ + "omit it or use #{vocabulary.fetch(:validity).join(' or ')}" next end if retired?(metadata) validate_retirement(metadata, location, vocabulary) else - recorded = RETIREMENT_FIELDS.select { |field| metadata.key?(field) && !blank?(metadata[field]) } + recorded = RETIREMENT_FIELDS.select { |field| metadata.key?(field) } next if recorded.empty? @errors << "#{location} is active but records retirement field(s): #{recorded.sort.join(', ')}" @@ -214,12 +219,22 @@ def validate_validity(artifacts, vocabulary) def validate_retirement(metadata, location, vocabulary) validate_retirement_date(metadata, location) + validate_retirement_reason(metadata, location, vocabulary) if metadata.key?('retired_reason') + return unless metadata.key?('retired_note') - reason = metadata['retired_reason'] - return if blank?(reason) + # Mirrors `type: string, minLength: 1` exactly. A whitespace-only note would + # be a stricter rule than the schema states, which is drift in the other + # direction. + note = metadata['retired_note'] + return if note.is_a?(String) && !note.empty? + + @errors << "#{location} records an empty 'retired_note'; write the explanation or omit the field" + end + def validate_retirement_reason(metadata, location, vocabulary) + reason = metadata['retired_reason'] unless vocabulary.fetch(:reasons).include?(reason) - @errors << "#{location} uses unknown retirement reason '#{reason}'" + @errors << "#{location} uses unknown retirement reason #{quoted(reason)}" return end @@ -229,6 +244,11 @@ def validate_retirement(metadata, location, vocabulary) @errors << "#{location} uses retirement reason '#{reason}', which applies to #{owner} artifacts only" end + # A null and an empty string are different mistakes in YAML; name which one. + def quoted(value) + value.nil? ? 'null' : "'#{value}'" + end + def validate_retirement_date(metadata, location) retired_on = metadata['retired_on'] if blank?(retired_on) diff --git a/test/validate_metamodel_test.rb b/test/validate_metamodel_test.rb index 723dd68..f3570c8 100644 --- a/test/validate_metamodel_test.rb +++ b/test/validate_metamodel_test.rb @@ -927,6 +927,31 @@ def test_active_artifact_carrying_retirement_fields_reports_an_error FileUtils.rm_rf(temp_dir) if temp_dir end + def test_present_but_empty_validity_metadata_reports_an_error + # Given: artifacts that declare validity or retirement fields with an empty + # or null value + temp_dir = ROOT.join('tmp/test-present-but-empty') + docs_dir = temp_dir.join('src/docs') + write_metadata_artifact(docs_dir.join('r-101-empty-validity.adoc'), + risk_metadata('R-101-empty-validity').merge('validity' => '')) + write_metadata_artifact(docs_dir.join('r-102-null-date.adoc'), + risk_metadata('R-102-null-date').merge('retired_on' => nil)) + write_metadata_artifact(docs_dir.join('r-103-empty-note.adoc'), + retired_risk_metadata('R-103-empty-note').merge('retired_note' => '')) + + # When: the validator runs + validator = build_validator(temp_dir, docs_dir) + validator.validate + errors = validator.errors.join("\n") + + # Then: it reports each one instead of reading it as omitted + assert_includes errors, "r-101-empty-validity.adoc uses unknown validity ''" + assert_includes errors, 'r-102-null-date.adoc is active but records retirement field(s): retired_on' + assert_includes errors, "r-103-empty-note.adoc records an empty 'retired_note'" + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + def test_risk_retirement_reason_on_another_artifact_type_reports_an_error # Given: a retired document that claims the risk-specific reason mitigated temp_dir = ROOT.join('tmp/test-reason-wrong-type') @@ -1054,6 +1079,50 @@ def test_validity_vocabulary_matches_the_artifact_schema end end + # Outcome parity for present-but-blank values. The vocabulary test above + # proves both sides name the same values; this one proves they reach the same + # verdict on the same metadata. Each expectation is the outcome the artifact + # schema prescribes, with the schema rule that decides it. There is no JSON + # Schema engine in the pinned toolchain to derive them mechanically, so they + # are written out and reviewed instead. + SCHEMA_OUTCOMES = [ + # [description, base, overrides, schema verdict, deciding schema rule] + ['no validity', :active, {}, :valid, 'validity is optional'], + ['validity active', :active, { 'validity' => 'active' }, :valid, 'validity enum'], + ['empty validity', :active, { 'validity' => '' }, :invalid, 'validity enum'], + ['null validity', :active, { 'validity' => nil }, :invalid, 'validity enum'], + ['active with null retired_on', :active, { 'retired_on' => nil }, :invalid, 'else: retired_on false'], + ['active with empty reason', :active, { 'retired_reason' => '' }, :invalid, 'else: retired_reason false'], + ['active with empty note', :active, { 'retired_note' => '' }, :invalid, 'else: retired_note false'], + ['complete retirement', :retired, {}, :valid, 'then: required retired_on'], + ['retired with null retired_on', :retired, { 'retired_on' => nil }, :invalid, 'retired_on type string'], + ['retired with empty reason', :retired, { 'retired_reason' => '' }, :invalid, 'retired_reason enum'], + ['retired with null reason', :retired, { 'retired_reason' => nil }, :invalid, 'retired_reason enum'], + ['retired with empty note', :retired, { 'retired_note' => '' }, :invalid, 'retired_note minLength 1'], + ['retired with null note', :retired, { 'retired_note' => nil }, :invalid, 'retired_note type string'], + ['retired with blank-ish note', :retired, { 'retired_note' => ' ' }, :valid, 'retired_note minLength 1'] + ].freeze + + def test_blank_and_null_values_reach_the_schema_verdict + SCHEMA_OUTCOMES.each_with_index do |(description, base, overrides, verdict, rule), index| + temp_dir = ROOT.join("tmp/test-schema-outcome-#{index}") + docs_dir = temp_dir.join('src/docs') + id = "R-#{200 + index}-case" + metadata = (base == :retired ? retired_risk_metadata(id) : risk_metadata(id)).merge(overrides) + write_metadata_artifact(docs_dir.join("r-#{200 + index}-case.adoc"), metadata) + + validator = build_validator(temp_dir, docs_dir) + validator.validate + actual = validator.errors.empty? ? :valid : :invalid + + assert_equal verdict, actual, + "#{description}: the schema says #{verdict} (#{rule}), the validator says #{actual}: " \ + "#{validator.errors.join('; ')}" + ensure + FileUtils.rm_rf(temp_dir) if temp_dir + end + end + def test_example_artifact_schema_matches_the_metamodel_schema canonical = YAML.load_file(ROOT.join('metamodel/artifact.schema.yaml')) example = YAML.load_file(ROOT.join('example/metamodel/artifact.schema.yaml'))