From 0a64659ee7a679769d1baa54cece25e5e2b9d759 Mon Sep 17 00:00:00 2001 From: Colin McDonnell Date: Sat, 16 May 2026 05:09:52 +0000 Subject: [PATCH] refactor: slim action/main.ts to an orchestrator + extract helpers (#755) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: extract helpers out of action/main.ts so non-orchestration churn stops touching the file main.ts had grown to ~1240 lines holding ~500 lines of helpers that have nothing to do with the resolver pipeline — billing-error UI/copy, proxy minting, summary/learnings persistence, log formatters, end-of-run cleanup waterfalls. any PR adding a new billing code branch or a new log line was forced to edit main.ts, and since main.ts is in ALWAYS_RUN_ALL the entire 52-job LLM CI matrix fired on what should have been a 0-job change (e.g. #748). extractions: - action/utils/billingErrors.ts — BillingError, TransientError, the format*Summary renderers, billingConsoleUrl - action/utils/proxy.ts — mintProxyKey, buildProxyTokenHeaders, resolveProxyModel, plus runProxyResolution wrapper that renders + rethrows BillingError/TransientError before the outer catch - action/utils/prSummary.ts — fetchPreviousSnapshot, persistSummary co-located with the existing seed/read file helpers - action/utils/learnings.ts — persistLearnings co-located with the existing seed/read file helpers - action/utils/runStartupLog.ts — resolveOutputSchema + logRunStartup (the model/agent/push/shell/timeout block) - action/utils/runErrorRenderer.ts — renderRunError classifies (BillingError reclassify / hang detect / API-key auth) and emits {summary, comment} markdown bodies - action/utils/runLifecycle.ts — persistRunArtifacts, finalizeSuccessRun, writeRunErrorOutputs — the three end-of-run cleanup phases shared between the success path and the error catch path main.ts is now ~570 lines — the irreducible orchestrator: disposables (`await using` for tokenRef / gitAuthServer / mcpHttpServer), the toolContext construction, the agent-timeout race, the catch/finally shape, and the named phase calls. behavior is preserved verbatim (verified: pnpm -r typecheck + pnpm test 695/695 pass, action/test 596/596 pass). wiki/main.md gets a new "file layout" section describing the split. AGENTS.md gets a single line pointing future edits at the helpers instead of main.ts. * anneal: address review findings - restore MainResult.result?: string (accidental removal in initial commit; field was unused in current code but is part of the exported interface surface — keep the diff truly behavior-preserving) - move resolveOutputSchema from runStartupLog.ts to payload.ts (it's an action-input resolver alongside resolvePromptInput / resolvePayload, not a log helper — was placed in runStartupLog.ts for matrix-churn pragmatism but the domain fit is in payload.ts) - un-export resolveProxyModel (only used internally by runProxyResolution in proxy.ts; no external importer) - fix runErrorRenderer.ts JSDoc "Three classifications" → four (Billing, hang, API-key, default) - expand runLifecycle.ts module banner to note that finalizeSuccessRun calls persistRunArtifacts first, and to explain why the catch path splits writeRunErrorOutputs + persistRunArtifacts - update billingErrors.ts header to point at proxy.ts and runErrorRenderer.ts as the actual origin sites (was stale "main.ts") - expand proxy.ts header to spell out the runProxyResolution entrypoint contract (was stale "main.ts can render") - update wiki/main.md resolver chain + dependency table to name runProxyResolution as the actual call site and document the early BillingError/TransientError rendering branch - update wiki/main.md file-layout table to lead with runProxyResolution and describe mintProxyKey/buildProxyTokenHeaders/resolveProxyModel as internal helpers (was implying they were public surface) --- main.ts | 762 +++----------------------------------- utils/billingErrors.ts | 179 +++++++++ utils/learnings.ts | 63 ++++ utils/payload.ts | 24 ++ utils/prSummary.ts | 69 ++++ utils/proxy.ts | 221 +++++++++++ utils/runErrorRenderer.ts | 97 +++++ utils/runLifecycle.ts | 151 ++++++++ utils/runStartupLog.ts | 61 +++ 9 files changed, 915 insertions(+), 712 deletions(-) create mode 100644 utils/billingErrors.ts create mode 100644 utils/proxy.ts create mode 100644 utils/runErrorRenderer.ts create mode 100644 utils/runLifecycle.ts create mode 100644 utils/runStartupLog.ts diff --git a/main.ts b/main.ts index 9ea308c..62f9930 100644 --- a/main.ts +++ b/main.ts @@ -3,12 +3,11 @@ import { existsSync, readdirSync } from "node:fs"; import { readFile } from "node:fs/promises"; import { join } from "node:path"; -import * as core from "@actions/core"; -import { deleteProgressComment, reportProgress } from "./mcp/comment.ts"; +import { reportProgress } from "./mcp/comment.ts"; import { startInstallation } from "./mcp/dependencies.ts"; import { startMcpHttpServer, type ToolContext } from "./mcp/server.ts"; import { computeModes } from "./modes.ts"; -import { initToolState, type ToolState } from "./toolState.ts"; +import { initToolState } from "./toolState.ts"; import { type ActivityTimeout, createProcessOutputActivityTimeout, @@ -16,35 +15,32 @@ 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, - isApiKeyAuthError, - validateAgentApiKey, -} from "./utils/apiKeys.ts"; -import { isLocalApiUrl } from "./utils/apiUrl.ts"; +import { validateAgentApiKey } from "./utils/apiKeys.ts"; import { resolveBody } from "./utils/body.ts"; -import { formatUsageSummary, log, writeSummary } from "./utils/cli.ts"; +import { log } from "./utils/cli.ts"; import { recordDiffReadFromToolUse } from "./utils/diffCoverage.ts"; -import { reportErrorToComment } from "./utils/errorReport.ts"; import { onExitSignal } from "./utils/exitHandler.ts"; import { resolveGit, setGitAuthServer } from "./utils/gitAuth.ts"; import { startGitAuthServer } from "./utils/gitAuthServer.ts"; import { createOctokit, writeGitHubUsageSummaryToFile } from "./utils/github.ts"; import { resolveInstructions } from "./utils/instructions.ts"; -import { readLearningsFile, seedLearningsFile } from "./utils/learnings.ts"; +import { persistLearnings, seedLearningsFile } from "./utils/learnings.ts"; import { executeLifecycleHook } from "./utils/lifecycle.ts"; import { normalizeEnv, sanitizeSecret } from "./utils/normalizeEnv.ts"; import { applyOverrides } from "./utils/overrides.ts"; import { aggregateUsage, patchWorkflowRunFields } from "./utils/patchWorkflowRunFields.ts"; -import { resolvePayload, resolvePromptInput } from "./utils/payload.ts"; -import { isRouterKeylimitExhaustedError } from "./utils/providerErrors.ts"; -import { readSummaryFile, seedSummaryFile } from "./utils/prSummary.ts"; -import { postReviewCleanup } from "./utils/reviewCleanup.ts"; +import { resolveOutputSchema, resolvePayload, resolvePromptInput } from "./utils/payload.ts"; +import { type OidcCredentials, runProxyResolution } from "./utils/proxy.ts"; +import { fetchPreviousSnapshot, persistSummary, seedSummaryFile } from "./utils/prSummary.ts"; import { handleAgentResult } from "./utils/run.ts"; -import { type AccountPlan, isInfraCovered } from "./utils/runContext.ts"; import { resolveRunContextData } from "./utils/runContextData.ts"; +import { renderRunError } from "./utils/runErrorRenderer.ts"; +import { + finalizeSuccessRun, + persistRunArtifacts, + writeRunErrorOutputs, +} from "./utils/runLifecycle.ts"; +import { logRunStartup } from "./utils/runStartupLog.ts"; import { setEnvAllowlist } from "./utils/secrets.ts"; import { createTempDirectory, setupGit } from "./utils/setup.ts"; import { killTrackedChildren } from "./utils/subprocess.ts"; @@ -63,477 +59,6 @@ export interface MainResult { result?: string | undefined; } -function resolveOutputSchema(): Record | undefined { - const raw = core.getInput("output_schema"); - if (!raw) return undefined; - - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch { - throw new Error(`invalid output_schema: not valid JSON`); - } - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { - throw new Error(`invalid output_schema: must be a JSON object`); - } - log.info("» structured output schema provided — output will be required"); - return parsed as Record; -} - -function resolveTimeoutForLog(timeout: string | undefined): string { - if (!timeout) return "1h (default)"; - if (timeout === TIMEOUT_DISABLED) return "none (disabled)"; - return timeout; -} - -function resolveModelForLog(ctx: { - payload: ResolvedPayload; - resolvedModel: string | undefined; -}): string { - const envModel = process.env.PULLFROG_MODEL?.trim(); - if (envModel) return `${envModel} (override via PULLFROG_MODEL)`; - if (ctx.payload.proxyModel) return `${ctx.payload.proxyModel} (proxy)`; - if (ctx.resolvedModel && ctx.payload.model && ctx.payload.model !== ctx.resolvedModel) { - return `${ctx.resolvedModel} (resolved from ${ctx.payload.model})`; - } - if (ctx.resolvedModel) return ctx.resolvedModel; - if (ctx.payload.model) return `${ctx.payload.model} (unresolved)`; - return "auto"; -} - -function resolveAgentForLog(ctx: { agentName: string; resolvedModel: string | undefined }): string { - const envAgent = process.env.PULLFROG_AGENT?.trim(); - if (envAgent && envAgent === ctx.agentName) { - return `${ctx.agentName} (override via PULLFROG_AGENT)`; - } - if (ctx.agentName === "claude" && ctx.resolvedModel) { - return `${ctx.agentName} (auto-selected for ${ctx.resolvedModel})`; - } - return ctx.agentName; -} - -import type { ResolvedPayload } from "./utils/payload.ts"; - -interface OidcCredentials { - requestUrl: string; - requestToken: string; -} - -/** - * Billing-layer error surfaced from `/api/proxy-token` as a 402. User-actionable - * — distinct from TransientError (503 / transient sync issue) so the job - * summary + PR comment can use affirmative "you need to do X" copy rather than - * the ambiguous "billing error" label that makes transient outages look like - * the user's fault. - * - * `code` is a server-side discriminator: `router_requires_card` (no card + no - * wallet balance on Router), or null for unclassified. `declineCode` is - * Stripe's more specific sub-reason on `card_declined` (e.g. - * `insufficient_funds`, `lost_card`). `needsReauthentication` is the 3DS case - * broken out for convenience. - */ -class BillingError extends Error { - code: string | null; - declineCode: string | null; - needsReauthentication: boolean; - - constructor( - message: string, - opts: { - code?: string | null; - declineCode?: string | null; - needsReauthentication?: boolean; - } = {} - ) { - super(message); - this.name = "BillingError"; - this.code = opts.code ?? null; - this.declineCode = opts.declineCode ?? null; - this.needsReauthentication = opts.needsReauthentication ?? false; - } -} - -/** - * Transient service failures from `/api/proxy-token` (503: partial OpenRouter - * usage sync, DB flake, in-flight payment intent). Not the user's fault — the - * summary uses "temporarily unavailable" framing, and the non-zero exit lets - * GH Actions apply whatever retry policy the workflow has configured. - */ -class TransientError extends Error { - constructor(message: string) { - super(message); - this.name = "TransientError"; - } -} - -/** - * Deep link into the right console section for the failing account. Anchors - * are defined in `app/console/[owner]/page.tsx` (`#billing`, `#model-access`). - * `owner` is the GitHub login of the repo's account — i.e. the org or user - * that pays for this repo's runs, which is the right scope for billing. - */ -function billingConsoleUrl(owner: string, anchor: "billing" | "model-access"): string { - return `https://pullfrog.com/console/${encodeURIComponent(owner)}#${anchor}`; -} - -/** - * Render a BillingError as user-facing markdown (shared between GH job summary - * and the PR progress comment). Goals: - * - * - quiet, not alarmist — bold first line instead of an `### ❌` H3, since - * the comment already has Pullfrog branding in the footer - * - actionable — every branch ends in a single CTA deep-linked to the - * correct section of the owner's console - * - honest — say what actually went wrong (card declined vs. balance - * empty vs. 3DS required), don't lump them under "billing error" - * - * Branches: - * - `router_requires_card`: user is on Router mode with no card AND no - * wallet balance (signup credit exhausted or not granted). Frame as - * "add a card to continue", link to `#model-access` where the Add - * Card flow lives. - * - `router_balance_exhausted`: user has a card on file but auto-reload is - * disabled and they've spent past their $5 overdraft buffer. Frame as - * "balance ran out" and surface both remediation paths (top up, or flip - * on auto-reload). - * - `router_keylimit_exhausted`: OpenRouter rejected mid-run because the - * per-run key budget was exhausted while the agent was working. The - * wallet is now negative; same remediation as `router_balance_exhausted` - * but framed for the after-the-fact case ("this run was cut short"). - * - `needsReauthentication`: issuer requires 3DS on every off-session - * charge. Re-adding the card won't help — the only escape is a manual - * top-up where 3DS runs interactively in Stripe Checkout. - * - `declineCode` set: Stripe declined a real charge. Show the sub-code - * so support can act on it; tell the user we'll retry on next dispatch. - * - default: balance hit zero with no in-flight charge (auto-reload off - * or amount below threshold). Direct them to top up or enable auto-reload. - */ -function formatBillingErrorSummary(error: BillingError, owner: string): string { - if (error.code === "router_requires_card") { - return [ - "**Add a card to start using Pullfrog Router.**", - "", - "Router proxies OpenRouter at raw cost — no platform markup. Add a card and we'll auto-reload your wallet so runs keep flowing.", - "", - `[Add a card →](${billingConsoleUrl(owner, "model-access")})`, - ].join("\n"); - } - - if (error.code === "router_balance_exhausted") { - return [ - "**Your Pullfrog Router balance is exhausted.**", - "", - "You have a card on file but auto-reload is disabled, so runs paused once your balance went past the overdraft buffer.", - "", - `[Top up balance →](${billingConsoleUrl(owner, "billing")}) · [Enable auto-reload →](${billingConsoleUrl(owner, "model-access")})`, - ].join("\n"); - } - - if (error.code === "router_keylimit_exhausted") { - return [ - "**This run was cut short — your Pullfrog Router balance ran out mid-run.**", - "", - "OpenRouter stopped the agent because the per-run budget was exhausted. Your wallet is now negative; top up or enable auto-reload to keep runs flowing.", - "", - `[Top up balance →](${billingConsoleUrl(owner, "billing")}) · [Enable auto-reload →](${billingConsoleUrl(owner, "model-access")})`, - ].join("\n"); - } - - if (error.needsReauthentication) { - const code = error.declineCode ?? "authentication_required"; - return [ - `**Your card issuer requires 3D Secure on every charge** (\`${code}\`).`, - "", - "Pullfrog can't complete a 3DS challenge from inside a workflow. Top up your Router balance once in Stripe Checkout — subsequent runs draw from the prepaid balance without re-triggering 3DS.", - "", - `[Top up balance →](${billingConsoleUrl(owner, "billing")})`, - ].join("\n"); - } - - if (error.declineCode) { - return [ - `**Your card was declined** (\`${error.declineCode}\`).`, - "", - "Update your payment method and Pullfrog will retry on the next run.", - "", - `[Update payment method →](${billingConsoleUrl(owner, "billing")})`, - ].join("\n"); - } - - return [ - "**Your Pullfrog balance is empty.**", - "", - "Top up your balance or enable auto-reload to keep runs flowing.", - "", - `[Manage billing →](${billingConsoleUrl(owner, "billing")})`, - ].join("\n"); -} - -/** - * Render a TransientError as user-facing markdown. Distinct framing from - * BillingError so the user doesn't read an alarm and assume their card - * failed — this branch is "our fault, retry shortly", not theirs. - */ -function formatTransientErrorSummary(error: TransientError, owner: string): string { - return [ - "**Pullfrog billing is temporarily unavailable.**", - "", - error.message, - "", - `Usually transient — the next dispatch should succeed. If it persists, check [status.pullfrog.com](https://status.pullfrog.com) or [your console](${billingConsoleUrl(owner, "billing")}).`, - ].join("\n"); -} - -async function mintProxyKey(ctx: { - oidcCredentials: OidcCredentials | null; - repo: { owner: string; name: string }; -}): Promise { - try { - const headers = await buildProxyTokenHeaders(ctx); - if (!headers) return null; - - const response = await apiFetch({ - path: "/api/proxy-token", - method: "POST", - headers, - }); - - if (response.status === 402) { - const body = (await response.json().catch(() => null)) as { - error?: string; - code?: string; - declineCode?: string; - needsReauthentication?: boolean; - } | null; - throw new BillingError(body?.error ?? "insufficient balance", { - code: body?.code ?? null, - declineCode: body?.declineCode ?? null, - needsReauthentication: body?.needsReauthentication ?? false, - }); - } - - // 503 = transient sync issue (partial OpenRouter failure, DB flake, - // in-flight top-up). Not the user's fault — TransientError renders a - // "temporarily unavailable" summary instead of the "billing error" - // label that BillingError uses. - if (response.status === 503) { - const body = (await response.json().catch(() => null)) as { error?: string } | null; - throw new TransientError( - body?.error ?? "billing service temporarily unavailable — retry shortly" - ); - } - - if (!response.ok) { - log.warning(`proxy key mint failed (${response.status})`); - return null; - } - - const data = (await response.json()) as { key: string }; - return data.key; - } catch (error) { - if (error instanceof BillingError) throw error; - if (error instanceof TransientError) throw error; - log.warning(`proxy key mint error: ${error instanceof Error ? error.message : String(error)}`); - return null; - } finally { - delete process.env.ACTIONS_ID_TOKEN_REQUEST_URL; - delete process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN; - } -} - -/** - * choose how to authenticate the `/api/proxy-token` request: - * - * - production: mint a fresh OIDC token via `core.getIDToken` and send as - * `Authorization: Bearer …` (the server verifies it cryptographically). - * - local dev (no OIDC + `API_URL` is localhost): send `x-dev-repo: - * owner/repo` instead. the server-side route only honors this header - * when `NODE_ENV === "development"`, so prod is never reachable through - * this branch even if the action is misconfigured. - * - * returns null when neither path is available — caller treats as soft skip. - */ -async function buildProxyTokenHeaders(ctx: { - oidcCredentials: OidcCredentials | null; - repo: { owner: string; name: string }; -}): Promise | null> { - if (ctx.oidcCredentials) { - process.env.ACTIONS_ID_TOKEN_REQUEST_URL = ctx.oidcCredentials.requestUrl; - process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = ctx.oidcCredentials.requestToken; - const oidcToken = await core.getIDToken("pullfrog-api"); - delete process.env.ACTIONS_ID_TOKEN_REQUEST_URL; - delete process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN; - return { Authorization: `Bearer ${oidcToken}` }; - } - if (isLocalApiUrl()) { - log.info(`» proxy: dev bypass (x-dev-repo) for ${ctx.repo.owner}/${ctx.repo.name}`); - return { "x-dev-repo": `${ctx.repo.owner}/${ctx.repo.name}` }; - } - return null; -} - -async function resolveProxyModel(ctx: { - payload: ResolvedPayload; - oss: boolean; - plan: AccountPlan; - proxyModel?: string | undefined; - oidcCredentials: OidcCredentials | null; - repo: { owner: string; name: string }; -}): Promise { - // env override = BYOK escape hatch, don't proxy - if (process.env.PULLFROG_MODEL?.trim()) return; - - const needsProxy = isInfraCovered({ isOss: ctx.oss, plan: ctx.plan }) && ctx.proxyModel; - if (!needsProxy) return; - - // dev affordance: when talking to a localhost API, the server-side - // x-dev-repo bypass replaces OIDC verification, so a play run can - // exercise the proxy/router/oss path without GitHub Actions OIDC. - if (!ctx.oidcCredentials && !isLocalApiUrl()) { - log.warning("» proxy requested but no OIDC credentials available — skipping"); - return; - } - - const key = await mintProxyKey({ oidcCredentials: ctx.oidcCredentials, repo: ctx.repo }); - if (!key) return; - - process.env.OPENROUTER_API_KEY = key; - core.setSecret(key); - ctx.payload.proxyModel = ctx.proxyModel; - const label = ctx.oss ? "oss" : "router"; - log.info(`» proxy: ${label} → ${ctx.proxyModel}`); -} - -/** - * Fetch the most recent persisted PR summary snapshot for this PR. - * Returns null on first-time PRs, when summary is disabled, or on any error. - * Best-effort: a transient API failure should not block the run. - */ -async function fetchPreviousSnapshot(ctx: ToolContext, prNumber: number): Promise { - if (!ctx.githubInstallationToken) return null; - try { - const response = await apiFetch({ - path: `/api/repo/${ctx.repo.owner}/${ctx.repo.name}/pr/${prNumber}/summary-comment`, - method: "GET", - headers: { authorization: `Bearer ${ctx.githubInstallationToken}` }, - signal: AbortSignal.timeout(10_000), - }); - if (!response.ok) return null; - const data = (await response.json()) as { snapshot?: string | null }; - return typeof data.snapshot === "string" && data.snapshot.length > 0 ? data.snapshot : null; - } catch { - return null; - } -} - -/** - * Read the agent-edited PR summary tmpfile and persist to `WorkflowRun.summarySnapshot`. - * - * Best-effort: any failure is logged and does not affect the run's success - * status. Skips the PATCH when the file is byte-identical to its seed — - * persisting the seed verbatim would either re-write what the DB already has - * (on incremental runs) or serialize the placeholder scaffold (on first - * runs), neither of which is useful. - */ -/** - * Read the agent-edited repo-level learnings tmpfile and PATCH it to - * `Repo.learnings`. - * - * Best-effort: any failure is logged and does not affect the run's success - * status. Skips the PATCH when the file is byte-trim-identical to its seed — - * the agent didn't touch it, so writing the same content back would just - * burn a `LearningsRevision` row and an API round-trip. - * - * `model` is forwarded so `LearningsRevision.model` keeps populating; it - * powers the per-revision attribution badge in the UI history view. - */ -async function persistLearnings(ctx: ToolContext): Promise { - const filePath = ctx.toolState.learningsFilePath; - if (!filePath) return; - if (ctx.toolState.learningsPersistAttempted) return; - ctx.toolState.learningsPersistAttempted = true; - const current = await readLearningsFile(filePath); - if (current === null) { - log.debug(`learnings tmpfile missing or unreadable at ${filePath} — skipping persist`); - return; - } - const seed = ctx.toolState.learningsSeed?.trim() ?? ""; - if (current === seed) { - log.debug("learnings tmpfile unchanged from seed — skipping persist"); - return; - } - try { - const response = await apiFetch({ - path: `/api/repo/${ctx.repo.owner}/${ctx.repo.name}/learnings`, - method: "PATCH", - headers: { - authorization: `Bearer ${ctx.apiToken}`, - "content-type": "application/json", - }, - body: JSON.stringify({ - learnings: current, - model: ctx.toolState.model, - }), - signal: AbortSignal.timeout(10_000), - }); - if (!response.ok) { - const error = await response.text().catch(() => "(no body)"); - // promoted from debug → warning: this path means the agent edited the - // file (we already short-circuited the unchanged-from-seed case above) - // but the PATCH dropped it on the floor. silently losing real work is - // worse than the noise of a CI warning. - log.warning(`learnings persist failed (${response.status}): ${error}`); - return; - } - log.info("» learnings updated"); - } catch (err) { - log.warning(`learnings persist failed: ${err instanceof Error ? err.message : String(err)}`); - } -} - -async function persistSummary(ctx: ToolContext): Promise { - const filePath = ctx.toolState.summaryFilePath; - if (!filePath) return; - // already-completed guard: the error-path call (success path persisted, - // then a late step threw) and the SIGINT/SIGTERM handler all funnel - // through here; the first one to arrive wins. - if (ctx.toolState.summaryPersistAttempted) return; - ctx.toolState.summaryPersistAttempted = true; - const snapshot = await readSummaryFile(filePath); - if (!snapshot) { - log.debug(`pr summary tmpfile missing or invalid at ${filePath} — skipping persist`); - return; - } - // soft gate: agent never touched the seeded file. saving the seed back - // is a no-op at best (incremental run — DB already has it) and a bug at - // worst (first run — serializes the placeholder italics). log a warning - // so the failure mode is visible in CI without flipping the run to - // failed. - const seed = ctx.toolState.summarySeed?.trim(); - if (seed !== undefined && snapshot === seed) { - log.warning( - "» pr summary tmpfile unchanged from seed — skipping persist (agent did not edit it)" - ); - return; - } - await patchWorkflowRunFields(ctx, { summarySnapshot: snapshot }).catch((err) => { - log.debug(`pr summary persist failed: ${err instanceof Error ? err.message : String(err)}`); - }); -} - -// fall back to the agent's final assistant message when the agent never -// called report_progress (e.g. schedule/workflow_dispatch runs that have no -// PR/issue context to comment on). lastProgressBody wins when present so we -// don't double up the progress comment body in the job summary. -async function writeJobSummary(toolState: ToolState, finalOutput?: string): Promise { - const usageSummary = formatUsageSummary(toolState.usageEntries); - const body = toolState.lastProgressBody || finalOutput; - const summaryParts = [body, usageSummary].filter(Boolean); - if (summaryParts.length > 0) { - await writeSummary(summaryParts.join("\n\n")); - } -} - export async function main(): Promise { // normalize env var names to uppercase (handles case-insensitive workflow files) normalizeEnv(); @@ -631,39 +156,19 @@ export async function main(): Promise { } // Proxy decision: mint an OpenRouter key for OSS repos or managed billing - // accounts. BillingError (402) and TransientError (503) both surface here. - // Handle explicitly so the user sees an actionable message (job summary + - // PR progress comment when one exists) — otherwise the error unwinds past - // the main try/catch (which needs toolState) and lands in runMain with only - // a generic core.setFailed. - try { - await resolveProxyModel({ - payload, - oss: runContext.oss, - plan: runContext.plan, - proxyModel: runContext.proxyModel, - oidcCredentials, - repo: runContext.repo, - }); - } catch (error) { - if (error instanceof BillingError) { - const summary = formatBillingErrorSummary(error, runContext.repo.owner); - await writeSummary(summary).catch(() => {}); - // Mirror to the PR progress comment if the trigger created one - // (mention / PR event). Without this, auto-reload declines are only - // visible in the job summary — users rarely open that, so the agent - // just appears to silently stop mid-run. - await reportErrorToComment({ toolState, error: summary }).catch(() => {}); - throw error; - } - if (error instanceof TransientError) { - const summary = formatTransientErrorSummary(error, runContext.repo.owner); - await writeSummary(summary).catch(() => {}); - await reportErrorToComment({ toolState, error: summary }).catch(() => {}); - throw error; - } - throw error; - } + // accounts. BillingError (402) and TransientError (503) get rendered inside + // `runProxyResolution` before being rethrown — handled here (not in the + // outer catch) because the outer catch needs `toolContext` (not yet built) + // for its general-purpose error path. + await runProxyResolution({ + payload, + oss: runContext.oss, + plan: runContext.plan, + proxyModel: runContext.proxyModel, + oidcCredentials, + repo: runContext.repo, + toolState, + }); // create octokit with MCP token for GitHub API calls const octokit = createOctokit(tokenRef.mcpToken); @@ -784,11 +289,9 @@ export async function main(): Promise { // and an unwrapped filesystem failure (ENOSPC, EACCES, hostile sandbox) // would unwind into the outer main() catch and flip an otherwise- // successful run to "❌ Pullfrog failed" before the agent even starts. - // matches `persistLearnings`'s own best-effort contract — learnings - // are a peripheral artifact, not a load-bearing capability. on failure - // toolState.learningsFilePath stays unset, and downstream consumers - // (`persistLearnings`, agent harnesses, `resolveInstructions`) all - // treat undefined as "no learnings affordance this run". + // on failure toolState.learningsFilePath stays unset, and downstream + // consumers (`persistLearnings`, agent harnesses, `resolveInstructions`) + // all treat undefined as "no learnings affordance this run". try { const learningsPath = await seedLearningsFile({ tmpdir, @@ -822,9 +325,6 @@ export async function main(): Promise { // the post-run retry loop to detect the agent forgetting to edit // the file (byte-identical to seed → nudge once via resume turn) // and by persistSummary to skip the DB write when nothing changed. - // we just wrote the file, so the read shouldn't fail; the catch - // leaves summarySeed unset (its default), in which case the unchanged - // checks downstream are simply skipped. try { toolState.summarySeed = await readFile(filePath, "utf8"); } catch { @@ -846,14 +346,7 @@ export async function main(): Promise { startInstallation(toolContext); - const modelForLog = resolveModelForLog({ payload, resolvedModel }); - const agentForLog = resolveAgentForLog({ agentName: agent.name, resolvedModel }); - const timeoutForLog = resolveTimeoutForLog(payload.timeout); - log.info(`» model: ${modelForLog}`); - log.info(`» agent: ${agentForLog}`); - log.info(`» push: ${payload.push}`); - log.info(`» shell: ${payload.shell}`); - log.info(`» timeout: ${timeoutForLog}`); + logRunStartup({ payload, resolvedModel, agentName: agent.name }); const instructions = resolveInstructions({ payload, @@ -1026,101 +519,13 @@ export async function main(): Promise { ); } - // post-agent review cleanup: reportReviewNodeId → follow-up re-review dispatch. - // runs after the agent exits so ordering is architecturally guaranteed (no LLM involvement). - // best-effort: cleanup failures must not turn a successful agent run into a failure. - // - // note: progress-comment deletion on review submission is owned by - // create_pull_request_review (action/mcp/review.ts) and runs atomically - // with the submission, so it survives any path out of main (success, - // timeout, crash) without relying on cleanup ordering here. - if (toolContext) { - await postReviewCleanup(toolContext).catch((error) => { - log.debug(`post-review cleanup failed: ${error}`); - }); - } - - // read the agent-edited summary tmpfile and persist to the DB. happens - // after the agent exits so the file is in its final state. - if (toolContext) { - await persistSummary(toolContext); - } - - // same for the rolling repo-level learnings tmpfile. always seeded, so - // always read back; persistLearnings short-circuits when the file is - // unchanged from its seed. - if (toolContext) { - await persistLearnings(toolContext); - } - - // when the agent harness returns success=false (e.g. unsubmitted-review - // gate exhausted retries, stop-hook persistently failing), surface the - // error in the progress comment so the user sees it instead of a - // deleted-comment void. mirrors the catch-block error reporting for - // thrown errors. runs before the stranded-comment cleanup below so - // the comment is still around to update; reportErrorToComment sets - // wasUpdated=true and the !result.success guard skips deletion. - if (!result.success && toolContext && toolState.progressComment) { - const rawError = result.error || "agent run failed"; - const errorBody = isApiKeyAuthError(rawError) - ? formatApiKeyErrorSummary({ - owner: runContext.repo.owner, - name: runContext.repo.name, - raw: rawError, - }) - : rawError; - await reportErrorToComment({ toolState, error: errorBody }).catch((error) => { - log.debug(`failure error report failed: ${error}`); - }); - } - - // clean up stranded progress comments. the comment is stale unless - // report_progress wrote a final summary to it — three sub-cases all reduce - // to !finalSummaryWritten: - // 1. nothing wrote to the comment ("Leaping into action" orphan) - // 2. tracker published a checklist but the agent never finalized it - // 3. the agent produced a substantive artifact via another MCP write tool - // (create_issue_comment, update_pull_request_body, reply_to_review_comment) - // and skipped report_progress — wasUpdated is true, but the progress - // comment itself was never touched. - // create_pull_request_review owns its own deletion (see action/mcp/review.ts), - // so progressComment is already null by the time we get here for that path. - // uses finalSummaryWritten (not todoTracker.enabled or wasUpdated) so cleanup - // survives API failures in report_progress where cancel() ran but the write - // didn't succeed, and isn't fooled by writes to *other* artifacts. skipped - // entirely on result.success===false: the error message just written above - // is the user's only signal that the run happened — deleting it would - // restore the same empty-void UX this commit fixes. - if ( - toolContext && - result.success && - toolState.progressComment && - !toolState.finalSummaryWritten - ) { - await deleteProgressComment(toolContext).catch((error) => { - log.debug(`stranded progress comment cleanup failed: ${error}`); - }); - } - - // best-effort: failures writing the actions step summary must not throw - // past this point. on the result.success===false branch above we already - // wrote `result.error` to the progress comment, and a throw here would - // jump to the outer catch which calls reportErrorToComment again with - // the (less actionable) writeJobSummary error — silently overwriting the - // gate's failure message in the progress comment. the step-summary write - // is informational; let it fail silently rather than corrupt user-facing - // output. - try { - await writeJobSummary(toolState, result.output); - } catch (error) { - log.debug(`job summary write failed: ${error}`); - } - - // emit structured output marker for test validation - if (toolState.output) { - log.info(`::pullfrog-output::${Buffer.from(toolState.output).toString("base64")}`); - core.setOutput("result", toolState.output); - } + // success-path cleanup: postReview → persistSummary → persistLearnings → + // failure-error-report → stranded-comment cleanup → job summary → output + // marker. each step is best-effort; see `finalizeSuccessRun` for ordering + // rationale (notably: progress-comment deletion lives in + // create_pull_request_review for review-mode runs, so deletion here + // covers the non-review success paths). + await finalizeSuccessRun({ toolContext, toolState, result, repo: runContext.repo }); return await handleAgentResult({ result, @@ -1134,86 +539,19 @@ export async function main(): Promise { killTrackedChildren(); log.error(errorMessage); - // Reclassify OpenRouter "key budget exhausted" mid-run errors as - // BillingError. The agent runtime surfaces this as a generic APIError, - // but it's a Pullfrog billing concern — the user's Router wallet ran - // out partway through the run. Route through the same formatBillingErrorSummary - // path as proxy-token 402s so the user gets actionable copy + a top-up - // CTA on both the job summary and the PR progress comment, instead of - // a generic "❌ Pullfrog failed" stack-trace dump. - const billingError = isRouterKeylimitExhaustedError(errorMessage) - ? new BillingError(errorMessage, { code: "router_keylimit_exhausted" }) - : null; + // classify (BillingError reclassification + hang detection + API-key auth + // detection) and render to {summary, comment} markdown bodies. + const rendered = renderRunError({ + errorMessage, + repo: runContext.repo, + agentDiagnostic: toolState.agentDiagnostic, + }); + await writeRunErrorOutputs({ rendered, toolState }); - // 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(apiKeySource) - ? formatApiKeyErrorSummary({ - owner: runContext.repo.owner, - name: runContext.repo.name, - raw: apiKeySource, - }) - : null; - - // best-effort summary — write the error so it's visible in the Actions summary tab - try { - const errorSummary = billingError - ? formatBillingErrorSummary(billingError, runContext.repo.owner) - : (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")); - } catch {} - - try { - const commentBody = billingError - ? formatBillingErrorSummary(billingError, runContext.repo.owner) - : (apiKeyErrorSummary ?? hangBody ?? errorMessage); - await reportErrorToComment({ toolState, error: commentBody }); - } catch { - // error reporting failed, but don't let it mask the original error - } - - // best-effort review cleanup (e.g., agent timed out after submitting a review) + // best-effort cleanup: review dispatch, summary persist, learnings persist. + // a partial edit before the crash is still worth keeping. if (toolContext) { - await postReviewCleanup(toolContext).catch((error) => { - log.debug(`post-review cleanup failed: ${error}`); - }); - } - - // best-effort summary persist on the error path: if the agent successfully - // edited the summary file before timing out / crashing, those edits are - // worth keeping for the next incremental run. - if (toolContext) { - await persistSummary(toolContext); - } - - // same rationale for learnings: a partial edit before a crash is still - // worth keeping. persistLearnings is idempotent via learningsPersistAttempted. - if (toolContext) { - await persistLearnings(toolContext); + await persistRunArtifacts(toolContext); } return { diff --git a/utils/billingErrors.ts b/utils/billingErrors.ts new file mode 100644 index 0000000..6ec0799 --- /dev/null +++ b/utils/billingErrors.ts @@ -0,0 +1,179 @@ +/** + * Billing-error classification + user-facing copy for `/api/proxy-token` + * failures and OpenRouter mid-run exhaustion. Two error classes (Billing vs. + * Transient) keep the framing honest: a card decline is *not* the same UX as + * a 503 from the proxy service. Both originate in `utils/proxy.ts` (mint + * failures) and `utils/runErrorRenderer.ts` (mid-run keylimit reclassify). + * + * Renderers return markdown bodies that are written into both the GitHub + * Actions job summary and the PR progress comment. + * + * Lives outside `main.ts` so adding a new error `code` branch is a one-file + * edit that does not retrigger the full LLM CI matrix (`action/main.ts` is + * in `action/test/coverage.ts::ALWAYS_RUN_ALL`). + */ + +/** + * Billing-layer error surfaced from `/api/proxy-token` as a 402. User-actionable + * — distinct from TransientError (503 / transient sync issue) so the job + * summary + PR comment can use affirmative "you need to do X" copy rather than + * the ambiguous "billing error" label that makes transient outages look like + * the user's fault. + * + * `code` is a server-side discriminator: `router_requires_card` (no card + no + * wallet balance on Router), or null for unclassified. `declineCode` is + * Stripe's more specific sub-reason on `card_declined` (e.g. + * `insufficient_funds`, `lost_card`). `needsReauthentication` is the 3DS case + * broken out for convenience. + */ +export class BillingError extends Error { + code: string | null; + declineCode: string | null; + needsReauthentication: boolean; + + constructor( + message: string, + opts: { + code?: string | null; + declineCode?: string | null; + needsReauthentication?: boolean; + } = {} + ) { + super(message); + this.name = "BillingError"; + this.code = opts.code ?? null; + this.declineCode = opts.declineCode ?? null; + this.needsReauthentication = opts.needsReauthentication ?? false; + } +} + +/** + * Transient service failures from `/api/proxy-token` (503: partial OpenRouter + * usage sync, DB flake, in-flight payment intent). Not the user's fault — the + * summary uses "temporarily unavailable" framing, and the non-zero exit lets + * GH Actions apply whatever retry policy the workflow has configured. + */ +export class TransientError extends Error { + constructor(message: string) { + super(message); + this.name = "TransientError"; + } +} + +/** + * Deep link into the right console section for the failing account. Anchors + * are defined in `app/console/[owner]/page.tsx` (`#billing`, `#model-access`). + * `owner` is the GitHub login of the repo's account — i.e. the org or user + * that pays for this repo's runs, which is the right scope for billing. + */ +function billingConsoleUrl(owner: string, anchor: "billing" | "model-access"): string { + return `https://pullfrog.com/console/${encodeURIComponent(owner)}#${anchor}`; +} + +/** + * Render a BillingError as user-facing markdown (shared between GH job summary + * and the PR progress comment). Goals: + * + * - quiet, not alarmist — bold first line instead of an `### ❌` H3, since + * the comment already has Pullfrog branding in the footer + * - actionable — every branch ends in a single CTA deep-linked to the + * correct section of the owner's console + * - honest — say what actually went wrong (card declined vs. balance + * empty vs. 3DS required), don't lump them under "billing error" + * + * Branches: + * - `router_requires_card`: user is on Router mode with no card AND no + * wallet balance (signup credit exhausted or not granted). Frame as + * "add a card to continue", link to `#model-access` where the Add + * Card flow lives. + * - `router_balance_exhausted`: user has a card on file but auto-reload is + * disabled and they've spent past their $5 overdraft buffer. Frame as + * "balance ran out" and surface both remediation paths (top up, or flip + * on auto-reload). + * - `router_keylimit_exhausted`: OpenRouter rejected mid-run because the + * per-run key budget was exhausted while the agent was working. The + * wallet is now negative; same remediation as `router_balance_exhausted` + * but framed for the after-the-fact case ("this run was cut short"). + * - `needsReauthentication`: issuer requires 3DS on every off-session + * charge. Re-adding the card won't help — the only escape is a manual + * top-up where 3DS runs interactively in Stripe Checkout. + * - `declineCode` set: Stripe declined a real charge. Show the sub-code + * so support can act on it; tell the user we'll retry on next dispatch. + * - default: balance hit zero with no in-flight charge (auto-reload off + * or amount below threshold). Direct them to top up or enable auto-reload. + */ +export function formatBillingErrorSummary(error: BillingError, owner: string): string { + if (error.code === "router_requires_card") { + return [ + "**Add a card to start using Pullfrog Router.**", + "", + "Router proxies OpenRouter at raw cost — no platform markup. Add a card and we'll auto-reload your wallet so runs keep flowing.", + "", + `[Add a card →](${billingConsoleUrl(owner, "model-access")})`, + ].join("\n"); + } + + if (error.code === "router_balance_exhausted") { + return [ + "**Your Pullfrog Router balance is exhausted.**", + "", + "You have a card on file but auto-reload is disabled, so runs paused once your balance went past the overdraft buffer.", + "", + `[Top up balance →](${billingConsoleUrl(owner, "billing")}) · [Enable auto-reload →](${billingConsoleUrl(owner, "model-access")})`, + ].join("\n"); + } + + if (error.code === "router_keylimit_exhausted") { + return [ + "**This run was cut short — your Pullfrog Router balance ran out mid-run.**", + "", + "OpenRouter stopped the agent because the per-run budget was exhausted. Your wallet is now negative; top up or enable auto-reload to keep runs flowing.", + "", + `[Top up balance →](${billingConsoleUrl(owner, "billing")}) · [Enable auto-reload →](${billingConsoleUrl(owner, "model-access")})`, + ].join("\n"); + } + + if (error.needsReauthentication) { + const code = error.declineCode ?? "authentication_required"; + return [ + `**Your card issuer requires 3D Secure on every charge** (\`${code}\`).`, + "", + "Pullfrog can't complete a 3DS challenge from inside a workflow. Top up your Router balance once in Stripe Checkout — subsequent runs draw from the prepaid balance without re-triggering 3DS.", + "", + `[Top up balance →](${billingConsoleUrl(owner, "billing")})`, + ].join("\n"); + } + + if (error.declineCode) { + return [ + `**Your card was declined** (\`${error.declineCode}\`).`, + "", + "Update your payment method and Pullfrog will retry on the next run.", + "", + `[Update payment method →](${billingConsoleUrl(owner, "billing")})`, + ].join("\n"); + } + + return [ + "**Your Pullfrog balance is empty.**", + "", + "Top up your balance or enable auto-reload to keep runs flowing.", + "", + `[Manage billing →](${billingConsoleUrl(owner, "billing")})`, + ].join("\n"); +} + +/** + * Render a TransientError as user-facing markdown. Distinct framing from + * BillingError so the user doesn't read an alarm and assume their card + * failed — this branch is "our fault, retry shortly", not theirs. + */ +export function formatTransientErrorSummary(error: TransientError, owner: string): string { + return [ + "**Pullfrog billing is temporarily unavailable.**", + "", + error.message, + "", + `Usually transient — the next dispatch should succeed. If it persists, check [status.pullfrog.com](https://status.pullfrog.com) or [your console](${billingConsoleUrl(owner, "billing")}).`, + ].join("\n"); +} diff --git a/utils/learnings.ts b/utils/learnings.ts index dc0ed7d..e9b4b04 100644 --- a/utils/learnings.ts +++ b/utils/learnings.ts @@ -1,5 +1,8 @@ import { mkdir, readFile, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; +import type { ToolContext } from "../mcp/server.ts"; +import { apiFetch } from "./apiFetch.ts"; +import { log } from "./cli.ts"; /** * Repo-level learnings — operational facts about a repo (setup steps, test @@ -88,3 +91,63 @@ export async function readLearningsFile(path: string): Promise { } return truncateAtLineBoundary(raw.trim(), MAX_LEARNINGS_LENGTH); } + +/** + * Read the agent-edited repo-level learnings tmpfile and PATCH it to + * `Repo.learnings`. + * + * Best-effort: any failure is logged and does not affect the run's success + * status. Skips the PATCH when the file is byte-trim-identical to its seed — + * the agent didn't touch it, so writing the same content back would just + * burn a `LearningsRevision` row and an API round-trip. + * + * `ctx.toolState.model` is forwarded so `LearningsRevision.model` keeps + * populating; it powers the per-revision attribution badge in the UI + * history view. + * + * `learningsPersistAttempted` guards against double-execution between the + * normal end-of-run path and the SIGINT/SIGTERM handler. + */ +export async function persistLearnings(ctx: ToolContext): Promise { + const filePath = ctx.toolState.learningsFilePath; + if (!filePath) return; + if (ctx.toolState.learningsPersistAttempted) return; + ctx.toolState.learningsPersistAttempted = true; + const current = await readLearningsFile(filePath); + if (current === null) { + log.debug(`learnings tmpfile missing or unreadable at ${filePath} — skipping persist`); + return; + } + const seed = ctx.toolState.learningsSeed?.trim() ?? ""; + if (current === seed) { + log.debug("learnings tmpfile unchanged from seed — skipping persist"); + return; + } + try { + const response = await apiFetch({ + path: `/api/repo/${ctx.repo.owner}/${ctx.repo.name}/learnings`, + method: "PATCH", + headers: { + authorization: `Bearer ${ctx.apiToken}`, + "content-type": "application/json", + }, + body: JSON.stringify({ + learnings: current, + model: ctx.toolState.model, + }), + signal: AbortSignal.timeout(10_000), + }); + if (!response.ok) { + const error = await response.text().catch(() => "(no body)"); + // promoted from debug → warning: this path means the agent edited the + // file (we already short-circuited the unchanged-from-seed case above) + // but the PATCH dropped it on the floor. silently losing real work is + // worse than the noise of a CI warning. + log.warning(`learnings persist failed (${response.status}): ${error}`); + return; + } + log.info("» learnings updated"); + } catch (err) { + log.warning(`learnings persist failed: ${err instanceof Error ? err.message : String(err)}`); + } +} diff --git a/utils/payload.ts b/utils/payload.ts index 13003a9..2795f8a 100644 --- a/utils/payload.ts +++ b/utils/payload.ts @@ -3,6 +3,7 @@ import * as core from "@actions/core"; import { type } from "arktype"; import type { AuthorPermission, PayloadEvent } from "../external.ts"; import packageJson from "../package.json" with { type: "json" }; +import { log } from "./cli.ts"; import type { RepoSettings } from "./runContext.ts"; import { validateCompatibility } from "./versioning.ts"; @@ -175,3 +176,26 @@ export function resolvePayload( } export type ResolvedPayload = ReturnType; + +/** + * Parse and validate the optional `output_schema` action input. Returns the + * parsed object when present, or `undefined` when absent. Throws on invalid + * JSON or non-object payloads — these are workflow-author errors that should + * surface immediately, not silently degrade to "no schema". + */ +export function resolveOutputSchema(): Record | undefined { + const raw = core.getInput("output_schema"); + if (!raw) return undefined; + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + throw new Error(`invalid output_schema: not valid JSON`); + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new Error(`invalid output_schema: must be a JSON object`); + } + log.info("» structured output schema provided — output will be required"); + return parsed as Record; +} diff --git a/utils/prSummary.ts b/utils/prSummary.ts index dbbfbec..b094665 100644 --- a/utils/prSummary.ts +++ b/utils/prSummary.ts @@ -1,5 +1,9 @@ import { mkdir, readFile, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; +import type { ToolContext } from "../mcp/server.ts"; +import { apiFetch } from "./apiFetch.ts"; +import { log } from "./cli.ts"; +import { patchWorkflowRunFields } from "./patchWorkflowRunFields.ts"; /** * The PR-level summary snapshot is a markdown file the agent edits in place @@ -76,3 +80,68 @@ export async function readSummaryFile(path: string): Promise { if (trimmed.length > MAX_SNAPSHOT_LENGTH) return trimmed.slice(0, MAX_SNAPSHOT_LENGTH); return trimmed; } + +/** + * Fetch the most recent persisted PR summary snapshot for this PR. + * Returns null on first-time PRs, when summary is disabled, or on any error. + * Best-effort: a transient API failure should not block the run. + */ +export async function fetchPreviousSnapshot( + ctx: ToolContext, + prNumber: number +): Promise { + if (!ctx.githubInstallationToken) return null; + try { + const response = await apiFetch({ + path: `/api/repo/${ctx.repo.owner}/${ctx.repo.name}/pr/${prNumber}/summary-comment`, + method: "GET", + headers: { authorization: `Bearer ${ctx.githubInstallationToken}` }, + signal: AbortSignal.timeout(10_000), + }); + if (!response.ok) return null; + const data = (await response.json()) as { snapshot?: string | null }; + return typeof data.snapshot === "string" && data.snapshot.length > 0 ? data.snapshot : null; + } catch { + return null; + } +} + +/** + * Read the agent-edited PR summary tmpfile and persist to + * `WorkflowRun.summarySnapshot`. + * + * Best-effort: any failure is logged and does not affect the run's success + * status. Skips the PATCH when the file is byte-identical to its seed — + * persisting the seed verbatim would either re-write what the DB already has + * (on incremental runs) or serialize the placeholder scaffold (on first + * runs), neither of which is useful. + * + * Funnels through both the success path and the SIGINT/SIGTERM handler; + * `summaryPersistAttempted` guards against double-execution. + */ +export async function persistSummary(ctx: ToolContext): Promise { + const filePath = ctx.toolState.summaryFilePath; + if (!filePath) return; + if (ctx.toolState.summaryPersistAttempted) return; + ctx.toolState.summaryPersistAttempted = true; + const snapshot = await readSummaryFile(filePath); + if (!snapshot) { + log.debug(`pr summary tmpfile missing or invalid at ${filePath} — skipping persist`); + return; + } + // soft gate: agent never touched the seeded file. saving the seed back + // is a no-op at best (incremental run — DB already has it) and a bug at + // worst (first run — serializes the placeholder italics). log a warning + // so the failure mode is visible in CI without flipping the run to + // failed. + const seed = ctx.toolState.summarySeed?.trim(); + if (seed !== undefined && snapshot === seed) { + log.warning( + "» pr summary tmpfile unchanged from seed — skipping persist (agent did not edit it)" + ); + return; + } + await patchWorkflowRunFields(ctx, { summarySnapshot: snapshot }).catch((err) => { + log.debug(`pr summary persist failed: ${err instanceof Error ? err.message : String(err)}`); + }); +} diff --git a/utils/proxy.ts b/utils/proxy.ts new file mode 100644 index 0000000..b0fe2fb --- /dev/null +++ b/utils/proxy.ts @@ -0,0 +1,221 @@ +/** + * Mint an OpenRouter proxy key via `/api/proxy-token` and inject it as + * `OPENROUTER_API_KEY` for runs that route through Pullfrog Router (managed + * billing accounts) or OSS-grant paths. + * + * Authenticates one of two ways: + * - production: GitHub Actions OIDC token via `core.getIDToken` + * - local dev (`API_URL` is localhost): `x-dev-repo` header bypass + * + * `runProxyResolution` is the entrypoint `main.ts` calls. It wraps + * `resolveProxyModel` and renders the user-facing copy itself (job summary + * + PR progress comment) before rethrowing the structured error — handled + * here, not in the outer `main()` catch, because `toolContext` doesn't + * exist yet at this point in the pipeline. + * + * - 402 → `BillingError` (card declined, balance empty, 3DS, etc.) + * - 503 → `TransientError` (transient sync issue — retry next dispatch) + */ + +import * as core from "@actions/core"; +import type { ToolState } from "../toolState.ts"; +import { apiFetch } from "./apiFetch.ts"; +import { isLocalApiUrl } from "./apiUrl.ts"; +import { + BillingError, + formatBillingErrorSummary, + formatTransientErrorSummary, + TransientError, +} from "./billingErrors.ts"; +import { log, writeSummary } from "./cli.ts"; +import { reportErrorToComment } from "./errorReport.ts"; +import type { ResolvedPayload } from "./payload.ts"; +import { type AccountPlan, isInfraCovered } from "./runContext.ts"; + +export interface OidcCredentials { + requestUrl: string; + requestToken: string; +} + +async function mintProxyKey(ctx: { + oidcCredentials: OidcCredentials | null; + repo: { owner: string; name: string }; +}): Promise { + try { + const headers = await buildProxyTokenHeaders(ctx); + if (!headers) return null; + + const response = await apiFetch({ + path: "/api/proxy-token", + method: "POST", + headers, + }); + + if (response.status === 402) { + const body = (await response.json().catch(() => null)) as { + error?: string; + code?: string; + declineCode?: string; + needsReauthentication?: boolean; + } | null; + throw new BillingError(body?.error ?? "insufficient balance", { + code: body?.code ?? null, + declineCode: body?.declineCode ?? null, + needsReauthentication: body?.needsReauthentication ?? false, + }); + } + + // 503 = transient sync issue (partial OpenRouter failure, DB flake, + // in-flight top-up). Not the user's fault — TransientError renders a + // "temporarily unavailable" summary instead of the "billing error" + // label that BillingError uses. + if (response.status === 503) { + const body = (await response.json().catch(() => null)) as { error?: string } | null; + throw new TransientError( + body?.error ?? "billing service temporarily unavailable — retry shortly" + ); + } + + if (!response.ok) { + log.warning(`proxy key mint failed (${response.status})`); + return null; + } + + const data = (await response.json()) as { key: string }; + return data.key; + } catch (error) { + if (error instanceof BillingError) throw error; + if (error instanceof TransientError) throw error; + log.warning(`proxy key mint error: ${error instanceof Error ? error.message : String(error)}`); + return null; + } finally { + delete process.env.ACTIONS_ID_TOKEN_REQUEST_URL; + delete process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN; + } +} + +/** + * choose how to authenticate the `/api/proxy-token` request: + * + * - production: mint a fresh OIDC token via `core.getIDToken` and send as + * `Authorization: Bearer …` (the server verifies it cryptographically). + * - local dev (no OIDC + `API_URL` is localhost): send `x-dev-repo: + * owner/repo` instead. the server-side route only honors this header + * when `NODE_ENV === "development"`, so prod is never reachable through + * this branch even if the action is misconfigured. + * + * returns null when neither path is available — caller treats as soft skip. + */ +async function buildProxyTokenHeaders(ctx: { + oidcCredentials: OidcCredentials | null; + repo: { owner: string; name: string }; +}): Promise | null> { + if (ctx.oidcCredentials) { + process.env.ACTIONS_ID_TOKEN_REQUEST_URL = ctx.oidcCredentials.requestUrl; + process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = ctx.oidcCredentials.requestToken; + const oidcToken = await core.getIDToken("pullfrog-api"); + delete process.env.ACTIONS_ID_TOKEN_REQUEST_URL; + delete process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN; + return { Authorization: `Bearer ${oidcToken}` }; + } + if (isLocalApiUrl()) { + log.info(`» proxy: dev bypass (x-dev-repo) for ${ctx.repo.owner}/${ctx.repo.name}`); + return { "x-dev-repo": `${ctx.repo.owner}/${ctx.repo.name}` }; + } + return null; +} + +/** + * Decide whether this run needs a minted proxy key and, if so, mint and + * inject it as `OPENROUTER_API_KEY`. Mutates `payload.proxyModel` on success. + * + * Skipped when: + * - `PULLFROG_MODEL` env override is set (BYOK escape hatch) + * - `isInfraCovered({ oss, plan })` is false (BYOK account on a paid run) + * - `proxyModel` is not set on the run context + * - no OIDC credentials available and not talking to a localhost API + * + * Throws `BillingError` (402) or `TransientError` (503); caller renders. + */ +async function resolveProxyModel(ctx: { + payload: ResolvedPayload; + oss: boolean; + plan: AccountPlan; + proxyModel?: string | undefined; + oidcCredentials: OidcCredentials | null; + repo: { owner: string; name: string }; +}): Promise { + // env override = BYOK escape hatch, don't proxy + if (process.env.PULLFROG_MODEL?.trim()) return; + + const needsProxy = isInfraCovered({ isOss: ctx.oss, plan: ctx.plan }) && ctx.proxyModel; + if (!needsProxy) return; + + // dev affordance: when talking to a localhost API, the server-side + // x-dev-repo bypass replaces OIDC verification, so a play run can + // exercise the proxy/router/oss path without GitHub Actions OIDC. + if (!ctx.oidcCredentials && !isLocalApiUrl()) { + log.warning("» proxy requested but no OIDC credentials available — skipping"); + return; + } + + const key = await mintProxyKey({ oidcCredentials: ctx.oidcCredentials, repo: ctx.repo }); + if (!key) return; + + process.env.OPENROUTER_API_KEY = key; + core.setSecret(key); + ctx.payload.proxyModel = ctx.proxyModel; + const label = ctx.oss ? "oss" : "router"; + log.info(`» proxy: ${label} → ${ctx.proxyModel}`); +} + +/** + * Run `resolveProxyModel`; if it throws a Billing or Transient error, render + * the user-facing summary, mirror it to the PR progress comment, and rethrow. + * + * The rethrow is intentional: these errors are terminal for the run, and + * letting them surface lets `runMain` exit non-zero so GH Actions applies + * the workflow's retry policy. We catch them *here* (before the main try) + * because the outer catch needs `toolContext` (which isn't built yet) for + * its general-purpose rendering path — a BillingError landing in the outer + * catch would get rendered with `core.setFailed` only, losing the + * actionable copy + the PR-comment mirror. + */ +export async function runProxyResolution(ctx: { + payload: ResolvedPayload; + oss: boolean; + plan: AccountPlan; + proxyModel?: string | undefined; + oidcCredentials: OidcCredentials | null; + repo: { owner: string; name: string }; + toolState: ToolState; +}): Promise { + try { + await resolveProxyModel({ + payload: ctx.payload, + oss: ctx.oss, + plan: ctx.plan, + proxyModel: ctx.proxyModel, + oidcCredentials: ctx.oidcCredentials, + repo: ctx.repo, + }); + } catch (error) { + if (error instanceof BillingError) { + const summary = formatBillingErrorSummary(error, ctx.repo.owner); + await writeSummary(summary).catch(() => {}); + // Mirror to the PR progress comment if the trigger created one (mention / + // PR event). Without this, auto-reload declines are only visible in the + // job summary — users rarely open that, so the agent just appears to + // silently stop mid-run. + await reportErrorToComment({ toolState: ctx.toolState, error: summary }).catch(() => {}); + throw error; + } + if (error instanceof TransientError) { + const summary = formatTransientErrorSummary(error, ctx.repo.owner); + await writeSummary(summary).catch(() => {}); + await reportErrorToComment({ toolState: ctx.toolState, error: summary }).catch(() => {}); + throw error; + } + throw error; + } +} diff --git a/utils/runErrorRenderer.ts b/utils/runErrorRenderer.ts new file mode 100644 index 0000000..9d60a5d --- /dev/null +++ b/utils/runErrorRenderer.ts @@ -0,0 +1,97 @@ +/** + * Classify + render the error thrown out of the main run try-block into a + * pair of user-facing markdown bodies — one for the GitHub Actions job + * summary tab, one for the PR progress comment. + * + * Four classifications, in priority order: + * + * 1. `BillingError` — either the proxy-token mint already threw one (402 + * handled inline) or the agent runtime surfaced an OpenRouter + * "key budget exhausted" string mid-run. Both render via + * `formatBillingErrorSummary` so the user sees actionable copy. + * + * 2. Activity-timeout hang — `errorMessage` starts with + * `"activity timeout"` or `"agent still pending"`. The harness keeps + * structured diagnostic state on `toolState.agentDiagnostic`; + * `formatAgentHangBody` renders that as a markdown block. + * + * 3. API-key auth error — `isApiKeyAuthError` sniffs the raw error string; + * `formatApiKeyErrorSummary` renders provider + console-link copy. + * + * 4. Default — a generic `❌ Pullfrog failed` block with the raw error + * message in a fenced code block. Same body for both surfaces. + * + * The hang body and the API-key body diverge between the two surfaces only + * in that the job summary wraps them in the `### ❌ Pullfrog failed` H3 + * banner; the PR comment uses the bare body since it already has Pullfrog + * branding in its footer. + */ + +import type { AgentDiagnostic } from "./agentHangReport.ts"; +import { formatAgentHangBody } from "./agentHangReport.ts"; +import { formatApiKeyErrorSummary, isApiKeyAuthError } from "./apiKeys.ts"; +import { BillingError, formatBillingErrorSummary } from "./billingErrors.ts"; +import { isRouterKeylimitExhaustedError } from "./providerErrors.ts"; + +export type RenderedRunError = { + summary: string; + comment: string; +}; + +export function renderRunError(input: { + errorMessage: string; + repo: { owner: string; name: string }; + agentDiagnostic: AgentDiagnostic | undefined; +}): RenderedRunError { + // reclassify mid-run OpenRouter "key budget exhausted" as BillingError so + // the user gets the same actionable copy as a /api/proxy-token 402. + const billingError = isRouterKeylimitExhaustedError(input.errorMessage) + ? new BillingError(input.errorMessage, { code: "router_keylimit_exhausted" }) + : null; + + if (billingError) { + const body = formatBillingErrorSummary(billingError, input.repo.owner); + return { summary: body, comment: body }; + } + + // gated on isHang because the harness sets `agentDiagnostic` on entry, so + // any non-hang throw that hits the outer catch (e.g. 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 = + input.errorMessage.startsWith("activity timeout") || + input.errorMessage.startsWith("agent still pending"); + const hangBody = isHang + ? formatAgentHangBody({ + diagnostic: input.agentDiagnostic, + isHang: true, + errorMessage: input.errorMessage, + }) + : null; + + const apiKeySource = hangBody ?? input.errorMessage; + const apiKeyErrorSummary = isApiKeyAuthError(apiKeySource) + ? formatApiKeyErrorSummary({ + owner: input.repo.owner, + name: input.repo.name, + raw: apiKeySource, + }) + : null; + + if (apiKeyErrorSummary) { + return { summary: apiKeyErrorSummary, comment: apiKeyErrorSummary }; + } + + if (hangBody) { + return { + summary: `### ❌ Pullfrog failed\n\n${hangBody}`, + comment: hangBody, + }; + } + + return { + summary: `### ❌ Pullfrog failed\n\n\`\`\`\n${input.errorMessage}\n\`\`\``, + comment: input.errorMessage, + }; +} diff --git a/utils/runLifecycle.ts b/utils/runLifecycle.ts new file mode 100644 index 0000000..e70fc65 --- /dev/null +++ b/utils/runLifecycle.ts @@ -0,0 +1,151 @@ +/** + * End-of-run cleanup phases extracted out of `main.ts`. Three shapes: + * + * - `persistRunArtifacts`: best-effort post-review cleanup + summary + + * learnings persistence. Shared by both the success path and the + * error-catch path; idempotent (each step has its own guard against + * double-execution). + * + * - `finalizeSuccessRun`: success-only — calls `persistRunArtifacts` + * first, then surfaces harness-side failures in the progress comment, + * deletes stranded progress comments, writes the GitHub Actions job + * summary, and emits the structured output marker. + * + * - `writeRunErrorOutputs`: error-only — writes the rendered error + * summary to the Actions summary tab and mirrors it to the PR + * progress comment. The catch path calls this and then + * `persistRunArtifacts` separately so the rendered error lands before + * the persistence calls, in case the latter throw. + * + * All three swallow their own non-fatal errors (`log.debug` or empty + * `catch {}`) so a cleanup failure can't flip an already-decided run + * outcome. + */ + +import * as core from "@actions/core"; +import type { AgentResult } from "../agents/shared.ts"; +import { deleteProgressComment } from "../mcp/comment.ts"; +import type { ToolContext } from "../mcp/server.ts"; +import type { ToolState } from "../toolState.ts"; +import { formatApiKeyErrorSummary, isApiKeyAuthError } from "./apiKeys.ts"; +import { formatUsageSummary, log, writeSummary } from "./cli.ts"; +import { reportErrorToComment } from "./errorReport.ts"; +import { persistLearnings } from "./learnings.ts"; +import { persistSummary } from "./prSummary.ts"; +import { postReviewCleanup } from "./reviewCleanup.ts"; +import type { RenderedRunError } from "./runErrorRenderer.ts"; + +/** + * Best-effort cleanup shared by both run-end paths: + * 1. post-review cleanup (dispatch follow-up re-review on submitted reviews) + * 2. persist the agent-edited PR summary tmpfile + * 3. persist the agent-edited repo-level learnings tmpfile + * + * Each step is idempotent and swallows its own errors. Safe to call from + * both `main()`'s success path and its catch path. + */ +export async function persistRunArtifacts(toolContext: ToolContext): Promise { + await postReviewCleanup(toolContext).catch((error) => { + log.debug(`post-review cleanup failed: ${error}`); + }); + await persistSummary(toolContext); + await persistLearnings(toolContext); +} + +/** + * Run the success-path cleanup waterfall: + * + * 1. shared best-effort cleanup via `persistRunArtifacts` + * 2. when the harness returned `success=false` (e.g. unsubmitted-review + * gate exhausted retries, stop-hook persistently failing), surface + * the error in the progress comment so the user sees it instead of a + * deleted-comment void + * 3. when the run succeeded and the progress comment was never finalized + * via `report_progress`, delete it (three sub-cases — orphan + * "Leaping into action" comment, abandoned checklist, agent wrote + * a substantive artifact via another MCP write tool but skipped + * report_progress) + * 4. write the GitHub Actions step summary (best-effort — a write + * failure must not throw past this point because we'd hit the outer + * catch and clobber any progress comment we just wrote) + * 5. emit the structured output marker for tests + workflow consumers + */ +export async function finalizeSuccessRun(input: { + toolContext: ToolContext; + toolState: ToolState; + result: AgentResult; + repo: { owner: string; name: string }; +}): Promise { + await persistRunArtifacts(input.toolContext); + + if (!input.result.success && input.toolState.progressComment) { + const rawError = input.result.error || "agent run failed"; + const errorBody = isApiKeyAuthError(rawError) + ? formatApiKeyErrorSummary({ + owner: input.repo.owner, + name: input.repo.name, + raw: rawError, + }) + : rawError; + await reportErrorToComment({ toolState: input.toolState, error: errorBody }).catch((error) => { + log.debug(`failure error report failed: ${error}`); + }); + } + + // create_pull_request_review owns its own deletion (see mcp/review.ts), so + // progressComment is already null by the time we get here for that path. + // uses finalSummaryWritten (not todoTracker.enabled or wasUpdated) so + // cleanup survives API failures in report_progress where cancel() ran but + // the write didn't succeed, and isn't fooled by writes to *other* artifacts. + if ( + input.result.success && + input.toolState.progressComment && + !input.toolState.finalSummaryWritten + ) { + await deleteProgressComment(input.toolContext).catch((error) => { + log.debug(`stranded progress comment cleanup failed: ${error}`); + }); + } + + try { + const usageSummary = formatUsageSummary(input.toolState.usageEntries); + const body = input.toolState.lastProgressBody || input.result.output; + const parts = [body, usageSummary].filter(Boolean); + if (parts.length > 0) { + await writeSummary(parts.join("\n\n")); + } + } catch (error) { + log.debug(`job summary write failed: ${error}`); + } + + if (input.toolState.output) { + log.info(`::pullfrog-output::${Buffer.from(input.toolState.output).toString("base64")}`); + core.setOutput("result", input.toolState.output); + } +} + +/** + * Write the rendered error to the GitHub Actions job summary tab + mirror + * to the PR progress comment when one exists. Catch path only. + * + * `lastProgressBody` and the usage table are appended to the summary so the + * partial work the agent did before failing isn't lost. + */ +export async function writeRunErrorOutputs(input: { + rendered: RenderedRunError; + toolState: ToolState; +}): Promise { + try { + const usageSummary = formatUsageSummary(input.toolState.usageEntries); + const parts = [input.rendered.summary, input.toolState.lastProgressBody, usageSummary].filter( + Boolean + ); + await writeSummary(parts.join("\n\n")); + } catch {} + + try { + await reportErrorToComment({ toolState: input.toolState, error: input.rendered.comment }); + } catch { + // error reporting failed, but don't let it mask the original error + } +} diff --git a/utils/runStartupLog.ts b/utils/runStartupLog.ts new file mode 100644 index 0000000..1da9d12 --- /dev/null +++ b/utils/runStartupLog.ts @@ -0,0 +1,61 @@ +/** + * Startup log formatting for the resolver pipeline. Computes the + * "model / agent / push / shell / timeout" block that main.ts prints + * after resolving the agent + model + payload. + */ + +import { log } from "./cli.ts"; +import type { ResolvedPayload } from "./payload.ts"; +import { TIMEOUT_DISABLED } from "./time.ts"; + +function resolveTimeoutForLog(timeout: string | undefined): string { + if (!timeout) return "1h (default)"; + if (timeout === TIMEOUT_DISABLED) return "none (disabled)"; + return timeout; +} + +function resolveModelForLog(ctx: { + payload: ResolvedPayload; + resolvedModel: string | undefined; +}): string { + const envModel = process.env.PULLFROG_MODEL?.trim(); + if (envModel) return `${envModel} (override via PULLFROG_MODEL)`; + if (ctx.payload.proxyModel) return `${ctx.payload.proxyModel} (proxy)`; + if (ctx.resolvedModel && ctx.payload.model && ctx.payload.model !== ctx.resolvedModel) { + return `${ctx.resolvedModel} (resolved from ${ctx.payload.model})`; + } + if (ctx.resolvedModel) return ctx.resolvedModel; + if (ctx.payload.model) return `${ctx.payload.model} (unresolved)`; + return "auto"; +} + +function resolveAgentForLog(ctx: { agentName: string; resolvedModel: string | undefined }): string { + const envAgent = process.env.PULLFROG_AGENT?.trim(); + if (envAgent && envAgent === ctx.agentName) { + return `${ctx.agentName} (override via PULLFROG_AGENT)`; + } + if (ctx.agentName === "claude" && ctx.resolvedModel) { + return `${ctx.agentName} (auto-selected for ${ctx.resolvedModel})`; + } + return ctx.agentName; +} + +/** + * Emit the startup block ("» model / agent / push / shell / timeout") after + * the agent and model are resolved. Single side-effect; no return. + */ +export function logRunStartup(ctx: { + payload: ResolvedPayload; + resolvedModel: string | undefined; + agentName: string; +}): void { + log.info( + `» model: ${resolveModelForLog({ payload: ctx.payload, resolvedModel: ctx.resolvedModel })}` + ); + log.info( + `» agent: ${resolveAgentForLog({ agentName: ctx.agentName, resolvedModel: ctx.resolvedModel })}` + ); + log.info(`» push: ${ctx.payload.push}`); + log.info(`» shell: ${ctx.payload.shell}`); + log.info(`» timeout: ${resolveTimeoutForLog(ctx.payload.timeout)}`); +}