From 8dff91ac497e3eb25eef59b9981047d88ec7bb56 Mon Sep 17 00:00:00 2001 From: Colin McDonnell Date: Wed, 20 May 2026 02:10:29 +0000 Subject: [PATCH] router: fix unspendable signup credit on no-card private repos (#792) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * router: fix unspendable signup credit on no-card private repos (#791) The bug ------- `run-context/route.ts` gated `proxyModel` minting on `isInfraCovered`, which is `oss || hasCard`. So a no-card account with positive wallet balance (signup credit, top-up, etc.) on a private repo would never get a `proxyModel` set on the run context. The action runtime then fell through to whatever provider keys happened to be in the workflow env — using the user's BYOK keys without their knowledge if any were configured, or failing the run entirely otherwise. Meanwhile `proxy-token/route.ts` already gated correctly on `oss || hasCard || balance > 0`. The two routes disagreed, with run-context being strictly more restrictive, so the agent never even attempted to call proxy-token for these accounts. The wiki at `billing.md:1052` documented the *intended* behavior ("a Router usage row can debit a wallet with no card on file"), aspirational against the actual code. The action side had a parallel bug at `action/utils/proxy.ts:151` — it re-derived `isInfraCovered({ isOss, plan })` and short-circuited mint even when the server set `proxyModel`. Belt-and-suspenders that was strictly more restrictive than the server. Production impact ----------------- Queried 55 router-mode no-card accounts holding signup credit: - ALL have wallet balance = exactly $10.00 (untouched) - ALL have 0 router proxy keys ever minted, 0 hwm usage - ~25 have successful runs (using BYOK env vars from their workflow, unaware their credit isn't being touched) - The rest have zero successes; some accumulated 25+ failures (e.g. `onechannelpe`: 25 failures, 0 successes, no card, $10 credit). The fix ------- - `run-context/route.ts`: widen `useRouter` to match proxy-token's gate. OSS short-circuits as before. Otherwise: router mode + card on file → mint; router mode + no card + positive balance → fetch balance, mint if > 0. Skip the balance read when a card is on file (auto-reload covers it without needing pre-flight balance — keeps the hot path single-query). - `action/utils/proxy.ts`: drop the redundant `isInfraCovered` check. `ctx.proxyModel` IS the signal — the server is the authority on funding decisions; the action just trusts and mints. - `wiki/pricing.md`: correct the Router proxy key minting gate row + add a paragraph explaining why this gate diverges from `isInfraCovered`. - `wiki/billing.md`: rewrite the misleading "proxy-token returns 402" paragraph to describe what actually happens at both routes. `isInfraCovered` is unchanged. It still gates Pullfrog-paid features (learnings writes, indexing). The bug was in conflating "Pullfrog pays for marginal infra" with "user can fund a Router run via wallet" — different concerns, now untangled. * action: drop dead isInfraCovered + plan param post-fix Cleanup the action-side dead code introduced by the previous commit's removal of the redundant `isInfraCovered` re-derivation in proxy.ts: - delete `isInfraCovered` from action/utils/runContext.ts (was the only callsite; mirror in server's utils/billing.ts is unchanged and still load-bearing for learnings/indexing) - drop unused `plan: AccountPlan` param from `resolveProxyModel` / `runProxyResolution` (and the corresponding `AccountPlan` import + the `plan: runContext.plan` arg at the main.ts call site) - update the action/mcp/server.ts comment that pointed at the now-gone action mirror to reference the server-side `utils/billing.ts` instead `AccountPlan` itself is still load-bearing (mcp/server, runContextData, run-context fetch), only `isInfraCovered` and the dead `plan` parameter go away. --- main.ts | 1 - mcp/server.ts | 8 ++++---- utils/proxy.ts | 16 +++++++++------- utils/runContext.ts | 9 --------- 4 files changed, 13 insertions(+), 21 deletions(-) diff --git a/main.ts b/main.ts index 62f9930..dbc66a1 100644 --- a/main.ts +++ b/main.ts @@ -163,7 +163,6 @@ export async function main(): Promise { await runProxyResolution({ payload, oss: runContext.oss, - plan: runContext.plan, proxyModel: runContext.proxyModel, oidcCredentials, repo: runContext.repo, diff --git a/mcp/server.ts b/mcp/server.ts index 26705d9..c622721 100644 --- a/mcp/server.ts +++ b/mcp/server.ts @@ -63,10 +63,10 @@ export interface ToolContext { mcpServerUrl: string; tmpdir: string; // repo-level OSS flag + account-level billing plan. together they decide - // whether pullfrog is paying for marginal infra — see isInfraCovered in - // utils/runContext.ts. plan gating for endpoints like the learnings PATCH - // is enforced server-side via 402, so we pass plan along mostly for future - // use / observability. see wiki/pricing.md. + // whether pullfrog is paying for marginal infra — see `isInfraCovered` in + // the server's `utils/billing.ts`. plan gating for endpoints like the + // learnings PATCH is enforced server-side via 402, so we pass plan along + // mostly for future use / observability. see wiki/pricing.md. oss: boolean; plan: AccountPlan; // resolved upstream model specifier (e.g. "google/gemini-3.1-pro-preview"). diff --git a/utils/proxy.ts b/utils/proxy.ts index b0fe2fb..9833007 100644 --- a/utils/proxy.ts +++ b/utils/proxy.ts @@ -30,7 +30,6 @@ import { import { log, writeSummary } from "./cli.ts"; import { reportErrorToComment } from "./errorReport.ts"; import type { ResolvedPayload } from "./payload.ts"; -import { type AccountPlan, isInfraCovered } from "./runContext.ts"; export interface OidcCredentials { requestUrl: string; @@ -129,9 +128,16 @@ async function buildProxyTokenHeaders(ctx: { * Decide whether this run needs a minted proxy key and, if so, mint and * inject it as `OPENROUTER_API_KEY`. Mutates `payload.proxyModel` on success. * + * `ctx.proxyModel` IS the signal — the server (`run-context/route.ts`) is + * the authority on "should this run use the Router". It already knows the + * full picture (OSS, plan, wallet balance, modelAccessMode) and only sets + * `proxyModel` when the gate passes. The action just trusts that signal + * and mints. Re-deriving the gate locally was redundant and was strictly + * more restrictive (no balance check), which made signup-credit runs on + * no-card private repos silently fall through to BYOK. + * * Skipped when: * - `PULLFROG_MODEL` env override is set (BYOK escape hatch) - * - `isInfraCovered({ oss, plan })` is false (BYOK account on a paid run) * - `proxyModel` is not set on the run context * - no OIDC credentials available and not talking to a localhost API * @@ -140,7 +146,6 @@ async function buildProxyTokenHeaders(ctx: { async function resolveProxyModel(ctx: { payload: ResolvedPayload; oss: boolean; - plan: AccountPlan; proxyModel?: string | undefined; oidcCredentials: OidcCredentials | null; repo: { owner: string; name: string }; @@ -148,8 +153,7 @@ async function resolveProxyModel(ctx: { // env override = BYOK escape hatch, don't proxy if (process.env.PULLFROG_MODEL?.trim()) return; - const needsProxy = isInfraCovered({ isOss: ctx.oss, plan: ctx.plan }) && ctx.proxyModel; - if (!needsProxy) return; + if (!ctx.proxyModel) return; // dev affordance: when talking to a localhost API, the server-side // x-dev-repo bypass replaces OIDC verification, so a play run can @@ -184,7 +188,6 @@ async function resolveProxyModel(ctx: { export async function runProxyResolution(ctx: { payload: ResolvedPayload; oss: boolean; - plan: AccountPlan; proxyModel?: string | undefined; oidcCredentials: OidcCredentials | null; repo: { owner: string; name: string }; @@ -194,7 +197,6 @@ export async function runProxyResolution(ctx: { await resolveProxyModel({ payload: ctx.payload, oss: ctx.oss, - plan: ctx.plan, proxyModel: ctx.proxyModel, oidcCredentials: ctx.oidcCredentials, repo: ctx.repo, diff --git a/utils/runContext.ts b/utils/runContext.ts index ffb7fae..8d15b65 100644 --- a/utils/runContext.ts +++ b/utils/runContext.ts @@ -48,15 +48,6 @@ export interface RepoSettings { */ export type AccountPlan = "none" | "payg"; -/** - * "Is Pullfrog absorbing marginal infra cost for this repo?" — composite - * predicate over the two orthogonal dimensions (repo-level OSS, account-level - * plan). Mirrors `isInfraCovered` in the server's `utils/billing.ts`. - */ -export function isInfraCovered(params: { isOss: boolean; plan: AccountPlan }): boolean { - return params.isOss || params.plan === "payg"; -} - export interface RunContext { settings: RepoSettings; apiToken: string;