From 724d0ed946feb4e79c00d94c9d4e22621faaf581 Mon Sep 17 00:00:00 2001 From: brokemac79 Date: Sun, 5 Jul 2026 12:20:06 +0100 Subject: [PATCH 1/3] fix: skip stale label sync for newer review comments --- src/clawsweeper.ts | 2 +- test/apply-label-sync.test.ts | 182 ++++++++++++++++++++++++++++++++++ 2 files changed, 183 insertions(+), 1 deletion(-) diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 18778a86c8..801caaf0f6 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -18492,7 +18492,7 @@ async function applyDecisionsCommand(args: Args): Promise { existingReviewComment, number, ); - if (state === "open" && isCloseProposal && staleReviewCommentReason) { + if (state === "open" && staleReviewCommentReason) { markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); if (!dryRun) writeReportMarkdown(path, markdown); results.push({ diff --git a/test/apply-label-sync.test.ts b/test/apply-label-sync.test.ts index c8b98a4604..640405afb1 100644 --- a/test/apply-label-sync.test.ts +++ b/test/apply-label-sync.test.ts @@ -479,6 +479,188 @@ if (args[0] === "api" && /\\/issues\\/74481$/.test(path)) { } }); +test("apply-decisions skips stale label cleanup when the durable review comment is newer", () => { + const root = mkdtempSync(tmpPrefix); + try { + const itemsDir = join(root, "items"); + const closedDir = join(root, "closed"); + const plansDir = join(root, "plans"); + const reportPath = join(root, "apply-report.json"); + const logPath = join(root, "gh.log"); + const itemPath = join(itemsDir, "74483.md"); + mkdirSync(itemsDir, { recursive: true }); + mkdirSync(plansDir, { recursive: true }); + const readyLabel = "status: \u{1F440} ready for maintainer look"; + const mergeRiskLabel = "merge-risk: \u{1F6A8} message-delivery"; + const staleLabels = [ + "P1", + "rating: \u{1F99E} diamond lobster", + mergeRiskLabel, + readyLabel, + "proof: sufficient", + ]; + writeFileSync( + itemPath, + `${reportFrontMatter({ + repository: "openclaw/openclaw", + type: "pull_request", + number: "74483", + title: "Stale report with newer durable comment", + url: "https://github.com/openclaw/openclaw/pull/74483", + decision: "keep_open", + close_reason: "none", + confidence: "high", + action_taken: "kept_open", + review_status: "complete", + local_checkout_access: "verified", + author: "contributor", + author_association: "CONTRIBUTOR", + labels: JSON.stringify(staleLabels), + item_snapshot_hash: "snapshot-a", + item_updated_at: "2026-05-19T20:00:00Z", + reviewed_at: "2026-05-19T20:00:00.000Z", + pull_head_sha: "old-head", + merge_risk_labels: JSON.stringify([mergeRiskLabel]), + })} + +## Summary + +This stored report is stale because a later review already updated the durable comment. + +${realBehaviorProofReportSection()} + +${prRatingReportSection({ overallTier: "A", proofTier: "A", patchTier: "A" })} + +## Review Findings + +Overall correctness: patch is correct + +Overall confidence: 0.9 + +Full review comments: + +- none +`, + "utf8", + ); + + const newerComment = [ + "Codex review: needs maintainer review before merge.", + "", + "", + "", + ].join("\n"); + const ghMock = ` +const { appendFileSync, readFileSync } = require("fs"); +const logPath = ${JSON.stringify(logPath)}; +const newerComment = ${JSON.stringify(newerComment)}; +const staleLabels = ${JSON.stringify(staleLabels)}; +const rawArgs = process.argv.slice(2); +const args = rawArgs[0] === "--repo" ? rawArgs.slice(2) : rawArgs; +appendFileSync(logPath, JSON.stringify(args) + "\\n"); +const path = args[1] || ""; +if (args[0] === "api" && /\\/issues\\/74483$/.test(path)) { + console.log(JSON.stringify({ + number: 74483, + title: "Stale report with newer durable comment", + html_url: "https://github.com/openclaw/openclaw/pull/74483", + created_at: "2026-05-19T19:00:00Z", + updated_at: "2026-05-19T20:10:00Z", + closed_at: null, + state: "open", + locked: false, + active_lock_reason: null, + author_association: "CONTRIBUTOR", + user: { login: "contributor" }, + labels: staleLabels, + pull_request: {} + })); +} else if (args[0] === "api" && args[1] === "-i" && /\\/issues\\/74483\\/timeline(?:\\?|$)/.test(args[2] || "")) { + console.log("HTTP/2 200\\n\\n[]"); +} else if (args[0] === "api" && /\\/issues\\/74483\\/timeline(?:\\?|$)/.test(path)) { + console.log(JSON.stringify([[]])); +} else if (args[0] === "api" && /\\/pulls\\/74483$/.test(path)) { + console.log(JSON.stringify({ + number: 74483, + html_url: "https://github.com/openclaw/openclaw/pull/74483", + state: "open", + changed_files: 1, + commits: 2, + review_comments: 0, + head: { sha: "new-head", ref: "branch", repo: { full_name: "fork/openclaw" } }, + base: { sha: "base-sha", ref: "main", repo: { full_name: "openclaw/openclaw" } }, + user: { login: "contributor" } + })); +} else if (args[0] === "api" && /\\/pulls\\/74483\\/(files|commits|comments)(?:\\?|$)/.test(path)) { + console.log(JSON.stringify([[]])); +} else if (args[0] === "api" && /\\/issues\\/74483\\/comments(?:\\?|$)/.test(path)) { + console.log(JSON.stringify([[ + { + id: 987483, + html_url: "https://github.com/openclaw/openclaw/pull/74483#issuecomment-987483", + body: newerComment, + user: { login: "clawsweeper[bot]" }, + created_at: "2026-05-20T00:00:00Z", + updated_at: "2026-05-20T00:00:00Z" + } + ]])); +} else if (args[0] === "api" && /\\/issues\\/comments\\/987483$/.test(path)) { + const input = args[args.indexOf("--input") + 1]; + appendFileSync(logPath, JSON.stringify(["patched-review-body", JSON.parse(readFileSync(input, "utf8")).body]) + "\\n"); + console.log(JSON.stringify({ + id: 987483, + html_url: "https://github.com/openclaw/openclaw/pull/74483#issuecomment-987483", + updated_at: "2026-05-20T00:01:00Z" + })); +} else if (args[0] === "issue" && args[1] === "edit") { + console.log(""); +} else { + console.error("unexpected gh args", JSON.stringify(args)); + process.exit(1); +} +`; + withMockGh(root, ghMock, () => { + runApplyDecisionsForTest({ + targetRepo: "openclaw/openclaw", + itemsDir, + closedDir, + plansDir, + reportPath, + extraArgs: ["--sync-comments-only", "--item-numbers", "74483"], + }); + }); + + const calls = readFileSync(logPath, "utf8") + .trim() + .split("\n") + .filter(Boolean) + .map((line) => JSON.parse(line) as string[]); + assert.deepEqual( + calls.filter((args) => args[0] === "issue" && args[1] === "edit"), + [], + ); + assert.equal( + calls.some((args) => args[0] === "patched-review-body"), + false, + ); + const updatedReport = readFileSync(itemPath, "utf8"); + assert.ok(updatedReport.includes(`labels: ${JSON.stringify(staleLabels)}`)); + const result = JSON.parse(readFileSync(reportPath, "utf8")) as Array<{ + number: number; + action: string; + reason: string; + }>; + assert.equal(result[0]?.number, 74483); + assert.equal(result[0]?.action, "skipped_stale_review_comment_sync"); + assert.match( + result[0]?.reason ?? "", + /live durable review comment is newer than the local report/, + ); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + test("apply-decisions syncs fresh-head PR labels after a command-only re-review comment", () => { const root = mkdtempSync(tmpPrefix); try { From 2108bc9711330f132f24c00cd571db5c46c6c0f4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 5 Jul 2026 17:51:31 +0100 Subject: [PATCH 2/3] fix: validate fresh review head before label sync --- src/clawsweeper.ts | 24 +++++++++++++++++------- test/apply-label-sync.test.ts | 8 ++++++-- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 801caaf0f6..67d06f9334 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -16058,9 +16058,10 @@ function commentBody(comment: Record | undefined): string | und return typeof body === "string" ? body : undefined; } -function newestReviewMarkerReviewedAt( +function newestReviewMarkerAttribute( comment: Record | undefined, number: number, + attribute: "reviewed_at" | "sha", ): string | undefined { if (!canPatchReviewComment(comment)) return undefined; const body = commentBody(comment); @@ -16075,25 +16076,33 @@ function newestReviewMarkerReviewedAt( .match(/(?:\s*)+$/)?.[0]; if (!markerBlock) return undefined; const markerPattern = //g; - let lastReviewedAt: string | undefined; + let lastValue: string | undefined; for (const match of markerBlock.matchAll(markerPattern)) { const attributes = match[1] ?? ""; if (!new RegExp(`\\bitem=${number}\\b`).test(attributes)) continue; - const reviewedAt = attributes.match(/\breviewed_at=([^\s>]+)/)?.[1]; - if (!reviewedAt || reviewedAt === "unknown") continue; - lastReviewedAt = reviewedAt; + const value = attributes.match(new RegExp(`\\b${attribute}=([^\\s>]+)`))?.[1]; + if (!value || value === "unknown") continue; + lastValue = value; } - return lastReviewedAt; + return lastValue; } function staleReviewCommentSyncReason( markdown: string, existingReviewComment: Record | undefined, number: number, + context: ItemContext, ): string | null { if (frontMatterValue(markdown, "type") !== "pull_request") return null; // Comment updated_at can move for command/status edits; only review markers prove verdict freshness. - const liveReviewedAt = newestReviewMarkerReviewedAt(existingReviewComment, number); + const liveReviewedAt = newestReviewMarkerAttribute(existingReviewComment, number, "reviewed_at"); + const liveReviewedSha = newestReviewMarkerAttribute(existingReviewComment, number, "sha"); + const currentHeadSha = pullHeadShaFromContext(context); + const reportHeadSha = pullHeadShaFromReport(markdown); + if (!liveReviewedSha || !currentHeadSha) return null; + // Trust a newer comment when it matches the live head, or when the live API still reports the + // report's head and may be lagging the comment. Otherwise the newer comment is stale too. + if (liveReviewedSha !== currentHeadSha && currentHeadSha !== reportHeadSha) return null; const reportReviewedAt = frontMatterValue(markdown, "reviewed_at"); const liveReviewedAtMs = timestampMs(liveReviewedAt); const reportReviewedAtMs = timestampMs(reportReviewedAt); @@ -18491,6 +18500,7 @@ async function applyDecisionsCommand(args: Args): Promise { markdown, existingReviewComment, number, + currentItemContext(), ); if (state === "open" && staleReviewCommentReason) { markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); diff --git a/test/apply-label-sync.test.ts b/test/apply-label-sync.test.ts index 640405afb1..9e234dfb85 100644 --- a/test/apply-label-sync.test.ts +++ b/test/apply-label-sync.test.ts @@ -356,11 +356,15 @@ Full review comments: `; const synced = reportWithSyncedReviewComment(sourceReport, 74481); writeFileSync(itemPath, synced.report, "utf8"); + const newerStaleHeadComment = synced.comment.replace( + /\breviewed_at=[^\s>]+/, + "reviewed_at=2026-05-20T00:00:00.000Z", + ); const ghMock = ` const { appendFileSync, readFileSync } = require("fs"); const logPath = ${JSON.stringify(logPath)}; -const comment = ${JSON.stringify(synced.comment)}; +const comment = ${JSON.stringify(newerStaleHeadComment)}; const rawArgs = process.argv.slice(2); const args = rawArgs[0] === "--repo" ? rawArgs.slice(2) : rawArgs; appendFileSync(logPath, JSON.stringify(args) + "\\n"); @@ -461,7 +465,7 @@ if (args[0] === "api" && /\\/issues\\/74481$/.test(path)) { assert.match(patchedBody, /clawsweeper-review-history v=1 total=1/); assert.match( patchedBody, - /- reviewed 2026-05-19T20:00:00Z sha old-head :: needs maintainer review before merge\./, + /- reviewed 2026-05-20T00:00:00\.000Z sha old-head :: needs maintainer review before merge\./, ); assert.doesNotMatch(patchedBody, /clawsweeper-verdict:/); const updatedReport = readFileSync(itemPath, "utf8"); From 92ce1b8b30948fb7c495afca1880e269053f8499 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 5 Jul 2026 17:54:35 +0100 Subject: [PATCH 3/3] fix: avoid loading PR context for issues --- src/clawsweeper.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 67d06f9334..4e9921ad5f 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -18496,12 +18496,15 @@ async function applyDecisionsCommand(args: Args): Promise { if (existingReviewCommentUpdatedAt) { allowedSelfMutationUpdatedAts.add(existingReviewCommentUpdatedAt); } - const staleReviewCommentReason = staleReviewCommentSyncReason( - markdown, - existingReviewComment, - number, - currentItemContext(), - ); + const staleReviewCommentReason = + item.kind === "pull_request" + ? staleReviewCommentSyncReason( + markdown, + existingReviewComment, + number, + currentItemContext(), + ) + : null; if (state === "open" && staleReviewCommentReason) { markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); if (!dryRun) writeReportMarkdown(path, markdown);