fix(ci): write PR labels additively, never as a whole set

[claude]

Fixes#10703

Both label-writing jobs in .github/workflows/pr-automation.yml reached PUT /issues/{n}/labels through a third-party action. A whole-set PUT is a read-modify-write across a network round trip, so it destroys any label that lands in between. This replaces both with scripts/pr-labels.mjs, which issues POST and targeted DELETE only.

Premise check — one half of the dispatch brief was wrong

The brief said one of the two PUTs was "a step of ours doing curl -X PUT" at :16 and could be made additive, while the other was third-party. Re-derived against origin/main at 01c50322a1: line 16 is not a step. It is a comment line inside the header block, quoting codelytv/pr-size-labeler's own src/github.sh:68-91. This workflow contained no step of ours that wrote labels at all — a grep for label writes across .github/workflows/** returns only these two uses: lines. So the "one is ours / one is theirs" split does not exist; both were third-party and both needed the same remedy. Both were replaced.

The issue's own premise — that the erasure is real and recurring — is confirmed. I re-fetched the #10698 timeline independently and it matches the reported table event-for-event.

The mechanism, reproduced mechanically

Only three label verbs exist and exactly one is destructive:

verb	effect
POST /issues/{n}/labels	adds the named labels; touches nothing else
DELETE /issues/{n}/labels/{name}	removes one label, by name
PUT /issues/{n}/labels	replaces the whole set — destructive

The race cannot be re-run live, so it is built from the code path plus the timeline. github.sh:68-91 reads the label set, greps out its own size family, appends, and PUTs. On #10698 that read happened before the seat's skip-changeset POST landed, so it read [] and PUT ["size/l"] onto a PR that by then carried ["skip-changeset"].

That interleaving is now a permanent test, not a paragraph. --self-test replays it against both shapes:

FAIL the additive plan preserves it at the SAME interleaving
expected: ["skip-changeset","size/l"]
actual: ["size/l"]

(that is the ablation output — the retired shape, reproduced. The shipped shape returns ["skip-changeset","size/l"].)

The write is additive — exact verbs and paths

Driving the real exported plan builders over PR #10698's real file list:

--- the exact requests the new writer issues ----------------
at the ORIGINAL interleaving (stale read = [], PR already carries skip-changeset):
POST /repos/{owner}/{repo}/issues/10698/labels body={"labels":["size/l"]}
POST /repos/{owner}/{repo}/issues/10698/labels body={"labels":["ci/cd"]}
with a stale size/m also on the PR:
POST /repos/{owner}/{repo}/issues/10698/labels body={"labels":["size/l"]}
DELETE /repos/{owner}/{repo}/issues/10698/labels/size%2Fm

Neither verb carries a label the writer does not name, so neither can destroy a concurrent writer's label — at any interleaving, with no ordering constraint between writers and no window left to narrow. That is the difference from every configuration change before it: correctness no longer depends on timing.

The same run reproduces the retired actions' output exactly on real data: total modifications = 724 -> size/l (the bot applied size/l) and path labels = [ci/cd] (the bot applied ci/cd).

Decisions the brief left to me

1. Additive writes, not reconcile-after. Reconcile-after cannot know what to restore: the label it would repair arrives during the window, so it is in neither the before-snapshot nor the after-snapshot, and distinguishing "erased by us" from "deliberately removed by a human" needs a timeline heuristic that is wrong in both directions. Additive writes need no such inference. The triage note reached the same conclusion; direction 2 is not included even as a rider, because a self-healing step on top of a write that can no longer lose anything is machinery guarding nothing.

2. Both writers replaced, not just the measured one.actions/labeler@v7.0.0 is partly mitigated already — labeler.ts:111-133 re-reads the live set and carries forward labels added during the run — but the window [re-read .. PUT] stays open, and #5533 lost skip-changeset to exactly this writer. Closing only the measured half would have left a 12-second-later window that has already caused a loss.

3. Header prose rewritten. It asserted this half was open and pointed at a closed card, which would have been wrong in a new way. It now documents the verb table, the measured specimen, why this is a fix rather than a narrowing, and what is still open.

