fix(lifecycle): harden CI failure summaries
Address review feedback: escape log-tail lines that could close markdown fences, report the last failed-step column from gh failed logs, and pass already-known failed checks into getCIFailureSummary to avoid duplicate CI check fetches. State invariants preserved: lifecycle transition decisions, retry/escalation budgets, persistent ci-failed tracker behavior, and stable CI passing reset semantics remain unchanged.
This commit is contained in:
parent
c854183090
commit
5390f3a740
|
|
@ -2392,7 +2392,7 @@ describe("reactions", () => {
|
|||
failedStep: "Run pnpm test",
|
||||
runUrl: "https://github.com/org/repo/actions/runs/123/job/456",
|
||||
logTail:
|
||||
"AssertionError: expected true to be false\nProcess completed with exit code 1",
|
||||
"AssertionError: expected true to be false\n```\nProcess completed with exit code 1",
|
||||
},
|
||||
],
|
||||
}),
|
||||
|
|
@ -2417,9 +2417,11 @@ describe("reactions", () => {
|
|||
expect(sentMessage).toContain("CI is failing on your PR.");
|
||||
expect(sentMessage).toContain("Failed: build → Run pnpm test");
|
||||
expect(sentMessage).toContain("Run: https://github.com/org/repo/actions/runs/123/job/456");
|
||||
expect(sentMessage).toContain("Log tail (last 2 lines):");
|
||||
expect(sentMessage).toContain("Log tail (last 3 lines):");
|
||||
expect(sentMessage).toContain("AssertionError: expected true to be false");
|
||||
expect(sentMessage).toContain("\u200B```");
|
||||
expect(sentMessage).toContain("Fix the issues and push again.");
|
||||
expect(mockSCM.getCIFailureSummary).toHaveBeenCalledWith(makePR(), ciChecks);
|
||||
});
|
||||
|
||||
it("falls back to check names and URLs when SCM lacks getCIFailureSummary", async () => {
|
||||
|
|
|
|||
|
|
@ -1942,7 +1942,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
if (job.logTail) {
|
||||
const lineCount = job.logTail.split(/\r?\n/).length;
|
||||
const lineLabel = lineCount === 1 ? "line" : "lines";
|
||||
lines.push("", `Log tail (last ${lineCount} ${lineLabel}):`, "```", job.logTail, "```");
|
||||
const escapedTail = escapeMarkdownCodeFenceClosers(job.logTail);
|
||||
lines.push("", `Log tail (last ${lineCount} ${lineLabel}):`, "```", escapedTail, "```");
|
||||
}
|
||||
|
||||
lines.push("");
|
||||
|
|
@ -1952,6 +1953,13 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
return lines.join("\n");
|
||||
}
|
||||
|
||||
function escapeMarkdownCodeFenceClosers(logTail: string): string {
|
||||
return logTail
|
||||
.split(/\r?\n/)
|
||||
.map((line) => (line.startsWith("```") ? `\u200B${line}` : line))
|
||||
.join("\n");
|
||||
}
|
||||
|
||||
function formatCIFailureChecksFallback(failedChecks: CICheck[]): string {
|
||||
const lines = ["CI checks are failing on your PR. Here are the failed checks:", ""];
|
||||
for (const check of failedChecks) {
|
||||
|
|
@ -1975,7 +1983,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
): Promise<string> {
|
||||
if (scm.getCIFailureSummary) {
|
||||
try {
|
||||
const summary = await scm.getCIFailureSummary(pr);
|
||||
const summary = await scm.getCIFailureSummary(pr, failedChecks);
|
||||
if (summary?.failedJobs.length) {
|
||||
return formatCIFailureSummaryMessage(summary);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -829,7 +829,7 @@ export interface SCM {
|
|||
getCIChecks(pr: PRInfo): Promise<CICheck[]>;
|
||||
|
||||
/** Get failed CI jobs/steps with a bounded failed-log tail, if supported. */
|
||||
getCIFailureSummary?(pr: PRInfo): Promise<CIFailureSummary | null>;
|
||||
getCIFailureSummary?(pr: PRInfo, failedChecks?: CICheck[]): Promise<CIFailureSummary | null>;
|
||||
|
||||
/** Get overall CI summary */
|
||||
getCISummary(pr: PRInfo): Promise<CIStatus>;
|
||||
|
|
|
|||
|
|
@ -212,12 +212,13 @@ function tailLines(text: string, maxLines: number): string | undefined {
|
|||
}
|
||||
|
||||
function extractFailedStep(log: string): string | undefined {
|
||||
let lastStep: string | undefined;
|
||||
for (const line of log.split(/\r?\n/)) {
|
||||
const parts = line.split("\t");
|
||||
const step = parts.length >= 3 ? parts[1]?.trim() : undefined;
|
||||
if (step) return step;
|
||||
if (step) lastStep = step;
|
||||
}
|
||||
return undefined;
|
||||
return lastStep;
|
||||
}
|
||||
|
||||
async function getCIChecksFromStatusRollup(pr: PRInfo): Promise<CICheck[]> {
|
||||
|
|
@ -864,10 +865,14 @@ function createGitHubSCM(): SCM {
|
|||
});
|
||||
},
|
||||
|
||||
async getCIFailureSummary(pr: PRInfo): Promise<CIFailureSummary | null> {
|
||||
async getCIFailureSummary(
|
||||
pr: PRInfo,
|
||||
providedFailedChecks?: CICheck[],
|
||||
): Promise<CIFailureSummary | null> {
|
||||
try {
|
||||
const checks = await this.getCIChecks(pr);
|
||||
const failedChecks = checks.filter(isFailedCheck);
|
||||
const failedChecks = (providedFailedChecks ?? (await this.getCIChecks(pr))).filter(
|
||||
isFailedCheck,
|
||||
);
|
||||
if (failedChecks.length === 0) return null;
|
||||
|
||||
const failedJobs: CIFailureSummary["failedJobs"] = [];
|
||||
|
|
|
|||
|
|
@ -651,25 +651,30 @@ describe("scm-github plugin", () => {
|
|||
|
||||
describe("getCIFailureSummary", () => {
|
||||
it("returns failed job step and caps the failed log tail", async () => {
|
||||
mockGh([
|
||||
const checks = [
|
||||
{
|
||||
name: "build",
|
||||
state: "FAILURE",
|
||||
link: "https://github.com/acme/repo/actions/runs/123/job/456",
|
||||
status: "failed" as const,
|
||||
conclusion: "FAILURE",
|
||||
url: "https://github.com/acme/repo/actions/runs/123/job/456",
|
||||
},
|
||||
{
|
||||
name: "lint",
|
||||
state: "SUCCESS",
|
||||
link: "https://github.com/acme/repo/actions/runs/124/job/457",
|
||||
status: "passed" as const,
|
||||
conclusion: "SUCCESS",
|
||||
url: "https://github.com/acme/repo/actions/runs/124/job/457",
|
||||
},
|
||||
]);
|
||||
];
|
||||
const logLines = Array.from(
|
||||
{ length: 125 },
|
||||
(_, index) => `build\tRun pnpm test\t2026-05-12T00:00:00Z line ${index + 1}`,
|
||||
(_, index) => {
|
||||
const step = index < 100 ? "Install dependencies" : "Run pnpm test";
|
||||
return `build\t${step}\t2026-05-12T00:00:00Z line ${index + 1}`;
|
||||
},
|
||||
);
|
||||
mockGhRaw(logLines.join("\n"));
|
||||
|
||||
const summary = await scm.getCIFailureSummary?.(pr);
|
||||
const summary = await scm.getCIFailureSummary?.(pr, checks);
|
||||
|
||||
expect(summary?.failedJobs).toHaveLength(1);
|
||||
expect(summary?.failedJobs[0]).toEqual({
|
||||
|
|
@ -686,19 +691,21 @@ describe("scm-github plugin", () => {
|
|||
["run", "view", "123", "--repo", "acme/repo", "--log-failed", "--job", "456"],
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(ghMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("returns null when failed-log fetch fails", async () => {
|
||||
mockGh([
|
||||
const checks = [
|
||||
{
|
||||
name: "build",
|
||||
state: "FAILURE",
|
||||
link: "https://github.com/acme/repo/actions/runs/123/job/456",
|
||||
status: "failed" as const,
|
||||
conclusion: "FAILURE",
|
||||
url: "https://github.com/acme/repo/actions/runs/123/job/456",
|
||||
},
|
||||
]);
|
||||
];
|
||||
mockGhError("run view failed");
|
||||
|
||||
await expect(scm.getCIFailureSummary?.(pr)).resolves.toBeNull();
|
||||
await expect(scm.getCIFailureSummary?.(pr, checks)).resolves.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue