Skip to content

ci(adr): make a duplicate ADR number go red - #1481

Merged
lilyshen0722 merged 2 commits into
mainfrom
guard/adr-numbering
Sep 2, 2026
Merged

lilyshen0722 merged 2 commits into
mainfrom
guard/adr-numbering

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Wires the ADR numbering guard Sam ruled on at 04:30Z ("wire it before #1295").

Why

An ADR number is a name two documents can claim at once, and every mechanical check we own is blind to it. #1295 and #1268 both added docs/adr/ADR-025-*.md under different slugs — different filenames, so no textual conflict, merge-tree clean, every check green — and main carried two ADR-025s from the moment the second merged.

It had already happened at ADR-018 (agent-attention-claims / agent-identity), and there it cost something: the duplicate lived long enough that #963's author followed the wrong one and shipped a wake-policy regression. Both collisions were found by a human reading a directory listing.

What it checks

scripts/verify-adr-numbering.js, over a directory of ADRs:

  1. every file is named ADR-NNN-slug.md
  2. no two files claim the same NNN
  3. the H1's number matches the filename's number — a renamed file whose title still carries the old number is cited by both numbers and found by neither

Not contiguity. main has no 029 and that is fine; requiring density would make every renumber a cascade.

The workflow runs it twice, because the collision has two distinct lifetimes:

Two design choices worth the review time

Not the merge ref. actions/checkout gives you refs/pull/N/merge by default, and that ref is recomputed lazily: #1295's still contained the duplicate ADR-018 an hour after #1463 renumbered it away on main. A guard reading that tree fails the PR for a collision somebody else already fixed — worse than not running, because it teaches authors the check is noise. So it checks out the PR head, fetches main fresh, and assembles the post-merge state itself.

Both gh calls fail closed. An unchecked API error yields an empty ADR list, which reads as "this PR claims nothing" and passes. That is the guard at its most reassuring exactly when it has gone blind — the same shape as the || true removed from package-version-guard.yml.

Verification

main today pass — 29 ADRs, 29 numbers
duplicate number added (the real ADR-025-user-scoped-connectors filename) red, names both files
file renumbered, H1 left stale (the #1463 class) red
ADR-31-... two-digit filename red
assembled post-merge tree for #1295's head 684d9ce7 red — exactly one error, ADR-025, and no false ADR-018 error, which is the merge-ref artifact this design avoids
open-PR clash, older PR claims the same number red, names the older PR
open-PR clash, no overlap pass
either gh call failing red, does not pass blind

Known limit

Like every check in this repo it only runs on a PR event, so it cannot see main moving underneath a PR nobody pushes to again. That gap does not close in this file — it closes with strict: true on the branch protection, which is a separate open ask.

🤖 Generated with Claude Code

An ADR number is a name two documents can claim at once, and every
mechanical check we own is blind to it. #1295 and #1268 both added
docs/adr/ADR-025-*.md under different slugs: different filenames, so no
textual conflict, merge-tree clean, all checks green — and main carried
two ADR-025s the moment the second merged. The same thing had already
happened at ADR-018, where the duplicate survived long enough that
#963's author followed the wrong one and shipped a wake-policy
regression.

Both were found by a human reading a directory listing.

Two checks, because the collision has two distinct lifetimes:

  1. Against main-as-it-is-right-now, unioned with this PR's own ADR
     adds/edits/deletes. Catches a PR claiming a number main already
     uses. Verified against #1295's head: exactly one error, ADR-025.

  2. Against the other OPEN PRs, which check 1 cannot see because
     neither tree contains the other's file — the state #1295 and #1268
     were in for days. Older PR keeps the number, newer renumbers, so
     it is always unilaterally fixable rather than a mutual deadlock.

Deliberately not the merge ref. refs/pull/1295/merge still contained the
duplicate ADR-018 an hour after #1463 renumbered it away, so a guard
reading that tree fails a PR for a collision somebody else already
fixed — worse than not running, because it teaches authors the check is
noise.

Deliberately not contiguity: main has no 029 and that is fine.

Both gh calls fail closed. An unchecked API error would produce an empty
ADR list, which reads as "this PR claims nothing" and passes — the guard
at its most reassuring exactly when blind.

Known limit: like every check here, this only runs on a PR event, so it
cannot see main moving underneath a PR that is not pushed to again. That
gap closes with strict: true on the branch protection, not in this file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@samxu01
samxu01 force-pushed the guard/adr-numbering branch from 0b7a7b1 to 1d723fd Compare September 2, 2026 04:38
The PR arms are triggered by PR events, so a green freezes at the last
one. Two PRs that each passed when last run — one against a main holding
neither number, one before the other pushed its ADR file — can still
merge into a duplicate, and no PR-triggered check can see it happen.

This arm cannot prevent that either. It makes main say so within a
minute, instead of waiting for someone to read a directory listing,
which is how both known duplicates were actually found.

Reds main, deliberately: a duplicated number silently mis-routes every
citation of it, and #963 shipped a wake-policy regression because an
author followed the wrong member of the ADR-018 pair.

Also moves the concurrency group off the PR number, which is empty on a
push event and would put every main build in one group cancelling its
predecessor.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
samxu01 pushed a commit that referenced this pull request Sep 2, 2026
Sam's ruling of 2026-08-30T01:44:52Z (pod message 60455): rebase and
reconcile #1295 and the merged ADR-025 into ONE file. This is the fold.

- The nine channel-routing decisions land in ADR-025-connector-substrate.md
  as D8–D16 under a titled section with a provenance note, their own scope
  boundary (ADR-017/018 own the attention gate; ADR-027 is the structured
  sibling), context, consequences, and alternatives — text verbatim from
  #1295 at 684d9ce, only the numbers moved (sprint-review's D8+ rule, so
  "ADR-025 D<n>" resolves to exactly one decision). D1→D8 replaces D7 for
  the private-chat case per pod-architect's half (#1473).
