Skip to content

ci: at most one open PR may modify a declared single-writer path - #9598

Merged
os-steve merged 2 commits into
mainfrom
claude/issue-9402-diff-keyed-duplicate-guard
Aug 18, 2026
Merged

ci: at most one open PR may modify a declared single-writer path#9598
os-steve merged 2 commits into
mainfrom
claude/issue-9402-diff-keyed-duplicate-guard

Conversation

@claude

@claudeclaudeBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Fixes#9402

Two PRs reached green independently while shipping one physical change: both rewrote the objectui pin from the same old sha to the same new sha, and both added the same generated changeset — a byte-identical hunk under two card numbers. The first entered the merge queue; the second was green and one flip from enqueueing behind it, where it would have conflicted on both files.

The card-keyed Duplicate Fix Guard is not broken, and this PR does not touch, weaken or widen it. It asks whether two open PRs claim the same card, and it answered correctly: those two claimed two genuinely different, independently filed cards, neither a duplicate of the other as written. The duplication existed only in the diff. This adds the second question — do two open PRs write the same at-most-one-writer path — and the two gates run side by side.

Implements the adjudicated option 2 on the card: a small declared list of at-most-one-writer paths, enforced in CI alongside the existing gate. Option 1 (general diff-keyed detection) is explicitly not implemented, and the measurement below is why.

What ships

filerole
scripts/check-single-claim-paths.mjsthe rule, the declared list, the verdict layer, and a 54-case --self-test
.github/workflows/single-claim-path-guard.ymlthe wiring — the PR-scoped blocking check
.github/workflows/lint.ymlruns the self-test in the farm
package.jsoncheck:single-claim-paths

Separate check, not a second question inside the existing one — its name says "claim the same issue", and this asks something else. That also matches the house shape: the Part-of Closing-Keyword Guard is the repo's other single-script PR-scoped blocking check, and this copies its job shape, runtime pin and trigger set.

The key, and the measurement that chose it

