Skip to content

fix: skip stale label sync for newer review comments#417

Merged
steipete merged 3 commits into
openclaw:mainfrom
brokemac79:codex/fix-ready-label-reconciliation
Jul 5, 2026
Merged

fix: skip stale label sync for newer review comments#417
steipete merged 3 commits into
openclaw:mainfrom
brokemac79:codex/fix-ready-label-reconciliation

Conversation

@brokemac79

@brokemac79 brokemac79 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Prevent stale stored PR reports from removing ClawSweeper-owned PR readiness labels when the live durable review comment contains a newer review marker.
  • Keep the existing stale-head cleanup behavior when the durable comment is not newer than the report.
  • Add regression coverage for a stale report plus newer durable comment carrying status: 👀 ready for maintainer look.

Problem

Live intake for openclaw/openclaw#97259 showed a persistent mismatch: the latest durable ClawSweeper comment still says the PR is ready for maintainer review, but the status: 👀 ready for maintainer look label is absent.

Evidence from GitHub on 2026-07-05:

  • Latest ClawSweeper review comment: fix: A2A handoffs can duplicate message-tool replies openclaw#97259 (comment), updated 2026-07-04T21:34:33Z.
  • That comment includes Result: ready for maintainer review. and a label justification for status: 👀 ready for maintainer look.
  • Label timeline: ClawSweeper added status: 👀 ready for maintainer look at 2026-07-04T21:28:04Z, then removed it at 2026-07-05T01:43:37Z.
  • Current live labels no longer include the ready label.
  • Current relevant checks on the PR head are green, including latest Real behavior proof and auto-response checks.

State evidence:

  • origin/state:records/openclaw-openclaw/items/97259.md is stale relative to the live durable comment: report reviewed_at is 2026-06-27T20:56:58.627Z and pull_head_sha is da1a789..., while the live comment marker is reviewed on 2026-07-04 for head 0a88f037....
  • The state report records labels_synced_at: 2026-07-05T01:43:37.636Z and labels: ["gateway","agents","size: L","extensions: qa-lab","P1"], matching the later label removal.
  • The apply path already detects the newer durable comment through staleReviewCommentSyncReason, but only skipped early for close proposals. For keep-open PRs it could remove stale labels first and only later skip comment sync, creating the observed contradiction.

Related PR check:

Implementation

  • Broaden the existing staleReviewCommentSyncReason guard so open PR apply processing skips before any label mutation whenever the live durable review marker is newer than the local report.
  • Add a regression test that models a stale report, a newer durable comment on the live head, and live ready/merge-risk/proof labels. The test asserts no gh issue edit label removal and no comment downgrade occur.

Validation

Passed:

  • pnpm install --frozen-lockfile
  • pnpm run build
  • node --test test/apply-label-sync.test.ts (13/13 pass)
  • pnpm run lint:src
  • pnpm run lint:scripts
  • pnpm exec oxfmt --check src\clawsweeper.ts test\apply-label-sync.test.ts
  • git diff --check
  • pnpm run build:all

Broader lane attempted:

  • pnpm run test:unit ran 670 tests: 663 passed, 1 skipped, 6 failed due this Windows host environment.
  • Four failures are test/dispatch-receipt-owner.test.ts invoking bash, which resolves through WSL and fails because this host's default WSL distro lacks /bin/bash.
  • Two failures are test/local-range-review.test.ts expecting fake Codex shim output files that were not produced in this Windows run.
  • The focused apply-label sync coverage passed both separately and inside this broader run.

Proof

The regression test exercises the exact failure ordering:

  1. Stored report has an old pull_head_sha and stale ClawSweeper-owned labels.
  2. Live PR head is newer.
  3. Existing durable ClawSweeper comment has a newer clawsweeper-verdict marker.
  4. Apply returns skipped_stale_review_comment_sync before label mutation.
  5. No issue edit --remove-label call is made and no stale review comment is patched over the newer durable comment.

This preserves the existing stale-head behavior when the durable comment is not newer; the adjacent existing test still removes stale labels and patches the comment to Codex review: stale review; fresh review needed.

Docker-Backed Local Container Proof

I reran the focused regression and the apply replay inside Docker-backed Crabbox local-container, using the Playwright Linux image from the project proof instructions.

COMMAND=C:\Users\marti\.local\bin\crabbox.exe run --provider local-container --local-container-image mcr.microsoft.com/playwright:v1.60.0-noble --local-container-memory 8g --no-hydrate --timing-json --script-stdin
PROVIDER=local-container
LEASE=cbx_53731180c485
SLUG=jade-shrimp
IMAGE=mcr.microsoft.com/playwright:v1.60.0-noble
EXIT_CODE=0
PROOF_TARGET=openclaw/openclaw#97259
PROOF_SHA=385a0791873321f6840afbb424a8962fec0f7f4d
NODE_VERSION=v24.15.0
PNPM_VERSION=10.33.2

CRABBOX_PHASE:install
pnpm install --frozen-lockfile

CRABBOX_PHASE:build
pnpm run build

CRABBOX_PHASE:focused_regression_test
node --test test/apply-label-sync.test.ts
RESULT=13 tests, 13 pass, 0 fail

CRABBOX_PHASE:apply_replay_97259
[apply] 2026-07-05T12:35:58.008Z starting apply: files=1 dry_run=false apply_kind=issue sync_comments_only=true item_numbers=97259
[apply] 2026-07-05T12:35:58.256Z finished apply closed=0/10 processed=1/1 counts={"skipped_stale_review_comment_sync":1}

