router: fix bugs from PR #616 review (#625)

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).
This commit is contained in:
Colin McDonnell
2026-05-08 21:02:38 +00:00
committed by pullfrog[bot]
parent 93cc7b1a44
commit ec43c0e0d1
3 changed files with 32 additions and 12 deletions
+15 -11
View File
@@ -56,19 +56,23 @@ type OpenCodeConfig = {
agent?: Record<string, unknown>;
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,
};
+11
View File
@@ -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);
});
});
+6 -1
View File
@@ -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);