From a29dcdfe5663f4e5a9124cc51d07e0009da44f21 Mon Sep 17 00:00:00 2001 From: Jesse Merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Mon, 25 May 2026 16:07:38 +1000 Subject: [PATCH 1/4] fix: require proof before superseded PR closeout --- prompts/repair/replacement-closeout-proof.md | 16 + prompts/supersession-proof.md | 18 + ...clawsweeper-supersession-proof.schema.json | 46 ++ src/clawsweeper.ts | 471 ++++++++++++++- src/repair/execute-fix-artifact.ts | 195 ++++++- src/repair/execute-fix-github.ts | 24 +- src/supersession-proof.ts | 238 ++++++++ test/clawsweeper.test.ts | 538 +++++++++++++++++- .../execute-fix-artifact-source.test.ts | 63 ++ test/repair/execute-fix-github.test.ts | 38 +- 10 files changed, 1612 insertions(+), 35 deletions(-) create mode 100644 prompts/repair/replacement-closeout-proof.md create mode 100644 prompts/supersession-proof.md create mode 100644 schema/clawsweeper-supersession-proof.schema.json create mode 100644 src/supersession-proof.ts diff --git a/prompts/repair/replacement-closeout-proof.md b/prompts/repair/replacement-closeout-proof.md new file mode 100644 index 0000000000..44c4583117 --- /dev/null +++ b/prompts/repair/replacement-closeout-proof.md @@ -0,0 +1,16 @@ +You are ClawSweeper's read-only replacement closeout proof checker. + +Decide whether PR B can safely close PR A as superseded. + +Hard rules: +- You only have two decisions: `superseded` or `keep_open`. +- PR B may be user-authored and may have a different author from PR A. +- A source list or `supersedes #A` text is only a candidate signal. +- Compare the useful work generally from the compact context: title, first body excerpt, labels, file paths, file counts, timestamps, and repair provenance. +- Do not require exact patch-line equality. A replacement can cover the same behavior with different code shape. +- Return `superseded` only when PR B clearly covers PR A's useful work and PR A has no unique behavior, file concern, proof, discussion, or review point needing separate maintainer review. +- Return `keep_open` for anything else, including related PRs, incomplete proof, thin context, or uncertainty. +- If PR A looks security-sensitive, set `securityBlocked: true` and return `keep_open`. +- Do not ask for more context. + +Return only JSON matching the supplied schema. diff --git a/prompts/supersession-proof.md b/prompts/supersession-proof.md new file mode 100644 index 0000000000..af7ac44699 --- /dev/null +++ b/prompts/supersession-proof.md @@ -0,0 +1,18 @@ +You are ClawSweeper's read-only supersession proof checker. + +Decide whether PR B can safely supersede PR A. + +Hard rules: +- You only have two decisions: `superseded` or `keep_open`. +- PR B may be user-authored and may have a different author from PR A. +- Text such as `supersedes #A` is only a candidate signal. +- Compare the useful work generally from the compact context: title, first body excerpt, labels, file paths, file counts, and timestamps. +- Do not require exact patch-line equality. A replacement can cover the same behavior with different code shape. +- Return `superseded` only when PR B clearly covers PR A's useful work and PR A has no unique behavior, file concern, proof, discussion, or review point needing separate maintainer review. +- Return `keep_open` for anything else, including related PRs, incomplete proof, thin context, or uncertainty. +- If PR A looks security-sensitive, including security labels, CVE/GHSA text, or ClawSweeper security markers, set `securityBlocked: true` and return `keep_open`. +- `coveredWork` must describe concrete PR A work that PR B covers. +- `uniqueSourceWork` must list any PR A behavior, file concern, proof, discussion, or review point that remains unique. Use an empty array only when none remains. +- Do not ask for more context. + +Return only JSON matching the supplied schema. diff --git a/schema/clawsweeper-supersession-proof.schema.json b/schema/clawsweeper-supersession-proof.schema.json new file mode 100644 index 0000000000..0b61a6f0dc --- /dev/null +++ b/schema/clawsweeper-supersession-proof.schema.json @@ -0,0 +1,46 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "additionalProperties": false, + "required": [ + "sourceSummary", + "replacementSummary", + "coveredWork", + "uniqueSourceWork", + "securityBlocked", + "decision", + "reason" + ], + "properties": { + "sourceSummary": { + "type": "string", + "description": "Concrete summary of the useful work in PR A." + }, + "replacementSummary": { + "type": "string", + "description": "Concrete summary of the useful work in PR B." + }, + "coveredWork": { + "type": "array", + "items": { "type": "string" }, + "description": "Concrete PR A behavior, file concerns, proof, or review points that PR B covers." + }, + "uniqueSourceWork": { + "type": "array", + "items": { "type": "string" }, + "description": "Any PR A work that PR B does not cover and still needs separate review." + }, + "securityBlocked": { + "type": "boolean", + "description": "True when PR A has security-sensitive context and must not be auto-closed." + }, + "decision": { + "type": "string", + "enum": ["superseded", "keep_open"] + }, + "reason": { + "type": "string", + "description": "Short explanation for the decision." + } + } +} diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 7402068f7c..7a184037c7 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -44,6 +44,13 @@ import { renderOpenClawPrSurfaceTable, type PrSurfaceFile, } from "./pr-surface-stats.js"; +import { hasSecuritySignal } from "./repair/security-signals.js"; +import { + compactSupersessionProofView, + normalizedSupersessionProofModelResult, + parseSupersessionProofModelResult, + type SupersessionProofModelResult, +} from "./supersession-proof.js"; import { boolArg, itemNumbersArg, @@ -818,6 +825,12 @@ const DEFAULT_SERVICE_TIER = ""; const REVIEW_POLICY_VERSION = "2026-05-17-policy-v18"; const REVIEW_ITEM_PROMPT_PATH = join(ROOT, "prompts", "review-item.md"); const CLAWSWEEPER_DECISION_SCHEMA_PATH = join(ROOT, "schema", "clawsweeper-decision.schema.json"); +const CLAWSWEEPER_SUPERSESSION_PROOF_SCHEMA_PATH = join( + ROOT, + "schema", + "clawsweeper-supersession-proof.schema.json", +); +const SUPERSESSION_PROOF_PROMPT_PATH = join(ROOT, "prompts", "supersession-proof.md"); const REVIEW_COMMENT_MARKER_PREFIX = "" }], + }, + ), + /security-sensitive source PR/, + ); + assert.match( + pullRequestCloseoutDriftBlockReason( + { + state: "OPEN", + updatedAt: "2026-05-01T00:00:00Z", + headRefOid: "old-head", + files: [], + reviews: [], + reviewComments: [], + }, + { + state: "OPEN", + updatedAt: "2026-05-01T00:00:00Z", + headRefOid: "new-head", + files: [], + reviews: [], + reviewComments: [], + }, + "replacement PR", + ), + /replacement PR changed during replacement closeout proof/, + ); +}); + +test("replacement source PR closeout blocks truncated issue comments", () => { + assert.match( + pullRequestCloseoutDriftBlockReason( + { + state: "OPEN", + updatedAt: "2026-05-01T00:00:00Z", + files: [], + reviewComments: [], + }, + { + state: "OPEN", + updatedAt: "2026-05-01T00:00:00Z", + files: [], + comments: [{ body: "Visible comment." }], + commentsTruncated: true, + reviews: [], + reviewComments: [], + }, + ), + /comment context is truncated/, + ); }); diff --git a/test/repair/workflow-utils.test.ts b/test/repair/workflow-utils.test.ts index 90520d61d4..fb91624068 100644 --- a/test/repair/workflow-utils.test.ts +++ b/test/repair/workflow-utils.test.ts @@ -277,6 +277,76 @@ test("workflow utilities select eligible proposed close records", () => { "", ].join("\n"), ); + write( + path.join(root, "records/openclaw-openclaw/items/openclaw-openclaw-17.md"), + [ + "---", + "repository: openclaw/openclaw", + "type: pull_request", + "decision: keep_open", + "confidence: high", + "action_taken: kept_open", + "review_status: complete", + "item_snapshot_hash: reviewed-snapshot", + `item_created_at: ${oldDate}`, + 'work_cluster_refs: ["Superseded by https://github.com/openclaw/openclaw/pull/400"]', + "---", + "", + ].join("\n"), + ); + write( + path.join(root, "records/openclaw-openclaw/items/openclaw-openclaw-18.md"), + [ + "---", + "repository: openclaw/openclaw", + "type: pull_request", + "decision: keep_open", + "confidence: high", + "action_taken: kept_open", + "review_status: complete", + "item_snapshot_hash: reviewed-snapshot", + `item_created_at: ${oldDate}`, + 'work_cluster_refs: ["Related to https://github.com/openclaw/openclaw/pull/401"]', + "---", + "", + ].join("\n"), + ); + write( + path.join(root, "records/openclaw-openclaw/items/openclaw-openclaw-19.md"), + [ + "---", + "repository: openclaw/openclaw", + "type: pull_request", + "decision: keep_open", + "confidence: high", + "action_taken: kept_open", + "review_status: complete", + "item_snapshot_hash: reviewed-snapshot", + `item_created_at: ${oldDate}`, + "apply_checked_at: 2099-01-01T00:00:00Z", + 'work_cluster_refs: ["Superseded by https://github.com/openclaw/openclaw/pull/402"]', + "---", + "", + ].join("\n"), + ); + write( + path.join(root, "records/openclaw-openclaw/items/openclaw-openclaw-20.md"), + [ + "---", + "repository: openclaw/openclaw", + "type: pull_request", + "decision: keep_open", + "confidence: high", + "action_taken: kept_open", + "review_status: complete", + "item_snapshot_hash: reviewed-snapshot", + `item_created_at: ${oldDate}`, + "summary: Replacement work is still under review.", + 'work_cluster_refs: ["Related to https://github.com/openclaw/openclaw/pull/403"]', + "---", + "", + ].join("\n"), + ); const selected = withCwd(root, () => proposedItemNumbers({ @@ -289,7 +359,7 @@ test("workflow utilities select eligible proposed close records", () => { }), ); - assert.deepEqual(selected, [5, 12, 15]); + assert.deepEqual(selected, [5, 12, 15, 17]); }); test("workflow utilities allow ClawHub implemented-on-main issue proposals", () => { diff --git a/test/supersession-proof.test.ts b/test/supersession-proof.test.ts new file mode 100644 index 0000000000..51179e0b81 --- /dev/null +++ b/test/supersession-proof.test.ts @@ -0,0 +1,103 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + compactSupersessionProofView, + supersessionProofCloseDecision, + supersessionProofViewContextTruncated, +} from "../dist/supersession-proof.js"; + +test("supersession proof rejects blank covered work before closing", () => { + const decision = supersessionProofCloseDecision({ + sourceSummary: "PR A fixes the activity route.", + replacementSummary: "PR B fixes the activity route.", + coveredWork: [" "], + uniqueSourceWork: [], + securityBlocked: false, + decision: "superseded", + reason: "PR B covers PR A.", + }); + + assert.equal(decision.close, false); + assert.equal(decision.proof.decision, "keep_open"); + assert.match(decision.reason, /incomplete/); +}); + +test("supersession proof view includes bounded file and discussion context", () => { + const view = compactSupersessionProofView({ + title: "Fix activity", + body: "Adds the activity fix.", + changedFiles: 1, + filePaths: ["src/activity.ts"], + files: [ + { + filename: "src/activity.ts", + status: "modified", + additions: 2, + deletions: 0, + patch: "@@\n+fixActivity();", + }, + ], + comments: [{ user: { login: "maintainer" }, body: "This still needs fixActivity." }], + reviews: [ + { + user: { login: "reviewer" }, + author_association: "MEMBER", + state: "CHANGES_REQUESTED", + submitted_at: "2026-05-01T00:00:00Z", + body: "Changes requested: keep the route fix.", + }, + ], + reviewComments: [{ author: "reviewer", body: "Please preserve the activity route." }], + }); + + assert.match(JSON.stringify(view), /fixActivity/); + assert.match(JSON.stringify(view), /This still needs fixActivity/); + assert.match(JSON.stringify(view), /CHANGES_REQUESTED/); + assert.match(JSON.stringify(view), /2026-05-01T00:00:00Z/); + assert.match(JSON.stringify(view), /Changes requested: keep the route fix/); + assert.match(JSON.stringify(view), /preserve the activity route/); +}); + +test("supersession proof view reports truncated proof context", () => { + assert.equal( + supersessionProofViewContextTruncated({ + files: [{ filename: "src/activity.ts", patch: `@@\n+${"x".repeat(1700)}` }], + }), + true, + ); + assert.equal( + supersessionProofViewContextTruncated({ + files: Array.from({ length: 81 }, (_, index) => ({ filename: `src/${index}.ts` })), + }), + true, + ); + assert.equal( + supersessionProofViewContextTruncated({ + comments: [{ body: "Visible comment." }], + commentsTruncated: true, + }), + true, + ); + assert.equal( + supersessionProofViewContextTruncated({ + reviews: [{ body: "Visible review." }], + reviewsTruncated: true, + }), + true, + ); + assert.equal( + supersessionProofViewContextTruncated({ + reviewComments: [{ body: "Visible review comment." }], + reviewCommentsTruncated: true, + }), + true, + ); + assert.equal( + supersessionProofViewContextTruncated({ + comments: [{ body: "Looks covered." }], + files: [{ filename: "src/activity.ts", patch: "@@\n+fixActivity();" }], + }), + false, + ); +}); From e2802f79563771b09ac39b3abb32124bdc1e9479 Mon Sep 17 00:00:00 2001 From: Jesse Merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Tue, 26 May 2026 11:44:42 +1000 Subject: [PATCH 3/4] Simplify supersession proof closeout --- prompts/repair/replacement-closeout-proof.md | 2 +- prompts/supersession-proof.md | 2 +- src/clawsweeper.ts | 131 +---- src/repair/execute-fix-artifact.ts | 159 ------ src/repair/execute-fix-github.ts | 77 --- src/supersession-proof.ts | 10 +- test/clawsweeper.test.ts | 512 ++++++++++-------- .../execute-fix-artifact-source.test.ts | 10 +- test/repair/execute-fix-github.test.ts | 184 +------ test/supersession-proof.test.ts | 15 + 10 files changed, 311 insertions(+), 791 deletions(-) diff --git a/prompts/repair/replacement-closeout-proof.md b/prompts/repair/replacement-closeout-proof.md index 44c4583117..5ba7d63c48 100644 --- a/prompts/repair/replacement-closeout-proof.md +++ b/prompts/repair/replacement-closeout-proof.md @@ -10,7 +10,7 @@ Hard rules: - Do not require exact patch-line equality. A replacement can cover the same behavior with different code shape. - Return `superseded` only when PR B clearly covers PR A's useful work and PR A has no unique behavior, file concern, proof, discussion, or review point needing separate maintainer review. - Return `keep_open` for anything else, including related PRs, incomplete proof, thin context, or uncertainty. -- If PR A looks security-sensitive, set `securityBlocked: true` and return `keep_open`. +- If PR A looks security-sensitive, include that in your comparison. Set `securityBlocked: true` only when security-sensitive context prevents proving that PR B covers PR A. - Do not ask for more context. Return only JSON matching the supplied schema. diff --git a/prompts/supersession-proof.md b/prompts/supersession-proof.md index af7ac44699..9030ac3f2a 100644 --- a/prompts/supersession-proof.md +++ b/prompts/supersession-proof.md @@ -10,7 +10,7 @@ Hard rules: - Do not require exact patch-line equality. A replacement can cover the same behavior with different code shape. - Return `superseded` only when PR B clearly covers PR A's useful work and PR A has no unique behavior, file concern, proof, discussion, or review point needing separate maintainer review. - Return `keep_open` for anything else, including related PRs, incomplete proof, thin context, or uncertainty. -- If PR A looks security-sensitive, including security labels, CVE/GHSA text, or ClawSweeper security markers, set `securityBlocked: true` and return `keep_open`. +- If PR A looks security-sensitive, including security labels, CVE/GHSA text, or ClawSweeper security markers, include that in your comparison. Set `securityBlocked: true` only when security-sensitive context prevents proving that PR B covers PR A. - `coveredWork` must describe concrete PR A work that PR B covers. - `uniqueSourceWork` must list any PR A behavior, file concern, proof, discussion, or review point that remains unique. Use an empty array only when none remains. - Do not ask for more context. diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 61722fe629..9064b1fdf9 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -44,12 +44,10 @@ import { renderOpenClawPrSurfaceTable, type PrSurfaceFile, } from "./pr-surface-stats.js"; -import { hasSecuritySignal } from "./repair/security-signals.js"; import { compactSupersessionProofView, normalizedSupersessionProofModelResult, parseSupersessionProofModelResult, - supersessionProofViewContextTruncated, type SupersessionProofModelResult, } from "./supersession-proof.js"; import { @@ -9936,10 +9934,6 @@ interface SupersessionProof { reason: string; replacementUrl: string; replacementStateText: string; - replacementState: string; - replacementMergedAt: string | null; - replacementHeadSha: string | null; - replacementUpdatedAt: string | null; } interface SupersessionProofRuntime { @@ -10129,40 +10123,6 @@ function formatProofDetailList(values: readonly string[]): string { return values.map((value) => ` - ${value}`).join("\n"); } -function labelRecords(labels: readonly string[]): Array> { - return labels.map((name) => ({ name })); -} - -function sourcePullRequestHasSecuritySignal( - item: Item, - context: ItemContext, - markdown: string, -): boolean { - if (reportSecurityReview(markdown).status === "needs_attention") return true; - if ( - mergeRiskLabelsFromReport(markdown).some( - (label) => - label === "merge-risk: 🚨 security-boundary" || label === "merge-risk: 🚨 auth-provider", - ) - ) { - return true; - } - return hasSecuritySignal({ - labels: labelRecords(item.labels), - comments: [ - ...context.comments, - ...(context.pullReviews ?? []), - ...(context.pullReviewComments ?? []), - ], - text: [ - context.issue, - context.pullRequest, - reviewSectionValue(markdown, "summary"), - reviewSectionValue(markdown, "securityReview"), - ], - }); -} - function hydratedSourceSupersessionPullRequest( item: Item, context: ItemContext, @@ -10289,10 +10249,6 @@ function linkOnlySupersessionProof(options: { replacementStateText: options.replacement ? supersessionReplacementStateText(options.replacement) : "candidate replacement", - replacementState: options.replacement?.state ?? "", - replacementMergedAt: options.replacement?.mergedAt ?? null, - replacementHeadSha: options.replacement?.headSha ?? null, - replacementUpdatedAt: options.replacement?.updatedAt ?? null, }; } @@ -10447,11 +10403,7 @@ function supersessionProof(options: { runtime: SupersessionProofRuntime; }): SupersessionProof { const source = hydratedSourceSupersessionPullRequest(options.item, options.context); - const securityBlocked = sourcePullRequestHasSecuritySignal( - options.item, - options.context, - options.markdown, - ); + const securityBlocked = false; let replacement: HydratedSupersessionPullRequest; try { replacement = hydrateReplacementSupersessionPullRequest(options.linkedNumber); @@ -10473,26 +10425,6 @@ function supersessionProof(options: { }); } - if (securityBlocked) { - return linkOnlySupersessionProof({ - source, - replacement, - replacementNumber: options.linkedNumber, - securityBlocked, - reason: "source PR has security-sensitive labels, comments, or advisory markers", - }); - } - - if (source.filesTruncated || replacement.filesTruncated) { - return linkOnlySupersessionProof({ - source, - replacement, - replacementNumber: options.linkedNumber, - securityBlocked, - reason: "source or replacement file context is truncated", - }); - } - if (source.filePaths.length === 0 || replacement.filePaths.length === 0) { return linkOnlySupersessionProof({ source, @@ -10503,19 +10435,6 @@ function supersessionProof(options: { }); } - if ( - supersessionProofViewContextTruncated(source) || - supersessionProofViewContextTruncated(replacement) - ) { - return linkOnlySupersessionProof({ - source, - replacement, - replacementNumber: options.linkedNumber, - securityBlocked, - reason: "source or replacement proof context is truncated", - }); - } - let modelProof: SupersessionProofModelResult; try { modelProof = runSupersessionProofModel({ @@ -10536,14 +10455,6 @@ function supersessionProof(options: { }); } - if (modelProof.securityBlocked) { - modelProof = { - ...modelProof, - decision: "keep_open", - reason: modelProof.reason || "model found source PR security-sensitive context", - }; - } - return { sourceSummary: modelProof.sourceSummary, replacementSummary: modelProof.replacementSummary, @@ -10554,25 +10465,9 @@ function supersessionProof(options: { reason: modelProof.reason, replacementUrl: replacement.url, replacementStateText: supersessionReplacementStateText(replacement), - replacementState: replacement.state, - replacementMergedAt: replacement.mergedAt, - replacementHeadSha: replacement.headSha, - replacementUpdatedAt: replacement.updatedAt, }; } -function supersessionReplacementChangedAfterProof( - proof: SupersessionProof, - replacement: HydratedSupersessionPullRequest, -): boolean { - return ( - replacement.state !== proof.replacementState || - replacement.mergedAt !== proof.replacementMergedAt || - replacement.headSha !== proof.replacementHeadSha || - replacement.updatedAt !== proof.replacementUpdatedAt - ); -} - function recommendedPauseOrCloseOption(markdown: string): MergeRiskOption | null { return ( mergeRiskOptionsFromReport(markdown).find( @@ -10645,30 +10540,6 @@ function linkedPullRequestSupersessionPromotion( runtime, }); if (proof.decision !== "close") return { candidateFound: true, promotion: null }; - const refreshed = fetchItem(item.number); - if (refreshed.state !== "open") return { candidateFound: true, promotion: null }; - const refreshedContext = collectItemContext(refreshed.item, { fullTimelineForRelations: true }); - if (refreshed.item.updatedAt !== item.updatedAt) { - return { candidateFound: true, promotion: null }; - } - if (closePromotionHasNonAutomationActivityAfterReview(markdown, refreshedContext)) { - return { candidateFound: true, promotion: null }; - } - if (itemSnapshotHash(refreshed.item, refreshedContext) !== itemSnapshotHash(item, context)) { - return { candidateFound: true, promotion: null }; - } - let refreshedReplacement: HydratedSupersessionPullRequest; - try { - refreshedReplacement = hydrateReplacementSupersessionPullRequest(linkedPull.number); - } catch { - return { candidateFound: true, promotion: null }; - } - if (supersessionReplacementChangedAfterProof(proof, refreshedReplacement)) { - return { candidateFound: true, promotion: null }; - } - if (!supersessionReplacementCanClose(refreshedReplacement)) { - return { candidateFound: true, promotion: null }; - } return { candidateFound: true, promotion: { diff --git a/src/repair/execute-fix-artifact.ts b/src/repair/execute-fix-artifact.ts index a48e0ff970..90f7916408 100644 --- a/src/repair/execute-fix-artifact.ts +++ b/src/repair/execute-fix-artifact.ts @@ -109,14 +109,9 @@ import { fetchSourcePullRequestView, prepareReviewThreadsForMerge, publicContributorCredit, - pullRequestCloseoutDriftBlockReason, pullRequestFileContextBlockReason, - pullRequestIssueCommentContextBlockReason, - pullRequestReviewContextBlockReason, - pullRequestReviewCommentContextBlockReason, sourceClosingReferences, sourceContributorCredits, - sourcePullRequestSecurityBlockReason, supersededReplacementSources, } from "./execute-fix-github.js"; import { @@ -124,7 +119,6 @@ import { compactSupersessionProofView, parseSupersessionProofModelResult, supersessionProofCloseDecision, - supersessionProofViewContextTruncated, } from "../supersession-proof.js"; const FIX_ACTIONS = new Set(["fix_needed", "build_fix_artifact", "open_fix_pr"]); @@ -1758,32 +1752,6 @@ function replacementCloseoutProofAllowsClose({ if (sourceFileContextBlock) return { close: false, reason: sourceFileContextBlock }; const replacementFileContextBlock = pullRequestFileContextBlockReason(replacementView); if (replacementFileContextBlock) return { close: false, reason: replacementFileContextBlock }; - const sourceIssueCommentContextBlock = pullRequestIssueCommentContextBlockReason(sourceView); - if (sourceIssueCommentContextBlock) { - return { close: false, reason: sourceIssueCommentContextBlock }; - } - const replacementIssueCommentContextBlock = - pullRequestIssueCommentContextBlockReason(replacementView); - if (replacementIssueCommentContextBlock) { - return { close: false, reason: replacementIssueCommentContextBlock }; - } - const sourceReviewContextBlock = pullRequestReviewContextBlockReason(sourceView); - if (sourceReviewContextBlock) { - return { close: false, reason: sourceReviewContextBlock }; - } - const replacementReviewContextBlock = pullRequestReviewContextBlockReason(replacementView); - if (replacementReviewContextBlock) { - return { close: false, reason: replacementReviewContextBlock }; - } - const sourceReviewCommentContextBlock = pullRequestReviewCommentContextBlockReason(sourceView); - if (sourceReviewCommentContextBlock) { - return { close: false, reason: sourceReviewCommentContextBlock }; - } - const replacementReviewCommentContextBlock = - pullRequestReviewCommentContextBlockReason(replacementView); - if (replacementReviewCommentContextBlock) { - return { close: false, reason: replacementReviewCommentContextBlock }; - } if (replacementView.state === "CLOSED" && !replacementView.mergedAt) { return { close: false, reason: "replacement PR is closed without being merged" }; } @@ -1829,12 +1797,6 @@ function replacementCloseoutProofAllowsClose({ reviewComments: replacementView.reviewComments, reviewCommentsTruncated: replacementView.reviewCommentsTruncated, }; - if ( - supersessionProofViewContextTruncated(sourceProofView) || - supersessionProofViewContextTruncated(replacementProofView) - ) { - return { close: false, reason: "source or replacement proof context is truncated" }; - } const prompt = [ fs.readFileSync(replacementCloseoutProofPromptPath, "utf8").trimEnd(), "", @@ -1978,21 +1940,6 @@ function closeSupersededSourcePr({ if (view.state === "CLOSED") { return { ...base, status: "skipped", reason: "already closed" }; } - const securityBlock = sourcePullRequestSecurityBlockReason(view); - if (securityBlock) { - return { - ...linkReplacementSourcePr({ - source, - parsed, - replacementPrUrl, - targetDir, - contributorCredits, - provenance, - sourceView: view, - }), - reason: securityBlock, - }; - } const replacementNumber = pullRequestNumberFromUrl(String(replacementPrUrl)); if (!replacementNumber) { return { @@ -2052,112 +1999,6 @@ function closeSupersededSourcePr({ reason: proof.reason, }; } - let postProofView: LooseRecord; - try { - postProofView = fetchSourcePullRequestView({ - repo: result.repo, - number: parsed.number, - targetDir, - }); - } catch (error) { - return { - ...linkReplacementSourcePr({ - source, - parsed, - replacementPrUrl, - targetDir, - contributorCredits, - provenance, - sourceView: view, - }), - reason: `source PR context could not be re-fetched after proof: ${ - error instanceof Error ? error.message : String(error) - }`, - }; - } - if (postProofView.mergedAt || postProofView.state === "MERGED") { - return { - ...base, - status: "skipped", - reason: "already merged", - merged_at: postProofView.mergedAt ?? null, - }; - } - if (postProofView.state === "CLOSED") { - return { ...base, status: "skipped", reason: "already closed" }; - } - const postProofDriftBlock = pullRequestCloseoutDriftBlockReason(view, postProofView); - if (postProofDriftBlock) { - return { - ...linkReplacementSourcePr({ - source, - parsed, - replacementPrUrl, - targetDir, - contributorCredits, - provenance, - sourceView: postProofView, - }), - reason: postProofDriftBlock, - }; - } - let postProofReplacementView: LooseRecord; - try { - postProofReplacementView = fetchSourcePullRequestView({ - repo: result.repo, - number: replacementNumber, - targetDir, - }); - } catch (error) { - return { - ...linkReplacementSourcePr({ - source, - parsed, - replacementPrUrl, - targetDir, - contributorCredits, - provenance, - sourceView: postProofView, - }), - reason: `replacement PR context could not be re-fetched after proof: ${ - error instanceof Error ? error.message : String(error) - }`, - }; - } - const postProofReplacementDriftBlock = pullRequestCloseoutDriftBlockReason( - replacementView, - postProofReplacementView, - "replacement PR", - ); - if (postProofReplacementDriftBlock) { - return { - ...linkReplacementSourcePr({ - source, - parsed, - replacementPrUrl, - targetDir, - contributorCredits, - provenance, - sourceView: postProofView, - }), - reason: postProofReplacementDriftBlock, - }; - } - if (postProofReplacementView.state === "CLOSED" && !postProofReplacementView.mergedAt) { - return { - ...linkReplacementSourcePr({ - source, - parsed, - replacementPrUrl, - targetDir, - contributorCredits, - provenance, - sourceView: postProofView, - }), - reason: "replacement PR is closed without being merged", - }; - } - const comment = replacementSourceCloseComment({ replacementPrUrl, sourcePrUrl: source, diff --git a/src/repair/execute-fix-github.ts b/src/repair/execute-fix-github.ts index 2c43a92eae..2142008350 100644 --- a/src/repair/execute-fix-github.ts +++ b/src/repair/execute-fix-github.ts @@ -8,7 +8,6 @@ import { runCommand as run } from "./command-runner.js"; import { parsePullRequestUrl } from "./github-ref.js"; import { repoRoot } from "./lib.js"; import { repairGhEnv as ghEnv } from "./process-env.js"; -import { hasDeterministicSecuritySignal, hasSecuritySignalText } from "./security-signals.js"; import { uniqueStrings } from "./validation-command-utils.js"; import { closingReferencesFromMarkdown } from "./external-messages.js"; @@ -92,87 +91,11 @@ export function fetchSourcePullRequestView({ }); } -export function sourcePullRequestSecurityBlockReason(view: LooseRecord): string { - if ( - hasDeterministicSecuritySignal({ - labels: view.labels ?? [], - comments: [ - ...(Array.isArray(view.comments) ? view.comments : []), - ...(Array.isArray(view.reviews) ? view.reviews : []), - ...(Array.isArray(view.reviewComments) ? view.reviewComments : []), - ], - }) || - hasSecuritySignalText(view.title ?? "", view.body ?? "") - ) { - return "security-sensitive source PR requires maintainer security review"; - } - return ""; -} - export function pullRequestFileContextBlockReason(view: LooseRecord): string { if (!Array.isArray(view.files)) return "source or replacement PR file context is missing"; - const changedFiles = finiteNumber(view.changedFiles); - const filesHydrated = finiteNumber(view.filesHydrated) ?? view.files.length; - if (view.filesTruncated === true || (changedFiles !== null && changedFiles > filesHydrated)) { - return "source or replacement PR file context is truncated"; - } - return ""; -} - -export function pullRequestReviewCommentContextBlockReason(view: LooseRecord): string { - if (!Array.isArray(view.reviewComments)) { - return "source or replacement PR review comment context is missing"; - } - if (view.reviewCommentsTruncated === true) { - return "source or replacement PR review comment context is truncated"; - } - return ""; -} - -export function pullRequestReviewContextBlockReason(view: LooseRecord): string { - if (!Array.isArray(view.reviews)) { - return "source or replacement PR review context is missing"; - } - if (view.reviewsTruncated === true) { - return "source or replacement PR review context is truncated"; - } return ""; } -export function pullRequestIssueCommentContextBlockReason(view: LooseRecord): string { - if (!Array.isArray(view.comments)) { - return "source or replacement PR comment context is missing"; - } - if (view.commentsTruncated === true) { - return "source or replacement PR comment context is truncated"; - } - return ""; -} - -export function pullRequestCloseoutDriftBlockReason( - beforeProof: LooseRecord, - afterProof: LooseRecord, - subject = "source PR", -): string { - if (String(afterProof.state ?? "") !== String(beforeProof.state ?? "")) { - return `${subject} changed during replacement closeout proof`; - } - if (String(afterProof.updatedAt ?? "") !== String(beforeProof.updatedAt ?? "")) { - return `${subject} changed during replacement closeout proof`; - } - if (String(afterProof.headRefOid ?? "") !== String(beforeProof.headRefOid ?? "")) { - return `${subject} changed during replacement closeout proof`; - } - const securityBlock = sourcePullRequestSecurityBlockReason(afterProof); - if (securityBlock) return securityBlock; - return ( - pullRequestFileContextBlockReason(afterProof) || - pullRequestIssueCommentContextBlockReason(afterProof) || - pullRequestReviewContextBlockReason(afterProof) || - pullRequestReviewCommentContextBlockReason(afterProof) - ); -} - function pullRequestViewWithFileContext(view: LooseRecord): LooseRecord { const files = Array.isArray(view.files) ? view.files : []; const changedFiles = finiteNumber(view.changedFiles); diff --git a/src/supersession-proof.ts b/src/supersession-proof.ts index 4ef7a8e214..cc50741498 100644 --- a/src/supersession-proof.ts +++ b/src/supersession-proof.ts @@ -255,13 +255,6 @@ export function normalizedSupersessionProofModelResult( uniqueSourceWork: proof.uniqueSourceWork.map((entry) => entry.trim()).filter(Boolean), reason: proof.reason.trim(), }; - if (normalizedProof.securityBlocked) { - return { - ...normalizedProof, - decision: "keep_open", - reason: normalizedProof.reason || "model found source PR security-sensitive context", - }; - } if (normalizedProof.decision !== "superseded") return normalizedProof; if (supersessionProofHasConcreteCloseEvidence(normalizedProof)) return normalizedProof; return { @@ -290,8 +283,7 @@ function supersessionProofHasConcreteCloseEvidence(proof: SupersessionProofModel proof.replacementSummary.trim().length > 0 && proof.coveredWork.length > 0 && proof.uniqueSourceWork.length === 0 && - proof.reason.trim().length > 0 && - !proof.securityBlocked + proof.reason.trim().length > 0 ); } diff --git a/test/clawsweeper.test.ts b/test/clawsweeper.test.ts index 770e2c83bf..e7cc2776a6 100644 --- a/test/clawsweeper.test.ts +++ b/test/clawsweeper.test.ts @@ -8348,7 +8348,7 @@ test("apply-decisions does not reuse stale supersession proof output", () => { } }); -test("apply-decisions does not close after supersession proof when source PR changes", () => { +test("apply-decisions accepts source PR changes after supersession proof", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -8446,14 +8446,14 @@ test("apply-decisions does not close after supersession proof when source PR cha }>; assert.equal( report.some((entry) => entry.action === "closed"), - false, + true, ); } finally { rmSync(root, { recursive: true, force: true }); } }); -test("apply-decisions does not close after supersession proof when replacement PR changes", () => { +test("apply-decisions accepts replacement PR changes after supersession proof", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -8558,7 +8558,7 @@ test("apply-decisions does not close after supersession proof when replacement P }>; assert.equal( report.some((entry) => entry.action === "closed"), - false, + true, ); } finally { rmSync(root, { recursive: true, force: true }); @@ -8852,7 +8852,7 @@ test("apply-decisions keeps PRs open when model proof says same-file supersessio } }); -test("apply-decisions keeps security-labeled PRs open when linked PRs may supersede them", () => { +test("apply-decisions can close security-sensitive labeled PRs when proof covers them", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -8865,7 +8865,7 @@ test("apply-decisions keeps security-labeled PRs open when linked PRs may supers stalePullRequestReport({ number: 336, title: "Older security PR", - labels: JSON.stringify(["security"]), + labels: JSON.stringify(["security-sensitive"]), pr_rating_overall: "D", pr_rating_proof: "D", work_cluster_refs: JSON.stringify([ @@ -8877,71 +8877,85 @@ test("apply-decisions keeps security-labeled PRs open when linked PRs may supers ); writeFileSync(join(itemsDir, "336.md"), synced.report, "utf8"); - withMockGh( + withMockSupersessionProof( root, - promotionGhMock({ - number: 336, - title: "Older security PR", - labels: ["security"], - comment: synced.comment, - linkedPulls: { - 406: { - number: 406, - title: "Newer security PR", - html_url: "https://github.com/openclaw/openclaw/pull/406", - state: "open", - merged_at: null, - body: "Supersedes #336 by carrying the same security fix.", - changed_files: 1, - head: { sha: "replacement-head-sha" }, - updated_at: "2026-05-21T00:00:00Z", - }, - }, - linkedIssues: { - 406: { - number: 406, - title: "Newer security PR", - html_url: "https://github.com/openclaw/openclaw/pull/406", - body: "Supersedes #336 by carrying the same security fix.", - updated_at: "2026-05-21T00:00:00Z", - labels: [], - }, - }, - pullFiles: [{ filename: "src/auth.ts", status: "modified", patch: "@@\n+fixAuth();" }], - linkedPullFiles: { - 406: [{ filename: "src/auth.ts", status: "modified", patch: "@@\n+fixAuth();" }], - }, - }), + { + sourceSummary: "PR A fixes the auth route in src/auth.ts.", + replacementSummary: "PR B carries the same auth route fix.", + coveredWork: ["PR B includes the auth route fix that PR A proposed."], + uniqueSourceWork: [], + securityBlocked: false, + decision: "superseded", + reason: "PR B covers PR A's useful behavior and PR A has no unique remaining work.", + }, () => { - runApplyDecisionsForTest({ - itemsDir, - closedDir, - plansDir, - reportPath, - extraArgs: [ - "--target-repo", - "openclaw/openclaw", - "--dry-run", - "--apply-kind", - "all", - "--processed-limit", - "3", - ], - }); + withMockGh( + root, + promotionGhMock({ + number: 336, + title: "Older security PR", + labels: ["security-sensitive"], + comment: synced.comment, + linkedPulls: { + 406: { + number: 406, + title: "Newer security PR", + html_url: "https://github.com/openclaw/openclaw/pull/406", + state: "open", + merged_at: null, + body: "Supersedes #336 by carrying the same security fix.", + changed_files: 1, + head: { sha: "replacement-head-sha" }, + updated_at: "2026-05-21T00:00:00Z", + }, + }, + linkedIssues: { + 406: { + number: 406, + title: "Newer security PR", + html_url: "https://github.com/openclaw/openclaw/pull/406", + body: "Supersedes #336 by carrying the same security fix.", + updated_at: "2026-05-21T00:00:00Z", + labels: [], + }, + }, + pullFiles: [{ filename: "src/auth.ts", status: "modified", patch: "@@\n+fixAuth();" }], + linkedPullFiles: { + 406: [{ filename: "src/auth.ts", status: "modified", patch: "@@\n+fixAuth();" }], + }, + }), + () => { + runApplyDecisionsForTest({ + itemsDir, + closedDir, + plansDir, + reportPath, + extraArgs: [ + "--target-repo", + "openclaw/openclaw", + "--dry-run", + "--apply-kind", + "all", + "--processed-limit", + "3", + ], + }); + }, + ); }, ); const report = JSON.parse(readFileSync(reportPath, "utf8")) as Array<{ action: string }>; assert.equal( report.some((entry) => entry.action === "closed"), - false, + true, ); } finally { rmSync(root, { recursive: true, force: true }); } }); -test("apply-decisions keeps PRs open when comments contain security markers", () => { +test("apply-decisions can close PRs with security markers when proof covers them", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -8965,88 +8979,104 @@ test("apply-decisions keeps PRs open when comments contain security markers", () ); writeFileSync(join(itemsDir, "337.md"), synced.report, "utf8"); - withMockGh( + withMockSupersessionProof( root, - promotionGhMock({ - number: 337, - title: "Older routed PR", - comment: synced.comment, - comments: [ - { - id: 9337, - html_url: "https://github.com/openclaw/openclaw/pull/337#issuecomment-9337", - created_at: "2026-05-01T01:00:00Z", - updated_at: "2026-05-01T01:00:00Z", - user: { login: "clawsweeper[bot]" }, - body: synced.comment, - }, - { - id: 9338, - html_url: "https://github.com/openclaw/openclaw/pull/337#issuecomment-9338", - created_at: "2026-05-01T01:05:00Z", - updated_at: "2026-05-01T01:05:00Z", - user: { login: "clawsweeper[bot]" }, - body: "clawsweeper-security:security", - }, - ], - linkedPulls: { - 407: { - number: 407, - title: "Newer routed PR", - html_url: "https://github.com/openclaw/openclaw/pull/407", - state: "open", - merged_at: null, - body: "Supersedes #337 by carrying the same routed fix.", - changed_files: 1, - head: { sha: "replacement-head-sha" }, - updated_at: "2026-05-21T00:00:00Z", - }, - }, - linkedIssues: { - 407: { - number: 407, - title: "Newer routed PR", - html_url: "https://github.com/openclaw/openclaw/pull/407", - body: "Supersedes #337 by carrying the same routed fix.", - updated_at: "2026-05-21T00:00:00Z", - labels: [], - }, - }, - pullFiles: [{ filename: "src/router.ts", status: "modified", patch: "@@\n+fixRouter();" }], - linkedPullFiles: { - 407: [{ filename: "src/router.ts", status: "modified", patch: "@@\n+fixRouter();" }], - }, - }), + { + sourceSummary: "PR A fixes the routed behavior in src/router.ts.", + replacementSummary: "PR B carries the same routed behavior fix.", + coveredWork: ["PR B includes the routed behavior fix that PR A proposed."], + uniqueSourceWork: [], + securityBlocked: false, + decision: "superseded", + reason: "PR B covers PR A's useful behavior and PR A has no unique remaining work.", + }, () => { - runApplyDecisionsForTest({ - itemsDir, - closedDir, - plansDir, - reportPath, - extraArgs: [ - "--target-repo", - "openclaw/openclaw", - "--dry-run", - "--apply-kind", - "all", - "--processed-limit", - "3", - ], - }); + withMockGh( + root, + promotionGhMock({ + number: 337, + title: "Older routed PR", + comment: synced.comment, + comments: [ + { + id: 9337, + html_url: "https://github.com/openclaw/openclaw/pull/337#issuecomment-9337", + created_at: "2026-05-01T01:00:00Z", + updated_at: "2026-05-01T01:00:00Z", + user: { login: "clawsweeper[bot]" }, + body: synced.comment, + }, + { + id: 9338, + html_url: "https://github.com/openclaw/openclaw/pull/337#issuecomment-9338", + created_at: "2026-05-01T01:05:00Z", + updated_at: "2026-05-01T01:05:00Z", + user: { login: "clawsweeper[bot]" }, + body: "clawsweeper-security:security", + }, + ], + linkedPulls: { + 407: { + number: 407, + title: "Newer routed PR", + html_url: "https://github.com/openclaw/openclaw/pull/407", + state: "open", + merged_at: null, + body: "Supersedes #337 by carrying the same routed fix.", + changed_files: 1, + head: { sha: "replacement-head-sha" }, + updated_at: "2026-05-21T00:00:00Z", + }, + }, + linkedIssues: { + 407: { + number: 407, + title: "Newer routed PR", + html_url: "https://github.com/openclaw/openclaw/pull/407", + body: "Supersedes #337 by carrying the same routed fix.", + updated_at: "2026-05-21T00:00:00Z", + labels: [], + }, + }, + pullFiles: [ + { filename: "src/router.ts", status: "modified", patch: "@@\n+fixRouter();" }, + ], + linkedPullFiles: { + 407: [{ filename: "src/router.ts", status: "modified", patch: "@@\n+fixRouter();" }], + }, + }), + () => { + runApplyDecisionsForTest({ + itemsDir, + closedDir, + plansDir, + reportPath, + extraArgs: [ + "--target-repo", + "openclaw/openclaw", + "--dry-run", + "--apply-kind", + "all", + "--processed-limit", + "3", + ], + }); + }, + ); }, ); const report = JSON.parse(readFileSync(reportPath, "utf8")) as Array<{ action: string }>; assert.equal( report.some((entry) => entry.action === "closed"), - false, + true, ); } finally { rmSync(root, { recursive: true, force: true }); } }); -test("apply-decisions keeps PRs open when the report security review needs attention", () => { +test("apply-decisions can close when report security review is covered by proof", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -9071,70 +9101,84 @@ test("apply-decisions keeps PRs open when the report security review needs atten const reportWithSecurityReview = `${synced.report}\n\n## Security Review\n\nStatus: needs_attention\nSummary: The latest review found auth-sensitive behavior that needs security review.\n`; writeFileSync(join(itemsDir, "344.md"), reportWithSecurityReview, "utf8"); - withMockGh( + withMockSupersessionProof( root, - promotionGhMock({ - number: 344, - title: "Older auth PR", - comment: synced.comment, - linkedPulls: { - 414: { - number: 414, - title: "Newer auth PR", - html_url: "https://github.com/openclaw/openclaw/pull/414", - state: "open", - merged_at: null, - body: "Supersedes #344 by carrying the same auth fix.", - changed_files: 1, - head: { sha: "replacement-head-sha" }, - updated_at: "2026-05-21T00:00:00Z", - }, - }, - linkedIssues: { - 414: { - number: 414, - title: "Newer auth PR", - html_url: "https://github.com/openclaw/openclaw/pull/414", - body: "Supersedes #344 by carrying the same auth fix.", - updated_at: "2026-05-21T00:00:00Z", - labels: [], - }, - }, - pullFiles: [{ filename: "src/auth.ts", status: "modified", patch: "@@\n+fixAuth();" }], - linkedPullFiles: { - 414: [{ filename: "src/auth.ts", status: "modified", patch: "@@\n+fixAuth();" }], - }, - }), + { + sourceSummary: "PR A fixes the auth behavior in src/auth.ts.", + replacementSummary: "PR B carries the same auth behavior fix.", + coveredWork: ["PR B includes the auth behavior fix that PR A proposed."], + uniqueSourceWork: [], + securityBlocked: false, + decision: "superseded", + reason: "PR B covers PR A's useful behavior and PR A has no unique remaining work.", + }, () => { - runApplyDecisionsForTest({ - itemsDir, - closedDir, - plansDir, - reportPath, - extraArgs: [ - "--target-repo", - "openclaw/openclaw", - "--dry-run", - "--apply-kind", - "all", - "--processed-limit", - "3", - ], - }); + withMockGh( + root, + promotionGhMock({ + number: 344, + title: "Older auth PR", + comment: synced.comment, + linkedPulls: { + 414: { + number: 414, + title: "Newer auth PR", + html_url: "https://github.com/openclaw/openclaw/pull/414", + state: "open", + merged_at: null, + body: "Supersedes #344 by carrying the same auth fix.", + changed_files: 1, + head: { sha: "replacement-head-sha" }, + updated_at: "2026-05-21T00:00:00Z", + }, + }, + linkedIssues: { + 414: { + number: 414, + title: "Newer auth PR", + html_url: "https://github.com/openclaw/openclaw/pull/414", + body: "Supersedes #344 by carrying the same auth fix.", + updated_at: "2026-05-21T00:00:00Z", + labels: [], + }, + }, + pullFiles: [{ filename: "src/auth.ts", status: "modified", patch: "@@\n+fixAuth();" }], + linkedPullFiles: { + 414: [{ filename: "src/auth.ts", status: "modified", patch: "@@\n+fixAuth();" }], + }, + }), + () => { + runApplyDecisionsForTest({ + itemsDir, + closedDir, + plansDir, + reportPath, + extraArgs: [ + "--target-repo", + "openclaw/openclaw", + "--dry-run", + "--apply-kind", + "all", + "--processed-limit", + "3", + ], + }); + }, + ); }, ); const report = JSON.parse(readFileSync(reportPath, "utf8")) as Array<{ action: string }>; assert.equal( report.some((entry) => entry.action === "closed"), - false, + true, ); } finally { rmSync(root, { recursive: true, force: true }); } }); -test("apply-decisions keeps PRs open when report merge risk is security-sensitive", () => { +test("apply-decisions can close when security merge risk is covered by proof", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -9159,65 +9203,81 @@ test("apply-decisions keeps PRs open when report merge risk is security-sensitiv ); writeFileSync(join(itemsDir, "345.md"), synced.report, "utf8"); - withMockGh( + withMockSupersessionProof( root, - promotionGhMock({ - number: 345, - title: "Older provider PR", - comment: synced.comment, - linkedPulls: { - 415: { - number: 415, - title: "Newer provider PR", - html_url: "https://github.com/openclaw/openclaw/pull/415", - state: "open", - merged_at: null, - body: "Supersedes #345 by carrying the same provider fix.", - changed_files: 1, - head: { sha: "replacement-head-sha" }, - updated_at: "2026-05-21T00:00:00Z", - }, - }, - linkedIssues: { - 415: { - number: 415, - title: "Newer provider PR", - html_url: "https://github.com/openclaw/openclaw/pull/415", - body: "Supersedes #345 by carrying the same provider fix.", - updated_at: "2026-05-21T00:00:00Z", - labels: [], - }, - }, - pullFiles: [ - { filename: "src/provider.ts", status: "modified", patch: "@@\n+fixProvider();" }, - ], - linkedPullFiles: { - 415: [{ filename: "src/provider.ts", status: "modified", patch: "@@\n+fixProvider();" }], - }, - }), + { + sourceSummary: "PR A fixes provider behavior in src/provider.ts.", + replacementSummary: "PR B carries the same provider behavior fix.", + coveredWork: ["PR B includes the provider behavior fix that PR A proposed."], + uniqueSourceWork: [], + securityBlocked: false, + decision: "superseded", + reason: "PR B covers PR A's useful behavior and PR A has no unique remaining work.", + }, () => { - runApplyDecisionsForTest({ - itemsDir, - closedDir, - plansDir, - reportPath, - extraArgs: [ - "--target-repo", - "openclaw/openclaw", - "--dry-run", - "--apply-kind", - "all", - "--processed-limit", - "3", - ], - }); + withMockGh( + root, + promotionGhMock({ + number: 345, + title: "Older provider PR", + comment: synced.comment, + linkedPulls: { + 415: { + number: 415, + title: "Newer provider PR", + html_url: "https://github.com/openclaw/openclaw/pull/415", + state: "open", + merged_at: null, + body: "Supersedes #345 by carrying the same provider fix.", + changed_files: 1, + head: { sha: "replacement-head-sha" }, + updated_at: "2026-05-21T00:00:00Z", + }, + }, + linkedIssues: { + 415: { + number: 415, + title: "Newer provider PR", + html_url: "https://github.com/openclaw/openclaw/pull/415", + body: "Supersedes #345 by carrying the same provider fix.", + updated_at: "2026-05-21T00:00:00Z", + labels: [], + }, + }, + pullFiles: [ + { filename: "src/provider.ts", status: "modified", patch: "@@\n+fixProvider();" }, + ], + linkedPullFiles: { + 415: [ + { filename: "src/provider.ts", status: "modified", patch: "@@\n+fixProvider();" }, + ], + }, + }), + () => { + runApplyDecisionsForTest({ + itemsDir, + closedDir, + plansDir, + reportPath, + extraArgs: [ + "--target-repo", + "openclaw/openclaw", + "--dry-run", + "--apply-kind", + "all", + "--processed-limit", + "3", + ], + }); + }, + ); }, ); const report = JSON.parse(readFileSync(reportPath, "utf8")) as Array<{ action: string }>; assert.equal( report.some((entry) => entry.action === "closed"), - false, + true, ); } finally { rmSync(root, { recursive: true, force: true }); diff --git a/test/repair/execute-fix-artifact-source.test.ts b/test/repair/execute-fix-artifact-source.test.ts index aedd36d7be..bbbca3d160 100644 --- a/test/repair/execute-fix-artifact-source.test.ts +++ b/test/repair/execute-fix-artifact-source.test.ts @@ -92,7 +92,7 @@ test("merged source replacement skip runs before publishing replacement PRs", () ); }); -test("superseded source closeout checks source security before gh pr close", () => { +test("superseded source closeout requires replacement proof before gh pr close", () => { const sourcePath = path.join(process.cwd(), "src/repair/execute-fix-artifact.ts"); const source = fs.readFileSync(sourcePath, "utf8"); @@ -100,17 +100,15 @@ test("superseded source closeout checks source security before gh pr close", () assert.notEqual(helperStart, -1); const helper = source.slice(helperStart); - const securityIndex = helper.indexOf("sourcePullRequestSecurityBlockReason(view)"); const proofIndex = helper.indexOf("replacementCloseoutProofAllowsClose({"); const linkIndex = helper.indexOf("linkReplacementSourcePr({"); const closeIndex = helper.indexOf('"pr", "close"'); - assert.notEqual(securityIndex, -1); assert.notEqual(proofIndex, -1); assert.notEqual(linkIndex, -1); assert.notEqual(closeIndex, -1); assert.ok( - securityIndex < linkIndex && proofIndex < closeIndex && linkIndex < closeIndex, - "source PRs must pass security and replacement proof checks before gh pr close can run", + proofIndex < closeIndex && linkIndex < closeIndex, + "source PRs must pass replacement proof checks before gh pr close can run", ); }); @@ -136,9 +134,7 @@ test("superseded source closeout uses general proof instead of patch-line matchi assert.match(helper, /replacementCloseoutProofPromptPath/); assert.match(helper, /compactSupersessionProofView/); - assert.match(helper, /pullRequestReviewContextBlockReason/); assert.match(helper, /reviews: sourceView\.reviews/); - assert.match(helper, /pullRequestReviewCommentContextBlockReason/); assert.match(helper, /reviewComments: sourceView\.reviewComments/); assert.match(sharedProofSource, /export function proofBodyExcerpt/); assert.doesNotMatch(helper, /patchSignature/); diff --git a/test/repair/execute-fix-github.test.ts b/test/repair/execute-fix-github.test.ts index 66e1ec9cbb..1752eac117 100644 --- a/test/repair/execute-fix-github.test.ts +++ b/test/repair/execute-fix-github.test.ts @@ -4,12 +4,7 @@ import test from "node:test"; import { CLAWSWEEPER_CO_AUTHOR_TRAILER } from "../../dist/repair/co-author-credit.js"; import { coAuthorTrailers, - pullRequestCloseoutDriftBlockReason, pullRequestFileContextBlockReason, - pullRequestIssueCommentContextBlockReason, - pullRequestReviewContextBlockReason, - pullRequestReviewCommentContextBlockReason, - sourcePullRequestSecurityBlockReason, } from "../../dist/repair/execute-fix-github.js"; test("replacement co-author trailers include contributor and ClawSweeper credit", () => { @@ -39,61 +34,7 @@ test("replacement co-author trailers dedupe ClawSweeper credit", () => { ); }); -test("replacement source PR security gate allows ordinary source PRs", () => { - assert.equal( - sourcePullRequestSecurityBlockReason({ - title: "Fix stale activity test", - body: "Regular bug fix.", - labels: [{ name: "bug" }], - comments: [{ body: "Looks good." }], - }), - "", - ); -}); - -test("replacement source PR security gate blocks labels and comments", () => { - assert.match( - sourcePullRequestSecurityBlockReason({ - title: "Fix auth bypass", - body: "Regular body.", - labels: [{ name: "security" }], - comments: [], - }), - /security-sensitive source PR/, - ); - assert.match( - sourcePullRequestSecurityBlockReason({ - title: "Fix auth bypass", - body: "Regular body.", - labels: [], - comments: [{ body: "clawsweeper-security:security" }], - }), - /security-sensitive source PR/, - ); - assert.match( - sourcePullRequestSecurityBlockReason({ - title: "Fix auth bypass", - body: "Regular body.", - labels: [], - comments: [], - reviews: [{ body: "clawsweeper-security:security" }], - reviewComments: [], - }), - /security-sensitive source PR/, - ); - assert.match( - sourcePullRequestSecurityBlockReason({ - title: "Fix auth bypass", - body: "Regular body.", - labels: [], - comments: [], - reviewComments: [{ body: "clawsweeper-security:security" }], - }), - /security-sensitive source PR/, - ); -}); - -test("replacement source PR file context blocks truncated file lists", () => { +test("replacement source PR file context only blocks missing file lists", () => { assert.equal( pullRequestFileContextBlockReason({ changedFiles: 2, @@ -101,131 +42,12 @@ test("replacement source PR file context blocks truncated file lists", () => { }), "", ); - assert.match( + assert.equal( pullRequestFileContextBlockReason({ changedFiles: 101, files: Array.from({ length: 100 }, (_, index) => ({ path: `src/${index}.ts` })), }), - /file context is truncated/, - ); -}); - -test("replacement source PR review comment context blocks missing or truncated comments", () => { - assert.equal( - pullRequestReviewCommentContextBlockReason({ - reviewComments: [{ body: "Keep this unique test." }], - }), - "", - ); - assert.match( - pullRequestReviewCommentContextBlockReason({ - reviewComments: [], - reviewCommentsTruncated: true, - }), - /review comment context is truncated/, - ); - assert.match(pullRequestReviewCommentContextBlockReason({}), /review comment context is missing/); -}); - -test("replacement source PR review context blocks missing or truncated reviews", () => { - assert.equal( - pullRequestReviewContextBlockReason({ - reviews: [{ body: "Changes requested: keep this unique test." }], - }), "", ); - assert.match( - pullRequestReviewContextBlockReason({ - reviews: [], - reviewsTruncated: true, - }), - /review context is truncated/, - ); - assert.match(pullRequestReviewContextBlockReason({}), /review context is missing/); -}); - -test("replacement source PR issue comment context blocks missing or truncated comments", () => { - assert.equal( - pullRequestIssueCommentContextBlockReason({ - comments: [{ body: "Keep this unique proof." }], - }), - "", - ); - assert.match( - pullRequestIssueCommentContextBlockReason({ - comments: [], - commentsTruncated: true, - }), - /comment context is truncated/, - ); - assert.match(pullRequestIssueCommentContextBlockReason({}), /comment context is missing/); -}); - -test("replacement source PR closeout blocks post-proof drift", () => { - assert.match( - pullRequestCloseoutDriftBlockReason( - { state: "OPEN", updatedAt: "2026-05-01T00:00:00Z", files: [], reviewComments: [] }, - { state: "OPEN", updatedAt: "2026-05-01T00:01:00Z", files: [], reviewComments: [] }, - ), - /changed during replacement closeout proof/, - ); - assert.match( - pullRequestCloseoutDriftBlockReason( - { state: "OPEN", updatedAt: "2026-05-01T00:00:00Z", files: [] }, - { - state: "OPEN", - updatedAt: "2026-05-01T00:00:00Z", - files: [], - reviews: [], - reviewComments: [], - comments: [{ body: "" }], - }, - ), - /security-sensitive source PR/, - ); - assert.match( - pullRequestCloseoutDriftBlockReason( - { - state: "OPEN", - updatedAt: "2026-05-01T00:00:00Z", - headRefOid: "old-head", - files: [], - reviews: [], - reviewComments: [], - }, - { - state: "OPEN", - updatedAt: "2026-05-01T00:00:00Z", - headRefOid: "new-head", - files: [], - reviews: [], - reviewComments: [], - }, - "replacement PR", - ), - /replacement PR changed during replacement closeout proof/, - ); -}); - -test("replacement source PR closeout blocks truncated issue comments", () => { - assert.match( - pullRequestCloseoutDriftBlockReason( - { - state: "OPEN", - updatedAt: "2026-05-01T00:00:00Z", - files: [], - reviewComments: [], - }, - { - state: "OPEN", - updatedAt: "2026-05-01T00:00:00Z", - files: [], - comments: [{ body: "Visible comment." }], - commentsTruncated: true, - reviews: [], - reviewComments: [], - }, - ), - /comment context is truncated/, - ); + assert.match(pullRequestFileContextBlockReason({}), /file context is missing/); }); diff --git a/test/supersession-proof.test.ts b/test/supersession-proof.test.ts index 51179e0b81..ef51ed9fc1 100644 --- a/test/supersession-proof.test.ts +++ b/test/supersession-proof.test.ts @@ -23,6 +23,21 @@ test("supersession proof rejects blank covered work before closing", () => { assert.match(decision.reason, /incomplete/); }); +test("supersession proof can close security-sensitive work when coverage is concrete", () => { + const decision = supersessionProofCloseDecision({ + sourceSummary: "PR A fixes the auth route.", + replacementSummary: "PR B fixes the same auth route.", + coveredWork: ["PR B includes the auth route fix that PR A proposed."], + uniqueSourceWork: [], + securityBlocked: true, + decision: "superseded", + reason: "PR B covers PR A's auth behavior and PR A has no unique remaining work.", + }); + + assert.equal(decision.close, true); + assert.equal(decision.proof.decision, "superseded"); +}); + test("supersession proof view includes bounded file and discussion context", () => { const view = compactSupersessionProofView({ title: "Fix activity", From 96d9315c8786342de88995a392fb42a81b3a6bb5 Mon Sep 17 00:00:00 2001 From: Jesse Merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Tue, 26 May 2026 14:06:29 +1000 Subject: [PATCH 4/4] Treat security supersession as coverage proof --- prompts/repair/replacement-closeout-proof.md | 3 ++- prompts/supersession-proof.md | 3 ++- ...clawsweeper-supersession-proof.schema.json | 2 +- src/clawsweeper.ts | 20 +++++++++++++++---- test/clawsweeper.test.ts | 9 ++++++--- 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/prompts/repair/replacement-closeout-proof.md b/prompts/repair/replacement-closeout-proof.md index 5ba7d63c48..0049043956 100644 --- a/prompts/repair/replacement-closeout-proof.md +++ b/prompts/repair/replacement-closeout-proof.md @@ -10,7 +10,8 @@ Hard rules: - Do not require exact patch-line equality. A replacement can cover the same behavior with different code shape. - Return `superseded` only when PR B clearly covers PR A's useful work and PR A has no unique behavior, file concern, proof, discussion, or review point needing separate maintainer review. - Return `keep_open` for anything else, including related PRs, incomplete proof, thin context, or uncertainty. -- If PR A looks security-sensitive, include that in your comparison. Set `securityBlocked: true` only when security-sensitive context prevents proving that PR B covers PR A. +- Security-sensitive work is not a separate close blocker. Treat security labels, CVE/GHSA text, and ClawSweeper security markers as PR A content to compare. If PR B proves it covers that content, PR A can be `superseded`. +- Use `securityBlocked: true` only when PR A has security-sensitive content that PR B does not prove it covers. Also list that uncovered content in `uniqueSourceWork`. - Do not ask for more context. Return only JSON matching the supplied schema. diff --git a/prompts/supersession-proof.md b/prompts/supersession-proof.md index 9030ac3f2a..066cdd9574 100644 --- a/prompts/supersession-proof.md +++ b/prompts/supersession-proof.md @@ -10,7 +10,8 @@ Hard rules: - Do not require exact patch-line equality. A replacement can cover the same behavior with different code shape. - Return `superseded` only when PR B clearly covers PR A's useful work and PR A has no unique behavior, file concern, proof, discussion, or review point needing separate maintainer review. - Return `keep_open` for anything else, including related PRs, incomplete proof, thin context, or uncertainty. -- If PR A looks security-sensitive, including security labels, CVE/GHSA text, or ClawSweeper security markers, include that in your comparison. Set `securityBlocked: true` only when security-sensitive context prevents proving that PR B covers PR A. +- Security-sensitive work is not a separate close blocker. Treat security labels, CVE/GHSA text, and ClawSweeper security markers as PR A content to compare. If PR B proves it covers that content, PR A can be `superseded`. +- Use `securityBlocked: true` only when PR A has security-sensitive content that PR B does not prove it covers. Also list that uncovered content in `uniqueSourceWork`. - `coveredWork` must describe concrete PR A work that PR B covers. - `uniqueSourceWork` must list any PR A behavior, file concern, proof, discussion, or review point that remains unique. Use an empty array only when none remains. - Do not ask for more context. diff --git a/schema/clawsweeper-supersession-proof.schema.json b/schema/clawsweeper-supersession-proof.schema.json index 0b61a6f0dc..7c58bdffae 100644 --- a/schema/clawsweeper-supersession-proof.schema.json +++ b/schema/clawsweeper-supersession-proof.schema.json @@ -32,7 +32,7 @@ }, "securityBlocked": { "type": "boolean", - "description": "True when PR A has security-sensitive context and must not be auto-closed." + "description": "True only when PR A has security-sensitive content that PR B does not prove it covers. Security-sensitive content is not a standalone close blocker when coverage is proven." }, "decision": { "type": "string", diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 9064b1fdf9..fddd8438c4 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -11791,7 +11791,7 @@ function canClose(decision: Decision): boolean { export function validateCloseDecision( item: Pick & Partial>, decision: Decision, - options: { requireCloseComment?: boolean } = {}, + options: { requireCloseComment?: boolean; allowProtectedLabels?: boolean } = {}, ): { ok: true } | { ok: false; actionTaken: ActionTaken; reason: string } { const requireCloseComment = options.requireCloseComment !== false; const profile = repositoryProfileFor(item.repo ?? targetRepo()); @@ -11802,7 +11802,10 @@ export function validateCloseDecision( reason: "not a close decision", }; } - if (applyBlockingProtectedLabels(item.labels, decision.closeReason).length > 0) { + if ( + !options.allowProtectedLabels && + applyBlockingProtectedLabels(item.labels, decision.closeReason).length > 0 + ) { return { ok: false, actionTaken: "skipped_protected_label", @@ -13437,6 +13440,7 @@ async function applyDecisionsCommand(args: Args): Promise { let currentClosingPullRequests: unknown[] | undefined; let clawSweeperLabelsChanged = false; let issueAdvisoryLabelsChanged = false; + let closeProposalHasSupersessionProof = false; const currentItemContext = (): ItemContext => { currentContext ??= collectItemContext(item, { fullTimelineForRelations: true }); return currentContext; @@ -13451,6 +13455,7 @@ async function applyDecisionsCommand(args: Args): Promise { reportDecision(markdown, closeReason), { requireCloseComment: !isRetryableSkippedClose, + allowProtectedLabels: closeProposalHasSupersessionProof, }, ).ok ) { @@ -13704,6 +13709,7 @@ async function applyDecisionsCommand(args: Args): Promise { storedHash = itemSnapshotHash(item, promotionContext); closeReason = "duplicate_or_superseded"; isCloseProposal = true; + closeProposalHasSupersessionProof = true; } else if (promotionResult.checkedLinkedSupersession) { markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); if (!dryRun) writeFileSync(path, markdown, "utf8"); @@ -13824,7 +13830,10 @@ async function applyDecisionsCommand(args: Args): Promise { ? issueCommentWithMarker(number, hatchCommentMarker(number)) : undefined; const protectedApplyReason = applyProtectedLabelReason(item.labels, closeReason); - if (applyBlockingProtectedLabels(item.labels, closeReason).length > 0) { + if ( + !closeProposalHasSupersessionProof && + applyBlockingProtectedLabels(item.labels, closeReason).length > 0 + ) { if (isCloseProposal) { if (markApplySkipped("skipped_protected_label", protectedApplyReason)) break; } @@ -14176,7 +14185,10 @@ async function applyDecisionsCommand(args: Args): Promise { const currentReportValidation = validateCloseDecision( { repo, kind: item.kind, labels: item.labels }, reportDecision(markdown, closeReason), - { requireCloseComment: !isRetryableSkippedClose }, + { + requireCloseComment: !isRetryableSkippedClose, + allowProtectedLabels: closeProposalHasSupersessionProof, + }, ); if (!currentReportValidation.ok && currentReportValidation.actionTaken !== "kept_open") { if (markApplySkipped(currentReportValidation.actionTaken, currentReportValidation.reason)) diff --git a/test/clawsweeper.test.ts b/test/clawsweeper.test.ts index e7cc2776a6..56d9e9c588 100644 --- a/test/clawsweeper.test.ts +++ b/test/clawsweeper.test.ts @@ -8852,7 +8852,7 @@ test("apply-decisions keeps PRs open when model proof says same-file supersessio } }); -test("apply-decisions can close security-sensitive labeled PRs when proof covers them", () => { +test("apply-decisions can close security-labeled PRs when proof covers them", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -8865,7 +8865,7 @@ test("apply-decisions can close security-sensitive labeled PRs when proof covers stalePullRequestReport({ number: 336, title: "Older security PR", - labels: JSON.stringify(["security-sensitive"]), + labels: JSON.stringify(["security"]), pr_rating_overall: "D", pr_rating_proof: "D", work_cluster_refs: JSON.stringify([ @@ -8894,7 +8894,7 @@ test("apply-decisions can close security-sensitive labeled PRs when proof covers promotionGhMock({ number: 336, title: "Older security PR", - labels: ["security-sensitive"], + labels: ["security"], comment: synced.comment, linkedPulls: { 406: { @@ -11591,10 +11591,13 @@ test("supersession proof prompt requires general coverage proof", () => { assert.match(prompt, /Do not ask for more context/); assert.match(prompt, /Do not require exact patch-line equality/); assert.match(prompt, /Text such as `supersedes #A` is only a candidate signal/); + assert.match(prompt, /Security-sensitive work is not a separate close blocker/); + assert.match(prompt, /If PR B proves it covers that content, PR A can be `superseded`/); assert.match( prompt, /PR A has no unique behavior, file concern, proof, discussion, or review point/, ); + assert.doesNotMatch(prompt, /must not be auto-closed/); assert.doesNotMatch(prompt, /patchSignature/); });