Skip to content

Commit 5f3e46c

Browse files
colinhackspullfrog[bot]
colinhacks
authored and
pullfrog[bot]
committed
fix: don't reuse disabled proxy key on workflow re-runs; non-fatal title-gen errors (#636)
* fix: don't reuse disabled proxy key on workflow re-runs; non-fatal title-gen errors Three small surgical fixes addressing run https://github.com/pullfrog/app/actions/runs/25580969379: 1. **`/api/proxy-token` idempotency now checks `finalizedAt`.** GitHub re-runs share the same `run_id` (only `run_attempt` increments), so attempt N+1's action calls /api/proxy-token and inherits attempt N's `proxyKeyId`. The `workflow_run.completed` webhook between attempts retires that key on OpenRouter (`disableKey`), so attempt N+1 was getting back a disabled key and OpenRouter responded with `401 User not found` on every call. Falling through when finalized routes through the same billing gate (`handleRouterBilling` balance check), so no new attack surface. 2. **OpenCode title-gen / small-model errors no longer fatal.** OpenCode auto-spawns a small `agent=title small=true` background call at session start to name the thread, defaulting to `anthropic/claude-haiku-4.5` (anomalyco/opencode#1243). Pre-fix, the wrapper's `error` event handler treated any `type=error` as fatal, so a cosmetic title failure killed the run before primary inference even started. Now: stderr matching `small=true` sets a one-shot suppression flag for the next stdout `error` event, which is logged as a warning instead. 3. **Provider-error classifier puts auth patterns above rate-limit.** OpenRouter 401 payloads bundle `x-ratelimit-*` response headers, and the loose `\brate[_ ]limit/i` pattern was winning. Added 401/403 status, `User not found`, `Invalid authentication`, `No auth credentials found` patterns ahead of rate-limit. Updated the existing 401-headers regression test to assert correct auth classification rather than `null`. * opencode: correlate small-model error suppression by message, not by next-event Pullfrog self-review on #636 flagged a real concurrency hole. OpenCode forks the title-gen call (`session/prompt.ts:1452-1457` via `Effect.forkIn(scope)`) so it races primary inference. The previous one-shot `suppressNextErrorEvent` boolean had no per-call correlation: it was consumed by whichever stdout `type=error` event landed next, regardless of which subagent produced it. Under concurrent failures, a primary-agent error landing first could be silently downgraded to a warning while the small-model error then propagated fatally — the inverse of the bug the suppression was meant to prevent. Replaced the boolean with a `Set<string>` of pending small-model error messages. stderr extracts the inner `"message":"..."` from any classified provider error tagged `small=true`; the stdout `error` handler suppresses only when `event.error.data.message` matches a pending entry. Set is capped at 32 entries so a long stream of small-model failures can't wedge memory. Also corrected the comment that referenced "session summarizer" — verified in opencode source that summarize() does NOT use `small: true`; only the title generator does today (only `small: true` match in the codebase). * revert: drop opencode title-gen suppression We have no evidence — and can't construct a realistic scenario — where title-gen fails on an otherwise-successful run. Title-gen and primary share the same OPENROUTER_API_KEY and hit the same proxy/upstream; whatever breaks one breaks the other. The original repro on run 25580969379 is fully explained by the stale proxy key (fix #1) — title-gen happened to be the first call that surfaced the auth error, but every subsequent primary call would have died the same way. Suppression code adds complexity (cross-stream correlation logic, message matching, set capping) and a real failure mode of its own (a small-model error with a unique message could mask an unrelated primary error landing shortly after). Net negative. Removing.
1 parent 3d393c3 commit 5f3e46c

2 files changed

Lines changed: 50 additions & 2 deletions

File tree

‎utils/providerErrors.test.ts‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@ describe("detectProviderError", () => {
77
expect(detectProviderError("commit f609cc89e84596ab125d60dac568bfb2ef398396 429")).toBeNull();
88
});
99