4. The reverse race. The labeler removing a size label a seat is concurrently adding: under sync-labels: false the path labeler never removed anything, and it still never does — the path half has no DELETE at all. The size family is owned by this writer, so a seat's hand-applied size/xl is still retired when the computed size disagrees. That behaviour is unchanged from codelytv (which grepped the family out of its PUT payload); the difference is that its version also carried, and could drop, every bystander label. Noted, not changed — it is the writer removing a label it owns, not an erasure.

Behaviour deliberately not carried over

• fail_if_xl: 'false' — selected the do-nothing branch.
• message_if_xl — dead as this workflow configured it. labeler.sh calls add_label_to_pr and only then asks ! github::has_label "$pr_number" "$xl_label", i.e. it tests for the label it has just written, so the guard is false and the comment never posts. Reimplementing it would add a comment this repo has never actually seen — a feature request, not a port.
• One deliberate divergence: github.sh:23 caps at per_page=100 and never paginates (its own NOTE says so), so a 400-file PR was sized off its first 100 files. This paginates, so a PR over 100 files may now get a larger, correct label.

What CI on this PR does and does not prove

pr-automation.yml triggers on plain pull_request — there is no pull_request_target and no merge-triggered job in it — and pull_request workflows run the head branch's version of the file. So both replaced steps really do execute on this PR, with a real token, against a real PR. The labels this PR carries are written by the new code. That is unusually strong for a workflow change, and it is worth stating precisely because it is normally not true.

Not proven here: behaviour under a fork PR (a read-only token), and the >100-file pagination path (this PR changes 3 files).

Live proof: this PR is its own specimen

I applied skip-changeset with an additive POST at 11:18:56Z, seconds after opening the PR — deliberately into the window that erased it on #10698. Every label event on this PR since:

time	event	label	actor
11:18:57Z	labeled	skip-changeset	claude[bot] (additive POST, HTTP 200)
11:19:03Z	labeled	size/xl	github-actions[bot]
11:19:20Z	labeled	ci/cd	github-actions[bot]

No unlabeled event. The label was written 6 seconds before the size labeler and 23 seconds before the path labeler — squarely inside the window — and survived both. Read back at 11:20:36Z, after the bots settled: ci/cd, size/xl, skip-changeset. Compare #10698, where the gap was one second and the label was gone.

The steps that ran are the new ones, by name, all green (run 32476631817):

JOB: Check PR Size -> success
3 Self-test the additive label writer -> success
4 Add size label (additive POST, then a targeted DELETE) -> success
JOB: Auto Label -> success
3 Label based on changed files (additive POST) -> success

and their real logs show the verbs actually issued against a real PR with a real token:

VERDICT: pr-labels self-test PASSED
pr-labels: 3 changed file(s); total modifications (additions + deletions, ignoring pnpm-lock.yaml package-lock.json yarn.lock): 1013 -> size/xl
pr-labels: labels on PR #10777 right now: skip-changeset
pr-labels: POST /issues/10777/labels -- add the computed size label 'size/xl'
pr-labels: 3 changed file(s) match: ci/cd
pr-labels: labels on PR #10777 right now: size/xl, skip-changeset
pr-labels: POST /issues/10777/labels -- add path label(s) ci/cd

Note the third line of each: the writer readskip-changeset and did not carry it into any write. That is the whole difference. The retired code path would have taken that same read, appended its own label and PUT the union back — and any label arriving after the read would not have been in it.

The follow-up push gave a second, independent specimen — the idempotent path. Both writers ran again against the settled set and wrote nothing at all (run 32477473695):

pr-labels: labels on PR #10777 right now: ci/cd, size/xl, skip-changeset
pr-labels: nothing to write.

That is the run the retired header called out as "one more chance to erase a concurrent writer in exchange for no new information": codelytv would have re-PUT the whole set here. Two full runs of both label writers on this PR, and still zero unlabeled events.

Check Changeset also went green with the exemption honoured (its Require a changeset step skipped), which is the downstream consequence the card is really about.

Gates

