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/example/metamodel/artifact.schema.yaml b/example/metamodel/artifact.schema.yaml index 779a447..8e97007 100644 --- a/example/metamodel/artifact.schema.yaml +++ b/example/metamodel/artifact.schema.yaml @@ -42,7 +42,11 @@ 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. 'superseded' and 'deprecated' are transitional + exceptions that still carry temporal meaning here; ADR-009 defers their + migration. enum: - draft - proposed @@ -51,6 +55,41 @@ 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 + 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. Omit it + rather than leaving it empty. owner: type: string minLength: 1 @@ -134,6 +173,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..369c73e 100644 --- a/features/metamodel-validation.feature +++ b/features/metamodel-validation.feature @@ -90,6 +90,46 @@ 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: 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 + 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..3dd343e 100644 --- a/metamodel/artifact.schema.yaml +++ b/metamodel/artifact.schema.yaml @@ -42,7 +42,11 @@ 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. 'superseded' and 'deprecated' are transitional + exceptions that still carry temporal meaning here; ADR-009 defers their + migration. enum: - draft - proposed @@ -51,6 +55,41 @@ 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 + 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. Omit it + rather than leaving it empty. owner: type: string minLength: 1 @@ -134,6 +173,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..9051d51 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,121 @@ 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. + # + # 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) + + 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) } + 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) + validate_retirement_reason(metadata, location, vocabulary) if metadata.key?('retired_reason') + return unless metadata.key?('retired_note') + + # 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 #{quoted(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 + + # 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) + @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 +337,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 +461,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 +527,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 +548,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 +557,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 +824,8 @@ def cell(value) end class TraceabilityFragmentGenerator + include RelationValidity + attr_reader :output_paths def initialize(root:, docs_dir:) @@ -708,15 +871,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 +921,8 @@ def traceability_output_path(artifact) end class ImpactFragmentGenerator + include RelationValidity + attr_reader :output_paths def initialize(root:, docs_dir:) @@ -800,8 +967,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/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..e23062f 100644 --- a/skills/references/metadata-rules.md +++ b/skills/references/metadata-rules.md @@ -30,13 +30,41 @@ 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, 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 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/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 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 diff --git a/test/validate_metamodel_test.rb b/test/validate_metamodel_test.rb index 9262009..f3570c8 100644 --- a/test/validate_metamodel_test.rb +++ b/test/validate_metamodel_test.rb @@ -830,8 +830,368 @@ 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_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') + 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 + + # 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')) + + 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)