diff --git a/.github/workflows/cluster-worker.yml b/.github/workflows/cluster-worker.yml index 3e5209d..6f464cc 100644 --- a/.github/workflows/cluster-worker.yml +++ b/.github/workflows/cluster-worker.yml @@ -207,6 +207,7 @@ jobs: CLOWNFISH_TARGET_VALIDATION_MODE: ${{ vars.CLOWNFISH_TARGET_VALIDATION_MODE || 'changed-only' }} CLOWNFISH_POST_FLIGHT_IGNORE_CHECKS: ${{ vars.CLOWNFISH_POST_FLIGHT_IGNORE_CHECKS || 'auto-response,Labeler,Stale' }} CLOWNFISH_MAX_ACTIVE_PRS_PER_AREA: ${{ vars.CLOWNFISH_MAX_ACTIVE_PRS_PER_AREA || '50' }} + CLOWNFISH_CLOSE_SUPERSEDED_SOURCE_PRS: ${{ vars.CLOWNFISH_CLOSE_SUPERSEDED_SOURCE_PRS || '0' }} CLOWNFISH_GIT_USER_NAME: ${{ vars.CLOWNFISH_GIT_USER_NAME || 'projectclownfish' }} CLOWNFISH_GIT_USER_EMAIL: ${{ vars.CLOWNFISH_GIT_USER_EMAIL || 'projectclownfish@users.noreply.github.com' }} CODEX_CLI_VERSION: ${{ vars.CLOWNFISH_CODEX_CLI_VERSION || '0.125.0' }} diff --git a/package.json b/package.json index c464677..7a74508 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,8 @@ "requeue": "node scripts/requeue-job.mjs", "self-heal": "node scripts/self-heal-failed-runs.mjs", "publish-result": "node scripts/publish-result.mjs", - "review-results": "node scripts/review-results.mjs" + "review-results": "node scripts/review-results.mjs", + "message-samples": "node scripts/render-message-samples.mjs" }, "engines": { "node": ">=24" diff --git a/scripts/apply-result.mjs b/scripts/apply-result.mjs index 12b5d51..f479951 100644 --- a/scripts/apply-result.mjs +++ b/scripts/apply-result.mjs @@ -4,6 +4,7 @@ import path from "node:path"; import { execFileSync } from "node:child_process"; import { createHash } from "node:crypto"; import { assertAllowedOwner, hasSecuritySignalText, parseArgs, parseJob, repoRoot, validateJob } from "./lib.mjs"; +import { defaultCloseComment, externalMessageProvenance } from "./external-messages.mjs"; const MAINTAINER_AUTHOR_ASSOCIATIONS = new Set(["OWNER", "MEMBER", "COLLABORATOR"]); const CLOSE_ACTIONS = new Set([ @@ -18,8 +19,9 @@ const MERGE_ACTIONS = new Set(["merge_candidate", "merge_canonical"]); const CLOSE_CLASSIFICATIONS = new Set(["duplicate", "superseded", "fixed_by_candidate", "low_signal"]); const PASSING_CHECK_CONCLUSIONS = new Set(["SUCCESS", "SKIPPED", "NEUTRAL"]); const CLEAN_MERGE_STATES = new Set(["CLEAN"]); -const HUMAN_REVIEW_LABEL = "clownfish:human-review"; -const MERGE_READY_LABEL = "clownfish:merge-ready"; +const CLOWNFISH_LABEL = "clownfish"; +const CLOWNFISH_LABEL_COLOR = "F97316"; +const CLOWNFISH_LABEL_DESCRIPTION = "Tracked by Clownfish automation"; const args = parseArgs(process.argv.slice(2)); const jobPath = args._[0]; @@ -406,11 +408,11 @@ function applyMergeAction({ job, result, action, dryRun, allowMissingUpdatedAt, } if (process.env.CLOWNFISH_ALLOW_MERGE !== "1") { - if (!dryRun) labelForHumanMergeReview(result.repo, target); + if (!dryRun) labelForClownfishReview(result.repo, target); return { ...base, status: "blocked", - reason: "merge requires CLOWNFISH_ALLOW_MERGE=1; labeled for human review", + reason: "merge requires CLOWNFISH_ALLOW_MERGE=1; labeled clownfish", live_state: live.state, live_updated_at: live.updated_at, merge_method: "squash", @@ -499,11 +501,9 @@ function validateMergePolicy({ job, action }) { return ""; } -function labelForHumanMergeReview(repo, target) { - ensureLabel(repo, HUMAN_REVIEW_LABEL, "B60205", "Needs maintainer review before Clownfish can finish"); - ensureLabel(repo, MERGE_READY_LABEL, "0E8A16", "Clownfish found a merge-ready candidate; human owns the final merge"); - ghBestEffort(["issue", "edit", String(target), "--repo", repo, "--add-label", HUMAN_REVIEW_LABEL]); - ghBestEffort(["issue", "edit", String(target), "--repo", repo, "--add-label", MERGE_READY_LABEL]); +function labelForClownfishReview(repo, target) { + ensureLabel(repo, CLOWNFISH_LABEL, CLOWNFISH_LABEL_COLOR, CLOWNFISH_LABEL_DESCRIPTION); + ghBestEffort(["issue", "edit", String(target), "--repo", repo, "--add-label", CLOWNFISH_LABEL]); } function ensureLabel(repo, name, color, description) { @@ -701,45 +701,20 @@ function renderCloseComment({ action, classification, result, target, live }) { const canonical = normalizeIssueRef(action.canonical ?? action.duplicate_of); const candidateFix = normalizeIssueRef(action.candidate_fix ?? action.fixed_by ?? action.fix_candidate); const title = typeof live.title === "string" ? live.title : `#${target}`; - const evidence = Array.isArray(action.evidence) ? action.evidence : []; - const evidenceLines = evidence - .slice(0, 5) - .map((item) => `- ${typeof item === "string" ? item : (item.detail ?? JSON.stringify(item))}`); const reason = action.reason ? String(action.reason).trim() : closeReasonText(classification); - const lines = [`Thanks for this. Clownfish reviewed this cluster and is closing #${target}.`]; - lines.push(""); - if (classification === "duplicate" && canonical) { - lines.push( - `This appears to duplicate #${canonical}. I'm keeping #${canonical} as the canonical thread so fixes, validation, and follow-up stay in one place.`, - ); - } else if (classification === "superseded" && canonical) { - lines.push( - `This is superseded by #${canonical}. I'm keeping that thread as the canonical path so the useful context and contributor credit stay visible.`, - ); - } else if (classification === "superseded" && candidateFix) { - lines.push( - `This is superseded by landed fix #${candidateFix}. I'm closing this older overlap so validation and follow-up stay attached to the shipped path.`, - ); - } else if (classification === "fixed_by_candidate" && candidateFix) { - lines.push( - `This is covered by candidate fix #${candidateFix}. I'm closing this thread so validation and follow-up stay attached to that fix path.`, - ); - } else if (classification === "low_signal") { - lines.push( - "This falls under the low-signal PR cleanup policy: the PR does not currently present a reviewable OpenClaw fix with maintainer signal, current validation, or a focused product path. Please reopen from a clean branch with a scoped summary, linked issue or rationale, and validation if this is still needed.", - ); - } else { - lines.push(reason); - } - lines.push(""); - lines.push(`Cluster: \`${result.cluster_id}\``); - lines.push(`Reviewed item: #${target} - ${title}`); - if (evidenceLines.length) lines.push("", "Evidence:", ...evidenceLines); - lines.push( - "", - "If this has a different reproduction path or still reproduces after the canonical fix lands, reply and we can reopen or split it back out.", - ); - return lines.join("\n"); + return defaultCloseComment({ + action, + classification, + clusterId: result.cluster_id, + target, + title, + canonical, + candidateFix, + reason, + provenance: externalMessageProvenance({ + reviewedSha: action.reviewed_sha ?? action.head_sha ?? result.reviewed_sha ?? result.head_sha, + }), + }); } function closeReasonText(classification) { diff --git a/scripts/comment-router-core.mjs b/scripts/comment-router-core.mjs index 18b0029..4841909 100644 --- a/scripts/comment-router-core.mjs +++ b/scripts/comment-router-core.mjs @@ -190,7 +190,7 @@ export function renderResponse(command, dispatched) { marker, "Got it. Clownfish will leave this item for human review.", "", - "I added `clownfish:human-review` when permissions allowed it. Future automation should treat this as a maintainer handoff signal, so this stays out of the repair lane until someone asks again.", + "I kept the regular `clownfish` label on it and paused the automation trail until a maintainer asks again.", ].join("\n"); } if (command.intent === "automerge") { @@ -248,7 +248,7 @@ export function renderResponse(command, dispatched) { `Source: \`${command.trusted_bot_author ?? command.author ?? "trusted automation"}\``, `Reason: ${command.repair_reason ?? "ClawSweeper requested human review."}`, "", - "I added `clownfish:human-review` when permissions allowed it.", + "I kept the regular `clownfish` label on it and left the final call with a maintainer.", ].join("\n"); } if (!dispatched) { diff --git a/scripts/comment-router.mjs b/scripts/comment-router.mjs index c7aff23..5a771bd 100644 --- a/scripts/comment-router.mjs +++ b/scripts/comment-router.mjs @@ -44,8 +44,8 @@ const DEFAULT_TARGET_REPO = "openclaw/openclaw"; const DEFAULT_HEAD_PREFIX = "clownfish/"; const DEFAULT_LABEL = "clownfish"; const AUTOMERGE_LABEL = "clownfish:automerge"; -const HUMAN_REVIEW_LABEL = "clownfish:human-review"; -const MERGE_READY_LABEL = "clownfish:merge-ready"; +const DEFAULT_LABEL_COLOR = "F97316"; +const DEFAULT_LABEL_DESCRIPTION = "Tracked by Clownfish automation"; const DEFAULT_ALLOWED_ASSOCIATIONS = ["OWNER", "MEMBER", "COLLABORATOR"]; const DEFAULT_TRUSTED_BOTS = ["clawsweeper[bot]", "openclaw-clawsweeper[bot]"]; const DEFAULT_CLOWNFISH_AUTHORS = ["openclaw-clownfish", "openclaw-clownfish[bot]"]; @@ -251,7 +251,7 @@ function classifyCommand(command) { ...next, status: "ready", actions: [ - { action: "label", label: HUMAN_REVIEW_LABEL, status: execute ? "pending" : "planned" }, + { action: "label", label, status: execute ? "pending" : "planned" }, { action: "comment", status: execute ? "pending" : "planned" }, ], }; @@ -314,7 +314,6 @@ function classifyAutomergePass(command, issue, pull) { if (String(issue.state ?? "").toLowerCase() !== "open") return { ...command, status: "skipped", reason: "PR is not open" }; if (!pull) return { ...command, status: "skipped", reason: "ClawSweeper pass marker is not on a PR" }; if (!hasLabel(command.target, AUTOMERGE_LABEL)) return { ...command, status: "skipped", reason: "PR is not opted into Clownfish automerge" }; - if (hasLabel(command.target, HUMAN_REVIEW_LABEL)) return { ...command, status: "skipped", reason: "PR is paused for human review" }; if (!command.expected_head_sha || command.expected_head_sha === "unknown") { return { ...command, status: "skipped", reason: "ClawSweeper pass marker must include the reviewed PR head SHA" }; } @@ -344,7 +343,7 @@ function classifyNeedsHuman(command, issue, pull) { ...command, status: "ready", actions: [ - { action: "label", label: HUMAN_REVIEW_LABEL, status: execute ? "pending" : "planned" }, + { action: "label", label, status: execute ? "pending" : "planned" }, { action: "comment", status: execute ? "pending" : "planned" }, ], }; @@ -486,17 +485,17 @@ function executeCommand(command) { } } if (command.intent === "clawsweeper_needs_human" && command.issue_number) { - ensureHumanReviewLabel(command.repo); - ghBestEffort(["issue", "edit", String(command.issue_number), "--repo", command.repo, "--add-label", HUMAN_REVIEW_LABEL]); + ensureDefaultLabel(command.repo); + ghBestEffort(["issue", "edit", String(command.issue_number), "--repo", command.repo, "--add-label", label]); command.actions = command.actions.map((action) => - action.action === "label" ? { ...action, status: "executed", label: HUMAN_REVIEW_LABEL } : action, + action.action === "label" ? { ...action, status: "executed", label } : action, ); } if (command.intent === "stop" && command.issue_number) { - ensureHumanReviewLabel(command.repo); - ghBestEffort(["issue", "edit", String(command.issue_number), "--repo", command.repo, "--add-label", HUMAN_REVIEW_LABEL]); + ensureDefaultLabel(command.repo); + ghBestEffort(["issue", "edit", String(command.issue_number), "--repo", command.repo, "--add-label", label]); command.actions = command.actions.map((action) => - action.action === "label" ? { ...action, status: "executed", label: HUMAN_REVIEW_LABEL } : action, + action.action === "label" ? { ...action, status: "executed", label } : action, ); } @@ -686,8 +685,8 @@ function executeAutomerge(command) { } const gateBlock = automergeGateBlockReason(process.env); if (gateBlock) { - ensureMergeReadyLabel(command.repo); - ghBestEffort(["issue", "edit", String(command.issue_number), "--repo", command.repo, "--add-label", MERGE_READY_LABEL]); + ensureDefaultLabel(command.repo); + ghBestEffort(["issue", "edit", String(command.issue_number), "--repo", command.repo, "--add-label", label]); return { action: "merge", status: "blocked", reason: gateBlock, merge_method: "squash" }; } const result = spawnSync( @@ -725,7 +724,6 @@ function executeAutomerge(command) { function validateAutomergeReadiness({ command, view, target }) { if (!hasLabel(target, AUTOMERGE_LABEL)) return "PR is not opted into Clownfish automerge"; - if (hasLabel(target, HUMAN_REVIEW_LABEL)) return "PR is paused for human review"; if (view.state && view.state !== "OPEN") return `pull request is ${String(view.state).toLowerCase()}`; if (view.isDraft) return "pull request is draft"; if (String(view.baseRefName ?? "") !== "main") return "pull request base is not main"; @@ -910,17 +908,17 @@ function postComment(command, body) { ghText(["api", `repos/${command.repo}/issues/${command.issue_number}/comments`, "--method", "POST", "--input", payloadPath]); } -function ensureHumanReviewLabel(repo) { +function ensureDefaultLabel(repo) { ghBestEffort([ "label", "create", - HUMAN_REVIEW_LABEL, + label, "--repo", repo, "--color", - "B60205", + DEFAULT_LABEL_COLOR, "--description", - "Needs maintainer review before Clownfish can continue", + DEFAULT_LABEL_DESCRIPTION, ]); } @@ -938,20 +936,6 @@ function ensureAutomergeLabel(repo) { ]); } -function ensureMergeReadyLabel(repo) { - ghBestEffort([ - "label", - "create", - MERGE_READY_LABEL, - "--repo", - repo, - "--color", - "0E8A16", - "--description", - "Clownfish found a merge-ready candidate; human owns the final merge", - ]); -} - function hasLabel(target, name) { return (target?.labels ?? []).some((labelName) => String(labelName).toLowerCase() === String(name).toLowerCase()); } diff --git a/scripts/execute-fix-artifact.mjs b/scripts/execute-fix-artifact.mjs index dbd13f6..e477c35 100644 --- a/scripts/execute-fix-artifact.mjs +++ b/scripts/execute-fix-artifact.mjs @@ -4,6 +4,13 @@ import os from "node:os"; import path from "node:path"; import { spawnSync } from "node:child_process"; import { assertAllowedOwner, parseArgs, parseJob, repoRoot, validateJob } from "./lib.mjs"; +import { + externalMessageProvenance, + repairContributorBranchComment, + replacementPrBody, + replacementSourceCloseComment, + replacementSourceLinkComment, +} from "./external-messages.mjs"; const FIX_ACTIONS = new Set(["fix_needed", "build_fix_artifact", "open_fix_pr"]); const REPAIR_STRATEGIES = new Set(["repair_contributor_branch", "replace_uneditable_branch", "new_fix_pr"]); @@ -31,11 +38,13 @@ const skipCodexWritePreflight = process.env.CLOWNFISH_SKIP_CODEX_WRITE_PREFLIGHT const allowExpensiveValidation = process.env.CLOWNFISH_ALLOW_EXPENSIVE_VALIDATION === "1"; const installTargetDeps = process.env.CLOWNFISH_INSTALL_TARGET_DEPS !== "0"; const allowBroadFixArtifacts = process.env.CLOWNFISH_ALLOW_BROAD_FIX_ARTIFACTS === "1"; +const closeSupersededSourcePrs = process.env.CLOWNFISH_CLOSE_SUPERSEDED_SOURCE_PRS === "1"; const maxAutonomousFixFiles = Math.max(1, Number(process.env.CLOWNFISH_MAX_AUTONOMOUS_FIX_FILES ?? 8)); const maxAutonomousFixSurfaces = Math.max(1, Number(process.env.CLOWNFISH_MAX_AUTONOMOUS_FIX_SURFACES ?? 4)); const maxActivePrsPerArea = Number(process.env.CLOWNFISH_MAX_ACTIVE_PRS_PER_AREA ?? 50); const CLOWNFISH_LABEL = "clownfish"; -const COMMIT_FINDING_LABEL = "clownfish:commit-finding"; +const CLOWNFISH_LABEL_COLOR = "F97316"; +const CLOWNFISH_LABEL_DESCRIPTION = "Tracked by Clownfish automation"; const strictTargetValidation = process.env.CLOWNFISH_STRICT_TARGET_VALIDATION === "1" || String(process.env.CLOWNFISH_TARGET_VALIDATION_MODE ?? "changed-only") === "strict"; @@ -367,13 +376,15 @@ function executeRepairBranch({ fixArtifact, targetDir }) { const pushArgs = repairBranchPushArgs({ pull, rebased }); run("git", pushArgs, { cwd: targetDir }); const threadResolution = prepareReviewThreadsForMerge({ repo: result.repo, number: sourcePr.number, targetDir }); - const comment = [ - "Thanks for the work here. Clownfish pushed a narrow repair to this branch so the original contributor path can stay canonical.", - "", - `Source PR: ${sourcePr.url}`, - `Validation: ${fixArtifact.validation_commands.join("; ")}`, - "Contributor credit stays preserved in the branch history and PR context.", - ].join("\n"); + const comment = repairContributorBranchComment({ + sourcePrUrl: sourcePr.url, + validationCommands: fixArtifact.validation_commands, + provenance: externalMessageProvenance({ + model, + reasoning: codexReasoningEffort, + reviewedSha: prep.commit, + }), + }); run("gh", ["pr", "comment", String(sourcePr.number), "--repo", result.repo, "--body", comment], { cwd: targetDir, env: ghEnv() }); return { action: "repair_contributor_branch", @@ -444,7 +455,12 @@ function executeReplacementBranch({ fixArtifact, targetDir, supersedeSources, fa if (refreshValidatedBranchBase({ targetDir, branch, baseBranch })) { prep.commit = currentHead(targetDir); } - const body = replacementPrBody(fixArtifact, fallbackReason, contributorCredits); + const provenance = externalMessageProvenance({ + model, + reasoning: codexReasoningEffort, + reviewedSha: prep.commit, + }); + const body = replacementPrBody({ fixArtifact, fallbackReason, clusterId: result.cluster_id, provenance }); if (dryRun) { return { action: "open_fix_pr", @@ -486,7 +502,9 @@ function executeReplacementBranch({ fixArtifact, targetDir, supersedeSources, fa const parsed = parsePullRequestUrl(source); if (!parsed || parsed.repo !== result.repo) continue; supersededSourceActions.push( - closeSupersededSourcePr({ source, parsed, replacementPrUrl: prUrl, targetDir, contributorCredits }), + closeSupersededSourcePrs + ? closeSupersededSourcePr({ source, parsed, replacementPrUrl: prUrl, targetDir, contributorCredits, provenance }) + : linkReplacementSourcePr({ source, parsed, replacementPrUrl: prUrl, targetDir, provenance }), ); } } @@ -508,12 +526,8 @@ function executeReplacementBranch({ fixArtifact, targetDir, supersedeSources, fa } function labelReplacementPullRequest({ number, targetDir }) { - ensureLabel(result.repo, CLOWNFISH_LABEL, "0E8A16", "Tracked by Clownfish automation", targetDir); + ensureLabel(result.repo, CLOWNFISH_LABEL, CLOWNFISH_LABEL_COLOR, CLOWNFISH_LABEL_DESCRIPTION, targetDir); addLabel(result.repo, number, CLOWNFISH_LABEL, targetDir); - if (job.frontmatter.source === "clawsweeper_commit" || job.frontmatter.commit_sha) { - ensureLabel(result.repo, COMMIT_FINDING_LABEL, "1D76DB", "PR created from a ClawSweeper commit finding", targetDir); - addLabel(result.repo, number, COMMIT_FINDING_LABEL, targetDir); - } } function ensureLabel(repo, name, color, description, targetDir) { @@ -534,7 +548,25 @@ function addLabel(repo, number, name, targetDir) { }); } -function closeSupersededSourcePr({ source, parsed, replacementPrUrl, targetDir, contributorCredits }) { +function linkReplacementSourcePr({ source, parsed, replacementPrUrl, targetDir, provenance }) { + const base = { source, pr: `#${parsed.number}`, action: "link_replacement_source" }; + const view = fetchSourcePullRequestView({ repo: result.repo, number: parsed.number, targetDir }); + if (view.mergedAt || view.state === "MERGED") { + return { ...base, status: "skipped", reason: "already merged", merged_at: view.mergedAt ?? null }; + } + if (view.state === "CLOSED") { + return { ...base, status: "skipped", reason: "already closed" }; + } + + const comment = replacementSourceLinkComment({ replacementPrUrl, sourcePrUrl: source, provenance }); + run("gh", ["pr", "comment", String(parsed.number), "--repo", result.repo, "--body", comment], { + cwd: targetDir, + env: ghEnv(), + }); + return { ...base, status: "executed", reason: "linked replacement PR without closing source PR" }; +} + +function closeSupersededSourcePr({ source, parsed, replacementPrUrl, targetDir, contributorCredits, provenance }) { const base = { source, pr: `#${parsed.number}`, action: "close_superseded_source" }; const view = fetchSourcePullRequestView({ repo: result.repo, number: parsed.number, targetDir }); if (view.mergedAt || view.state === "MERGED") { @@ -544,16 +576,7 @@ function closeSupersededSourcePr({ source, parsed, replacementPrUrl, targetDir, return { ...base, status: "skipped", reason: "already closed" }; } - const carriedCredit = sourceCreditLines({ source, contributorCredits }); - const comment = [ - "Thanks for the source work here. Clownfish could not safely update this branch, so I opened a narrow replacement PR instead.", - "", - `Replacement PR: ${replacementPrUrl}`, - `Source PR: ${source}`, - ...carriedCredit, - "Closing this PR to keep review focused on the replacement. The contribution is carried forward, not discarded.", - "If the replacement misses any intent from this PR, please carry that context over there and I can adjust it.", - ].join("\n"); + const comment = replacementSourceCloseComment({ replacementPrUrl, sourcePrUrl: source, provenance }); run("gh", ["pr", "comment", String(parsed.number), "--repo", result.repo, "--body", comment], { cwd: targetDir, env: ghEnv(), @@ -1302,22 +1325,6 @@ function codexReviewSchemaPath() { return schemaPath; } -function replacementPrBody(fixArtifact, fallbackReason, contributorCredits = []) { - const creditLines = contributorCredits.map((credit) => `- ${publicCreditLabel(credit)} from ${credit.sources.join(", ")}`); - const lines = [ - fixArtifact.pr_body.trim(), - "", - "Clownfish replacement details:", - `- Cluster: ${result.cluster_id}`, - `- Source PRs: ${(fixArtifact.source_prs ?? []).join(", ") || "none"}`, - `- Credit: ${fixArtifact.credit_notes.join("; ")}`, - ...(creditLines.length > 0 ? ["", "Carried-forward contributor credit:", ...creditLines] : []), - `- Validation: ${fixArtifact.validation_commands.join("; ")}`, - ]; - if (fallbackReason) lines.push(`- Repair fallback: ${fallbackReason}`); - return `${lines.join("\n")}\n`; -} - function sourceContributorCredits({ fixArtifact, targetDir }) { const byLogin = new Map(); for (const source of fixArtifact.source_prs ?? []) { @@ -1367,10 +1374,6 @@ function publicContributorCredit(credit) { }; } -function publicCreditLabel(credit) { - return `@${credit.login} (${credit.name || credit.login})`; -} - function safeTrailerName(value, fallback = "Contributor") { const name = String(value ?? "") .replace(/[<>\r\n]/g, "") @@ -1382,14 +1385,6 @@ function isBotLogin(login) { return /\[bot\]$|bot$/i.test(String(login ?? "")); } -function sourceCreditLines({ source, contributorCredits }) { - const matching = contributorCredits.filter((credit) => credit.sources.includes(source)); - if (matching.length === 0) { - return ["Contributor credit is preserved in the replacement PR body and changelog plan."]; - } - return matching.map((credit) => `Credit preserved: thanks @${credit.login}; they are carried forward as co-author in the replacement PR.`); -} - function validateActivePrAreaCapacity({ fixArtifact, targetDir, branch }) { if (!Number.isFinite(maxActivePrsPerArea) || maxActivePrsPerArea < 1) return null; const areas = affectedAreasForFiles(fixArtifact.likely_files ?? []); diff --git a/scripts/external-messages.mjs b/scripts/external-messages.mjs new file mode 100644 index 0000000..3858581 --- /dev/null +++ b/scripts/external-messages.mjs @@ -0,0 +1,360 @@ +import { randomInt } from "node:crypto"; + +const SIGNATURE = "Clownfish 🐠"; +const EVIDENCE_LIMIT = 5; + +function listOrNone(items) { + return items?.length ? items.join("; ") : "none"; +} + +function issueRef(value) { + return value ? `#${value}` : ""; +} + +function pick(items) { + return items[randomInt(items.length)]; +} + +function variant(items, context = {}) { + const item = pick(items); + return typeof item === "function" ? item(context) : item; +} + +function evidenceLines(evidence) { + return (Array.isArray(evidence) ? evidence : []) + .slice(0, EVIDENCE_LIMIT) + .map((item) => `- ${typeof item === "string" ? item : (item.detail ?? JSON.stringify(item))}`); +} + +const repairBranchOpeners = [ + "Thanks for the contribution here. Clownfish gave this branch a little current boost so the original PR can stay the canonical swim lane instead of opening a replacement.", + "Thanks for the contribution here. Clownfish nudged this branch back into the current, so the original PR can stay the review lane.", + "Thanks for the work here. Clownfish patched this branch directly so the contributor trail stays right where it started.", + "Thanks for the contribution here. Clownfish was able to repair this branch in place, which keeps the original PR as the clean canonical path.", + "Thanks for the work here. Clownfish got this branch swimming again without needing a replacement PR.", + "Thanks for the contribution here. Clownfish pushed the narrow repair here so review, credit, and history stay together.", + "Thanks for the work on this. Clownfish could write to this branch, so it kept the fix in the original PR instead of making a new one.", + "Thanks for the contribution here. Clownfish gave the branch a tidy little reef repair and kept this PR as the main lane.", +]; + +const preservedCreditLines = [ + "Contributor credit stays right here with this PR's history and changelog context. no lost treasure, no mystery bubbles.", + "Contributor credit stays attached to this PR and its history. no one gets washed out with the tide.", + "The contributor trail stays intact here: branch history, PR context, and changelog credit all still point back to this work.", + "Credit stays anchored to this PR. Clownfish is just moving the fix along, not stealing the shiny bits.", + "The useful context and credit stay on this branch. tidy swim, same contributor trail.", + "This keeps attribution in the original current: commits, review context, and changelog notes all stay visible.", + "Credit remains with the original contribution. Clownfish is just doing the branch-maintenance snorkel.", + "The contribution stays credited here. no vanishing act, no suspicious bubbles.", + "Attribution stays exactly where it should: with the original branch and PR history.", + "Contributor credit stays on this reef marker, with the PR history doing the receipts.", +]; + +const replacementPermissionLines = [ + "Thanks for the work on this. Clownfish could not push to this branch with the permissions available, so it opened a narrow replacement PR to keep the fix swimming forward without losing the contributor trail. not your fault, just GitHub branch-permission tides.", + "Thanks for the work on this. Clownfish did not have permission to update this branch directly, so it opened a narrow replacement PR instead. that's a branch access thing, not a knock on the contribution.", + "Thanks for the contribution here. Clownfish could not safely push to this branch, so it opened a replacement PR from a writable branch and carried the contributor trail along with it.", + "Thanks for the work here. GitHub would not let Clownfish push to this branch with the available credentials, so the fix moved to a narrow replacement PR. nothing personal, just permission currents.", + "Thanks for the contribution. Clownfish hit a branch-permission wall on this PR, so it opened a replacement branch to keep review moving while preserving credit.", + "Thanks for the work here. Clownfish could not write to the source branch, so it opened a replacement PR rather than letting the fix drift. attribution still points back here.", + "Thanks for the contribution here. Clownfish tried the original lane first, but branch permissions blocked the push, so a replacement PR is carrying the fix forward.", + "Thanks for the work on this. Clownfish opened a replacement PR only because the source branch was not writable from the available bot permissions. branch tides, not contributor blame.", + "Thanks for the useful work here. Clownfish could not update this branch directly, so the replacement PR is the writable swim lane for the same fix path.", + "Thanks for the contribution. The source branch was not safely writable by Clownfish, so it opened a replacement PR and kept the credit trail visible.", +]; + +const sourceStaysOpenLines = [ + "This source PR is staying open for maintainer and contributor review.", + "This source PR stays open so maintainers and the original contributor can compare the paths.", + "Leaving this source PR open for review context and contributor follow-up.", + "This source PR remains open; the replacement is just the writable fix lane.", + "Keeping this PR open so the original context is easy to inspect.", + "This PR stays open for now, with the replacement linked as the current fix path.", +]; + +const carriedCreditLines = [ + "Contributor credit is carried into the replacement PR body and changelog plan.", + "Contributor credit is copied into the replacement PR notes and changelog path.", + "The replacement PR carries the original credit trail forward.", + "Attribution is preserved in the replacement PR body and release-note trail.", + "The original contribution stays credited in the replacement PR context.", + "Credit follows the fix over to the replacement PR. no sneaky treasure grab.", + "The replacement PR keeps the contributor trail visible for review and changelog credit.", + "Attribution stays attached; the replacement just gives the fix a writable branch.", +]; + +const closeOnlyWhenEnabledLines = [ + "Closing this source PR only because source-PR closing was explicitly enabled for this run.", + "Closing this source PR because this run explicitly enabled source-PR closeout.", + "This source PR is being closed only under the explicit source-close setting for this Clownfish run.", + "Closing this one because the run was configured to close superseded source PRs after opening the replacement.", + "This closeout is intentional for this run: the replacement PR is now the active review lane.", +]; + +const cleanupOpeners = [ + ({ target }) => `Thanks for the report and the useful trail here. Clownfish reviewed this cluster and is closing #${target}.`, + ({ target }) => `Thanks for the report and all the context here. Clownfish reviewed the cluster and is closing #${target}.`, + ({ target }) => `Thanks for leaving a clear trail here. Clownfish checked this cluster and is closing #${target}.`, + ({ target }) => `Thanks for the useful report. Clownfish traced this through the cluster and is closing #${target}.`, + ({ target }) => `Thanks for the context here. Clownfish matched this against the cluster and is closing #${target}.`, + ({ target }) => `Thanks for the signal on this one. Clownfish reviewed the related path and is closing #${target}.`, +]; + +const duplicateLines = [ + ({ canonical }) => `This looks like the same school as ${issueRef(canonical)}, so Clownfish is keeping ${issueRef(canonical)} as the canonical thread where fixes, validation, and follow-up can all swim together.`, + ({ canonical }) => `This appears to overlap ${issueRef(canonical)}, so Clownfish is keeping that thread as the canonical place for the fix, validation, and follow-up.`, + ({ canonical }) => `This is tracking the same current as ${issueRef(canonical)}. Keeping ${issueRef(canonical)} canonical keeps review and validation in one place.`, + ({ canonical }) => `This duplicates the path in ${issueRef(canonical)}, so Clownfish is keeping the canonical trail there instead of splitting the reef map.`, + ({ canonical }) => `Same school as ${issueRef(canonical)}. Clownfish is keeping the canonical discussion there so the useful bits do not scatter.`, + ({ canonical }) => `This matches ${issueRef(canonical)} closely enough that keeping one canonical thread is the cleaner swim lane.`, +]; + +const supersededCanonicalLines = [ + ({ canonical }) => `This is superseded by ${issueRef(canonical)}. Clownfish is keeping that reef marker as the canonical path so the useful context and contributor credit stay visible.`, + ({ canonical }) => `This has been overtaken by ${issueRef(canonical)}, so Clownfish is keeping that as the current canonical path.`, + ({ canonical }) => `${issueRef(canonical)} is now the better canonical thread. Closing this keeps validation and context from drifting apart.`, + ({ canonical }) => `This is superseded by ${issueRef(canonical)}. Keeping the newer path canonical makes follow-up easier to review.`, + ({ canonical }) => `The active fix trail is now ${issueRef(canonical)}, so Clownfish is closing this older marker and keeping the context attached there.`, +]; + +const supersededFixLines = [ + ({ candidateFix }) => `This is superseded by landed fix ${issueRef(candidateFix)}. Clownfish is closing this older overlap so validation and follow-up stay attached to the shipped path instead of drifting around the reef.`, + ({ candidateFix }) => `Landed fix ${issueRef(candidateFix)} covers this path now, so Clownfish is closing the older overlap and keeping the receipts on the shipped fix.`, + ({ candidateFix }) => `This has been handled by landed fix ${issueRef(candidateFix)}. Closing this keeps follow-up tied to the code that actually shipped.`, + ({ candidateFix }) => `${issueRef(candidateFix)} has landed for this path, so Clownfish is closing this older thread to keep validation from scattering.`, + ({ candidateFix }) => `The shipped fix is ${issueRef(candidateFix)}. Closing this older overlap keeps the current tidy.`, +]; + +const candidateFixLines = [ + ({ candidateFix }) => `This is covered by candidate fix ${issueRef(candidateFix)}. Clownfish is closing this thread so validation and follow-up stay attached to that fix path instead of scattering like bubbles.`, + ({ candidateFix }) => `Candidate fix ${issueRef(candidateFix)} is carrying this path now, so Clownfish is keeping follow-up attached there.`, + ({ candidateFix }) => `This is covered by ${issueRef(candidateFix)}. Closing this keeps the active review lane focused instead of splitting the school.`, + ({ candidateFix }) => `${issueRef(candidateFix)} is the active fix path for this one, so Clownfish is closing this duplicate current.`, + ({ candidateFix }) => `This belongs with candidate fix ${issueRef(candidateFix)} now. Keeping one lane makes review and validation less slippery.`, +]; + +const reopenLines = [ + "If this still reproduces by a different route, reply here and we can fish it back out.", + "If this still splashes on current main through a different path, reply here and we can reopen or split it back out.", + "If there is a separate reproduction path hiding under this, reply here and Clownfish can pull it back into the light.", + "If this is still real on current main by a different route, reply here and we can reopen the trail.", + "If this closeout misses a distinct bug path, reply here and we can separate it cleanly.", + "If the canonical path does not cover your case, reply here and we can fish the thread back out.", +]; + +const postMergeCloseLines = [ + "Closing this now that the validated fix is merged. If this still splashes on current main by a different path, reply here and we can reopen or split it back out.", + "Closing this now that the validated fix has landed. If current main still shows a different path, reply here and we can reopen.", + "The validated fix is merged, so Clownfish is closing this trail. If a separate reproduction remains, reply here and we can split it back out.", + "Closing after the canonical fix landed. If another route still reproduces, reply here and we can pull that thread back up.", + "This is closed now that the fix is on main. If the issue still swims through a different channel, reply here and we can reopen.", +]; + +function fishNotes(provenance) { + const model = provenance?.model ?? process.env.CLOWNFISH_MODEL ?? "gpt-5.5"; + const reasoning = provenance?.reasoning ?? process.env.CLOWNFISH_CODEX_REASONING_EFFORT ?? "medium"; + const reviewedSha = provenance?.reviewedSha ?? provenance?.reviewed_sha; + const reviewed = reviewedSha ? `; reviewed against ${String(reviewedSha).slice(0, 12)}` : ""; + return `fish notes: model ${model}, reasoning ${reasoning}${reviewed}.`; +} + +export function externalMessageProvenance({ model, reasoning, reviewedSha } = {}) { + return { + model: model ?? process.env.CLOWNFISH_MODEL ?? "gpt-5.5", + reasoning: reasoning ?? process.env.CLOWNFISH_CODEX_REASONING_EFFORT ?? "medium", + reviewedSha, + }; +} + +function withFishNotes(lines, provenance) { + return [...lines, "", fishNotes(provenance)].join("\n"); +} + +export function repairContributorBranchComment({ sourcePrUrl, validationCommands, provenance }) { + return withFishNotes([ + `${SIGNATURE} reef update`, + "", + variant(repairBranchOpeners), + "", + `Source PR: ${sourcePrUrl}`, + `Validation: ${listOrNone(validationCommands)}`, + variant(preservedCreditLines), + ], provenance); +} + +export function replacementSourceLinkComment({ replacementPrUrl, sourcePrUrl, provenance }) { + return withFishNotes([ + `${SIGNATURE} reef update`, + "", + variant(replacementPermissionLines), + "", + `Replacement PR: ${replacementPrUrl}`, + `Source PR: ${sourcePrUrl}`, + variant(sourceStaysOpenLines), + variant(carriedCreditLines), + ], provenance); +} + +export function replacementSourceCloseComment({ replacementPrUrl, sourcePrUrl, provenance }) { + return withFishNotes([ + `${SIGNATURE} reef update`, + "", + variant(replacementPermissionLines), + "", + `Replacement PR: ${replacementPrUrl}`, + `Source PR: ${sourcePrUrl}`, + variant(closeOnlyWhenEnabledLines), + variant(carriedCreditLines), + ], provenance); +} + +export function replacementPrBody({ fixArtifact, fallbackReason, clusterId, provenance }) { + const lines = [ + fixArtifact.pr_body.trim(), + "", + `${SIGNATURE} replacement reef notes:`, + `- Cluster: ${clusterId}`, + `- Source PRs: ${(fixArtifact.source_prs ?? []).join(", ") || "none"}`, + `- Credit: ${listOrNone(fixArtifact.credit_notes)}`, + `- Validation: ${listOrNone(fixArtifact.validation_commands)}`, + ]; + if (fallbackReason) lines.push(`- Repair fallback: ${fallbackReason}`); + lines.push("", fishNotes(provenance)); + return `${lines.join("\n")}\n`; +} + +export function defaultCloseComment({ + action, + classification, + clusterId, + target, + title, + canonical, + candidateFix, + reason, + provenance, +}) { + const lines = [`${SIGNATURE} reef cleanup`, "", variant(cleanupOpeners, { target })]; + + lines.push(""); + if (classification === "duplicate" && canonical) { + lines.push(variant(duplicateLines, { canonical })); + } else if (classification === "superseded" && canonical) { + lines.push(variant(supersededCanonicalLines, { canonical })); + } else if (classification === "superseded" && candidateFix) { + lines.push(variant(supersededFixLines, { candidateFix })); + } else if (classification === "fixed_by_candidate" && candidateFix) { + lines.push(variant(candidateFixLines, { candidateFix })); + } else if (classification === "low_signal") { + lines.push( + "This falls under low-signal PR cleanup: the PR does not currently present a reviewable OpenClaw fix with maintainer signal, current validation, or a focused product path. Please reopen from a clean branch with a scoped summary, linked issue or rationale, and validation if this still needs another swim.", + ); + } else { + lines.push(reason); + } + + lines.push("", `Cluster: \`${clusterId}\``, `Reviewed item: #${target} - ${title}`); + const renderedEvidence = evidenceLines(action.evidence); + if (renderedEvidence.length) lines.push("", "Evidence:", ...renderedEvidence); + lines.push("", variant(reopenLines)); + lines.push("", fishNotes(provenance)); + return lines.join("\n"); +} + +export function postMergeCloseoutComment({ actionName, fixUrl, provenance }) { + const relation = actionName === "close_superseded" ? "superseded by" : "covered by"; + return withFishNotes([ + `${SIGNATURE} landed`, + "", + `Thanks for the report and context here. This is ${relation} ${fixUrl}, which has landed as the canonical Clownfish fix path for this cluster.`, + "", + variant(postMergeCloseLines), + ], provenance); +} + +export function sampleExternalMessages() { + const provenance = externalMessageProvenance({ + model: "gpt-5.5", + reasoning: "high", + reviewedSha: "ba0f2e948fc0cafe1234567890abcdef12345678", + }); + const baseAction = { + evidence: [ + "The same reproduction is tracked on the canonical thread.", + { detail: "The replacement PR carries the current validation path." }, + ], + }; + return [ + { + title: "Contributor Branch Repair", + body: repairContributorBranchComment({ + sourcePrUrl: "https://github.com/openclaw/openclaw/pull/12345", + validationCommands: ["pnpm test:serial src/example.test.ts", "pnpm check:changed"], + provenance, + }), + }, + { + title: "Replacement PR Link", + body: replacementSourceLinkComment({ + replacementPrUrl: "https://github.com/openclaw/openclaw/pull/67890", + sourcePrUrl: "https://github.com/openclaw/openclaw/pull/12345", + provenance, + }), + }, + { + title: "Replacement PR Close", + body: replacementSourceCloseComment({ + replacementPrUrl: "https://github.com/openclaw/openclaw/pull/67890", + sourcePrUrl: "https://github.com/openclaw/openclaw/pull/12345", + provenance, + }), + }, + { + title: "Replacement PR Body", + body: replacementPrBody({ + clusterId: "ghcrawl-123456-agentic-merge", + fixArtifact: { + pr_body: "Fixes the focused provider auth regression.\n\nValidation: `pnpm check:changed`", + source_prs: ["https://github.com/openclaw/openclaw/pull/12345"], + credit_notes: ["Thanks @contributor for the original report and branch."], + validation_commands: ["pnpm check:changed"], + }, + fallbackReason: "source branch was not safely writable", + provenance, + }), + }, + { + title: "Duplicate Closeout", + body: defaultCloseComment({ + action: baseAction, + classification: "duplicate", + clusterId: "ghcrawl-123456-agentic-merge", + target: 54321, + title: "Duplicate provider auth bug", + canonical: 12345, + reason: "duplicate of the canonical thread", + provenance, + }), + }, + { + title: "Low-Signal Closeout", + body: defaultCloseComment({ + action: { evidence: ["No current validation or focused OpenClaw change was present."] }, + classification: "low_signal", + clusterId: "low-signal-pr-sweep-20260427T0530-01", + target: 55555, + title: "Unscoped cleanup draft", + reason: "low-signal PR cleanup", + provenance, + }), + }, + { + title: "Post-Merge Closeout", + body: postMergeCloseoutComment({ + actionName: "close_fixed_by_candidate", + fixUrl: "https://github.com/openclaw/openclaw/pull/67890", + provenance, + }), + }, + ]; +} diff --git a/scripts/post-flight.mjs b/scripts/post-flight.mjs index 4e88d21..cd7ee83 100644 --- a/scripts/post-flight.mjs +++ b/scripts/post-flight.mjs @@ -3,6 +3,7 @@ import fs from "node:fs"; import path from "node:path"; import { execFileSync } from "node:child_process"; import { assertAllowedOwner, hasSecuritySignalText, parseArgs, parseJob, repoRoot, validateJob } from "./lib.mjs"; +import { externalMessageProvenance, postMergeCloseoutComment } from "./external-messages.mjs"; const PASSING_CHECK_CONCLUSIONS = new Set(["SUCCESS", "SKIPPED", "NEUTRAL"]); const CLEAN_MERGE_STATES = new Set(["CLEAN", "HAS_HOOKS"]); @@ -11,8 +12,9 @@ const FIX_PR_ACTIONS = new Set(["open_fix_pr", "repair_contributor_branch"]); const FIX_PR_READY_STATUSES = new Set(["opened", "pushed"]); const POST_MERGE_CLOSE_ACTIONS = new Set(["close_duplicate", "close_superseded", "close_fixed_by_candidate", "post_merge_close"]); const DEFAULT_IGNORED_CHECKS = ["auto-response", "Labeler", "Stale"]; -const HUMAN_REVIEW_LABEL = "clownfish:human-review"; -const MERGE_READY_LABEL = "clownfish:merge-ready"; +const CLOWNFISH_LABEL = "clownfish"; +const CLOWNFISH_LABEL_COLOR = "F97316"; +const CLOWNFISH_LABEL_DESCRIPTION = "Tracked by Clownfish automation"; const POST_FLIGHT_WAIT_MS = numberEnv("CLOWNFISH_POST_FLIGHT_WAIT_MS", 10 * 60 * 1000); const POST_FLIGHT_POLL_MS = numberEnv("CLOWNFISH_POST_FLIGHT_POLL_MS", 15 * 1000); @@ -161,11 +163,11 @@ function finalizeFixPr(action) { } if (process.env.CLOWNFISH_ALLOW_MERGE !== "1") { - labelForHumanMergeReview(result.repo, parsed.number); + labelForClownfishReview(result.repo, parsed.number); return { ...prBase, status: "blocked", - reason: "merge requires CLOWNFISH_ALLOW_MERGE=1; labeled for human review", + reason: "merge requires CLOWNFISH_ALLOW_MERGE=1; labeled clownfish", merge_method: "squash", waited_ms: waitedMs, }; @@ -255,7 +257,13 @@ function finalizePostMergeCloseout({ action, actionName, target, fixRef, fixUrl, "--repo", result.repo, "--body", - closeoutBody({ actionName, fixUrl }), + postMergeCloseoutComment({ + actionName, + fixUrl, + provenance: externalMessageProvenance({ + reviewedSha: finalized.merge_commit_sha ?? action.commit ?? result.reviewed_sha ?? result.head_sha, + }), + }), ]); if (live.pull_request) { ghWithRetry(["pr", "close", String(target), "--repo", result.repo]); @@ -272,15 +280,6 @@ function finalizePostMergeCloseout({ action, actionName, target, fixRef, fixUrl, }; } -function closeoutBody({ actionName, fixUrl }) { - const relation = actionName === "close_superseded" ? "superseded by" : "covered by"; - return [ - `Thanks for this. Clownfish sees this as ${relation} ${fixUrl}, which has landed as the canonical fix path for this cluster.`, - "", - "Closing this now that the validated fix is merged. If this still reproduces on current main with a different path, reply here and we can reopen or split it back out.", - ].join("\n"); -} - function validateMergePolicy() { if (!job.frontmatter.allowed_actions.includes("merge")) return "job does not allow merge"; if ((job.frontmatter.blocked_actions ?? []).includes("merge")) return "merge is blocked by job frontmatter"; @@ -288,11 +287,9 @@ function validateMergePolicy() { return ""; } -function labelForHumanMergeReview(repo, number) { - ensureLabel(repo, HUMAN_REVIEW_LABEL, "B60205", "Needs maintainer review before Clownfish can finish"); - ensureLabel(repo, MERGE_READY_LABEL, "0E8A16", "Clownfish found a merge-ready candidate; human owns the final merge"); - ghBestEffort(["issue", "edit", String(number), "--repo", repo, "--add-label", HUMAN_REVIEW_LABEL]); - ghBestEffort(["issue", "edit", String(number), "--repo", repo, "--add-label", MERGE_READY_LABEL]); +function labelForClownfishReview(repo, number) { + ensureLabel(repo, CLOWNFISH_LABEL, CLOWNFISH_LABEL_COLOR, CLOWNFISH_LABEL_DESCRIPTION); + ghBestEffort(["issue", "edit", String(number), "--repo", repo, "--add-label", CLOWNFISH_LABEL]); } function ensureLabel(repo, name, color, description) { diff --git a/scripts/render-message-samples.mjs b/scripts/render-message-samples.mjs new file mode 100644 index 0000000..3afdc95 --- /dev/null +++ b/scripts/render-message-samples.mjs @@ -0,0 +1,15 @@ +#!/usr/bin/env node +import { sampleExternalMessages } from "./external-messages.mjs"; + +const iterations = Math.max(1, Number(process.env.CLOWNFISH_MESSAGE_SAMPLE_ITERATIONS ?? 3)); + +for (let iteration = 1; iteration <= iterations; iteration += 1) { + console.log(`# Iteration ${iteration}`); + console.log(""); + for (const sample of sampleExternalMessages()) { + console.log(`## ${sample.title}`); + console.log(""); + console.log(sample.body); + console.log(""); + } +}