diff --git a/.abcd/development/intents/drafts/itd-82-review-bar-fires-itself.md b/.abcd/development/intents/drafts/itd-83-review-bar-fires-itself.md similarity index 99% rename from .abcd/development/intents/drafts/itd-82-review-bar-fires-itself.md rename to .abcd/development/intents/drafts/itd-83-review-bar-fires-itself.md index 2ccf7642..3dbe80dc 100644 --- a/.abcd/development/intents/drafts/itd-82-review-bar-fires-itself.md +++ b/.abcd/development/intents/drafts/itd-83-review-bar-fires-itself.md @@ -1,5 +1,5 @@ --- -id: itd-82 +id: itd-83 slug: review-bar-fires-itself spec_id: null kind: null diff --git a/.abcd/work/issues/open/iss-80-record-id-allocators-itd-n-spc-n-iss-n-are-branch-local-para.md b/.abcd/work/issues/open/iss-80-record-id-allocators-itd-n-spc-n-iss-n-are-branch-local-para.md new file mode 100644 index 00000000..5bcd61fd --- /dev/null +++ b/.abcd/work/issues/open/iss-80-record-id-allocators-itd-n-spc-n-iss-n-are-branch-local-para.md @@ -0,0 +1,11 @@ +--- +schema_version: 1 +id: "iss-80" +slug: "record-id-allocators-itd-n-spc-n-iss-n-are-branch-local-para" +severity: "major" +category: "bug" +source: "agent-finding" +found_during: "parallel-agent-run" +--- + +Record id allocators (itd-N, spc-N, iss-N) are branch-local: parallel agents on separate branches each scan for max+1 and mint the SAME id. Two intents both claimed itd-82 and both merged to main (PRs #46, #47). The iss-N allocator hit the same class before (iss-77 collision, manually renumbered to iss-79; class recorded as iss-74). Resolving a collision forces a renumber, which breaks the record's stated 'ids are capture-stable, never renumbered' invariant -- so the minting scheme, not the renumber, is the defect. This capture is the MINTING half: a collision-free allocation scheme is needed (forge-minted / random-suffix / timestamp / reserve-registry -- SOTA under research). The DETECTION half is armed separately: intent_lifecycle now blocks duplicate intent ids. \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 56a24f7a..07f399ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,17 @@ called out in a **Breaking** section. ## [Unreleased] +### Fixed + +- The `intent_lifecycle` record-lint rule now **blocks duplicate intent ids**. + Id allocators are branch-local — parallel agents on separate branches each + scan for `max + 1` and mint the same id — so two intents both claimed + `itd-82` and both merged with every gate green. The rule flags *every* file in + a colliding set, not just one: the linter cannot know which claimant is + authoritative, and flagging a single file would imply the others are fine. The + collision itself is resolved (the later claimant renumbered to `itd-83`); the + underlying minting scheme is tracked as `iss-80`. + ### Added - **Four reviewer agents ship with the plugin**: `abcd:ruthless-reviewer` diff --git a/internal/core/lint/lint.go b/internal/core/lint/lint.go index e50cf410..3b831d06 100644 --- a/internal/core/lint/lint.go +++ b/internal/core/lint/lint.go @@ -1071,14 +1071,19 @@ func checkIntentLifecycle(repoRoot, rootAbs string, cfg RuleConfig, top Config) } // Collect every intent id that exists as a file in any bucket, so the - // superseded_by target can be checked for existence. + // superseded_by target can be checked for existence. Track the files each id + // claims: ids are the intent's identity across the record, and parallel + // branches each allocating "the next free id" collide silently otherwise. known := map[string]bool{} + idFiles := map[string][]string{} _ = filepath.WalkDir(intentsRoot, func(path string, d os.DirEntry, err error) error { if err != nil || d.IsDir() { return nil } if intentFileRe.MatchString(d.Name()) { - known[intentIDRe.FindString(d.Name())] = true + id := intentIDRe.FindString(d.Name()) + known[id] = true + idFiles[id] = append(idFiles[id], path) } return nil }) @@ -1113,11 +1118,40 @@ func checkIntentLifecycle(repoRoot, rootAbs string, cfg RuleConfig, top Config) continue } out = append(out, validateIntent(rel, bucket, fields, known, cfg.Severity)...) + out = append(out, validateIntentIDUnique(repoRoot, rel, e.Name(), fields, idFiles, cfg.Severity)...) } } return out, nil } +// validateIntentIDUnique flags every file in a colliding set, not just one: +// the linter cannot know which claimant is authoritative, and flagging a single +// file would imply the others are fine. +func validateIntentIDUnique(repoRoot, rel, name string, fields map[string]fmField, idFiles map[string][]string, severity string) []Finding { + id := intentIDRe.FindString(name) + claimants := idFiles[id] + if len(claimants) < 2 { + return nil + } + others := make([]string, 0, len(claimants)-1) + for _, p := range claimants { + if r := repoRel(repoRoot, p); r != rel { + others = append(others, r) + } + } + sort.Strings(others) + + line := 1 + if f, ok := fields["id"]; ok && f.line > 0 { + line = f.line + } + return []Finding{{ + File: rel, Line: line, RuleID: "intent_lifecycle", Severity: severity, + Message: "duplicate intent id " + id + "; also claimed by " + strings.Join(others, ", ") + + " — an id is the intent's identity across the record and must be unique", + }} +} + func validateIntent(rel, bucket string, fields map[string]fmField, known map[string]bool, severity string) []Finding { var out []Finding add := func(line int, msg string) { diff --git a/internal/core/lint/lint_test.go b/internal/core/lint/lint_test.go index 5d4dbacf..f2ce444e 100644 --- a/internal/core/lint/lint_test.go +++ b/internal/core/lint/lint_test.go @@ -300,6 +300,54 @@ func TestIntentLifecycle(t *testing.T) { } } +// Two parallel branches each allocate "the next free" intent id, and both land. +// The id is the intent's identity across the whole record, so a duplicate is a +// blocker on BOTH files -- neither is authoritative, and a reader cannot tell +// which itd-N a cross-reference means. +func TestIntentLifecycleDuplicateID(t *testing.T) { + root := t.TempDir() + base := "rec/intents" + + // Same id, different slugs, different buckets -- the real collision shape. + writeFile(t, root, base+"/drafts/itd-82-first-one.md", "---\nid: itd-82\nkind: null\nspec_id: null\n---\n# one\n") + writeFile(t, root, base+"/drafts/itd-82-second-one.md", "---\nid: itd-82\nkind: null\nspec_id: null\n---\n# two\n") + // A collision across buckets must be caught too, not just within one. + writeFile(t, root, base+"/planned/itd-90-here.md", "---\nid: itd-90\nkind: standalone\nspec_id: spc-3-x\n---\n# a\n") + writeFile(t, root, base+"/drafts/itd-90-there.md", "---\nid: itd-90\nkind: null\nspec_id: null\n---\n# b\n") + // A unique id must stay clean. + writeFile(t, root, base+"/drafts/itd-91-unique.md", "---\nid: itd-91\nkind: null\nspec_id: null\n---\n# ok\n") + + cfg := Config{ + Roots: []string{"rec"}, + Rules: map[string]RuleConfig{ + "intent_lifecycle": {Enabled: true, Severity: "blocker", IntentsDir: "intents"}, + }, + } + fs, err := Lint(cfg, root) + if err != nil { + t.Fatal(err) + } + + // Every file in a colliding set is flagged -- flagging only one would imply + // the other is the "right" one, which the linter cannot know. + dupes := []string{ + filepath.Join(base, "drafts", "itd-82-first-one.md"), + filepath.Join(base, "drafts", "itd-82-second-one.md"), + filepath.Join(base, "planned", "itd-90-here.md"), + filepath.Join(base, "drafts", "itd-90-there.md"), + } + for _, f := range dupes { + if !hasFinding(fs, f, "intent_lifecycle", 2) { // the id: line + t.Errorf("expected duplicate-id finding on %s:2; got %+v", f, fs) + } + } + for _, f := range fs { + if filepath.Base(f.File) == "itd-91-unique.md" { + t.Errorf("unexpected finding on unique intent: %+v", f) + } + } +} + func TestExemptions(t *testing.T) { root := t.TempDir() // A banned token in an exempt_paths file → no finding.