Skip to content

fix(labels): make the label sync identity locale independent - #1139

Merged
steipete merged 6 commits into
openclaw:mainfrom
dwin-gharibi:fix/label-order-determinism
Sep 1, 2026
Merged

steipete merged 6 commits into
openclaw:mainfrom
dwin-gharibi:fix/label-order-determinism

Conversation

@dwin-gharibi

@dwin-gharibi dwin-gharibi commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Related: issue 1136.

Problem and Scope

For a given target and the same effective addition/removal label values, flushIssueLabelMutationBatch could emit different issue_labels_sync identities on different runners. localeCompare follows the runner's ICU locale and can also return zero for distinct strings, leaving collator-tied labels in queue order. Both differences reach the recorded ledger business key.

Use the existing compareCodeUnits for both surviving value lists. Normalized map keys, last-operation-wins handling, exact surviving label spellings, required/optional handling, and freshness/receipt rules remain intact. Argument-list and optional fallback order intentionally become code-unit order; the existing order-sensitive assertions were updated without removing their checks.

Scope correction: this is canonical recording, not duplicate-edit prevention. The apply runner records an attempt and, after its normal guards, invokes the operation; it does not consult the business identity to suppress execution. No execution-deduplication gate, legacy-key migration, or general collision-free label encoding is added. UTF-16 ordering of label values is separate from the ledger's UTF-8 ordering of canonical object keys. The issue reference above is not an automatic-closing claim about duplicate publication.

Simplification and Ownership

The original contribution by @dwin-gharibi is preserved in branch ancestry and co-author credit. The updated proposal is 174 lines smaller, with four files instead of six. Two feature-specific installer/compiler-repair and baseline-staging wrappers are gone. One bounded producer-to-persisted-recorder executable replaces the duplicated proof suite and embedded locale program; the focused test invokes that same executable. Runtime/dependency provisioning stays with the repository's normal manifest/build tooling.

Final diff against integrated main 220ad5673ace96b2fab6473203796639c17ac710:

Surface Added / removed Net
Production +4 / -2 +2
Tests +51 / -9 +42
Executable proof +279 / -0 +279
Proof documentation +55 / -0 +55

The complete repair still adds regression/proof coverage; it is not presented as a net-negative repository change. No dependency, workflow, configuration, guard, threshold, or changelog change is introduced. OpenClaw Bay is unaffected: no observer API/schema, status, telemetry, lifecycle or control contract changes.

Real Behavior Proof

Published head: 7aa1c8ed0a0d1788ab600d31d49440266d08765f
Tree: 399cce4b4e046b15fac8f7dc07bd89b7354295c4

Surface: the committed probe invokes the real label-batch producer and real apply-ledger recorder, then reads actual spooled attempt/outcome events. Its adapter is synthetic; it does not call GitHub or substitute for a full apply-workflow execution. Reproduction and limits remain runnable from a normal build, without task-local files or a custom installer.

Scenarios and controls: two separate Node children assert their actual en-US/sv-SE locales and successful exits. Each exercises nine cases: normal, reversed, default-sorted and rotated queues; duplicate exact values; changed additions; changed removals; tied distinct labels on both sides; and reversed ties. Assertions cover exact values and arguments, golden identity, persisted business-key readback, different keys for changed effective sets, and distinct attempt/outcome IDs for repeated synthetic invocations. Each child reads 20 persisted events with 18 distinct mutation-receipt IDs from nine synthetic invocations. These counts are per child; they are not live GitHub effects.

Before/after contrast: a qualified local macOS build retained the actual label-owner module from main 220ad567; it was compared with the candidate using the same current recorder/helper modules, isolating the label-owner change rather than claiming execution of an entire older checkout. All four locale children ran on Node 24.20.0 / ICU 78.3. The old owner reproduced both locale divergence and collator-tie insertion-order divergence. The candidate produced identical observations across both locales and queue permutations.

The normal-case identities were:

Before, en-US: issue_labels_sync:321:add=Alpha|apple|äpple|zulu:remove=Beta|omega|örebro|zeta
Before, sv-SE: issue_labels_sync:321:add=Alpha|apple|zulu|äpple:remove=Beta|omega|zeta|örebro
After, both:   issue_labels_sync:321:add=Alpha|apple|zulu|äpple:remove=Beta|omega|zeta|örebro

The corresponding persisted normal-case business keys were:

Before en-US: c5995584ef0ba1269567f66787815fc0c691deeca148079bcf9ded5b01bdc669
Before sv-SE: 8398379d2aad753ab3a6dbbd20deffe524736dbf1d3123cf83bde44e118fa81f
After both:   8398379d2aad753ab3a6dbbd20deffe524736dbf1d3123cf83bde44e118fa81f

Fresh published-head AWS execution: run run_25769b696ec4, 2026-09-01, lease cbx_1ca33d2ff4cf, Linux x86_64 image ami-0461d919be7deb53c, eu-west-1, c7a.8xlarge; Node 24.20.0, pnpm 11.10.0, ICU 78.3. The current candidate ran in both verified locales with the same golden identity/business key, nine cases, 20 persisted events and 18 distinct receipt IDs per child. The pre-fix contrast above was local; it is not relabeled as an AWS baseline run.

The fresh lease had no instance role (IMDS credential endpoint 404), no Tailscale attachment, no hydration or normal workspace sync. Only CI was forwarded into the source wrapper. The exact public commit was fetched with depth 1 into fresh Git metadata; no local history, source overlay, credential or compiled baseline artifact was uploaded. The recorder children used their own canonical temporary roots and fixed synthetic producer metadata, with no inherited workflow/projection/credential context.

After checksum-verified Node bootstrap and pinned pnpm installation, the actual commands were:

pnpm install --frozen-lockfile
pnpm run build:all
node docs/proof/label-sync-identity-determinism/run-proof.mjs
node --test --test-concurrency=1 --test-reporter=tap test/label-mutation-batch.test.ts
pnpm run check

Validation and Trace

  • Fresh AWS build and proof succeeded; focused suite 14 passed, 0 failed/skipped/cancelled.
  • Fresh AWS full check: 4,240 passed, 0 failed, 8 skipped, 0 cancelled out of 4,248 tests, plus 13/13 changed-coverage script tests. Skips are one macOS-only identity check and seven unavailable Linux namespace/Landlock capability checks, not changed label cases.
  • Local qualified build, 14/14 focused tests, 2/2 existing business-key/request-attempt controls, before/after recorder contrast, narrow lint, format, syntax and diff checks passed. Earlier setup attempts rejected a noncanonical temp-root alias and an unnamespaced synthetic record path; those fixture inputs were corrected without weakening production guards. An overlapping preparation build was excluded and replaced by a stable-tree build.
  • Managed Codex reviews before commit and on the committed branch both found no accepted/actionable P0 findings. The scope is stated explicitly, not treated as a certificate at every severity.
  • Exact-head CI and CodeQL succeeded, including pnpm check, sparse repair build smoke, Windows launcher and both analyses. An incidental bot dispatch run 33475143550 was cancelled before any job and was not a failing CI/CodeQL check; its cause is unproven. No rerun or approval bypass was used, and no zero-alert claim is inferred merely from successful analyses.
Validation exit=0; source-wrapper exit=0
Full source/index inventory unchanged after validation
Probe owner and six recorder/helper module hashes unchanged after the full check
Pre-fix compiled label owner: 8ba387364870bd1a326e2ffe6d64f72b54f5ebb97fed49d8ab748280191170c2
Candidate compiled label owner (local and AWS): 3c501c295aa9b40b747e4c2b8b6cacc90231c998cceec36c88d698e370efcd16
AWS probe JSON SHA-256: f5bc2e4057ce66247341ad393f43695482aed7f2eeae38828888de72f8470a06
Evidence archive SHA-256: 4e470bc12f39135a4c61cbb45dcc9f52ea7f9772c358d78090c8ad1ff9450bc2
Source inventory SHA-256: 71f978283908a5f29aaf98356d7b4f4812949e9f21785ed71bf132b0734f1f35

The evidence was downloaded and verified, then the owned AWS lease was released; broker readback confirmed released.

The inline trace and immutable executable make the claim inspectable without relying solely on portal access. Earlier historical archives were not independently recoverable and are not used as current proof. No live GitHub mutation, model service, complete apply workflow, namespace containment, production deployment, execution deduplication or migration is claimed.

Copilot AI lite review requested due to automatic review settings August 12, 2026 11:45
@clawsweeper

clawsweeper Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 12, 2026
@clawsweeper

clawsweeper Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 1, 2026, 2:19 AM ET / 06:19 UTC.

ClawSweeper review

What this changes

The PR makes batched label-sync identities deterministic across runner locales and queue order, with regression tests and a recorder-backed proof executable.