node scripts/pm/dispatch-gates.mjs re-derived from the merge base at final commit ce525a05d3 — editing lint.yml pulled in two families the dispatch list did not name (check:type-check-coverage, check:type-check-debt). Each gate's verdict line, exit code captured before any pipe:

gate	exit	its own verdict line
check:cross-package-test-inputs	0	OK: 13 package(s) read outside themselves, all declared
check:node-version	0	OK (32 setup-node step(s) across 26 workflow(s), all on Node 22)
check:required-contexts	0	(pin table printed, no problems)
check:shard-attestation	0	2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s)
check:workflow-status-functions	0	OK (scanned 26 workflow file(s), 49 job(s) ...)
check:type-check-coverage	0	OK — 64/77 workspace packages type-checked
check:nul-bytes	0	OK (scanned 6238 text file(s) ... no raw ASCII control bytes)
check-aggregator-roster.mjs	0	roster == needs: in both directions

Declared narrowing:check:type-check-debt is --self-test && --re-measure. The self-test half passed; --re-measure refused to run without a built workspace closure and said so explicitly, leaving the ledger untouched. It re-measures tsc error counts for 13 packages, and this diff contains zero TypeScript — a YAML edit and one .mjs script cannot move a tsc number. I did not build the full closure to satisfy it; CI runs that half with the closure built.

Workflow syntax: actionlint is not installed in this container, so both edited workflows were validated by a real YAML parse (PyYAML safe_load), plus the four repo gates that parse workflows for real (check-workflow-status-functions, check-required-contexts, check-aggregator-roster, check-shard-attestation).

Reverse verification

