fix(repair): cap generated PR titles (#21)
This commit is contained in:
parent
bbc45f4933
commit
d21e3b0f44
@ -326,7 +326,8 @@
|
||||
}
|
||||
},
|
||||
"pr_title": {
|
||||
"type": "string"
|
||||
"type": "string",
|
||||
"maxLength": 256
|
||||
},
|
||||
"pr_body": {
|
||||
"type": "string"
|
||||
|
||||
@ -13,6 +13,7 @@ import {
|
||||
} from "./lib.js";
|
||||
import { ghText } from "./github-cli.js";
|
||||
import { readJsonFileIfExists as readJsonIfExists } from "./json-file.js";
|
||||
import { commitFindingPrTitle } from "./pr-title.js";
|
||||
import { escapeRegExp, slug } from "./text-utils.js";
|
||||
|
||||
const args = parseArgs(process.argv.slice(2));
|
||||
@ -323,7 +324,7 @@ function writeSyntheticRun(context: LooseRecord) {
|
||||
? `Original commit author: ${stripEmailIdentity(context.report.author)}.`
|
||||
: "Original commit author unknown.",
|
||||
],
|
||||
pr_title: prTitle(summary, context.report.body),
|
||||
pr_title: commitFindingPrTitle(summary, context.report.body),
|
||||
pr_body: prBody({ ...context, summary, likelyFiles, validation }),
|
||||
source_prs: [],
|
||||
repair_strategy: "new_fix_pr",
|
||||
@ -503,30 +504,6 @@ function validationCommands(repo: string) {
|
||||
return repo === "openclaw/openclaw" ? ["pnpm check:changed"] : ["git diff --check"];
|
||||
}
|
||||
|
||||
function prTitle(summary: LooseRecord, markdown: string = "") {
|
||||
const text = [summary, markdown].join("\n");
|
||||
if (
|
||||
/extension[- ]shard matrix|extension shard|run_checks_node_extensions|checks-node-extensions/i.test(
|
||||
text,
|
||||
)
|
||||
) {
|
||||
return "fix(ci): gate extension aggregate on shard matrix";
|
||||
}
|
||||
|
||||
let title = summary.replace(/^[-*\s]+/, "");
|
||||
title = title.replace(
|
||||
/^Found (?:one|an?|the)?\s*(?:high|medium|low|critical)?\s*(?:CI\s+)?(?:regression|bug|issue|finding):\s*/i,
|
||||
"",
|
||||
);
|
||||
title = compact(title, 68).replace(/[.!?]+$/, "");
|
||||
const prefix = /\b(?:CI|workflow|check|job|matrix|GitHub Actions)\b/i.test(summary)
|
||||
? "fix(ci):"
|
||||
: "fix:";
|
||||
return /^fix(?:\(|:)/i.test(title)
|
||||
? title
|
||||
: `${prefix} ${title || "address ClawSweeper finding"}`;
|
||||
}
|
||||
|
||||
function prBody(context: LooseRecord) {
|
||||
const findings = findingsFromReport(context.report.body);
|
||||
const reviewed = sectionBullets(context.report.body, "Reviewed").slice(0, 6);
|
||||
|
||||
@ -2,6 +2,7 @@ import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
|
||||
import type { JsonValue, LooseRecord } from "./json-types.js";
|
||||
import { GITHUB_PR_TITLE_MAX_LENGTH } from "./pr-title.js";
|
||||
|
||||
const REPAIR_STRATEGIES = new Set([
|
||||
"repair_contributor_branch",
|
||||
@ -18,6 +19,11 @@ export function validateFixArtifact(fixArtifact: LooseRecord): LooseRecord {
|
||||
throw new Error(`fix_artifact.${key} is required`);
|
||||
}
|
||||
}
|
||||
if (String(fixArtifact.pr_title).length > GITHUB_PR_TITLE_MAX_LENGTH) {
|
||||
throw new Error(
|
||||
`fix_artifact.pr_title must be ${GITHUB_PR_TITLE_MAX_LENGTH} characters or fewer`,
|
||||
);
|
||||
}
|
||||
for (const key of [
|
||||
"affected_surfaces",
|
||||
"likely_files",
|
||||
|
||||
100
src/repair/pr-title.ts
Normal file
100
src/repair/pr-title.ts
Normal file
@ -0,0 +1,100 @@
|
||||
export const GITHUB_PR_TITLE_MAX_LENGTH = 256;
|
||||
export const CLAWSWEEPER_GENERATED_PR_TITLE_MAX_LENGTH = 96;
|
||||
|
||||
const FALLBACK_REPAIR_PR_TITLE = "fix: address ClawSweeper finding";
|
||||
|
||||
export function normalizeGithubPrTitle(
|
||||
value: unknown,
|
||||
fallback = FALLBACK_REPAIR_PR_TITLE,
|
||||
maxLength = GITHUB_PR_TITLE_MAX_LENGTH,
|
||||
) {
|
||||
const title = compactTitle(value, maxLength) || compactTitle(fallback, maxLength);
|
||||
return title || "fix";
|
||||
}
|
||||
|
||||
export function commitFindingPrTitle(summary: unknown, markdown: unknown = "") {
|
||||
const text = [summary, markdown].join("\n");
|
||||
if (
|
||||
/extension[- ]shard matrix|extension shard|run_checks_node_extensions|checks-node-extensions/i.test(
|
||||
text,
|
||||
)
|
||||
) {
|
||||
return normalizeGithubPrTitle(
|
||||
"fix(ci): gate extension aggregate on shard matrix",
|
||||
FALLBACK_REPAIR_PR_TITLE,
|
||||
CLAWSWEEPER_GENERATED_PR_TITLE_MAX_LENGTH,
|
||||
);
|
||||
}
|
||||
|
||||
const rawSummary = normalizeTitleText(summary).replace(/^[-*]\s+/, "");
|
||||
const stem = findingTitleStem(rawSummary);
|
||||
const prefix = /\b(?:CI|workflow|check|job|matrix|GitHub Actions)\b/i.test(rawSummary)
|
||||
? "fix(ci):"
|
||||
: "fix:";
|
||||
const prefixed = /^fix(?:\(|:)/i.test(stem)
|
||||
? stem
|
||||
: `${prefix} ${stem || "address ClawSweeper finding"}`;
|
||||
return normalizeGithubPrTitle(
|
||||
prefixed,
|
||||
FALLBACK_REPAIR_PR_TITLE,
|
||||
CLAWSWEEPER_GENERATED_PR_TITLE_MAX_LENGTH,
|
||||
);
|
||||
}
|
||||
|
||||
function findingTitleStem(value: string) {
|
||||
const withoutFindingPrefix = value
|
||||
.replace(/^Found\s+(?:one|two|three|four|five|\d+|an?|the)?\s*/i, "")
|
||||
.replace(
|
||||
/^(?:(?:concrete|low-severity|medium-severity|high-severity|low-confidence|medium-confidence|high-confidence|critical|high|medium|low)\s+)*/i,
|
||||
"",
|
||||
)
|
||||
.trim();
|
||||
const colonIssue = withoutFindingPrefix.match(
|
||||
/^((?:[A-Za-z0-9_-]+\s+){0,4}?)(regressions?|bugs?|issues?|findings?|risks?)\s*:\s*(?:the\s+)?(.+?)(?:[.!?]\s|$)/i,
|
||||
);
|
||||
if (colonIssue) {
|
||||
return repairStem(colonIssue[3], `${colonIssue[1] ?? ""}${colonIssue[2] ?? ""}`);
|
||||
}
|
||||
|
||||
const scopedIssue = withoutFindingPrefix.match(
|
||||
/^((?:[A-Za-z0-9_-]+\s+){0,4}?)(regressions?|bugs?|issues?|findings?|risks?)\s+(?:in|with|around|for|from)\s+(?:the\s+)?(.+?)(?:[.!?:;]\s|$)/i,
|
||||
);
|
||||
if (scopedIssue) {
|
||||
return repairStem(scopedIssue[3], `${scopedIssue[1] ?? ""}${scopedIssue[2] ?? ""}`);
|
||||
}
|
||||
|
||||
const beforeColon = withoutFindingPrefix.match(/^([^:]{8,80}?)\s*:/)?.[1];
|
||||
return firstSentence(beforeColon || withoutFindingPrefix);
|
||||
}
|
||||
|
||||
function repairStem(surface: unknown, issue: unknown) {
|
||||
const subject = firstSentence(surface)
|
||||
.replace(/^(?:the\s+)?new\s+/i, "")
|
||||
.replace(/^(?:the|a|an)\s+/i, "")
|
||||
.trim();
|
||||
const suffix = normalizeTitleText(issue).toLowerCase();
|
||||
return [subject, suffix].filter(Boolean).join(" ");
|
||||
}
|
||||
|
||||
function firstSentence(value: unknown) {
|
||||
return normalizeTitleText(value)
|
||||
.split(/(?<=[.!?])\s+/)[0]
|
||||
.replace(/[.!?]+$/, "")
|
||||
.trim();
|
||||
}
|
||||
|
||||
function compactTitle(value: unknown, maxLength: number) {
|
||||
const text = normalizeTitleText(value).replace(/[.!?]+$/, "");
|
||||
if (text.length <= maxLength) return text;
|
||||
const suffix = "...";
|
||||
return `${text
|
||||
.slice(0, Math.max(0, maxLength - suffix.length))
|
||||
.replace(/[\s,;:./-]+$/, "")}${suffix}`;
|
||||
}
|
||||
|
||||
function normalizeTitleText(value: unknown) {
|
||||
return String(value ?? "")
|
||||
.replace(/[`*_]+/g, "")
|
||||
.replace(/\s+/g, " ")
|
||||
.trim();
|
||||
}
|
||||
62
test/repair/pr-title.test.mjs
Normal file
62
test/repair/pr-title.test.mjs
Normal file
@ -0,0 +1,62 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import {
|
||||
CLAWSWEEPER_GENERATED_PR_TITLE_MAX_LENGTH,
|
||||
GITHUB_PR_TITLE_MAX_LENGTH,
|
||||
commitFindingPrTitle,
|
||||
normalizeGithubPrTitle,
|
||||
} from "../../dist/repair/pr-title.js";
|
||||
import { validateFixArtifact } from "../../dist/repair/execute-fix-validation.js";
|
||||
|
||||
test("commit finding PR titles summarize scoped findings without leaking report prose", () => {
|
||||
const title = commitFindingPrTitle(
|
||||
"Found two concrete regressions in the shared helper extraction. The first failure drops docker state and the second breaks script cleanup.",
|
||||
);
|
||||
|
||||
assert.equal(title, "fix: shared helper extraction regressions");
|
||||
assert.equal(title.length <= CLAWSWEEPER_GENERATED_PR_TITLE_MAX_LENGTH, true);
|
||||
});
|
||||
|
||||
test("commit finding PR titles keep CI prefix and stay under the generated title cap", () => {
|
||||
const title = commitFindingPrTitle(
|
||||
"Found one low-severity formatting bug in the new loose-list paragraph for GitHub Actions output. The rest of the report explains why it matters.",
|
||||
);
|
||||
|
||||
assert.equal(title, "fix(ci): loose-list paragraph for GitHub Actions output formatting bug");
|
||||
assert.equal(title.length <= CLAWSWEEPER_GENERATED_PR_TITLE_MAX_LENGTH, true);
|
||||
});
|
||||
|
||||
test("commit finding PR titles retain known special-case titles", () => {
|
||||
assert.equal(
|
||||
commitFindingPrTitle("extension-shard matrix handling regressed"),
|
||||
"fix(ci): gate extension aggregate on shard matrix",
|
||||
);
|
||||
});
|
||||
|
||||
test("github PR title normalization applies the hard GitHub ceiling", () => {
|
||||
const title = normalizeGithubPrTitle(`fix: ${"a".repeat(400)}`);
|
||||
|
||||
assert.equal(title.length, GITHUB_PR_TITLE_MAX_LENGTH);
|
||||
assert.match(title, /\.\.\.$/);
|
||||
});
|
||||
|
||||
test("fix artifact validation rejects titles past the GitHub ceiling", () => {
|
||||
assert.throws(
|
||||
() =>
|
||||
validateFixArtifact({
|
||||
summary: "summary",
|
||||
pr_title: `fix: ${"a".repeat(GITHUB_PR_TITLE_MAX_LENGTH)}`,
|
||||
pr_body: "body",
|
||||
affected_surfaces: ["src"],
|
||||
likely_files: ["src/example.ts"],
|
||||
linked_refs: ["none"],
|
||||
validation_commands: ["pnpm check:changed"],
|
||||
credit_notes: ["ClawSweeper"],
|
||||
changelog_required: false,
|
||||
repair_strategy: "new_fix_pr",
|
||||
source_prs: [],
|
||||
}),
|
||||
/fix_artifact\.pr_title must be 256 characters or fewer/,
|
||||
);
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user