diff --git a/agents/opencode.ts b/agents/opencode.ts index b497e0c..0be9fbd 100644 --- a/agents/opencode.ts +++ b/agents/opencode.ts @@ -17,7 +17,9 @@ import { join } from "node:path"; import { performance } from "node:perf_hooks"; import { pullfrogMcpName } from "../external.ts"; import { BEDROCK_MODEL_ID_ENV, modelAliases } from "../models.ts"; +import type { ToolState } from "../toolState.ts"; import { getIdleMs, markActivity } from "../utils/activity.ts"; +import { type AgentDiagnostic, formatAgentHangBody } from "../utils/agentHangReport.ts"; import { formatJsonValue, log } from "../utils/cli.ts"; import { installFromNpmTarball } from "../utils/install.ts"; import { findProviderErrorMatch } from "../utils/providerErrors.ts"; @@ -312,8 +314,11 @@ interface OpenCodeStepFinishEvent { * `session/message-v2.ts`). error parts carry the reason on `error`, * completed parts on `output` — reading the wrong field is what caused * the silent `(no error message)` log in #662. + * + * Named `ToolPartState` locally (not `ToolState`) so it doesn't shadow the + * action-wide `ToolState` imported above. */ -type ToolState = +type ToolPartState = | { status: "pending" | "running"; input?: unknown } | { status: "completed"; input?: unknown; output: string } | { status: "error"; input?: unknown; error: string }; @@ -326,7 +331,7 @@ interface OpenCodeToolUseEvent { id?: string; callID?: string; tool?: string; - state?: ToolState; + state?: ToolPartState; }; [key: string]: unknown; } @@ -335,7 +340,7 @@ interface OpenCodeToolResultEvent { type: "tool_result"; timestamp?: number; sessionID?: string; - part?: { callID?: string; state?: ToolState }; + part?: { callID?: string; state?: ToolPartState }; tool_id?: string; status?: "success" | "error"; output?: string; @@ -420,6 +425,7 @@ type RunParams = { args: string[]; cwd: string; env: Record; + toolState: ToolState; todoTracker?: TodoTracker | undefined; onActivityTimeout?: (() => void) | undefined; onToolUse?: ((event: { toolName: string; input: unknown }) => void) | undefined; @@ -936,6 +942,19 @@ async function runOpenCode(params: RunParams): Promise { let lastProviderError: string | null = null; let agentErrorEvent: OpenCodeErrorEvent | null = null; + // shared with main.ts via toolState. updated in place as events stream and + // stderr accumulates so the outer activity-timeout catch sees the same + // context the harness's own catch path uses to format `result.error`. + // recentStderr is shared by reference; the scalar fields are mirrored on + // each update below. + const diagnostic: AgentDiagnostic = { + label: params.label, + recentStderr, + lastProviderError: undefined, + eventCount: 0, + }; + params.toolState.agentDiagnostic = diagnostic; + // capped accumulator for the agent's narration. used as a post-run fallback // when `finalOutput` (the orchestrator's final assistant message) is empty. // unbounded `output += text` previously grew to ~1 GiB on multi-lens Reviews @@ -994,6 +1013,7 @@ async function runOpenCode(params: RunParams): Promise { } eventCount++; + diagnostic.eventCount = eventCount; log.debug(JSON.stringify(event, null, 2)); const timeSinceLastActivity = getIdleMs(); @@ -1035,6 +1055,7 @@ async function runOpenCode(params: RunParams): Promise { const match = findProviderErrorMatch(trimmed); if (match) { lastProviderError = match.label; + diagnostic.lastProviderError = match.label; log.info(`» provider error detected (${match.label}): ${match.excerpt}`); } else { log.debug(trimmed); @@ -1166,10 +1187,11 @@ async function runOpenCode(params: RunParams): Promise { `» recent stderr (last ${Math.min(recentStderr.length, 10)} lines):\n${stderrContext}` ); + const body = formatAgentHangBody({ diagnostic, isHang: isActivityTimeout, errorMessage }); return { success: false, output: finalOutput || output.toString(), - error: `${errorMessage} [${diagnosis}]`, + error: body ?? `${errorMessage} [${diagnosis}]`, usage: buildUsage(), }; } @@ -1262,6 +1284,7 @@ export const opencode = agent({ cliPath, cwd: repoDir, env, + toolState: ctx.toolState, todoTracker: ctx.todoTracker, onActivityTimeout: ctx.onActivityTimeout, onToolUse: ctx.onToolUse, diff --git a/main.ts b/main.ts index 9609e16..f57bc9c 100644 --- a/main.ts +++ b/main.ts @@ -16,6 +16,7 @@ import { DEFAULT_ACTIVITY_TIMEOUT_MS, } from "./utils/activity.ts"; import { resolveAgent, resolveModel } from "./utils/agent.ts"; +import { formatAgentHangBody } from "./utils/agentHangReport.ts"; import { apiFetch } from "./utils/apiFetch.ts"; import { formatApiKeyErrorSummary, @@ -1123,12 +1124,32 @@ export async function main(): Promise { ? new BillingError(errorMessage, { code: "router_keylimit_exhausted" }) : null; + // when the activity-timeout watchdog wins the race against the agent + // harness's own catch, the bare timer reject reason ("activity timeout: + // no output for 302s") tells the user nothing actionable. the harness + // keeps a structured diagnostic on toolState as it runs — recent stderr, + // last provider-error label, event count — and `formatAgentHangBody` + // renders that as a markdown body suitable for both the job summary tab + // and the PR progress comment. + // + // gated on isHang because the harness sets `agentDiagnostic` on entry, + // so any non-hang throw that hits the outer catch (e.g. the post-success + // output_schema validator, or a late cleanup throw after the run already + // succeeded) would otherwise render "Pullfrog failed" with stale event + // counts and silently drop the real `errorMessage`. + const isHang = + errorMessage.startsWith("activity timeout") || errorMessage.startsWith("agent still pending"); + const hangBody = isHang + ? formatAgentHangBody({ diagnostic: toolState.agentDiagnostic, isHang: true, errorMessage }) + : null; + + const apiKeySource = hangBody ?? errorMessage; const apiKeyErrorSummary = - !billingError && isApiKeyAuthError(errorMessage) + !billingError && isApiKeyAuthError(apiKeySource) ? formatApiKeyErrorSummary({ owner: runContext.repo.owner, name: runContext.repo.name, - raw: errorMessage, + raw: apiKeySource, }) : null; @@ -1136,7 +1157,10 @@ export async function main(): Promise { try { const errorSummary = billingError ? formatBillingErrorSummary(billingError, runContext.repo.owner) - : (apiKeyErrorSummary ?? `### ❌ Pullfrog failed\n\n\`\`\`\n${errorMessage}\n\`\`\``); + : (apiKeyErrorSummary ?? + (hangBody + ? `### ❌ Pullfrog failed\n\n${hangBody}` + : `### ❌ Pullfrog failed\n\n\`\`\`\n${errorMessage}\n\`\`\``)); const usageSummary = formatUsageSummary(toolState.usageEntries); const parts = [errorSummary, toolState.lastProgressBody, usageSummary].filter(Boolean); await writeSummary(parts.join("\n\n")); @@ -1145,7 +1169,7 @@ export async function main(): Promise { try { const commentBody = billingError ? formatBillingErrorSummary(billingError, runContext.repo.owner) - : (apiKeyErrorSummary ?? errorMessage); + : (apiKeyErrorSummary ?? hangBody ?? errorMessage); await reportErrorToComment({ toolState, error: commentBody }); } catch { // error reporting failed, but don't let it mask the original error diff --git a/toolState.ts b/toolState.ts index 083c6cb..134885a 100644 --- a/toolState.ts +++ b/toolState.ts @@ -1,5 +1,6 @@ import type { AgentUsage } from "./agents/shared.ts"; import type { PrepResult } from "./prep/types.ts"; +import type { AgentDiagnostic } from "./utils/agentHangReport.ts"; import { log } from "./utils/cli.ts"; import type { DiffCoverageState } from "./utils/diffCoverage.ts"; import { @@ -157,6 +158,12 @@ export interface ToolState { model?: string | undefined; todoTracker?: TodoTracker | undefined; diffCoverage?: DiffCoverageState | undefined; + // mutable handle the agent harness writes to as a run progresses (recent + // stderr ring buffer reference, last provider-error label, event count). + // read by main.ts's outer catch so a watchdog-fired activity timeout still + // surfaces the same agent-side context the harness's own catch path returns + // via `result.error`. see `utils/agentHangReport.ts`. + agentDiagnostic?: AgentDiagnostic | undefined; } interface InitToolStateParams { diff --git a/utils/agentHangReport.ts b/utils/agentHangReport.ts new file mode 100644 index 0000000..e744417 --- /dev/null +++ b/utils/agentHangReport.ts @@ -0,0 +1,115 @@ +const MAX_STDERR_BYTES = 3000; + +/** + * mutable per-run handle the agent harness writes to as a run progresses. + * the action's outer try/catch in `main.ts` reads this off `toolState` when + * the activity-timeout watchdog wins the race against the harness's own + * catch — the bare timer reject reason ("activity timeout: no output for + * 302s") tells the user nothing actionable, but `recentStderr` + + * `lastProviderError` together usually point straight at the upstream cause. + * + * `recentStderr` is shared by reference with the harness's bounded ring + * buffer, so the diagnostic always reflects the latest captured tail. + */ +export type AgentDiagnostic = { + /** display label for the agent, e.g. "Pullfrog". used in the headline. */ + label: string; + /** shared reference to the harness's bounded stderr ring buffer. */ + recentStderr: string[]; + /** most-recent provider-error label from `detectProviderError`, if any. */ + lastProviderError: string | undefined; + /** count of stdout events successfully parsed before the failure. */ + eventCount: number; +}; + +/** + * Build a user-facing markdown body for an agent hang or failure. + * + * Rendered into both the PR progress comment and the GitHub Actions job + * summary. Returns `null` when no diagnostic is available, which signals to + * the caller to fall back to its bare-error rendering. + * + * `errorMessage` is the underlying timer / spawn reject string (e.g. + * `activity timeout: no output for 301s`). The idle seconds are parsed out + * of it for the hang explanation — total runtime would overstate the stall + * for runs that streamed for a long time before going quiet. + */ +export function formatAgentHangBody(input: { + diagnostic: AgentDiagnostic | undefined; + isHang: boolean; + errorMessage: string; +}): string | null { + if (!input.diagnostic) return null; + + const verb = input.isHang ? "stalled" : "failed"; + const cause = input.diagnostic.lastProviderError + ? ` — likely cause: \`${input.diagnostic.lastProviderError}\`` + : ""; + const headline = `**${input.diagnostic.label} ${verb}**${cause}`; + + const explanation = formatExplanation({ + isHang: input.isHang, + errorMessage: input.errorMessage, + }); + const parts = [headline, "", `${explanation} ${formatEventsPart(input.diagnostic)}`]; + + const tail = renderStderrTail(input.diagnostic.recentStderr); + if (tail) { + // pick a fence longer than any backtick run in the body so a stderr line + // containing ``` (provider error JSON occasionally embeds it) can't + // terminate the fence early and corrupt the rest of the markdown. + const fence = pickFence(tail); + parts.push( + "", + "
Recent agent stderr", + "", + fence, + tail, + fence, + "", + "
" + ); + } + + return parts.join("\n"); +} + +function formatExplanation(input: { isHang: boolean; errorMessage: string }): string { + if (!input.isHang) return `The agent exited unexpectedly: ${input.errorMessage}`; + const idleSec = parseIdleSec(input.errorMessage); + if (idleSec === undefined) { + return "The agent stopped emitting events and was killed by the activity-timeout watchdog."; + } + return `The agent stopped emitting events for ${idleSec}s and was killed by the activity-timeout watchdog.`; +} + +function parseIdleSec(message: string): number | undefined { + const match = /no output for (\d+)s/.exec(message); + return match ? Number(match[1]) : undefined; +} + +function formatEventsPart(diagnostic: AgentDiagnostic): string { + if (diagnostic.eventCount > 0) { + return `${diagnostic.eventCount} events were processed before the failure.`; + } + // when the provider-error label already names the cause in the headline, + // the reachability nudge below contradicts it (e.g. an immediate 401 also + // produces zero events but isn't a reachability problem). suppress it. + if (diagnostic.lastProviderError) return "No events were emitted before the failure."; + return "No events were emitted — check whether the model provider is reachable."; +} + +function renderStderrTail(lines: readonly string[]): string { + if (lines.length === 0) return ""; + const joined = lines.join("\n"); + if (joined.length <= MAX_STDERR_BYTES) return joined; + return `... (older lines truncated)\n${joined.slice(-MAX_STDERR_BYTES)}`; +} + +function pickFence(content: string): string { + let max = 0; + for (const match of content.matchAll(/`+/g)) { + if (match[0].length > max) max = match[0].length; + } + return "`".repeat(Math.max(3, max + 1)); +}