APPLY_REPORT:
[
  {
    "number": 97259,
    "action": "skipped_stale_review_comment_sync",
    "reason": "live durable review comment is newer than the local report: comment reviewed_at=2026-07-04T21:34:33.000Z, report reviewed_at=2026-06-27T20:56:58.627Z"
  }
]

GH_CALLS:
["api","repos/openclaw/openclaw/issues/97259","--jq","{number,title,html_url,created_at,updated_at,closed_at,state,locked,active_lock_reason,author_association,user:{login:.user.login},labels:[.labels[].name],pull_request:(.pull_request // null)}"]
["api","repos/openclaw/openclaw/issues/97259"]
["api","repos/openclaw/openclaw/issues/97259/comments?per_page=100","--paginate","--slurp"]
["api","-i","repos/openclaw/openclaw/issues/97259/timeline?per_page=100&page=1"]
["api","repos/openclaw/openclaw/pulls/97259"]
["api","repos/openclaw/openclaw/pulls/97259/files?per_page=100&page=1"]
["api","repos/openclaw/openclaw/pulls/97259/commits?per_page=100&page=1"]
["api","repos/openclaw/openclaw/issues/97259/comments?per_page=100","--paginate","--slurp"]

ASSERT_ACTION=skipped_stale_review_comment_sync
ASSERT_MUTATION_CALLS=[]
ASSERT_LABELS_PRESERVED=labels: ["gateway","agents","size: L","extensions: qa-lab","P1","rating: 🦞 diamond lobster","merge-risk: 🚨 message-delivery","status: 👀 ready for maintainer look","proof: sufficient"]

TIMING_JSON={"provider":"local-container","leaseId":"cbx_53731180c485","slug":"jade-shrimp","syncSkipped":false,"commandPhases":[{"name":"toolchain","ms":2170},{"name":"install","ms":2578},{"name":"build","ms":776},{"name":"focused_regression_test","ms":5543},{"name":"apply_replay_97259","ms":396}],"exitCode":0,"leaseStopped":true}

This is stronger than a screenshot for this change because the changed behavior is a CLI mutation boundary, not a visual UI. The proof exercises the compiled public apply entrypoint on Linux, against a #97259-shaped stale report and a newer durable review marker. The fake gh shim records every GitHub boundary call; the assertion proves there was no gh issue edit label mutation and no durable comment patch while the ready/proof/merge-risk labels stayed preserved in the report.

Risks / Rollout

  • Low behavioral risk: this only suppresses label/comment mutation when ClawSweeper already has a newer durable review marker than the state report.
  • The tradeoff is intentionally conservative: labels may remain temporarily stale until state catches up, but apply will no longer make labels contradict a newer public review comment.
  • No workflow files changed, so there is no GitHub OAuth/workflow-scope risk.

Links

Codex Review Closeout

Target reviewed:

  • PR head: 385a0791873321f6840afbb424a8962fec0f7f4d
  • PR base: origin/main at 44d3b656300f647a20e1c82a48e84f8ad24225f9
  • Changed files: src/clawsweeper.ts, test/apply-label-sync.test.ts

Review commands:

C:\msys64\usr\bin\bash.exe C:\Users\marti\.codex\skills\codex-review\scripts\codex-review --mode branch --base origin/main --parallel-tests "node --test test/apply-label-sync.test.ts" --dry-run
# selected: codex review --base origin/main

codex review -c service_tier='"fast"' --base origin/main
# exit: 0, elapsed: 153s

The full helper parallel run was not used as the final run on this Windows host because its test child invokes bash -lc, which resolved through the broken WSL shim, and the local Codex config contains an unsupported service_tier = "default" value for this CLI. The final review kept the same branch/base target and used the supported service_tier="fast" override; no model override was used.

Codex review result:

The change safely broadens stale durable review-comment protection to open PR keep-open reports, preventing stale label/comment mutations. Focused build and apply-label-sync tests passed.

Accepted findings: none.

Rejected findings: none. One [P1] string appears in the raw review log only because the diff includes a test fixture line (Old finding - src/runtime.ts:10); it is not a Codex review finding.

Focused validation rerun after review:

node --test test/apply-label-sync.test.ts
# 13 tests, 13 pass, 0 fail

@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 5, 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 repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Codex review: stale review; fresh review needed.

Summary
The latest durable ClawSweeper review was for head 2108bc9711330f132f24c00cd571db5c46c6c0f4, but the PR head is now 92ce1b8b30948fb7c495afca1880e269053f8499. Its old verdict and PR readiness labels are no longer current.

Next step
Run or wait for a fresh ClawSweeper review on the current PR head.

Review history (9 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-05T11:28:55.001Z sha 385a079 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T11:33:05.713Z sha 385a079 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T11:37:54.390Z sha 385a079 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T12:40:40.563Z sha 385a079 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T12:45:34.303Z sha 385a079 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T12:49:45.083Z sha 385a079 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T13:05:18.356Z sha 385a079 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T13:09:37.318Z sha 385a079 :: needs maintainer review before merge. :: none

@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. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jul 5, 2026
@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. 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. labels Jul 5, 2026
@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

1 similar comment
@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@steipete steipete force-pushed the codex/fix-ready-label-reconciliation branch from 385a079 to 2108bc9 Compare July 5, 2026 16:51
@clawsweeper clawsweeper Bot removed merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. proof: sufficient Contributor real behavior proof is sufficient. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Jul 5, 2026
@steipete steipete merged commit 181d8d0 into openclaw:main Jul 5, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 Urgent regression or broken agent/channel workflow affecting real users now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants