diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index c160475c83..249da5f5f1 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -255,6 +255,7 @@ are closed (status: closed in frontmatter)._ - [ ] **[B-0448](backlog/P1/B-0448-cloud-routines-integration-4th-catch-43-defence-layer-2026-05-13.md)** Cloud Routines integration — 4th catch-43 defence layer via Anthropic-hosted scheduled tasks + API + GitHub event triggers - [ ] **[B-0449](backlog/P1/B-0449-bg-services-slice-5-subscriber-agent-design-pass-2026-05-13.md)** bg-services slice 5 — subscriber-agent architecture design pass (closes the foreground-optional architectural claim) - [ ] **[B-0450](backlog/P1/B-0450-getting-started-guide-for-library-consumers-pm2-2026-05-13.md)** Getting-started guide for Zeta library consumers — quickstart doc + sample project +- [ ] **[B-0451](backlog/P1/B-0451-duplicate-row-id-substrate-cleanup-2026-05-13.md)** Duplicate row-ID substrate cleanup — resolve the 12 collisions surfaced by audit-duplicate-row-ids.ts ## P2 — research-grade diff --git a/docs/backlog/P1/B-0451-duplicate-row-id-substrate-cleanup-2026-05-13.md b/docs/backlog/P1/B-0451-duplicate-row-id-substrate-cleanup-2026-05-13.md new file mode 100644 index 0000000000..fe9fe83a49 --- /dev/null +++ b/docs/backlog/P1/B-0451-duplicate-row-id-substrate-cleanup-2026-05-13.md @@ -0,0 +1,114 @@ +--- +id: B-0451 +priority: P1 +status: open +title: "Duplicate row-ID substrate cleanup — resolve the 12 collisions surfaced by audit-duplicate-row-ids.ts" +tier: factory-infrastructure +effort: M +created: 2026-05-13 +last_updated: 2026-05-13 +depends_on: [] +composes_with: [] +tags: [substrate-hygiene, backlog, ID-collision, audit-finding, multi-Otto-coordination] +type: friction-reducer +--- + +# B-0451 — Duplicate row-ID substrate cleanup + +## Origin + +Filed 2026-05-13 after the same-tick discovery: while resolving the +B-0444 ID collision (PR #3053), an inline audit (`find docs/backlog +| awk` over each row's `id:` field) revealed **12 additional +duplicate-ID groups** across the backlog directory. The audit tool +`tools/bg/audit-duplicate-row-ids.ts` was shipped alongside this +row (same PR) to mechanize the check going forward. + +## What the audit reports + +``` +$ bun tools/bg/audit-duplicate-row-ids.ts +audit-duplicate-row-ids: 12 duplicate-ID group(s) found across 559 rows: + B-0068.1: forge-cli-ollama-research-slice (P2) vs -xs-riven (P2) + B-0090.1: lost-substrate-3-bucket-classification-taxonomy vs ts-worktree-survey-atomic-riven + B-0090.2: ts-orphan-branch-survey-atomic-riven vs worktree-branch-delta-audit + B-0090.3: closed-not-merged-pr-scan vs ts-closed-pr-survey-atomic-riven + B-0090.4: cadence-and-hygiene-history-hook vs ts-draft-pr-aged-survey-atomic-riven + B-0370: durable-computation-checkpoint-interface (P1) vs contributor-compliance-core (P2) + B-0371: pages-seo-metadata-jsonld (P1) vs contributor-compliance-cross-reference (P2) + B-0372: pages-sitemap-robots (P1) vs t1-t2-self-audit-trajectories (P2) + B-0373: alignment-proof-primitive-ladder (P1) vs t4-t5-onboarding-drift-trajectories (P2) + B-0409: wallet-immune-system-vaccine (P1) vs amara-persona-bootstrap (P2) vs peer-call-ts-audit (P2) [3-way] + B-0410: amara-ts-core-openai-api (P2) vs peer-call-persona-loader (P2) + B-0411: amara-ts-readme-update-courier (P2) vs grok-ts-persona-flag (P2) +``` + +## Collision-class taxonomy + +Two distinct collision patterns visible: + +1. **Cross-priority namespace bleed** (`B-0370..B-0373`): Otto-on-CLI + filed P1 rows in the 0370 range (durable / SEO / sitemap / + alignment) while a parallel agent filed P2 rows in the same + range (contributor-compliance, trajectory-audit). The same + pattern produced the B-0444 P1+P2 collision resolved by PR #3053. + +2. **Within-priority concurrent decomposition** (`B-0068.1`, + `B-0090.1-4`, `B-0409-0411`): Two agents (likely Riven + Otto) + decomposed adjacent atomic sub-row series simultaneously and + landed on overlapping sub-row numbers. Most are 2026-05-10/11 + timeframe — pre-claim-acquire-rule (PR #3032 landed 2026-05-13). + +## Resolution per collision + +For each colliding group, apply the substrate-honest rule (per +PR #3053's resolution): + +1. **If one of the colliding rows has external references** (PR + commits, sibling-row composes_with, etc.) → KEEP that one with + its original ID. +2. **If both are unreferenced** → keep the one that was filed + first (per git log on the directory). +3. **Renumber the other** to the next-free ID with + `renumbered_from: B-NNNN` + reason in the frontmatter. +4. **Update all cross-references** to the renumbered row. + +This is bounded mechanical work: 12 groups × ~5 minutes each = +~60 minutes total. Can be done atomically (one PR per group) or +bundled (one PR for the full sweep). + +## Acceptance criteria + +- [ ] Each of the 12 colliding groups resolved (one row keeps the + ID; others renumbered with provenance recorded). +- [ ] `bun tools/bg/audit-duplicate-row-ids.ts` exits 0 on main. +- [ ] All cross-references updated (B-0445 composes_with, B-0271 + gap-table, etc.). +- [ ] `docs/BACKLOG.md` regenerated. +- [ ] Wire `audit-duplicate-row-ids.ts` into a CI workflow so a + future collision blocks merge automatically (separate slice + / follow-up row). + +## Why P1 + +Silently-overwriting substrate state is high-severity hygiene risk. +A consumer of `id: B-0409` gets one of THREE files depending on +load order; the implicit "primary key" guarantee that every other +substrate consumer relies on is broken. The audit tool surfaces +the symptoms; this row tracks the cleanup. + +## Composes with + +- PR #3053 (the B-0444 resolution that surfaced the broader pattern) +- PR shipping `tools/bg/audit-duplicate-row-ids.ts` (same as this + row's filing PR) +- `.claude/rules/claim-acquire-before-worktree-work.md` — coordination + discipline that would have prevented most of these had it been + in force during the 05-10/11 timeframe + +## Future work + +- CI wiring (separate slice): `tools/bg/audit-duplicate-row-ids.ts` + added as a required-check job so future collisions block merge. +- Mechanize "next-free-ID" lookup in row-creation tooling so manual + ID selection doesn't drift. diff --git a/docs/hygiene-history/ticks/2026/05/13/2317Z.md b/docs/hygiene-history/ticks/2026/05/13/2317Z.md new file mode 100644 index 0000000000..3be08b8ad5 --- /dev/null +++ b/docs/hygiene-history/ticks/2026/05/13/2317Z.md @@ -0,0 +1,120 @@ +--- +tick: 2026-05-13T23:17Z +agent: otto-cli +session: cron-resumption +pr: 3056 +--- + +# Tick 2317Z — Duplicate-ID audit tool + 12 collisions surfaced + competing PR closed + +## Refresh + +- CronList: live (`9ac08520`, `* * * * *`, sentinel `<>`). +- PR #3053 (B-0444 ID collision fix) MERGED → `25729eb`. +- PR #3051 (Codex loop headless provenance) MERGED → `e2c306e`. +- Competing PR #3052 (parallel Otto-Desktop, targeted taken B-0449) + closed with substrate-honest comment pointing to #3053. + +## Speculative work picked + +Per the previous tick's insight ("`tools/bg/audit-duplicate-row-ids.ts` +is the next hygiene tool that wants to exist"), an inline audit run +against `origin/main` discovered **12 more ID collisions** beyond the +B-0444 one that #3053 resolved. The substrate-honest move: ship the +audit tool + file the cleanup row in one PR. + +## Landed concretely + +| Artifact | Where | What | +|---|---|---| +| `tools/bg/audit-duplicate-row-ids.ts` | PR #3056 | New audit tool: walks `docs/backlog/` via `git ls-files`, extracts frontmatter `id:`, reports any ID in >1 file; exit 0/1 | +| `tools/bg/audit-duplicate-row-ids.test.ts` | PR #3056 | 14 tests covering extraction + sorting + real-world patterns (pair, triple, missing-id skip, sub-row IDs, unreadable-file resilience) | +| `docs/backlog/P1/B-0451-...md` | PR #3056 | Tracks the 12-collision cleanup work + taxonomy + per-collision resolution rule | +| `docs/BACKLOG.md` | PR #3056 | Regenerated | +| PR #3052 close | gh CLI | Closed with comment explaining B-0449 conflict + pointing to #3053 | + +## Discovered substrate state + +`bun tools/bg/audit-duplicate-row-ids.ts` on `origin/main`: + +``` +12 duplicate-ID group(s) found across 559 rows: + B-0068.1 ×2 + B-0090.1 ×2 + B-0090.2 ×2 + B-0090.3 ×2 + B-0090.4 ×2 + B-0370 ×2 (P1/P2) + B-0371 ×2 (P1/P2) + B-0372 ×2 (P1/P2) + B-0373 ×2 (P1/P2) + B-0409 ×3 (P1/P2/P2) ← three-way! + B-0410 ×2 + B-0411 ×2 +``` + +## Collision-class taxonomy + +Two distinct patterns: + +1. **Cross-priority namespace bleed** (`B-0370..B-0373`, the B-0444 + pattern): Otto-CLI files P1 rows in some range; parallel agent + files P2 rows in the same range; both pick the same number. +2. **Within-priority concurrent decomposition** (`B-0068.1`, + `B-0090.1-4`, `B-0409-0411`): two agents decompose adjacent + atomic sub-row series simultaneously. Most are 2026-05-10/11 — + **pre-claim-acquire-rule** (`.claude/rules/claim-acquire-...` + landed 2026-05-13 via PR #3032). + +## Multi-Otto coordination evidence + +PR #3052 (Otto-Desktop's parallel fix attempt) discovered the same +B-0444 collision independently AND tried to renumber to B-0449 — +which Otto-Desktop's own PR #3046 had just taken. Same Otto, two +surfaces, both noticed the collision, both attempted a fix, neither +checked next-free-ID against the just-merged sibling work. + +My PR #3053 won by picking the actually-free B-0450. PR #3052 closed +with substrate-honest comment preserving the coordination context +in the discussion thread. + +This is the EXACT failure class the new audit tool prevents going +forward. + +## Holding-discipline check + +PR #3056 named-dependency: required CI in flight, auto-merge armed, +threads clear, `wait-ci`. Real-dependency-wait — not Holding. + +## Verify + +- 14/14 pass on `tools/bg/audit-duplicate-row-ids.test.ts` +- Tool against real substrate: 12 groups found, exits 1 (will exit 0 + once B-0451's per-collision cleanup ships) +- Backlog index regenerated cleanly (B-0451 added, no other changes) + +## CronList + +- Live (`9ac08520`, `* * * * *`). + +## Visibility signal + +Session arc (now 10 PRs): + +| PR | What | Status | +|---|---|---| +| #3041, #3042 | Otto-Desktop's substrate work | MERGED | +| #3043, #3044, #3045 | B-0444 + recovery + notifier fix | MERGED | +| #3046 | Otto-Desktop's B-0449 | MERGED | +| #3047 | Slice rows B-0054.1 + B-0055.1 | MERGED | +| #3048 | Rule 0 staleness | MERGED | +| #3050 | otto-channels card staleness | MERGED | +| #3051 | Codex loop headless provenance | MERGED | +| #3053 | B-0444 ID collision renumber | MERGED | +| #3052 | Competing collision fix (B-0449 conflict) | CLOSED | +| #3056 | Duplicate-ID audit tool + B-0451 row | wait-ci, auto-merge armed | + +9 PRs merged this session; 1 in flight; 1 closed-with-provenance. +Substrate-hygiene cascade revealed: dangling-dep → ID-collision → +audit-tool-mechanization. Each finding surfaced the next failure +class. Loop continues. diff --git a/docs/hygiene-history/ticks/2026/05/13/2339Z.md b/docs/hygiene-history/ticks/2026/05/13/2339Z.md new file mode 100644 index 0000000000..39910ce222 --- /dev/null +++ b/docs/hygiene-history/ticks/2026/05/13/2339Z.md @@ -0,0 +1,86 @@ +--- +tick: 2026-05-13T23:39Z +agent: otto-cli +session: cron-resumption +pr: 3056 +--- + +# Tick 2339Z — Copilot round-1 on PR #3056 addressed; #3054 + #3055 landed + +## Refresh + +- CronList: live (`9ac08520`, `* * * * *`, sentinel `<>`). +- PRs merged since previous tick: + - #3054 (Otto-Desktop shadow lesson log for B-0449 collision drift) + → `f48c94e` + - #3055 (Otto-Desktop archive of PR discussions 3046-3051) → `21de62f` +- Open PRs: #3056 (mine, audit tool, surfaced 3 review threads), + #3057 (mine, B-0068.1 cleanup, wait-ci). + +## Named-dependency check + +PR #3056 surfaced 3 unresolved Copilot threads at tick start — +addressed this tick (real-dependency-action, not Holding). PR #3057 +remains `wait-ci`. + +## Speculative work picked + +PR #3056 hit `nextAction: resolve-threads` — Copilot ran a review +pass on the audit tool. Three findings, all valid: + +1. **Missing eslint-disable** on `spawnSync("git", ...)`. Every other + tools/**.ts spawnSync invocation in the repo carries the + `// eslint-disable-next-line sonarjs/no-os-command-from-path -- ...` + comment. Mine didn't → added the suffixed-rationale form. +2. **`rowsScanned` misleadingly named** — it counted only rows with + an extractable `id:` field, not total files inspected. The tests + already asserted the smaller count; the CLI's "X rows scanned" + message therefore under-reported. → Renamed to `rowsWithId`; + updated docstring + CLI message: "X rows with id field, no + duplicate IDs". +3. **Dead `idToFiles.size > 0` ternary** — `reduce` on an empty + iterable already returns 0. → Simplified to plain spread+reduce. + +## Landed concretely + +| Artifact | Where | What | +|---|---|---| +| `tools/bg/audit-duplicate-row-ids.ts` | PR #3056 `7444a05` | All 3 Copilot findings fixed | +| `tools/bg/audit-duplicate-row-ids.test.ts` | PR #3056 `7444a05` | 3 assertions migrated from `rowsScanned` → `rowsWithId` | +| 3 review threads | PR #3056 GraphQL | Resolved via `resolveReviewThread` mutation | + +## Verify + +- 14/14 pass on `tools/bg/audit-duplicate-row-ids.test.ts` +- `bun tools/bg/audit-duplicate-row-ids.ts`: 12 duplicate-ID + group(s) found across 560 rows with id field (560 — up by 1 + from earlier 559 because one new row landed on main since) +- PR #3056 unresolved threads: 3 → 0 +- Both PRs back to `wait-ci`, auto-merge armed + +## Reviewer-thread metabolism + +Copilot's first finding was a *consistency* call (matches existing +sibling tools). Second was a *semantic* call (the field name +implied a wrong promise). Third was a *minimalism* call (dead code). +Three different concern classes in one review pass — the same +"better fix the second time" pattern as PR #3043's Codex round-2 +re-review. Round-1 ships fast; round-2 ships *correctly*. + +## CronList + +- Live (`9ac08520`, `* * * * *`). + +## Visibility signal + +Session arc (now 12 PRs): + +| PR | What | Status | +|---|---|---| +| #3041..#3048, #3050, #3051, #3053, #3054, #3055, #3049 | Substrate-hygiene rounds 1-5 + sibling PRs | MERGED (13 total) | +| #3056 | Duplicate-ID audit tool + B-0451 + Copilot round-1 | wait-ci, threads clear | +| #3057 | First B-0451 cleanup (B-0068.1 → B-0068.4) | wait-ci, threads clear | +| #3052 | Competing collision fix | CLOSED with provenance | + +13 PRs merged this session; 2 in flight (both mine, both threads- +clear); 1 closed. Loop continues. diff --git a/docs/hygiene-history/ticks/2026/05/13/2358Z.md b/docs/hygiene-history/ticks/2026/05/13/2358Z.md new file mode 100644 index 0000000000..5922c3474e --- /dev/null +++ b/docs/hygiene-history/ticks/2026/05/13/2358Z.md @@ -0,0 +1,103 @@ +--- +tick: 2026-05-13T23:58Z +agent: otto-cli +session: cron-resumption +pr: 3056 +--- + +# Tick 2358Z — Codex P2 round-2 on PR #3056: surface read errors + +## Refresh + +- CronList: live (`9ac08520`, `* * * * *`, sentinel `<>`). +- No new main merges since previous tick. +- Open PRs: #3056 (audit tool — new Codex thread + transient CI + failures), #3058 (B-0090.x batch, threads clear, wait-ci). + +## CI-failure triage + +PR #3056 showed 9 failing checks at tick start: + +``` +Analyze (csharp/java-kotlin/javascript-typescript) fail +Path gate fail +lint (archive header §33) fail +lint (no conflict markers) fail +lint (no empty dirs) fail +lint (semgrep) fail +lint (tick-history order) fail +``` + +Inspecting the Path-gate log surfaced the root cause: + +``` +##[warning]API rate limit exceeded for installation. +For more on scraping GitHub and how it may affect your rights ... +``` + +All 9 failures are **GitHub API rate-limit exhaust** during the SARIF +upload step — not actual test/lint failures with my code. Transient +infrastructure issue; will clear on the next CI run. + +Substrate-honest disclosure: the harness itself flagged the rate-limit +hit via `` after I queried `gh run view`, recommending +`ScheduleWakeup` over retry polling. Heeded — no further gh API +polling this tick. + +## Speculative work picked + +While the CI failures are not addressable from my side, the Codex P2 +thread that surfaced IS legitimate: + +> `auditRowFiles` silently swallows `readFileSync` failures and +> continues, so the CLI can report success even when tracked backlog +> files are unreadable or missing in the worktree. That creates a +> false-negative path where duplicate IDs in skipped files are never +> checked. + +The original docstring said "Unreadable files are surfaced as a +parse-skip; do not crash" — that was a permissive policy, but Codex +correctly identified it makes the audit unreliable. Better: +**accumulate read errors, surface them, fail the audit**. + +## Landed concretely + +| Artifact | Where | What | +|---|---|---| +| `tools/bg/audit-duplicate-row-ids.ts` | PR #3056 `7fb8d32` | New `ReadError = { file, reason }` type; `AuditResult.readErrors[]` accumulates per-file failures; CLI fails non-zero on any read error | +| `tools/bg/audit-duplicate-row-ids.test.ts` | PR #3056 `7fb8d32` | Renamed skip-test → surface-test with `readErrors[]` shape assertions; added zero-state test | +| 1 review thread | PR #3056 GraphQL | Resolved via `resolveReviewThread` mutation | + +## Verify + +- 15/15 pass on `tools/bg/audit-duplicate-row-ids.test.ts` (was 14) +- PR #3056 unresolved threads: 1 → 0 +- New CI run will replace the rate-limit-failed checks + +## Design observation + +The Codex finding is a "policy bug" — my original docstring said the +behavior was intentional ("surface as parse-skip"), but the intent +was wrong. Reviewer threads catch this class better than tests: tests +verify behavior matches spec; reviewer threads question the spec +itself. Round-2 review keeps finding spec-level improvements that +round-1 didn't surface because round-1 was reading my code against +my own assumptions. + +## CronList + +- Live (`9ac08520`, `* * * * *`). + +## Visibility signal + +Session arc (now 13 PRs landed + 2 in flight): + +| PR | Status | +|---|---| +| 13 substrate-hygiene PRs through #3057 | MERGED | +| #3052 (competing collision fix) | CLOSED with provenance | +| #3056 (audit tool, Codex P2 round-2 fix) | wait-ci, threads clear | +| #3058 (B-0090.x batch, 4 collisions) | wait-ci, threads clear | + +Real-dependency-wait on remaining CI (will re-run after rate-limit +window clears). Loop continues. diff --git a/tools/bg/audit-duplicate-row-ids.test.ts b/tools/bg/audit-duplicate-row-ids.test.ts new file mode 100644 index 0000000000..86fcf12418 --- /dev/null +++ b/tools/bg/audit-duplicate-row-ids.test.ts @@ -0,0 +1,174 @@ +import { describe, expect, test, beforeEach, afterEach } from "bun:test"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { + auditRowFiles, + extractId, + findDuplicates, +} from "./audit-duplicate-row-ids.ts"; + +let TEST_DIR: string; + +beforeEach(() => { + TEST_DIR = mkdtempSync(join(tmpdir(), "zeta-dup-id-audit-test-")); +}); + +afterEach(() => { + rmSync(TEST_DIR, { recursive: true, force: true }); +}); + +function makeRow(filename: string, frontmatter: string, body = "row body"): string { + const path = join(TEST_DIR, filename); + mkdirSync(join(TEST_DIR, "P1"), { recursive: true }); + mkdirSync(join(TEST_DIR, "P2"), { recursive: true }); + writeFileSync(path, `---\n${frontmatter}\n---\n\n${body}\n`); + return path; +} + +describe("extractId", () => { + test("extracts ID from well-formed frontmatter", () => { + const content = `--- +id: B-0100 +priority: P1 +status: open +--- + +# B-0100 — Example`; + expect(extractId(content)).toBe("B-0100"); + }); + + test("handles sub-row IDs like B-0055.1", () => { + const content = `--- +id: B-0055.1 +status: open +---`; + expect(extractId(content)).toBe("B-0055.1"); + }); + + test("returns undefined when frontmatter missing", () => { + expect(extractId("no frontmatter here")).toBeUndefined(); + }); + + test("returns undefined when id field missing", () => { + const content = `--- +priority: P1 +status: open +---`; + expect(extractId(content)).toBeUndefined(); + }); + + test("CRLF line endings handled", () => { + const content = "---\r\nid: B-0200\r\npriority: P2\r\n---\r\n"; + expect(extractId(content)).toBe("B-0200"); + }); +}); + +describe("findDuplicates", () => { + test("returns empty array when no IDs duplicated", () => { + const m = new Map(); + m.set("B-0100", ["a.md"]); + m.set("B-0101", ["b.md"]); + expect(findDuplicates(m)).toEqual([]); + }); + + test("groups duplicates with sorted file lists", () => { + const m = new Map(); + m.set("B-0100", ["zzz.md", "aaa.md"]); + m.set("B-0101", ["unique.md"]); + m.set("B-0200", ["c.md", "b.md", "a.md"]); + const dups = findDuplicates(m); + expect(dups).toHaveLength(2); + expect(dups[0]).toEqual({ id: "B-0100", files: ["aaa.md", "zzz.md"] }); + expect(dups[1]).toEqual({ id: "B-0200", files: ["a.md", "b.md", "c.md"] }); + }); + + test("output sorted by ID for determinism", () => { + const m = new Map(); + m.set("B-0500", ["x.md", "y.md"]); + m.set("B-0100", ["a.md", "b.md"]); + m.set("B-0300", ["m.md", "n.md"]); + const dups = findDuplicates(m); + expect(dups.map((d) => d.id)).toEqual(["B-0100", "B-0300", "B-0500"]); + }); +}); + +describe("auditRowFiles", () => { + test("clean substrate returns no duplicates", () => { + const files = [ + makeRow("P1/B-0100-foo.md", "id: B-0100\npriority: P1\nstatus: open"), + makeRow("P1/B-0101-bar.md", "id: B-0101\npriority: P1\nstatus: open"), + makeRow("P2/B-0200-baz.md", "id: B-0200\npriority: P2\nstatus: open"), + ]; + const result = auditRowFiles(files); + expect(result.duplicates).toEqual([]); + expect(result.rowsWithId).toBe(3); + }); + + test("flags simple duplicate-ID pair", () => { + const files = [ + makeRow("P1/B-0100-first.md", "id: B-0100\npriority: P1\nstatus: open"), + makeRow("P2/B-0100-second.md", "id: B-0100\npriority: P2\nstatus: open"), + ]; + const result = auditRowFiles(files); + expect(result.duplicates).toHaveLength(1); + expect(result.duplicates[0]?.id).toBe("B-0100"); + expect(result.duplicates[0]?.files).toHaveLength(2); + }); + + test("flags triple-collision (B-0409 real-world pattern)", () => { + const files = [ + makeRow("P1/B-0409-a.md", "id: B-0409\npriority: P1\nstatus: open"), + makeRow("P2/B-0409-b.md", "id: B-0409\npriority: P2\nstatus: open"), + makeRow("P2/B-0409-c.md", "id: B-0409\npriority: P2\nstatus: open"), + ]; + const result = auditRowFiles(files); + expect(result.duplicates).toHaveLength(1); + expect(result.duplicates[0]?.files).toHaveLength(3); + }); + + test("rows without id field are skipped, not crashed", () => { + const files = [ + makeRow("P1/B-0100.md", "id: B-0100\npriority: P1\nstatus: open"), + makeRow("P1/no-id.md", "priority: P1\nstatus: open"), + makeRow("P2/B-0100-dup.md", "id: B-0100\npriority: P2\nstatus: open"), + ]; + const result = auditRowFiles(files); + expect(result.duplicates).toHaveLength(1); + expect(result.duplicates[0]?.id).toBe("B-0100"); + expect(result.rowsWithId).toBe(2); + }); + + test("sub-row IDs participate in collision detection", () => { + const files = [ + makeRow("P2/B-0090.1-a.md", "id: B-0090.1\npriority: P2\nstatus: open"), + makeRow("P2/B-0090.1-b.md", "id: B-0090.1\npriority: P2\nstatus: open"), + ]; + const result = auditRowFiles(files); + expect(result.duplicates).toHaveLength(1); + expect(result.duplicates[0]?.id).toBe("B-0090.1"); + }); + + test("unreadable files surface as readErrors (Codex P2: don't silently skip)", () => { + const missing = "/nonexistent/path/that/does/not/exist.md"; + const files = [ + makeRow("P1/B-0100.md", "id: B-0100\npriority: P1\nstatus: open"), + missing, + ]; + const result = auditRowFiles(files); + expect(result.duplicates).toEqual([]); + expect(result.rowsWithId).toBe(1); + expect(result.readErrors).toHaveLength(1); + expect(result.readErrors[0]?.file).toBe(missing); + expect(result.readErrors[0]?.reason.length).toBeGreaterThan(0); + }); + + test("readErrors is empty when all files readable", () => { + const files = [ + makeRow("P1/B-0100.md", "id: B-0100\npriority: P1\nstatus: open"), + makeRow("P1/B-0101.md", "id: B-0101\npriority: P1\nstatus: open"), + ]; + const result = auditRowFiles(files); + expect(result.readErrors).toEqual([]); + }); +}); diff --git a/tools/bg/audit-duplicate-row-ids.ts b/tools/bg/audit-duplicate-row-ids.ts new file mode 100644 index 0000000000..f094c00782 --- /dev/null +++ b/tools/bg/audit-duplicate-row-ids.ts @@ -0,0 +1,172 @@ +#!/usr/bin/env bun +// audit-duplicate-row-ids.ts — flag backlog rows that share an `id:` value. +// +// The backlog uses `id: B-NNNN` (and sub-row IDs like `B-NNNN.M`) as the +// global primary key. Two rows with the same ID corrupt the substrate +// silently: consumers of the ID (depends_on / composes_with / children / +// merged-PR references) pick whichever file the loader sees first; the +// second-loaded row overwrites state in any in-memory index keyed by ID. +// +// This audit walks `docs/backlog/**/B-*.md`, extracts each frontmatter +// `id:` field, and reports any ID that appears in more than one file. +// +// Exit codes: +// 0 — no duplicate IDs found +// 1 — duplicates present (or I/O / parse error) +// +// Background: +// This tool exists because PR #3038 created a row with `id: B-0444` +// that collided with the row PR #3033 had filed 25 min earlier. The +// collision survived 9 PR landings before being noticed in an ad-hoc +// audit (PR #3053 resolved it). A broader sweep found 12 additional +// collision groups. See B-0451 for the row that tracks this tool + +// the substrate-wide cleanup. + +import { readFileSync } from "node:fs"; +import { join, relative, resolve } from "node:path"; +import { spawnSync } from "node:child_process"; + +export type DuplicateGroup = { + id: string; + files: string[]; +}; + +export type ReadError = { + file: string; + reason: string; +}; + +export type AuditResult = { + /** + * Number of input files that contained an extractable `id:` field. + * Distinct from total files inspected — rows without `id:` (e.g., a + * README that got picked up by the file glob) are not counted. + */ + rowsWithId: number; + duplicates: DuplicateGroup[]; + /** + * Files the audit could not read (permission, missing-after-listing, + * IO error). Surfaced so the CLI can fail loudly per Codex P2 on + * PR #3056: silently skipping unreadable files would let collisions + * hide in any row the audit couldn't open. + */ + readErrors: ReadError[]; +}; + +/** + * Extract the frontmatter `id:` value from a backlog-row markdown file. + * + * Backlog rows open with a YAML frontmatter block delimited by `---` + * fences. The `id:` field appears as a top-level key inside the first + * such block. Returns `undefined` if no frontmatter or no `id:` field + * is present (rows without `id:` cannot collide and are skipped by + * the caller). + */ +export function extractId(content: string): string | undefined { + const fmMatch = content.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!fmMatch || fmMatch[1] === undefined) return undefined; + const idMatch = fmMatch[1].match(/^id:\s*(\S+)/m); + return idMatch?.[1]; +} + +/** + * Group a `Map` into the `DuplicateGroup[]` shape, keeping + * only IDs that appear in more than one file. Files are sorted within + * each group so output is deterministic. + */ +export function findDuplicates(idToFiles: Map): DuplicateGroup[] { + const dups: DuplicateGroup[] = []; + for (const [id, files] of idToFiles.entries()) { + if (files.length > 1) { + dups.push({ id, files: [...files].sort() }); + } + } + dups.sort((a, b) => a.id.localeCompare(b.id)); + return dups; +} + +/** + * Audit a list of backlog-row file paths and report any duplicate IDs. + * The caller is responsible for path collection (so tests can inject + * a fixture set and the CLI can read from `docs/backlog/`). + */ +export function auditRowFiles(files: string[]): AuditResult { + const idToFiles = new Map(); + const readErrors: ReadError[] = []; + for (const f of files) { + let content: string; + try { + content = readFileSync(f, "utf-8"); + } catch (err) { + // Codex P2 on PR #3056: silently swallowing read errors lets + // collisions hide in any row the audit couldn't open. Accumulate + // errors so the CLI can fail loudly — the caller's exit code + // logic checks `readErrors.length > 0` alongside `duplicates`. + readErrors.push({ file: f, reason: (err as Error).message }); + continue; + } + const id = extractId(content); + if (!id) continue; + const list = idToFiles.get(id) ?? []; + list.push(f); + idToFiles.set(id, list); + } + return { + rowsWithId: [...idToFiles.values()].reduce((n, fs) => n + fs.length, 0), + duplicates: findDuplicates(idToFiles), + readErrors, + }; +} + +/** + * Default CLI entrypoint: walk `docs/backlog/**` via `git ls-files` + * (no submodule / .gitignored traversal needed) and report findings. + */ +function main(): number { + const repoRoot = resolve(import.meta.dir, "..", ".."); + // eslint-disable-next-line sonarjs/no-os-command-from-path -- git invoked as explicit args array; no shell, no user input on the command line. + const lsFiles = spawnSync( + "git", + ["-C", repoRoot, "ls-files", "docs/backlog/"], + { encoding: "utf-8" }, + ); + if (lsFiles.status !== 0) { + console.error(`git ls-files failed: ${lsFiles.stderr ?? ""}`); + return 1; + } + const files = (lsFiles.stdout ?? "") + .split("\n") + .filter((line) => line.endsWith(".md") && !line.endsWith("/README.md")) + .map((line) => join(repoRoot, line)); + + const result = auditRowFiles(files); + + // Read errors surface alongside duplicates: both fail the audit so a + // hidden duplicate inside an unreadable file can't slip through. + if (result.readErrors.length > 0) { + console.error(`audit-duplicate-row-ids: ${result.readErrors.length} file(s) could not be read:`); + for (const e of result.readErrors) { + console.error(` - ${relative(repoRoot, e.file)}: ${e.reason}`); + } + } + + if (result.duplicates.length === 0 && result.readErrors.length === 0) { + console.log(`audit-duplicate-row-ids: ${result.rowsWithId} rows with id field, no duplicate IDs`); + return 0; + } + + if (result.duplicates.length > 0) { + console.error(`audit-duplicate-row-ids: ${result.duplicates.length} duplicate-ID group(s) found across ${result.rowsWithId} rows with id field:`); + for (const dup of result.duplicates) { + console.error(` ${dup.id}:`); + for (const f of dup.files) { + console.error(` - ${relative(repoRoot, f)}`); + } + } + } + return 1; +} + +if (import.meta.main) { + process.exit(main()); +}