Regression provenance

Possible regression — probable (reproduction; reviewed change). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

The PR remains necessary: current main still uses locale-sensitive sorting, while this patch replaces it with the repository’s documented total ordering and supplies current-head recorder proof. The earlier duplicate-execution claim has been explicitly scoped away, so no blocking correctness finding remains.

Priority: P2
Reviewed head: 7aa1c8ed0a0d1788ab600d31d49440266d08765f

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused production fix has strong current-head recorder evidence, clear scope limits, and regression coverage.
Proof confidence 🦞 diamond lobster (5/6) ✨ media proof bonus Sufficient (linked_artifact): The changed production owner is the label-batch ordering in src/clawsweeper-label-mutations.ts; the supplied current-head Crabbox trace runs the committed producer-to-real-ledger recorder probe in en-US and sv-SE children and observes identical identities and persisted keys after the fix. Its synthetic adapter appropriately limits the claim to recording behavior rather than live GitHub effects.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (linked_artifact): The changed production owner is the label-batch ordering in src/clawsweeper-label-mutations.ts; the supplied current-head Crabbox trace runs the committed producer-to-real-ledger recorder probe in en-US and sv-SE children and observes identical identities and persisted keys after the fix. Its synthetic adapter appropriately limits the claim to recording behavior rather than live GitHub effects.
Evidence reviewed 6 items Current main still has the defect: The fetched current-main version sorts additions and removals with default-locale localeCompare, so the identity remains locale-sensitive without this PR.
Introduced fix uses the established canonical comparator: The PR applies compareCodeUnits to both surviving lists immediately before they form the label mutation identity.
Comparator contract: compareCodeUnits is explicitly documented as a strict total UTF-16 code-unit order, avoiding both locale dependence and collator ties.
Findings None None.
Security None None.

How this fits together

ClawSweeper groups desired issue-label changes into one GitHub mutation and records its identity in the action ledger. Deterministic ordering ensures equivalent label sets produce the same recorded business key before the GitHub adapter is invoked.

flowchart LR
  A[Review decisions] --> B[Label batch]
  B --> C[Canonical code-unit ordering]
  C --> D[Label-sync identity]
  D --> E[Action ledger receipt]
  C --> F[GitHub label mutation]
Loading

Before merge

  • Complete next step (P2) - No discrete repair remains; the PR is ready for normal maintainer landing review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Change footprint 4 files; production +2 net, tests +42 net, proof/docs +334 The functional change is narrow, while the larger accompanying surface documents and exercises the deterministic ledger behavior.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #1136
Summary: This PR implements the exact ordering change proposed by the open locale-dependent label-identity issue, but it does not claim to close that issue before merge.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Land the narrow canonical-ordering repair with its current recorder proof, leaving any duplicate-execution policy work to the separate canonical issue.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main still uses default-locale sorting, and the committed probe defines a concrete locale and collator-tie reproduction path with supplied current-head after-fix observations.

Is this the best way to solve the issue?

Yes. Reusing the existing strict code-unit comparator at the identity-construction boundary is the smallest maintainable repair and preserves the existing label-set semantics.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 57b4b4496721.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is the label-batch ordering in src/clawsweeper-label-mutations.ts; the supplied current-head Crabbox trace runs the committed producer-to-real-ledger recorder probe in en-US and sv-SE children and observes identical identities and persisted keys after the fix. Its synthetic adapter appropriately limits the claim to recording behavior rather than live GitHub effects.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The changed production owner is the label-batch ordering in src/clawsweeper-label-mutations.ts; the supplied current-head Crabbox trace runs the committed producer-to-real-ledger recorder probe in en-US and sv-SE children and observes identical identities and persisted keys after the fix. Its synthetic adapter appropriately limits the claim to recording behavior rather than live GitHub effects.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove merge-risk: 🚨 automation: Current PR review selected no merge-risk labels.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: Locale-dependent ledger identity affects automation consistency but has no demonstrated outage or data-loss incident.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The changed production owner is the label-batch ordering in src/clawsweeper-label-mutations.ts; the supplied current-head Crabbox trace runs the committed producer-to-real-ledger recorder probe in en-US and sv-SE children and observes identical identities and persisted keys after the fix. Its synthetic adapter appropriately limits the claim to recording behavior rather than live GitHub effects.
  • proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is the label-batch ordering in src/clawsweeper-label-mutations.ts; the supplied current-head Crabbox trace runs the committed producer-to-real-ledger recorder probe in en-US and sv-SE children and observes identical identities and persisted keys after the fix. Its synthetic adapter appropriately limits the claim to recording behavior rather than live GitHub effects.

Evidence

What I checked:

  • Current main still has the defect: The fetched current-main version sorts additions and removals with default-locale localeCompare, so the identity remains locale-sensitive without this PR. (src/clawsweeper-label-mutations.ts:238, 57b4b4496721)
  • Introduced fix uses the established canonical comparator: The PR applies compareCodeUnits to both surviving lists immediately before they form the label mutation identity. (src/clawsweeper-label-mutations.ts:240, 7aa1c8ed0a0d)
  • Comparator contract: compareCodeUnits is explicitly documented as a strict total UTF-16 code-unit order, avoiding both locale dependence and collator ties. (src/stable-json.ts:47, 7aa1c8ed0a0d)
  • Regression coverage and persisted-recorder proof: The focused test covers reversed input and collator-tied label strings, then invokes the committed probe; the probe drives the real batch producer and apply-ledger recorder and verifies persisted attempt/outcome records. (test/label-mutation-batch.test.ts:462, 7aa1c8ed0a0d)
  • Feature-history routing: History identifies the earlier batched-label publication work in this module; the parent needed to verify exact line introduction was unavailable locally, so this is routing evidence rather than an authorship claim. (src/clawsweeper-label-mutations.ts:238, e10a866ba75f)
  • Current-head real behavior evidence: The supplied PR body records a fresh Crabbox execution of head 7aa1c8e on Node 24.20.0 in verified en-US and sv-SE children, observing identical candidate identities and persisted business keys across nine scenarios per child. (docs/proof/label-sync-identity-determinism/run-proof.mjs:187, 7aa1c8ed0a0d)

Likely related people:

  • Martin Cleary: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (9 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-12T12:19:28.853Z sha 1630b5c :: needs real behavior proof before merge. :: [P1] Gate the GitHub edit on the idempotency identity
  • reviewed 2026-08-12T13:03:15.158Z sha f6071b7 :: needs real behavior proof before merge. :: [P1] Gate the GitHub edit before closing the duplicate-prevention issue
  • reviewed 2026-08-12T23:32:11.187Z sha f6071b7 :: found issues before merge. :: [P1] Reconcile the closing claim with the actual enforcement boundary
  • reviewed 2026-08-18T09:31:03.668Z sha f6071b7 :: found issues before merge. :: [P1] Align the closing claim with the enforcement boundary
  • reviewed 2026-08-19T06:12:02.873Z sha 2518cf2 :: found issues before merge. :: [P1] Align the duplicate-edit claim with the enforcement boundary
  • reviewed 2026-08-25T13:16:40.058Z sha 2518cf2 :: found issues before merge. :: [P1] Correct the duplicate-edit claim
  • reviewed 2026-09-01T04:24:28.562Z sha 2518cf2 :: found issues before merge. :: [P1] Correct the duplicate-prevention claim
  • reviewed 2026-09-01T05:49:20.861Z sha 7aa1c8e :: needs real behavior proof before merge. :: [P1] Do not close the duplicate-edit issue without a gate

@dwin-gharibi

Copy link
Copy Markdown
Contributor Author

@steipete

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. proof: sufficient Contributor real behavior proof is sufficient. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 12, 2026
@steipete

Copy link
Copy Markdown
Contributor

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

dwin-gharibi and others added 3 commits August 19, 2026 09:34
Use the existing compareCodeUnits comparator for surviving addition and
removal values so recorded label identities do not depend on locale or
collator ties. Preserve normalized batch, fallback, freshness and receipt
semantics. Recording identity remains separate from execution policy;
repeated attempts still retain distinct receipts.

Remove redundant proof provisioning and staging wrappers. Share one
producer-to-persisted-recorder probe between the focused test and proof
commands, with explicit baseline inputs and isolated verified locales.

Local validation: 14 focused tests and 2 ledger controls passed, along with
the qualified four-child baseline/candidate recorder probe and narrow
checks. Remote/full validation remains pending.

Ref: openclaw#1136

Co-authored-by: Dwin Gharibi <dwin.gharibi@email.kntu.ac.ir>
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. proof: sufficient Contributor real behavior proof is sufficient. labels Sep 1, 2026
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Sep 1, 2026
@steipete
steipete merged commit d887e65 into openclaw:main Sep 1, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants