7e90e5cae655355da118ec925b73a256deeed892
2 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Colin McDonnell
|
8dff91ac49 |
router: fix unspendable signup credit on no-card private repos (#792)
* 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. |
||
|
Colin McDonnell
|
0a64659ee7 |
refactor: slim action/main.ts to an orchestrator + extract helpers (#755)
* refactor: extract helpers out of action/main.ts so non-orchestration churn stops touching the file main.ts had grown to ~1240 lines holding ~500 lines of helpers that have nothing to do with the resolver pipeline — billing-error UI/copy, proxy minting, summary/learnings persistence, log formatters, end-of-run cleanup waterfalls. any PR adding a new billing code branch or a new log line was forced to edit main.ts, and since main.ts is in ALWAYS_RUN_ALL the entire 52-job LLM CI matrix fired on what should have been a 0-job change (e.g. #748). extractions: - action/utils/billingErrors.ts — BillingError, TransientError, the format*Summary renderers, billingConsoleUrl - action/utils/proxy.ts — mintProxyKey, buildProxyTokenHeaders, resolveProxyModel, plus runProxyResolution wrapper that renders + rethrows BillingError/TransientError before the outer catch - action/utils/prSummary.ts — fetchPreviousSnapshot, persistSummary co-located with the existing seed/read file helpers - action/utils/learnings.ts — persistLearnings co-located with the existing seed/read file helpers - action/utils/runStartupLog.ts — resolveOutputSchema + logRunStartup (the model/agent/push/shell/timeout block) - action/utils/runErrorRenderer.ts — renderRunError classifies (BillingError reclassify / hang detect / API-key auth) and emits {summary, comment} markdown bodies - action/utils/runLifecycle.ts — persistRunArtifacts, finalizeSuccessRun, writeRunErrorOutputs — the three end-of-run cleanup phases shared between the success path and the error catch path main.ts is now ~570 lines — the irreducible orchestrator: disposables (`await using` for tokenRef / gitAuthServer / mcpHttpServer), the toolContext construction, the agent-timeout race, the catch/finally shape, and the named phase calls. behavior is preserved verbatim (verified: pnpm -r typecheck + pnpm test 695/695 pass, action/test 596/596 pass). wiki/main.md gets a new "file layout" section describing the split. AGENTS.md gets a single line pointing future edits at the helpers instead of main.ts. * anneal: address review findings - restore MainResult.result?: string (accidental removal in initial commit; field was unused in current code but is part of the exported interface surface — keep the diff truly behavior-preserving) - move resolveOutputSchema from runStartupLog.ts to payload.ts (it's an action-input resolver alongside resolvePromptInput / resolvePayload, not a log helper — was placed in runStartupLog.ts for matrix-churn pragmatism but the domain fit is in payload.ts) - un-export resolveProxyModel (only used internally by runProxyResolution in proxy.ts; no external importer) - fix runErrorRenderer.ts JSDoc "Three classifications" → four (Billing, hang, API-key, default) - expand runLifecycle.ts module banner to note that finalizeSuccessRun calls persistRunArtifacts first, and to explain why the catch path splits writeRunErrorOutputs + persistRunArtifacts - update billingErrors.ts header to point at proxy.ts and runErrorRenderer.ts as the actual origin sites (was stale "main.ts") - expand proxy.ts header to spell out the runProxyResolution entrypoint contract (was stale "main.ts can render") - update wiki/main.md resolver chain + dependency table to name runProxyResolution as the actual call site and document the early BillingError/TransientError rendering branch - update wiki/main.md file-layout table to lead with runProxyResolution and describe mintProxyKey/buildProxyTokenHeaders/resolveProxyModel as internal helpers (was implying they were public surface) |