From 5390f3a740a9c8fe22db079b2f3fb6c36e21c7d3 Mon Sep 17 00:00:00 2001 From: i-trytoohard <193449657+i-trytoohard@users.noreply.github.com> Date: Tue, 12 May 2026 17:35:39 +0530 Subject: [PATCH] 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. --- .../src/__tests__/lifecycle-manager.test.ts | 6 ++-- packages/core/src/lifecycle-manager.ts | 12 +++++-- packages/core/src/types.ts | 2 +- packages/plugins/scm-github/src/index.ts | 15 ++++++--- .../plugins/scm-github/test/index.test.ts | 33 +++++++++++-------- 5 files changed, 45 insertions(+), 23 deletions(-) diff --git a/packages/core/src/__tests__/lifecycle-manager.test.ts b/packages/core/src/__tests__/lifecycle-manager.test.ts index 3ae2a06aa..355cf0fbf 100644 --- a/packages/core/src/__tests__/lifecycle-manager.test.ts +++ b/packages/core/src/__tests__/lifecycle-manager.test.ts @@ -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 () => { diff --git a/packages/core/src/lifecycle-manager.ts b/packages/core/src/lifecycle-manager.ts index 25823d5bd..80752376a 100644 --- a/packages/core/src/lifecycle-manager.ts +++ b/packages/core/src/lifecycle-manager.ts @@ -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 { if (scm.getCIFailureSummary) { try { - const summary = await scm.getCIFailureSummary(pr); + const summary = await scm.getCIFailureSummary(pr, failedChecks); if (summary?.failedJobs.length) { return formatCIFailureSummaryMessage(summary); } diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index a9c070ecb..d759c7769 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -829,7 +829,7 @@ export interface SCM { getCIChecks(pr: PRInfo): Promise; /** Get failed CI jobs/steps with a bounded failed-log tail, if supported. */ - getCIFailureSummary?(pr: PRInfo): Promise; + getCIFailureSummary?(pr: PRInfo, failedChecks?: CICheck[]): Promise; /** Get overall CI summary */ getCISummary(pr: PRInfo): Promise; diff --git a/packages/plugins/scm-github/src/index.ts b/packages/plugins/scm-github/src/index.ts index 7d6d2c931..78ebd4e9e 100644 --- a/packages/plugins/scm-github/src/index.ts +++ b/packages/plugins/scm-github/src/index.ts @@ -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 { @@ -864,10 +865,14 @@ function createGitHubSCM(): SCM { }); }, - async getCIFailureSummary(pr: PRInfo): Promise { + async getCIFailureSummary( + pr: PRInfo, + providedFailedChecks?: CICheck[], + ): Promise { 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"] = []; diff --git a/packages/plugins/scm-github/test/index.test.ts b/packages/plugins/scm-github/test/index.test.ts index e6b4934b9..c9e90bcd0 100644 --- a/packages/plugins/scm-github/test/index.test.ts +++ b/packages/plugins/scm-github/test/index.test.ts @@ -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(); }); });