-
Notifications
You must be signed in to change notification settings - Fork 294
The issue_labels_sync idempotency key varies by runner locale #1136
Copy link
Copy link
Open
Labels
P2Normal priority bug or improvement with limited blast radius.Normal priority bug or improvement with limited blast radius.clawsweeper:linked-pr-openClawSweeper found an open linked pull request for this issue.ClawSweeper found an open linked pull request for this issue.clawsweeper:no-new-fix-prClawSweeper does not recommend queueing a new automated fix PR for this issue.ClawSweeper does not recommend queueing a new automated fix PR for this issue.clawsweeper:source-reproClawSweeper found a high-confidence source-level issue reproduction.ClawSweeper found a high-confidence source-level issue reproduction.impact:otherThis issue has meaningful maintainer-visible impact outside the owned taxonomy.This issue has meaningful maintainer-visible impact outside the owned taxonomy.issue-rating: 🦞 diamond lobsterVery strong issue quality with high-confidence source-level or clear reproduction.Very strong issue quality with high-confidence source-level or clear reproduction.
Description
Metadata
Metadata
Assignees
Labels
P2Normal priority bug or improvement with limited blast radius.Normal priority bug or improvement with limited blast radius.clawsweeper:linked-pr-openClawSweeper found an open linked pull request for this issue.ClawSweeper found an open linked pull request for this issue.clawsweeper:no-new-fix-prClawSweeper does not recommend queueing a new automated fix PR for this issue.ClawSweeper does not recommend queueing a new automated fix PR for this issue.clawsweeper:source-reproClawSweeper found a high-confidence source-level issue reproduction.ClawSweeper found a high-confidence source-level issue reproduction.impact:otherThis issue has meaningful maintainer-visible impact outside the owned taxonomy.This issue has meaningful maintainer-visible impact outside the owned taxonomy.issue-rating: 🦞 diamond lobsterVery strong issue quality with high-confidence source-level or clear reproduction.Very strong issue quality with high-confidence source-level or clear reproduction.
Type
Fields
Priority
None yet
Summary
flushIssueLabelMutationBatchinsrc/clawsweeper-label-mutations.tssorts thebatched additions and removals with
String.prototype.localeCompareand joins theminto the mutation identity the action ledger dedupes on:
localeCompareis locale-sensitive, so the identity depends on the ICUconfiguration of the machine that produced it.
Impact
The identity is an idempotency key. Two spellings of the same key are two different
keys, so a label sync that has already been applied is not recognized as a repeat
and can be published again.
Observed against the shipped module, same label set and same batching:
There is a second, locale-independent failure in the same line.
localeComparereturns
0for strings a collator treats as equivalent but that are not equal, soit is not a total order. GitHub permits emoji in label names, and two names
differing only by a zero-width joiner tie:
Array.prototype.sorttherefore leaves them in whatever order they were queued, andthe identity follows the queue rather than the label set — on a single runner, with
no locale involved.
Reproduction
Queue the same label set in two different orders and compare the published
identity, or run the same set under twoLC_ALLvalues. Both are covered bydocs/proof/label-sync-identity-determinism/run-proof.mjs.Proposed solution
Sort with
compareCodeUnitsfromsrc/stable-json.ts:Code-unit order is a total order and is fully specified, so the key depends only on
the label set. It is also the ordering the action ledger already uses for its
canonical JSON, which keeps the two consistent.
Expected fallout
Code-unit order puts uppercase before lowercase, so
P2sorts beforeimpact:message-loss. That changes the order of names inside the--add-labeland--remove-labelarguments. GitHub treats those as sets, so the resulting labelstate is unchanged, but three assertions in
test/label-mutation-batch.test.tspin the current order and need updating:
an exact-publication label batch emits one combined deterministic issue editlabel definition discovery is cached across item batchesoptional batch failures retain successful final operations and report skipped additionsIn the third, the per-label retry order also flips, which moves where the receipt
lands.
result.skippedAdditionsis unaffected.