Two ablations, each confirmed on disk by grepping the injected and the removed text before running (an editor's exit code is not evidence). No build step is involved — this script runs from source, no dist/.

1. method: 'POST' → 'PUT' in planSizeWrites (on-disk: PUT 0→1, POST 3→2): self-test exit 1, 4 failures, including the interleaving replay showing ["size/l"] where ["skip-changeset","size/l"] was expected — the defect itself.
2. ** zero-segment case broken (k = j → k = j + 1): self-test exit 1, 3 failures, exactly the assertions where ** must match zero segments. matches through the parsed config correctly stayed green — that input has enough segments that the mutation does not change its answer.

Both restored and proved restored (grep counts back, git diff empty, self-test exit 0).

Follow-up commit: the entry guard (e0accbffc7)

check:entry-guard went red on the first commit, and it was right. ce525a05d3 guarded its CLI dispatch with a hand-typed process.argv[1] comparison, and scripts/** has exactly one sanctioned predicate. That spelling avoided the percent-encoding trap (pathToFileURL(...).href encodes, so a # in a parent directory is harmless), but it still fails the symlink direction: node resolves symlinks for the module graph and leaves process.argv[1] as the caller typed it. Measured here with both spellings side by side, each reached directly and through a symlink:

old-guard -> exit=0 output=RAN
old-link -> exit=0 output=(nothing) <-- silently inert, exit 0
new-guard -> exit=0 output=RAN
new-link -> exit=0 output=RAN

That is the silent-success direction this repo treats as worse than no check at all — a caller reading result.status gets a green from a tool that never ran. Now import { isEntrypoint } from './invoked-as.mjs';, with the newly unused pathToFileURL import dropped.

The file stays offKNOWN_IMPORT_UNSAFE: the gate still reports 10 known-unsafe (unchanged) with this file among the 75 inert exporters. Verified directly rather than inferred — importing it with --size in argv runs nothing and still yields all eight exports.

Gates re-run at e0accbffc7, exit codes captured before any pipe: check:entry-guard0 (both legs — 47 cases pass, then every entry guard goes through invoked-as.mjs; 85 export bindings, 75 of them inert on import), check:parse-guard0, check:type-check-coverage0, check:nul-bytes0, node scripts/pr-labels.mjs --self-test0. The families in the table above were re-run unchanged.

Why it was absent from my gate list.node scripts/pm/dispatch-gates.mjs never named it — and still does not, re-derived at e0accbffc7 with the file committed and tracked (grep -c entry-guard over the output: 0). --residue places it in Silent, because the derivation reads the gate's declared population off KNOWN_IMPORT_UNSAFE (check-entry-guard.mjs:437-448) — an enumeration of the ten files that already violate the rule. A newly added script can never be in that list, so the derivation returns its weakest verdict for exactly the case most likely to fire. Its sibling check:parse-guard covers the same directory and fails the opposite way, reported Unreachable — dead: 'scripts'. Both scripts/** gates are invisible to the derivation, by opposite routes. Filed as #10784; I ran check:parse-guard here by hand for the same reason.

Still open

This file no longer writes a whole set, but nothing mechanically stops a future workflow, action or agent from doing so. There is no repo gate that bans the verb; the header paragraph and this script's self-test are the whole guard. Filed as a follow-up rather than added here, because it needs a check:* entry in the root package.json, which is fenced by the @changesets/cli v3 migration lane.

Fenced surfaces were not touched: the Check Changeset job and its steps, and root package.json. The self-test is wired into lint.yml's Lint & Repo Gates job by direct node scripts/... invocation for exactly that reason — and it needs a required-context home anyway, since Check PR Size is deliberately excluded from the required set.

---

Generated by Claude Code

---

Generated by Claude Code

[claude]

PM review (domain:devx seat) — sound, but held for a maintainer on scope, not on correctness

First: the dispatch brief was wrong and this PR is right

I wrote that pr-automation.yml:16 was "a step of ours doing curl -X PUT" that could be made additive, and split the work into "one ours / one theirs". That split does not exist. Verified against origin/main = 9c0215f61e: line 16 is a comment inside the header block describing codelytv/pr-size-labeler's src/github.sh:68-91, and the only label writers in the file are two uses: lines — codelytv/pr-size-labeler@v1.10.4 (:67) and actions/labeler@v7.0.0 (:108). Both third-party.

I produced that error by grepping the file for PUT and reading the matching lines as steps without noticing the leading #. The agent re-derived instead of building on it, which is what the brief asks for and is the only reason this landed as a correct change rather than a wrong one.

That error is also why this PR is larger than the card implies: with no step of ours to amend, "make the writes additive" can only be reached by replacing both actions.

What I verified myself, against the source rather than the report

The core contract holds. Every HTTP verb the script emits, from scripts/pr-labels.mjs at ce525a05d3:

:337 method: 'POST'
:347 method: 'DELETE'
:363 method: 'POST'
:666 method: FORBIDDEN_VERB <- self-test only, replaying the RETIRED shape
:116 const FORBIDDEN_VERB = 'PUT'

There is no PUT on any live path. The only occurrence exists so the self-test can demonstrate the destructive behaviour it replaced.

The lint.yml placement is also right for a non-obvious reason the PR states: Check PR Size is deliberately excluded from the required set, and a labeled event republishes that context as skipped, so a red there blocks nothing. Wiring the self-test into Lint & Repo Gates is what makes it load-bearing.

Why I am not arming this myself

Not a correctness objection — a cost-and-ownership one, which is above this seat:

1. Blast radius is every PR in the repo. If the path matcher or size bucketing is subtly wrong, the failure mode is quiet mislabelling, not a red check.
2. It moves maintenance of two upstream actions in-house — 775 lines this repo now owns, including a minimatch subset. The card asked for additive writes; in-housing is one route to that, and the repo has not previously ruled on paying that price.
3. Two deliberate behaviour changes ride along, both defensible and both stated plainly in the PR body: pagination past 100 files (so a large PR may now get a different, more correct size label), and message_if_xl dropped as dead-as-configured.

My recommendation: land it

The harm is measured and recurring, not theoretical — #5533 and #10698 each lost a real skip-changeset, and the consequence is a false changeset-check red on a PR that legitimately publishes nothing. The workflow's own header already concluded that no configuration can close this and that the writes themselves must become additive; neither action exposes such an input. So the alternatives are this change or leaving a known label-corrupting race open.

The evidence is also unusually strong for a workflow change, and the PR is right to say so: pr-automation.yml triggers on plain pull_request with no pull_request_target, so the labels on this very PR are written by the new code.

A maintainer wanting to reduce scope has one clean option: replace only codelytv/pr-size-labeler (the writer measured erasing a label on #10698) and leave actions/labeler@v7.0.0, whose re-read at labeler.ts:111-133 narrows but does not close its window. That halves the new surface and leaves a smaller, already-demonstrated hole open — #5533 was lost to that second writer.

Holding as draft, unarmed, pending that call. Not blocked on anything else; CI is in flight and I will report if it goes red.

---

Generated by Claude Code

[claude]

A third specimen, 13 minutes after I held this PR — the cost of waiting is now measured

Adding this because it bears directly on the scope decision above, and because it happened while that decision was pending rather than in the past.

PR #10786 (a domain:devx test-only PR, opened 11:31:38Z by another agent in this same batch) applied skip-changeset with a correct additive POST. From its timeline:

time	event	label	actor
11:31:53Z	labeled	skip-changeset	claude[bot] — additive POST, HTTP 200
11:31:53Z	unlabeled	skip-changeset	github-actions[bot]
11:31:53Z	labeled	size/m	github-actions[bot]

Same second. The size labeler's whole-set PUT erased it, bundled with its own size/m write — the #10698 pattern reproduced exactly, on a PR nobody was testing this against. I re-applied the label by hand at 11:32:52Z.

That makes three measured losses to this verb: #5533, #10698, and now #10786. The third is the one that changes the arithmetic, because it is not archaeology — it landed after the fix was written, reviewed and ready, while the PR sat unarmed awaiting a call on scope. skip-changeset is the exemption for a PR that publishes nothing, so each loss is a false changeset-check red on a PR that legitimately has no changeset, plus a hand re-application by whoever notices.

What this does and does not change

It does not change why I held this PR. Replacing two maintained upstream actions with 775 lines of in-repo code is still a maintenance-ownership decision above this seat, and a maintainer may still legitimately prefer the reduced-scope option I named (replace only codelytv/pr-size-labeler, leave actions/labeler@v7.0.0). Nothing here argues for skipping that call.

It does sharpen the cost side of it. The rate is roughly one loss per batch of PRs from this seat, and every one of them is silent unless someone reads the label set back after the bots settle — which is not the happy path any brief describes. My recommendation to land stands, and now rests on a measured recurrence rather than on two historical incidents.

One thing worth noting for whoever rules: this PR's own live evidence is the mirror image of the specimen above. Its author applied skip-changeset at 11:18:56Z deliberately inside the same window, with the new writer active, and the timeline shows no unlabeled event — the label survived bot writes 6s and 23s later. Same window, same bots, opposite outcome, twelve minutes apart.

---

Generated by Claude Code

[github-actions]

⛔ merge queue 构建失败 — 先分诊，再决定要不要重排

队列构建 32550920278 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集），
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job（日志抽取，best effort）：

• Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

  ✗ Build failed in 4.09s
  

↳ 失败原因 是判读的关键：超时（Test timed out in … / Hook timed out in …）多半是负载/时序，不是本 PR 的回归；
断言（AssertionError: …）才指向真实的行为改变。两者的 FAIL 行长得一模一样，只有这一行能区分。

跨 PR 相同签名（24h，按失败测试文件聚合）：

• ⚠️本次没有可用的聚合签名（日志里没有能解析出测试文件名的 FAIL 行）—— 这不是「没有同签名的其他 PR」，是这一轮没测到。跨 PR 聚合本次不可用，请手工比对其他 PR 的同类评论。
• ⚠️ 24h 评论账本没读完（超过 5 页仍未读到窗口尽头），所以上面的「不同 PR 数」是下界，不是全量。

历史信号：

• 本 PR 过去 24h 无队列失败记录（首次）。
• 过去 24h 队列共有 75 个失败构建（不含本次）。

分诊清单：

1. 失败测试在本 PR 改动的包里 → 真回归，修 PR。
2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」；已有汇总 issue ⇒ flaky/环境问题实锤，去那张 issue 上谈，修好前重排只会再烧一轮全队列。
3. 两者都不是 → 可能与同组 PR 语义冲突；等前面的 PR 落地或失败出队后再重排一次即可，不要连续重排。

---

Generated by Claude Code · merge-queue-triage workflow (#4859)