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>
314 lines
14 KiB
TypeScript
314 lines
14 KiB
TypeScript
import {
|
|
detectProviderError,
|
|
extractProviderId,
|
|
findProviderErrorMatch,
|
|
isProviderBillingExhausted,
|
|
isRouterKeylimitExhaustedError,
|
|
} from "./providerErrors.ts";
|
|
|
|
describe("detectProviderError", () => {
|
|
describe("false positives previously seen in production", () => {
|
|
it("returns null for commit SHAs containing 429", () => {
|
|
expect(detectProviderError("hash=7a46d89f505b36df49b4f54429daffa1a9459b11")).toBeNull();
|
|
expect(detectProviderError("commit f609cc89e84596ab125d60dac568bfb2ef398396 429")).toBeNull();
|
|
});
|
|
|
|
it("classifies 401 + x-ratelimit-* headers as auth, not rate-limited", () => {
|
|
// OpenRouter 401 responses bundle `x-ratelimit-*` rate-limit headers
|
|
// alongside the auth error. the auth patterns must win — pre-fix this
|
|
// got tagged as `rate limited` because of the loose `\brate[_ ]limit`
|
|
// match against header names like `ratelimit-limit-requests`. note: in
|
|
// OpenRouter's actual format the header name is `ratelimit` (one word),
|
|
// but the dumped JSON sometimes contains `rate-limit` separators too.
|
|
const stderr = JSON.stringify({
|
|
error: { name: "APIError", statusCode: 401, message: "Invalid authentication credentials" },
|
|
headers: {
|
|
"x-ratelimit-limit-requests": 50,
|
|
"x-ratelimit-remaining-requests": 49,
|
|
"x-ratelimit-reset-tokens": "2025-01-01T00:00:00Z",
|
|
},
|
|
});
|
|
expect(detectProviderError(stderr)).toBe("auth error (401)");
|
|
});
|
|
|
|
it("returns null for INTERNAL_SERVER_ERROR substring", () => {
|
|
expect(detectProviderError("HTTP/1.1 500 INTERNAL_SERVER_ERROR")).toBeNull();
|
|
expect(detectProviderError("expected: not INTERNAL_SERVER_ERROR")).toBeNull();
|
|
});
|
|
|
|
it("returns null for INTERNALS substring", () => {
|
|
expect(detectProviderError("debugging INTERNALS of the parser")).toBeNull();
|
|
});
|
|
});
|
|
|
|
describe("auth errors", () => {
|
|
it("detects 401 / 403 status codes as auth errors", () => {
|
|
expect(detectProviderError('{"statusCode": 401}')).toBe("auth error (401)");
|
|
expect(detectProviderError('{"statusCode": 403}')).toBe("auth error (403)");
|
|
expect(detectProviderError("status_code: 401")).toBe("auth error (401)");
|
|
});
|
|
|
|
it("detects OpenRouter 'User not found' (disabled/invalid key)", () => {
|
|
// bare `"code":401` lacks a status-key prefix so the 401 status pattern
|
|
// intentionally doesn't fire; the User-not-found pattern catches it.
|
|
expect(detectProviderError('{"error":{"message":"User not found","code":401}}')).toBe(
|
|
"auth error (invalid/disabled key)"
|
|
);
|
|
expect(detectProviderError("APIError: User not found.")).toBe(
|
|
"auth error (invalid/disabled key)"
|
|
);
|
|
});
|
|
|
|
it("detects 'Invalid authentication' phrasing", () => {
|
|
expect(detectProviderError("Invalid authentication credentials")).toBe(
|
|
"auth error (invalid credentials)"
|
|
);
|
|
});
|
|
|
|
it("detects 'No auth credentials found' phrasing", () => {
|
|
expect(detectProviderError("AI_APICallError: No auth credentials found")).toBe(
|
|
"auth error (missing credentials)"
|
|
);
|
|
});
|
|
});
|
|
|
|
describe("billing exhaustion", () => {
|
|
// see #778 — providers return 401 / 429 for billing/quota exhaustion
|
|
// (OpenCode Zen `CreditsError` / `FreeUsageLimitError`, Gemini
|
|
// `RESOURCE_EXHAUSTED` + spending cap, "Insufficient balance"). these
|
|
// are non-retryable; status-code patterns must NOT win and surface the
|
|
// misleading "auth error (401)" / "rate limited (429)" labels.
|
|
it("classifies OpenCode Zen CreditsError as billing exhausted, not 401", () => {
|
|
const stderr = JSON.stringify({
|
|
statusCode: 401,
|
|
responseBody:
|
|
'{"type":"error","error":{"type":"CreditsError","message":"Insufficient balance. Manage your billing here: https://opencode.ai/workspace/x/billing"}}',
|
|
});
|
|
expect(detectProviderError(stderr)).toBe("provider billing exhausted");
|
|
});
|
|
|
|
it("classifies OpenCode Zen FreeUsageLimitError as billing exhausted, not 429", () => {
|
|
const stderr = JSON.stringify({
|
|
statusCode: 429,
|
|
responseBody:
|
|
'{"type":"error","error":{"type":"FreeUsageLimitError","message":"Rate limit exceeded. Please try again later."}}',
|
|
});
|
|
expect(detectProviderError(stderr)).toBe("provider billing exhausted");
|
|
});
|
|
|
|
it("classifies Gemini spending-cap RESOURCE_EXHAUSTED as billing exhausted, not 429", () => {
|
|
const stderr =
|
|
'statusCode: 429, body: {"code": 429, "status": "RESOURCE_EXHAUSTED", "message": "Your project has exceeded its monthly spending cap..."}';
|
|
expect(detectProviderError(stderr)).toBe("provider billing exhausted");
|
|
});
|
|
|
|
it("classifies bare 'Insufficient balance' as billing exhausted", () => {
|
|
expect(detectProviderError("error: Insufficient balance")).toBe("provider billing exhausted");
|
|
});
|
|
|
|
it("classifies Anthropic 'credit balance is too low' as billing exhausted (#835)", () => {
|
|
// Anthropic-direct BYOK returns this string verbatim when the user's
|
|
// Anthropic console credit balance can't cover the request. distinct
|
|
// wording from "Insufficient balance" used by DeepSeek / OpenCode Zen.
|
|
const stderr =
|
|
"APIError: 400 Your credit balance is too low to access the Anthropic API. Please go to Plans & Billing to upgrade or purchase credits.";
|
|
expect(detectProviderError(stderr)).toBe("provider billing exhausted");
|
|
});
|
|
});
|
|
|
|
describe("real provider errors", () => {
|
|
it("detects 429 only when adjacent to a status key", () => {
|
|
expect(detectProviderError('{"statusCode": 429}')).toBe("rate limited (429)");
|
|
expect(detectProviderError('{"status_code": 429, "message": "..."}')).toBe(
|
|
"rate limited (429)"
|
|
);
|
|
expect(detectProviderError("http_status: 429")).toBe("rate limited (429)");
|
|
expect(detectProviderError("status=429")).toBe("rate limited (429)");
|
|
});
|
|
|
|
it("detects rate_limit_error and rate_limit_exceeded", () => {
|
|
expect(detectProviderError('{"type":"rate_limit_error"}')).toBe("rate limited");
|
|
expect(detectProviderError("rate_limit_exceeded")).toBe("rate limited");
|
|
expect(detectProviderError("plain rate limit reached")).toBe("rate limited");
|
|
});
|
|
|
|
it("detects rate-limit phrasing with trailing inflection", () => {
|
|
expect(detectProviderError("Error: rate limited by provider")).toBe("rate limited");
|
|
expect(detectProviderError("rate limits exceeded for this key")).toBe("rate limited");
|
|
});
|
|
|
|
it("detects RESOURCE_EXHAUSTED", () => {
|
|
expect(detectProviderError('"status": "RESOURCE_EXHAUSTED"')).toBe("quota exhausted");
|
|
});
|
|
|
|
it("detects gRPC INTERNAL status as a whole word", () => {
|
|
expect(detectProviderError('"status": "INTERNAL"')).toBe("provider internal error");
|
|
});
|
|
|
|
it("detects UNAVAILABLE as a whole word", () => {
|
|
expect(detectProviderError('"status": "UNAVAILABLE"')).toBe("provider unavailable");
|
|
});
|
|
|
|
it("detects 500 / 503 only when adjacent to a status key", () => {
|
|
expect(detectProviderError('"statusCode": 500')).toBe("provider 500 error");
|
|
expect(detectProviderError('"statusCode": 503')).toBe("provider unavailable (503)");
|
|
expect(detectProviderError("v1.503.0 release notes")).toBeNull();
|
|
});
|
|
|
|
it("detects quota and zero-quota responses", () => {
|
|
expect(detectProviderError('"message": "quota exceeded"')).toBe("quota error");
|
|
expect(detectProviderError('{"code":"insufficient_quota"}')).toBe("quota error");
|
|
expect(detectProviderError('"error":"quota_exceeded"')).toBe("quota error");
|
|
expect(detectProviderError('{"reason":"quotaExceeded"}')).toBe("quota error");
|
|
expect(detectProviderError('{"limit": 0, "remaining": 0}')).toBe("zero quota");
|
|
expect(detectProviderError('"time_limit": 0')).toBeNull();
|
|
});
|
|
});
|
|
});
|
|
|
|
describe("findProviderErrorMatch", () => {
|
|
// regression for issue #703: when stderr arrives as a multi-KB buffer
|
|
// (mcp tool-schema dump + the actual error message), the old
|
|
// `chunk.substring(0, 500)` excerpt showed the head of the buffer
|
|
// (schema) instead of the matched error text. the windowed excerpt
|
|
// must center on the matched line.
|
|
it("excerpt centers on the matched line, not the head of the buffer", () => {
|
|
const schemaDump =
|
|
"{".repeat(2000) +
|
|
'"name":"pullfrog_create_pull_request_review","description":"Submit a review..."';
|
|
const errorLine = "ERROR 2026-05-13 service=session error=rate_limit_exceeded retry-after=30";
|
|
const chunk = `${schemaDump}\n${errorLine}\ncaller stack at handler.ts:42`;
|
|
|
|
const match = findProviderErrorMatch(chunk);
|
|
expect(match).not.toBeNull();
|
|
expect(match?.label).toBe("rate limited");
|
|
expect(match?.excerpt).toContain("rate_limit_exceeded");
|
|
expect(match?.excerpt).toContain("retry-after=30");
|
|
expect(match?.excerpt).not.toContain("pullfrog_create_pull_request_review");
|
|
});
|
|
|
|
it("includes a small surrounding-line window for stack-trace context", () => {
|
|
const chunk =
|
|
"» about to call session.processor\n" +
|
|
"ERROR rate_limit_exceeded for key=abc\n" +
|
|
"at handler.ts:42\n" +
|
|
"at runtime.ts:88";
|
|
const match = findProviderErrorMatch(chunk);
|
|
expect(match?.excerpt).toContain("about to call session.processor");
|
|
expect(match?.excerpt).toContain("rate_limit_exceeded");
|
|
expect(match?.excerpt).toContain("handler.ts:42");
|
|
expect(match?.excerpt).toContain("runtime.ts:88");
|
|
});
|
|
|
|
it("falls back to the matched line alone when adjacent lines are huge", () => {
|
|
const giantPrefix = "x".repeat(5000);
|
|
const errorLine = '"statusCode": 429, "message": "slow down"';
|
|
const giantSuffix = "y".repeat(5000);
|
|
const chunk = `${giantPrefix}\n${errorLine}\n${giantSuffix}`;
|
|
|
|
const match = findProviderErrorMatch(chunk);
|
|
expect(match?.label).toBe("rate limited (429)");
|
|
expect(match?.excerpt).toBe(errorLine);
|
|
});
|
|
|
|
it("head-truncates the matched line if it alone exceeds the byte cap", () => {
|
|
const padding = "z".repeat(700);
|
|
const chunk = `${padding} "statusCode": 429 ${padding}`;
|
|
const match = findProviderErrorMatch(chunk);
|
|
expect(match?.label).toBe("rate limited (429)");
|
|
expect(match?.excerpt.length).toBeLessThanOrEqual(600);
|
|
});
|
|
|
|
it("returns null when no pattern matches", () => {
|
|
expect(findProviderErrorMatch("just some normal log line\nnothing wrong here")).toBeNull();
|
|
});
|
|
});
|
|
|
|
describe("isProviderBillingExhausted (#835)", () => {
|
|
it("matches DeepSeek 'Insufficient Balance' payloads", () => {
|
|
expect(isProviderBillingExhausted("AI_APICallError: Insufficient Balance")).toBe(true);
|
|
});
|
|
|
|
it("matches Anthropic 'credit balance is too low' payloads", () => {
|
|
expect(
|
|
isProviderBillingExhausted("Your credit balance is too low to access the Anthropic API")
|
|
).toBe(true);
|
|
});
|
|
|
|
it("matches OpenCode Zen CreditsError / FreeUsageLimitError", () => {
|
|
expect(isProviderBillingExhausted("CreditsError: out of credit")).toBe(true);
|
|
expect(isProviderBillingExhausted("FreeUsageLimitError: limit hit")).toBe(true);
|
|
});
|
|
|
|
it("returns false for unrelated provider errors", () => {
|
|
expect(isProviderBillingExhausted('{"statusCode": 401}')).toBe(false);
|
|
expect(isProviderBillingExhausted("rate_limit_exceeded")).toBe(false);
|
|
expect(isProviderBillingExhausted("just some log noise")).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("extractProviderId", () => {
|
|
it("parses providerID= from OpenCode harness logs", () => {
|
|
expect(
|
|
extractProviderId(
|
|
'ERROR providerID=deepseek modelID=deepseek-v4-pro error={"name":"AI_APICallError"}'
|
|
)
|
|
).toBe("deepseek");
|
|
});
|
|
|
|
it("lowercases the captured slug", () => {
|
|
expect(extractProviderId("providerID=Anthropic modelID=claude")).toBe("anthropic");
|
|
});
|
|
|
|
it("returns null when providerID is absent", () => {
|
|
expect(extractProviderId("APIError: Insufficient Balance")).toBeNull();
|
|
});
|
|
});
|
|
|
|
describe("isRouterKeylimitExhaustedError", () => {
|
|
it("matches the canonical OpenRouter mid-run error", () => {
|
|
expect(
|
|
isRouterKeylimitExhaustedError(
|
|
"APIError: This request requires more credits, or fewer max_tokens. " +
|
|
"You requested up to 32000 tokens, but can only afford 22800. " +
|
|
"To increase, visit https://openrouter.ai/settings/keys and create a key with a higher total limit"
|
|
)
|
|
).toBe(true);
|
|
});
|
|
|
|
it("matches the 'requires more credits' phrasing on its own", () => {
|
|
expect(
|
|
isRouterKeylimitExhaustedError("This request requires more credits, or fewer max_tokens.")
|
|
).toBe(true);
|
|
});
|
|
|
|
it("matches the 'requested up to ... can only afford' phrasing on its own", () => {
|
|
expect(
|
|
isRouterKeylimitExhaustedError("You requested up to 8000 tokens but can only afford 1234")
|
|
).toBe(true);
|
|
});
|
|
|
|
it("does not match generic out-of-credit text", () => {
|
|
expect(isRouterKeylimitExhaustedError("Your account has insufficient credits")).toBe(false);
|
|
expect(isRouterKeylimitExhaustedError("rate_limit_exceeded")).toBe(false);
|
|
expect(isRouterKeylimitExhaustedError('{"limit": 0}')).toBe(false);
|
|
});
|
|
|
|
it("does not match unrelated mentions of max_tokens", () => {
|
|
expect(isRouterKeylimitExhaustedError("max_tokens parameter must be a positive integer")).toBe(
|
|
false
|
|
);
|
|
});
|
|
|
|
it("matches across newlines (defends against upstream wrapping/reformatting)", () => {
|
|
expect(
|
|
isRouterKeylimitExhaustedError(
|
|
"APIError: This request requires more credits, or\nfewer max_tokens. You requested up to 32000 tokens"
|
|
)
|
|
).toBe(true);
|
|
expect(
|
|
isRouterKeylimitExhaustedError("You requested up to 32000 tokens,\nbut can only afford 22800")
|
|
).toBe(true);
|
|
});
|
|
});
|