- One status line covers both halves; D12/D13 stay named as guesses.
- Consequences gain the three schema costs the 2026-08-30 review measured
  (podId required:true, no Integration.scope, findLiveIntegration inverts
  to a fan-out), the strict-schema trap from #1282, and the counterpart↔
  caller gap that #1297's follow-up closes. D15 records #1297 as the
  implementation of the 128-bit code + outbound chatType gate.
- The separate ADR-025-user-scoped-connectors-and-channel-routing.md is
  removed; #1481's guard passes on the result (29 ADRs, 29 numbers).
- ADR-027 cited the folded decisions by their old numbers (D2, D3); moved
  to D9 and D10. Its D6 citation is the substrate's and is unchanged.
- D7's note adopts #1478's corrected ruling citation, so #1478 is
  superseded by this.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@lilyshen0722
lilyshen0722 merged commit 8bb63d2 into main Sep 2, 2026
13 checks passed
@lilyshen0722
lilyshen0722 deleted the guard/adr-numbering branch September 2, 2026 12:26
lilyshen0722 added a commit that referenced this pull request Sep 2, 2026
…#1508)

The PR arm checks out the PR HEAD by design (the merge ref is stale),
but GitHub takes the WORKFLOW from the merge ref. So a PR branched
before #1481 gets the workflow file and not scripts/verify-adr-numbering.js,
and the step dies with MODULE_NOT_FOUND before printing a single ADR line —
red, with nothing an author can act on. Hit #1504 first; it would have hit
most open PRs on their next event.

Resolve the checker the same way the ADRs are resolved: the PR's version if
it changed the script, main's otherwise.

Verified against #1504's head (script absent): green, 29 ADRs. Same head
with a synthetic second ADR-025 added: red, naming both files. Against main
(script present): resolves from HEAD, green.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
lilyshen0722 added a commit that referenced this pull request Sep 2, 2026
…doc — one ADR-025, D8–D16 (#1295)

* docs(adr): ADR-025 — user-scoped connectors and channel routing

The private-only gate (#1289) times the one-chat-one-pod claim caps a
user at one bridged pod ever; rebind the chat to the user and make pod
routing an addressing property (tags, quote-reply, slash commands,
judge for ambiguity only).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK

* docs(adr): ADR-025 amendments — routing precedence, D8 corrected, D9 Commander persona

Sam's decisions 2026-08-26: connector reveals/selects target pods via
slash commands; Commander persona (distinct from Scout) as conversational
routing front-end with profile-level auto-join opt-in. Review findings
folded: D8 inbound-only qualification, 128-bit codes, precedence chain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK

* docs(adr-025): fold #1295 into the substrate doc — one ADR-025, D8–D16

Sam's ruling of 2026-08-30T01:44:52Z (pod message 60455): rebase and
reconcile #1295 and the merged ADR-025 into ONE file. This is the fold.

- The nine channel-routing decisions land in ADR-025-connector-substrate.md
  as D8–D16 under a titled section with a provenance note, their own scope
  boundary (ADR-017/018 own the attention gate; ADR-027 is the structured
  sibling), context, consequences, and alternatives — text verbatim from
  #1295 at 684d9ce, only the numbers moved (sprint-review's D8+ rule, so
  "ADR-025 D<n>" resolves to exactly one decision). D1→D8 replaces D7 for
  the private-chat case per pod-architect's half (#1473).
- One status line covers both halves; D12/D13 stay named as guesses.
- Consequences gain the three schema costs the 2026-08-30 review measured
  (podId required:true, no Integration.scope, findLiveIntegration inverts
  to a fan-out), the strict-schema trap from #1282, and the counterpart↔
  caller gap that #1297's follow-up closes. D15 records #1297 as the
  implementation of the 128-bit code + outbound chatType gate.
- The separate ADR-025-user-scoped-connectors-and-channel-routing.md is
  removed; #1481's guard passes on the result (29 ADRs, 29 numbers).
- ADR-027 cited the folded decisions by their old numbers (D2, D3); moved
  to D9 and D10. Its D6 citation is the substrate's and is unchanged.
- D7's note adopts #1478's corrected ruling citation, so #1478 is
  superseded by this.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant