From ca43c93fe837c0b008c8a4054512b40961cda77a Mon Sep 17 00:00:00 2001 From: PollyBot13 Date: Sat, 1 Aug 2026 19:43:17 +0200 Subject: [PATCH] fix(review): suppress owned activity churn --- src/clawsweeper-apply-decision-workflow.ts | 10 +++- src/clawsweeper-record-metadata.ts | 2 + src/clawsweeper-review-planning.ts | 5 ++ src/clawsweeper-types.ts | 1 + src/pr-close-coverage-proof.ts | 16 ++++++- src/repair/record-tuple.ts | 2 +- src/review-structural-cache.ts | 2 + src/scheduler-policy.ts | 2 + test/apply-label-sync.test.ts | 4 +- test/pr-close-coverage-proof.test.ts | 33 +++++++++++++ test/review-structural-cache.test.ts | 10 ++++ test/scheduler-policy.test.ts | 54 ++++++++++++++++++++++ 12 files changed, 137 insertions(+), 4 deletions(-) diff --git a/src/clawsweeper-apply-decision-workflow.ts b/src/clawsweeper-apply-decision-workflow.ts index dd73b785b2..1477b4b3b2 100644 --- a/src/clawsweeper-apply-decision-workflow.ts +++ b/src/clawsweeper-apply-decision-workflow.ts @@ -1926,7 +1926,14 @@ export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWor ) : recordReviewGuardSkip("skipped_stale_review_comment_sync", reason); const rememberSelfMutationUpdatedAt = (): void => { - if (!dryRun) allowedSelfMutationUpdatedAts.add(fetchItem(number).item.updatedAt); + if (dryRun) return; + const automationItemUpdatedAt = fetchItem(number).item.updatedAt; + allowedSelfMutationUpdatedAts.add(automationItemUpdatedAt); + markdown = replaceFrontMatterValue( + markdown, + "automation_item_updated_at", + automationItemUpdatedAt, + ); }; let cachedPrCloseCoverageProofGateResult: PrCloseCoverageProofGateResult | undefined; let cachedAuthorPrBudgetApplyGate: AuthorPrBudgetApplyGate | undefined; @@ -3685,6 +3692,7 @@ export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWor if (syncedCommentUpdatedAt) { allowedSelfMutationUpdatedAts.add(syncedCommentUpdatedAt); } + rememberSelfMutationUpdatedAt(); syncReasons.push("updated durable Codex review comment"); // The durable review comment is now published, so stale "review // started" placeholders from failed earlier attempts are clutter. diff --git a/src/clawsweeper-record-metadata.ts b/src/clawsweeper-record-metadata.ts index 7f3858ab63..3e39acad97 100644 --- a/src/clawsweeper-record-metadata.ts +++ b/src/clawsweeper-record-metadata.ts @@ -385,6 +385,7 @@ export function createRecordMetadata({ markdown, reviewedAt: frontMatterValue(markdown, "reviewed_at"), itemUpdatedAt: frontMatterValue(markdown, "item_updated_at"), + automationItemUpdatedAt: frontMatterValue(markdown, "automation_item_updated_at"), reviewCommentSyncedAt: frontMatterValue(markdown, "review_comment_synced_at"), labelsSyncedAt: frontMatterValue(markdown, "labels_synced_at"), decision: frontMatterValue(markdown, "decision"), @@ -416,6 +417,7 @@ export function createRecordMetadata({ markdown, reviewedAt: frontMatterValue(markdown, "reviewed_at"), itemUpdatedAt: frontMatterValue(markdown, "item_updated_at"), + automationItemUpdatedAt: frontMatterValue(markdown, "automation_item_updated_at"), reviewCommentSyncedAt: frontMatterValue(markdown, "review_comment_synced_at"), labelsSyncedAt: frontMatterValue(markdown, "labels_synced_at"), decision: frontMatterValue(markdown, "decision"), diff --git a/src/clawsweeper-review-planning.ts b/src/clawsweeper-review-planning.ts index 2728c0ba46..50b6dd886c 100644 --- a/src/clawsweeper-review-planning.ts +++ b/src/clawsweeper-review-planning.ts @@ -252,6 +252,9 @@ export function createReviewPlanning({ const reviewedAt = review.reviewedAt ? Date.parse(review.reviewedAt) : Number.NaN; if (!Number.isFinite(updatedAt) || !Number.isFinite(reviewedAt)) return true; if (review.itemUpdatedAt && item.updatedAt === review.itemUpdatedAt) return false; + if (review.automationItemUpdatedAt && item.updatedAt === review.automationItemUpdatedAt) { + return false; + } if (updatedAt <= reviewedAt) return false; const reviewCommentSyncedAt = review.reviewCommentSyncedAt ? Date.parse(review.reviewCommentSyncedAt) @@ -304,6 +307,7 @@ export function createReviewPlanning({ currentItemUpdatedAt?: string; itemUpdatedAt?: string; reviewItemUpdatedAt?: string; + automationItemUpdatedAt?: string; reviewCommentSyncedAt?: string; labelsSyncedAt?: string; reviewPolicy?: string; @@ -317,6 +321,7 @@ export function createReviewPlanning({ itemSourceRevision: options.reviewSourceRevision, reviewPolicy: options.reviewPolicy, itemUpdatedAt: options.reviewItemUpdatedAt, + automationItemUpdatedAt: options.automationItemUpdatedAt, reviewCommentSyncedAt: options.reviewCommentSyncedAt, labelsSyncedAt: options.labelsSyncedAt, } as ExistingReview; diff --git a/src/clawsweeper-types.ts b/src/clawsweeper-types.ts index 06bddaf223..13bcb47947 100644 --- a/src/clawsweeper-types.ts +++ b/src/clawsweeper-types.ts @@ -294,6 +294,7 @@ export interface ExistingReview { markdown: string; reviewedAt: string | undefined; itemUpdatedAt: string | undefined; + automationItemUpdatedAt?: string | undefined; reviewCommentSyncedAt: string | undefined; labelsSyncedAt: string | undefined; decision: string | undefined; diff --git a/src/pr-close-coverage-proof.ts b/src/pr-close-coverage-proof.ts index 73201c49f6..47ff78525b 100644 --- a/src/pr-close-coverage-proof.ts +++ b/src/pr-close-coverage-proof.ts @@ -227,6 +227,18 @@ function stringifyPrCloseCoverageProofPromptJson(value: unknown, space?: number) return (serialized ?? "null").replace(/`/g, "\\u0060"); } +function prCloseCoverageProofReportMarkdown(markdown: string): string { + const match = markdown.match(/^(---\r?\n)([\s\S]*?)(\r?\n---(?:\r?\n|$))/); + if (!match) return markdown.trim(); + const frontMatter = (match[2] ?? "") + .split(/\r?\n/) + .filter((line) => !/^automation_item_updated_at\s*:/.test(line)) + .join("\n"); + return `${match[1] ?? "---\n"}${frontMatter}${match[3] ?? "\n---\n"}${markdown.slice( + match[0].length, + )}`.trim(); +} + export function buildPrCloseCoverageProofPrompt(options: { source: PrCloseCoverageProofPullRequestView; covering: PrCloseCoverageProofPullRequestView; @@ -244,7 +256,9 @@ export function buildPrCloseCoverageProofPrompt(options: { "", "PR A source report JSON string:", "```json", - stringifyPrCloseCoverageProofPromptJson(options.reportMarkdown.trim()), + stringifyPrCloseCoverageProofPromptJson( + prCloseCoverageProofReportMarkdown(options.reportMarkdown), + ), "```", "", "Current PR title, body, and comments:", diff --git a/src/repair/record-tuple.ts b/src/repair/record-tuple.ts index 92765dd4e7..98dc1727bc 100644 --- a/src/repair/record-tuple.ts +++ b/src/repair/record-tuple.ts @@ -36,7 +36,7 @@ const VERSION_GROUPS = [ ["reviewed_at", "last_full_review_at"], ["reconciled_at"], ["applied_at", "apply_checked_at"], - ["review_comment_synced_at", "review_comment_checked_at"], + ["review_comment_synced_at", "review_comment_checked_at", "automation_item_updated_at"], ["labels_synced_at"], ["failed_review_retry_last_at"], ] as const; diff --git a/src/review-structural-cache.ts b/src/review-structural-cache.ts index b336ba81ce..c2ac1343c7 100644 --- a/src/review-structural-cache.ts +++ b/src/review-structural-cache.ts @@ -122,6 +122,7 @@ export interface ReviewStructuralPriorReview { reviewPolicy?: string | undefined; reviewModel?: string | undefined; itemSourceRevision?: string | undefined; + automationItemUpdatedAt?: string | undefined; reviewCommentSyncedAt?: string | undefined; labelsSyncedAt?: string | undefined; } @@ -1190,6 +1191,7 @@ function activityCoveredByReview( review: ReviewStructuralPriorReview, ): boolean { if (current.activityUpdatedAt === prior.activityUpdatedAt) return true; + if (current.activityUpdatedAt === review.automationItemUpdatedAt) return true; const priorActivity = timestampMs(prior.activityUpdatedAt); const currentActivity = timestampMs(current.activityUpdatedAt); const latestOwnedSync = Math.max( diff --git a/src/scheduler-policy.ts b/src/scheduler-policy.ts index 46ec10837b..4197d55b17 100644 --- a/src/scheduler-policy.ts +++ b/src/scheduler-policy.ts @@ -12,6 +12,7 @@ export interface SchedulerItem { export interface SchedulerExistingReview { reviewedAt?: string | undefined; itemUpdatedAt?: string | undefined; + automationItemUpdatedAt?: string | undefined; reviewCommentSyncedAt?: string | undefined; labelsSyncedAt?: string | undefined; reviewStatus?: string | undefined; @@ -88,6 +89,7 @@ function hasActivitySinceReview( if (!review) return false; const updatedAt = Date.parse(item.updatedAt); const reviewedAt = reviewedAtMs(review); + if (item.updatedAt === review.automationItemUpdatedAt) return false; const reviewCommentSyncedAt = timestampMs(review.reviewCommentSyncedAt); const labelsSyncedAt = timestampMs(review.labelsSyncedAt); const botOwnedSyncedAt = Math.max( diff --git a/test/apply-label-sync.test.ts b/test/apply-label-sync.test.ts index f8599fe406..80fe810a32 100644 --- a/test/apply-label-sync.test.ts +++ b/test/apply-label-sync.test.ts @@ -1204,12 +1204,13 @@ const args = rawArgs[0] === "--repo" ? rawArgs.slice(2) : rawArgs; appendFileSync(logPath, JSON.stringify(args) + "\\n"); const path = args[1] || ""; if (args[0] === "api" && /\\/issues\\/74478$/.test(path)) { + const commentWasPosted = readFileSync(logPath, "utf8").includes("posted-comment-body"); console.log(JSON.stringify({ number: 74478, title: "Record PR label churn", html_url: "https://github.com/openclaw/clawsweeper/pull/74478", created_at: "2026-05-19T19:00:00Z", - updated_at: "2026-05-19T20:00:00Z", + updated_at: commentWasPosted ? "2026-05-19T20:00:02Z" : "2026-05-19T20:00:00Z", closed_at: null, state: "open", locked: false, @@ -1269,6 +1270,7 @@ if (args[0] === "api" && /\\/issues\\/74478$/.test(path)) { const report = readFileSync(itemPath, "utf8"); assert.match(report, /^labels_synced_at: /m); + assert.match(report, /^automation_item_updated_at: 2026-05-19T20:00:02Z$/m); assert.match(report, /proof: sufficient/); assert.match(report, /proof: 📸 screenshot/); assert.match(report, /rating: 🦞 diamond lobster/); diff --git a/test/pr-close-coverage-proof.test.ts b/test/pr-close-coverage-proof.test.ts index a894f50383..6a0acb77ae 100644 --- a/test/pr-close-coverage-proof.test.ts +++ b/test/pr-close-coverage-proof.test.ts @@ -20,6 +20,7 @@ import { prCloseCoverageProofEnvelopePath, parsePrCloseCoverageProofModelResult, prCloseCoverageProofCloseDecision, + prCloseCoverageProofPromptSha256, validatePrCloseCoverageProofEnvelopeBinding, } from "../dist/pr-close-coverage-proof.js"; @@ -397,6 +398,38 @@ test("PR close coverage proof prompt escapes fenced blocks in JSON payloads", () assert.doesNotMatch(prompt, /\\n```\\n/); }); +test("PR close coverage proof binding ignores the owned item update timestamp", () => { + const source = proofPullRequest(10); + const covering = proofPullRequest(20); + const options = { + source, + covering, + reportMarkdown: [ + "---", + "decision: close", + "automation_item_updated_at: 2026-08-01T14:53:29Z", + "---", + "", + "Close as covered.", + ].join("\n"), + relationshipSignalSnippets: ["#20 covers #10"], + promptTemplate: "Decide whether PR B covers PR A.", + }; + + const original = prCloseCoverageProofPromptSha256(options); + const updatedOperationalTimestamp = prCloseCoverageProofPromptSha256({ + ...options, + reportMarkdown: options.reportMarkdown.replace("2026-08-01T14:53:29Z", "2026-08-01T14:54:30Z"), + }); + const updatedDecision = prCloseCoverageProofPromptSha256({ + ...options, + reportMarkdown: options.reportMarkdown.replace("decision: close", "decision: keep_open"), + }); + + assert.equal(updatedOperationalTimestamp, original); + assert.notEqual(updatedDecision, original); +}); + test("PR close coverage proof prompt requires concrete coverage proof", () => { const prompt = readFileSync("prompts/pr-close-coverage-proof.md", "utf8"); diff --git a/test/review-structural-cache.test.ts b/test/review-structural-cache.test.ts index 3de32db16d..2b21902dda 100644 --- a/test/review-structural-cache.test.ts +++ b/test/review-structural-cache.test.ts @@ -485,6 +485,16 @@ test("owned comment or label synchronization may explain metadata-only activity" const priorRecord = record(); const currentRecord = record(issueSnapshot({ activityUpdatedAt: "2026-07-10T10:02:00Z" })); assert.equal(decision({ priorRecord, currentRecord }).hit, true); + + const serverLaggedRecord = record(issueSnapshot({ activityUpdatedAt: "2026-07-10T10:02:01Z" })); + assert.equal( + decision({ + priorRecord, + currentRecord: serverLaggedRecord, + review: review({ automationItemUpdatedAt: "2026-07-10T10:02:01Z" }), + }).hit, + true, + ); }); test("changed target head forces issue hydration", () => { diff --git a/test/scheduler-policy.test.ts b/test/scheduler-policy.test.ts index 67111c3866..47de1d6975 100644 --- a/test/scheduler-policy.test.ts +++ b/test/scheduler-policy.test.ts @@ -225,6 +225,49 @@ test("scheduler keeps ambiguous post-sync activity due after review", () => { } }); +test("scheduler treats the authoritative post-mutation item timestamp as owned activity", () => { + const reviewedAt = "2026-08-01T14:52:41Z"; + const automationItemUpdatedAt = "2026-08-01T14:53:29Z"; + const review = { + path: "items/117.md", + markdown: "", + reviewedAt, + itemUpdatedAt: "2026-08-01T12:44:07Z", + reviewCommentSyncedAt: "2026-08-01T14:53:28Z", + automationItemUpdatedAt, + decision: "keep_open", + reviewStatus: "complete", + reviewPolicy: "current", + }; + + assert.equal( + shouldReviewItem( + item({ + createdAt: "2026-07-24T06:00:00Z", + updatedAt: automationItemUpdatedAt, + }), + review, + Date.parse("2026-08-01T16:50:00Z"), + "current", + ), + false, + "the exact GitHub item timestamp observed after ClawSweeper mutation must not create hourly activity", + ); + assert.equal( + shouldReviewItem( + item({ + createdAt: "2026-07-24T06:00:00Z", + updatedAt: "2026-08-01T14:53:30Z", + }), + review, + Date.parse("2026-08-01T16:50:00Z"), + "current", + ), + true, + "a later target-side update remains due", + ); +}); + test("hot new item priority is protected from older activity churn", () => { const now = Date.parse("2026-04-30T12:00:00Z"); const review = (reviewedAt, itemUpdatedAt) => ({ @@ -786,6 +829,17 @@ test("CSW-088 suppresses only the immediate same-head and same-body hot-intake r }), true, ); + const automationItemUpdatedAt = new Date(Date.parse(reviewedAt) + 2_000).toISOString(); + assert.equal( + shouldSkipScheduledHotIntakeExactReviewForTest({ + ...sameSnapshot, + itemUpdatedAt: automationItemUpdatedAt, + currentItemUpdatedAt: automationItemUpdatedAt, + automationItemUpdatedAt, + reviewCommentSyncedAt: new Date(Date.parse(reviewedAt) + 1_000).toISOString(), + }), + true, + ); assert.equal( shouldSkipScheduledHotIntakeExactReviewForTest({ ...sameSnapshot,