From c84b12d77bc04ce14013a85664f7b571f7fc9e82 Mon Sep 17 00:00:00 2001 From: proxynico <156801777+proxynico@users.noreply.github.com> Date: Sat, 27 Jun 2026 19:28:07 +0800 Subject: [PATCH] make autofix reviews action-first --- src/clawsweeper.ts | 203 ++++++---------------- src/repair/comment-router-core.test.ts | 69 ++++++++ src/repair/comment-router-core.ts | 42 +++++ src/repair/comment-router.ts | 55 +++--- src/repair/config.ts | 7 +- src/repair/execute-fix-artifact.ts | 50 ++++-- test/pr-comment-action-policy.test.ts | 77 ++++++++ test/repair/comment-router-config.test.ts | 9 +- 8 files changed, 330 insertions(+), 182 deletions(-) create mode 100644 test/pr-comment-action-policy.test.ts diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index b517182c52..b210f2b358 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -8915,93 +8915,6 @@ function publicRealBehaviorProofLine(proof: RealBehaviorProof): string { } } -function publicRankDetailsBlock(): string { - return collapsedDetailsBlock("What the crustacean ranks mean", [ - "- 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.", - "- 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.", - "- 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.", - "- 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.", - "- 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.", - "- 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.", - "- 🌊 off-meta tidepool: rating does not apply to this item.", - "", - "Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.", - ]); -} - -function publicMergeReadinessResult(rating: PrRating, proof: RealBehaviorProof): string { - if (rating.overallTier === "NA") return "rating does not apply to this item."; - switch (proof.status) { - case "missing": - return "blocked until real behavior proof is added."; - case "mock_only": - return "blocked until real behavior proof from a real setup is added."; - case "insufficient": - return "blocked until stronger real behavior proof is added."; - case "sufficient": - case "override": - if (rating.patchTier === "F" || rating.patchTier === "D") { - return "blocked by patch quality or review findings."; - } - if (rating.overallTier === "S" || rating.overallTier === "A" || rating.overallTier === "B") { - return "ready for maintainer review."; - } - return "needs maintainer review before merge."; - case "not_applicable": - return rating.patchTier === "F" || rating.patchTier === "D" - ? "blocked by patch quality or review findings." - : "ready for maintainer review."; - } -} - -function publicMergeReadinessBlock(rating: PrRating, proof: RealBehaviorProof): string { - const shiny = hasShinyProof(proof) ? " ✨ media proof bonus" : ""; - const proofGuidance = - proof.status === "missing" || proof.status === "mock_only" || proof.status === "insufficient" - ? publicPriorityBullet("P1", publicRealBehaviorProofLine(proof)) - : ""; - const lines = [ - `Overall: ${themedRatingName(rating.overallTier)}`, - `Proof: ${themedRatingName(rating.proofTier)}${shiny}`, - `Patch quality: ${themedRatingName(rating.patchTier)}`, - `Result: ${publicMergeReadinessResult(rating, proof)}`, - "", - "Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.", - ]; - if (rating.nextSteps.length) { - const nextSteps = rating.nextSteps - .slice(0, 3) - .filter((step) => !isReportNoneList(stripPriorityPrefix(step))); - lines.push( - "", - "Rank-up moves:", - ...(nextSteps.length - ? nextSteps.map((step) => - publicPriorityBulletIfActionable(step, proofGuidance ? "P1" : "P2"), - ) - : ["- none"]), - ); - } - if (proofGuidance) { - lines.push("", "Proof guidance:", proofGuidance); - } - return lines.join("\n"); -} - -function publicFailedReviewReadinessBlock(markdown: string): string { - const reason = - reportEvidence(markdown) - .find((entry) => entry.label === "failure reason") - ?.detail.trim() || "Codex review failed before completion."; - return [ - "Not assessed.", - `Failure reason: ${sentence(reason)}`, - "", - "This is a ClawSweeper/Codex infrastructure failure, not a PR readiness or patch-quality verdict.", - "Keep any merge decision on the normal maintainer review path until ClawSweeper can complete a fresh review.", - ].join("\n"); -} - function prStatusLabelKindFromLabels(labels: readonly string[]): PrStatusLabelKind | null { for (const label of PR_STATUS_LABELS) { if (labels.includes(label.name)) return label.kind; @@ -10783,15 +10696,6 @@ function reviewMetricsFromReport(markdown: string): ReviewMetric[] { .filter((entry): entry is ReviewMetric => Boolean(entry)); } -function renderReviewMetricsDigest(metrics: readonly ReviewMetric[]): string { - if (metrics.length === 0) return "**Review metrics:** none identified."; - const noun = metrics.length === 1 ? "metric" : "metrics"; - return [ - `**Review metrics:** ${metrics.length} noteworthy ${noun}.`, - ...metrics.map((metric) => `- **${metric.label}:** ${metric.value}. ${metric.reason}`), - ].join("\n"); -} - function isDocsPath(file: string): boolean { return file.startsWith("docs/"); } @@ -14551,19 +14455,7 @@ function mergeRiskOptionsLines(options: readonly MergeRiskOption[]): string[] { function mergeRiskAutomergeInstructionBlock(instruction: string): string { const specialInstructions = normalizeMergeRiskAutomergeInstruction(instruction); if (!specialInstructions) return ""; - return [ - "
", - "Copy recommended automerge instruction", - "", - "```text", - "@clawsweeper automerge", - "", - "Special instructions:", - specialInstructions, - "```", - "", - "
", - ].join("\n"); + return ["**Repair target:**", publicPriorityBulletFromText(specialInstructions, "P1")].join("\n"); } function normalizeMergeRiskAutomergeInstruction(instruction: string): string { @@ -14638,27 +14530,45 @@ function appendReviewQuestionDetails( } } -function reviewWorkflowCallout(): string[] { - return [ - collapsedDetailsBlock("How this review workflow works", [ - "- ClawSweeper keeps one durable marker-backed review comment per issue or PR.", - "- Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.", - "- A fresh review can be triggered by eligible `@clawsweeper re-review` comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.", - "- PR/issue authors and users with repository write access can comment `@clawsweeper re-review` or `@clawsweeper re-run` on an open PR or issue to request a fresh review only.", - "- Maintainers can also comment `@clawsweeper review` to request a fresh review only.", - "- Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.", - "- Maintainer-only repair and merge flows require explicit commands such as `@clawsweeper autofix`, `@clawsweeper automerge`, `@clawsweeper fix ci`, or `@clawsweeper address review`.", - "- Maintainers can comment `@clawsweeper explain` to ask for more context, or `@clawsweeper stop` to stop active automation.", - ]), - "", - ]; -} - function reviewFreshnessText(markdown: string): string { const timestamp = formatReviewFreshnessTimestamp(frontMatterValue(markdown, "reviewed_at")); return timestamp ? ` _Reviewed ${timestamp}._` : ""; } +function publicReviewActionLine(options: { + markdown: string; + isPullRequest: boolean; + reviewFailed: boolean; + hasRealBehaviorProofBlocker: boolean; + securityReview: SecurityReview; + isRepairCandidate: boolean; + hasReviewFindings: boolean; + isRepairLoopPass: boolean; +}): string { + if (!options.isPullRequest) { + return "Action: keep this issue open until the missing behavior is implemented or the maintainer closes it."; + } + if (options.reviewFailed) { + return "Action: rerun review; no repair was planned from this failed pass."; + } + if (options.hasRealBehaviorProofBlocker) { + return "Action: human/live proof required; Dita will not edit this head until that blocker is cleared."; + } + if ( + options.securityReview.status === "needs_attention" && + !securitySensitiveRepairAllowed(options.markdown) + ) { + return "Action: owner security review required; Dita will not edit this head from the review comment alone."; + } + if (options.isRepairLoopPass) { + return "Action: no repair needed; this head passed the ClawSweeper repair loop."; + } + if (options.isRepairCandidate || options.hasReviewFindings) { + return "Action: Dita should queue a fix-only repair for this exact head. Final merge remains separately gated."; + } + return "Action: maintainer review; Dita did not find a narrow code repair to make."; +} + function renderKeepOpenCommentFromReport( markdown: string, options: ReviewCommentRenderOptions = {}, @@ -14667,8 +14577,6 @@ function renderKeepOpenCommentFromReport( const likelyOwners = reportLikelyOwners(markdown).slice(0, 5).map(likelyOwnerLine); const reviewFindings = reportReviewFindings(markdown); const securityReview = reportSecurityReview(markdown); - const realBehaviorProof = reportRealBehaviorProof(markdown); - const prRating = reportPrRating(markdown); const mantisRecommendation = reportMantisRecommendation(markdown); const agentsPolicyStatus = reportAgentsPolicyStatus(markdown); const rootCauseCluster = reportRootCauseCluster(markdown); @@ -14679,7 +14587,6 @@ function renderKeepOpenCommentFromReport( const solutionAssessment = reviewSectionValue(markdown, "solutionAssessment"); const risks = reviewSectionValue(markdown, "risks"); const mergeRiskOptions = mergeRiskOptionsFromReport(markdown); - const reviewMetrics = reviewMetricsFromReport(markdown); const workReason = reportWorkCandidateReason(markdown); const workCandidate = frontMatterValue(markdown, "work_candidate"); const isPullRequest = frontMatterValue(markdown, "type") === "pull_request"; @@ -14727,14 +14634,27 @@ function renderKeepOpenCommentFromReport( const lines = [`${verdictLine}${reviewFreshnessText(markdown)}`, ""]; const prSurface = renderOpenClawPrSurfaceFromReport(markdown); const prSurfaceSummary = prSurface.split("\n\n", 1)[0]?.trim() ?? ""; - if (prSurface) evidenceDetails.push("PR surface:", "", prSurface); + if (prSurfaceSummary) evidenceDetails.push("PR surface:", "", prSurfaceSummary); if (isPullRequest) { appendPublicSection( lines, "Summary", publicPrSummaryBody(changeSummaryLine, reproductionAssessment, prSurfaceSummary), ); - lines.push(renderReviewMetricsDigest(reviewMetrics), ""); + appendPublicSection( + lines, + "Action", + publicReviewActionLine({ + markdown, + isPullRequest, + reviewFailed, + hasRealBehaviorProofBlocker, + securityReview, + isRepairCandidate, + hasReviewFindings, + isRepairLoopPass, + }), + ); const dataModelWarning = renderDataModelWarningFromReport(markdown); if (dataModelWarning) appendPublicSection(lines, "Stored data model", dataModelWarning); } else { @@ -14754,15 +14674,6 @@ function renderKeepOpenCommentFromReport( ); } } - if (isPullRequest) { - appendPublicSection( - lines, - "Merge readiness", - reviewFailed - ? publicFailedReviewReadinessBlock(markdown) - : publicMergeReadinessBlock(prRating, realBehaviorProof), - ); - } const mantisSuggestion = isPullRequest ? publicMantisRecommendationBlock(mantisRecommendation) : ""; @@ -14865,14 +14776,12 @@ function renderKeepOpenCommentFromReport( } const reviewLine = closeReviewLineFromReport(markdown); if (reviewLine) reviewDetails.push(...(reviewDetails.length ? [""] : []), reviewLine); - const detailsBlock = collapsedDetailsBlock("Review details", reviewDetails); - if (detailsBlock) lines.push("", detailsBlock); - const labelDetailsBlock = collapsedDetailsBlock("Label changes", labelDetails); - if (labelDetailsBlock) lines.push("", labelDetailsBlock); - const evidenceDetailsBlock = collapsedDetailsBlock("Evidence reviewed", evidenceDetails); - if (evidenceDetailsBlock) lines.push("", evidenceDetailsBlock); - if (isPullRequest && !reviewFailed) lines.push("", publicRankDetailsBlock()); - lines.push("", ...reviewWorkflowCallout()); + if (reviewDetails.length || labelDetails.length || evidenceDetails.length) { + lines.push( + "", + "_Detailed review evidence is retained in the ClawSweeper artifact; this public comment is kept action-first._", + ); + } return sanitizePublicSelfReferences( lines.join("\n"), Number(frontMatterValue(markdown, "number")), diff --git a/src/repair/comment-router-core.test.ts b/src/repair/comment-router-core.test.ts index 73a85a50eb..bb113295d6 100644 --- a/src/repair/comment-router-core.test.ts +++ b/src/repair/comment-router-core.test.ts @@ -4,6 +4,8 @@ import test from "node:test"; import { automergeMergeFailureRepairReason, automergeRebaseRepairReason, + branchRepairCanContinueAutomerge, + trustedAutoRepairShouldOptInAutofix, } from "./comment-router-core.js"; test("automerge rebase repair reason detects dirty merge state", () => { @@ -44,3 +46,70 @@ test("automerge merge failure repair reason ignores unrelated merge failures", ( null, ); }); + +test("trusted repair markers can auto-opt normal PRs into autofix", () => { + assert.equal( + trustedAutoRepairShouldOptInAutofix( + { intent: "clawsweeper_auto_repair", trusted_bot: true }, + { + kind: "pull_request", + labels: [], + }, + ), + true, + ); + + assert.equal( + trustedAutoRepairShouldOptInAutofix( + { intent: "clawsweeper_auto_repair", trusted_bot: false }, + { + kind: "pull_request", + labels: [], + }, + ), + false, + ); + + assert.equal( + trustedAutoRepairShouldOptInAutofix( + { intent: "clawsweeper_auto_repair", trusted_bot: true }, + { + kind: "pull_request", + labels: ["clawsweeper:autofix"], + }, + ), + false, + ); +}); + +test("fix-only branch repairs do not continue automerge", () => { + assert.equal( + branchRepairCanContinueAutomerge({ + source: "pr_automerge", + clusterId: "automerge-proxynico-example-17", + allowMerge: false, + blockedActions: ["merge"], + }), + false, + ); + + assert.equal( + branchRepairCanContinueAutomerge({ + source: "pr_automerge", + clusterId: "automerge-proxynico-example-17", + allowMerge: true, + blockedActions: [], + }), + true, + ); + + assert.equal( + branchRepairCanContinueAutomerge({ + source: "issue_implementation", + clusterId: "issue-proxynico-example-17", + allowMerge: true, + blockedActions: [], + }), + false, + ); +}); diff --git a/src/repair/comment-router-core.ts b/src/repair/comment-router-core.ts index 67b5818b8d..3f2f58f757 100644 --- a/src/repair/comment-router-core.ts +++ b/src/repair/comment-router-core.ts @@ -153,6 +153,48 @@ export function selectPullRepairJob({ }; } +export function canRepairPullTarget(target: LooseRecord) { + if (target?.kind !== "pull_request") return false; + return Boolean(target.job_path || target.is_clawsweeper_pr || targetHasRepairLoopLabel(target)); +} + +export function targetHasRepairLoopLabel(target: LooseRecord) { + return ( + hasLabelName(target?.labels ?? [], AUTOFIX_LABEL) || + hasLabelName(target?.labels ?? [], AUTOMERGE_LABEL) + ); +} + +function hasLabelName(labels: JsonValue[], name: string) { + return labels.some( + (labelName: JsonValue) => String(labelName).toLowerCase() === name.toLowerCase(), + ); +} + +export function trustedAutoRepairShouldOptInAutofix(command: LooseRecord, target: LooseRecord) { + return ( + command?.intent === "clawsweeper_auto_repair" && + Boolean(command?.trusted_bot) && + !canRepairPullTarget(target) + ); +} + +export function branchRepairCanContinueAutomerge({ + source = null, + clusterId = null, + allowMerge = false, + blockedActions = [], +}: LooseRecord) { + const blocked = Array.isArray(blockedActions) + ? blockedActions.map((action: JsonValue) => String(action)) + : []; + return ( + (source === AUTOMERGE_JOB_SOURCE || String(clusterId ?? "").startsWith("automerge-")) && + allowMerge === true && + !blocked.includes("merge") + ); +} + export function issueImplementationClusterId(repo: string, issueNumber: JsonValue) { return `issue-${repoSlug(repo)}-${Number(issueNumber)}`; } diff --git a/src/repair/comment-router.ts b/src/repair/comment-router.ts index fc274acea0..167c34a122 100644 --- a/src/repair/comment-router.ts +++ b/src/repair/comment-router.ts @@ -38,6 +38,7 @@ import { buildClawSweeperAssistDispatchPayload, buildAutomergeMergeArgs, buildAutomergeSquashMessage, + canRepairPullTarget, commandHasAction, commandStatusMarkerFromBody, createCachedIssueCommentsLookup, @@ -76,6 +77,8 @@ import { sharedAutomergeStatusMarkerPrefix, staleClosedItemCommandReason, shouldClearMaintainerCommandReaction, + targetHasRepairLoopLabel, + trustedAutoRepairShouldOptInAutofix, trustedCloseBlockReason, usesSharedAutomergeStatus, } from "./comment-router-core.js"; @@ -770,20 +773,12 @@ function classifyCommand(command: LooseRecord): JsonValue { if (!pull) { return repairBlocked(next, "repair commands require a pull request"); } + const trustedAutoRepair = command.intent === "clawsweeper_auto_repair" && command.trusted_bot; if (command.trusted_bot) { const stoppedReason = repairLoopStoppedReason(next); if (stoppedReason) return { ...next, status: "skipped", reason: stoppedReason }; } - if (!canRepairPullTarget(target)) { - return repairBlocked( - next, - "repair commands require a ClawSweeper PR or a PR opted into ClawSweeper autofix or automerge", - ); - } - if (command.trusted_bot && hasLabel(target, HUMAN_REVIEW_LABEL)) { - return { ...next, status: "skipped", reason: "PR is paused for human review" }; - } - if (command.intent === "clawsweeper_auto_repair") { + if (trustedAutoRepair) { if ( command.expected_head_sha && command.expected_head_sha !== "unknown" && @@ -799,8 +794,25 @@ function classifyCommand(command: LooseRecord): JsonValue { const alreadyPlanned = autoRepairAlreadyPlanned(next); if (alreadyPlanned) return { ...next, status: "skipped", reason: alreadyPlanned }; } + if (command.trusted_bot && pauseLabelsOn(target).length > 0) { + return { ...next, status: "skipped", reason: "PR is paused for human review" }; + } + const autoOptInRepair = trustedAutoRepairShouldOptInAutofix(next, target); + if (!canRepairPullTarget(target) && !autoOptInRepair) { + return repairBlocked( + next, + "repair commands require a ClawSweeper PR or a PR opted into ClawSweeper autofix or automerge", + ); + } const actions: LooseRecord[] = []; const repairJobPath = target.automerge_job_path; + if (autoOptInRepair) { + actions.push({ + action: "label", + label: AUTOFIX_LABEL, + status: execute ? "pending" : "planned", + }); + } if (!target.has_automerge_job) { actions.push({ action: "ensure_automerge_job", @@ -946,7 +958,7 @@ function classifyAutomergePass( return { ...command, status: "skipped", reason: "ClawSweeper pass marker is not on a PR" }; const stoppedReason = repairLoopStoppedReason(command); if (stoppedReason) return { ...command, status: "skipped", reason: stoppedReason }; - if (!hasRepairLoopLabel(command.target)) + if (!targetHasRepairLoopLabel(command.target)) return { ...command, status: "skipped", @@ -1137,7 +1149,7 @@ function classifyNeedsHuman( if (String(issue.state ?? "").toLowerCase() !== "open") return { ...command, status: "skipped", reason: "target is not open" }; if (!pull) return { ...command, status: "skipped", reason: "human-review marker is not on a PR" }; - if (!hasRepairLoopLabel(command.target)) + if (!targetHasRepairLoopLabel(command.target)) return { ...command, status: "skipped", @@ -1302,15 +1314,6 @@ function repairBlocked(command: LooseRecord, reason: string) { }; } -function canRepairPullTarget(target: LooseRecord) { - if (target?.kind !== "pull_request") return false; - return Boolean(target.job_path || target.is_clawsweeper_pr || hasRepairLoopLabel(target)); -} - -function hasRepairLoopLabel(target: LooseRecord) { - return hasLabel(target, AUTOFIX_LABEL) || hasLabel(target, AUTOMERGE_LABEL); -} - function autoRepairAlreadyPlanned(command: LooseRecord) { const resumeBoundary = latestAutomergeResumeAt(command); const block = autoRepairBlockReason({ @@ -1419,12 +1422,20 @@ function executeCommand(command: LooseRecord) { const shouldDispatchAssist = commandHasAction(command, "dispatch_assist"); const shouldMerge = commandHasAction(command, "merge"); const shouldApplyHumanReviewLabel = commandHasAction(command, "label"); + const trustedAutoRepairOptIn = + command.intent === "clawsweeper_auto_repair" && + command.trusted_bot && + (command.actions ?? []).some( + (action: JsonValue) => action.action === "label" && action.label === AUTOFIX_LABEL, + ); if (!command.trusted_bot) reactToComment(command, "eyes"); if ( shouldDispatchRepair && (canRepairPullTarget(command.target) || - ["autofix", "automerge", "implement_issue"].includes(command.intent)) + ["autofix", "automerge", "implement_issue"].includes(command.intent) || + trustedAutoRepairOptIn) ) { + if (trustedAutoRepairOptIn) applyLabelActions(command); if (["autofix", "automerge"].includes(command.intent)) { applyRepairLoopOptIn(command); } diff --git a/src/repair/config.ts b/src/repair/config.ts index b146435965..5e276771d4 100644 --- a/src/repair/config.ts +++ b/src/repair/config.ts @@ -12,7 +12,12 @@ import { export { DEFAULT_HEAD_PREFIX, DEFAULT_TARGET_REPO } from "./constants.js"; const DEFAULT_ALLOWED_ASSOCIATIONS = ["OWNER", "MEMBER", "COLLABORATOR"]; -export const DEFAULT_TRUSTED_BOTS = ["clawsweeper[bot]", "openclaw-clawsweeper[bot]"]; +export const DEFAULT_TRUSTED_BOTS = [ + "clawsweeper[bot]", + "openclaw-clawsweeper[bot]", + "dita-clawsweeper[bot]", + "nico-clawsweeper[bot]", +]; export type CommentRouterConfig = { targetRepo: string; diff --git a/src/repair/execute-fix-artifact.ts b/src/repair/execute-fix-artifact.ts index c0f944fffc..7e9ac68d5e 100644 --- a/src/repair/execute-fix-artifact.ts +++ b/src/repair/execute-fix-artifact.ts @@ -47,7 +47,10 @@ import { canUseAutomergeFastRebase, } from "./automerge-shepherd.js"; import { automergeOutcomeReviewedShaFromResult } from "./automerge-outcome.js"; -import { isCanonicalLandingNeedsHumanText } from "./comment-router-core.js"; +import { + branchRepairCanContinueAutomerge, + isCanonicalLandingNeedsHumanText, +} from "./comment-router-core.js"; import { parsePullRequestUrl, pullRequestNumberFromUrl } from "./github-ref.js"; import { clawsweeperGitUserEmail, @@ -3754,10 +3757,23 @@ function updateAutomergeStatusCommentForBranchRepair({ if (Number(target) !== Number(automergeOutcomeTargetPrNumber())) return false; const existingStatus = findAutomergeStatusComment(target); const runUrl = currentActionsRunUrl(); - const reviewDispatch = dispatchAutomergeReviewAfterBranchRepair({ target, commit }); + const automergeEnabled = canContinueAutomergeAfterBranchRepair(); + const reviewDispatch = automergeEnabled + ? dispatchAutomergeReviewAfterBranchRepair({ target, commit }) + : { status: "skipped", reason: "merge disabled for autofix" }; + const actionLine = automergeEnabled + ? "Action: branch repair was pushed; continue merge only after checks and exact-head review are green." + : "Action: branch repair was pushed; leave final merge to a maintainer or explicit automerge opt-in."; + const currentStateLine = automergeEnabled + ? reviewDispatch.status === "executed" + ? "Current state: exact-head review queued immediately; GitHub checks and the review verdict gate final merge." + : "Current state: waiting for GitHub checks and the next router pass to continue automerge." + : "Current state: autofix applied for this head. Final merge remains disabled for this job."; + const reviewDispatchRecord = reviewDispatch as LooseRecord; + const reviewDispatchedAt = String(reviewDispatchRecord.dispatched_at ?? ""); const body = [ - "🦞🔧", - "ClawSweeper applied a repair to this PR branch.", + "ClawSweeper repair applied.", + actionLine, "", fastRepair?.status === "ready" ? "Repair: rebased this branch deterministically; Codex fix/edit was not needed." @@ -3766,10 +3782,10 @@ function updateAutomergeStatusCommentForBranchRepair({ `Updated head: ${markdownCommitLink(result.repo, commit)}`, ...(runUrl ? [`Run: ${runUrl}`] : []), "", - reviewDispatch.status === "executed" - ? "Current state: exact-head review queued immediately; GitHub checks and the review verdict gate final merge." - : "Current state: waiting for GitHub checks and the next router pass to continue automerge.", - ...(reviewDispatch.status === "failed" ? [`Review dispatch: ${reviewDispatch.reason}`] : []), + currentStateLine, + ...(automergeEnabled && reviewDispatch.status === "failed" + ? [`Review dispatch: ${reviewDispatch.reason}`] + : []), ].join("\n"); const bodyWithTimeline = mergeAutomergeTimelineSection({ body, @@ -3787,12 +3803,12 @@ function updateAutomergeStatusCommentForBranchRepair({ status: fastRepair?.status === "ready" ? "deterministic rebase" : "branch updated", details: fastRepair?.reason ?? null, }, - ...(reviewDispatch.status === "executed" + ...(reviewDispatch.status === "executed" && reviewDispatchedAt ? [ { - id: `review-queued:${commit}:${reviewDispatch.dispatched_at}`, + id: `review-queued:${commit}:${reviewDispatchedAt}`, label: "review queued", - at: reviewDispatch.dispatched_at, + at: reviewDispatchedAt, headSha: commit, repo: result.repo, status: "after repair", @@ -3814,6 +3830,9 @@ function updateAutomergeStatusCommentForBranchRepair({ function waitForAutomergeAfterBranchRepair({ target, commit }: LooseRecord) { if (!isAutomergeRepairJob()) return { status: "skipped", reason: "not automerge repair" }; + if (!canContinueAutomergeAfterBranchRepair()) { + return { status: "skipped", reason: "merge disabled for autofix" }; + } if (process.env.CLAWSWEEPER_AUTOMERGE_SHEPHERD_WAIT === "0") { return { status: "skipped", reason: "disabled by CLAWSWEEPER_AUTOMERGE_SHEPHERD_WAIT=0" }; } @@ -4000,6 +4019,15 @@ function isAutomergeRepairJob() { ); } +function canContinueAutomergeAfterBranchRepair() { + return branchRepairCanContinueAutomerge({ + source: job.frontmatter.source, + clusterId: result.cluster_id, + allowMerge: job.frontmatter.allow_merge, + blockedActions: job.frontmatter.blocked_actions, + }); +} + function isSelfHealRepairJob() { return ( job.frontmatter.source === "clawsweeper_self_rebase" || diff --git a/test/pr-comment-action-policy.test.ts b/test/pr-comment-action-policy.test.ts new file mode 100644 index 0000000000..5fab3daf4e --- /dev/null +++ b/test/pr-comment-action-policy.test.ts @@ -0,0 +1,77 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { renderReviewCommentFromReport } from "../dist/clawsweeper.js"; +import { reportFrontMatter } from "./helpers.ts"; + +test("pull request review comments stay compact and action-first", () => { + const comment = renderReviewCommentFromReport( + `${reportFrontMatter({ + type: "pull_request", + number: "83400", + decision: "keep_open", + close_reason: "none", + work_candidate: "queue_fix_pr", + pull_head_sha: "abc123def456", + merge_risk_options: JSON.stringify([ + { + title: "Repair delivery before merge", + body: "Fix duplicate active-requester delivery and add regression coverage before merge.", + category: "fix_before_merge", + recommended: true, + automergeInstruction: + "@clawsweeper autofix this PR: prevent duplicate active-requester delivery and add focused regression coverage before merging.", + }, + ]), + })} + +## Summary + +Keep this PR open because the delivery repair is still needed. + +## What This Changes + +Updates generated review-comment formatting. + +## Best Possible Solution + +Repair duplicate delivery and add regression coverage before merge. + +## Risks / Open Questions + +Delivery repair should not run with nested bot commands in the pasteable instruction. + +## Work Candidate + +Candidate: queue_fix_pr + +Confidence: high + +Priority: medium + +Status: ready + +Reason: prevent duplicate active-requester delivery and add focused regression coverage before merging. + +## Review Findings + +Overall correctness: patch is incorrect + +Overall confidence: 0.91 + +Full review comments: + +- [P2] Prevent duplicate delivery: \`src/runtime.ts:12\` +`, + "none", + ); + + assert.match(comment, /\*\*Action\*\*\nAction: Dita should queue a fix-only repair/); + assert.match(comment, /\*\*Repair target:\*\*/); + assert.doesNotMatch(comment, /@clawsweeper automerge/); + assert.doesNotMatch(comment, /@clawsweeper autofix/); + assert.doesNotMatch(comment, /Copy recommended automerge instruction/); + assert.doesNotMatch(comment, /Review details<\/summary>/); + assert.doesNotMatch(comment, /Evidence reviewed<\/summary>/); + assert.ok(comment.length < 4000, `comment was ${comment.length} chars`); +}); diff --git a/test/repair/comment-router-config.test.ts b/test/repair/comment-router-config.test.ts index 5a4e6208c2..4498409d26 100644 --- a/test/repair/comment-router-config.test.ts +++ b/test/repair/comment-router-config.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { readCommentRouterConfig } from "../../dist/repair/config.js"; +import { DEFAULT_TRUSTED_BOTS, readCommentRouterConfig } from "../../dist/repair/config.js"; test("comment router config preserves target branch from dispatch args", () => { const config = readCommentRouterConfig({ @@ -35,3 +35,10 @@ test("comment router config omits target branch by default", () => { } } }); + +test("comment router defaults trust deployed ClawSweeper app identities", () => { + assert.ok(DEFAULT_TRUSTED_BOTS.includes("clawsweeper[bot]")); + assert.ok(DEFAULT_TRUSTED_BOTS.includes("openclaw-clawsweeper[bot]")); + assert.ok(DEFAULT_TRUSTED_BOTS.includes("dita-clawsweeper[bot]")); + assert.ok(DEFAULT_TRUSTED_BOTS.includes("nico-clawsweeper[bot]")); +});