feat(gates): make an outstanding-issues id collision a red gate by BigSimmo · Pull Request #1410 · BigSimmo/Database

BigSimmo · 2026-07-30T04:54:12Z

Summary

Fixes ledger #112. The issues:next-id marker is a plain HTML comment that every editor read-modify-writes with no lock, and docs/outstanding-issues.md — unlike docs/branch-review-ledger.md — has no merge=union driver. So two agents allocating in the same hour collide, and the collision surfaces as an ordinary content conflict that a hurried resolution can settle by taking one side wholesale and dropping the other's rows.

On 2026-07-29 that happened three times in one hour on a single PR (#1391 lost #096/#097, then #098/#099, then collided again on #108/#109). Nothing noticed, because no gate read this file's structure at all.

npm run check:outstanding-issues now does — in verify:cheap and in the static-pr CI job. It fails on:

a duplicate id, keyed by number — a merge that kept both sides' rows under one number
a non-canonical id (#1, #0001, #OO1, a dropped #) — two spellings of one allocation
an id in both tables — an archive move that copied instead of moving
a marker at or below the highest id — a merge that kept a row and lost the bump
more than one marker — a conflict resolution that kept both sides
a row whose cell count differs from its own block's declared width
a missing separator row — a section that declares no width and renders as no table
a row that lost its leading pipe or gained indentation — out of the table while still sitting in it
a row outside any table — a blank line that silently ended the table above it

Scope is deliberately structural. It says nothing about whether a row's content is right, because that is a judgement a gate cannot make, and pretending otherwise would make it noisy enough to ignore.

The live defect this found

Running the checker against the real file rather than its fixtures exposed a rendering defect that has been on main and that no earlier version of the gate was structurally able to report.

The archive section carried blank lines part way down its rows. GFM ends a table at the first blank line, so 56 of the 60 archived rows were rendering as a paragraph of literal pipe characters, not as table rows. The pre-fix checker passed that file with zero problems while counting all 114 rows — it counted them and never noticed they were outside a table:

PRIOR verdict: 0 problem(s) -> would have PASSED CI
PRIOR counted rows: 114

NOW:
  - line 166 (archive section) starts 7 pipe row(s) that are outside any table
  - line 174 (archive section) starts 49 pipe row(s) that are outside any table

The two stray blank lines are removed here. Cell content is unchanged — the row set sorts byte-identical before and after — and the remaining churn in docs/outstanding-issues.md is Prettier realigning what is now a single 60-row table. The guard reports 115 rows (55 open, 60 archived) where the pre-fix reader saw 58.

How the row detector was rebuilt, and why

Three review rounds each found another row shape the detector silently dropped. Each time the cause was the same: the detector asked "does this line look like a row?", so anything that did not look like one was invisible to every check below — including the checks meant to report it. Patching the predicate a fourth time would only have moved the blind spot.

So detection is now positional. A table body is everything from its separator to the next heading or blank line — exactly what Markdown treats as one table — and every line in that span must be a well-formed row. A line that is not one is a reported problem rather than a skipped line. Detection no longer depends on the row being parseable, which is what kept regenerating the bug.

Two further defects fell out of that work:

The body scan ended at startsWith("#"). A row that loses its leading pipe begins #001 | … — every id row does — so the scan mistook the damaged row for a heading, ended the body one line early, and hid precisely the row it was meant to catch. Now /^#{1,6}(\s|$)/, since an ATX heading requires whitespace after its hashes.
Column width was read once per section. The archive is more than one block, so a per-section expectation is wrong by construction; each block is now checked against its own separator.

Verification

Replayed the actual collision against the real file, not just fixtures:

$ sed -i '0,/^| #111 /s//| #110 /' docs/outstanding-issues.md && node scripts/check-outstanding-issues.mjs
  - #110 appears 2 times (lines 151, 164) — ids are never reused; a collision usually
    means a merge kept both sides under one number
  - #110 is in BOTH the open and archive tables
exit=1

Every new self-test case was run against the previous logic to confirm it was actually green before, rather than assuming the test was meaningful:

PROVEN RED (was green)   short id colliding with padded     prior=0 now=3
PROVEN RED (was green)   over-padded id                     prior=0 now=1
PROVEN RED (was green)   deleted separator row              prior=0 now=2
PROVEN RED (was green)   row that lost its leading pipe     prior=0 now=1
PROVEN RED (was green)   indented row                       prior=0 now=1

--self-test covers 16 failure shapes plus a well-formed file and an escaped pipe, mirroring the check-branch-review-ledger convention
npm run verify:cheap — exit 0, Test Files 432 passed (432), Tests 4470 passed | 4 skipped (4474)
npm run check:gate-manifest — all verify:cheap gates enforced in CI
npm run format:check clean; ESLint clean
Merge with main resolved and verified row-by-row: same 115-id set as main, none lost, none added, no duplicates
No provider-backed gate run; none applies.

Three things the first draft got wrong, all caught by running it rather than reading it — worth recording, because each would have shipped an inert or actively annoying gate:

Table width was inferred from the modal row width, which cannot flag the anomaly when a table holds one row. It now comes from the separator row, which is where a table actually declares its shape.
The separator regex /^\|[\s:-]+\|$/ excluded the inner pipes, so it only ever matched a two-column table. Every real table therefore had no declared width and that check was silently inert — the self-test caught it.
cells() split on every pipe, so the checker's first run against the live file reported row #042 as malformed. That row is correctly escaped (absent \| valid \| invalid) — the checker was wrong, not the file. It now splits on unescaped pipes only. A gate with false positives is a gate people switch off.

Risk and rollout

Risk: low. Adds a read-only checker plus one verify:cheap and one CI step; no product code. The one content change is the removal of two blank lines from a docs file, with the row set proven identical. The realistic failure mode is a false positive blocking an unrelated PR, which is why (3) above mattered and why the scope stays structural.
Rollback: revert the commit; the gate disappears and the file returns to its previous (mis-rendering) state.
Provider or production effects: none.

Clinical Governance Preflight

No ingestion, answer generation, search/ranking, document access, privacy or clinical output path is touched.

Source-backed claims still require linked source verification before clinical use
No patient-identifiable document workflow was introduced or expanded
Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
Service-role keys and private document access remain server-only
Demo/synthetic content remains clearly separated from real clinical sources
Source metadata, review status, and outdated/unknown-source behavior remain conservative
Deployment classification/TGA SaMD impact checked — no clinical decision-support behaviour changed

RAG impact: no retrieval behaviour change — a docs checker, one npm script, one CI step, and one docs file's whitespace; no retrieval, ranking, selection, or answer-generation code is touched.

Notes

#112 is archived rather than left open-and-done, and its outcome states the limit plainly: the underlying race is not fixed. Ids are still allocated by read-modify-write with no lock, and the file still has no union merge driver. What changed is that a collision can no longer land silently. A real fix would be merge=union plus allocation that does not depend on reading a counter — worth doing, but a different change, and row-level union alone would not prevent two rows sharing a number.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P

Summary by CodeRabbit

Bug Fixes
- Added automated validation for the outstanding-issues ledger to detect duplicate, malformed, or incorrectly numbered issue IDs.
- CI and verification checks now fail when ledger structure or integrity problems are found.
- Improved detection of malformed table rows and misplaced content.
Documentation
- Updated the issue ledger to archive a resolved integrity concern and document the new validation coverage.

Ledger #112. The `issues:next-id` marker is a plain HTML comment that every editor read-modify-writes with no lock, and this file — unlike the branch review ledger — has NO `merge=union` driver. So two agents allocating in the same hour collide, and the collision surfaces as an ordinary content conflict that a hurried resolution can settle by taking one side wholesale and dropping the other's rows. On 2026-07-29 that happened three times in one hour on a single PR, and nothing noticed, because no gate read this file's structure at all. `npm run check:outstanding-issues` now does, in `verify:cheap` and in the `static-pr` CI job — the gate-manifest check refuses a local gate CI does not run, which is how I learned to add the second one. It fails on: - a duplicate id (a merge that kept both sides under one number) - an id in BOTH tables (an archive move that copied instead of moving) - a marker at or below the highest id (a merge that lost the bump) - a row whose cell count differs from its table's declared width - a missing heading or marker Verified by replaying the actual collision against the real file rather than only against fixtures: two rows claiming `#110` produce "#110 appears 2 times (lines 151, 164)", and a lost marker bump produces "issues:next-id=113 is not above the highest id #114". Two things the first draft got wrong, both caught by running it rather than reading it: Table width was inferred from the modal row width, which cannot flag the anomaly when a table holds one row. It now comes from the separator row, which is where a table actually declares its shape. The separator regex then had to include the inner pipes — without them it only ever matched a two-column table, so every real table silently had no declared width and the check was inert. And `cells()` split on every pipe, so its first run against the live file reported row #42 as malformed. That row is correctly escaped (`absent \| valid \| invalid`); the checker was wrong. It now splits on unescaped pipes only. A gate with false positives is a gate people switch off. Scope is deliberately structural. It says nothing about whether a row's content is right, because that is a judgement a gate cannot make, and pretending otherwise would make it noisy enough to ignore. #112 is archived rather than left open-and-done, with the limit stated: the underlying race is NOT fixed. Ids are still allocated by read-modify-write with no lock and the file still has no union driver. What changed is that a collision can no longer land silently. verify:cheap exit 0 — 28 gates, Tests 4450 passed | 4 skipped. format:check clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P

supabase · 2026-07-30T04:54:17Z

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

coderabbitai · 2026-07-30T04:54:18Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 91f38f9c-9435-4795-961c-604b24e92f4e

📥 Commits

Reviewing files that changed from the base of the PR and between b15e267 and 8aa01d5.

📒 Files selected for processing (4)

.github/workflows/ci.yml
docs/outstanding-issues.md
package.json
scripts/check-outstanding-issues.mjs

📝 Walkthrough

Walkthrough

A new check-outstanding-issues.mjs validator checks the issue ledger’s structure, IDs, tables, and marker. It runs in cheap verification and the static-pr CI job, while issue #112 is moved to the resolved archive.

Changes

Outstanding issues integrity

Layer / File(s)	Summary
Ledger parser and validation `scripts/check-outstanding-issues.mjs`, `docs/outstanding-issues.md`	Adds markdown table parsing, structural checks, canonical ID validation, duplicate detection, marker validation, self-tests, and CLI error reporting; archives issue `#112` with the gate’s scope documented.
Verification and CI wiring `package.json`, `.github/workflows/ci.yml`	Runs the new check during `verify:cheap:internal` and the `static-pr` workflow job.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant verifyCheap
  participant StaticPR
  participant checkOutstandingIssues
  participant OutstandingIssuesLedger
  verifyCheap->>checkOutstandingIssues: run self-test and ledger check
  StaticPR->>checkOutstandingIssues: run ledger integrity check
  checkOutstandingIssues->>OutstandingIssuesLedger: read and validate markdown
  OutstandingIssuesLedger-->>checkOutstandingIssues: ledger contents
  checkOutstandingIssues-->>verifyCheap: pass or fail
  checkOutstandingIssues-->>StaticPR: pass or fail

Possibly related PRs

BigSimmo/Database#1002: Adds a related gate-manifest check for verification and CI wiring.
BigSimmo/Database#1043: Updates the ledger consumed by this validator.
BigSimmo/Database#1066: Updates issue rows and the marker now checked by this validator.

Suggested reviewers: claude, cursoragent, copilot

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: turning outstanding-issues ID collisions into a failing gate.
Description check	✅ Passed	The description matches the template well, with summary, verification, risk, governance, and notes sections filled out.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d4445166b2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Both review findings are right, and both are the same failure this gate keeps producing in draft: a check that looks like it covers a case and does not. A row whose id cell is `001`, `#OO1` or empty did not match `ROW`, so it was dropped before every structural check — duplicate detection, the marker comparison, the width check — and the file reported green while carrying exactly the malformed row the gate advertises. Rows are now recognised by being a table body row at all (not a header, not a separator) and the id shape is validated rather than assumed, so an unparseable id is a reported problem instead of an invisible one. `markdown.match(MARKER)` returned only the first marker, so a conflict that kept two left a stale allocation the checker never saw. With markers 116 and 115 and a highest id of #115, nothing failed — and a later editor following the stale 115 reuses an id, which is the exact outcome this gate exists to prevent. Exactly one marker is now required. Four adversarial self-tests, all verified red against the previous logic: self-test FAILED: a dropped # on an id — expected 1 problem(s), got 0 self-test FAILED: a letter O for a zero — expected 1 problem(s), got 0 self-test FAILED: an empty id cell — expected 1 problem(s), got 0 self-test FAILED: a second next-id marker kept by a conflict — got 0 verify:cheap exit 0 — Tests 4450 passed | 4 skipped. format:check clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 412afc59e4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Three review rounds each found another row shape the detector silently dropped, because it asked "does this line look like a row?" and anything that did not was invisible to every check below. Patching the predicate a fourth time would only move the blind spot, so this inverts it: a table body is defined positionally, from its separator to the next heading or blank line, and every line in that span must be a well-formed row. Running that against the real file rather than its fixtures exposed the defect the previous logic was structurally unable to report. The archive section carries blank lines part way down its rows. GFM ends a table at the first blank line, so 56 of the 60 archived rows have been rendering as a paragraph of literal pipe characters, not as table rows. The prior checker passed that file with zero problems while counting all 114 rows. Rows outside every table are now their own reported failure, and the two stray blank lines are removed. Cell content is byte-identical; the row set sorts equal before and after, and the remaining churn is Prettier realigning what is now a single 60-row table. Also fixed while proving the new cases red: - ids are compared against a canonical zero-padded form, so `#1` and `#1` can no longer both exist as one allocation split across a merge - a deleted separator is reported rather than silently disabling the width check for its whole section - column counts are checked per block, against the width that block's own separator declares - body scanning uses an ATX heading test rather than startsWith("#"), because a row that loses its leading pipe begins `#1` — every id row does, so the old test ended the body early and hid the damaged row Six new self-test cases; five were green under the previous logic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P

…collision # Conflicts: # docs/outstanding-issues.md

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

The #113 archive row cited `c2edda18` as PR #1405's merge commit. That was the branch tip, which the squash merge discarded, so the claim was already unverifiable when it was written: git merge-base --is-ancestor c2edda1 origin/main -> fails git merge-base --is-ancestor 020c126 origin/main -> ok 020c126 fix(mode-nav): size slots to their content ... (#1405) In a ledger whose whole purpose is durable, auditable resolution history, a resolution nobody can check is worse than no resolution note. The trap is that this repo mixes merge strategies: #1407 and #1410 landed as merge commits, so their branch SHAs stay reachable; #1405 was squashed and its did not. Copying the PR head works three times in four, which is exactly the kind of rule that survives review until it doesn't. Verified after the edit: `c2edda18` no longer appears anywhere in the file, the replacement is an ancestor of main, and `check:outstanding-issues` still passes with `117 rows (55 open, 62 archived), unique ids, next-id=118 above the highest`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P

* process: cut future PR conflict churn and silent CI gaps Add outstanding-issues ID/marker/union guards (#112), a read-only pull_request_target mergeability signal for dirty heads (#116), and an anti-conflict CI-speed operating procedure that prefers bundling and format-before-push without weakening required gates or touching active PRs. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record PR #1416 merge-readiness as not ready Main advanced with #1410 overlapping the outstanding-issues gate; merge-tree is conflicting. Unique value remains the anti-conflict playbook, #116 signal, and merge=union. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * style(issues): format outstanding-issues after main sync Prettier realigns the archive table after the #116 close and #112 note update. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): supersede #1416 merge-readiness as ready Record the post-sync review: merge-tree clean, unique #116/process value kept, duplicate #112 checker dropped in favor of #1410. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): pin #1416 READY review to final tip SHA Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(test): shrink zip-bomb fixture to stop CI coverage timeout Unit coverage failed because reject-high-compression-ratio allocated and deflated 24MB of zeros under the 30s Vitest timeout. 1MB still exceeds the 150:1 admission ratio (~480:1) without the CI flake. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix: refresh mergeability after base advances * fix: preserve ledger rotations during merges * fix: resolve issue ledger merge collision --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

BigSimmo marked this pull request as ready for review July 30, 2026 04:55

chatgpt-codex-connector Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread scripts/check-outstanding-issues.mjs Outdated

Comment thread scripts/check-outstanding-issues.mjs Outdated

BigSimmo enabled auto-merge July 30, 2026 05:04

chatgpt-codex-connector Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread scripts/check-outstanding-issues.mjs

Comment thread scripts/check-outstanding-issues.mjs Outdated

Comment thread scripts/check-outstanding-issues.mjs Outdated

claude and others added 4 commits July 30, 2026 05:22

Merge remote-tracking branch 'origin/main' into claude/fix-issues-id-…

56dd341

…collision # Conflicts: # docs/outstanding-issues.md

merge(main): sync PR #1410 with latest main

8aa01d5

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

merge(main): sync PR #1410 with latest main

c2a65e7

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

BigSimmo merged commit f5a76c5 into main Jul 30, 2026
19 checks passed

BigSimmo deleted the claude/fix-issues-id-collision branch July 30, 2026 05:52

cursor Bot mentioned this pull request Jul 30, 2026

process: anti-conflict operating procedure and silent-CI signal #1416

Merged

8 tasks

BigSimmo mentioned this pull request Jul 30, 2026

docs(issues): archive #113 and #114, open #117 for the stale spec reference #1420

Merged

10 tasks

coderabbitai Bot mentioned this pull request Jul 30, 2026

docs(issues): capture CI/merge findings and a merged-PR gate gap from PR babysitting #1421

Merged

2 tasks

coderabbitai Bot mentioned this pull request Aug 7, 2026

fix: Grok-safe quick wins (tokens, a11y, perf, gates) #1651

Merged

4 tasks

coderabbitai Bot mentioned this pull request Aug 20, 2026

feat(hardening): safe tooling, ui layout, and workflow guards bundle #2180

Merged

12 tasks

Navigation Menu

feat(gates): make an outstanding-issues id collision a red gate - #1410

feat(gates): make an outstanding-issues id collision a red gate#1410
BigSimmo merged 6 commits into
mainfrom
claude/fix-issues-id-collision

BigSimmo commented Jul 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

supabase Bot commented Jul 30, 2026

Uh oh!

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

BigSimmo commented Jul 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The live defect this found

How the row detector was rebuilt, and why

Verification

Risk and rollout

Clinical Governance Preflight

Notes

Summary by CodeRabbit

Uh oh!

supabase Bot commented Jul 30, 2026

Uh oh!

coderabbitai Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

BigSimmo commented Jul 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading