diff --git a/src/clawsweeper-apply-decision-workflow.ts b/src/clawsweeper-apply-decision-workflow.ts index 7229b4ad05..da6d368367 100644 --- a/src/clawsweeper-apply-decision-workflow.ts +++ b/src/clawsweeper-apply-decision-workflow.ts @@ -11,7 +11,10 @@ import { import { evaluateApplyClosePolicy } from "./clawsweeper-apply-close-policies.js"; import type { CreateApplyDecisionWorkflowDependencies } from "./clawsweeper-apply-dependencies.js"; import { createApplyLeaseGuards } from "./clawsweeper-apply-lease-guards.js"; -import { createApplyProofFreshnessGuards } from "./clawsweeper-apply-proof-freshness.js"; +import { + type ApplySelfMutationItemReceipt, + createApplyProofFreshnessGuards, +} from "./clawsweeper-apply-proof-freshness.js"; import { syncApplyPullRequestLabels } from "./clawsweeper-apply-pull-request-labels.js"; import { promoteApplyPullRequest } from "./clawsweeper-apply-pull-request-promotion.js"; import { syncApplyReportLabels } from "./clawsweeper-apply-report-labels.js"; @@ -39,6 +42,7 @@ import { STALE_INSUFFICIENT_INFO_MIN_AGE_DAYS, } from "./clawsweeper-policy.js"; import { rawCommentBody } from "./clawsweeper-review-comments.js"; +import { completeActivityContextSymbol } from "./clawsweeper-types.js"; import type { AcquiredReviewStartLease, ActionTaken, @@ -63,7 +67,9 @@ import { isLockedConversationCommentError, } from "./github-retry.js"; import { type PrCloseCoverageProofRuntime } from "./pr-close-coverage-proof.js"; +import { isReviewedPrActivityCursor } from "./review-activity-cursor.js"; import { isAutoCloseAllowed, repositoryProfileFor } from "./repository-profiles.js"; +import { stableJson } from "./stable-json.js"; export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWorkflowDependencies) { const { @@ -94,7 +100,9 @@ export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWor ensureDir, exactEventReviewLeaseDisposition, fetchItem, + fetchReviewedPrActivityCursor, finishApplyMutationAttempt, + freshPullRequestReviewHead, frontMatterStringArray, frontMatterValue, ghJson, @@ -721,14 +729,54 @@ export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWor let currentClosingPullRequests: unknown[] | undefined; let clawSweeperLabelsChanged = false; let issueAdvisoryLabelsChanged = false; - const allowedSelfMutationUpdatedAts = new Set(); + const selfMutationItemReceipts: ApplySelfMutationItemReceipt[] = []; const currentItemContext = (): ItemContext => { - currentContext ??= collectItemContext(item, { fullTimelineForRelations: true }); + currentContext ??= collectItemContext(item, { + fullTimelineForRelations: true, + reviewCacheDigest: true, + }); return currentContext; }; const markdownBeforeApplyDecisionMutations = markdown; + const expectedReviewActivityCursor = frontMatterValue( + markdownBeforeApplyDecisionMutations, + "review_activity_cursor", + ); + const reviewedSourceRevision = frontMatterValue( + markdownBeforeApplyDecisionMutations, + "item_source_revision", + ); + const reviewedTimelineRevision = frontMatterValue( + markdownBeforeApplyDecisionMutations, + "review_timeline_revision", + ); + const reviewHasCompleteActivityIdentity = Boolean( + reviewedSourceRevision && + reviewedSourceRevision !== "unknown" && + reviewedTimelineRevision && + /^[0-9a-f]{64}$/.test(reviewedTimelineRevision) && + (item.kind !== "pull_request" || + (frontMatterValue(markdownBeforeApplyDecisionMutations, "pull_head_sha") && + isReviewedPrActivityCursor(expectedReviewActivityCursor))), + ); + const completeReviewActivityReceiptMatches = (context: ItemContext): boolean => { + if (!reviewHasCompleteActivityIdentity || !context[completeActivityContextSymbol]) { + return false; + } + if ( + context.sourceRevision !== reviewedSourceRevision || + context.timelineRevision !== reviewedTimelineRevision + ) { + return false; + } + return ( + item.kind !== "pull_request" || + (freshPullRequestReviewHead(markdownBeforeApplyDecisionMutations, context) && + context.pullReviewActivityCursor === expectedReviewActivityCursor) + ); + }; const currentReviewActivityBlock = createApplyReviewActivityGuard(dependencies, { - expectedCursor: frontMatterValue(markdownBeforeApplyDecisionMutations, "review_activity_cursor"), + expectedCursor: expectedReviewActivityCursor, itemKind: item.kind, number, }); @@ -869,7 +917,37 @@ export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWor if (initialCanonicalCommentSyncGuard.stopApply) break; if (initialCanonicalCommentSyncGuard.skipCurrentItem) continue; const rememberSelfMutationUpdatedAt = (): void => { - if (!dryRun) allowedSelfMutationUpdatedAts.add(fetchItem(number).item.updatedAt); + if (dryRun) return; + const automationItem = fetchItem(number).item; + const automationItemUpdatedAt = automationItem.updatedAt; + markdown = replaceFrontMatterValue( + markdown, + "automation_item_updated_at", + automationItemUpdatedAt, + ); + // A post-mutation item timestamp is not operation-specific. Admit it + // into this apply run only when an immediate structural receipt still + // matches the reviewed source, PR head, and review-activity cursor. The + // final close gate repeats those checks and verifies that no target-side + // activity landed after proof. + if (!reviewedSourceRevision || reviewedSourceRevision === "unknown") return; + const receiptContext = collectItemContext(automationItem, { + fullTimelineForRelations: true, + reviewCacheDigest: true, + }); + if (!completeReviewActivityReceiptMatches(receiptContext)) return; + const completeActivityContext = receiptContext[completeActivityContextSymbol]; + if (!completeActivityContext) return; + selfMutationItemReceipts.push({ + updatedAt: automationItemUpdatedAt, + sourceRevision: reviewedSourceRevision, + activityReceipt: stableJson(completeActivityContext), + prHeadMatches: + item.kind !== "pull_request" || + freshPullRequestReviewHead(markdownBeforeApplyDecisionMutations, receiptContext), + reviewActivityCursor: + item.kind === "pull_request" ? fetchReviewedPrActivityCursor(number) : null, + }); }; const candidateGuards = createApplyCandidateGuards(dependencies, { authorPrBudgetClosesThisRun, @@ -1149,9 +1227,10 @@ export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWor labelSyncFreshEnough, reviewedSourceFresh, retryCloseCoverageCommandStatusOnlyUpdate, + sameSecondCloseActivityIsAmbiguous, } = createApplySourceFreshness(dependencies, { action, - allowedSelfMutationUpdatedAts, + completeReviewActivityReceiptMatches, currentItemContext, currentState: () => ({ isCloseProposal, markdown, storedUpdatedAt }), existingReviewComment, @@ -1162,6 +1241,7 @@ export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWor reportLabelsBeforeApply, reportReviewLeaseCommentId, reportReviewLeaseOwner, + reviewHasCompleteActivityIdentity, requiresApplyMutationLease, storedHash, }); @@ -1361,14 +1441,23 @@ export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWor createApplyProofFreshnessGuards({ ...dependencies, action, - allowedSelfMutationUpdatedAts, + automationItemUpdatedAt: frontMatterValue( + markdownBeforeApplyDecisionMutations, + "automation_item_updated_at", + ), + completeReviewActivityReceiptMatches, currentProofState: () => ({ ...coverageProofState, storedHash, storedUpdatedAt, }), + expectedReviewActivityCursor, + itemKind: item.kind, number, + reviewHasCompleteActivityIdentity, + reviewMarkdown: markdownBeforeApplyDecisionMutations, retryCloseCoverageCommandStatusOnlyUpdate, + selfMutationItemReceipts, }); if (state !== "open") { if (item.closedAt) { @@ -1430,6 +1519,16 @@ export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWor break; continue; } + if (sameSecondCloseActivityIsAmbiguous) { + if ( + markChangedSinceReview({ + reason: "same-second activity requires a fresh review", + currentUpdatedAt: item.updatedAt, + }) + ) + break; + continue; + } const labelsCanSync = !lockedMetadataOnly && !stalePrReviewHead && labelSyncFreshEnough(); const complete = frontMatterValue(markdown, "review_status") === "complete" && labelsCanSync; const reportLabelSync = syncApplyReportLabels(dependencies, { @@ -1779,10 +1878,7 @@ export function createApplyDecisionWorkflow(dependencies: CreateApplyDecisionWor markedReviewComment, existingReviewComment, ); - const syncedCommentUpdatedAt = commentUpdatedAt(syncedComment); - 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-apply-dependencies.ts b/src/clawsweeper-apply-dependencies.ts index 91f6f1c094..ddb2822449 100644 --- a/src/clawsweeper-apply-dependencies.ts +++ b/src/clawsweeper-apply-dependencies.ts @@ -149,7 +149,7 @@ export interface CreateApplyDecisionWorkflowDependencies { actionTaken: string, nowMs: number, ) => boolean; - coveringPrCloseCoveragePullRequestUpdatedAt: (number: number) => string | null; + coveringPrCloseCoveragePullRequestSnapshotSha256: (number: number) => string; decisionPacketsDirFromArgs: (args: Args, itemsDir: string, closedDir: string) => string; defaultClosedDir: (profile?: RepositoryProfile) => string; defaultItemsDir: (profile?: RepositoryProfile) => string; diff --git a/src/clawsweeper-apply-proof-freshness.ts b/src/clawsweeper-apply-proof-freshness.ts index 9601060bd5..f0cd01f932 100644 --- a/src/clawsweeper-apply-proof-freshness.ts +++ b/src/clawsweeper-apply-proof-freshness.ts @@ -1,22 +1,37 @@ import type { CreateApplyDecisionWorkflowDependencies } from "./clawsweeper-apply-dependencies.js"; +import { completeActivityContextSymbol } from "./clawsweeper-types.js"; import type { Item, ItemContext, + ItemKind, PrCloseCoverageProofGateBlock, PrCloseCoverageProofGateResult, } from "./clawsweeper-types.js"; +import { isReviewedPrActivityCursor } from "./review-activity-cursor.js"; +import { stableJson } from "./stable-json.js"; + +export interface ApplySelfMutationItemReceipt { + updatedAt: string; + sourceRevision: string; + activityReceipt: string; + prHeadMatches: boolean; + reviewActivityCursor: string | null; +} type ApplyProofFreshnessDependencies = Pick< CreateApplyDecisionWorkflowDependencies, | "collectItemContext" | "contextHasNonAutomationActivityAfter" - | "coveringPrCloseCoveragePullRequestUpdatedAt" + | "coveringPrCloseCoveragePullRequestSnapshotSha256" | "fetchItem" + | "fetchReviewedPrActivityCursor" + | "freshPullRequestReviewHead" | "GitHubRuntimeBudgetError" | "itemSnapshotHash" > & { action: string | undefined; - allowedSelfMutationUpdatedAts: ReadonlySet; + automationItemUpdatedAt: string | undefined; + completeReviewActivityReceiptMatches: (context: ItemContext) => boolean; currentProofState: () => { cachedPrCloseCoverageProofGateResult: PrCloseCoverageProofGateResult | undefined; prCloseCoverageProofGateChecked: boolean; @@ -24,22 +39,35 @@ type ApplyProofFreshnessDependencies = Pick< storedHash: string | undefined; storedUpdatedAt: string | undefined; }; + expectedReviewActivityCursor: string | undefined; + itemKind: ItemKind; number: number; + reviewMarkdown: string; + reviewHasCompleteActivityIdentity: boolean; retryCloseCoverageCommandStatusOnlyUpdate: (item: Item, context: ItemContext) => boolean; + selfMutationItemReceipts: readonly ApplySelfMutationItemReceipt[]; }; export function createApplyProofFreshnessGuards({ action, - allowedSelfMutationUpdatedAts, + automationItemUpdatedAt, collectItemContext, + completeReviewActivityReceiptMatches, contextHasNonAutomationActivityAfter, - coveringPrCloseCoveragePullRequestUpdatedAt, + coveringPrCloseCoveragePullRequestSnapshotSha256, currentProofState, + expectedReviewActivityCursor, fetchItem, + fetchReviewedPrActivityCursor, + freshPullRequestReviewHead, GitHubRuntimeBudgetError, + itemKind, itemSnapshotHash, number, + reviewMarkdown, + reviewHasCompleteActivityIdentity, retryCloseCoverageCommandStatusOnlyUpdate, + selfMutationItemReceipts, }: ApplyProofFreshnessDependencies) { const postProofFreshnessBlock = (): { reason: string; @@ -73,21 +101,72 @@ export function createApplyProofFreshnessGuards({ refreshed.item, (refreshedContext ??= collectItemContext(refreshed.item, { fullTimelineForRelations: true, + reviewCacheDigest: true, })), ); + const candidateItemReceipts = selfMutationItemReceipts.filter( + (receipt) => receipt.updatedAt === refreshed.item.updatedAt, + ); + const mayMatchPersistedAutomationReceipt = + Boolean(automationItemUpdatedAt) && refreshed.item.updatedAt === automationItemUpdatedAt; + if (candidateItemReceipts.length > 0 || mayMatchPersistedAutomationReceipt) { + refreshedContext ??= collectItemContext(refreshed.item, { + fullTimelineForRelations: true, + reviewCacheDigest: true, + }); + } + const refreshedCompleteActivityContext = refreshedContext?.[completeActivityContextSymbol]; + const refreshedActivityReceipt = refreshedCompleteActivityContext + ? stableJson(refreshedCompleteActivityContext) + : null; + const refreshedReviewActivityCursor = + itemKind === "pull_request" && + (candidateItemReceipts.length > 0 || + mayMatchPersistedAutomationReceipt || + refreshed.item.updatedAt === storedUpdatedAt) + ? fetchReviewedPrActivityCursor(number) + : null; + const refreshedItemReceiptMatches = candidateItemReceipts.some( + (receipt) => + refreshedContext?.sourceRevision === receipt.sourceRevision && + refreshedActivityReceipt === receipt.activityReceipt && + receipt.prHeadMatches && + refreshedContext !== null && + (itemKind !== "pull_request" || + (freshPullRequestReviewHead(reviewMarkdown, refreshedContext) && + isReviewedPrActivityCursor(expectedReviewActivityCursor) && + receipt.reviewActivityCursor === expectedReviewActivityCursor && + refreshedReviewActivityCursor === expectedReviewActivityCursor)), + ); + const refreshedCompleteReceiptMatchesReview = (): boolean => { + refreshedContext ??= collectItemContext(refreshed.item, { + fullTimelineForRelations: true, + reviewCacheDigest: true, + }); + if (!completeReviewActivityReceiptMatches(refreshedContext)) return false; + return ( + itemKind !== "pull_request" || + refreshedReviewActivityCursor === expectedReviewActivityCursor + ); + }; + const persistedAutomationReceiptMatches = + mayMatchPersistedAutomationReceipt && + reviewHasCompleteActivityIdentity && + refreshedCompleteReceiptMatchesReview(); const refreshedSelfMutationOnlyUpdate = - allowedSelfMutationUpdatedAts.has(refreshed.item.updatedAt) || + refreshedItemReceiptMatches || + persistedAutomationReceiptMatches || refreshedCommandStatusOnlyUpdate; const selfMutationMaskedNonAutomationActivity = (): boolean => { if (prCloseCoverageProofStartedAtMs === null) return true; refreshedContext ??= collectItemContext(refreshed.item, { fullTimelineForRelations: true, + reviewCacheDigest: true, + }); + const proofSecondStartMs = Math.floor(prCloseCoverageProofStartedAtMs / 1000) * 1000; + return contextHasNonAutomationActivityAfter(refreshedContext, proofSecondStartMs - 1, { + truncationCountsAsActivity: false, }); - return contextHasNonAutomationActivityAfter( - refreshedContext, - prCloseCoverageProofStartedAtMs, - { truncationCountsAsActivity: false }, - ); }; if (storedUpdatedAt && refreshed.item.updatedAt !== storedUpdatedAt) { if (refreshedSelfMutationOnlyUpdate) { @@ -102,11 +181,23 @@ export function createApplyProofFreshnessGuards({ currentUpdatedAt: refreshed.item.updatedAt, }; } + if ( + storedUpdatedAt && + refreshed.item.updatedAt === storedUpdatedAt && + reviewHasCompleteActivityIdentity && + !refreshedCompleteReceiptMatchesReview() + ) { + return { + reason: "same-second activity requires a fresh review", + currentUpdatedAt: refreshed.item.updatedAt, + }; + } if (!storedUpdatedAt && storedHash) { const refreshedHash = itemSnapshotHash( refreshed.item, (refreshedContext ??= collectItemContext(refreshed.item, { fullTimelineForRelations: true, + reviewCacheDigest: true, })), ); if (refreshedHash !== storedHash) { @@ -134,10 +225,11 @@ export function createApplyProofFreshnessGuards({ return null; } const { covering } = cachedPrCloseCoverageProofGateResult; - if (!covering.updatedAt) return null; try { - const currentUpdatedAt = coveringPrCloseCoveragePullRequestUpdatedAt(covering.number); - if (currentUpdatedAt === covering.updatedAt) return null; + const currentSnapshotSha256 = coveringPrCloseCoveragePullRequestSnapshotSha256( + covering.number, + ); + if (currentSnapshotSha256 === covering.snapshotSha256) return null; return { actionTaken: "retry_pr_close_coverage_proof", reason: `linked canonical PR #${covering.number} changed after coverage proof`, diff --git a/src/clawsweeper-apply-source-freshness.ts b/src/clawsweeper-apply-source-freshness.ts index ddbd74910a..b5bd178011 100644 --- a/src/clawsweeper-apply-source-freshness.ts +++ b/src/clawsweeper-apply-source-freshness.ts @@ -22,7 +22,7 @@ type ApplySourceFreshnessDependencies = Pick< interface ApplySourceFreshnessOptions { action: string | undefined; - allowedSelfMutationUpdatedAts: Set; + completeReviewActivityReceiptMatches: (context: ItemContext) => boolean; currentItemContext: () => ItemContext; currentState: () => { isCloseProposal: boolean; @@ -37,6 +37,7 @@ interface ApplySourceFreshnessOptions { reportLabelsBeforeApply: readonly string[]; reportReviewLeaseCommentId: number; reportReviewLeaseOwner: string | undefined; + reviewHasCompleteActivityIdentity: boolean; requiresApplyMutationLease: boolean; storedHash: string | undefined; } @@ -158,7 +159,7 @@ export function createApplySourceFreshness( } = dependencies; const { action, - allowedSelfMutationUpdatedAts, + completeReviewActivityReceiptMatches, currentItemContext, currentState, existingReviewComment, @@ -169,13 +170,11 @@ export function createApplySourceFreshness( reportLabelsBeforeApply, reportReviewLeaseCommentId, reportReviewLeaseOwner, + reviewHasCompleteActivityIdentity, requiresApplyMutationLease, storedHash, } = options; const existingReviewCommentUpdatedAt = commentUpdatedAt(existingReviewComment); - if (existingReviewCommentUpdatedAt) { - allowedSelfMutationUpdatedAts.add(existingReviewCommentUpdatedAt); - } const reportOwnedLeaseComments = requiresApplyMutationLease ? leaseComments.filter( (comment) => @@ -183,11 +182,6 @@ export function createApplySourceFreshness( reviewStartLeaseOwner(comment) === reportReviewLeaseOwner, ) : []; - for (const updatedAt of reportOwnedLeaseComments - .map(commentUpdatedAt) - .filter((value): value is string => timestampMs(value) !== null)) { - allowedSelfMutationUpdatedAts.add(updatedAt); - } const latestAutomationUpdatedAt = [existingReviewComment, ...reportOwnedLeaseComments] .map(commentUpdatedAt) .filter((value): value is string => timestampMs(value) !== null) @@ -209,18 +203,24 @@ export function createApplySourceFreshness( const labelSyncOnlyUpdate = Boolean( recordedLabelSyncMatches && storedUpdatedAtMs !== null && - !contextHasNonAutomationActivityAfter(currentItemContext(), storedUpdatedAtMs, { - truncationCountsAsActivity: true, - }), + (reviewHasCompleteActivityIdentity + ? completeReviewActivityReceiptMatches(currentItemContext()) + : !contextHasNonAutomationActivityAfter(currentItemContext(), storedUpdatedAtMs - 1, { + truncationCountsAsActivity: true, + useCompleteActivityContext: true, + })), ); const ownedIssueReviewLeaseOnlyUpdate = Boolean( item.kind === "issue" && updatedSinceReview && storedUpdatedAtMs !== null && reportOwnedLeaseComments.some((comment) => commentUpdatedAt(comment) === item.updatedAt) && - !contextHasNonAutomationActivityAfter(currentItemContext(), storedUpdatedAtMs, { - truncationCountsAsActivity: true, - }), + (reviewHasCompleteActivityIdentity + ? completeReviewActivityReceiptMatches(currentItemContext()) + : !contextHasNonAutomationActivityAfter(currentItemContext(), storedUpdatedAtMs - 1, { + truncationCountsAsActivity: true, + useCompleteActivityContext: true, + })), ); let statusComments: Record[] | undefined; const reviewedSourceRevision = frontMatterValue( @@ -251,20 +251,37 @@ export function createApplySourceFreshness( const createdAt = comment ? stringOrUndefined(comment.created_at) : undefined; return Boolean( createdAt && - !contextHasNonAutomationActivityAfter(candidateContext, storedUpdatedAtMs, { - truncationCountsAsActivity: true, - ignoreTrustedTimelineComment: { authors: CLAWSWEEPER_BOT_AUTHORS, createdAt }, - }), + (reviewHasCompleteActivityIdentity + ? completeReviewActivityReceiptMatches(candidateContext) + : !contextHasNonAutomationActivityAfter(candidateContext, storedUpdatedAtMs - 1, { + truncationCountsAsActivity: true, + useCompleteActivityContext: true, + ignoreTrustedTimelineComment: { authors: CLAWSWEEPER_BOT_AUTHORS, createdAt }, + })), ); }; const commandStatusOnlyUpdate = action === "retry_pr_close_coverage_proof" && retryCloseCoverageCommandStatusOnlyUpdate(item, currentItemContext()); - const automationOnlyUpdate = - reviewCommentOnlyUpdate || - labelSyncOnlyUpdate || - ownedIssueReviewLeaseOnlyUpdate || - commandStatusOnlyUpdate; + const completeAutomationReceiptMatchesReview = (): boolean => + completeReviewActivityReceiptMatches(currentItemContext()); + const { isCloseProposal } = currentState(); + const automationOnlyUpdate = Boolean( + (reviewCommentOnlyUpdate || + labelSyncOnlyUpdate || + ownedIssueReviewLeaseOnlyUpdate || + commandStatusOnlyUpdate) && + (!isCloseProposal || + !reviewHasCompleteActivityIdentity || + completeAutomationReceiptMatchesReview()), + ); + const sameSecondCloseActivityIsAmbiguous = Boolean( + isCloseProposal && + reviewHasCompleteActivityIdentity && + storedUpdatedAt && + item.updatedAt === storedUpdatedAt && + !completeAutomationReceiptMatchesReview(), + ); const reviewedSourceFresh = (): boolean => storedUpdatedAt ? !updatedSinceReview || automationOnlyUpdate @@ -299,6 +316,7 @@ export function createApplySourceFreshness( reviewedSourceFresh, retryCloseCoverageCommandStatusOnlyUpdate, reviewCommentOnlyUpdate, + sameSecondCloseActivityIsAmbiguous, updatedSinceReview, }; } diff --git a/src/clawsweeper-coverage-proof.ts b/src/clawsweeper-coverage-proof.ts index 4ed4cd825c..8ccff2f2e4 100644 --- a/src/clawsweeper-coverage-proof.ts +++ b/src/clawsweeper-coverage-proof.ts @@ -19,6 +19,7 @@ import { prCloseCoverageProofCloseDecision, prCloseCoverageProofEnvelopePath, prCloseCoverageProofPromptSha256, + prCloseCoverageProofSnapshotSha256, readPrCloseCoverageProofEnvelope, runPrCloseCoverageProofModel, validatePrCloseCoverageProofEnvelopeBinding, @@ -294,12 +295,8 @@ export function createPullRequestCoverageProof( }; } - function coveringPrCloseCoveragePullRequestUpdatedAt(number: number): string | null { - const pull = asRecord(ghJson(["api", `repos/${targetRepo()}/pulls/${number}`])); - const pullUpdatedAt = stringOrUndefined(pull.updated_at); - if (pullUpdatedAt) return pullUpdatedAt; - const issue = asRecord(ghJson(["api", `repos/${targetRepo()}/issues/${number}`])); - return stringOrUndefined(issue.updated_at) ?? null; + function coveringPrCloseCoveragePullRequestSnapshotSha256(number: number): string { + return prCloseCoverageProofSnapshotSha256(coveringPrCloseCoveragePullRequestView(number)); } function prCloseCoverageProofSignalSnippets( @@ -457,6 +454,7 @@ export function createPullRequestCoverageProof( covering: { number: covering.number, provedAtMs: proofStartedAtMs, + snapshotSha256: prCloseCoverageProofSnapshotSha256(covering), updatedAt: covering.updatedAt, url: covering.url, proof: closeDecision.proof, @@ -702,7 +700,7 @@ export function createPullRequestCoverageProof( prCloseCoverageRuntime, sourcePrCloseCoveragePullRequestView, coveringPrCloseCoveragePullRequestView, - coveringPrCloseCoveragePullRequestUpdatedAt, + coveringPrCloseCoveragePullRequestSnapshotSha256, prCloseCoverageProofSignalSnippets, prCloseCoverageProofGateResult, renderPrCloseCoverageProofReportSection, diff --git a/src/clawsweeper-record-metadata.ts b/src/clawsweeper-record-metadata.ts index 047bc57b81..af392299ea 100644 --- a/src/clawsweeper-record-metadata.ts +++ b/src/clawsweeper-record-metadata.ts @@ -406,6 +406,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"), @@ -437,6 +438,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-report-document.ts b/src/clawsweeper-report-document.ts index 3b7d4e33c8..1e257cae17 100644 --- a/src/clawsweeper-report-document.ts +++ b/src/clawsweeper-report-document.ts @@ -587,6 +587,7 @@ review_semantic_eligible: ${options.semanticRecord?.eligible ?? false} review_semantic_eligibility_reason: ${options.semanticRecord?.eligibilityReason ?? "unknown"} review_semantic_cache_hit: false item_source_revision: ${options.context.sourceRevision ?? "unknown"} +review_timeline_revision: ${options.context.timelineRevision ?? "unknown"} review_activity_cursor: ${options.context.pullReviewActivityCursor ?? "unknown"} close_comment_sha256: ${options.action.closeComment ? sha256(options.action.closeComment) : "none"} review_comment_sha256: none diff --git a/src/clawsweeper-report-orchestration.ts b/src/clawsweeper-report-orchestration.ts index ec6769a418..3b1e827976 100644 --- a/src/clawsweeper-report-orchestration.ts +++ b/src/clawsweeper-report-orchestration.ts @@ -192,7 +192,7 @@ export function createReportOrchestration(dependencies: CreateReportOrchestratio linkedPullRequestSignalContextsFromText, duplicateCanonicalPullRequestBlockReason, canonicalPullRequestCommentSyncBlock, - coveringPrCloseCoveragePullRequestUpdatedAt, + coveringPrCloseCoveragePullRequestSnapshotSha256, prCloseCoverageProofGateResult, applyPrCloseCoverageProofReportSection, applyPrCloseCoverageProofBlockedReport, @@ -360,7 +360,7 @@ export function createReportOrchestration(dependencies: CreateReportOrchestratio configSurfaceReviewRequired, contextHasNonAutomationActivityAfter, contextHasNonAutomationActivityAfterForTest, - coveringPrCloseCoveragePullRequestUpdatedAt, + coveringPrCloseCoveragePullRequestSnapshotSha256, currentReviewRevision, dataModelSurfaceReviewRequired, duplicateCanonicalPullRequestBlockReason, diff --git a/src/clawsweeper-review-command-workflow.ts b/src/clawsweeper-review-command-workflow.ts index 6cab29b914..87c71dc7ac 100644 --- a/src/clawsweeper-review-command-workflow.ts +++ b/src/clawsweeper-review-command-workflow.ts @@ -1051,6 +1051,11 @@ export function createReviewCommandWorkflow(dependencies: CreateReviewCommandWor "item_source_revision", context.sourceRevision ?? "unknown", ); + carried = replaceFrontMatterValue( + carried, + "review_timeline_revision", + context.timelineRevision ?? "unknown", + ); carried = replaceFrontMatterValue( carried, "pull_head_sha", diff --git a/src/clawsweeper-review-planning-hot-intake.ts b/src/clawsweeper-review-planning-hot-intake.ts index 91081bea71..013e0ed7e9 100644 --- a/src/clawsweeper-review-planning-hot-intake.ts +++ b/src/clawsweeper-review-planning-hot-intake.ts @@ -129,17 +129,15 @@ export function createReviewPlanningHotIntake( const updatedAt = Date.parse(item.updatedAt); 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 (updatedAt <= reviewedAt) return false; - const reviewCommentSyncedAt = review.reviewCommentSyncedAt - ? Date.parse(review.reviewCommentSyncedAt) - : Number.NaN; - const labelsSyncedAt = review.labelsSyncedAt ? Date.parse(review.labelsSyncedAt) : Number.NaN; - const botOwnedSyncedAt = Math.max( - Number.isFinite(reviewCommentSyncedAt) ? reviewCommentSyncedAt : -Infinity, - Number.isFinite(labelsSyncedAt) ? labelsSyncedAt : -Infinity, - ); - return !Number.isFinite(botOwnedSyncedAt) || updatedAt > botOwnedSyncedAt; + // Local synchronization clocks are not ownership receipts. GitHub rounds + // item activity to seconds, so target-side activity can share a timestamp + // with a bot comment or label mutation. Keep all post-review activity hot; + // the structural cache can still reuse the prior verdict after comparing a + // complete source and timeline receipt. + if (review.itemUpdatedAt && item.updatedAt === review.itemUpdatedAt) { + return updatedAt === Math.floor(reviewedAt / 1000) * 1000; + } + return updatedAt >= Math.floor(reviewedAt / 1000) * 1000; } function shouldSkipScheduledHotIntakeExactReview( item: Item, @@ -180,6 +178,7 @@ export function createReviewPlanningHotIntake( currentItemUpdatedAt?: string; itemUpdatedAt?: string; reviewItemUpdatedAt?: string; + automationItemUpdatedAt?: string; reviewCommentSyncedAt?: string; labelsSyncedAt?: string; reviewPolicy?: string; @@ -193,6 +192,7 @@ export function createReviewPlanningHotIntake( 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..a89cbdd7c7 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; @@ -1217,6 +1218,7 @@ export interface PrCloseCoverageProofGateBlock { export interface PrCloseCoverageProofCoveringWitness { number: number; provedAtMs: number; + snapshotSha256: string; updatedAt: string | null; url: string; proof: PrCloseCoverageProofModelResult; 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..436cba54e9 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,11 @@ function activityCoveredByReview( review: ReviewStructuralPriorReview, ): boolean { if (current.activityUpdatedAt === prior.activityUpdatedAt) return true; + // Timestamp equality only clears the activity-clock gate. The caller has + // already compared the complete structural source revision and still checks + // target and pull heads before returning a cache hit, so a same-second target + // mutation cannot be attributed to automation by this value alone. + 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..2e5b47df65 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,32 +89,30 @@ function hasActivitySinceReview( if (!review) return false; const updatedAt = Date.parse(item.updatedAt); const reviewedAt = reviewedAtMs(review); - const reviewCommentSyncedAt = timestampMs(review.reviewCommentSyncedAt); - const labelsSyncedAt = timestampMs(review.labelsSyncedAt); - const botOwnedSyncedAt = Math.max( - reviewCommentSyncedAt ?? -Infinity, - labelsSyncedAt ?? -Infinity, - ); + // GitHub item timestamps have second-level precision. Comment and label + // synchronization clocks therefore cannot prove ownership of later item + // activity: an independent target update can share the same value. Keep + // post-review activity eligible here and let the structural cache verify a + // complete item receipt before suppressing the expensive review. if (review.itemUpdatedAt) { - if (item.updatedAt === review.itemUpdatedAt) return false; - if (Number.isFinite(updatedAt) && reviewedAt !== null && updatedAt <= reviewedAt) return false; - if ( - Number.isFinite(updatedAt) && - Number.isFinite(botOwnedSyncedAt) && - updatedAt <= botOwnedSyncedAt - ) { - return false; + if (item.updatedAt === review.itemUpdatedAt) { + return ( + Number.isFinite(updatedAt) && + reviewedAt !== null && + updatedAt === Math.floor(reviewedAt / 1000) * 1000 + ); } - return true; + return ( + Number.isFinite(updatedAt) && + reviewedAt !== null && + updatedAt >= Math.floor(reviewedAt / 1000) * 1000 + ); } - if ( + return ( + reviewedAt !== null && Number.isFinite(updatedAt) && - Number.isFinite(botOwnedSyncedAt) && - updatedAt <= botOwnedSyncedAt - ) { - return false; - } - return reviewedAt !== null && Number.isFinite(updatedAt) && updatedAt > reviewedAt; + updatedAt >= Math.floor(reviewedAt / 1000) * 1000 + ); } function isCreatedWithinDays( diff --git a/test/apply-label-sync.test.ts b/test/apply-label-sync.test.ts index 17a83ac74e..a35ca351b4 100644 --- a/test/apply-label-sync.test.ts +++ b/test/apply-label-sync.test.ts @@ -776,9 +776,11 @@ const oldBody = ${JSON.stringify(oldLiveComment)}; const newerBody = ${JSON.stringify(newerComment)}; const rawArgs = process.argv.slice(2); const args = rawArgs[0] === "--repo" ? rawArgs.slice(2) : rawArgs; -const path = args[1] || ""; +const path = args[1] === "-i" ? args[2] || "" : args[1] || ""; appendFileSync(logPath, JSON.stringify(args) + "\\n"); -if (args[0] === "api" && new RegExp("/issues/${number}/comments(?:\\\\?|$)").test(path) && !args.includes("--method")) { +if (args[0] === "api" && args[1] === "-i" && new RegExp("/issues/${number}/timeline(?:\\\\?|$)").test(path)) { + console.log("HTTP/2 200\\n\\n[]"); +} else if (args[0] === "api" && new RegExp("/issues/${number}/comments(?:\\\\?|$)").test(path) && !args.includes("--method")) { const count = (existsSync(countPath) ? Number(readFileSync(countPath, "utf8")) : 0) + 1; writeFileSync(countPath, String(count)); const body = count >= 6 ? newerBody : oldBody; @@ -1397,12 +1399,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, @@ -1462,6 +1465,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/apply-pr-coverage-proof-close.test.ts b/test/apply-pr-coverage-proof-close.test.ts index db1935d7aa..b4e5f16a94 100644 --- a/test/apply-pr-coverage-proof-close.test.ts +++ b/test/apply-pr-coverage-proof-close.test.ts @@ -332,6 +332,7 @@ test("apply-decisions permits a trusted deferred close-proof command status thro title: "Provider route fallback", action_taken: "retry_pr_close_coverage_proof", item_source_revision: sourceRevisionForTest("Provider route fallback"), + pull_head_sha: "head-sha", close_reason: "duplicate_or_superseded", work_cluster_refs: JSON.stringify([ "Superseded by https://github.com/openclaw/openclaw/pull/400", @@ -358,7 +359,7 @@ test("apply-decisions permits a trusted deferred close-proof command status thro html_url: "https://github.com/openclaw/openclaw/pull/363#issuecomment-9363", created_at: "2026-05-01T01:00:00Z", updated_at: "2026-05-01T01:00:00Z", - user: { login: "clawsweeper" }, + user: { login: "clawsweeper[bot]" }, body: synced.comment, }, { @@ -366,7 +367,7 @@ test("apply-decisions permits a trusted deferred close-proof command status thro html_url: "https://github.com/openclaw/openclaw/pull/363#issuecomment-9364", created_at: "2026-05-01T01:30:00Z", updated_at: "2026-05-01T02:00:00Z", - user: { login: "clawsweeper" }, + user: { login: "clawsweeper[bot]" }, body: "", }, ], @@ -374,7 +375,7 @@ test("apply-decisions permits a trusted deferred close-proof command status thro { event: "commented", created_at: "2026-05-01T01:30:00Z", - actor: { login: "clawsweeper" }, + actor: { login: "clawsweeper[bot]" }, }, ], linkedPulls: { @@ -426,6 +427,7 @@ test("apply-decisions permits a trusted deferred close-proof command status thro assert.equal( report.some((entry) => entry.action === "closed"), true, + JSON.stringify(report, null, 2), ); assert.match( report.find((entry) => entry.action === "closed")?.reason ?? "", @@ -1192,7 +1194,7 @@ test("apply-decisions rechecks duplicate PR freshness after coverage proof passe } }); -test("apply-decisions rechecks covering PR freshness after coverage proof passes", () => { +test("apply-decisions rejects a same-timestamp covering PR snapshot change after proof", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -1206,6 +1208,8 @@ test("apply-decisions rechecks covering PR freshness after coverage proof passes lowSignalCloseReport({ number: 360, title: "Provider route fallback", + item_source_revision: sourceRevisionForTest("Provider route fallback"), + pull_head_sha: "head-sha", close_reason: "duplicate_or_superseded", work_cluster_refs: JSON.stringify([ "Superseded by https://github.com/openclaw/openclaw/pull/400", @@ -1246,7 +1250,7 @@ test("apply-decisions rechecks covering PR freshness after coverage proof passes html_url: "https://github.com/openclaw/openclaw/pull/400", state: "closed", merged_at: "2026-05-02T00:00:00Z", - updated_at: "2026-05-01T00:05:00Z", + updated_at: "2026-05-01T00:00:00Z", body: "Changed after proof ran.", comments: [], labels: [], @@ -1293,6 +1297,7 @@ test("apply-decisions rechecks covering PR freshness after coverage proof passes assert.match( report.find((entry) => entry.action === "retry_pr_close_coverage_proof")?.reason ?? "", /linked canonical PR #400 changed after coverage proof/, + JSON.stringify(report, null, 2), ); assert.equal(existsSync(join(closedDir, "360.md")), false); } finally { diff --git a/test/apply-pr-coverage-proof-recheck.test.ts b/test/apply-pr-coverage-proof-recheck.test.ts index 5f1e9b5f0a..4a9bb5fa7a 100644 --- a/test/apply-pr-coverage-proof-recheck.test.ts +++ b/test/apply-pr-coverage-proof-recheck.test.ts @@ -1,9 +1,13 @@ import assert from "node:assert/strict"; +import { createHash } from "node:crypto"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import test from "node:test"; +import { createApplyProofFreshnessGuards } from "../dist/clawsweeper-apply-proof-freshness.js"; +import { completeActivityContextSymbol } from "../dist/clawsweeper-types.js"; import { + emptyReviewedPrActivityCursor, lowSignalCloseReport, promotionGhMock, reportWithSyncedReviewComment, @@ -13,6 +17,105 @@ import { withMockGh, } from "./helpers.ts"; +test("post-proof freshness accepts a prior-run automation receipt only with a complete match", () => { + const automationItemUpdatedAt = "2026-08-01T14:53:29Z"; + const context = { + sourceRevision: "a".repeat(64), + timelineRevision: "b".repeat(64), + pullReviewActivityCursor: emptyReviewedPrActivityCursor, + [completeActivityContextSymbol]: { + comments: [], + timeline: [], + pullReviewComments: [], + }, + }; + const guard = (completeReceiptMatches: boolean) => + createApplyProofFreshnessGuards({ + action: undefined, + automationItemUpdatedAt, + collectItemContext: () => context, + completeReviewActivityReceiptMatches: () => completeReceiptMatches, + contextHasNonAutomationActivityAfter: () => false, + coveringPrCloseCoveragePullRequestSnapshotSha256: () => "c".repeat(64), + currentProofState: () => ({ + cachedPrCloseCoverageProofGateResult: { + status: "allowed", + covering: { + number: 400, + provedAtMs: Date.parse("2026-08-01T14:53:30Z"), + snapshotSha256: "c".repeat(64), + updatedAt: automationItemUpdatedAt, + url: "https://github.com/openclaw/openclaw/pull/400", + proof: { + decision: "covered", + reason: "covered", + coveredWork: ["same behavior"], + uniqueSourceWork: [], + reviewerConcerns: [], + }, + }, + }, + prCloseCoverageProofGateChecked: true, + prCloseCoverageProofStartedAtMs: Date.parse("2026-08-01T14:53:30Z"), + storedHash: undefined, + storedUpdatedAt: "2026-08-01T14:52:41Z", + }), + expectedReviewActivityCursor: emptyReviewedPrActivityCursor, + fetchItem: () => ({ + state: "open", + item: { kind: "pull_request", updatedAt: automationItemUpdatedAt }, + }), + fetchReviewedPrActivityCursor: () => emptyReviewedPrActivityCursor, + freshPullRequestReviewHead: () => true, + GitHubRuntimeBudgetError: class extends Error {}, + itemKind: "pull_request", + itemSnapshotHash: () => "d".repeat(64), + number: 359, + reviewHasCompleteActivityIdentity: true, + reviewMarkdown: "---\ntype: pull_request\n---\n", + retryCloseCoverageCommandStatusOnlyUpdate: () => false, + selfMutationItemReceipts: [], + } as never); + + assert.equal(guard(true).postProofFreshnessBlock(), null); + assert.match(guard(false).postProofFreshnessBlock()?.reason ?? "", /updated_at changed/); +}); + +function sourceRevisionForTest(title: string): string { + return createHash("sha256") + .update( + JSON.stringify({ + title, + body: "Stale PR body.", + labels: [], + comments: [], + }), + ) + .digest("hex"); +} + +function timelineRevisionForTest( + events: Array<{ + id: number; + event: string; + actor: { login: string }; + commit_id?: string; + label?: { name: string }; + rename?: unknown; + }>, +): string { + const digestParts = events.map((event) => ({ + actor: event.actor.login, + commitId: event.commit_id ?? null, + event: event.event, + id: event.id, + label: event.label?.name ?? null, + rename: event.rename ?? null, + sourceIssue: null, + })); + return createHash("sha256").update(JSON.stringify(digestParts)).digest("hex"); +} + function boundDuplicateCloseComment(number: number, canonicalUrl: string): string { const markerFields = [ `item=${number}`, @@ -35,7 +138,7 @@ function boundDuplicateCloseComment(number: number, canonicalUrl: string): strin ].join("\n"); } -test("apply-decisions allows self-synced labels after proof with truncated context", () => { +test("apply-decisions fails closed when a self-mutation receipt is truncated", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -51,6 +154,8 @@ test("apply-decisions allows self-synced labels after proof with truncated conte number: 359, title: "Provider route fallback", close_reason: "duplicate_or_superseded", + item_source_revision: sourceRevisionForTest("Provider route fallback"), + pull_head_sha: "head-sha", labels: JSON.stringify(["status: 📣 needs proof"]), work_cluster_refs: JSON.stringify([ "Superseded by https://github.com/openclaw/openclaw/pull/400", @@ -121,16 +226,19 @@ test("apply-decisions allows self-synced labels after proof with truncated conte }>; assert.equal( report.some((entry) => entry.action === "closed"), - true, + false, + JSON.stringify(report, null, 2), ); assert.match(readFileSync(labelLogPath, "utf8"), /issue edit 359/); - assert.ok(existsSync(join(closedDir, "359.md"))); + assert.equal(report[0]?.action, "skipped_changed_since_review"); + assert.match(report[0]?.reason ?? "", /same-second activity requires a fresh review/); + assert.equal(existsSync(join(closedDir, "359.md")), false); } finally { rmSync(root, { recursive: true, force: true }); } }); -test("apply-decisions blocks post-proof human activity hidden by self-updates", () => { +test("apply-decisions blocks same-second human timeline activity hidden by self-updates", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -146,6 +254,8 @@ test("apply-decisions blocks post-proof human activity hidden by self-updates", number: 362, title: "Provider route fallback", close_reason: "duplicate_or_superseded", + item_source_revision: sourceRevisionForTest("Provider route fallback"), + pull_head_sha: "head-sha", labels: JSON.stringify(["status: 📣 needs proof"]), work_cluster_refs: JSON.stringify([ "Superseded by https://github.com/openclaw/openclaw/pull/400", @@ -165,17 +275,18 @@ test("apply-decisions blocks post-proof human activity hidden by self-updates", number: 362, title: "Provider route fallback", comment: synced.comment, - comments: [ + timeline: [ { id: 9362, - html_url: "https://github.com/openclaw/openclaw/pull/362#issuecomment-9362", - created_at: "2099-01-01T00:00:00Z", - updated_at: "2099-01-01T00:00:00Z", - user: { login: "contributor" }, - body: "Please do not close this yet.", + event: "assigned", + created_at: "2026-05-01T00:00:00Z", + actor: { login: "maintainer" }, }, ], - itemUpdatedAtAfterLabelSync: "2026-05-01T00:04:00Z", + // GitHub timestamps are second-granular: the human assignment and the + // bot-owned label mutation can leave the item timestamp equal to the + // reviewed value even though the activity receipt changed. + itemUpdatedAtAfterLabelSync: "2026-05-01T00:00:00Z", itemUpdatedAtAfterLabelSyncLogPath: labelLogPath, linkedPulls: { 400: { @@ -228,7 +339,7 @@ test("apply-decisions blocks post-proof human activity hidden by self-updates", false, ); assert.equal(report[0]?.action, "skipped_changed_since_review"); - assert.match(report[0]?.reason ?? "", /non-automation activity after coverage proof/); + assert.match(report[0]?.reason ?? "", /same-second activity requires a fresh review/); assert.match(readFileSync(labelLogPath, "utf8"), /issue edit 362/); assert.equal(existsSync(join(closedDir, "362.md")), false); } finally { @@ -236,6 +347,109 @@ test("apply-decisions blocks post-proof human activity hidden by self-updates", } }); +test("apply-decisions accepts same-second human activity already captured by the review", () => { + 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 proofLogPath = join(root, "proof.log"); + mkdirSync(itemsDir, { recursive: true }); + mkdirSync(plansDir, { recursive: true }); + const reviewedTimeline = [ + { + id: 9364, + event: "assigned", + created_at: "2026-05-01T00:00:00Z", + actor: { login: "maintainer" }, + }, + ]; + const synced = reportWithSyncedReviewComment( + lowSignalCloseReport({ + number: 364, + title: "Provider route fallback", + close_reason: "duplicate_or_superseded", + item_source_revision: sourceRevisionForTest("Provider route fallback"), + review_timeline_revision: timelineRevisionForTest(reviewedTimeline), + pull_head_sha: "head-sha", + work_cluster_refs: JSON.stringify([ + "Superseded by https://github.com/openclaw/openclaw/pull/400", + ]), + }).replace( + "Closing this PR because the branch is not a useful landing base.", + "Closing this PR as superseded by https://github.com/openclaw/openclaw/pull/400.", + ), + 364, + "duplicate_or_superseded", + ); + writeFileSync(join(itemsDir, "364.md"), synced.report, "utf8"); + + withMockGh( + root, + promotionGhMock({ + number: 364, + title: "Provider route fallback", + comment: synced.comment, + timeline: reviewedTimeline, + itemUpdatedAtAfterLabelSync: "2026-05-01T00:00:00Z", + linkedPulls: { + 400: { + number: 400, + title: "Provider cleanup", + html_url: "https://github.com/openclaw/openclaw/pull/400", + state: "closed", + merged_at: "2026-05-02T00:00:00Z", + body: "Includes the fallback route behavior from PR 364.", + comments: [], + labels: [], + }, + }, + }), + () => { + withMockCodexProof( + root, + { + type: "decision", + decision: "covered", + reason: "PR B carries forward PR A's fallback route behavior.", + invocationLogPath: proofLogPath, + }, + () => { + runApplyDecisionsForTest({ + itemsDir, + closedDir, + plansDir, + reportPath, + extraArgs: [ + "--target-repo", + "openclaw/openclaw", + "--apply-kind", + "all", + "--processed-limit", + "3", + ], + }); + }, + ); + }, + ); + + const report = JSON.parse(readFileSync(reportPath, "utf8")) as Array<{ + action: string; + reason: string; + }>; + assert.equal( + report.some((entry) => entry.action === "closed"), + true, + JSON.stringify(report, null, 2), + ); + assert.equal(existsSync(join(closedDir, "364.md")), true); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + test("apply-decisions keeps existing duplicate PR close proposals open when coverage proof fails", () => { const root = mkdtempSync(tmpPrefix); try { diff --git a/test/apply-same-author-pair-close.test.ts b/test/apply-same-author-pair-close.test.ts index ab5a217296..c428f6c973 100644 --- a/test/apply-same-author-pair-close.test.ts +++ b/test/apply-same-author-pair-close.test.ts @@ -344,6 +344,7 @@ test("apply-decisions records PR coverage proof retry before same-author pair sk number: 321, title: "Paired PR", author: "reporter", + item_source_revision: "unknown", close_reason: "duplicate_or_superseded", action_taken: "proposed_close", work_cluster_refs: JSON.stringify([ diff --git a/test/helpers.ts b/test/helpers.ts index 100d25bf10..0cda7fdced 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -465,6 +465,7 @@ export function lowSignalCloseReport(overrides = {}) { item_snapshot_hash: "reviewed-snapshot", item_created_at: "2026-05-01T00:00:00Z", item_updated_at: "2026-05-01T00:00:00Z", + review_timeline_revision: sha256ForTest("[]"), author_association: "CONTRIBUTOR", ...overrides, })}\n\n## Evidence\n\n- **branch shape:** PR diff is mostly unrelated provider churn around a tiny possible useful tweak\n\n## Close Comment\n\nClosing this PR because the branch is not a useful landing base.\n`; 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..31a6a93393 100644 --- a/test/review-structural-cache.test.ts +++ b/test/review-structural-cache.test.ts @@ -485,6 +485,57 @@ 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("automation timestamp collisions still require an identical structural receipt", () => { + const priorRecord = record(); + const currentRecord = record( + issueSnapshot({ + activityUpdatedAt: "2026-07-10T10:02:01Z", + labels: ["bug", "human-update"], + }), + ); + assert.equal( + decision({ + priorRecord, + currentRecord, + review: review({ automationItemUpdatedAt: "2026-07-10T10:02:01Z" }), + }).reason, + "source_changed", + ); + + const timelineChangedRecord = record( + issueSnapshot({ + activityUpdatedAt: "2026-07-10T10:02:01Z", + timeline: [ + ...issueSnapshot().timeline, + { + type: "AssignedEvent", + id: "AE_same_second", + createdAt: "2026-07-10T10:02:01Z", + author: "maintainer", + }, + ], + }), + ); + assert.equal( + decision({ + priorRecord, + currentRecord: timelineChangedRecord, + review: review({ automationItemUpdatedAt: "2026-07-10T10:02:01Z" }), + }).reason, + "source_changed", + ); }); test("changed target head forces issue hydration", () => { diff --git a/test/scheduler-policy.test.ts b/test/scheduler-policy.test.ts index 67111c3866..6bfb665f3a 100644 --- a/test/scheduler-policy.test.ts +++ b/test/scheduler-policy.test.ts @@ -205,7 +205,8 @@ test("scheduler keeps ambiguous post-sync activity due after review", () => { now, "current", ), - false, + true, + `${syncField} cannot suppress ambiguous activity before its local sync clock`, ); for (let lagSeconds = 1; lagSeconds <= 5; lagSeconds += 1) { assert.equal( @@ -225,6 +226,62 @@ test("scheduler keeps ambiguous post-sync activity due after review", () => { } }); +test("scheduler keeps an exact post-mutation timestamp eligible for structural verification", () => { + 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", + ), + true, + "timestamp equality alone cannot prove that the matching activity belongs to ClawSweeper", + ); + 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", + ); + assert.equal( + shouldReviewItem( + item({ + createdAt: "2026-07-24T06:00:00Z", + updatedAt: reviewedAt, + }), + { ...review, itemUpdatedAt: reviewedAt }, + Date.parse("2026-08-01T16:50:00Z"), + "current", + ), + true, + "an unchanged item timestamp in the reviewed second remains structurally ambiguous", + ); +}); + test("hot new item priority is protected from older activity churn", () => { const now = Date.parse("2026-04-30T12:00:00Z"); const review = (reviewedAt, itemUpdatedAt) => ({ @@ -725,6 +782,7 @@ test("CSW-088 suppresses only the immediate same-head and same-body hot-intake r const sourceRevision = "4055368d78b5997d42460145ba92e74397576bb4b0aaf91bb063725f2f1cb63d"; const pullStateDigest = "b".repeat(64); const reviewActivityCursor = `v1:0:${"c".repeat(64)}`; + const unchangedItemUpdatedAt = new Date(Date.parse(reviewedAt) - 1_000).toISOString(); const sameSnapshot = { reviewStatus: "complete", reviewedAt, @@ -736,13 +794,23 @@ test("CSW-088 suppresses only the immediate same-head and same-body hot-intake r currentSourceRevision: sourceRevision, currentPullStateDigest: pullStateDigest, currentReviewActivityCursor: reviewActivityCursor, - itemUpdatedAt: reviewedAt, - reviewItemUpdatedAt: reviewedAt, - currentItemUpdatedAt: reviewedAt, + itemUpdatedAt: unchangedItemUpdatedAt, + reviewItemUpdatedAt: unchangedItemUpdatedAt, + currentItemUpdatedAt: unchangedItemUpdatedAt, now, }; assert.equal(shouldSkipScheduledHotIntakeExactReviewForTest(sameSnapshot), true); + assert.equal( + shouldSkipScheduledHotIntakeExactReviewForTest({ + ...sameSnapshot, + itemUpdatedAt: reviewedAt, + reviewItemUpdatedAt: reviewedAt, + currentItemUpdatedAt: reviewedAt, + }), + false, + "same-second reviewed item activity requires the complete structural path", + ); assert.equal( shouldSkipScheduledHotIntakeExactReviewForTest({ ...sameSnapshot, @@ -784,7 +852,32 @@ test("CSW-088 suppresses only the immediate same-head and same-body hot-intake r currentItemUpdatedAt: new Date(Date.parse(reviewedAt) + 1).toISOString(), reviewCommentSyncedAt: new Date(Date.parse(reviewedAt) + 1).toISOString(), }), - true, + false, + "a local comment-sync clock cannot suppress same-second target activity", + ); + 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(), + }), + false, + "hot intake cannot suppress work from an item timestamp without a complete timeline receipt", + ); + assert.equal( + shouldSkipScheduledHotIntakeExactReviewForTest({ + ...sameSnapshot, + itemUpdatedAt: automationItemUpdatedAt, + currentItemUpdatedAt: automationItemUpdatedAt, + automationItemUpdatedAt, + reviewCommentSyncedAt: new Date(Date.parse(reviewedAt) + 1_000).toISOString(), + currentSourceRevision: "a".repeat(64), + }), + false, + "same-timestamp target activity remains due when the exact source receipt changed", ); assert.equal( shouldSkipScheduledHotIntakeExactReviewForTest({