* surface agent failures in job summary (#632) when the agent harness returns `{success: false, error}`, main.ts went through the success path so the catch block — which renders the `### ❌ Pullfrog failed` banner via renderRunError — never fired. result: the GitHub Actions job summary showed only the partial body + usage table, no error block. the progress comment had a narrow workaround that re-implemented the api-key classifier inline. unify: in finalizeSuccessRun, call renderRunError once when `!success`, use `.summary` for the job summary (prepended to the existing body/usage parts) and `.comment` for the progress comment. removes the duplicated isApiKeyAuthError / formatApiKeyErrorSummary branch and picks up BillingError reclassification + hang body for free. * docs: note dual-surface failure rendering in finalizeSuccessRun * fix copilot review nit: clarify which renderRunError classifications carry the H3 banner
This commit is contained in:
committed by
pullfrog[bot]
parent
01e4daa0b5
commit
e2eb26573f
+29
-18
@@ -27,13 +27,12 @@ import type { AgentResult } from "../agents/shared.ts";
|
|||||||
import { deleteProgressComment } from "../mcp/comment.ts";
|
import { deleteProgressComment } from "../mcp/comment.ts";
|
||||||
import type { ToolContext } from "../mcp/server.ts";
|
import type { ToolContext } from "../mcp/server.ts";
|
||||||
import type { ToolState } from "../toolState.ts";
|
import type { ToolState } from "../toolState.ts";
|
||||||
import { formatApiKeyErrorSummary, isApiKeyAuthError } from "./apiKeys.ts";
|
|
||||||
import { formatUsageSummary, log, writeSummary } from "./cli.ts";
|
import { formatUsageSummary, log, writeSummary } from "./cli.ts";
|
||||||
import { reportErrorToComment } from "./errorReport.ts";
|
import { reportErrorToComment } from "./errorReport.ts";
|
||||||
import { persistLearnings } from "./learnings.ts";
|
import { persistLearnings } from "./learnings.ts";
|
||||||
import { persistSummary } from "./prSummary.ts";
|
import { persistSummary } from "./prSummary.ts";
|
||||||
import { postReviewCleanup } from "./reviewCleanup.ts";
|
import { postReviewCleanup } from "./reviewCleanup.ts";
|
||||||
import type { RenderedRunError } from "./runErrorRenderer.ts";
|
import { type RenderedRunError, renderRunError } from "./runErrorRenderer.ts";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Best-effort cleanup shared by both run-end paths:
|
* Best-effort cleanup shared by both run-end paths:
|
||||||
@@ -57,9 +56,12 @@ export async function persistRunArtifacts(toolContext: ToolContext): Promise<voi
|
|||||||
*
|
*
|
||||||
* 1. shared best-effort cleanup via `persistRunArtifacts`
|
* 1. shared best-effort cleanup via `persistRunArtifacts`
|
||||||
* 2. when the harness returned `success=false` (e.g. unsubmitted-review
|
* 2. when the harness returned `success=false` (e.g. unsubmitted-review
|
||||||
* gate exhausted retries, stop-hook persistently failing), surface
|
* gate exhausted retries, stop-hook persistently failing), render via
|
||||||
* the error in the progress comment so the user sees it instead of a
|
* `renderRunError` and surface the error in BOTH the progress comment
|
||||||
* deleted-comment void
|
* (rendered.comment) and the Actions job summary (rendered.summary,
|
||||||
|
* prepended below in step 4) — same classifier as the catch path so
|
||||||
|
* the user sees it instead of a deleted-comment void / empty summary
|
||||||
|
* tab
|
||||||
* 3. when the run succeeded and the progress comment was never finalized
|
* 3. when the run succeeded and the progress comment was never finalized
|
||||||
* via `report_progress`, delete it (three sub-cases — orphan
|
* via `report_progress`, delete it (three sub-cases — orphan
|
||||||
* "Leaping into action" comment, abandoned checklist, agent wrote
|
* "Leaping into action" comment, abandoned checklist, agent wrote
|
||||||
@@ -78,18 +80,27 @@ export async function finalizeSuccessRun(input: {
|
|||||||
}): Promise<void> {
|
}): Promise<void> {
|
||||||
await persistRunArtifacts(input.toolContext);
|
await persistRunArtifacts(input.toolContext);
|
||||||
|
|
||||||
if (!input.result.success && input.toolState.progressComment) {
|
// shared rendering for the !success branch — same classifier as the
|
||||||
const rawError = input.result.error || "agent run failed";
|
// outer catch path (BillingError reclassify → hang → api-key → generic),
|
||||||
const errorBody = isApiKeyAuthError(rawError)
|
// so a harness-returned `{success: false}` lands an actionable error
|
||||||
? formatApiKeyErrorSummary({
|
// block in the job summary alongside the matching body in the progress
|
||||||
owner: input.repo.owner,
|
// comment. hang and generic get the `### ❌ Pullfrog failed` H3 banner;
|
||||||
name: input.repo.name,
|
// BillingError and api-key render their own provider-specific framing
|
||||||
raw: rawError,
|
// (no banner). renders once; reused for both surfaces below.
|
||||||
})
|
const rendered = !input.result.success
|
||||||
: rawError;
|
? renderRunError({
|
||||||
await reportErrorToComment({ toolState: input.toolState, error: errorBody }).catch((error) => {
|
errorMessage: input.result.error || "agent run failed",
|
||||||
log.debug(`failure error report failed: ${error}`);
|
repo: input.repo,
|
||||||
});
|
agentDiagnostic: input.toolState.agentDiagnostic,
|
||||||
|
})
|
||||||
|
: null;
|
||||||
|
|
||||||
|
if (rendered && input.toolState.progressComment) {
|
||||||
|
await reportErrorToComment({ toolState: input.toolState, error: rendered.comment }).catch(
|
||||||
|
(error) => {
|
||||||
|
log.debug(`failure error report failed: ${error}`);
|
||||||
|
}
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// create_pull_request_review owns its own deletion (see mcp/review.ts), so
|
// create_pull_request_review owns its own deletion (see mcp/review.ts), so
|
||||||
@@ -110,7 +121,7 @@ export async function finalizeSuccessRun(input: {
|
|||||||
try {
|
try {
|
||||||
const usageSummary = formatUsageSummary(input.toolState.usageEntries);
|
const usageSummary = formatUsageSummary(input.toolState.usageEntries);
|
||||||
const body = input.toolState.lastProgressBody || input.result.output;
|
const body = input.toolState.lastProgressBody || input.result.output;
|
||||||
const parts = [body, usageSummary].filter(Boolean);
|
const parts = [rendered?.summary, body, usageSummary].filter(Boolean);
|
||||||
if (parts.length > 0) {
|
if (parts.length > 0) {
|
||||||
await writeSummary(parts.join("\n\n"));
|
await writeSummary(parts.join("\n\n"));
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user