66
77jobs :
88 # ===========================================================================
9- # Both label-writing jobs below write this PR's label set with a WHOLE-SET PUT
10- # (`PUT /issues/{n}/labels`), never an additive POST. Read out of the pinned
11- # sources rather than inferred from the docs (#5649):
9+ # LABEL WRITES IN THIS FILE ARE ADDITIVE. Nothing here may issue
10+ # `PUT /issues/{n}/labels`.
11+ #
12+ # A whole-set PUT REPLACES a PR's label set, so every whole-set write is a
13+ # read-modify-write across a network round trip and destroys any label that
14+ # lands in between -- silently, with an `unlabeled` event nobody watches for.
15+ # Only three verbs exist and only one is destructive:
16+ #
17+ # POST /issues/{n}/labels adds the named labels, touches nothing else
18+ # DELETE /issues/{n}/labels/{name} removes ONE label, BY NAME
19+ # PUT /issues/{n}/labels replaces the whole set -- DESTRUCTIVE
20+ #
21+ # Both jobs below used to reach the third verb through a third-party action.
22+ # Read out of the pinned sources rather than inferred from the docs (#5649):
1223 #
1324 # * codelytv/pr-size-labeler@v1.10.4 -- src/github.sh:68-91
1425 # (`github::add_label_to_pr`): GETs the PR, greps its OWN size family out
1526 # of the result, appends the new size label, then
16- # `curl -X PUT .../issues/$pr_number/labels` with the whole set.
27+ # `curl -X PUT .../issues/$pr_number/labels` with the whole set. No
28+ # mitigation of any kind: the window is the entire round trip.
1729 # * actions/labeler@v7.0.0 -- src/labeler.ts:56,111-133 plus
1830 # src/api/set-labels.ts: snapshots `preexistingLabels` at run start,
19- # unions in the config matches, re-reads the live label list once, then
20- # calls `client.rest.issues.setLabels` -- which IS the PUT.
31+ # unions in the config matches, re-reads the live label list once and
32+ # carries forward whatever appeared in between, then calls
33+ # `client.rest.issues.setLabels` -- which IS the PUT. That re-read
34+ # NARROWS the window to [re-read .. PUT]; it does not close it.
35+ #
36+ # Neither action exposed an input that made its write additive, and
37+ # `sync-labels` was never that input: it only decided whether a label the
38+ # CONFIG owns is dropped once its globs stop matching (labeler.ts:81-83).
39+ #
40+ # ## The measured loss (#10703)
41+ #
42+ # PR #10698, every label event from the timeline API:
43+ #
44+ # 09:05:29Z labeled skip-changeset claude[bot] (additive POST, HTTP 200)
45+ # 09:05:30Z unlabeled skip-changeset github-actions[bot] <-- the size labeler's PUT
46+ # 09:05:30Z labeled size/l github-actions[bot]
47+ # 09:05:42Z labeled ci/cd github-actions[bot]
48+ # 09:06:03Z labeled skip-changeset claude[bot] (re-applied after read-back)
2149 #
22- # Neither action exposes an input that makes its write additive, and
23- # `sync-labels` is NOT that input: it only decides whether a label the CONFIG
24- # owns is dropped once its globs stop matching (labeler.ts:81-83). It is
25- # pinned explicitly below for upgrade-drift protection only. It does not, and
26- # cannot, stop the clobbering described here.
50+ # One second. The writer did everything right -- additive POST, HTTP 200,
51+ # read-back confirmed -- and still lost the label. `skip-changeset` is the
52+ # exemption for a PR that publishes nothing, so its erasure makes
53+ # `changeset-check` demand a changeset from a PR that legitimately has none.
54+ # #5533 lost the same label the same way, that time to the path labeler's PUT
55+ # of `{size/m, tests}`.
2756 #
28- # A whole-set PUT only destroys someone else's label when that label lands
29- # inside the window between the writer's read and its PUT. What this file can
30- # therefore fix is the OVERLAP, and two changes below do exactly that:
57+ # ## Why this is a fix and not another narrowing
3158 #
32- # 1. The two writers no longer run concurrently -- `auto-label` needs
33- # `pr-size`. They used to be started by the same event and overlapped
34- # exactly. Live specimen, PR #5650 run 31051251795 (the `opened` run):
35- # `Add size label` ran 22:03:47->22:03:49 and
36- # `Label based on changed files` ran 22:03:47->22:03:49, and the
37- # labeler's PUT emitted `unlabeled size/s` at 22:03:49 -- one second
38- # after the size job added it, for a label the labeler does not manage.
39- # 2. Neither writer runs on `labeled`/`unlabeled` any more. Their only input
40- # is the diff, which a label event cannot change, so such a run could
41- # only ever re-PUT the same set -- one more chance to erase a concurrent
42- # writer in exchange for no new information. Same PR, run 31051273625
43- # (started by a label event): `Auto Label` recomputed and wrote nothing,
44- # `Check PR Size` re-PUT at 22:04:22. The two event types stay in `on:`
45- # because `changeset-check` genuinely needs them (#5580).
59+ # Both steps now call `scripts/pr-labels.mjs`, which issues POST and targeted
60+ # DELETE only. Neither verb carries a label the writer does not name, so
61+ # neither can destroy a concurrent writer's label -- at ANY interleaving, with
62+ # no ordering constraint between writers and no window left to narrow.
63+ # Correctness no longer depends on timing, which is what every configuration
64+ # change before it could only ever improve. The plan builders in that script
65+ # are pure functions and its `--self-test` asserts they emit no PUT, so a
66+ # future edit that reaches for a whole-set write goes red in lint before it
67+ # can reach a PR.
4668 #
47- # NOT closed by either change, and deliberately recorded rather than implied:
48- # a writer OUTSIDE this workflow -- an agent or a human labelling the PR
49- # seconds after `gh pr create`, i.e. exactly while these jobs run -- can still
50- # land inside a PUT window and be erased. That is how #5533 lost its
51- # `skip-changeset` exemption for one second (15:46:44 applied, 15:46:45 erased
52- # by the labeler's PUT of `{size/m, tests}`). Closing that half needs the
53- # writes themselves to become additive, not merely better ordered; it is the
54- # open half of #5649 and no configuration here can stand in for it.
69+ # Two earlier ordering changes are RETAINED below, now as belt-and-braces
70+ # rather than as the mitigation:
71+ #
72+ # 1. `auto-label` still `needs: pr-size`. They used to be started by the
73+ # same event and overlapped exactly -- live specimen, PR #5650 run
74+ # 31051251795: `Add size label` ran 22:03:47->22:03:49 and `Label based
75+ # on changed files` ran 22:03:47->22:03:49, and the labeler's PUT emitted
76+ # `unlabeled size/s` at 22:03:49, one second after the size job added it,
77+ # for a label the labeler does not manage. Additive writes make that
78+ # overlap harmless; the edge is kept because removing it is an unrelated
79+ # change to this file's job graph and it costs one job's queue time.
80+ # 2. Neither writer runs on `labeled`/`unlabeled`. Their only input is the
81+ # diff, which a label event cannot change, so such a run could only ever
82+ # recompute the same answer. It no longer risks an erasure, but it still
83+ # buys nothing. The two event types stay in `on:` because
84+ # `changeset-check` genuinely needs them (#5580).
85+ #
86+ # ## What is still open
87+ #
88+ # This file no longer writes a whole set, but nothing MECHANICALLY stops a
89+ # future workflow, action or agent from doing so -- a seat calling the labels
90+ # endpoint with a `labels` array, or a re-introduced third-party labeler,
91+ # reopens exactly this defect with no gate to catch it. There is no repo gate
92+ # that bans the verb; until there is, this paragraph and the script's
93+ # self-test are the whole guard.
5594 # ===========================================================================
5695 pr-size :
5796 name : Check PR Size
5897 # A `labeled`/`unlabeled` event cannot change this job's input (the diff),
59- # so running it there buys nothing and costs one whole-set PUT . See above.
98+ # so running it there recomputes the same answer for a fee . See above.
6099 if : github.event.action != 'labeled' && github.event.action != 'unlabeled'
61100 runs-on : ubuntu-latest
62101 permissions :
102+ # `contents: read` is for the checkout the label writer needs. Declaring
103+ # any `permissions:` block drops every scope not listed, so it has to be
104+ # spelled even though the default token would have had it.
105+ contents : read
63106 pull-requests : write
64107
65108 steps :
66- - name : Add size label
67- uses : codelytv/pr-size-labeler@v1.10.4
68- with :
109+ - name : Checkout repository
110+ uses : actions/checkout@v7
111+
112+ # The self-test runs BEFORE the write, in the same job, so a matcher or a
113+ # plan builder that has drifted fails without touching the PR.
114+ - name : Self-test the additive label writer
115+ run : node scripts/pr-labels.mjs --self-test
116+
117+ # Every threshold and label below carries the SAME name and the SAME value
118+ # the retired `codelytv/pr-size-labeler` input had, so this replacement is
119+ # auditable value-for-value against the diff that introduced it. The
120+ # comparison is `<` and not `<=`, matching that action's labeler.sh:50-60
121+ # (`-lt`): a 10-line PR is `size/s`, not `size/xs`.
122+ #
123+ # Two of its inputs are deliberately NOT carried over:
124+ # * `fail_if_xl: 'false'` -- it selected the do-nothing branch.
125+ # * `message_if_xl` -- DEAD as this workflow configured it. labeler.sh
126+ # calls `add_label_to_pr` and only then asks
127+ # `! github::has_label "$pr_number" "$xl_label"`, i.e. it tests for the
128+ # label it has just written, so the guard is false and the comment
129+ # never posts. Reimplementing it here would be adding a comment this
130+ # repo has never actually seen, which is a feature request, not a
131+ # port. Ask for it on its own card if it is wanted.
132+ #
133+ # Unlike the action, this paginates `pulls/{n}/files` (github.sh:23 caps at
134+ # `per_page=100` and says so in its own NOTE), so a PR over 100 files is
135+ # now sized on all of them and may land a larger, correct label.
136+ - name : Add size label (additive POST, then a targeted DELETE)
137+ env :
69138 GITHUB_TOKEN : ${{ secrets.GITHUB_TOKEN }}
70- xs_label : ' size/xs '
71- xs_max_size : ' 10 '
72- s_label : ' size/s '
73- s_max_size : ' 100 '
74- m_label : ' size/m '
75- m_max_size : ' 500 '
76- l_label : ' size/l '
77- l_max_size : ' 1000 '
78- xl_label : ' size/xl '
79- fail_if_xl : ' false '
80- message_if_xl : ' This PR is very large. Consider breaking it into smaller PRs for easier review. '
81- files_to_ignore : ' pnpm-lock.yaml package-lock.json yarn.lock '
139+ PR_NUMBER : ${{ github.event.pull_request.number }}
140+ XS_LABEL : ' size/xs '
141+ XS_MAX_SIZE : ' 10 '
142+ S_LABEL : ' size/s '
143+ S_MAX_SIZE : ' 100 '
144+ M_LABEL : ' size/m '
145+ M_MAX_SIZE : ' 500 '
146+ L_LABEL : ' size/l '
147+ L_MAX_SIZE : ' 1000 '
148+ XL_LABEL : ' size/xl '
149+ FILES_TO_IGNORE : ' pnpm-lock.yaml package-lock.json yarn.lock '
150+ run : node scripts/pr-labels.mjs --size
82151
83152 auto-label :
84153 name : Auto Label
85- # ORDERING ONLY, not a dependency: this job wants `pr-size`'s PUT to be
86- # already done, so that the label set this one reads includes the size
87- # label and its own PUT carries it forward. `!cancelled()` is written out
88- # because GitHub would otherwise wrap this `if:` in an implicit `success()`
89- # -- a failed or skipped size job must not silently stop path labelling.
90- # (Same reasoning the check-workflow-status-functions gate exists to make
91- # explicit; that gate scans only `needs.*.outputs.*` reads, so this one is
92- # out of its scope and has to state its intent by hand.)
154+ # ORDERING ONLY, not a dependency: see point 1 in the header. `!cancelled()`
155+ # is written out because GitHub would otherwise wrap this `if:` in an
156+ # implicit `success()` -- a failed or skipped size job must not silently
157+ # stop path labelling. (Same reasoning the
158+ # check-workflow-status-functions gate exists to make explicit; that gate
159+ # scans only `needs.*.outputs.*` reads, so this one is out of its scope and
160+ # has to state its intent by hand.)
93161 needs : pr-size
94162 if : >-
95163 !cancelled()
@@ -104,19 +172,23 @@ jobs:
104172 - name : Checkout repository
105173 uses : actions/checkout@v7
106174
107- - name : Label based on changed files
108- uses : actions/labeler@v7.0.0
109- with :
110- repo-token : ${{ secrets.GITHUB_TOKEN }}
111- configuration-path : .github/labeler.yml
112- # Pinned at the value it already defaults to (action.yml), because a
113- # default is not a decision: an upgrade may move it, and `true` would
114- # make this step REMOVE a label of its own config whenever the globs
115- # stop matching -- on a `synchronize` that reverts a docs file, for
116- # instance. Pinning it is upgrade-drift protection and nothing more:
117- # `sync-labels` never governed foreign labels, so it is NOT the fix
118- # for the clobbering documented at the top of this file (#5649).
119- sync-labels : false
175+ # Path labels are ADD-ONLY, which is what `sync-labels: false` meant for
176+ # the retired `actions/labeler`: a label whose globs stop matching is left
177+ # alone. So this half issues POST and has no DELETE at all.
178+ #
179+ # `.github/labeler.yml` stays the single source of truth. The script
180+ # implements the minimatch subset that config actually uses -- `*`, `?`,
181+ # `**` as a whole segment, `dot: true` semantics (which is what
182+ # actions/labeler v7 defaults to) -- and REFUSES anything else with a
183+ # non-zero exit naming the offending line. The self-test parses the real
184+ # checked-in config, so a pattern that drifts outside the subset fails
185+ # there rather than silently mislabelling PRs.
186+ - name : Label based on changed files (additive POST)
187+ env :
188+ GITHUB_TOKEN : ${{ secrets.GITHUB_TOKEN }}
189+ PR_NUMBER : ${{ github.event.pull_request.number }}
190+ LABELER_CONFIG : .github/labeler.yml
191+ run : node scripts/pr-labels.mjs --paths
120192
121193 changeset-check :
122194 name : Check Changeset
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?v=4&size=40){kind=avatar}
0 commit comments