The general form — flag two open PRs whose changed-path sets intersect at all — is unusable, and that is measured rather than assumed. Over the 300 most recent PRs (#8936#9584, complete file sets, no pagination truncation), pairs whose open windows overlapped and which shared a changed path:

keyconcurrent pairs
any shared changed path (repo-wide)68
identical added path0
the declared list0

The repo-wide key's own top collisions say why it can never ship: the lock file (33 pairs), one plugin manifest (21), the root manifest (15). That is ordinary concurrent work in a repo taking ~18 merges a day — roughly 68 false accusations per 300 PRs, each naming two authors who both did nothing wrong. Those three paths are now pinned by name in the self-test as paths that must stay out of the declared list.

One caveat, stated because the method has a real limit and a clean-looking 0 should not hide it: the files endpoint returns a PR's diff against its current merge base, so a retrospective sweep sees today's diffs, not the ones that were live at the time. The incident pair is invisible to it — the second PR's pin write left its cumulative diff once a merge commit brought in a main that already carried the first PR's identical write. Read commit by commit, that PR's first commit ffc79b028does touch the pin, so the pair is a true positive the sweep can no longer see. The number that decides the design is the 68; the 0 is a floor, not a proof.

What it catches, and what it does not

Catches two open PRs that both write a listed path, whether their hunks are identical or not — two different pin targets in flight at once is as much a violation as the same one twice.

Does not catch two PRs that fix the same thing differently on unlisted paths. They share no listed path and no hash, and the measurement says the only key that reaches them is the one with 68 false positives. The limit is named in the script header rather than hidden.

The action

First come, first served, taken unchanged from the sibling gate: the earlier PR keeps its claim and stays green; the later one goes red naming it by number, URL and branch. Failing both would be symmetric and unhelpful — neither author is wrong — and failing the earlier one would reward racing.

Red rather than a warning, deliberately: #9424 ruled that a signal landing where nobody looks is the expensive half, and a red check on the PR is the visible channel. The gate is read-only — it never closes a PR, comments, or edits a body.

Cost

The objection to any diff-keyed gate is that it must fetch every open PR's files on every run. This one asks the cheap question first: it lists this PR's files, intersects with the declared list, and when the intersection is empty it returns after exactly one paginated call. 1 of those 300 PRs touched a listed path, so the branch that walks other open PRs runs on ~0.3% of runs — and when it does, it walks the open set (9 PRs today), not 300. The short-circuit is pinned by an assertion that counts API calls against a recording fake.

Verification — all at 99e3c060

Self-test: 54 cases pass. Reverse-verified by ablation; every mutation reddens a named assertion:

ablationassertion that caught it
add the lock file to the declared listthe measured high-collision paths are NOT declared
blank an entry's reasonevery declared entry carries a reason
remove the short-circuit...and it costs exactly ONE api call (got 3, want 1)
flip first-come-first-servedan EARLIER open PR on a declared path fails THIS PR (+5 more)
drop synchronize from the triggerthe wiring subscribes to 'synchronize'
silence UNDETERMINED into the clean answeran unwalkable file list is reported as UNDETERMINED
revoke contents: readthe wiring can read contents, which its checkout step requires

The sweep found a real bug in its own tests, fixed in 99e3c060: revoking contents: read — which would kill the job in its checkout step, before the gate judges anything — left the self-test green, because the comment explaining why that scope is needed contains those same two words and the assertion scanned the whole file. A phantom check, green because of the prose describing it. Both permission assertions now scan the comment-stripped workflow, and the ablation reddens with the string still present in the file.

Live end-to-end against the real API (the fake-API self-test cannot prove the fetch layer):

PR #9584 (touches no declared path) -> exit 0, one API call [short-circuit]
PR #9393 (the incident PR, writes the pin) -> exit 0, walked all 9 open PRs, no earlier claimant
no PR_NUMBER -> exit 2, NOT WIRED

Gate families derived from the actual changed paths with node scripts/pm/dispatch-gates.mjs — the derivation discovers the new gate itself, which confirms the registration:

check:single-claim-paths PASS (54 cases)
check:node-version PASS
check:required-contexts PASS
check:shard-attestation PASS
check:workflow-status-functions PASS
check:type-check-coverage PASS
check:type-check-debt FAIL — pre-existing, not from this diff

check:type-check-debt needs a built package closure and says so itself ("Build the closure first, exactly as lint.yml does before this step"). Reproduced byte-identically on a pristine origin/main worktree at 1c6da6ea in the same unbuilt container; this diff touches no TypeScript. CI runs it after the build step.

eslint clean. Prettier is not enforced for scripts/ (its siblings deviate too) and this introduces no new deviation in lint.yml (its pre-existing ones are at lines 22/56/1167; this insertion is 224–253).

Not addressed here

The gate publishes a check run; whether it becomes a required context is a settings change no agent seat can make and carries a maintainer ruling — the Duplicate Fix Guard sits in the same position. #9408 remains open and is a separate defect.

Draft, base main. Not merged, no auto-merge, still draft.


Generated by Claude Code

Two PRs reached green independently shipping one physical change: both
rewrote the objectui pin from the same old sha to the same new sha and
both added the same generated changeset -- a byte-identical hunk under
two card numbers. The first entered the merge queue; the second was
green and one flip from enqueueing behind it.
The card-keyed Duplicate Fix Guard answered correctly and is untouched:
the two PRs claimed two genuinely different, independently filed cards.
The duplication existed only in the diff. This adds the second question
and the two gates run side by side.
The key is a declared list, not repo-wide diff intersection, and that is
measured rather than assumed. Over the 300 most recent PRs, pairs whose
open windows overlapped and which shared a changed path: 68 for the
repo-wide key (top collisions: the lock file 33, one plugin manifest 21,
the root manifest 15), 0 for the declared list. A repo-wide key would be
~68 false accusations per 300 PRs, each naming two authors who did
nothing wrong.
Cost is bounded by asking the cheap question first: the gate lists this
PR's own files, intersects with the declared list, and returns after one
paginated call when the intersection is empty. 1 of those 300 PRs
touched a listed path, so the branch that walks other open PRs runs on
~0.3% of runs.
First come, first served, unchanged from the sibling gate: the earlier
PR keeps its claim and the later one goes red naming it by number, URL
and branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
Found by this gate's own ablation sweep. Revoking `contents: read` from
the guard workflow -- which would kill the job in its checkout step,
before the gate judges anything -- left the self-test GREEN, because the
comment explaining why that scope is needed contains those same two
words and the assertion scanned the whole file.
A phantom check: green because of the prose describing the thing it was
meant to verify. Both permission assertions now scan the comment-stripped
workflow, as the package-manager assertion beside them already did, and
the ablation reddens with the string still present in the file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 18, 2026
@github-actionsgithub-actionsBot added size/l ci/cd dependencies Pull requests that update a dependency file labels Aug 18, 2026
@os-steve
os-steve marked this pull request as ready for review August 18, 2026 12:58
@os-steve
os-steve added this pull request to the merge queueAug 18, 2026
Merged via the queue into main with commit ba0a846Aug 18, 2026
29 checks passed
@os-steve
os-steve deleted the claude/issue-9402-diff-keyed-duplicate-guard branch August 18, 2026 13:22
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cddependenciesPull requests that update a dependency filesize/lskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Two PRs can ship the identical diff under two card numbers — the duplicate-claim gate is issue-keyed, not diff-keyed

2 participants

@os-steve@claude