10-
it("returns null for x-ratelimit-* response headers in 401 error JSON", () => {
10+
it("classifies 401 + x-ratelimit-* headers as auth, not rate-limited", () => {
11+
// OpenRouter 401 responses bundle `x-ratelimit-*` rate-limit headers
12+
// alongside the auth error. the auth patterns must win — pre-fix this
13+
// got tagged as `rate limited` because of the loose `\brate[_ ]limit`
14+
// match against header names like `ratelimit-limit-requests`. note: in
15+
// OpenRouter's actual format the header name is `ratelimit` (one word),
16+
// but the dumped JSON sometimes contains `rate-limit` separators too.
1117
const stderr = JSON.stringify({
1218
error: { name: "APIError", statusCode: 401, message: "Invalid authentication credentials" },
1319
headers: {
@@ -16,7 +22,7 @@ describe("detectProviderError", () => {
1622
"x-ratelimit-reset-tokens": "2025-01-01T00:00:00Z",
1723
},
1824
});
19-
expect(detectProviderError(stderr)).toBeNull();
25+
expect(detectProviderError(stderr)).toBe("auth error (401)");
2026
});
2127

2228
it("returns null for INTERNAL_SERVER_ERROR substring", () => {
@@ -29,6 +35,37 @@ describe("detectProviderError", () => {
2935
});
3036
});
3137

38+
describe("auth errors", () => {
39+
it("detects 401 / 403 status codes as auth errors", () => {
40+
expect(detectProviderError('{"statusCode": 401}')).toBe("auth error (401)");
41+
expect(detectProviderError('{"statusCode": 403}')).toBe("auth error (403)");
42+
expect(detectProviderError("status_code: 401")).toBe("auth error (401)");
43+
});
44+
45+
it("detects OpenRouter 'User not found' (disabled/invalid key)", () => {
46+
// bare `"code":401` lacks a status-key prefix so the 401 status pattern
47+
// intentionally doesn't fire; the User-not-found pattern catches it.
48+
expect(detectProviderError('{"error":{"message":"User not found","code":401}}')).toBe(
49+
"auth error (invalid/disabled key)"
50+
);
51+
expect(detectProviderError("APIError: User not found.")).toBe(
52+
"auth error (invalid/disabled key)"
53+
);
54+
});
55+
56+
it("detects 'Invalid authentication' phrasing", () => {
57+
expect(detectProviderError("Invalid authentication credentials")).toBe(
58+
"auth error (invalid credentials)"
59+
);
60+
});
61+
62+
it("detects 'No auth credentials found' phrasing", () => {
63+
expect(detectProviderError("AI_APICallError: No auth credentials found")).toBe(
64+
"auth error (missing credentials)"
65+
);
66+
});
67+
});
68+
3269
describe("real provider errors", () => {
3370
it("detects 429 only when adjacent to a status key", () => {
3471
expect(detectProviderError('{"statusCode": 429}')).toBe("rate limited (429)");

‎utils/providerErrors.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,17 @@ type ProviderErrorPattern = { regex: RegExp; label: string };
66
const statusKey = `\\b(?:status[_ ]?code|http[_ ]?status|status)["']?\\s*[:=]\\s*["']?`;
77

88
const PROVIDER_ERROR_PATTERNS: ProviderErrorPattern[] = [
9+
// auth patterns must come BEFORE rate-limit patterns. OpenRouter 401 error
10+
// payloads carry `x-ratelimit-*` response headers in the dump, and the
11+
// free-form rate-limit regex below would otherwise win on word-boundary
12+
// matches inside header names. canonical 401 messages: OpenRouter returns
13+
// `{"error":{"message":"User not found","code":401}}` for disabled or
14+
// invalid keys (https://openai.luzhipeng.com/docs/api/reference/errors-and-debugging).
15+
{ regex: new RegExp(`${statusKey}401\\b`, "i"), label: "auth error (401)" },
16+
{ regex: new RegExp(`${statusKey}403\\b`, "i"), label: "auth error (403)" },
17+
{ regex: /\bUser not found\b/i, label: "auth error (invalid/disabled key)" },
18+
{ regex: /\bInvalid authentication\b/i, label: "auth error (invalid credentials)" },
19+
{ regex: /\bNo auth credentials found\b/i, label: "auth error (missing credentials)" },
920
{ regex: new RegExp(`${statusKey}429\\b`, "i"), label: "rate limited (429)" },
1021
{ regex: new RegExp(`${statusKey}500\\b`, "i"), label: "provider 500 error" },
1122
{ regex: new RegExp(`${statusKey}503\\b`, "i"), label: "provider unavailable (503)" },

0 commit comments

Comments
 (0)