Skip to content

Commit 6247d7f

Browse files
os-zhuangclaude
andauthored
ci: 第二个声明 Fixes 同一 issue 的 open PR 开出即挂红 —— #4555/#4559 重复开发的三重堵漏 (#4588) (#4589)
* ci: fail a PR at open time when an earlier open PR already fixes the same issue (#4588) GitHub lets any number of open PRs declare Fixes #N for the same issue; #4555 and #4559 proved the cost this morning. Three changes, one per hole: - duplicate-fix-guard.yml: same-repo closing keywords are checked against every other open PR; the LATER PR goes red with a pointer to the earlier one (first come, first served, matching the pm-dispatch claim convention). Re-runs on 'edited'/'synchronize' so resolving the conflict either way clears the red. Branch names that carry no declared issue number get a warning, never a failure. - Claim comments now require a session ID (pm-dispatch template, AGENTS.md, CLAUDE.md): under the shared GitHub identity the assignee field cannot answer "is this claim mine?" — the comment's session line is what can. - Branch naming claude/issue-<n>-<slug> documented in AGENTS.md so git ls-remote | grep issue-<n> is a one-command pre-check. Script validated with node --check under the full injected-identifier wrapper; keyword regex exercised against 10 samples including the prefixes-#N false-positive and cross-repo exclusions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD * fix(ci): duplicate-fix-guard ignores closing keywords inside code spans and fenced blocks The guard's first real run — on its own PR — counted the PR body's backticked DISCUSSION of 'Fixes #4551' as a declaration. GitHub's own closing-keyword parser ignores code formatting; a guard stricter than the linker it protects turns prose mentions of other PRs' fix lines into spurious reds. Strip ```fences``` and `spans` before matching. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b9b7310 commit 6247d7f

5 files changed

Lines changed: 206 additions & 4 deletions

File tree

.changeset/duplicate-fix-guard.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
---
3+
4+
ci: fail a PR at open time when an earlier open PR already declares a fix for
5+
the same issue (#4588)
6+
7+
Release-nothing: adds `.github/workflows/duplicate-fix-guard.yml` and updates
8+
agent process docs (AGENTS.md, CLAUDE.md, pm-dispatch claim template) — no
9+
package code.
10+
11+
GitHub lets any number of open PRs declare `Fixes #N` for the same issue.
12+
On 2026-08-02, #4555 and #4559 both declared `Fixes #4551` and both were
13+
implemented in full — 834 duplicate lines through the whole gate suite — with
14+
the duplication machine-detectable from the second PR's open (03:08) yet
15+
unnoticed by any human until 08:52. The shared GitHub identity made the
16+
issue's assignee useless as a warning: "assigned to os-zhuang" reads the same
17+
whether the claimant is you or another session.
18+
19+
Three changes, one per hole:
20+
21+
- **Duplicate Fix Guard workflow**: on PR opened/edited/reopened/synchronize,
22+
parse same-repo closing keywords and fail the PR if an EARLIER open PR
23+
(lower number) declares the same issue, naming it. First come, first
24+
served — matching the pm-dispatch "first claim comment wins" convention.
25+
The check is body-driven and re-runs on `edited`, so a red PR goes green
26+
the moment the conflict is resolved either way.
27+
- **Claim comments must carry a session ID** (pm-dispatch template, AGENTS.md,
28+
CLAUDE.md): under a shared identity, the comment's session line is the only
29+
thing that makes "is this claim mine?" answerable.
30+
- **Branch naming `claude/issue-<n>-<slug>`** (AGENTS.md): puts the issue
31+
number where `git ls-remote | grep issue-<n>` can find it; the workflow
32+
warns (never fails) when a fix PR's branch names no declared issue.

.claude/skills/pm-dispatch/SKILL.md

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,14 +205,20 @@ execute atomically, in order:
205205
1. **Assign** to yourself (`@me`) and add `pm:dispatched`. Skip — and drop
206206
from the batch — any issue that acquired an assignee since step 1.
207207
2. **Claim comment** (Chinese), fixed shape — the branch name is the key,
208-
every later artifact (worktree, push, PR) hangs off it:
208+
every later artifact (worktree, push, PR) hangs off it. The session ID is
209+
NOT optional: under the shared identity it is the only line that lets a
210+
later reader — including your own future self after a context reset —
211+
answer "is this claim mine?". A claim without it caused the #4555/#4559
212+
duplicate (#4588): the second session saw its own shared name as assignee
213+
and could not tell the claim was someone else's.
209214
> 认领:PM 循环第 N 轮
215+
> 会话:`session_<id>`
210216
> 分支:`claude/issue-<n>-<slug>`
211217
> Worktree:`<repo>-issue-<n>`
212218
3. **Race check**: assignment is idempotent, so two agents can both
213219
"succeed". Re-read the comments; if an earlier claim comment with a
214-
*different* branch name exists, you lost — touch nothing of theirs,
215-
reply 「已有认领,让行」, and pick another issue. First comment wins.
220+
*different* session ID or branch name exists, you lost — touch nothing of
221+
theirs, reply 「已有认领,让行」, and pick another issue. First comment wins.
216222

217223
Dev agents push their branch early — a remote branch is the hardest evidence
218224
of work in flight, closing the gap between "claimed" and "PR exists".
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# GitHub happily lets any number of open PRs declare `Fixes #N` for the same
2+
# issue. On 2026-08-02 that cost a full duplicate implementation: #4555 and
3+
# #4559 both declared `Fixes #4551`, both ran the whole gate suite green, and
4+
# the duplication sat machine-detectable from the moment the second PR opened
5+
# (03:08) until a human noticed it (08:52). All agents here share one GitHub
6+
# identity, so the issue's assignee could not warn the second author either —
7+
# "assigned to os-zhuang" reads the same whether it is you or another session.
8+
# Full post-mortem: #4588.
9+
#
10+
# This gate makes the second PR red at open time. First come, first served —
11+
# the EARLIER open PR (lower number) keeps its claim and stays green; the
12+
# later one fails with a pointer to it. That matches the pm-dispatch claim
13+
# convention ("first claim comment wins") already in .claude/skills/.
14+
#
15+
# Scope: same-repo references only (bare `#N` and qualified `<this repo>#N`).
16+
# Cross-repo references are the cross-repo-issue-closer's territory, and a
17+
# duplicate across repos cannot be resolved by failing one side's CI anyway.
18+
name: Duplicate Fix Guard
19+
20+
# `edited` matters as much as `opened`: a PR that adds `Fixes #N` to its body
21+
# after the fact must re-run this check, and one that drops the line must be
22+
# able to go green again.
23+
on:
24+
pull_request:
25+
types: [opened, edited, reopened, synchronize]
26+
27+
permissions:
28+
pull-requests: read
29+
30+
jobs:
31+
duplicate-fix-guard:
32+
name: No other open PR may claim the same issue
33+
runs-on: ubuntu-latest
34+
steps:
35+
- name: Check declared issues against other open PRs
36+
uses: actions/github-script@v9
37+
with:
38+
script: |
39+
const pr = context.payload.pull_request;
40+
const thisRepo = `${context.repo.owner}/${context.repo.repo}`;
41+
42+
// GitHub's own closing-keyword set. The optional colon is part of
43+
// GitHub's accepted syntax (`Fixes: #123`). Two same-repo forms:
44+
// bare `#N` and qualified `owner/repo#N` naming THIS repo.
45+
const KEYWORDS = 'close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved';
46+
const pattern = new RegExp(
47+
`\\b(?:${KEYWORDS}):?\\s+(?:([\\w.-]+)\\/([\\w.-]+))?#(\\d+)\\b`,
48+
'gi',
49+
);
50+
51+
// Strip fenced blocks and inline code spans before matching.
52+
// GitHub's own closing-keyword parser ignores code formatting, and
53+
// this guard must not be stricter than the linker it protects:
54+
// this very PR's first run counted its own DISCUSSION of
55+
// `Fixes #4551` (in backticks, describing the incident) as a
56+
// declaration. Benign there — #4551 is closed — but a prose
57+
// mention of an issue some other open PR really fixes would have
58+
// been a spurious red.
59+
const declaredIssues = (body) => {
60+
const prose = (body || '')
61+
.replace(/```[\s\S]*?```/g, ' ')
62+
.replace(/`[^`\n]*`/g, ' ');
63+
const found = new Set();
64+
for (const [, owner, repo, number] of prose.matchAll(pattern)) {
65+
// A qualified reference to ANOTHER repo is not ours to judge.
66+
if (owner && `${owner}/${repo}`.toLowerCase() !== thisRepo.toLowerCase()) continue;
67+
found.add(Number(number));
68+
}
69+
return found;
70+
};
71+
72+
const mine = declaredIssues(pr.body);
73+
if (mine.size === 0) {
74+
core.info('This PR declares no same-repo closing keywords — nothing to guard.');
75+
return;
76+
}
77+
core.info(`This PR declares: ${[...mine].map((n) => `#${n}`).join(', ')}`);
78+
79+
// Branch-name convention (advisory, never red): a fix branch named
80+
// `claude/issue-<n>-<slug>` is discoverable by the next session
81+
// with one `git ls-remote | grep issue-<n>`. #4555 vs #4559
82+
// happened partly because the branches shared no token to grep.
83+
// Warning only — existing branches must not go red retroactively.
84+
const branch = pr.head.ref;
85+
if (![...mine].some((n) => branch.includes(`issue-${n}`))) {
86+
core.warning(
87+
`Branch \`${branch}\` does not name any declared issue. ` +
88+
`Convention: claude/issue-<n>-<slug> (e.g. claude/issue-${[...mine][0]}-short-slug) ` +
89+
`so parallel sessions can discover in-flight work with git ls-remote.`,
90+
);
91+
}
92+
93+
// Drafts count: a draft PR is work in flight, which is exactly
94+
// what the second session needs to see.
95+
const openPrs = await github.paginate(github.rest.pulls.list, {
96+
owner: context.repo.owner,
97+
repo: context.repo.repo,
98+
state: 'open',
99+
per_page: 100,
100+
});
101+
102+
const conflicts = [];
103+
for (const other of openPrs) {
104+
if (other.number === pr.number) continue;
105+
const theirs = declaredIssues(other.body);
106+
const shared = [...mine].filter((n) => theirs.has(n));
107+
if (shared.length > 0) conflicts.push({ other, shared });
108+
}
109+
110+
if (conflicts.length === 0) {
111+
core.info('No other open PR declares these issues.');
112+
return;
113+
}
114+
115+
// First come, first served: only the LATER PR goes red. Failing
116+
// both would leave the original author red through no action of
117+
// their own; failing the earlier one would reward racing.
118+
const older = conflicts.filter((c) => c.other.number < pr.number);
119+
const newer = conflicts.filter((c) => c.other.number > pr.number);
120+
121+
for (const { other, shared } of newer) {
122+
core.info(
123+
`#${other.number} (newer) also declares ${shared.map((n) => `#${n}`).join(', ')} — ` +
124+
`it will fail its own run of this guard; this PR keeps its claim.`,
125+
);
126+
}
127+
128+
if (older.length > 0) {
129+
const lines = older.map(({ other, shared }) =>
130+
` - ${shared.map((n) => `#${n}`).join(', ')} is already claimed by #${other.number} ` +
131+
`(${other.html_url}, branch \`${other.head.ref}\`${other.draft ? ', draft' : ''})`,
132+
);
133+
core.setFailed(
134+
`Another open PR already declares a fix for the same issue(s):\n${lines.join('\n')}\n` +
135+
`If this PR is the duplicate, close it and add anything it uniquely covers to the ` +
136+
`earlier PR or a follow-up issue (that is what saved #4560 when #4559 was closed). ` +
137+
`If the EARLIER one is abandoned, close it first — this check re-runs on 'edited' ` +
138+
`and 'synchronize', and goes green once the conflict is gone.`,
139+
);
140+
}

AGENTS.md

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,18 @@ then race to land conflicting shapes for the same problem, which is worse than
118118
either one alone. If it is already assigned to someone else it is taken — pick
119119
another, or say so and ask; never reassign it to yourself.
120120

121+
Because every agent here shares one GitHub identity, the assignee field alone
122+
cannot answer "is this claim *mine*?" — seeing your own shared name on an issue
123+
is exactly what another session's claim looks like. So a claim is two acts, not
124+
one: assign, **and leave a claim comment carrying your session ID and branch
125+
name** (`claude/issue-<n>-<slug>`). Before writing code, re-read the issue's
126+
comments; an earlier claim comment with a different session ID or branch means
127+
the issue is taken no matter what the assignee field seems to say. Skipping
128+
this read is how #4551 got implemented twice in one morning (#4555 and #4559
129+
post-mortem in #4588), and misreading shared-identity state is also how a
130+
maintainer's manual ready-flip got reverted by an agent that assumed its own
131+
write had failed.
132+
121133
The claim is also what makes the *finding* rule (Prime Directive #10) safe to
122134
follow. Once out-of-scope discoveries become issues, the issue list is a real
123135
queue other agents read, and a claim is the only thing separating "someone is on
@@ -130,7 +142,13 @@ Even inside your own worktree, operate defensively:
130142
reverts, or other agents' in-flight edits, and don't try to manage the whole
131143
working tree. If a file you didn't change shows as modified, leave it.
132144
2. **One feature branch + one PR per task.** Branch off `main`. **Never commit
133-
task work straight to `main`.**
145+
task work straight to `main`.** Name the branch after the issue it fixes:
146+
`claude/issue-<n>-<slug>`. The issue number in the name is what makes
147+
in-flight work *discoverable* — `git ls-remote --heads origin | grep
148+
issue-<n>` is a one-command pre-check, and the Duplicate Fix Guard workflow
149+
warns on fix PRs whose branch names no declared issue. The #4555/#4559
150+
duplicate (#4588) stayed invisible partly because one branch carried the
151+
issue number and the other didn't.
134152
3. **Never `git push --force` / `--force-with-lease`, and never push `main`.** A
135153
force-push can clobber a parallel agent's work; `main` is shared — land
136154
everything via PR.

CLAUDE.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ conflicting shapes for one problem. Already assigned to someone else? It is take
1515
another or ask; never reassign it to yourself. File findings unassigned when you are only
1616
recording them; assign at the moment you start.
1717

18+
All agents share one GitHub identity, so the assignee field can't tell you whether a claim
19+
is **yours** — a claim is assign **plus a claim comment with your session ID and branch**
20+
(`claude/issue-<n>-<slug>`), and before writing code you must re-read the comments: an
21+
earlier claim with a different session ID means it's taken, whatever the assignee says.
22+
(#4551 was implemented twice in one morning because this read was skipped — see #4588.)
23+
1824
## ⛔ Worktree-first — before your FIRST file edit (AGENTS.md Prime Directive #11)
1925

2026
This repo — **and every sibling repo you touch (`objectui`, `cloud`)** — is edited by

0 commit comments

Comments
 (0)