diff --git a/.abcd/development/brief/04-surfaces/05-intent.md b/.abcd/development/brief/04-surfaces/05-intent.md index 2df7b69..cc18bc4 100644 --- a/.abcd/development/brief/04-surfaces/05-intent.md +++ b/.abcd/development/brief/04-surfaces/05-intent.md @@ -226,6 +226,7 @@ kind: null # set by /abcd:intent plan: "standalone" | "bundle-memb suggested_kind: null # advisory, written by capture-time LLM classifier; can be ignored bundle: null # for kind: bundle-member, the bundle ID spec_id: null # or spc-N (set by /abcd:intent plan) +impact: null # "additive" | "breaking" | "fix" — the compatibility judgement the derived version is computed from. Never "internal" (a press-release-first intent is user-facing by definition), and required before the intent may move to shipped/ reclassification_history: [] # appended to by /abcd:intent reclassify (kind changes only) surface_history: [] # appended when an intent's user-facing surface shape changes (e.g., skill → sub-verb, top-level command → sub-verb, command → flag) WITHOUT changing kind. Distinct from reclassification_history. Schema: { date, from, to, reason }. Hand-edited or written by future tooling. --- diff --git a/.abcd/development/brief/04-surfaces/06-capture.md b/.abcd/development/brief/04-surfaces/06-capture.md index 593a937..e261ce7 100644 --- a/.abcd/development/brief/04-surfaces/06-capture.md +++ b/.abcd/development/brief/04-surfaces/06-capture.md @@ -37,6 +37,7 @@ schema_version: 1 id: iss-N # unpadded, mirrors itd-N slug: severity: nitpick|minor|major|critical +impact: additive|breaking|fix|internal # required in resolved/; drives the derived version and changelog inclusion category: bug|documentation|drift|inconsistency|tech-debt|security|ux|process|architectural-insight|future-work-seed|observation source: plan-review|impl-review|manual-test|review-followup|agent-finding|agent-observation|user-observation|drift-detection|memory-curation found_during: diff --git a/.abcd/development/intents/shipped/itd-3-modular-rules-loader.md b/.abcd/development/intents/shipped/itd-3-modular-rules-loader.md index c83425e..4514545 100644 --- a/.abcd/development/intents/shipped/itd-3-modular-rules-loader.md +++ b/.abcd/development/intents/shipped/itd-3-modular-rules-loader.md @@ -6,6 +6,7 @@ kind: standalone suggested_kind: null reclassification_history: [] severity: major +impact: additive --- # abcd Loads Its Own Rules On Demand diff --git a/.abcd/development/intents/shipped/itd-40-folder-classification.md b/.abcd/development/intents/shipped/itd-40-folder-classification.md index 84a536b..4692331 100644 --- a/.abcd/development/intents/shipped/itd-40-folder-classification.md +++ b/.abcd/development/intents/shipped/itd-40-folder-classification.md @@ -6,6 +6,7 @@ kind: standalone suggested_kind: standalone reclassification_history: [] severity: major +impact: additive --- # abcd Knows What Kind of Folder You're In diff --git a/.abcd/development/intents/shipped/itd-46-abcd-intent-quoted-text-create-symmetric.md b/.abcd/development/intents/shipped/itd-46-abcd-intent-quoted-text-create-symmetric.md index eeedf68..9568486 100644 --- a/.abcd/development/intents/shipped/itd-46-abcd-intent-quoted-text-create-symmetric.md +++ b/.abcd/development/intents/shipped/itd-46-abcd-intent-quoted-text-create-symmetric.md @@ -8,6 +8,7 @@ reclassification_history: [] related_adrs: [] builds_on: [itd-4] severity: minor +impact: additive --- # `/abcd:intent ""` And `/abcd:capture ""` Become Symmetric Create Paths diff --git a/.abcd/development/intents/shipped/itd-80-intent-lifecycle-automation.md b/.abcd/development/intents/shipped/itd-80-intent-lifecycle-automation.md index 366d2d7..a80a3f1 100644 --- a/.abcd/development/intents/shipped/itd-80-intent-lifecycle-automation.md +++ b/.abcd/development/intents/shipped/itd-80-intent-lifecycle-automation.md @@ -9,6 +9,7 @@ reclassification_history: [] builds_on: [itd-34, itd-1] related_adrs: [adr-25, adr-26, adr-27] severity: major +impact: additive --- # An Intent Ships Itself: `planned → shipped` Follows Its Spec Closing, With A Fidelity Audit Attached diff --git a/.abcd/development/intents/shipped/itd-89-start-the-transcript-clock.md b/.abcd/development/intents/shipped/itd-89-start-the-transcript-clock.md index 4d4427f..e0a23cf 100644 --- a/.abcd/development/intents/shipped/itd-89-start-the-transcript-clock.md +++ b/.abcd/development/intents/shipped/itd-89-start-the-transcript-clock.md @@ -7,6 +7,7 @@ suggested_kind: null reclassification_history: [] builds_on: [itd-3] severity: critical +impact: additive related_adrs: [adr-29] --- diff --git a/.abcd/record-lint.json b/.abcd/record-lint.json index 8f701c6..a335e02 100644 --- a/.abcd/record-lint.json +++ b/.abcd/record-lint.json @@ -21,7 +21,9 @@ "no_brittle_line_refs": {"enabled": true, "severity": "warn"}, "directory_coverage": {"enabled": true, "severity": "warn", "exempt": []}, "intent_lifecycle": {"enabled": true, "severity": "blocker", "intents_dir": "intents"}, + "intent_impact_valid": {"enabled": true, "severity": "blocker", "intents_dir": "intents"}, "issue_id_unique": {"enabled": true, "severity": "blocker", "issues_dir": ".abcd/work/issues"}, + "issue_impact_valid": {"enabled": true, "severity": "blocker", "issues_dir": ".abcd/work/issues"}, "spec_lifecycle": {"enabled": true, "severity": "blocker", "specs_dir": "specs", "intents_dir": "intents"}, "persona_registry": {"enabled": true, "severity": "blocker", "registry": ".abcd/development/personas.json"}, "context_status_free": {"enabled": true, "severity": "blocker", "target": ".abcd/work/CONTEXT.md"}, diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 9bbe087..8d5674e 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -640,6 +640,16 @@ parallel-agent merge contention bites. heading in the reviewed ship PR; the unchanged `auto-release.yml` greps it on merge and creates the tag. ADR-37 is preserved, not superseded: the reviewed ship IS the release decision, and the bot-on-main alternative stays rejected. +- 2026-07-21 — `impact` is a KNOWN property of the issue ledger schema, and + `internal/core/capture` validates it against the shared enum in + `internal/core/changelog` rather than a private copy. The back-fill added + `impact:` to every resolved issue, which `validateStrict`'s + additionalProperties:false allow-list rejected — `abcd capture` reported + "resolved 0" and skipped all 57 records as malformed. Accepting the field + without validating it was rejected: severity/category/source are all + enum-checked on read, and a third definition of the impact enum is exactly what + spc-10 exists to prevent. capture -> changelog is the import direction (no + cycle: changelog imports launch/frontmatter/gitutil only). - 2026-07-21 — The mapping table's per-tier status columns are a **ceiling**, not merely a prediction. Every conventions adapter already honours its row diff --git a/.abcd/work/issues/resolved/iss-1-launch-phase-ownership.md b/.abcd/work/issues/resolved/iss-1-launch-phase-ownership.md index c5a7f4c..0022223 100644 --- a/.abcd/work/issues/resolved/iss-1-launch-phase-ownership.md +++ b/.abcd/work/issues/resolved/iss-1-launch-phase-ownership.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-1" slug: "launch-phase-ownership" severity: "critical" +impact: internal category: "inconsistency" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-10-phase-0-user-command-wording.md b/.abcd/work/issues/resolved/iss-10-phase-0-user-command-wording.md index b25ca65..f46a16c 100644 --- a/.abcd/work/issues/resolved/iss-10-phase-0-user-command-wording.md +++ b/.abcd/work/issues/resolved/iss-10-phase-0-user-command-wording.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-10" slug: "phase-0-user-command-wording" severity: "major" +impact: internal category: "inconsistency" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-104-intent-quoted-text-create-treats-any-non-subcommand-first-to.md b/.abcd/work/issues/resolved/iss-104-intent-quoted-text-create-treats-any-non-subcommand-first-to.md index 19eaedf..33ca9e5 100644 --- a/.abcd/work/issues/resolved/iss-104-intent-quoted-text-create-treats-any-non-subcommand-first-to.md +++ b/.abcd/work/issues/resolved/iss-104-intent-quoted-text-create-treats-any-non-subcommand-first-to.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-104" slug: "intent-quoted-text-create-treats-any-non-subcommand-first-to" severity: "minor" +impact: fix category: "observation" source: "user-observation" found_during: "manual-capture" diff --git a/.abcd/work/issues/resolved/iss-106-gl002-stripinlinecode-restores-the-whole-line-on-an-unpaired.md b/.abcd/work/issues/resolved/iss-106-gl002-stripinlinecode-restores-the-whole-line-on-an-unpaired.md index a366756..613164e 100644 --- a/.abcd/work/issues/resolved/iss-106-gl002-stripinlinecode-restores-the-whole-line-on-an-unpaired.md +++ b/.abcd/work/issues/resolved/iss-106-gl002-stripinlinecode-restores-the-whole-line-on-an-unpaired.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-106" slug: "gl002-stripinlinecode-restores-the-whole-line-on-an-unpaired" severity: "minor" +impact: fix category: "observation" source: "user-observation" found_during: "manual-capture" diff --git a/.abcd/work/issues/resolved/iss-107-ahoy-install-visibility-v-yes-reports-already-up-to-date-wit.md b/.abcd/work/issues/resolved/iss-107-ahoy-install-visibility-v-yes-reports-already-up-to-date-wit.md index 665f5a2..7c7462b 100644 --- a/.abcd/work/issues/resolved/iss-107-ahoy-install-visibility-v-yes-reports-already-up-to-date-wit.md +++ b/.abcd/work/issues/resolved/iss-107-ahoy-install-visibility-v-yes-reports-already-up-to-date-wit.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-107" slug: "ahoy-install-visibility-v-yes-reports-already-up-to-date-wit" severity: "minor" +impact: fix category: "observation" source: "user-observation" found_during: "manual-capture" diff --git a/.abcd/work/issues/resolved/iss-108-iss-35-semantic-release-gate-was-self-referential-under-chan.md b/.abcd/work/issues/resolved/iss-108-iss-35-semantic-release-gate-was-self-referential-under-chan.md index e78f118..2cc48a4 100644 --- a/.abcd/work/issues/resolved/iss-108-iss-35-semantic-release-gate-was-self-referential-under-chan.md +++ b/.abcd/work/issues/resolved/iss-108-iss-35-semantic-release-gate-was-self-referential-under-chan.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-108" slug: "iss-35-semantic-release-gate-was-self-referential-under-chan" severity: "minor" +impact: internal category: "observation" source: "user-observation" found_during: "manual-capture" diff --git a/.abcd/work/issues/resolved/iss-109-ahoy-manual-guard-reads-carry-a-residual-lstat-readfile-toct.md b/.abcd/work/issues/resolved/iss-109-ahoy-manual-guard-reads-carry-a-residual-lstat-readfile-toct.md index 1673c13..653394d 100644 --- a/.abcd/work/issues/resolved/iss-109-ahoy-manual-guard-reads-carry-a-residual-lstat-readfile-toct.md +++ b/.abcd/work/issues/resolved/iss-109-ahoy-manual-guard-reads-carry-a-residual-lstat-readfile-toct.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-109" slug: "ahoy-manual-guard-reads-carry-a-residual-lstat-readfile-toct" severity: "minor" +impact: fix category: "observation" source: "user-observation" found_during: "manual-capture" diff --git a/.abcd/work/issues/resolved/iss-11-agent-count-drift.md b/.abcd/work/issues/resolved/iss-11-agent-count-drift.md index 03c1ac2..4a4af60 100644 --- a/.abcd/work/issues/resolved/iss-11-agent-count-drift.md +++ b/.abcd/work/issues/resolved/iss-11-agent-count-drift.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-11" slug: "agent-count-drift" severity: "minor" +impact: internal category: "inconsistency" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-113-convopenquestionssource-s-truncation-evidence-names-only-the.md b/.abcd/work/issues/resolved/iss-113-convopenquestionssource-s-truncation-evidence-names-only-the.md index 8e1ef44..1f9a6ee 100644 --- a/.abcd/work/issues/resolved/iss-113-convopenquestionssource-s-truncation-evidence-names-only-the.md +++ b/.abcd/work/issues/resolved/iss-113-convopenquestionssource-s-truncation-evidence-names-only-the.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-113" slug: "convopenquestionssource-s-truncation-evidence-names-only-the" severity: "nitpick" +impact: internal category: "tech-debt" source: "impl-review" found_during: "itd-96 P1 review" diff --git a/.abcd/work/issues/resolved/iss-12-memory-shipped-vs-planned.md b/.abcd/work/issues/resolved/iss-12-memory-shipped-vs-planned.md index b637555..893657a 100644 --- a/.abcd/work/issues/resolved/iss-12-memory-shipped-vs-planned.md +++ b/.abcd/work/issues/resolved/iss-12-memory-shipped-vs-planned.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-12" slug: "memory-shipped-vs-planned" severity: "minor" +impact: internal category: "inconsistency" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-13-brief-stale-status-narration.md b/.abcd/work/issues/resolved/iss-13-brief-stale-status-narration.md index 729afa9..ef39dc3 100644 --- a/.abcd/work/issues/resolved/iss-13-brief-stale-status-narration.md +++ b/.abcd/work/issues/resolved/iss-13-brief-stale-status-narration.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-13" slug: "brief-stale-status-narration" severity: "minor" +impact: internal category: "drift" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-14-stale-intent-section-deeplinks.md b/.abcd/work/issues/resolved/iss-14-stale-intent-section-deeplinks.md index 8c1d3e1..6a6a8bb 100644 --- a/.abcd/work/issues/resolved/iss-14-stale-intent-section-deeplinks.md +++ b/.abcd/work/issues/resolved/iss-14-stale-intent-section-deeplinks.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-14" slug: "stale-intent-section-deeplinks" severity: "nitpick" +impact: internal category: "documentation" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-15-capture-vs-record-lint.md b/.abcd/work/issues/resolved/iss-15-capture-vs-record-lint.md index a535d83..9184c1c 100644 --- a/.abcd/work/issues/resolved/iss-15-capture-vs-record-lint.md +++ b/.abcd/work/issues/resolved/iss-15-capture-vs-record-lint.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-15" slug: "capture-vs-record-lint" severity: "critical" +impact: internal category: "tech-debt" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-16-itd-66-carries-a-non-canonical-do-not-implement-banner-while.md b/.abcd/work/issues/resolved/iss-16-itd-66-carries-a-non-canonical-do-not-implement-banner-while.md index 5ef47f8..dcb8497 100644 --- a/.abcd/work/issues/resolved/iss-16-itd-66-carries-a-non-canonical-do-not-implement-banner-while.md +++ b/.abcd/work/issues/resolved/iss-16-itd-66-carries-a-non-canonical-do-not-implement-banner-while.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-16" slug: "itd-66-carries-a-non-canonical-do-not-implement-banner-while" severity: "major" +impact: internal category: "inconsistency" source: "agent-finding" found_during: "intent-dependency-sweep" diff --git a/.abcd/work/issues/resolved/iss-17-itd-50-sits-in-planned-but-carries-an-implementation-complet.md b/.abcd/work/issues/resolved/iss-17-itd-50-sits-in-planned-but-carries-an-implementation-complet.md index 13c6614..13ac188 100644 --- a/.abcd/work/issues/resolved/iss-17-itd-50-sits-in-planned-but-carries-an-implementation-complet.md +++ b/.abcd/work/issues/resolved/iss-17-itd-50-sits-in-planned-but-carries-an-implementation-complet.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-17" slug: "itd-50-sits-in-planned-but-carries-an-implementation-complet" severity: "major" +impact: internal category: "inconsistency" source: "agent-finding" found_during: "intent-dependency-sweep" diff --git a/.abcd/work/issues/resolved/iss-18-phase-order-violation-phase-0-scopes-itd-37-whose-declared-b.md b/.abcd/work/issues/resolved/iss-18-phase-order-violation-phase-0-scopes-itd-37-whose-declared-b.md index 33aa1d1..d8b328a 100644 --- a/.abcd/work/issues/resolved/iss-18-phase-order-violation-phase-0-scopes-itd-37-whose-declared-b.md +++ b/.abcd/work/issues/resolved/iss-18-phase-order-violation-phase-0-scopes-itd-37-whose-declared-b.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-18" slug: "phase-order-violation-phase-0-scopes-itd-37-whose-declared-b" severity: "major" +impact: internal category: "inconsistency" source: "agent-finding" found_during: "intent-dependency-sweep" diff --git a/.abcd/work/issues/resolved/iss-19-phase-3-doc-asserts-itd-27-depends-on-itd-42-while-itd-42-s.md b/.abcd/work/issues/resolved/iss-19-phase-3-doc-asserts-itd-27-depends-on-itd-42-while-itd-42-s.md index 183ef3e..d514f1d 100644 --- a/.abcd/work/issues/resolved/iss-19-phase-3-doc-asserts-itd-27-depends-on-itd-42-while-itd-42-s.md +++ b/.abcd/work/issues/resolved/iss-19-phase-3-doc-asserts-itd-27-depends-on-itd-42-while-itd-42-s.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-19" slug: "phase-3-doc-asserts-itd-27-depends-on-itd-42-while-itd-42-s" severity: "major" +impact: internal category: "inconsistency" source: "agent-finding" found_during: "intent-dependency-sweep" diff --git a/.abcd/work/issues/resolved/iss-2-release-version-source-of-truth.md b/.abcd/work/issues/resolved/iss-2-release-version-source-of-truth.md index 203c3db..e160aac 100644 --- a/.abcd/work/issues/resolved/iss-2-release-version-source-of-truth.md +++ b/.abcd/work/issues/resolved/iss-2-release-version-source-of-truth.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-2" slug: "release-version-source-of-truth" severity: "critical" +impact: internal category: "inconsistency" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-20-launch-cluster-unscheduled.md b/.abcd/work/issues/resolved/iss-20-launch-cluster-unscheduled.md index 13d1258..8de0484 100644 --- a/.abcd/work/issues/resolved/iss-20-launch-cluster-unscheduled.md +++ b/.abcd/work/issues/resolved/iss-20-launch-cluster-unscheduled.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-20" slug: "launch-cluster-unscheduled" severity: "major" +impact: internal category: "inconsistency" source: "agent-finding" found_during: "intent-dependency-sweep" diff --git a/.abcd/work/issues/resolved/iss-21-itd-launch-name-refs.md b/.abcd/work/issues/resolved/iss-21-itd-launch-name-refs.md index 971c9ff..ca2e329 100644 --- a/.abcd/work/issues/resolved/iss-21-itd-launch-name-refs.md +++ b/.abcd/work/issues/resolved/iss-21-itd-launch-name-refs.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-21" slug: "itd-launch-name-refs" severity: "minor" +impact: internal category: "inconsistency" source: "agent-finding" found_during: "intent-dependency-sweep" diff --git a/.abcd/work/issues/resolved/iss-22-itd-6-lifecycle-look.md b/.abcd/work/issues/resolved/iss-22-itd-6-lifecycle-look.md index d845633..f71acff 100644 --- a/.abcd/work/issues/resolved/iss-22-itd-6-lifecycle-look.md +++ b/.abcd/work/issues/resolved/iss-22-itd-6-lifecycle-look.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-22" slug: "itd-6-lifecycle-look" severity: "minor" +impact: internal category: "inconsistency" source: "agent-finding" found_during: "intent-dependency-sweep" diff --git a/.abcd/work/issues/resolved/iss-23-itd-36-adr-28-reconcile.md b/.abcd/work/issues/resolved/iss-23-itd-36-adr-28-reconcile.md index 4042c71..9dfa288 100644 --- a/.abcd/work/issues/resolved/iss-23-itd-36-adr-28-reconcile.md +++ b/.abcd/work/issues/resolved/iss-23-itd-36-adr-28-reconcile.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-23" slug: "itd-36-adr-28-reconcile" severity: "minor" +impact: internal category: "inconsistency" source: "agent-finding" found_during: "intent-dependency-sweep" diff --git a/.abcd/work/issues/resolved/iss-28-hermetic-git-test-env.md b/.abcd/work/issues/resolved/iss-28-hermetic-git-test-env.md index c135fa1..bd6bfa0 100644 --- a/.abcd/work/issues/resolved/iss-28-hermetic-git-test-env.md +++ b/.abcd/work/issues/resolved/iss-28-hermetic-git-test-env.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-28" slug: "hermetic-git-test-env" severity: "major" +impact: internal category: "future-work-seed" source: "agent-finding" found_during: "external agent finding, recorded 2026-07-08" diff --git a/.abcd/work/issues/resolved/iss-29-fail-closed-capture-surface.md b/.abcd/work/issues/resolved/iss-29-fail-closed-capture-surface.md index 5989b9b..ed30699 100644 --- a/.abcd/work/issues/resolved/iss-29-fail-closed-capture-surface.md +++ b/.abcd/work/issues/resolved/iss-29-fail-closed-capture-surface.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-29" slug: "fail-closed-capture-surface" severity: "major" +impact: fix category: "bug" source: "agent-finding" found_during: "2026-07-08 multi-agent review" diff --git a/.abcd/work/issues/resolved/iss-3-phase-membership-vs-lifecycle.md b/.abcd/work/issues/resolved/iss-3-phase-membership-vs-lifecycle.md index e151903..ed0a177 100644 --- a/.abcd/work/issues/resolved/iss-3-phase-membership-vs-lifecycle.md +++ b/.abcd/work/issues/resolved/iss-3-phase-membership-vs-lifecycle.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-3" slug: "phase-membership-vs-lifecycle" severity: "critical" +impact: internal category: "inconsistency" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-31-launch-dogfood-gate.md b/.abcd/work/issues/resolved/iss-31-launch-dogfood-gate.md index d5fc806..226fab4 100644 --- a/.abcd/work/issues/resolved/iss-31-launch-dogfood-gate.md +++ b/.abcd/work/issues/resolved/iss-31-launch-dogfood-gate.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-31" slug: "launch-dogfood-gate" severity: "major" +impact: fix category: "bug" source: "agent-finding" found_during: "2026-07-08 multi-agent review" diff --git a/.abcd/work/issues/resolved/iss-32-atomic-write-consolidation.md b/.abcd/work/issues/resolved/iss-32-atomic-write-consolidation.md index 361e942..c71eb1b 100644 --- a/.abcd/work/issues/resolved/iss-32-atomic-write-consolidation.md +++ b/.abcd/work/issues/resolved/iss-32-atomic-write-consolidation.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-32" slug: "atomic-write-consolidation" severity: "major" +impact: fix category: "tech-debt" source: "agent-finding" found_during: "2026-07-08 multi-agent review" diff --git a/.abcd/work/issues/resolved/iss-36-retired-name-banlist.md b/.abcd/work/issues/resolved/iss-36-retired-name-banlist.md index a2ed372..d055407 100644 --- a/.abcd/work/issues/resolved/iss-36-retired-name-banlist.md +++ b/.abcd/work/issues/resolved/iss-36-retired-name-banlist.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-36" slug: "retired-name-banlist" severity: "critical" +impact: internal category: "drift" source: "agent-finding" found_during: "2026-07-08 multi-agent review" diff --git a/.abcd/work/issues/resolved/iss-4-six-vs-seven-phases.md b/.abcd/work/issues/resolved/iss-4-six-vs-seven-phases.md index 8f14571..c85e8ca 100644 --- a/.abcd/work/issues/resolved/iss-4-six-vs-seven-phases.md +++ b/.abcd/work/issues/resolved/iss-4-six-vs-seven-phases.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-4" slug: "six-vs-seven-phases" severity: "major" +impact: internal category: "inconsistency" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-47-cli-reference-generation.md b/.abcd/work/issues/resolved/iss-47-cli-reference-generation.md index 858563d..4fc08f8 100644 --- a/.abcd/work/issues/resolved/iss-47-cli-reference-generation.md +++ b/.abcd/work/issues/resolved/iss-47-cli-reference-generation.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-47" slug: "cli-reference-generation" severity: "minor" +impact: additive category: "future-work-seed" source: "agent-finding" found_during: "2026-07-08 multi-agent review" diff --git a/.abcd/work/issues/resolved/iss-5-disembark-embark-phase-banners.md b/.abcd/work/issues/resolved/iss-5-disembark-embark-phase-banners.md index 5b9f007..5eef058 100644 --- a/.abcd/work/issues/resolved/iss-5-disembark-embark-phase-banners.md +++ b/.abcd/work/issues/resolved/iss-5-disembark-embark-phase-banners.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-5" slug: "disembark-embark-phase-banners" severity: "major" +impact: internal category: "inconsistency" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-51-context-scoped-successor-bans.md b/.abcd/work/issues/resolved/iss-51-context-scoped-successor-bans.md index 2f0750e..ab6fe6a 100644 --- a/.abcd/work/issues/resolved/iss-51-context-scoped-successor-bans.md +++ b/.abcd/work/issues/resolved/iss-51-context-scoped-successor-bans.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-51" slug: "context-scoped-successor-bans" severity: "minor" +impact: breaking category: "process" source: "agent-finding" found_during: "2026-07-09 practice/MVP/tool extraction" diff --git a/.abcd/work/issues/resolved/iss-57-capture-source-agent-provenance.md b/.abcd/work/issues/resolved/iss-57-capture-source-agent-provenance.md index 7b5d8d5..c83a1ac 100644 --- a/.abcd/work/issues/resolved/iss-57-capture-source-agent-provenance.md +++ b/.abcd/work/issues/resolved/iss-57-capture-source-agent-provenance.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-57" slug: "capture-source-agent-provenance" severity: "minor" +impact: additive category: "observation" source: "agent-finding" found_during: "autonomous-run" diff --git a/.abcd/work/issues/resolved/iss-58-history-store-bootstrap-prereq.md b/.abcd/work/issues/resolved/iss-58-history-store-bootstrap-prereq.md index 418541a..4b7435f 100644 --- a/.abcd/work/issues/resolved/iss-58-history-store-bootstrap-prereq.md +++ b/.abcd/work/issues/resolved/iss-58-history-store-bootstrap-prereq.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-58" slug: "history-store-bootstrap-prereq" severity: "minor" +impact: fix category: "observation" source: "agent-finding" found_during: "autonomous-run" diff --git a/.abcd/work/issues/resolved/iss-6-command-inventory-seven-vs-nine.md b/.abcd/work/issues/resolved/iss-6-command-inventory-seven-vs-nine.md index a9c8efe..c5fcad5 100644 --- a/.abcd/work/issues/resolved/iss-6-command-inventory-seven-vs-nine.md +++ b/.abcd/work/issues/resolved/iss-6-command-inventory-seven-vs-nine.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-6" slug: "command-inventory-seven-vs-nine" severity: "major" +impact: internal category: "inconsistency" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-61-launch-payload-drops-skills.md b/.abcd/work/issues/resolved/iss-61-launch-payload-drops-skills.md index 2d93fda..6c246c8 100644 --- a/.abcd/work/issues/resolved/iss-61-launch-payload-drops-skills.md +++ b/.abcd/work/issues/resolved/iss-61-launch-payload-drops-skills.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-61" slug: "launch-payload-drops-skills" severity: "major" +impact: internal category: "drift" source: "agent-finding" found_during: "autonomous-run" diff --git a/.abcd/work/issues/resolved/iss-63-identity-gate-followups.md b/.abcd/work/issues/resolved/iss-63-identity-gate-followups.md index 3e1d4e9..41a8d5c 100644 --- a/.abcd/work/issues/resolved/iss-63-identity-gate-followups.md +++ b/.abcd/work/issues/resolved/iss-63-identity-gate-followups.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-63" slug: "identity-gate-followups" severity: "minor" +impact: fix category: "future-work-seed" source: "agent-finding" found_during: "iss-62-security-review" diff --git a/.abcd/work/issues/resolved/iss-65-scanner-serialized-secret-crossleak.md b/.abcd/work/issues/resolved/iss-65-scanner-serialized-secret-crossleak.md index daeae3f..fa31812 100644 --- a/.abcd/work/issues/resolved/iss-65-scanner-serialized-secret-crossleak.md +++ b/.abcd/work/issues/resolved/iss-65-scanner-serialized-secret-crossleak.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-65" slug: "scanner-serialized-secret-crossleak" severity: "critical" +impact: fix category: "bug" source: "agent-finding" found_during: "clean-slate-sweep" diff --git a/.abcd/work/issues/resolved/iss-66-rules-loader-trust-boundary.md b/.abcd/work/issues/resolved/iss-66-rules-loader-trust-boundary.md index f4c64d2..a067d90 100644 --- a/.abcd/work/issues/resolved/iss-66-rules-loader-trust-boundary.md +++ b/.abcd/work/issues/resolved/iss-66-rules-loader-trust-boundary.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-66" slug: "rules-loader-trust-boundary" severity: "major" +impact: fix category: "bug" source: "agent-finding" found_during: "clean-slate-sweep" diff --git a/.abcd/work/issues/resolved/iss-67-intent-lifecycle-fidelity-gaps.md b/.abcd/work/issues/resolved/iss-67-intent-lifecycle-fidelity-gaps.md index 56abb4c..1d58279 100644 --- a/.abcd/work/issues/resolved/iss-67-intent-lifecycle-fidelity-gaps.md +++ b/.abcd/work/issues/resolved/iss-67-intent-lifecycle-fidelity-gaps.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-67" slug: "intent-lifecycle-fidelity-gaps" severity: "major" +impact: fix category: "bug" source: "agent-finding" found_during: "clean-slate-sweep" diff --git a/.abcd/work/issues/resolved/iss-68-spec-store-hardening.md b/.abcd/work/issues/resolved/iss-68-spec-store-hardening.md index ed57b14..b4254d2 100644 --- a/.abcd/work/issues/resolved/iss-68-spec-store-hardening.md +++ b/.abcd/work/issues/resolved/iss-68-spec-store-hardening.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-68" slug: "spec-store-hardening" severity: "minor" +impact: fix category: "bug" source: "agent-finding" found_during: "clean-slate-sweep" diff --git a/.abcd/work/issues/resolved/iss-69-frontmatter-canonical-consolidation.md b/.abcd/work/issues/resolved/iss-69-frontmatter-canonical-consolidation.md index 75c9a2e..b926c73 100644 --- a/.abcd/work/issues/resolved/iss-69-frontmatter-canonical-consolidation.md +++ b/.abcd/work/issues/resolved/iss-69-frontmatter-canonical-consolidation.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-69" slug: "frontmatter-canonical-consolidation" severity: "minor" +impact: fix category: "tech-debt" source: "agent-finding" found_during: "clean-slate-sweep" diff --git a/.abcd/work/issues/resolved/iss-7-hand-maintained-intent-counts.md b/.abcd/work/issues/resolved/iss-7-hand-maintained-intent-counts.md index bbf5f35..eff4d2e 100644 --- a/.abcd/work/issues/resolved/iss-7-hand-maintained-intent-counts.md +++ b/.abcd/work/issues/resolved/iss-7-hand-maintained-intent-counts.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-7" slug: "hand-maintained-intent-counts" severity: "major" +impact: internal category: "drift" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-70-lint-receipt-gate-detector-binding.md b/.abcd/work/issues/resolved/iss-70-lint-receipt-gate-detector-binding.md index 66cb804..e8c4d2c 100644 --- a/.abcd/work/issues/resolved/iss-70-lint-receipt-gate-detector-binding.md +++ b/.abcd/work/issues/resolved/iss-70-lint-receipt-gate-detector-binding.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-70" slug: "lint-receipt-gate-detector-binding" severity: "major" +impact: fix category: "bug" source: "agent-finding" found_during: "clean-slate-sweep" diff --git a/.abcd/work/issues/resolved/iss-71-capture-concurrency-and-forceid.md b/.abcd/work/issues/resolved/iss-71-capture-concurrency-and-forceid.md index d5ca762..cd48e65 100644 --- a/.abcd/work/issues/resolved/iss-71-capture-concurrency-and-forceid.md +++ b/.abcd/work/issues/resolved/iss-71-capture-concurrency-and-forceid.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-71" slug: "capture-concurrency-and-forceid" severity: "major" +impact: fix category: "bug" source: "agent-finding" found_during: "clean-slate-sweep" diff --git a/.abcd/work/issues/resolved/iss-72-core-isgitrepo-misses-worktrees.md b/.abcd/work/issues/resolved/iss-72-core-isgitrepo-misses-worktrees.md index 64e6833..b7486ff 100644 --- a/.abcd/work/issues/resolved/iss-72-core-isgitrepo-misses-worktrees.md +++ b/.abcd/work/issues/resolved/iss-72-core-isgitrepo-misses-worktrees.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-72" slug: "core-isgitrepo-misses-worktrees" severity: "nitpick" +impact: fix category: "bug" source: "agent-finding" found_during: "clean-slate-sweep" diff --git a/.abcd/work/issues/resolved/iss-73-relocate-memory-lint-logbook-output-and-scanner-skip-paths-f.md b/.abcd/work/issues/resolved/iss-73-relocate-memory-lint-logbook-output-and-scanner-skip-paths-f.md index a964194..24f779f 100644 --- a/.abcd/work/issues/resolved/iss-73-relocate-memory-lint-logbook-output-and-scanner-skip-paths-f.md +++ b/.abcd/work/issues/resolved/iss-73-relocate-memory-lint-logbook-output-and-scanner-skip-paths-f.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-73" slug: "relocate-memory-lint-logbook-output-and-scanner-skip-paths-f" severity: "minor" +impact: internal category: "tech-debt" source: "user-observation" found_during: "abcd-run-design" diff --git a/.abcd/work/issues/resolved/iss-74-duplicate-iss-56-id.md b/.abcd/work/issues/resolved/iss-74-duplicate-iss-56-id.md index 6ef536a..d63b4c2 100644 --- a/.abcd/work/issues/resolved/iss-74-duplicate-iss-56-id.md +++ b/.abcd/work/issues/resolved/iss-74-duplicate-iss-56-id.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-74" slug: "duplicate-iss-56-id" severity: "major" +impact: additive category: "bug" source: "user-observation" found_during: "abcd-run-design" diff --git a/.abcd/work/issues/resolved/iss-76-json-error-abspath-leak.md b/.abcd/work/issues/resolved/iss-76-json-error-abspath-leak.md index 6b84ab1..ec429c0 100644 --- a/.abcd/work/issues/resolved/iss-76-json-error-abspath-leak.md +++ b/.abcd/work/issues/resolved/iss-76-json-error-abspath-leak.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-76" slug: "json-error-abspath-leak" severity: "minor" +impact: fix category: "bug" source: "agent-finding" found_during: "2026-07-12 /abcd:run iss-29 security review" diff --git a/.abcd/work/issues/resolved/iss-77-launch-payload-omits-agents-hooks.md b/.abcd/work/issues/resolved/iss-77-launch-payload-omits-agents-hooks.md index 9de97f6..5e4a1f1 100644 --- a/.abcd/work/issues/resolved/iss-77-launch-payload-omits-agents-hooks.md +++ b/.abcd/work/issues/resolved/iss-77-launch-payload-omits-agents-hooks.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-77" slug: "launch-payload-omits-agents-hooks" severity: "major" +impact: fix category: "bug" source: "agent-finding" found_during: "2026-07-12 /abcd:run iss-31" diff --git a/.abcd/work/issues/resolved/iss-79-storeoriginal-inline-atomic-write.md b/.abcd/work/issues/resolved/iss-79-storeoriginal-inline-atomic-write.md index 6b8a75a..4e5da65 100644 --- a/.abcd/work/issues/resolved/iss-79-storeoriginal-inline-atomic-write.md +++ b/.abcd/work/issues/resolved/iss-79-storeoriginal-inline-atomic-write.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-79" slug: "storeoriginal-inline-atomic-write" severity: "minor" +impact: fix category: "tech-debt" source: "agent-finding" found_during: "2026-07-12 /abcd:run iss-32 review" diff --git a/.abcd/work/issues/resolved/iss-8-out-of-scope-stale-corpus.md b/.abcd/work/issues/resolved/iss-8-out-of-scope-stale-corpus.md index bda8def..12492ed 100644 --- a/.abcd/work/issues/resolved/iss-8-out-of-scope-stale-corpus.md +++ b/.abcd/work/issues/resolved/iss-8-out-of-scope-stale-corpus.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-8" slug: "out-of-scope-stale-corpus" severity: "major" +impact: internal category: "drift" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-81-machine-output-abspath-beyond-error-scrub.md b/.abcd/work/issues/resolved/iss-81-machine-output-abspath-beyond-error-scrub.md index 0bbadf0..2c87fe0 100644 --- a/.abcd/work/issues/resolved/iss-81-machine-output-abspath-beyond-error-scrub.md +++ b/.abcd/work/issues/resolved/iss-81-machine-output-abspath-beyond-error-scrub.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-81" slug: "machine-output-abspath-beyond-error-scrub" severity: "minor" +impact: fix category: "bug" source: "user-observation" found_during: "2026-07-12 iss-76 security re-review" diff --git a/.abcd/work/issues/resolved/iss-82-rules-inject-savestate-inline-atomic-write.md b/.abcd/work/issues/resolved/iss-82-rules-inject-savestate-inline-atomic-write.md index d99774b..f334cd5 100644 --- a/.abcd/work/issues/resolved/iss-82-rules-inject-savestate-inline-atomic-write.md +++ b/.abcd/work/issues/resolved/iss-82-rules-inject-savestate-inline-atomic-write.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-82" slug: "rules-inject-savestate-inline-atomic-write" severity: "minor" +impact: internal category: "tech-debt" source: "user-observation" found_during: "2026-07-12 iss-79 correctness review" diff --git a/.abcd/work/issues/resolved/iss-86-onboarding-audit-not-engine-backed.md b/.abcd/work/issues/resolved/iss-86-onboarding-audit-not-engine-backed.md index 8ee305b..5db7676 100644 --- a/.abcd/work/issues/resolved/iss-86-onboarding-audit-not-engine-backed.md +++ b/.abcd/work/issues/resolved/iss-86-onboarding-audit-not-engine-backed.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-86" slug: "onboarding-audit-not-engine-backed" severity: "major" +impact: additive category: "future-work-seed" source: "agent-finding" found_during: "2026-07-13 B1 dogfood: prepare-this-repo audit of Manuscripts" diff --git a/.abcd/work/issues/resolved/iss-88-managed-repo-label-overclaims.md b/.abcd/work/issues/resolved/iss-88-managed-repo-label-overclaims.md index 047d3eb..33c7184 100644 --- a/.abcd/work/issues/resolved/iss-88-managed-repo-label-overclaims.md +++ b/.abcd/work/issues/resolved/iss-88-managed-repo-label-overclaims.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-88" slug: "managed-repo-label-overclaims" severity: "minor" +impact: fix category: "observation" source: "agent-finding" found_during: "2026-07-13 B1 dogfood: prepare-this-repo audit of Manuscripts" diff --git a/.abcd/work/issues/resolved/iss-9-glossary-terminology-path.md b/.abcd/work/issues/resolved/iss-9-glossary-terminology-path.md index 5989a91..148e525 100644 --- a/.abcd/work/issues/resolved/iss-9-glossary-terminology-path.md +++ b/.abcd/work/issues/resolved/iss-9-glossary-terminology-path.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-9" slug: "glossary-terminology-path" severity: "major" +impact: internal category: "inconsistency" source: "review-followup" found_during: "roadmap-consistency-review" diff --git a/.abcd/work/issues/resolved/iss-97-detect-unbounded-marker-reads.md b/.abcd/work/issues/resolved/iss-97-detect-unbounded-marker-reads.md index bd3cb1f..5872c5e 100644 --- a/.abcd/work/issues/resolved/iss-97-detect-unbounded-marker-reads.md +++ b/.abcd/work/issues/resolved/iss-97-detect-unbounded-marker-reads.md @@ -3,6 +3,7 @@ schema_version: 1 id: "iss-97" slug: "detect-unbounded-marker-reads" severity: "minor" +impact: internal category: "security" source: "agent-finding" found_during: "iss-95 adversarial security review" diff --git a/internal/README.md b/internal/README.md index c921b71..754705b 100644 --- a/internal/README.md +++ b/internal/README.md @@ -11,7 +11,12 @@ plugin surface, and a future MCP server share one engine. - **`core/`** — the engine. One package per capability; no stdout, prompt text, or transport coupling. Currently: identity/version and the read-only status snapshot. Grows per phase (ahoy, launch, capture, memory, intent, brief, - review, spec, run, lifeboat, history). + review, spec, run, lifeboat, history, changelog). +- **`core/changelog/`** — release derivation. Holds `impact`, the one product + judgement a record declares, and derives the next SemVer from the records that + entered the terminal folders since the anchor tag. It owns the enum so the + lints that GATE the judgement (`core/lint`), the ledger reader that VALIDATES + it (`core/capture`), and the derivation that CONSUMES it cannot drift apart. - **`core/lifeboat/`** — the brief↔lifeboat contract. `mapping.go` is the single source of truth for which brief section a lifeboat fills from which source tier, and it is rendered into the brief's `00-meta.md` with a test asserting diff --git a/internal/core/capture/parse_test.go b/internal/core/capture/parse_test.go index 5074b8b..bb8405b 100644 --- a/internal/core/capture/parse_test.go +++ b/internal/core/capture/parse_test.go @@ -85,3 +85,54 @@ func TestValidateStrictTypeChecksResolvedBy(t *testing.T) { t.Fatalf("valid resolved_by rejected: %v", err) } } + +// TestValidateStrictImpact pins the issue schema against the impact field +// (spc-10): the ledger's own records carry it, so a reader that rejects it as an +// unknown property silently drops every resolved issue out of `abcd capture`. +// The value is checked against the one shared enum, exactly as severity, +// category, and source are, so a mislabel is caught on read rather than at the +// release cut. +func TestValidateStrictImpact(t *testing.T) { + base := func() map[string]any { + return map[string]any{ + "schema_version": 1, + "id": "iss-1", + "slug": "x", + "severity": "minor", + "category": "bug", + "source": "agent-finding", + "found_during": "review", + } + } + for _, v := range []string{"additive", "breaking", "fix", "internal"} { + fm := base() + fm["impact"] = v + if err := validateStrict(fm); err != nil { + t.Errorf("impact %q rejected: %v", v, err) + } + } + for _, v := range []any{"braking", "Additive", 42} { + fm := base() + fm["impact"] = v + if err := validateStrict(fm); err == nil { + t.Errorf("invalid impact %#v was accepted", v) + } + } + // The YAML nulls read as ABSENT, not as a malformed value — the same verdict + // the record-lint blocker issue_impact_valid reaches through + // frontmatter.IsNull. One value must not be legal to one gate and malformed + // to the other: `impact: null` on an open issue would otherwise pass the lint + // and then fail `abcd capture resolve` on the very record it judged fine. + for _, v := range []string{"", "null", "~"} { + fm := base() + fm["impact"] = v + if err := validateStrict(fm); err != nil { + t.Errorf("null-ish impact %q rejected: %v", v, err) + } + } + // Absent stays legal: an open issue has not been judged yet, and the + // record-lint blocker is what gates the move into resolved/. + if err := validateStrict(base()); err != nil { + t.Fatalf("absent impact rejected: %v", err) + } +} diff --git a/internal/core/capture/validate.go b/internal/core/capture/validate.go index f27bb29..fce99e7 100644 --- a/internal/core/capture/validate.go +++ b/internal/core/capture/validate.go @@ -5,6 +5,9 @@ import ( "path/filepath" "regexp" "strings" + + "github.com/REPPL/abcd-cli/internal/core/changelog" + "github.com/REPPL/abcd-cli/internal/core/frontmatter" ) // knownFields is the additionalProperties:false allow-list from @@ -16,6 +19,12 @@ var knownFields = map[string]bool{ "promoted_to": true, "related_specs": true, "related_issues": true, "synthesis_clusters": true, "wontfix_reason": true, "resolution": true, "resolved_by": true, "blocked_by": true, + // impact is the product judgement the derived version and the generated + // changelog are computed from (spc-10). It is optional here — an open issue + // has not been judged yet, and the record-lint blocker issue_impact_valid is + // what gates the move into resolved/ — but it must be a KNOWN property, or + // the reader drops every judged record as malformed. + "impact": true, // created/updated are no longer written, but legacy ledgers still carry // them. Tolerate (accept, then drop) them on read so an existing committed // ledger is not rejected as an unknown property; the reader ignores their @@ -76,6 +85,29 @@ func validateStrict(fm map[string]any) error { return fmt.Errorf("%w: found_during must be non-empty", ErrMalformedFrontmatter) } + // impact is optional but, when written, is checked against the ONE shared + // enum (internal/core/changelog) rather than a private copy — the same enum + // the record lint gates on and the release derivation consumes, so all three + // can never disagree about what a legal judgement is. + // + // A YAML null reads as ABSENT rather than as a value, via the repo's one null + // test (frontmatter.IsNull) — the same test the record-lint blocker + // issue_impact_valid applies. Both gates must reach the same verdict on one + // value: an open issue written `impact: null` ("not judged yet", the schema's + // own convention) would otherwise pass the lint and then be refused here, + // failing `abcd capture resolve` on a record nothing is wrong with. + if v, present := fm["impact"]; present { + s, isStr := v.(string) + if !isStr { + return fmt.Errorf("%w: %q must be a string", ErrMalformedFrontmatter, "impact") + } + if !frontmatter.IsNull(s) { + if _, err := changelog.ParseImpact(s); err != nil { + return fmt.Errorf("%w: %v", ErrMalformedFrontmatter, err) + } + } + } + // Optional scalar strings. for _, opt := range []string{"found_at", "details", "suggested_fix", "wontfix_reason", "resolution", "promoted_to"} { if v, present := fm[opt]; present { diff --git a/internal/core/changelog/anchor.go b/internal/core/changelog/anchor.go new file mode 100644 index 0000000..5ee29f6 --- /dev/null +++ b/internal/core/changelog/anchor.go @@ -0,0 +1,100 @@ +package changelog + +import ( + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/REPPL/abcd-cli/internal/core/launch" + "github.com/REPPL/abcd-cli/internal/fsutil" +) + +// maxChangelogBytes caps the guarded CHANGELOG read. The file is a few hundred +// kilobytes of prose at most; the cap exists so a symlinked or replaced +// CHANGELOG.md cannot stream an unbounded file into a preview command. +const maxChangelogBytes = 4 << 20 + +// datedHeadingRe matches a dated release heading in CHANGELOG.md, and it is a +// CONTRACT, not a convenience: .github/workflows/auto-release.yml greps +// `^## \[v?[0-9]+\.[0-9]+\.[0-9]+\] - ` for the heading it turns into a git tag. +// Reading a different set of headings here would let derivation reason about a +// release the tagger never sees. The only difference from the workflow's pattern +// is the capture group (which changes nothing about what matches); a test pins +// both the string and their agreement over fixtures. The trailing " - " is +// load-bearing: it is what skips the undated "## [Unreleased]" heading. +var datedHeadingRe = regexp.MustCompile(`^## \[v?([0-9]+\.[0-9]+\.[0-9]+)\] - `) + +// LatestReleaseTag resolves the base a release is derived against: the newest +// `vX.Y.Z` git tag under root, by SemVer order. +// +// The tag — not the newest CHANGELOG heading — is the anchor, and the +// distinction is load-bearing. auto-release.yml creates the tag AFTER the ship +// PR merges, so between the merge and the tag the heading is one release ahead +// of the tag and the two describe different releases; deriving from the heading +// in that window would compute the cut against a base that does not exist yet. +// The tag is the only immutable anchor. +// +// Tag listing and strictness are delegated to launch.GitExistingTags, the +// repo's one tag reader: it keeps only strict `v`-prefixed SemVer release cores, +// dropping prereleases (whose core would be a phantom release) and anything +// else. Ordering is launch.CoreGreater, the repo's one version comparison, so +// v0.10.0 wins over v0.9.0 where a lexical maximum would not. +// +// A repo with no release tag returns found=false and no error: an absent anchor +// is a state the caller decides about, not a failure to read git. +func LatestReleaseTag(root string) (launch.Semver, bool, error) { + tags, err := launch.GitExistingTags(root) + if err != nil { + return launch.Semver{}, false, err + } + var newest launch.Semver + found := false + for _, tag := range tags { + if !found || launch.CoreGreater(tag, newest) { + newest, found = tag, true + } + } + return newest, found, nil +} + +// LatestChangelogVersion returns the version of the newest dated heading in +// root's CHANGELOG.md — the heading auto-release.yml would tag on the next push +// to main. +// +// "Newest" is POSITIONAL, not the SemVer maximum: like the workflow's `grep +// -m1`, the first dated heading in the file wins, because Keep-a-Changelog order +// (newest first) is the record's own convention and mirroring the tagger exactly +// matters more here than being independently clever about ordering. +// +// The read is guarded and READ-ONLY. Nothing in this package writes to +// CHANGELOG.md: the whole point of derived releases is that changelog lines are +// generated from records at the ship, never edited in place by a preview. +// +// An absent CHANGELOG.md returns found=false and no error — the file is not this +// package's to require; the ship verb that writes the heading is. +func LatestChangelogVersion(root string) (launch.Semver, bool, error) { + data, err := fsutil.ReadGuarded(filepath.Join(root, "CHANGELOG.md"), maxChangelogBytes) + if err != nil { + if os.IsNotExist(err) { + return launch.Semver{}, false, nil + } + return launch.Semver{}, false, err + } + for _, line := range strings.Split(string(data), "\n") { + m := datedHeadingRe.FindStringSubmatch(strings.TrimRight(line, "\r")) + if m == nil { + continue + } + v, err := launch.ParseSemver(m[1]) + if err != nil { + // The regexp already constrains the shape, so this is only + // reachable for a leading-zero component ("## [0.04.0] - …") that + // strict SemVer rejects. Naming it beats deriving against a version + // the tagger and this package would read differently. + return launch.Semver{}, false, err + } + return v, true, nil + } + return launch.Semver{}, false, nil +} diff --git a/internal/core/changelog/anchor_test.go b/internal/core/changelog/anchor_test.go new file mode 100644 index 0000000..2710354 --- /dev/null +++ b/internal/core/changelog/anchor_test.go @@ -0,0 +1,167 @@ +package changelog + +import ( + "os" + "regexp" + "strings" + "testing" +) + +// TestLatestReleaseTagOrdersBySemverNotLexically pins the anchor's ordering: the +// newest tag is the maximum by SemVer, so v0.10.0 beats v0.9.0. A lexical +// maximum would pick v0.9.0 and derive the next release against a base two +// releases stale. +func TestLatestReleaseTagOrdersBySemverNotLexically(t *testing.T) { + r := newFixtureRepo(t) + r.commit("initial") + for _, tag := range []string{"v0.1.0", "v0.9.0", "v0.10.0", "v0.2.0"} { + r.git("tag", tag) + } + + got, ok, err := LatestReleaseTag(r.root) + if err != nil { + t.Fatalf("LatestReleaseTag: %v", err) + } + if !ok { + t.Fatal("expected a tag to be found") + } + if got.String() != "0.10.0" { + t.Errorf("LatestReleaseTag = %s, want 0.10.0 (semver order, not lexical)", got) + } +} + +// TestLatestReleaseTagIgnoresNonRelease pins that only strict vX.Y.Z release +// tags anchor a cut: a marketing tag, a two-part tag, an unprefixed one, and a +// prerelease all render or sort as something they are not. +func TestLatestReleaseTagIgnoresNonRelease(t *testing.T) { + r := newFixtureRepo(t) + r.commit("initial") + for _, tag := range []string{"v0.3.0", "v1.2", "release-9", "v9.9.9-rc1", "1.5.0", "v0.3.0.1"} { + r.git("tag", tag) + } + + got, ok, err := LatestReleaseTag(r.root) + if err != nil { + t.Fatalf("LatestReleaseTag: %v", err) + } + if !ok || got.String() != "0.3.0" { + t.Errorf("LatestReleaseTag = %s (found=%v), want 0.3.0", got, ok) + } +} + +// TestLatestReleaseTagNoTags pins the untagged repo: no tag is not an error, it +// is an absent anchor the caller decides about. +func TestLatestReleaseTagNoTags(t *testing.T) { + r := newFixtureRepo(t) + r.commit("initial") + + if _, ok, err := LatestReleaseTag(r.root); err != nil || ok { + t.Errorf("LatestReleaseTag = (found=%v, err=%v), want (false, nil)", ok, err) + } +} + +// TestLatestChangelogVersion reads the newest dated heading — the one +// auto-release.yml would tag — and ignores everything that is not one. +func TestLatestChangelogVersion(t *testing.T) { + r := newFixtureRepo(t) + r.write("CHANGELOG.md", strings.Join([]string{ + "# Changelog", + "", + "## [Unreleased]", + "", + "### Added", + "", + "- something", + "", + "## [0.4.0] - 2026-07-21", + "", + "## [0.3.0] - 2026-07-18", + "", + "## [v0.1.0] - 2026-07-07", + "", + }, "\n")) + r.commit("changelog") + + got, ok, err := LatestChangelogVersion(r.root) + if err != nil { + t.Fatalf("LatestChangelogVersion: %v", err) + } + if !ok || got.String() != "0.4.0" { + t.Errorf("LatestChangelogVersion = %s (found=%v), want 0.4.0", got, ok) + } +} + +// TestLatestChangelogVersionAbsent pins that a repo with no CHANGELOG.md yields +// no heading rather than an error. +func TestLatestChangelogVersionAbsent(t *testing.T) { + r := newFixtureRepo(t) + r.commit("initial") + + if _, ok, err := LatestChangelogVersion(r.root); err != nil || ok { + t.Errorf("LatestChangelogVersion = (found=%v, err=%v), want (false, nil)", ok, err) + } +} + +// headingFixtures are the lines the two heading matchers must agree on. +var headingFixtures = []string{ + "## [0.4.0] - 2026-07-21", + "## [v0.1.0] - 2026-07-07", + "## [10.20.30] - 2026-01-01", + "## [Unreleased]", + "## [0.4.0]", + "## [0.4.0] — 2026-07-21", + "### [0.4.0] - 2026-07-21", + " ## [0.4.0] - 2026-07-21", + "## [0.4] - 2026-07-21", + "##[0.4.0] - 2026-07-21", + "a ## [0.4.0] - 2026-07-21", +} + +// TestHeadingRegexMatchesTheWorkflow is the contract test between this package +// and .github/workflows/auto-release.yml: the workflow's grep decides which +// heading gets tagged, so a derivation that reads a DIFFERENT set of headings +// would derive against a release the tagger never sees. The workflow's own +// pattern is extracted from the file (a workflow edit therefore breaks this +// test loudly) and both are run over the same fixtures. +func TestHeadingRegexMatchesTheWorkflow(t *testing.T) { + pattern := workflowHeadingPattern(t) + workflowRe, err := regexp.Compile(pattern) + if err != nil { + t.Fatalf("workflow pattern %q does not compile: %v", pattern, err) + } + // The package pattern differs from the workflow's by its capture group + // alone, which changes nothing about what matches; strip the parentheses + // (the pattern contains no others) and the two strings must be identical. + bare := strings.NewReplacer("(", "", ")", "").Replace(datedHeadingRe.String()) + if pattern != bare { + t.Errorf("heading pattern drift:\n workflow: %s\n package: %s", pattern, bare) + } + for _, line := range headingFixtures { + if got, want := datedHeadingRe.MatchString(line), workflowRe.MatchString(line); got != want { + t.Errorf("disagreement on %q: package=%v workflow=%v", line, got, want) + } + } +} + +// workflowHeadingPattern extracts the single-quoted -E pattern from the +// auto-release workflow's grep line. +func workflowHeadingPattern(t *testing.T) string { + t.Helper() + raw, err := os.ReadFile("../../../.github/workflows/auto-release.yml") + if err != nil { + t.Fatalf("read workflow: %v", err) + } + for _, line := range strings.Split(string(raw), "\n") { + if !strings.Contains(line, "grep -m1 -E '") { + continue + } + rest := line[strings.Index(line, "grep -m1 -E '")+len("grep -m1 -E '"):] + end := strings.Index(rest, "'") + if end < 0 { + t.Fatalf("unterminated grep pattern in workflow line: %q", line) + } + return rest[:end] + } + t.Fatal("no `grep -m1 -E` line found in auto-release.yml; the CHANGELOG contract moved") + return "" +} diff --git a/internal/core/changelog/derive.go b/internal/core/changelog/derive.go new file mode 100644 index 0000000..75ec334 --- /dev/null +++ b/internal/core/changelog/derive.go @@ -0,0 +1,119 @@ +package changelog + +import ( + "strings" + + "github.com/REPPL/abcd-cli/internal/core/launch" +) + +// Derivation is the deterministic outcome of a release cut: what the next +// version is, which records decide it, and — when it cannot be decided — why. +// +// A refusal is modelled as a VALUE, not an error, in the same shape as +// launch.RetentionPlan: "this cut cannot be derived" is a legitimate result a +// read-only preview must render, not an exceptional failure. Errors are reserved +// for "the repository could not be read at all". +// +// Three states are deliberately distinguishable, because a caller that conflated +// them would write a wrong CHANGELOG heading: +// +// Refused — do not derive; RefusalReason says what to fix. +// !Refused, !Bumped — nothing to release; write no heading. +// !Refused, Bumped — Next/NextTag are the release. +type Derivation struct { + // Base is the version of the anchor tag the cut is measured from. + Base launch.Semver + // BaseTag is Base as a git tag ("v0.3.0"), empty when no anchor resolved. + BaseTag string + // Records is the cut: what entered and left the terminal record folders. + Records RecordSet + // Bump is the strongest impact in the cut — the judgement that decides the + // version. ImpactInternal means nothing user-facing shipped. + Bump Impact + // Next is the derived version; only meaningful when Bumped. + Next launch.Semver + // NextTag is Next as a git tag, empty when nothing is released. + NextTag string + // Bumped reports whether the cut moves the version at all. + Bumped bool + // Refused reports that the cut must not be derived. + Refused bool + // RefusalReason names what to fix; empty unless Refused. + RefusalReason string +} + +// Derive runs the whole deterministic release derivation over the repository at +// root and writes nothing. It is the one composition of this package's parts: +// resolve the anchor tag, refuse a release already in flight, diff the record +// end-states, then apply the version policy to the strongest impact in the cut. +// +// It refuses (rather than deriving a number that would be wrong) in three cases, +// each fail-closed: +// +// - No release tag. There is no immutable base; inventing one would report +// every record ever written as this release's contents. +// - The newest CHANGELOG heading is ahead of the newest tag. auto-release.yml +// tags AFTER the ship PR merges, so this is the post-merge/pre-tag window: +// the heading and the tag describe different releases and the base is +// mismatched. The next cut derives correctly once the tag lands. +// - A record ADDED by the cut carries no valid impact. An unlabelled record +// ranks below every real impact, so deriving over it would silently +// under-bump a release that may contain a break. The lints gate this at the +// record lifecycle; this is the backstop at the cut, and it names both the +// record and the file to edit. The removed side cannot refuse — its blob is +// read from the anchor tag's immutable tree, so an unlabelled one is +// unfixable by definition (see RecordSet.UnlabelledAdded). +func Derive(root string) (Derivation, error) { + var d Derivation + + base, hasTag, err := LatestReleaseTag(root) + if err != nil { + return Derivation{}, err + } + if !hasTag { + return refuse(d, "no release tag found — a cut needs an immutable base (tag the current release first)"), nil + } + d.Base, d.BaseTag = base, base.Tag() + + heading, hasHeading, err := LatestChangelogVersion(root) + if err != nil { + return Derivation{}, err + } + if hasHeading && launch.CoreGreater(heading, base) { + return refuse(d, "release "+heading.Tag()+" in flight — tag pending (the newest CHANGELOG heading is ahead of "+d.BaseTag+")"), nil + } + + records, err := ShippedSince(root, d.BaseTag) + if err != nil { + return Derivation{}, err + } + d.Records = records + + if unlabelled := records.UnlabelledAdded(); len(unlabelled) > 0 { + names := make([]string, 0, len(unlabelled)) + for _, rec := range unlabelled { + names = append(names, rec.ID+" ("+rec.Path+": "+rec.ImpactErr+")") + } + return refuse(d, "records added by the cut carry no valid impact: "+strings.Join(names, "; ")), nil + } + + d.Bump = records.Impact() + next, bumped := DeriveNext(base, d.Bump) + if bumped { + d.Next, d.NextTag, d.Bumped = next, next.Tag(), true + } + return d, nil +} + +// refuse stamps a refusal on a partially-filled derivation, clearing anything +// that could read as a derived release. Whatever was resolved before the refusal +// (the anchor, the records) is kept, because it is what the operator needs to +// see to fix the cut. +func refuse(d Derivation, reason string) Derivation { + d.Refused = true + d.RefusalReason = reason + d.Bumped = false + d.Next = launch.Semver{} + d.NextTag = "" + return d +} diff --git a/internal/core/changelog/derive_test.go b/internal/core/changelog/derive_test.go new file mode 100644 index 0000000..0c1a043 --- /dev/null +++ b/internal/core/changelog/derive_test.go @@ -0,0 +1,266 @@ +package changelog + +import ( + "reflect" + "strings" + "testing" +) + +// changelogWith renders a CHANGELOG.md whose newest dated heading is version. +func changelogWith(version string) string { + return strings.Join([]string{ + "# Changelog", + "", + "## [Unreleased]", + "", + "## [" + version + "] - 2026-07-21", + "", + "### Added", + "", + "- the release " + version, + "", + }, "\n") +} + +// releasedRepo is a repo at v0.1.0 whose CHANGELOG heading agrees with the tag — +// the steady state a derivation runs against. +func releasedRepo(t *testing.T) *fixtureRepo { + t.Helper() + r := newFixtureRepo(t) + r.write("CHANGELOG.md", changelogWith("0.1.0")) + r.record(shippedDir+"itd-1-first.md", "itd-1", "additive") + r.commit("release 0.1.0") + r.git("tag", "v0.1.0") + return r +} + +// TestDeriveHappyPath walks the whole deterministic pipeline: anchor on the tag, +// diff the record trees, take the strongest impact, apply the pre-1.0 table. +func TestDeriveHappyPath(t *testing.T) { + r := releasedRepo(t) + r.record(shippedDir+"itd-2-second.md", "itd-2", "additive") + r.record(resolvedDir+"iss-9-nine.md", "iss-9", "internal") + r.commit("ship itd-2, resolve iss-9") + + d, err := Derive(r.root) + if err != nil { + t.Fatalf("Derive: %v", err) + } + if d.Refused { + t.Fatalf("unexpected refusal: %s", d.RefusalReason) + } + if !d.Bumped { + t.Fatal("a cut with an additive record must bump") + } + if d.BaseTag != "v0.1.0" || d.Base.String() != "0.1.0" { + t.Errorf("base = %s (%s), want v0.1.0", d.Base, d.BaseTag) + } + if d.Bump != ImpactAdditive { + t.Errorf("bump = %q, want additive", d.Bump) + } + if d.Next.String() != "0.1.1" || d.NextTag != "v0.1.1" { + t.Errorf("next = %s (%s), want 0.1.1 (v0.1.1)", d.Next, d.NextTag) + } + if got := ids(d.Records.Added); len(got) != 2 { + t.Errorf("added = %v, want both records in the cut", got) + } + if got := ids(d.Records.ChangelogRequired()); len(got) != 1 || got[0] != "itd-2" { + t.Errorf("changelog-required = %v, want [itd-2] (internal excluded)", got) + } +} + +// TestDeriveRefusesReleaseInFlight is the post-merge/pre-tag guard: once the +// ship PR has merged, the newest CHANGELOG heading names a release +// auto-release.yml has not tagged yet. Deriving then would compute the cut +// against a base that does not exist, so the derivation refuses by name instead. +func TestDeriveRefusesReleaseInFlight(t *testing.T) { + r := releasedRepo(t) + r.write("CHANGELOG.md", changelogWith("0.2.0")) + r.record(shippedDir+"itd-2-second.md", "itd-2", "breaking") + r.commit("the ship PR merged; the tag has not landed yet") + + d, err := Derive(r.root) + if err != nil { + t.Fatalf("Derive: %v", err) + } + if !d.Refused { + t.Fatalf("expected a refusal; got next=%s bumped=%v", d.Next, d.Bumped) + } + if d.Bumped { + t.Error("a refusal must derive no version") + } + for _, want := range []string{"0.2.0", "in flight", "tag pending"} { + if !strings.Contains(d.RefusalReason, want) { + t.Errorf("refusal %q does not mention %q", d.RefusalReason, want) + } + } +} + +// TestDeriveEmptySetDoesNotBump pins "nothing to release": a cut with no records +// is not a refusal (nothing is wrong) and not a version (nothing changed) — the +// caller must be able to tell the two apart and write no heading. +func TestDeriveEmptySetDoesNotBump(t *testing.T) { + r := releasedRepo(t) + r.write("docs/README.md", "# docs\n") + r.commit("documentation only") + + d, err := Derive(r.root) + if err != nil { + t.Fatalf("Derive: %v", err) + } + if d.Refused { + t.Fatalf("an empty cut is not a refusal: %s", d.RefusalReason) + } + if d.Bumped { + t.Errorf("an empty cut must not bump; got %s", d.Next) + } + if d.Bump != ImpactInternal { + t.Errorf("bump = %q, want internal", d.Bump) + } + if d.NextTag != "" { + t.Errorf("NextTag = %q, want empty when nothing is released", d.NextTag) + } +} + +// TestDeriveRefusesUnlabelledRecord pins fail-closed labelling: a record ADDED +// to the cut with no valid impact ranks below every real one, so deriving over +// it would under-bump a release that may contain a break. The refusal names the +// record AND the file to edit, so the remedy is followable without a search. +func TestDeriveRefusesUnlabelledRecord(t *testing.T) { + r := releasedRepo(t) + r.write(shippedDir+"itd-7-unlabelled.md", "---\nid: itd-7\n---\n# itd-7\n") + r.commit("ship an unlabelled intent") + + d, err := Derive(r.root) + if err != nil { + t.Fatalf("Derive: %v", err) + } + if !d.Refused { + t.Fatalf("expected a refusal; got next=%s bumped=%v", d.Next, d.Bumped) + } + for _, want := range []string{"itd-7", shippedDir + "itd-7-unlabelled.md"} { + if !strings.Contains(d.RefusalReason, want) { + t.Errorf("refusal %q does not name %q", d.RefusalReason, want) + } + } +} + +// TestDeriveCarriesUnlabelledRemovedRecord pins that ONLY the added side of a +// cut can refuse it. A removed record is read from the anchor tag's immutable +// tree, which may predate the impact back-fill, so refusing on it would name a +// record the operator cannot label — at HEAD the file is either gone +// (supersession) or already carries a valid impact under its new name +// (re-slug) — and would block every release until the move was reverted. +func TestDeriveCarriesUnlabelledRemovedRecord(t *testing.T) { + cases := map[string]struct { + mutate func(r *fixtureRepo) + wantAdded []string + wantBumped bool + }{ + "supersession": { + mutate: func(r *fixtureRepo) { + r.remove(resolvedDir + "iss-4-unlabelled.md") + r.commit("supersede iss-4 out of resolved/") + }, + wantAdded: []string{}, + wantBumped: false, + }, + "re-slug": { + mutate: func(r *fixtureRepo) { + r.remove(resolvedDir + "iss-4-unlabelled.md") + r.record(resolvedDir+"iss-4-renamed.md", "iss-4", "fix") + r.commit("re-slug iss-4 and label it at HEAD") + }, + wantAdded: []string{"iss-4"}, + wantBumped: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + r := newFixtureRepo(t) + r.write("CHANGELOG.md", changelogWith("0.1.0")) + r.record(shippedDir+"itd-1-first.md", "itd-1", "additive") + // Written before the tag with no impact: the state every record + // resolved before the back-fill is frozen in at v0.3.0. + r.write(resolvedDir+"iss-4-unlabelled.md", "---\nid: iss-4\n---\n# iss-4\n") + r.commit("release 0.1.0") + r.git("tag", "v0.1.0") + tc.mutate(r) + + d, err := Derive(r.root) + if err != nil { + t.Fatalf("Derive: %v", err) + } + if d.Refused { + t.Fatalf("refused a cut whose only unlabelled record is on the removed side: %s", d.RefusalReason) + } + if got := ids(d.Records.Added); !reflect.DeepEqual(got, tc.wantAdded) { + t.Errorf("added = %v, want %v", got, tc.wantAdded) + } + if got := ids(d.Records.Removed); !reflect.DeepEqual(got, []string{"iss-4"}) { + t.Fatalf("removed = %v, want [iss-4] carried through the cut", got) + } + if d.Records.Removed[0].ImpactErr == "" { + t.Error("the removed record must keep the reason it is unlabelled") + } + if d.Bumped != tc.wantBumped { + t.Errorf("bumped = %v, want %v", d.Bumped, tc.wantBumped) + } + }) + } +} + +// TestDeriveRefusesWithoutAnchor pins the fail-closed no-tag case: with no +// release tag there is no immutable base, and inventing one (0.0.0, say) would +// report every record ever written as the next release's contents. +func TestDeriveRefusesWithoutAnchor(t *testing.T) { + r := newFixtureRepo(t) + r.record(shippedDir+"itd-1-first.md", "itd-1", "additive") + r.commit("no release has ever been tagged") + + d, err := Derive(r.root) + if err != nil { + t.Fatalf("Derive: %v", err) + } + if !d.Refused || d.Bumped { + t.Errorf("expected a refusal with no derived version, got %+v", d) + } +} + +// TestDeriveOnThisRepo exercises the derivation against abcd's own history — the +// only fixture that proves the anchor works over a real tag (v0.1.0/v0.2.0/ +// v0.3.0) and hundreds of real records, not just a synthetic two-commit repo. +// It asserts the shape of the outcome, never a specific version, so the test +// does not have to be edited every release. +// +// It is an OPPORTUNISTIC test: it needs a checkout that actually carries the +// tags. CI's `check` job checks out at the default depth with `fetch-tags: +// false`, so the tags are simply absent there and Derive correctly refuses with +// RefusalNoReleaseTag. That is an environment property, not a defect, so the +// test SKIPS rather than fails — asserting "this repo has tags" would be +// asserting something about the checkout, not about the code. The no-tag +// fail-closed refusal itself is pinned deterministically by +// TestDeriveRefusalKind/no_anchor_tag over a fixture repo, so skipping here +// loses no coverage of the behaviour. +func TestDeriveOnThisRepo(t *testing.T) { + d, err := Derive("../../..") + if err != nil { + t.Skipf("not runnable outside a git checkout: %v", err) + } + if d.BaseTag == "" { + t.Skipf("this checkout carries no release tags (a shallow clone fetched without them); "+ + "Derive correctly refuses: %s", d.RefusalReason) + } + if d.Refused { + t.Logf("derivation refuses: %s", d.RefusalReason) + return + } + for _, rec := range d.Records.All() { + if rec.ID == "" || rec.Path == "" { + t.Errorf("malformed record in the cut: %+v", rec) + } + } + t.Logf("base=%s bump=%s next=%s added=%d removed=%d", + d.BaseTag, d.Bump, d.NextTag, len(d.Records.Added), len(d.Records.Removed)) +} diff --git a/internal/core/changelog/fixture_test.go b/internal/core/changelog/fixture_test.go new file mode 100644 index 0000000..6ca66a0 --- /dev/null +++ b/internal/core/changelog/fixture_test.go @@ -0,0 +1,87 @@ +package changelog + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/REPPL/abcd-cli/internal/gittest" +) + +// fixtureRepo is a hermetic throwaway git repository for the derivation tests. +// The set-difference and the tag anchor are only trustworthy if they are +// exercised against real git objects — a stubbed git would prove nothing about +// the squash-merge caveat this phase must pin — so the tests build actual +// histories here rather than faking the plumbing. +type fixtureRepo struct { + t *testing.T + root string + env []string +} + +// newFixtureRepo initialises an empty repo on a fixed branch name, with an +// identity pinned per command, under the shared hermetic git environment +// (gittest.Env) so an ambient GIT_DIR cannot redirect it at the real tree. +func newFixtureRepo(t *testing.T) *fixtureRepo { + t.Helper() + r := &fixtureRepo{t: t, root: t.TempDir(), env: gittest.Env(t)} + init := exec.Command("git", "-C", r.root, "init", "--initial-branch=main") + init.Env = r.env + if out, err := init.CombinedOutput(); err != nil { + t.Skipf("git init unavailable: %v (%s)", err, out) + } + return r +} + +// git runs one git command in the fixture, failing the test on a non-zero exit. +func (r *fixtureRepo) git(args ...string) string { + r.t.Helper() + full := append([]string{ + "-C", r.root, + "-c", "user.email=fixture@example.invalid", + "-c", "user.name=Fixture", + "-c", "commit.gpgsign=false", + }, args...) + cmd := exec.Command("git", full...) + cmd.Env = r.env + out, err := cmd.CombinedOutput() + if err != nil { + r.t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +// write creates (or overwrites) a repo-relative file, creating parents. +func (r *fixtureRepo) write(rel, content string) { + r.t.Helper() + path := filepath.Join(r.root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + r.t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + r.t.Fatal(err) + } +} + +// remove deletes a repo-relative file from the working tree. +func (r *fixtureRepo) remove(rel string) { + r.t.Helper() + if err := os.Remove(filepath.Join(r.root, filepath.FromSlash(rel))); err != nil { + r.t.Fatal(err) + } +} + +// commit stages everything and records a commit. +func (r *fixtureRepo) commit(msg string) { + r.t.Helper() + r.git("add", "-A") + r.git("commit", "--allow-empty", "-m", msg) +} + +// record writes a minimal record file with the given impact frontmatter. +func (r *fixtureRepo) record(rel, id, impact string) { + r.t.Helper() + r.write(rel, "---\nid: "+id+"\nimpact: "+impact+"\n---\n# "+id+"\n") +} diff --git a/internal/core/changelog/impact.go b/internal/core/changelog/impact.go new file mode 100644 index 0000000..e59d033 --- /dev/null +++ b/internal/core/changelog/impact.go @@ -0,0 +1,128 @@ +// Package changelog is abcd's transport-agnostic release-derivation domain: the +// version and the changelog are facts derived from the records that shipped, not +// numbers and lines a human types (itd-73, spc-10). +// +// This file holds the domain's smallest and most-shared piece — impact, the +// one-word product judgement a record carries. It lives here, not in +// internal/core/lint, because the lints that GATE the judgement and the +// derivation that CONSUMES it must agree on exactly one enum; two copies of it +// would drift the moment one side gained a member. Nothing here reads git, reads +// records, or does bump arithmetic: it is a pure value type plus its boundary +// validator, so both consumers can import it without inheriting a dependency. +package changelog + +import "fmt" + +// Impact is the product judgement a record (an intent or an issue) declares +// about what shipping it does to the public surface. It drives the SemVer bump +// and changelog inclusion, and nothing else — it deliberately does not decide a +// Keep-a-Changelog section, because a four-value enum cannot express +// Security/Deprecated/Removed granularity and conflating the two would make the +// version hostage to editorial judgement. +type Impact string + +// The four members of the enum. The string values are the wire format: they are +// what a record's `impact:` frontmatter field carries, so renaming a constant +// without changing its value is safe and changing a value is a record migration. +const ( + // ImpactAdditive is a new capability that breaks nothing. + ImpactAdditive Impact = "additive" + // ImpactBreaking removes or narrows something callers depend on. + ImpactBreaking Impact = "breaking" + // ImpactFix corrects behaviour within the existing surface. + ImpactFix Impact = "fix" + // ImpactInternal is invisible to users: excluded from the changelog and + // drives no bump. It exists so plumbing work (lint internals, atomic-write + // hardening) is not forced into a user-facing changelog by a hard default. + ImpactInternal Impact = "internal" +) + +// impactValues lists the enum in the order the error messages name it, so the +// legal set is written once and every message stays in step with the constants. +var impactValues = []Impact{ImpactAdditive, ImpactBreaking, ImpactFix, ImpactInternal} + +// ParseImpact is the boundary validator for the `impact:` frontmatter field. +// +// Matching is exact: no case folding and no trimming. The shared frontmatter +// scanner (internal/core/frontmatter) already trims the value it hands over, so +// any whitespace still present here came from inside the value and is an +// authoring defect worth naming rather than silently absorbing. Case is not +// folded because the records are the source of truth for a machine-read enum; +// accepting "Additive" would make the corpus inconsistent with no gate to pull +// it back. +// +// An empty value is an error, not a default. The rule it replaces — "issues +// default to fix" — under-bumped genuinely feature-adding issues, and the +// surface guardrail only backstops breaks, never additive under-bumps. There is +// therefore no defaulting anywhere in this package: the judgement is the +// author's, made explicitly, or the gate refuses. +func ParseImpact(v string) (Impact, error) { + if v == "" { + return "", fmt.Errorf("impact is required and has no default: set it explicitly to one of %s", impactList()) + } + for _, known := range impactValues { + if Impact(v) == known { + return known, nil + } + } + return "", fmt.Errorf("invalid impact %q: want exactly one of %s (lower-case, no surrounding whitespace)", v, impactList()) +} + +// impactList renders the legal set for an error message. +func impactList() string { + out := "" + for i, v := range impactValues { + if i > 0 { + out += "|" + } + out += string(v) + } + return out +} + +// rank is the ONE ordering of the enum: breaking > additive > fix > internal. +// internal is the bottom because it drives no bump at all, which makes it the +// correct identity for a set with nothing user-facing in it. An unrecognised +// value ranks below internal so a zero-value or hand-built Impact that never +// passed ParseImpact can neither win a maximum nor satisfy a predicate. +func (i Impact) rank() int { + switch i { + case ImpactBreaking: + return 3 + case ImpactAdditive: + return 2 + case ImpactFix: + return 1 + case ImpactInternal: + return 0 + default: + return -1 + } +} + +// MaxImpact returns the strongest impact in a set — the single comparison in +// this package, so callers never re-derive the ordering. A set with nothing +// user-facing in it (empty, all-internal, or all-unrecognised) yields +// ImpactInternal, which reads correctly at the call site as "nothing to +// release": the result drives no bump and belongs in no changelog. The input is +// not modified. +func MaxImpact(impacts []Impact) Impact { + best := ImpactInternal + for _, i := range impacts { + if i.rank() > best.rank() { + best = i + } + } + return best +} + +// DrivesBump reports whether a record with this impact moves the version. Only +// internal (and anything that never passed ParseImpact) does not. +func (i Impact) DrivesBump() bool { return i.rank() > ImpactInternal.rank() } + +// InChangelog reports whether a record with this impact earns a changelog line. +// It coincides with DrivesBump today because outcome 8 states one rule for both +// — internal is excluded from the changelog AND drives no bump — but the two are +// named apart because they gate different pipelines (version arithmetic versus +// the prose bijection) and a future class could plausibly split them. +func (i Impact) InChangelog() bool { return i.rank() > ImpactInternal.rank() } diff --git a/internal/core/changelog/impact_test.go b/internal/core/changelog/impact_test.go new file mode 100644 index 0000000..966dd4c --- /dev/null +++ b/internal/core/changelog/impact_test.go @@ -0,0 +1,172 @@ +package changelog + +import ( + "strings" + "testing" +) + +// TestParseImpactAcceptsEveryConstant pins the wire spelling of each enum member: +// the frontmatter value a record carries must round-trip to its constant, so a +// renamed constant can never silently change what records mean. +func TestParseImpactAcceptsEveryConstant(t *testing.T) { + cases := []struct { + value string + want Impact + }{ + {"additive", ImpactAdditive}, + {"breaking", ImpactBreaking}, + {"fix", ImpactFix}, + {"internal", ImpactInternal}, + } + for _, tc := range cases { + got, err := ParseImpact(tc.value) + if err != nil { + t.Errorf("ParseImpact(%q) returned error %v, want %v", tc.value, err, tc.want) + continue + } + if got != tc.want { + t.Errorf("ParseImpact(%q) = %q, want %q", tc.value, got, tc.want) + } + if string(tc.want) != tc.value { + t.Errorf("constant %q does not carry its wire spelling %q", tc.want, tc.value) + } + } +} + +// TestParseImpactRejectsEverythingElse proves the exact-match contract. Nothing +// is case-folded and nothing is trimmed: frontmatter.Fields already trims the +// value it hands over, so any surviving whitespace is a real authoring defect +// and must be reported rather than absorbed. +func TestParseImpactRejectsEverythingElse(t *testing.T) { + invalid := []string{ + "Additive", // case is not folded + "BREAKING", // case is not folded + "major", // a SemVer bump name is not an impact + "minor", // + "patch", // + " fix ", // no trimming beyond what frontmatter parsing did + "internal ", // trailing space is a defect, not an internal record + "fix\n", // a stray newline is not a fix + "additive,fix", // one judgement per record + "null", // a YAML null is not an impact + "~", // + } + for _, v := range invalid { + got, err := ParseImpact(v) + if err == nil { + t.Errorf("ParseImpact(%q) = %q, want an error", v, got) + continue + } + if got != "" { + t.Errorf("ParseImpact(%q) returned %q alongside its error, want the zero Impact", v, got) + } + } +} + +// TestParseImpactEmptyIsRequiredNotDefaulted is the guard for outcome 8: the old +// "issues default to fix" rule under-bumped feature-adding issues, so an absent +// value must fail loudly and the message must say the field is required. +func TestParseImpactEmptyIsRequiredNotDefaulted(t *testing.T) { + got, err := ParseImpact("") + if err == nil { + t.Fatalf("ParseImpact(\"\") = %q, want an error (there is no default)", got) + } + if got != "" { + t.Errorf("ParseImpact(\"\") returned %q alongside its error, want the zero Impact", got) + } + msg := err.Error() + if !strings.Contains(msg, "required") { + t.Errorf("ParseImpact(\"\") error = %q, want it to say the field is required", msg) + } + if !strings.Contains(msg, "no default") { + t.Errorf("ParseImpact(\"\") error = %q, want it to say there is no default", msg) + } +} + +// TestParseImpactErrorNamesTheEnum keeps the boundary message actionable: a +// maintainer who mistypes the field learns the whole legal set from the error. +func TestParseImpactErrorNamesTheEnum(t *testing.T) { + for _, v := range []string{"", "Additive"} { + _, err := ParseImpact(v) + if err == nil { + t.Fatalf("ParseImpact(%q) unexpectedly succeeded", v) + } + msg := err.Error() + for _, want := range []string{"additive", "breaking", "fix", "internal"} { + if !strings.Contains(msg, want) { + t.Errorf("ParseImpact(%q) error = %q, want it to name %q", v, msg, want) + } + } + } +} + +// TestMaxImpactOrdering pins the single ordering breaking > additive > fix > +// internal, including ties and the empty cut. +func TestMaxImpactOrdering(t *testing.T) { + cases := []struct { + name string + in []Impact + want Impact + }{ + {"empty cut is internal (nothing to release)", nil, ImpactInternal}, + {"empty slice is internal", []Impact{}, ImpactInternal}, + {"single fix", []Impact{ImpactFix}, ImpactFix}, + {"single internal", []Impact{ImpactInternal}, ImpactInternal}, + {"tie on fix", []Impact{ImpactFix, ImpactFix}, ImpactFix}, + {"tie on breaking", []Impact{ImpactBreaking, ImpactBreaking}, ImpactBreaking}, + {"additive beats fix", []Impact{ImpactFix, ImpactAdditive}, ImpactAdditive}, + {"additive beats fix, order-independent", []Impact{ImpactAdditive, ImpactFix}, ImpactAdditive}, + {"fix beats internal", []Impact{ImpactInternal, ImpactFix}, ImpactFix}, + {"additive beats internal", []Impact{ImpactInternal, ImpactAdditive}, ImpactAdditive}, + {"breaking beats additive", []Impact{ImpactAdditive, ImpactBreaking}, ImpactBreaking}, + {"breaking beats everything", []Impact{ImpactInternal, ImpactFix, ImpactBreaking, ImpactAdditive}, ImpactBreaking}, + {"all internal stays internal", []Impact{ImpactInternal, ImpactInternal}, ImpactInternal}, + {"an unrecognised value never wins", []Impact{Impact("wat"), ImpactFix}, ImpactFix}, + {"an unrecognised value alone falls to internal", []Impact{Impact("wat")}, ImpactInternal}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := MaxImpact(tc.in); got != tc.want { + t.Errorf("MaxImpact(%v) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +// TestMaxImpactDoesNotMutateInput proves the helper is pure: the next stage +// passes the cut's impacts around and must not find them reordered. +func TestMaxImpactDoesNotMutateInput(t *testing.T) { + in := []Impact{ImpactFix, ImpactBreaking, ImpactInternal} + _ = MaxImpact(in) + want := []Impact{ImpactFix, ImpactBreaking, ImpactInternal} + for i := range want { + if in[i] != want[i] { + t.Fatalf("MaxImpact reordered its input: got %v, want %v", in, want) + } + } +} + +// TestPredicates pins outcome 8's single rule from both sides: internal drives +// no bump AND is excluded from the changelog; every other member does both. +func TestPredicates(t *testing.T) { + cases := []struct { + in Impact + drivesBump bool + inChangelog bool + }{ + {ImpactBreaking, true, true}, + {ImpactAdditive, true, true}, + {ImpactFix, true, true}, + {ImpactInternal, false, false}, + {Impact(""), false, false}, + {Impact("wat"), false, false}, + } + for _, tc := range cases { + if got := tc.in.DrivesBump(); got != tc.drivesBump { + t.Errorf("Impact(%q).DrivesBump() = %v, want %v", tc.in, got, tc.drivesBump) + } + if got := tc.in.InChangelog(); got != tc.inChangelog { + t.Errorf("Impact(%q).InChangelog() = %v, want %v", tc.in, got, tc.inChangelog) + } + } +} diff --git a/internal/core/changelog/shipped.go b/internal/core/changelog/shipped.go new file mode 100644 index 0000000..b840883 --- /dev/null +++ b/internal/core/changelog/shipped.go @@ -0,0 +1,246 @@ +package changelog + +import ( + "fmt" + "path" + "regexp" + "sort" + "strings" + + "github.com/REPPL/abcd-cli/internal/core/frontmatter" + "github.com/REPPL/abcd-cli/internal/gitutil" +) + +// The two record families a release is cut from. Directory-as-truth is the +// lifecycle authority in this repo — an intent's folder IS its status — so +// "what shipped" is exactly "which record files live in the terminal folders", +// and no separate release ledger is needed. +const ( + intentsShippedDir = ".abcd/development/intents/shipped" + issuesResolvedDir = ".abcd/work/issues/resolved" +) + +// recordPaths are the pathspecs handed to git ls-tree, in the order the sets are +// reported. +var recordPaths = []string{intentsShippedDir, issuesResolvedDir} + +// recordFileRe matches a record filename and captures its id. The record +// families are the only files that count: a directory README or a stray note +// living beside them is not a release line. internal/core/lint holds equivalent +// unexported filename matchers for its own scanners (intentFileRe/issueFileRe); +// if a third consumer appears, that pair and this one want consolidating into a +// single exported matcher rather than a third copy. +var recordFileRe = regexp.MustCompile(`^((?:itd|iss)-\d+)[^/]*\.md$`) + +// Record is one record in a release cut: where it lives, what it is called, and +// the one product judgement it declares. +type Record struct { + // Path is the repo-relative, slash-separated path of the record file. + Path string + // ID is the record id derived from the FILENAME (itd-73, iss-24), not from + // frontmatter: the filename is what the id-uniqueness lint already governs, + // and issue frontmatter quotes its id, so the filename is the cheaper and + // more consistent source. + ID string + // Impact is the parsed judgement, or "" when the record carries none. + Impact Impact + // ImpactErr is why the impact could not be parsed, empty when it parsed. A + // record is reported as unlabelled rather than defaulted, because defaulting + // a missing judgement silently under-bumps the release. + ImpactErr string +} + +// RecordSet is a release cut: the set-difference of record END STATES between +// the anchor tag and HEAD. +// +// It is deliberately NOT a git-log walk of moves. A squash or rebase merge +// collapses the commit that moved a record into its terminal folder, so a walk +// over `..HEAD -- ` reports a different set depending on how a +// branch happened to land — the same tree, two answers. Comparing end states +// asks the only question that matters ("which records are in the terminal +// folders now, and which were there at the tag?") and is immune to the shape of +// the history in between. +type RecordSet struct { + // BaseRef is the anchor the cut is measured from (a tag such as v0.3.0). + BaseRef string + // Added are records present at HEAD and absent at the anchor — the cut. + Added []Record + // Removed are records present at the anchor and absent at HEAD. A record + // leaves a terminal folder when it is superseded or re-slugged; either way + // it is a user-visible change that must surface as a Removed/Changed line + // rather than silently vanish from the release. + Removed []Record +} + +// All returns the whole cut — added then removed — for callers that must account +// for every record, such as the changelog completeness bijection. +func (s RecordSet) All() []Record { + out := make([]Record, 0, len(s.Added)+len(s.Removed)) + out = append(out, s.Added...) + out = append(out, s.Removed...) + return out +} + +// ChangelogRequired is the cut minus every internal record: exactly the set of +// records the generated prose must cite, no more and no less. internal records +// are excluded here (not filtered later, per-caller) so the changelog and the +// version agree on one definition of "user-facing". +func (s RecordSet) ChangelogRequired() []Record { + out := make([]Record, 0, len(s.Added)+len(s.Removed)) + for _, r := range s.All() { + if r.Impact.InChangelog() { + out = append(out, r) + } + } + return out +} + +// Unlabelled returns every record in the cut whose impact is absent or invalid, +// for a preview that must show the operator the whole picture. It is NOT the +// refusal set — see UnlabelledAdded for that. +func (s RecordSet) Unlabelled() []Record { return unlabelled(s.All()) } + +// UnlabelledAdded returns the unlabelled records on the ADDED side only: the +// ones a caller may refuse the cut over, because an unlabelled record ranks +// below every real impact and deriving over it would quietly under-bump a +// release that may contain a break. +// +// Removed records are deliberately excluded. Their blob is read from the anchor +// tag's immutable tree, so one that predates the impact back-fill can never be +// labelled: at HEAD the file is either gone (supersession) or already carries a +// valid impact under its new name (re-slug). Refusing over it would name a +// remedy the operator cannot perform and would block every release until the +// move was reverted. Such a record still travels in Removed, so the release +// still reports it rather than dropping it silently. +func (s RecordSet) UnlabelledAdded() []Record { return unlabelled(s.Added) } + +// unlabelled is the one filter both accessors share, so the definition of +// "unlabelled" is written once. +func unlabelled(records []Record) []Record { + var out []Record + for _, r := range records { + if r.ImpactErr != "" { + out = append(out, r) + } + } + return out +} + +// Impact is the strongest impact in the cut — the one that decides the bump. +// An empty or all-internal cut yields ImpactInternal, which reads at the call +// site as "nothing to release". +func (s RecordSet) Impact() Impact { + all := s.All() + impacts := make([]Impact, 0, len(all)) + for _, r := range all { + impacts = append(impacts, r.Impact) + } + return MaxImpact(impacts) +} + +// ShippedSince computes the release cut between baseRef (the anchor tag) and +// HEAD, as the set-difference of the record files in the terminal folders. +// +// Both sides are read out of git, never out of the working tree: a removed +// record exists only in the anchor's tree, and reading the added side from git +// too keeps a dirty or half-staged working tree from changing what a release +// reports. Every returned slice is sorted by path — the set feeds a rendered +// preview and a changelog bijection, both of which must be reproducible. +// +// A baseRef git cannot resolve is an error, never an empty set: reporting +// "nothing shipped" against a tag that does not exist would let a cut silently +// claim there is nothing to release. +func ShippedSince(root string, baseRef string) (RecordSet, error) { + set := RecordSet{BaseRef: baseRef} + + basePaths, err := recordPathsAt(root, baseRef) + if err != nil { + return RecordSet{}, err + } + headPaths, err := recordPathsAt(root, "HEAD") + if err != nil { + return RecordSet{}, err + } + + for p := range headPaths { + if _, atBase := basePaths[p]; !atBase { + set.Added = append(set.Added, newRecord(root, "HEAD", p)) + } + } + for p := range basePaths { + if _, atHead := headPaths[p]; !atHead { + set.Removed = append(set.Removed, newRecord(root, baseRef, p)) + } + } + sortRecords(set.Added) + sortRecords(set.Removed) + return set, nil +} + +// recordPathsAt lists the record files present in the terminal folders at ref, +// keyed by repo-relative path. Non-record files (READMEs, notes) are dropped +// here, so every later stage sees records only. +func recordPathsAt(root string, ref string) (map[string]struct{}, error) { + args := append([]string{"ls-tree", "-r", "-z", "--name-only", ref, "--"}, recordPaths...) + // -z makes git emit raw NUL-separated paths, so a path containing a quote, + // a backslash, or a newline cannot be mangled by git's default path quoting + // or desync the split. + out, err := gitutil.Run(root, args...) + if err != nil { + return nil, fmt.Errorf("listing records at %s: %w", ref, err) + } + paths := map[string]struct{}{} + for _, p := range strings.Split(out, "\x00") { + if p == "" { + continue + } + if !recordFileRe.MatchString(path.Base(p)) { + continue + } + paths[p] = struct{}{} + } + return paths, nil +} + +// maxRecordBytes caps the guarded blob read, in the same order as +// maxChangelogBytes and for the same reason: a record is a page of prose, so a +// file that is not one must not stream unbounded input into a read-only preview. +// Only the frontmatter is read, and that is at the top of the blob, so a +// truncated tail cannot change the parsed impact. +const maxRecordBytes = 4 << 20 + +// newRecord reads one record's blob at ref and parses its impact. A blob that +// cannot be read, or an impact that cannot be parsed, yields an unlabelled +// record carrying the reason rather than an error: one malformed record must not +// hide the rest of the cut from the operator who has to fix it. +func newRecord(root string, ref string, relPath string) Record { + rec := Record{Path: relPath, ID: recordID(relPath)} + blob, err := gitutil.RunLimited(root, maxRecordBytes, "cat-file", "blob", ref+":"+relPath) + if err != nil { + rec.ImpactErr = fmt.Sprintf("reading %s at %s: %v", relPath, ref, err) + return rec + } + field := frontmatter.Fields(strings.Split(blob, "\n"))["impact"] + impact, err := ParseImpact(field.Value) + if err != nil { + rec.ImpactErr = err.Error() + return rec + } + rec.Impact = impact + return rec +} + +// recordID extracts the id from a record path; the caller has already matched +// the filename, so the empty fallback is unreachable in practice. +func recordID(relPath string) string { + m := recordFileRe.FindStringSubmatch(path.Base(relPath)) + if m == nil { + return "" + } + return m[1] +} + +// sortRecords orders by path, the only field guaranteed unique in a set. +func sortRecords(records []Record) { + sort.Slice(records, func(i, j int) bool { return records[i].Path < records[j].Path }) +} diff --git a/internal/core/changelog/shipped_test.go b/internal/core/changelog/shipped_test.go new file mode 100644 index 0000000..b5ce379 --- /dev/null +++ b/internal/core/changelog/shipped_test.go @@ -0,0 +1,291 @@ +package changelog + +import ( + "reflect" + "testing" +) + +const ( + shippedDir = ".abcd/development/intents/shipped/" + resolvedDir = ".abcd/work/issues/resolved/" + plannedDir = ".abcd/development/intents/planned/" +) + +// ids renders a record slice as its ids, for readable assertions. +func ids(records []Record) []string { + out := make([]string, 0, len(records)) + for _, r := range records { + out = append(out, r.ID) + } + return out +} + +// baseRepo builds a repo with one shipped intent and one resolved issue at +// v0.1.0 — the state every caveat test diverges from. +func baseRepo(t *testing.T) *fixtureRepo { + t.Helper() + r := newFixtureRepo(t) + r.write(shippedDir+"README.md", "# shipped\n") + r.record(shippedDir+"itd-1-first.md", "itd-1", "additive") + r.record(resolvedDir+"iss-1-first.md", "iss-1", "fix") + r.commit("the released state") + r.git("tag", "v0.1.0") + return r +} + +// TestShippedSinceAgreesAcrossMergeAndSquash is the PHASE 1 STOP-CONDITION test. +// The set is computed as a difference of END STATES precisely so that HOW a +// branch landed cannot change it: a squash merge collapses the record's move +// into a single new commit with no move to walk, and a log walk would report a +// different set for the same tree. If these two histories ever disagree, the +// anchor model is unsound and the phase stops. +func TestShippedSinceAgreesAcrossMergeAndSquash(t *testing.T) { + // Each lands the SAME two records on main by a different route. An + // intervening commit on main makes the squash and rebase routes genuinely + // rewrite history rather than fast-forward. + land := map[string]func(r *fixtureRepo){ + "merge-commit": func(r *fixtureRepo) { + r.git("merge", "--no-ff", "-m", "merge feature", "feature") + }, + "squash-merge": func(r *fixtureRepo) { + r.git("merge", "--squash", "feature") + r.git("commit", "-m", "squashed feature") + }, + "rebase-merge": func(r *fixtureRepo) { + r.git("rebase", "main", "feature") + r.git("checkout", "main") + r.git("merge", "--ff-only", "feature") + }, + } + + got := map[string][]string{} + for name, landFn := range land { + t.Run(name, func(t *testing.T) { + r := baseRepo(t) + r.git("checkout", "-b", "feature") + r.record(shippedDir+"itd-2-second.md", "itd-2", "breaking") + r.record(resolvedDir+"iss-2-second.md", "iss-2", "fix") + r.commit("ship itd-2 and resolve iss-2") + r.git("checkout", "main") + r.write("README.md", "# moved on\n") + r.commit("unrelated work on main") + landFn(r) + + set, err := ShippedSince(r.root, "v0.1.0") + if err != nil { + t.Fatalf("ShippedSince: %v", err) + } + want := []string{"itd-2", "iss-2"} + if added := ids(set.Added); !reflect.DeepEqual(added, want) { + t.Errorf("STOP: %s history added = %v, want %v", name, added, want) + } + if len(set.Removed) != 0 { + t.Errorf("%s history removed = %v, want none", name, ids(set.Removed)) + } + got[name] = ids(set.Added) + }) + } + + for name, added := range got { + if !reflect.DeepEqual(added, got["merge-commit"]) { + t.Fatalf("STOP: the shipped set is history-dependent — %s=%v merge-commit=%v", + name, added, got["merge-commit"]) + } + } +} + +// TestShippedSinceRenameReadsAsDeleteAndAdd pins a KNOWN LIMIT rather than a +// desired behaviour: the set-difference compares paths, so re-slugging a file +// inside shipped/ surfaces as a removal plus an addition. It is pinned so a +// future change to it (a rename-detecting diff, say) is a loud test failure and +// a deliberate decision, not a silent shift in what a release reports. +func TestShippedSinceRenameReadsAsDeleteAndAdd(t *testing.T) { + r := baseRepo(t) + r.git("mv", shippedDir+"itd-1-first.md", shippedDir+"itd-1-renamed-slug.md") + r.commit("re-slug itd-1") + + set, err := ShippedSince(r.root, "v0.1.0") + if err != nil { + t.Fatalf("ShippedSince: %v", err) + } + if got := ids(set.Added); !reflect.DeepEqual(got, []string{"itd-1"}) { + t.Errorf("added = %v, want the renamed path to read as an addition of itd-1", got) + } + if got := ids(set.Removed); !reflect.DeepEqual(got, []string{"itd-1"}) { + t.Errorf("removed = %v, want the old path to read as a removal of itd-1", got) + } +} + +// TestShippedSinceSurfacesSupersession pins that an intent moved OUT of shipped/ +// is reported, not silently dropped: a supersession is a user-visible change +// (a Removed/Changed line), and a set that only ever grew would hide it. +func TestShippedSinceSurfacesSupersession(t *testing.T) { + r := baseRepo(t) + r.write(plannedDir+"itd-1-first.md", "---\nid: itd-1\nimpact: breaking\n---\n# itd-1\n") + r.remove(shippedDir + "itd-1-first.md") + r.commit("supersede itd-1 back to planned") + + set, err := ShippedSince(r.root, "v0.1.0") + if err != nil { + t.Fatalf("ShippedSince: %v", err) + } + if len(set.Added) != 0 { + t.Errorf("added = %v, want none (planned/ is not in the release set)", ids(set.Added)) + } + if got := ids(set.Removed); !reflect.DeepEqual(got, []string{"itd-1"}) { + t.Fatalf("removed = %v, want [itd-1]", got) + } + // The removed record's impact is read from the TAG's tree — the only place + // the record still exists — so a supersession can still drive the bump. + if set.Removed[0].Impact != ImpactAdditive { + t.Errorf("removed impact = %q, want the impact recorded at the tag (additive)", set.Removed[0].Impact) + } + if got := set.Impact(); got != ImpactAdditive { + t.Errorf("set impact = %q, want additive", got) + } +} + +// TestShippedSinceEmptyWhenNothingMoved pins the "nothing to release" input: a +// commit that touches no record leaves both sets empty and the impact at +// internal, which drives no bump. +func TestShippedSinceEmptyWhenNothingMoved(t *testing.T) { + r := baseRepo(t) + r.write("README.md", "# unrelated\n") + r.commit("docs only") + + set, err := ShippedSince(r.root, "v0.1.0") + if err != nil { + t.Fatalf("ShippedSince: %v", err) + } + if len(set.Added) != 0 || len(set.Removed) != 0 { + t.Errorf("expected an empty set, got added=%v removed=%v", ids(set.Added), ids(set.Removed)) + } + if got := set.Impact(); got != ImpactInternal { + t.Errorf("impact = %q, want internal (nothing to release)", got) + } + if _, bumped := DeriveNext(mustSemver(t, "0.1.0"), set.Impact()); bumped { + t.Error("an empty set must not bump") + } +} + +// TestShippedSinceExcludesInternalFromTheChangelog pins outcome 8: an internal +// record is part of the cut (so the bijection can account for it) but earns no +// changelog line and drives no bump. +func TestShippedSinceExcludesInternalFromTheChangelog(t *testing.T) { + r := baseRepo(t) + r.record(resolvedDir+"iss-2-plumbing.md", "iss-2", "internal") + r.record(resolvedDir+"iss-3-visible.md", "iss-3", "fix") + r.commit("resolve two issues") + + set, err := ShippedSince(r.root, "v0.1.0") + if err != nil { + t.Fatalf("ShippedSince: %v", err) + } + if got := ids(set.All()); !reflect.DeepEqual(got, []string{"iss-2", "iss-3"}) { + t.Errorf("all = %v, want [iss-2 iss-3]", got) + } + if got := ids(set.ChangelogRequired()); !reflect.DeepEqual(got, []string{"iss-3"}) { + t.Errorf("changelog-required = %v, want [iss-3] (internal excluded)", got) + } + if got := set.Impact(); got != ImpactFix { + t.Errorf("impact = %q, want fix (internal drives no bump)", got) + } +} + +// TestShippedSinceIgnoresNonRecords pins that only itd-N/iss-N markdown counts: +// a directory README living beside the records is not a release line. +func TestShippedSinceIgnoresNonRecords(t *testing.T) { + r := baseRepo(t) + r.write(shippedDir+"NOTES.md", "# notes\n") + r.write(shippedDir+"itd-9-draft.txt", "not markdown\n") + r.record(shippedDir+"itd-9-ninth.md", "itd-9", "fix") + r.commit("add noise and one record") + + set, err := ShippedSince(r.root, "v0.1.0") + if err != nil { + t.Fatalf("ShippedSince: %v", err) + } + if got := ids(set.Added); !reflect.DeepEqual(got, []string{"itd-9"}) { + t.Errorf("added = %v, want [itd-9]", got) + } +} + +// TestShippedSinceNamesUnlabelledRecords pins that a record with no (or an +// invalid) impact is surfaced by name rather than silently ranked at the bottom: +// treating a missing judgement as "internal" would under-bump the release, which +// is the exact failure the no-silent-default rule exists to prevent. +func TestShippedSinceNamesUnlabelledRecords(t *testing.T) { + r := baseRepo(t) + r.write(shippedDir+"itd-4-unlabelled.md", "---\nid: itd-4\n---\n# itd-4\n") + r.record(shippedDir+"itd-5-misspelled.md", "itd-5", "Additive") + r.commit("ship two badly-labelled intents") + + set, err := ShippedSince(r.root, "v0.1.0") + if err != nil { + t.Fatalf("ShippedSince: %v", err) + } + if got := ids(set.Unlabelled()); !reflect.DeepEqual(got, []string{"itd-4", "itd-5"}) { + t.Errorf("unlabelled = %v, want [itd-4 itd-5]", got) + } + for _, rec := range set.Unlabelled() { + if rec.ImpactErr == "" { + t.Errorf("%s: unlabelled record carries no reason", rec.ID) + } + } +} + +// TestShippedSinceSplitsUnlabelledBySide pins the two accessors apart: the whole +// cut is what a preview shows, but only the added side is refusable. A record +// unlabelled at the tag cannot be labelled anywhere — its blob lives in an +// immutable tree — so counting it as refusable would block the release with a +// remedy nobody can perform. +func TestShippedSinceSplitsUnlabelledBySide(t *testing.T) { + r := newFixtureRepo(t) + r.write(shippedDir+"itd-1-first.md", "---\nid: itd-1\n---\n# itd-1\n") + r.commit("a release that predates the impact field") + r.git("tag", "v0.1.0") + r.remove(shippedDir + "itd-1-first.md") + r.write(shippedDir+"itd-2-unlabelled.md", "---\nid: itd-2\n---\n# itd-2\n") + r.commit("supersede itd-1, ship an unlabelled itd-2") + + set, err := ShippedSince(r.root, "v0.1.0") + if err != nil { + t.Fatalf("ShippedSince: %v", err) + } + if got := ids(set.Unlabelled()); !reflect.DeepEqual(got, []string{"itd-2", "itd-1"}) { + t.Errorf("unlabelled = %v, want both sides [itd-2 itd-1]", got) + } + if got := ids(set.UnlabelledAdded()); !reflect.DeepEqual(got, []string{"itd-2"}) { + t.Errorf("unlabelled-added = %v, want [itd-2] (the removed side is unfixable)", got) + } +} + +// TestShippedSinceIsSorted pins deterministic ordering: the set feeds a rendered +// preview and a changelog bijection, both of which must be reproducible. +func TestShippedSinceIsSorted(t *testing.T) { + r := baseRepo(t) + for _, id := range []string{"itd-30", "itd-4", "itd-100"} { + r.record(shippedDir+id+"-x.md", id, "fix") + } + r.record(resolvedDir+"iss-7-x.md", "iss-7", "fix") + r.commit("ship several") + + set, err := ShippedSince(r.root, "v0.1.0") + if err != nil { + t.Fatalf("ShippedSince: %v", err) + } + want := []string{"itd-100", "itd-30", "itd-4", "iss-7"} + if got := ids(set.Added); !reflect.DeepEqual(got, want) { + t.Errorf("added = %v, want %v (sorted by path)", got, want) + } +} + +// TestShippedSinceUnknownRef pins that a bad anchor is an error, never an empty +// set: silently reporting "nothing shipped" against a tag that does not exist +// would let a release cut claim there is nothing to release. +func TestShippedSinceUnknownRef(t *testing.T) { + r := baseRepo(t) + if _, err := ShippedSince(r.root, "v9.9.9"); err == nil { + t.Error("expected an error for an unknown ref") + } +} diff --git a/internal/core/changelog/version.go b/internal/core/changelog/version.go new file mode 100644 index 0000000..99d914d --- /dev/null +++ b/internal/core/changelog/version.go @@ -0,0 +1,60 @@ +package changelog + +import "github.com/REPPL/abcd-cli/internal/core/launch" + +// DeriveNext is the whole version policy in one pure function: given the +// previous release and the strongest impact in the cut, it returns the next +// version. It is deliberately free of git, files, and clocks so the policy can +// be walked cell by cell in a table test rather than inferred from a pipeline. +// +// The arithmetic is over launch.Semver — the repo's one SemVer type — and the +// result is always a release CORE: any prerelease/build metadata on prev is +// dropped, because the derived number becomes a git tag and a manifest value, +// and carrying "rc1" forward would publish a release that claims to be a +// pre-release of itself. +// +// The policy, per plan §3: +// +// prev >= 1.0.0 breaking -> major++, minor = patch = 0 +// additive -> minor++, patch = 0 +// fix -> patch++ +// prev is 0.x breaking -> minor++, patch = 0 +// additive -> patch++ +// fix -> patch++ +// +// The pre-1.0 row is load-bearing, not a shortcut. While abcd is at 0.x it has +// declared no stable surface, so a break bumps the minor (ADR-37: "pre-1.0, a +// minor may break, called out under Breaking"). The consequence is deliberate: +// NO input can derive 1.0.0 from a 0.x base. The first 1.0.0 is a human's +// explicit override, because declaring stability is a product decision no set of +// records can make on the maintainer's behalf. +// +// The second return is the "nothing to release" signal, and it is a distinct +// value rather than a silent `return prev`: an all-internal or empty cut moves +// no version, and a caller that could not tell the two apart would write a +// duplicate dated heading — which auto-release.yml would then try to tag against +// an already-tagged version. +func DeriveNext(prev launch.Semver, bump Impact) (launch.Semver, bool) { + if !bump.DrivesBump() { + return launch.Semver{}, false + } + // Copy the core only: Prerelease/Build are deliberately left zero. + next := launch.Semver{Major: prev.Major, Minor: prev.Minor, Patch: prev.Patch} + preStable := prev.Major == 0 + switch { + case bump == ImpactBreaking && preStable: + next.Minor++ + next.Patch = 0 + case bump == ImpactBreaking: + next.Major++ + next.Minor, next.Patch = 0, 0 + case bump == ImpactAdditive && preStable: + next.Patch++ + case bump == ImpactAdditive: + next.Minor++ + next.Patch = 0 + default: // ImpactFix, the only remaining bump-driving member. + next.Patch++ + } + return next, true +} diff --git a/internal/core/changelog/version_test.go b/internal/core/changelog/version_test.go new file mode 100644 index 0000000..ee99d4c --- /dev/null +++ b/internal/core/changelog/version_test.go @@ -0,0 +1,104 @@ +package changelog + +import ( + "testing" + + "github.com/REPPL/abcd-cli/internal/core/launch" +) + +func mustSemver(t *testing.T, v string) launch.Semver { + t.Helper() + s, err := launch.ParseSemver(v) + if err != nil { + t.Fatalf("ParseSemver(%q): %v", v, err) + } + return s +} + +// TestDeriveNextTable walks every cell of the policy table (plan §3): the +// standard SemVer rows at >= 1.0 and the pre-1.0 rows, where `breaking` +// deliberately bumps the MINOR so a 0.x line can break without claiming the +// stability a 1.0.0 asserts. +func TestDeriveNextTable(t *testing.T) { + cases := []struct { + name string + prev string + bump Impact + want string + bumped bool + }{ + {"stable breaking bumps major", "1.2.3", ImpactBreaking, "2.0.0", true}, + {"stable additive bumps minor", "1.2.3", ImpactAdditive, "1.3.0", true}, + {"stable fix bumps patch", "1.2.3", ImpactFix, "1.2.4", true}, + {"stable internal does not bump", "1.2.3", ImpactInternal, "", false}, + + {"pre-1.0 breaking bumps minor", "0.3.0", ImpactBreaking, "0.4.0", true}, + {"pre-1.0 additive bumps patch", "0.3.0", ImpactAdditive, "0.3.1", true}, + {"pre-1.0 fix bumps patch", "0.3.0", ImpactFix, "0.3.1", true}, + {"pre-1.0 internal does not bump", "0.3.0", ImpactInternal, "", false}, + + {"pre-1.0 breaking never reaches 1.0.0", "0.9.0", ImpactBreaking, "0.10.0", true}, + {"pre-1.0 breaking clears the patch", "0.3.7", ImpactBreaking, "0.4.0", true}, + {"1.0.0 fix bumps patch", "1.0.0", ImpactFix, "1.0.1", true}, + {"0.0.x additive bumps patch", "0.0.1", ImpactAdditive, "0.0.2", true}, + {"an unparsed impact does not bump", "0.3.0", Impact("nonsense"), "", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + next, bumped := DeriveNext(mustSemver(t, tc.prev), tc.bump) + if bumped != tc.bumped { + t.Fatalf("DeriveNext(%s, %s) bumped = %v, want %v", tc.prev, tc.bump, bumped, tc.bumped) + } + if !bumped { + return + } + if got := next.String(); got != tc.want { + t.Errorf("DeriveNext(%s, %s) = %s, want %s", tc.prev, tc.bump, got, tc.want) + } + }) + } +} + +// TestDeriveNextNeverDerivesFirstStable pins the policy's hardest line: the +// first 1.0.0 is a deliberate human override, so NO input from a 0.x base may +// produce it. A regression here would silently promise API stability abcd has +// not declared. +func TestDeriveNextNeverDerivesFirstStable(t *testing.T) { + prevs := []string{"0.0.0", "0.0.9", "0.1.0", "0.3.0", "0.9.9", "0.99.99"} + for _, prev := range prevs { + for _, bump := range []Impact{ImpactAdditive, ImpactBreaking, ImpactFix, ImpactInternal} { + next, bumped := DeriveNext(mustSemver(t, prev), bump) + if bumped && next.Major != 0 { + t.Errorf("DeriveNext(%s, %s) = %s: derivation left the 0.x line", prev, bump, next) + } + } + } +} + +// TestDeriveNextDropsPrereleaseMetadata pins that a derived version is a +// release core: a base carrying prerelease/build metadata must not smuggle it +// into the next version, which becomes a tag and a manifest value. +func TestDeriveNextDropsPrereleaseMetadata(t *testing.T) { + prev := mustSemver(t, "1.2.3-rc1+build5") + next, bumped := DeriveNext(prev, ImpactFix) + if !bumped { + t.Fatal("a fix must bump") + } + if next.Prerelease != "" || next.Build != "" { + t.Errorf("derived version carries metadata: %+v", next) + } + if next.String() != "1.2.4" { + t.Errorf("DeriveNext = %s, want 1.2.4", next.String()) + } +} + +// TestDeriveNextEmptySetDoesNotBump pins the "nothing to release" signal: the +// max impact of an empty record set is ImpactInternal, which must report a +// distinct no-bump rather than silently returning the previous version (which +// the caller would write as a duplicate heading). +func TestDeriveNextEmptySetDoesNotBump(t *testing.T) { + prev := mustSemver(t, "0.3.0") + if _, bumped := DeriveNext(prev, MaxImpact(nil)); bumped { + t.Error("an empty record set must not bump") + } +} diff --git a/internal/core/launch/retention.go b/internal/core/launch/retention.go index a54fb9f..4b9bd48 100644 --- a/internal/core/launch/retention.go +++ b/internal/core/launch/retention.go @@ -32,7 +32,7 @@ func ComputeRetention(published Semver, existing []Semver) RetentionPlan { // (1) Refuse on any strictly-newer existing release. for _, e := range existing { - if coreGreater(e, published) { + if CoreGreater(e, published) { plan.Refused = true plan.RefusalReason = "existing release " + e.Tag() + " is newer than the published " + published.Tag() + @@ -59,7 +59,7 @@ func ComputeRetention(published Semver, existing []Semver) RetentionPlan { // (3) Keep the max-Patch version in the line; the rest are pruned. best := versions[0] for _, v := range versions[1:] { - if coreGreater(v, best) { + if CoreGreater(v, best) { best = v } } diff --git a/internal/core/launch/semver.go b/internal/core/launch/semver.go index 0437793..3d6dbf2 100644 --- a/internal/core/launch/semver.go +++ b/internal/core/launch/semver.go @@ -67,5 +67,9 @@ func coreLess(a, b Semver) bool { return a.Patch < b.Patch } -// coreGreater reports whether a is strictly newer than b by core version. -func coreGreater(a, b Semver) bool { return coreLess(b, a) } +// CoreGreater reports whether a is strictly newer than b by core version. It is +// exported because release ordering is decided in more than one place — the +// retention plan here and the tag anchor the changelog derivation resolves — and +// a second hand-rolled field comparison would be a second, silently divergent +// definition of "newer". +func CoreGreater(a, b Semver) bool { return coreLess(b, a) } diff --git a/internal/core/lint/config.go b/internal/core/lint/config.go index ac02d05..62552fc 100644 --- a/internal/core/lint/config.go +++ b/internal/core/lint/config.go @@ -61,15 +61,18 @@ type RuleConfig struct { Fields []string `json:"fields"` // Exempt is the directory_coverage glob allowlist. Exempt []string `json:"exempt"` - // IntentsDir is the intent_lifecycle intents subdirectory (relative to a root). - // spec_lifecycle also reads it to resolve the intent corpus its specs link to. + // IntentsDir is the intents subdirectory (relative to a root) read by the + // intent-tree rules, intent_lifecycle and intent_impact_valid. Rules that name + // the same directory share one scan of it. spec_lifecycle also reads it to + // resolve the intent corpus its specs link to. IntentsDir string `json:"intents_dir"` // SpecsDir is the spec_lifecycle specs subdirectory (relative to a root), // mirroring IntentsDir. Default "specs". SpecsDir string `json:"specs_dir"` - // IssuesDir is the issue_id_unique ledger root (repo-relative), holding the - // open/, resolved/, and wontfix/ status directories. Default .abcd/work/issues. - // It lies outside Roots — the rule reads the ledger and runs once. + // IssuesDir is the issue-ledger root (repo-relative) read by the ledger rules, + // issue_id_unique and issue_impact_valid; it holds the open/, resolved/, and + // wontfix/ status directories. Default .abcd/work/issues. It lies outside Roots + // — the rules read the ledger and run once, sharing one scan of it. IssuesDir string `json:"issues_dir"` // Allowlist is the stray_root_docs permitted basename-stem list (upper-cased, // extension-stripped) for top-level markdown files. diff --git a/internal/core/lint/impact_test.go b/internal/core/lint/impact_test.go new file mode 100644 index 0000000..d4838b1 --- /dev/null +++ b/internal/core/lint/impact_test.go @@ -0,0 +1,257 @@ +package lint + +import ( + "path/filepath" + "strings" + "testing" +) + +// impactCase is one record fixture plus the single finding it must (or must not) +// produce. wantMsg empty means the record is clean; otherwise the rule must emit +// exactly one finding, on wantLine, whose message contains wantMsg. +type impactCase struct { + name string + rel string + body string + wantLine int + wantMsg string +} + +// runImpactCases writes each fixture into its own temp repo and lints it with +// only the rule under test enabled, so a finding can come from nowhere else. +func runImpactCases(t *testing.T, ruleID string, rule RuleConfig, cases []impactCase) { + t.Helper() + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + root := t.TempDir() + writeFile(t, root, c.rel, c.body) + cfg := Config{ + Roots: []string{"rec"}, + Rules: map[string]RuleConfig{ruleID: rule}, + } + fs, err := Lint(cfg, root) + if err != nil { + t.Fatal(err) + } + if c.wantMsg == "" { + if n := countRule(fs, ruleID); n != 0 { + t.Fatalf("expected no %s finding, got %d: %+v", ruleID, n, fs) + } + return + } + if n := countRule(fs, ruleID); n != 1 { + t.Fatalf("expected exactly 1 %s finding, got %d: %+v", ruleID, n, fs) + } + f := fs[0] + if f.File != filepath.FromSlash(c.rel) || f.Line != c.wantLine { + t.Errorf("finding at %s:%d, want %s:%d", f.File, f.Line, c.rel, c.wantLine) + } + if f.Severity != severityBlocker { + t.Errorf("severity = %q, want %q", f.Severity, severityBlocker) + } + if !strings.Contains(f.Message, c.wantMsg) { + t.Errorf("message = %q, want it to contain %q", f.Message, c.wantMsg) + } + }) + } +} + +// An intent that has reached shipped/ is in the release set, so its impact is +// what the derived version is computed from: absent, misspelled, and internal +// must each block, with a message that says which of the three failures it is. +// A drafts/ or planned/ intent is NOT required to carry the judgement yet — the +// gate is the move into shipped/ — but a value it DOES carry must be a legal +// one, because a misspelling caught in the bucket where it was written is +// actionable and the same misspelling caught at the shipped/ gate is archaeology. +func TestIntentImpactValid(t *testing.T) { + const base = "rec/intents" + rule := RuleConfig{Enabled: true, Severity: severityBlocker, IntentsDir: "intents"} + + runImpactCases(t, "intent_impact_valid", rule, []impactCase{ + { + name: "shipped without impact", + rel: base + "/shipped/itd-48-thing.md", + body: "---\nid: itd-48\nkind: standalone\nspec_id: spc-10-x\n---\n# x\n", + wantLine: 1, + wantMsg: "impact must be set explicitly", + }, + { + name: "shipped with an explicit null impact", + rel: base + "/shipped/itd-49-thing.md", + body: "---\nid: itd-49\nkind: standalone\nspec_id: spc-10-x\nimpact: null\n---\n# x\n", + wantLine: 5, + wantMsg: "impact must be set explicitly", + }, + { + name: "shipped with a misspelled impact", + rel: base + "/shipped/itd-50-thing.md", + body: "---\nid: itd-50\nkind: standalone\nspec_id: spc-10-x\nimpact: additiv\n---\n# x\n", + wantLine: 5, + wantMsg: "invalid impact 'additiv'", + }, + { + name: "shipped with a capitalised impact", + rel: base + "/shipped/itd-51-thing.md", + body: "---\nid: itd-51\nkind: standalone\nspec_id: spc-10-x\nimpact: Additive\n---\n# x\n", + wantLine: 5, + wantMsg: "invalid impact 'Additive'", + }, + { + name: "shipped with impact internal", + rel: base + "/shipped/itd-52-thing.md", + body: "---\nid: itd-52\nkind: standalone\nspec_id: spc-10-x\nimpact: internal\n---\n# x\n", + wantLine: 5, + wantMsg: "must not be internal on an intent", + }, + { + name: "shipped with impact additive", + rel: base + "/shipped/itd-53-thing.md", + body: "---\nid: itd-53\nkind: standalone\nspec_id: spc-10-x\nimpact: additive\n---\n# x\n", + }, + { + name: "shipped with impact breaking", + rel: base + "/shipped/itd-54-thing.md", + body: "---\nid: itd-54\nkind: standalone\nspec_id: spc-10-x\nimpact: breaking\n---\n# x\n", + }, + { + name: "shipped with impact fix", + rel: base + "/shipped/itd-55-thing.md", + body: "---\nid: itd-55\nkind: standalone\nspec_id: spc-10-x\nimpact: fix\n---\n# x\n", + }, + { + name: "drafts without impact is not gated", + rel: base + "/drafts/itd-10-thing.md", + body: "---\nid: itd-10\nkind: null\nspec_id: null\n---\n# x\n", + }, + { + name: "planned without impact is not gated", + rel: base + "/planned/itd-20-thing.md", + body: "---\nid: itd-20\nkind: standalone\nspec_id: null\n---\n# x\n", + }, + { + name: "disciplines without impact is not gated", + rel: base + "/disciplines/itd-1-thing.md", + body: "---\nid: itd-1\nkind: discipline\nspec_id: null\n---\n# x\n", + }, + { + name: "drafts with a misspelled impact is still flagged", + rel: base + "/drafts/itd-11-thing.md", + body: "---\nid: itd-11\nkind: null\nspec_id: null\nimpact: braking\n---\n# x\n", + wantLine: 5, + wantMsg: "invalid impact 'braking'", + }, + { + name: "planned with impact internal is still flagged", + rel: base + "/planned/itd-21-thing.md", + body: "---\nid: itd-21\nkind: standalone\nspec_id: null\nimpact: internal\n---\n# x\n", + wantLine: 5, + wantMsg: "must not be internal on an intent", + }, + }) +} + +// An issue in resolved/ is in the release set exactly like a shipped intent, so +// the same gate applies — with one difference: internal is a LEGAL judgement on +// an issue, because most resolved issues are plumbing that no user can be told +// about. An open/ or wontfix/ issue is not required to carry the judgement. +func TestIssueImpactValid(t *testing.T) { + const base = ".abcd/work/issues" + rule := RuleConfig{Enabled: true, Severity: severityBlocker, IssuesDir: base} + + runImpactCases(t, "issue_impact_valid", rule, []impactCase{ + { + name: "resolved without impact", + rel: base + "/resolved/iss-56-thing.md", + body: "---\nid: \"iss-56\"\n---\n# x\n", + wantLine: 1, + wantMsg: "impact must be set explicitly", + }, + { + name: "resolved with an explicit null impact", + rel: base + "/resolved/iss-57-thing.md", + body: "---\nid: \"iss-57\"\nimpact: null\n---\n# x\n", + wantLine: 3, + wantMsg: "impact must be set explicitly", + }, + { + name: "resolved with a misspelled impact", + rel: base + "/resolved/iss-58-thing.md", + body: "---\nid: \"iss-58\"\nimpact: fixx\n---\n# x\n", + wantLine: 3, + wantMsg: "invalid impact \"fixx\"", + }, + { + // The ledger quotes ids (id: "iss-56"), so quoting the impact is the + // plausible authoring slip. It is rejected, not absorbed: the shared + // parser matches the enum exactly, and the frontmatter scanner is a line + // scanner rather than a YAML parser, so a tolerated `"fix"` here would + // read as a different string in every other consumer of the field. + name: "resolved with a YAML-quoted impact", + rel: base + "/resolved/iss-62-thing.md", + body: "---\nid: \"iss-62\"\nimpact: \"fix\"\n---\n# x\n", + wantLine: 3, + wantMsg: "invalid impact", + }, + { + name: "resolved with impact internal", + rel: base + "/resolved/iss-59-thing.md", + body: "---\nid: \"iss-59\"\nimpact: internal\n---\n# x\n", + }, + { + name: "resolved with impact fix", + rel: base + "/resolved/iss-60-thing.md", + body: "---\nid: \"iss-60\"\nimpact: fix\n---\n# x\n", + }, + { + name: "resolved with impact additive", + rel: base + "/resolved/iss-61-thing.md", + body: "---\nid: \"iss-61\"\nimpact: additive\n---\n# x\n", + }, + { + name: "open without impact is not gated", + rel: base + "/open/iss-70-thing.md", + body: "---\nid: \"iss-70\"\n---\n# x\n", + }, + { + name: "wontfix without impact is not gated", + rel: base + "/wontfix/iss-71-thing.md", + body: "---\nid: \"iss-71\"\n---\n# x\n", + }, + { + name: "open with a misspelled impact is still flagged", + rel: base + "/open/iss-72-thing.md", + body: "---\nid: \"iss-72\"\nimpact: breaking!\n---\n# x\n", + wantLine: 3, + wantMsg: "invalid impact \"breaking!\"", + }, + }) +} + +// The two rules only gate anything if the shipped config arms them. This is the +// wiring test: it reads the real .abcd/record-lint.json, so disabling either +// rule, or downgrading it out of blocker severity, fails here. It deliberately +// does NOT assert the live record is clean — the records are back-filled +// separately, and a rule that gates nothing until then is still armed. +func TestImpactRulesArmedInRealConfig(t *testing.T) { + repoRoot := filepath.Join("..", "..", "..") + cfg, err := LoadConfig(filepath.Join(repoRoot, ".abcd", "record-lint.json")) + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + for _, id := range []string{"intent_impact_valid", "issue_impact_valid"} { + rc, ok := cfg.Rules[id] + if !ok || !rc.Enabled { + t.Errorf("record-lint.json must enable the %s rule (spc-10)", id) + continue + } + if rc.Severity != severityBlocker { + t.Errorf("%s severity = %q, want %q", id, rc.Severity, severityBlocker) + } + } + if rc := cfg.Rules["intent_impact_valid"]; rc.IntentsDir == "" { + t.Error("intent_impact_valid must declare intents_dir, like intent_lifecycle") + } + if rc := cfg.Rules["issue_impact_valid"]; rc.IssuesDir == "" { + t.Error("issue_impact_valid must declare issues_dir, like issue_id_unique") + } +} diff --git a/internal/core/lint/lint.go b/internal/core/lint/lint.go index e0fb4d8..ebbf418 100644 --- a/internal/core/lint/lint.go +++ b/internal/core/lint/lint.go @@ -15,6 +15,7 @@ import ( "unicode" "unicode/utf8" + "github.com/REPPL/abcd-cli/internal/core/changelog" "github.com/REPPL/abcd-cli/internal/core/frontmatter" "github.com/REPPL/abcd-cli/internal/fsutil" ) @@ -79,8 +80,26 @@ var ( // issueStatusDirs are the issue ledger's status directories (issue_id_unique // scans all three for a duplicated iss-N id). issueStatusDirs = []string{"open", "resolved", "wontfix"} + // intentImpactValues and issueImpactValues render the legal impact set an + // error message offers. They are composed from the shared enum's constants + // (internal/core/changelog) rather than spelled as literals, so a rename there + // is a compile error here instead of a stale message. The intent list omits + // internal: an intent is press-release-first, so "invisible to users" is not a + // judgement it can make. + intentImpactValues = joinImpacts(changelog.ImpactAdditive, changelog.ImpactBreaking, changelog.ImpactFix) + issueImpactValues = joinImpacts(changelog.ImpactAdditive, changelog.ImpactBreaking, changelog.ImpactFix, changelog.ImpactInternal) ) +// joinImpacts renders an impact set the way the shared parser's own errors do, +// so every message about the enum reads alike. +func joinImpacts(impacts ...changelog.Impact) string { + parts := make([]string, 0, len(impacts)) + for _, i := range impacts { + parts = append(parts, string(i)) + } + return strings.Join(parts, "|") +} + // Lint runs every enabled check family against the record under repoRoot and // returns the findings sorted deterministically. An error is returned only for // malformed configuration (e.g. an uncompilable regexp, or a persona_registry @@ -154,12 +173,37 @@ func Lint(cfg Config, repoRoot string) ([]Finding, error) { findings = append(findings, dc...) } + // The intent-tree rules (intent_lifecycle, intent_impact_valid) read the + // SAME bucket tree, so it is scanned once per distinct intents_dir and each + // rule validates the records that scan produced. A rule is never a second + // walk of a tree another rule already read. + intentTrees := map[string]intentTree{} + scanIntents := func(dir string) (intentTree, error) { + if t, ok := intentTrees[dir]; ok { + return t, nil + } + t, err := scanIntentTree(repoRoot, rootAbs, dir, cfg) + if err != nil { + return intentTree{}, err + } + intentTrees[dir] = t + return t, nil + } + if intentCfg, ok := cfg.Rules["intent_lifecycle"]; ok && intentCfg.Enabled { - il, err := checkIntentLifecycle(repoRoot, rootAbs, intentCfg, cfg) + tree, err := scanIntents(intentsDirOf(intentCfg)) if err != nil { return nil, err } - findings = append(findings, il...) + findings = append(findings, checkIntentLifecycle(repoRoot, tree, intentCfg)...) + } + + if impactCfg, ok := cfg.Rules["intent_impact_valid"]; ok && impactCfg.Enabled { + tree, err := scanIntents(intentsDirOf(impactCfg)) + if err != nil { + return nil, err + } + findings = append(findings, checkIntentImpact(tree, impactCfg)...) } if specCfg, ok := cfg.Rules["spec_lifecycle"]; ok && specCfg.Enabled { @@ -232,15 +276,37 @@ func Lint(cfg Config, repoRoot string) ([]Finding, error) { findings = append(findings, gl...) } - // issue_id_unique scans the issue ledger (.abcd/work/issues/{open,resolved, - // wontfix} — outside cfg.Roots) for an iss-N id claimed by two or more files, - // so it too runs once here. + // The issue-ledger rules (issue_id_unique, issue_impact_valid) read the SAME + // ledger (.abcd/work/issues/{open,resolved,wontfix} — outside cfg.Roots), so it + // is scanned once per distinct issues_dir here and each rule validates the + // records that scan produced. + issueLedgers := map[string]issueLedger{} + scanIssues := func(dir string) (issueLedger, error) { + if l, ok := issueLedgers[dir]; ok { + return l, nil + } + l, err := scanIssueLedger(repoRoot, dir) + if err != nil { + return issueLedger{}, err + } + issueLedgers[dir] = l + return l, nil + } + if iiCfg, ok := cfg.Rules["issue_id_unique"]; ok && iiCfg.Enabled { - ii, err := checkIssueIDUnique(repoRoot, iiCfg) + ledger, err := scanIssues(issuesDirOf(iiCfg)) + if err != nil { + return nil, err + } + findings = append(findings, checkIssueIDUnique(repoRoot, ledger, iiCfg)...) + } + + if impactCfg, ok := cfg.Rules["issue_impact_valid"]; ok && impactCfg.Enabled { + ledger, err := scanIssues(issuesDirOf(impactCfg)) if err != nil { return nil, err } - findings = append(findings, ii...) + findings = append(findings, checkIssueImpact(ledger, impactCfg)...) } sortFindings(findings) @@ -1085,15 +1151,44 @@ func checkDirectoryCoverage(repoRoot, rootAbs string, cfg RuleConfig) ([]Finding return out, nil } -// checkIntentLifecycle implements check family F. -func checkIntentLifecycle(repoRoot, rootAbs string, cfg RuleConfig, top Config) ([]Finding, error) { - intentsDir := cfg.IntentsDir - if intentsDir == "" { - intentsDir = "intents" +// intentRecord is one intent file as the intent-tree rules see it: its +// repo-relative path, the bucket directory that IS its lifecycle state, its +// basename (which carries the id), and its parsed frontmatter. +type intentRecord struct { + rel string + bucket string + name string + fields map[string]fmField +} + +// intentTree is ONE scan of the intent buckets, shared by every rule that reads +// the tree (intent_lifecycle, intent_impact_valid). records excludes the +// historically-exempt files (contentExempt), because every rule over this tree +// is a content check; known and idFiles index EVERY intent file found anywhere +// beneath it, exempt or not, because a superseded_by target and an id collision +// must resolve against the whole corpus. +type intentTree struct { + records []intentRecord + known map[string]bool + idFiles map[string][]string +} + +// intentsDirOf resolves a rule's intents subdirectory (relative to a record +// root), defaulting to "intents". It is shared so every intent-tree rule spells +// the default once and two rules configured alike scan one tree, not two. +func intentsDirOf(cfg RuleConfig) string { + if cfg.IntentsDir == "" { + return "intents" } + return cfg.IntentsDir +} + +// scanIntentTree reads the intent buckets once. A missing intents/ directory is +// soft (no records, no error), mirroring the rest of the record lint. +func scanIntentTree(repoRoot, rootAbs, intentsDir string, top Config) (intentTree, error) { intentsRoot := filepath.Join(rootAbs, intentsDir) if _, err := os.Stat(intentsRoot); err != nil { - return nil, nil + return intentTree{}, nil } // Collect every intent id that exists as a file in any bucket, so the @@ -1116,9 +1211,9 @@ func checkIntentLifecycle(repoRoot, rootAbs string, cfg RuleConfig, top Config) buckets, err := os.ReadDir(intentsRoot) if err != nil { - return nil, err + return intentTree{}, err } - var out []Finding + tree := intentTree{known: known, idFiles: idFiles} for _, b := range buckets { if !b.IsDir() || !intentBuckets[b.Name()] { continue @@ -1127,7 +1222,7 @@ func checkIntentLifecycle(repoRoot, rootAbs string, cfg RuleConfig, top Config) bucketDir := filepath.Join(intentsRoot, bucket) entries, err := os.ReadDir(bucketDir) if err != nil { - return nil, err + return intentTree{}, err } for _, e := range entries { if e.IsDir() || !intentFileRe.MatchString(e.Name()) { @@ -1136,18 +1231,88 @@ func checkIntentLifecycle(repoRoot, rootAbs string, cfg RuleConfig, top Config) fileAbs := filepath.Join(bucketDir, e.Name()) content, err := os.ReadFile(fileAbs) if err != nil { - return nil, err + return intentTree{}, err } rel := repoRel(repoRoot, fileAbs) fields := frontmatterFields(strings.Split(string(content), "\n")) if contentExempt(rel, fields, top) { continue } - out = append(out, validateIntent(rel, bucket, fields, known, cfg.Severity)...) - out = append(out, validateIntentIDUnique(repoRoot, rel, e.Name(), fields, idFiles, cfg.Severity)...) + tree.records = append(tree.records, intentRecord{ + rel: rel, bucket: bucket, name: e.Name(), fields: fields, + }) } } - return out, nil + return tree, nil +} + +// checkIntentLifecycle implements check family F over an already-scanned tree. +func checkIntentLifecycle(repoRoot string, tree intentTree, cfg RuleConfig) []Finding { + var out []Finding + for _, r := range tree.records { + out = append(out, validateIntent(r.rel, r.bucket, r.fields, tree.known, cfg.Severity)...) + out = append(out, validateIntentIDUnique(repoRoot, r.rel, r.name, r.fields, tree.idFiles, cfg.Severity)...) + } + return out +} + +// checkIntentImpact implements intent_impact_valid: an intent may not sit in +// shipped/ without a valid, non-internal impact (spc-10, plan outcome 8). +// +// WHY the gate is the move into shipped/ and nothing earlier: the judgement is +// made when the intent is shaped, but it only becomes load-bearing when the +// intent enters the release set the version is derived from, and a draft nobody +// has designed yet cannot answer it honestly. Requiring it in drafts/ would +// trade a real signal for ceremony. +// +// WHY a value present OUTSIDE shipped/ is still validated: absence in drafts/ is +// "not decided yet", but `impact: braking` is not an undecided judgement, it is +// a wrong one. Caught in the bucket where it was written it is a typo the author +// fixes; caught only at the shipped/ gate it is archaeology. The two failures +// are therefore split — presence is gated at the terminal bucket, legality +// everywhere. +// +// WHY internal fails on an intent although it is legal on an issue: an intent is +// press-release-first — it exists because a user can be told about it — so +// "invisible to users" is a category error on an intent rather than a valid +// judgement. Plumbing belongs on an issue, which may be internal. +// +// `impact: null` reads as absent, matching the rest of the intent schema, where +// null is how a record says "this field does not apply yet" (kind, spec_id). +func checkIntentImpact(tree intentTree, cfg RuleConfig) []Finding { + var out []Finding + for _, r := range tree.records { + f := r.fields["impact"] + line := f.line + if line == 0 { + line = 1 + } + add := func(msg string) { + out = append(out, Finding{ + File: r.rel, Line: line, RuleID: "intent_impact_valid", + Severity: cfg.Severity, Message: msg, + }) + } + if isNull(f.value) { + if r.bucket == "shipped" { + add("shipped: impact must be set explicitly to one of " + intentImpactValues + + " — it decides the derived version, and there is no default") + } + continue + } + impact, err := changelog.ParseImpact(f.value) + if err != nil { + add("invalid impact '" + f.value + "': an intent must declare exactly one of " + + intentImpactValues + " (lower-case, no surrounding whitespace)") + continue + } + if impact == changelog.ImpactInternal { + add("impact must not be internal on an intent: a press-release-first intent is " + + "user-facing by definition — declare one of " + intentImpactValues + + ", or record the work as an issue instead") + } + } + return out } // validateIntentIDUnique flags a duplicated intent id, delegating to the shared @@ -1188,39 +1353,60 @@ func validateIDUnique(repoRoot, rel, id, noun, ruleID, severity string, fields m }} } -// checkIssueIDUnique flags any iss-N id claimed by two or more files across the -// issue ledger's status directories (open/, resolved/, wontfix/). The capture -// allocator rejects a duplicate on the reservation path, but a hand-added issue -// file that bypassed it — how a past iss-56 collision arose — slips straight -// through; this is the record-lint backstop that catches it. It is the issue-side -// mirror of the intent-id uniqueness rule and shares the same validateIDUnique -// primitive. The ledger lives outside cfg.Roots, so the rule runs once, not -// per-root. A missing ledger is not an error — it yields no findings. -func checkIssueIDUnique(repoRoot string, cfg RuleConfig) ([]Finding, error) { - issuesDir := cfg.IssuesDir - if issuesDir == "" { - issuesDir = ".abcd/work/issues" +// issueRecord is one issue file as the ledger rules see it: its repo-relative +// path, the status directory that IS its lifecycle state, its iss-N id, and its +// parsed frontmatter. +type issueRecord struct { + rel string + status string + id string + fields map[string]fmField +} + +// issueLedger is ONE scan of the issue ledger's status directories, shared by +// every rule that reads it (issue_id_unique, issue_impact_valid). idFiles maps +// each iss-N id to the repo-absolute paths claiming it. +type issueLedger struct { + records []issueRecord + idFiles map[string][]string +} + +// issuesDirOf resolves a rule's issue-ledger root (repo-relative), defaulting to +// .abcd/work/issues. It is shared so every ledger rule spells the default once +// and two rules configured alike scan one ledger, not two. +func issuesDirOf(cfg RuleConfig) string { + if cfg.IssuesDir == "" { + return ".abcd/work/issues" } + return cfg.IssuesDir +} + +// scanIssueLedger reads the ledger's status directories (open/, resolved/, +// wontfix/) once. The ledger lives outside cfg.Roots, so its rules run once, not +// per-root. A missing ledger, or a missing status directory within it, is not an +// error — it simply contributes no records. +func scanIssueLedger(repoRoot, issuesDir string) (issueLedger, error) { issuesRoot := filepath.Join(repoRoot, issuesDir) if _, err := os.Stat(issuesRoot); err != nil { if os.IsNotExist(err) { - return nil, nil + return issueLedger{}, nil } - return nil, err + return issueLedger{}, err } // Track every file each iss-N id claims across the three status dirs: an id is // the issue's identity across the ledger, and a bypassed-allocator file (or two // parallel branches) can land the same id silently otherwise. - idFiles := map[string][]string{} - var files []string + ledger := issueLedger{idFiles: map[string][]string{}} + type pending struct{ status, abs string } + var files []pending for _, sub := range issueStatusDirs { entries, err := os.ReadDir(filepath.Join(issuesRoot, sub)) if err != nil { if os.IsNotExist(err) { continue } - return nil, err + return issueLedger{}, err } for _, e := range entries { if e.IsDir() || !issueFileRe.MatchString(e.Name()) { @@ -1228,23 +1414,83 @@ func checkIssueIDUnique(repoRoot string, cfg RuleConfig) ([]Finding, error) { } fileAbs := filepath.Join(issuesRoot, sub, e.Name()) id := issueIDRe.FindString(e.Name()) - idFiles[id] = append(idFiles[id], fileAbs) - files = append(files, fileAbs) + ledger.idFiles[id] = append(ledger.idFiles[id], fileAbs) + files = append(files, pending{status: sub, abs: fileAbs}) } } - var out []Finding - for _, fileAbs := range files { - id := issueIDRe.FindString(filepath.Base(fileAbs)) - content, err := os.ReadFile(fileAbs) + for _, p := range files { + content, err := os.ReadFile(p.abs) if err != nil { - return nil, err + return issueLedger{}, err } - rel := repoRel(repoRoot, fileAbs) - fields := frontmatterFields(strings.Split(string(content), "\n")) - out = append(out, validateIDUnique(repoRoot, rel, id, "issue", "issue_id_unique", cfg.Severity, fields, idFiles)...) + ledger.records = append(ledger.records, issueRecord{ + rel: repoRel(repoRoot, p.abs), + status: p.status, + id: issueIDRe.FindString(filepath.Base(p.abs)), + fields: frontmatterFields(strings.Split(string(content), "\n")), + }) } - return out, nil + return ledger, nil +} + +// checkIssueIDUnique flags any iss-N id claimed by two or more files across the +// issue ledger's status directories (open/, resolved/, wontfix/). The capture +// allocator rejects a duplicate on the reservation path, but a hand-added issue +// file that bypassed it — how a past iss-56 collision arose — slips straight +// through; this is the record-lint backstop that catches it. It is the issue-side +// mirror of the intent-id uniqueness rule and shares the same validateIDUnique +// primitive. +func checkIssueIDUnique(repoRoot string, ledger issueLedger, cfg RuleConfig) []Finding { + var out []Finding + for _, r := range ledger.records { + out = append(out, validateIDUnique(repoRoot, r.rel, r.id, "issue", "issue_id_unique", cfg.Severity, r.fields, ledger.idFiles)...) + } + return out +} + +// checkIssueImpact implements issue_impact_valid: an issue may not sit in +// resolved/ without a valid impact (spc-10, plan outcome 8). +// +// It is the issue-side mirror of intent_impact_valid, with one deliberate +// difference: internal is a LEGAL judgement here. Most resolved issues are +// plumbing — a TOCTOU fix, an atomic-write hardening, a lint internal — that no +// user can be told about, and a rule that refused internal would either force +// that work into a user-facing changelog or push authors into a mislabel. +// +// Absence gates only the terminal bucket (resolved/), while legality is checked +// wherever a value is written, for the same reason as on the intent side: an +// open issue has not been judged yet, but a misspelt judgement is wrong the +// moment it is typed. `impact: null` reads as absent, matching the schema +// convention that null means "does not apply yet". The message for a malformed +// value is the shared parser's own, so the lint and the derivation describe the +// same defect in the same words. +func checkIssueImpact(ledger issueLedger, cfg RuleConfig) []Finding { + var out []Finding + for _, r := range ledger.records { + f := r.fields["impact"] + line := f.line + if line == 0 { + line = 1 + } + add := func(msg string) { + out = append(out, Finding{ + File: r.rel, Line: line, RuleID: "issue_impact_valid", + Severity: cfg.Severity, Message: msg, + }) + } + if isNull(f.value) { + if r.status == "resolved" { + add("resolved: impact must be set explicitly to one of " + issueImpactValues + + " — it decides the derived version and the changelog line, and there is no default") + } + continue + } + if _, err := changelog.ParseImpact(f.value); err != nil { + add(err.Error()) + } + } + return out } func validateIntent(rel, bucket string, fields map[string]fmField, known map[string]bool, severity string) []Finding {