fe2746198c
* fix: 6 unaddressed log-audit / run-audit findings - #836 + #818 (clerk middleware SyntaxError on action-runtime endpoints): narrow proxy.ts matcher to exclude /api/repo/<owner>/<repo>/run-context, /api/runtime/*, /api/proxy-token. these are server-to-server with their own auth and have no Clerk session to evaluate, so clerkMiddleware's decodeJwt throws turn into 500s on every request. - #837 (npx EBADDEVENGINES on customer's package.json): change runCli's bootstrap cwd from $GITHUB_WORKSPACE to os.tmpdir() so npm v11+ doesn't enforce devEngines.packageManager from the customer's tree before our bootstrap can install pullfrog. CLI process.chdir's to payload.cwd internally, so the runtime work still happens in $GITHUB_WORKSPACE. - #838 (createWorkflowDispatch silently dropped 39 user runs on a 5xx spike): add bounded retry (3 attempts, ~750ms total) on Octokit 5xx and network errors inside dispatchReservedRun. preserves the existing 422 "Unexpected inputs" path. retry budget stays well under GitHub's 10s webhook redelivery window. - #833 (bail() redirect("/signout") propagated NEXT_REDIRECT into webhook handlers, 40+ 500s/24h): drop the redirect side-effect; bail now just classifies bad-credentials as non-retryable and propagates. UI flows that wanted auto-signout on revoked tokens can detect it themselves; the side-effect was wrong for any non-page caller. - #835 (BYOK provider billing-exhausted fell through to raw error renderer, 37 review-mode runs/24h with no PR-side signal): - extend providerErrors patterns with Anthropic "credit balance is too low" + extract isProviderBillingExhausted / extractProviderId helpers - add a renderer branch in runErrorRenderer.ts that names the provider (parsed from providerID=) and links to its billing dashboard - route handleAgentResult's !result.success path through the renderer + reportErrorToComment with createIfMissing, so review-mode and silent triggers get an actionable PR comment regardless of mode - #834 (post-hook ERR_MODULE_NOT_FOUND already fixed on main, hardening): - add a vitest invariant that walks the entryPost.ts import graph and refuses any non-relative / non-node: specifier — catches the next `@actions/core` slip-up before publish - add an analyze-logs classifier so future entryPost crashes surface as failure:post-hook-module-not-found instead of hiding inside failure:unknown / failure:git-lock-file Co-authored-by: Cursor <cursoragent@cursor.com> * anneal: fix dead-code matcher, 404 url, silent-trigger gate, and grammar regression Round-1 anneal pass on the audit-fixes PR surfaced two critical issues + several majors that the original fixes shipped with: - proxy.ts matcher 1 (`(?!_next|[^?]*\.(...))`) still caught every /api/... route because matcher arrays are OR'd. The narrowing in matcher 2 was dead code → middleware still 500'd on /api/runtime/*, /api/proxy-token, /api/repo/.../run-context. Carve-out now lives in BOTH matchers. - opencode.ai/billing returns 404; canonical top-up surface is /zen. deepseek /usage is the consumption page, not the top-up flow (/top_up is correct). google /apikey is the keys list, not billing (/usage is the spend dashboard). All three URL strings updated. - handleAgentResult gated reportErrorToComment behind `if (!ctx.silent)`, contradicting the createIfMissing intent — silent IncrementalReview / pull_request_synchronize / auto-label still got zero PR signal on BYOK billing exhaustion, the exact failure mode #835 was meant to fix. Moved createIfMissing into finalizeSuccessRun's existing render-and- post block (single source of truth), reverted handleAgentResult to its prior shape. Side benefit: drops the double-PATCH that fired on every non-silent !success path with an existing progress comment. - Anthropic-direct error rendered "**Your your provider account is out of credit.**" because Anthropic SDK has no providerID= tag, so extractProviderId returned null and the headline composed "Your " + "your provider". Added detectProviderId Anthropic fallback (matches "Anthropic API" / "credit balance is too low") so the link is reachable AND made the headline conditional on whether a provider id was detected. - Reordered renderRunError classifier: BYOK billing-exhausted now runs BEFORE api-key auth detection. Providers commonly return 401 for billing exhaustion (DeepSeek, Gemini), and the OpenCode harness logs often include "API Error: 401" in the raw error body, which isApiKeyAuthError would otherwise match — surfacing "rotate your key" when the actual fix is "top up credits". - isTransientUpstreamError missed ENOTFOUND / ENETUNREACH / EHOSTUNREACH (undici DNS-class failures Octokit doesn't wrap with a status). Added to the prefix alternation. - Tightened "10s webhook redelivery budget" / "GitHub redelivers" wording in triggerWorkflow.ts and bail.ts JSDoc — GitHub's 10s is the response timeout (it doesn't auto-redeliver); upstream webhook proxy retries are what multiplied the failure. Co-authored-by: Cursor <cursoragent@cursor.com> * anneal r2: unbreak proxy.ts matcher 2, extend carve-out, mkdtemp bootstrap Round-2 anneal (security + cross-cutting lenses) on top of the round-1 audit-fixes commit caught a critical regression + several majors: - proxy.ts matcher 2 from r1 (`/(api|trpc)(?!...)(.*)`) does NOT compile. path-to-regexp rejects a top-level `(?!` after `)` as "Pattern cannot start with '?'", and Next.js's SourceSchema runs the same validator at build time and aborts via `process.exit(1)`. PR #840's Vercel + preview deployments have been failing since 978eca26 for exactly this reason. Fixed by nesting the lookahead inside an outer parameter group: `/(api|trpc)((?!...).*)`. Same shape matcher 1 already uses, which is why m1 always compiled. Verified `pnpm next build` succeeds end-to-end. - proxy.ts carve-out was incomplete relative to its own justification. The "no Clerk session, decodeJwt 500" failure mode applies to ALL server-to-server action-runtime endpoints — five more share the exact shape: /api/repo/.../learnings, /api/repo/.../pr/.../summary-comment, /api/repo/.../issue/.../plan-comment, /api/workflow-run/, and /api/github/installation-token. Extended both matchers. /api/upload/ signed-url stays in (dual auth: Clerk session OR bearer JWT — needs middleware for the user path). - proxy.ts carve-outs were unanchored: a future /api/proxy-token-info, /api/proxy-tokens, or /api/repo/X/Y/run-context-foo would silently bypass Clerk. Added `(?:$|/)` for path-prefix carve-outs (allow exact match or sub-path), `$` for routes with browser-callable siblings (e.g. `learnings/history` is browser-Clerk, `learnings$` is action- bearer-JWT). Verified 30/31 routes via path-to-regexp test harness. - runCli.ts cwd flipped from $GITHUB_WORKSPACE to os.tmpdir() in r1 (#837 fix for npm v11 devEngines.packageManager EBADDEVENGINES). But $TMPDIR is overridable from a prior $GITHUB_ENV step — a customer- authored or compromised prior step can plant /atk/node_modules/ pullfrog/ and `echo "TMPDIR=/atk" >> $GITHUB_ENV`, and our npx --yes pullfrog@<v> bootstrap resolves the local install first, executing attacker code with full action env (provider keys, OIDC, installation token, CODEX_AUTH_JSON). Switched to mkdtempSync(join (tmpdir(), "pullfrog-bootstrap-")) — fresh per-invocation 0700 dir, not pre-writable by anything earlier in the job. - runLifecycle.ts: writeRunErrorOutputs (catch-path) didn't pass createIfMissing: true, contradicting the symmetric intent of the r1 finalizeSuccessRun fix. Silent triggers (IncrementalReview / pull_request_synchronize / auto-label) that throw past the success path still got zero PR signal — exact failure mode #835 was meant to close. Now both paths pass createIfMissing: true. - runErrorRenderer.ts detectProviderId regex `/Anthropic API|credit balance is too low/i` could mis-tag a non-Anthropic billing-exhausted error that mentioned "Anthropic API" in passing (fallback-chain agent prompt text, OpenCode harness logs). Tightened to /credit balance is too low/i — Anthropic-specific phrasing, sufficient for the direct- Anthropic SDK case the fallback exists to handle. - runErrorRenderer.ts JSDoc: r1's classifier reorder put hang at #6 in code but the JSDoc still listed it at #2. Reordered the doc to match dispatch order, with explicit note that hang is a sub-source for the api-key check (which is why hangBody is precomputed early). - analyze-logs.ts: ERR_MODULE_NOT_FOUND.*entryPost regex needs `s` flag so it survives Node v23+ stack-trace reformatting onto multiple lines. One-char fix to defend the #834 classification bucket. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
173 lines
8.4 KiB
TypeScript
173 lines
8.4 KiB
TypeScript
type ProviderErrorPattern = { regex: RegExp; label: string };
|
||
|
||
/** Stable label for the BYOK provider-billing-exhausted classification. */
|
||
export const PROVIDER_BILLING_EXHAUSTED_LABEL = "provider billing exhausted";
|
||
|
||
// status codes are only treated as provider errors when they are adjacent to
|
||
// a recognised status key. this rejects commit SHAs that happen to contain
|
||
// "429", version strings, file hashes, etc.
|
||
const statusKey = `\\b(?:status[_ ]?code|http[_ ]?status|status)["']?\\s*[:=]\\s*["']?`;
|
||
|
||
const PROVIDER_ERROR_PATTERNS: ProviderErrorPattern[] = [
|
||
// billing-payload patterns come BEFORE bare status-code patterns. providers
|
||
// commonly return 401 / 429 for billing/quota exhaustion (OpenCode Zen
|
||
// `CreditsError` / `FreeUsageLimitError`, Gemini `RESOURCE_EXHAUSTED` +
|
||
// "spending cap", Anthropic "Insufficient balance" / "credit balance is
|
||
// too low"). these are non-retryable and require user-billing action —
|
||
// distinct from a transient auth error or rate-limit. status-code patterns
|
||
// would otherwise win and surface "auth error (401)" / "rate limited (429)"
|
||
// with no billing hint. see #778, #835.
|
||
{ regex: /\bCreditsError\b/, label: PROVIDER_BILLING_EXHAUSTED_LABEL },
|
||
{ regex: /\bFreeUsageLimitError\b/, label: PROVIDER_BILLING_EXHAUSTED_LABEL },
|
||
{ regex: /Insufficient balance/i, label: PROVIDER_BILLING_EXHAUSTED_LABEL },
|
||
{ regex: /credit balance is too low/i, label: PROVIDER_BILLING_EXHAUSTED_LABEL },
|
||
{ regex: /spending cap/i, label: PROVIDER_BILLING_EXHAUSTED_LABEL },
|
||
// auth patterns must come BEFORE rate-limit patterns. OpenRouter 401 error
|
||
// payloads carry `x-ratelimit-*` response headers in the dump, and the
|
||
// free-form rate-limit regex below would otherwise win on word-boundary
|
||
// matches inside header names. canonical 401 messages: OpenRouter returns
|
||
// `{"error":{"message":"User not found","code":401}}` for disabled or
|
||
// invalid keys (https://openai.luzhipeng.com/docs/api/reference/errors-and-debugging).
|
||
{ regex: new RegExp(`${statusKey}401\\b`, "i"), label: "auth error (401)" },
|
||
{ regex: new RegExp(`${statusKey}403\\b`, "i"), label: "auth error (403)" },
|
||
{ regex: /\bUser not found\b/i, label: "auth error (invalid/disabled key)" },
|
||
{ regex: /\bInvalid authentication\b/i, label: "auth error (invalid credentials)" },
|
||
{ regex: /\bNo auth credentials found\b/i, label: "auth error (missing credentials)" },
|
||
{ regex: new RegExp(`${statusKey}429\\b`, "i"), label: "rate limited (429)" },
|
||
{ regex: new RegExp(`${statusKey}500\\b`, "i"), label: "provider 500 error" },
|
||
{ regex: new RegExp(`${statusKey}503\\b`, "i"), label: "provider unavailable (503)" },
|
||
// matches `rate limit`, `rate limited`, `rate limits exceeded`,
|
||
// `rate_limit_error`, `rate_limit_exceeded`. the leading `\b` + `[_ ]`
|
||
// separator rejects `x-ratelimit-*` / `anthropic-ratelimit-*` response
|
||
// headers (no separator between "rate" and "limit") which routinely
|
||
// appear in dumped 401 / 4xx error JSON.
|
||
{ regex: /\brate[_ ]limit/i, label: "rate limited" },
|
||
{ regex: /\bRESOURCE_EXHAUSTED\b/, label: "quota exhausted" },
|
||
// Google gRPC `INTERNAL` status. word-boundary anchors reject
|
||
// `INTERNAL_SERVER_ERROR` (HTTP 500 message that may appear in unrelated
|
||
// log lines) and identifiers like `INTERNALS`.
|
||
{ regex: /\bINTERNAL\b/, label: "provider internal error" },
|
||
{ regex: /\bUNAVAILABLE\b/, label: "provider unavailable" },
|
||
// matches `quota`, `insufficient_quota`, `quota_exceeded`, `quotaExceeded`.
|
||
// word-character lookarounds would reject `_quota` / `quotaX`; `quota` is
|
||
// specific enough that a plain substring match is safe.
|
||
{ regex: /quota/i, label: "quota error" },
|
||
// explicit zero-quota response, e.g. `{"limit": 0}`. the `\b` anchor
|
||
// around `limit` rejects keys like `time_limit` or `field_limit`.
|
||
{ regex: /["']?\blimit\b["']?\s*:\s*0\b/, label: "zero quota" },
|
||
];
|
||
|
||
/**
|
||
* Result of a provider-error scan: the classification label plus a
|
||
* human-readable excerpt centered on the matched line. The excerpt is what
|
||
* gets surfaced in `» provider error detected (...)` log lines — see
|
||
* `extractExcerpt` for the windowing/byte-cap policy.
|
||
*/
|
||
export type ProviderErrorMatch = {
|
||
label: string;
|
||
excerpt: string;
|
||
};
|
||
|
||
// roughly half a wide terminal line by 4–5 lines of context; large enough
|
||
// to capture a structured error payload (request id, retry-after, model)
|
||
// plus its immediate stack/headers, small enough to not flood the log.
|
||
const EXCERPT_MAX_BYTES = 600;
|
||
const LINES_BEFORE = 1;
|
||
const LINES_AFTER = 2;
|
||
|
||
export function findProviderErrorMatch(text: string): ProviderErrorMatch | null {
|
||
for (const entry of PROVIDER_ERROR_PATTERNS) {
|
||
const m = entry.regex.exec(text);
|
||
if (!m) continue;
|
||
return { label: entry.label, excerpt: extractExcerpt(text, m.index) };
|
||
}
|
||
return null;
|
||
}
|
||
|
||
export function detectProviderError(text: string): string | null {
|
||
return findProviderErrorMatch(text)?.label ?? null;
|
||
}
|
||
|
||
/**
|
||
* Slice a context window around `matchIndex`: the matched line plus
|
||
* `LINES_BEFORE`/`LINES_AFTER` neighbours. If the windowed slice exceeds
|
||
* `EXCERPT_MAX_BYTES` (giant adjacent lines, e.g. JSON tool-schema dumps),
|
||
* fall back to the matched line alone, head-truncated if still too long.
|
||
* Replaces the old `chunk.substring(0, 500)` head-anchored excerpt which
|
||
* surfaced whatever happened to be at the front of the stderr buffer
|
||
* instead of the error itself. See issue #703.
|
||
*/
|
||
function extractExcerpt(text: string, matchIndex: number): string {
|
||
const lineStart = text.lastIndexOf("\n", matchIndex - 1) + 1;
|
||
const lineEndRaw = text.indexOf("\n", matchIndex);
|
||
const lineEnd = lineEndRaw === -1 ? text.length : lineEndRaw;
|
||
|
||
let start = lineStart;
|
||
for (let i = 0; i < LINES_BEFORE && start > 0; i++) {
|
||
const prev = text.lastIndexOf("\n", start - 2);
|
||
start = prev < 0 ? 0 : prev + 1;
|
||
}
|
||
|
||
let end = lineEnd;
|
||
for (let i = 0; i < LINES_AFTER && end < text.length; i++) {
|
||
const next = text.indexOf("\n", end + 1);
|
||
end = next < 0 ? text.length : next;
|
||
}
|
||
|
||
let excerpt = text.slice(start, end);
|
||
if (excerpt.length > EXCERPT_MAX_BYTES) {
|
||
excerpt = text.slice(lineStart, lineEnd);
|
||
if (excerpt.length > EXCERPT_MAX_BYTES) excerpt = excerpt.slice(0, EXCERPT_MAX_BYTES);
|
||
}
|
||
return excerpt.trim();
|
||
}
|
||
|
||
/**
|
||
* OpenRouter's response when the per-run key's remaining budget can't cover
|
||
* the agent's `max_tokens` reservation. Distinct from a generic provider error
|
||
* because it's a Pullfrog billing concern, not an upstream outage — the user's
|
||
* Router wallet ran out (or the key budget was undersized at mint time and the
|
||
* agent ran out of headroom partway through).
|
||
*
|
||
* Match must be specific to this exact OpenRouter error class. Generic "credits"
|
||
* or "limit" text shows up in unrelated errors and would mis-classify them.
|
||
*
|
||
* Sample:
|
||
* `APIError: This request requires more credits, or fewer max_tokens.
|
||
* You requested up to 32000 tokens, but can only afford 22800.`
|
||
*/
|
||
// `/s` (dotAll) lets `.*?` cross newlines so we still detect the error if any
|
||
// upstream layer reformats the message onto multiple lines. Without it, a
|
||
// single inserted `\n` would silently bypass the BillingError reclassification
|
||
// and the user would see the generic `❌ Pullfrog failed` dump instead of the
|
||
// actionable top-up CTA.
|
||
const ROUTER_KEYLIMIT_EXHAUSTED_PATTERN =
|
||
/requires more credits.*?fewer max_tokens|requested up to \d+ tokens.*?can only afford/is;
|
||
|
||
export function isRouterKeylimitExhaustedError(text: string): boolean {
|
||
return ROUTER_KEYLIMIT_EXHAUSTED_PATTERN.test(text);
|
||
}
|
||
|
||
/**
|
||
* BYOK billing-exhausted: provider rejected the request because the user's
|
||
* provider wallet is empty (DeepSeek "Insufficient Balance", Anthropic
|
||
* "credit balance is too low", OpenCode Zen `CreditsError` /
|
||
* `FreeUsageLimitError`, Gemini "spending cap"). Distinct from
|
||
* `isRouterKeylimitExhaustedError` — that's Pullfrog's Router wallet, this
|
||
* is the user's own provider account.
|
||
*/
|
||
export function isProviderBillingExhausted(text: string): boolean {
|
||
return findProviderErrorMatch(text)?.label === PROVIDER_BILLING_EXHAUSTED_LABEL;
|
||
}
|
||
|
||
/**
|
||
* Extract `providerID=foo` from agent error logs (OpenCode emits this on
|
||
* `provider error detected (...)` lines). Returns the lowercase provider
|
||
* slug, or null when absent. Used to render a provider-specific dashboard
|
||
* link in the BYOK billing-exhausted summary.
|
||
*/
|
||
export function extractProviderId(text: string): string | null {
|
||
const match = text.match(/\bproviderID=([a-z0-9_-]+)/i);
|
||
return match ? match[1].toLowerCase() : null;
|
||
}
|