From 1f833fcef4085ddd5b56ca93c9229e3b1a260a2a Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 12 Jul 2026 22:01:47 +0100 Subject: [PATCH 1/2] fix: block duplicate intent ids in intent_lifecycle Two agents worked in parallel on separate branches. Each allocated "the next free" intent id by scanning existing files for max + 1. Both got itd-82. Both merged. Two intents now claim itd-82 on main, and every gate stayed green -- record-lint never checked id uniqueness at all. The id is the intent's identity across the whole record: cross-references are written as "itd-N", so a duplicate makes every reference to it ambiguous. That is a blocker, not a warning. checkIntentLifecycle already walked every intent file and collected ids -- into a map[string]bool, which silently deduped exactly the condition we needed to detect. It now tracks the files each id claims and 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. Detector-first: the test was watched fail on a real collision shape (same id in one bucket, and the same id across two buckets) before the rule existed, then watched pass. Run against the live record it fires on both itd-82 files. This is the DETECTION half only. The minting scheme is the actual defect -- a branch-local allocator cannot see ids minted on unmerged branches, and the same class already bit the iss-N ledger (iss-74). Tracked as iss-80. Assisted-by: Claude:claude-opus-4-8 --- ...f.md => itd-83-review-bar-fires-itself.md} | 0 CHANGELOG.md | 11 +++++ internal/core/lint/lint.go | 38 ++++++++++++++- internal/core/lint/lint_test.go | 48 +++++++++++++++++++ 4 files changed, 95 insertions(+), 2 deletions(-) rename .abcd/development/intents/drafts/{itd-82-review-bar-fires-itself.md => itd-83-review-bar-fires-itself.md} (100%) 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 100% 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 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. From 4f88f349bc14571fcbd92f013dca5494504bff93 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 12 Jul 2026 22:01:57 +0100 Subject: [PATCH 2/2] docs: resolve the itd-82 collision (renumber to itd-83) + capture iss-80 The armed detector fires on both itd-82 files. This is the fix behind it. The later claimant yields: itd-82-review-bar-fires-itself (PR #47, merged second) becomes itd-83; itd-82-drain-ledger-triage (PR #46, merged first) keeps itd-82. This follows the precedent set when the ledger allocator collided -- iss-77 was renumbered to iss-79, the newer allocation giving way. No document referenced the id yet, so nothing else moves. Renumbering contradicts the record's own "ids are capture-stable, never renumbered" invariant. That contradiction is the point: the invariant is only holdable if ids cannot collide in the first place, and a branch-local max + 1 allocator guarantees they can. Fixing the collision protects the invariant going forward; it does not excuse the allocator. iss-80 captures the minting half -- a collision-free scheme (forge-minted, random-suffix, timestamp, or a reserve-registry) is under research. Until it lands, the detector is what stops a silent duplicate reaching main. Assisted-by: Claude:claude-opus-4-8 --- .../intents/drafts/itd-83-review-bar-fires-itself.md | 2 +- ...ocators-itd-n-spc-n-iss-n-are-branch-local-para.md | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 .abcd/work/issues/open/iss-80-record-id-allocators-itd-n-spc-n-iss-n-are-branch-local-para.md diff --git a/.abcd/development/intents/drafts/itd-83-review-bar-fires-itself.md b/.abcd/development/intents/drafts/itd-83-review-bar-fires-itself.md index 2ccf7642..3dbe80dc 100644 --- a/.abcd/development/intents/drafts/itd-83-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