From ec43c0e0d1bd34e49ea0aaeca9c7dd56f0afe68b Mon Sep 17 00:00:00 2001 From: Colin McDonnell Date: Fri, 8 May 2026 21:02:38 +0000 Subject: [PATCH] router: fix bugs from PR #616 review (#625) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three real defects flagged in the post-merge review of #616, plus one cheap hardening: 1. OpenCode `limit.output` override was a silent no-op on opencode-ai@1.1.56. Top-level `limit.output` has no read site in OpenCode (verified against the v1.1.56 source: `OUTPUT_TOKEN_MAX = Flag.OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX || 32_000` in session/llm.ts; per-model `model.limit.output` has its own scope). Plumbed via `OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX=5000` env var on the OpenCode spawn instead. Drops dead `OpenCodeConfig.limit?` type field and the corresponding config write in `buildSecurityConfig`. This was the headline mechanism of #616 — without the env var, the upfront `max_tokens` reservation stayed at 32_000 and low-wallet runs continued failing the way #616 was supposed to prevent. 2. Phantom auto-reload buffer for detached-card accounts. DELETE /payment-method clears `stripeCustomerId` but leaves `autoReloadEnabled` intact, so an account with welcome-credit residue and a detached card could mint a key with `keyLimitCents = balance + autoReloadAmountCents` ($50 default, schema-cap $100K) of free spend headroom we have no way to bill. Conjunctive `account.autoReloadEnabled && hasCard` in the buffer selection closes this. Defense-in-depth follow-up worth doing: clear `autoReloadEnabled` in the card-detach handler. 3. The autoReloadEnabled 402 branch fired for phase-1 noop paths (`!stripeCustomerId`, `reloadAmountCents < 50`, `balance >= threshold`) where `result.failure == null`, returning `"insufficient balance"` with no actionable code. Gated on `result.status === "failed"` so non-charge paths fall through to the `hasCard` / no-card branches and emit `router_balance_exhausted` / `router_requires_card` instead. 4. (cheap) `ROUTER_KEYLIMIT_EXHAUSTED_PATTERN` now uses `/is` instead of `/i` so `.*?` crosses newlines. Defends the BillingError reclassification against any upstream layer that wraps the OpenRouter error onto multiple lines. Trivial. Test plan: 488/488 unit tests pass (1 new test for newline regex behavior). --- agents/opencode.ts | 26 +++++++++++++++----------- utils/providerErrors.test.ts | 11 +++++++++++ utils/providerErrors.ts | 7 ++++++- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/agents/opencode.ts b/agents/opencode.ts index 0fba5e0..a59aa70 100644 --- a/agents/opencode.ts +++ b/agents/opencode.ts @@ -56,19 +56,23 @@ type OpenCodeConfig = { agent?: Record; model?: string; enabled_providers?: string[]; - /** - * OpenCode's `limit.output` controls the per-inference `max_tokens` the agent - * reserves with the upstream model. OpenCode defaults to 32_000 (sized for - * long-running TUI sessions where a human user might want big outputs). - * Pullfrog runs are headless and short — typical outputs are 1-3K tokens — - * so we override to a much smaller value. This drastically reduces the - * upfront budget reservation OpenRouter requires per call, which is what - * lets low-wallet runs actually start. - */ - limit?: { output?: number }; [key: string]: unknown; }; +/** + * Per-inference `max_tokens` reservation the agent sends to the upstream + * model. OpenCode's default is 32_000 (sized for long-running TUI sessions + * where a human user might want big outputs). Pullfrog runs are headless and + * short — typical outputs are 1-3K tokens — so we cap at 5_000. This + * drastically reduces the upfront budget reservation OpenRouter requires per + * call (~$0.38 vs ~$2.40 for Opus), which is what lets low-wallet runs + * actually start. + * + * Plumbed via `OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX` env var rather than the + * config JSON. OpenCode's `OUTPUT_TOKEN_MAX` (session/llm.ts) is sourced + * exclusively from this env var; top-level `limit.output` in the config + * has no read site and is silently dropped on merge. + */ const PULLFROG_OPENCODE_OUTPUT_LIMIT = 5000; function buildSecurityConfig(ctx: AgentRunContext, model: string | undefined): string { @@ -85,7 +89,6 @@ function buildSecurityConfig(ctx: AgentRunContext, model: string | undefined): s [pullfrogMcpName]: { type: "remote", url: ctx.mcpServerUrl }, }, agent: buildReviewerAgentConfig(), - limit: { output: PULLFROG_OPENCODE_OUTPUT_LIMIT }, }; if (model) { @@ -942,6 +945,7 @@ export const opencode = agent({ ...homeEnv, OPENCODE_CONFIG_CONTENT: buildSecurityConfig(ctx, model), OPENCODE_PERMISSION: permissionOverride, + OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX: PULLFROG_OPENCODE_OUTPUT_LIMIT.toString(), GOOGLE_GENERATIVE_AI_API_KEY: process.env.GOOGLE_GENERATIVE_AI_API_KEY || process.env.GEMINI_API_KEY, }; diff --git a/utils/providerErrors.test.ts b/utils/providerErrors.test.ts index 6371a69..bdc4d39 100644 --- a/utils/providerErrors.test.ts +++ b/utils/providerErrors.test.ts @@ -113,4 +113,15 @@ describe("isRouterKeylimitExhaustedError", () => { 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); + }); }); diff --git a/utils/providerErrors.ts b/utils/providerErrors.ts index cc7ba00..6a6b962 100644 --- a/utils/providerErrors.ts +++ b/utils/providerErrors.ts @@ -51,8 +51,13 @@ export function detectProviderError(text: string): string | null { * `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/i; + /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);