fix: make Clownfish comments friendlier
This commit is contained in:
parent
43b352a781
commit
e020282908
@ -30,7 +30,7 @@ Never close:
|
||||
Useful contributor PR replacement exception:
|
||||
|
||||
- close or supersede the PR only after the run has a concrete replacement fix plan or PR path;
|
||||
- the close comment must say why ProjectClownfish cannot safely update or land the branch;
|
||||
- the close comment must say why Clownfish cannot safely update or land the branch;
|
||||
- the comment must name the replacement path and state that the contributor will be credited;
|
||||
- the comment must explicitly contain credit wording such as `credit`, `attribution`, `Thanks @user`, or `source PR`;
|
||||
- the fix artifact must include the contributor username, original PR URL, validation plan, and changelog attribution when the fix is user-facing.
|
||||
|
||||
@ -35,7 +35,7 @@ Before drive mode:
|
||||
7. For each useful open contributor PR, choose the repair path before merge or close:
|
||||
- if `pull_request.maintainer_can_modify` is true and the diff is narrow enough, plan to update that PR branch, address review/bot findings, rebase, run checks, then emit `merge_canonical` only after it is clean;
|
||||
- if `maintainer_can_modify` is false, the branch is unsafe, or the PR contains broad/unrelated churn, do not merge it and do not ask whether to wait. Emit a replacement `build_fix_artifact` / `open_fix_pr` plan that preserves the contributor's credit in `credit_notes`, PR body, and changelog plan. Put every original contributor PR in `fix_artifact.source_prs` as a full `https://github.com/<owner>/<repo>/pull/<number>` URL;
|
||||
- when replacing a useful contributor PR, emit a blocked `close_superseded` comment that says ProjectClownfish cannot safely update that branch, will carry the narrow fix forward separately, and will credit the contributor by username and PR URL. Keep `candidate_fix` null until the replacement PR exists; do not point `candidate_fix` at the same PR being closed.
|
||||
- when replacing a useful contributor PR, emit a blocked `close_superseded` comment that says Clownfish cannot safely update that branch, will carry the narrow fix forward separately, and will credit the contributor by username and PR URL. Keep `candidate_fix` null until the replacement PR exists; do not point `candidate_fix` at the same PR being closed.
|
||||
8. Do not emit closure actions until the canonical path is explicit. If the cluster is over-broad, split it into subfamilies in the action matrix and use `keep_related`/`keep_independent` for clear non-targets instead of making the whole result `needs_human`.
|
||||
9. When `require_fix_before_close` blocks an otherwise-clear duplicate/superseded closeout, use `status: "blocked"` and say the close is blocked on the canonical fix path or fix PR. If the item is clearly covered by an already-merged candidate PR or current `main`, `close_fixed_by_candidate` may stay `planned` with that merged candidate evidence. Do not use different vague wording.
|
||||
10. If an item is not a true duplicate, run a single-item review/check/decide path: keep it related or independent when that is clear, emit a narrow fix artifact when it is a real bug or provider gap with no viable PR, and use `needs_human` only for product-direction or trust-boundary decisions that remain after checking the artifact.
|
||||
|
||||
@ -12,6 +12,14 @@ Priorities:
|
||||
4. stop on ambiguity;
|
||||
5. produce structured results.
|
||||
|
||||
Public comment voice:
|
||||
|
||||
- Sound like ClawSweeper: warm, concise, evidence-backed, and maintainer-helpful.
|
||||
- Start with thanks or a clear update when closing, replacing, or repairing someone's work.
|
||||
- Say `Clownfish`, not `ProjectClownfish`, in public-facing comments.
|
||||
- Preserve contributor dignity: explain what is happening, name the canonical path, and state how credit is carried forward.
|
||||
- Avoid robotic policy phrasing when a short human sentence says the same thing.
|
||||
|
||||
Before action:
|
||||
|
||||
- read the job frontmatter and body;
|
||||
@ -35,7 +43,7 @@ Execution guard:
|
||||
|
||||
- In `plan` mode, do not mutate GitHub.
|
||||
- In `execute` mode, do not mutate GitHub directly; emit structured actions for the applicator.
|
||||
- In `autonomous` mode, do not mutate GitHub directly; emit structured actions and fix artifacts for Projectclownfish scripts to apply.
|
||||
- In `autonomous` mode, do not mutate GitHub directly; emit structured actions and fix artifacts for Clownfish scripts to apply.
|
||||
- Never mark the overall result or an action as `executed`; only the deterministic applicator may record executed mutations after replaying a `planned` action.
|
||||
- Closure actions are only valid for targets that are open in live GitHub state.
|
||||
- Already-closed refs must not receive `close_*` actions. Use `keep_closed` with `status: "skipped"` only if you must mention them in the action matrix.
|
||||
|
||||
@ -500,8 +500,8 @@ function validateMergePolicy({ job, action }) {
|
||||
}
|
||||
|
||||
function labelForHumanMergeReview(repo, target) {
|
||||
ensureLabel(repo, HUMAN_REVIEW_LABEL, "B60205", "Needs maintainer review before ProjectClownfish can finish");
|
||||
ensureLabel(repo, MERGE_READY_LABEL, "0E8A16", "ProjectClownfish found a merge-ready candidate; human owns the final merge");
|
||||
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]);
|
||||
}
|
||||
@ -706,7 +706,7 @@ function renderCloseComment({ action, classification, result, target, live }) {
|
||||
.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. Projectclownfish reviewed this cluster and is closing #${target}.`];
|
||||
const lines = [`Thanks for this. Clownfish reviewed this cluster and is closing #${target}.`];
|
||||
lines.push("");
|
||||
if (classification === "duplicate" && canonical) {
|
||||
lines.push(
|
||||
|
||||
@ -35,9 +35,11 @@ export function renderResponse(command, dispatched) {
|
||||
if (command.intent === "help") {
|
||||
return [
|
||||
marker,
|
||||
"Clownfish is listening for maintainer commands.",
|
||||
"Clownfish is here and listening for maintainer commands.",
|
||||
"",
|
||||
"Supported commands: `/clownfish status`, `/clownfish fix ci`, `/clownfish address review`, `/clownfish rebase`, `/clownfish explain`, `/clownfish stop`.",
|
||||
"",
|
||||
"I only act for maintainers, or for trusted ClawSweeper repair feedback on an existing Clownfish PR.",
|
||||
].join("\n");
|
||||
}
|
||||
if (["status", "explain"].includes(command.intent)) {
|
||||
@ -46,25 +48,26 @@ export function renderResponse(command, dispatched) {
|
||||
if (command.intent === "stop") {
|
||||
return [
|
||||
marker,
|
||||
"Clownfish will leave this item for human review.",
|
||||
"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.",
|
||||
"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.",
|
||||
].join("\n");
|
||||
}
|
||||
if (!dispatched) {
|
||||
return [
|
||||
marker,
|
||||
"Clownfish did not dispatch a repair worker.",
|
||||
"Clownfish did not dispatch a repair worker for this one.",
|
||||
"",
|
||||
`Reason: ${command.reason ?? "unsupported command or target"}.`,
|
||||
"",
|
||||
"Supported repair commands currently work on existing Clownfish PRs only: `/clownfish fix ci`, `/clownfish address review`, `/clownfish rebase`.",
|
||||
"A maintainer can point me at one of those PRs and I can take another pass.",
|
||||
].join("\n");
|
||||
}
|
||||
if (command.intent === "clawsweeper_auto_repair") {
|
||||
return [
|
||||
marker,
|
||||
"Clownfish picked up ClawSweeper feedback.",
|
||||
"Thanks, ClawSweeper. Clownfish picked up the repair feedback.",
|
||||
"",
|
||||
`Source: \`${command.trusted_bot_author ?? command.author ?? "trusted automation"}\``,
|
||||
`Action: dispatched \`${dispatched.workflow}\` for \`${dispatched.job_path}\` in \`${dispatched.mode}\` mode.`,
|
||||
@ -81,7 +84,7 @@ export function renderResponse(command, dispatched) {
|
||||
`Action: dispatched \`${dispatched.workflow}\` for \`${dispatched.job_path}\` in \`${dispatched.mode}\` mode.`,
|
||||
`Model: \`${dispatched.model}\``,
|
||||
"",
|
||||
"I will update the PR branch if the repair worker finds a safe, narrow fix.",
|
||||
"I will keep the change narrow and update the PR branch if the repair worker finds a safe fix.",
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
|
||||
@ -288,7 +288,7 @@ function executableReplacementFixArtifact(fixArtifact, workerResult) {
|
||||
repair_strategy: "replace_uneditable_branch",
|
||||
branch_update_blockers: uniqueStrings([
|
||||
...(fixArtifact.branch_update_blockers ?? []),
|
||||
"ProjectClownfish policy: useful uneditable or unsafe source PRs are replaced with a narrow credited PR when fix execution is explicitly enabled.",
|
||||
"Clownfish policy: useful uneditable or unsafe source PRs are replaced with a narrow credited PR when fix execution is explicitly enabled.",
|
||||
]),
|
||||
credit_notes: uniqueStrings([
|
||||
...(fixArtifact.credit_notes ?? []),
|
||||
@ -368,11 +368,11 @@ function executeRepairBranch({ fixArtifact, targetDir }) {
|
||||
run("git", pushArgs, { cwd: targetDir });
|
||||
const threadResolution = prepareReviewThreadsForMerge({ repo: result.repo, number: sourcePr.number, targetDir });
|
||||
const comment = [
|
||||
"ProjectClownfish pushed a narrow repair to this branch so the original contributor path can stay canonical.",
|
||||
"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 is preserved in the branch history and PR context.",
|
||||
"Contributor credit stays preserved in the branch history and PR context.",
|
||||
].join("\n");
|
||||
run("gh", ["pr", "comment", String(sourcePr.number), "--repo", result.repo, "--body", comment], { cwd: targetDir, env: ghEnv() });
|
||||
return {
|
||||
@ -546,12 +546,13 @@ function closeSupersededSourcePr({ source, parsed, replacementPrUrl, targetDir,
|
||||
|
||||
const carriedCredit = sourceCreditLines({ source, contributorCredits });
|
||||
const comment = [
|
||||
"Clownfish could not safely update this branch, so it opened a narrow replacement PR instead.",
|
||||
"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");
|
||||
run("gh", ["pr", "comment", String(parsed.number), "--repo", result.repo, "--body", comment], {
|
||||
cwd: targetDir,
|
||||
@ -1386,7 +1387,7 @@ function sourceCreditLines({ source, contributorCredits }) {
|
||||
if (matching.length === 0) {
|
||||
return ["Contributor credit is preserved in the replacement PR body and changelog plan."];
|
||||
}
|
||||
return matching.map((credit) => `Credit preserved: @${credit.login} is carried forward as co-author in the replacement PR.`);
|
||||
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 }) {
|
||||
|
||||
@ -393,7 +393,7 @@ function buildFixArtifact(plan, job) {
|
||||
: "Post-merge closure disabled by job frontmatter.",
|
||||
fix_first_close:
|
||||
job.frontmatter.require_fix_before_close === true
|
||||
? "Do not emit close actions until ProjectClownfish has opened/pushed a fix PR or merged a canonical PR in this run."
|
||||
? "Do not emit close actions until Clownfish has opened/pushed a fix PR or merged a canonical PR in this run."
|
||||
: "Close actions may run independently when their own safety gates pass.",
|
||||
},
|
||||
required_validation: [
|
||||
@ -411,7 +411,7 @@ function buildFixArtifact(plan, job) {
|
||||
"use canonical/duplicate_of/candidate_fix refs only when those refs are hydrated preflight items; unhydrated PR refs found in comments belong in evidence or fix_artifact until hydrated",
|
||||
"include targeted tests and changelog plan for fix artifacts",
|
||||
"do not plan executable fix PRs for broad feature/config/docs rewrites; split them into narrower follow-up jobs or mark implementation blocked with exact sub-scopes",
|
||||
"if replacing a contributor PR, include source PR credit and the exact close comment that says ProjectClownfish will preserve attribution",
|
||||
"if replacing a contributor PR, include source PR credit and the exact close comment that says Clownfish will preserve attribution",
|
||||
"include full GitHub URLs in closure rationale",
|
||||
],
|
||||
};
|
||||
|
||||
@ -275,7 +275,7 @@ function finalizePostMergeCloseout({ action, actionName, target, fixRef, fixUrl,
|
||||
function closeoutBody({ actionName, fixUrl }) {
|
||||
const relation = actionName === "close_superseded" ? "superseded by" : "covered by";
|
||||
return [
|
||||
`This is ${relation} ${fixUrl}, which has landed as the canonical ProjectClownfish fix path for this cluster.`,
|
||||
`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");
|
||||
@ -289,8 +289,8 @@ function validateMergePolicy() {
|
||||
}
|
||||
|
||||
function labelForHumanMergeReview(repo, number) {
|
||||
ensureLabel(repo, HUMAN_REVIEW_LABEL, "B60205", "Needs maintainer review before ProjectClownfish can finish");
|
||||
ensureLabel(repo, MERGE_READY_LABEL, "0E8A16", "ProjectClownfish found a merge-ready candidate; human owns the final merge");
|
||||
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]);
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user