fix(sdk): make model pricing non-blocking for dollar budgets - #421
Conversation
Pricing is light enforcement. Under a frozen dollar budget, an LLM/agent step with no model or an unpriced model now warns `budget_unmetered` instead of refusing `budget_missing_price`, runs, and contributes no dollars. Codex selects its own model, so a Codex step without a declared model is reported as unmetered and Codex model ids need no MODEL_PRICING entry. Priced steps still accrue and a crossed limit still stops the run; `budget_syntax_invalid` still refuses. Fixes the burn#539 Cloud run where a model-less codex step was refused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6393578. Configure here.
Review swarm: maintainabilityMaintainability Review: PR #421PR: fix(sdk): make model pricing non-blocking for dollar budgets SummaryThis PR changes budget enforcement from hard refusal to soft warning when a model lacks frozen pricing. The change touches 15 files across documentation, types, preflight, and runtime accounting. Core safety question: will future maintainers understand when dollar budgets ARE enforced versus when they are NOT? FindingsF1: Implicit contract between preflight and runtime is documented but not enforcedLocation: Issue: The system now depends on coordinated behavior across two files:
This three-part contract is explained in comments but nothing enforces the alignment. A future change to Six-month test: A maintainer adding a new pricing source (e.g., real-time API prices) might see Missing: Either:
The comments explain the design but cannot prevent drift. F2: "Light enforcement" is ambiguous about what IS enforcedLocation: Quote from BUDGET.md:
Issue: The phrase "light enforcement" introduces a concept without defining its boundaries. The docs clarify what is NOT enforced (missing prices) but don't specify what IS enforced when prices exist. Six-month test: A maintainer sees "light enforcement" and needs to answer:
The answer exists (priced steps DO enforce, line 17-18: "a crossed limit still stops the run"), but it's buried. The term "light enforcement" suggests leniency everywhere when it actually means "lenient at preflight, strict at runtime for priced steps." Recommendation: Replace "light enforcement" with precise terms:
F3: Test coverage assumes Codex behavior but doesn't verify itLocation: Test added: it('lets a Codex step without a model run unmetered beside a priced Claude step (burn#539)', () => {
// ...
expect(result.diagnostics).toEqual([expect.objectContaining({
severity: 'warning', kind: 'budget_unmetered', stepId: 'plan-reviewer',
message: expect.stringContaining('Codex selects its own model'),
})]);Issue: The test asserts Codex gets a specific warning message but doesn't verify the claim the message makes: "Codex selects its own model." The test would pass even if:
Six-month test: A maintainer sees burn#539 resolved by this test and assumes Codex model selection is verified. They change Codex adapter registration, the test still passes (because it only checks the warning text), and dollar budgets break in production. Missing: Either:
The test prevents regression of warning text but not of the behavior the warning describes. F4: Error code removal leaves no breadcrumb for existing deploymentsLocation: Change: Deleted Issue: Any deployed flow that relied on catching this specific error code will now see generic failures. The PR removes the code but doesn't document:
Six-month test: A maintainer debugging a customer issue finds Missing:
RFC-0001 Appendix A.1 establishes that failure kinds must be forward-compatible ("readers must ignore unknown keys"). Does that apply in reverse? Can a reader encounter a historical F5:
|
Review swarm: historyPR #421 — history reviewReviewed head: FindingH1 — P2: finish replacing the old budget contract in documentationLocation in the change: The new budget paragraph promises warning-only admission for missing prices, but the same guide still says checking a compiled flow preserves the “missing-price refusal.” That is precisely the previous contract from budget-header commit The adjacent pricing guidance also still tells callers to omit Requested change: update the compiled-artifact paragraph to describe preservation of warning-only pricing, and describe the worker's token-preserving fallback in the pricing comment. No runtime redesign is requested by this finding. These are concrete remnants of superseded contracts, the same documentation-versus-behavior class recorded in DRIVE-LOG's review of #252. Historical assessment
Inputs, recovery, and limitsInitially, The requested Recovered Git metadata was copied to the originally referenced Evidence below is captured from this review. No runtime tests, mutation checks, or CI verification were run or claimed. The verdict rests on source/history contradictions described in H1, not on the repaired sandbox error. Captured evidenceCommandgit rev-parse HEADOutput (exit 0): Commandgit log --oneline -40Output (exit 0): Commandgit log --format=full ff8f778..HEADOutput (exit 0): Commandgit log --oneline HEAD -- packages/sdk/src/budget-preflight.tsOutput (exit 0): Commandgit show 72a162f:packages/sdk/src/budget-preflight.tsOutput (exit 0): Commandgit show 761cb1b:packages/sdk/tests/budget-preflight.test.ts | sed -n '52,78p'Output (exit 0): Commandgit -c core.abbrev=8 diff ff8f778 HEAD | cmp - .review-target/pr.diffOutput (exit 0): Commandnl -ba docs/BUDGET.md | sed -n '9,17p;57,61p'Output (exit 0): Commandnl -ba packages/sdk/src/model-pricing.ts | sed -n '15,24p'Output (exit 0): Commandnl -ba packages/sdk/src/worker-spend.ts | sed -n '1,32p'Output (exit 0): Commandgit diff ff8f778 HEAD -- packages/sdk/tests/budget-preflight.test.ts packages/sdk/tests/model-pricing.test.tsOutput (exit 0): Commandrg -n "refuses or warns|tokens.*dollars|memory tokens are charged|New: memory tokens" docs/RFC-0001-everything-is-a-relayflow.mdOutput (exit 0): Commandsed -n '7553,7577p;10295,10330p' ops/DRIVE-LOG.mdOutput (exit 0): It releases the reservation and returns from verification falsified my own headline. The history transcript's full sentence: I quoted it up to "absent" and stopped. It goes on to say it fell back to So the history lens DID review the actual change. My issue title -- "two lenses For maintainability I inferred blindness from citation style. Weak evidence, What survives: the /tmp handoff really is broken, all three lens tasks Also corrected the #259 comment, where I had over-corrected in the direction of Two lessons, both mine:
2026-09-10 11:56Z — relayflow-lead-0910 online; took over from -0903 |
Review swarm: structurePR #421 — structure reviewTarget: Blocking findingP1 — Unknown spend becomes a zero-dollar fact across the journal boundaryLocation: For a dollar-budgeted step with an unpriced or undeclared model, preflight now permits execution and the new helper encodes reported tokens with This makes SDK pricing availability determine whether a declared kernel dollar ceiling has any effect. A sequence of exclusively unpriced steps can incur model costs while the journal records zero dollars throughout; replay cannot recover the missing distinction. The warning is explicit at preflight, but it is not an accounting fact understood by the enforcing kernel. RFC-0001 §1 assigns token/dollar enforcement to the kernel and Gate 1 requires exact budget accounting; AGENTS.md requires a fail-closed boundary. Describing pricing as light enforcement in BUDGET.md does not reconcile those requirements. Keep dollar-budget admission closed when cost cannot be accounted for, or first establish an explicit advisory-budget contract that preserves unknown metering in durable accounting. Preserve token accounting without presenting unknown dollars as measured zero. No new step primitive or provider logic in the kernel is needed to retain the existing refusal behavior. Other structural observations
Captured evidenceFocused execution of the changed helper (not an end-to-end budget or crash test): /home/daytona/node_modules/.bin/tsx -e 'import { workerSpend } from "./packages/sdk/src/worker-spend.ts"; console.log(JSON.stringify(workerSpend({exit_code:0,stdout_tail:"ok",stderr_tail:"",tokens_input:100,tokens_output:50}, "unlisted-model"), null, 2));'Captured output (exit 0): File-size inspection: wc -l packages/sdk/src/preflight.ts packages/sdk/tests/preflight.test.ts packages/sdk/src/budget-preflight.ts packages/sdk/src/worker-spend.tsCaptured output (exit 0): The complete supplied diff was read from Delivery limitationThe workspace's git add -- ops/reviews/20260915-2121-pr421-structure.mdCaptured output (exit 128): REVIEW_FAILED |
Review swarm: FAILED
Cloud run: |
Bugbot: unpriced steps journaled no usage at all, so a token or dual
{ tokens, dollars } budget never saw them. workerSpend now attaches the
reported tokens with zero dollars for unpriced models (dollar budgets stay
unaffected); a CLI that reports no tokens still attaches no usage.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
maintainability lens — PASSI've read AGENTS.md, Review — maintainability lens on PR #421Blockers: none. Concerns
Notes
REVIEW_PASSED |
history lens — FAILReading additional input from stdin...
|
structure lens — MISSING |
|
🎯 review-swarm: FAILED (M:pass H:fail S:missing) Lens transcripts posted as sibling comments above. |
…ntract (#423) Follow-up to #421's review-swarm findings. P1: an unpriced step no longer journals its unknown cost as a measured `dollars: "0.000000"`. Worker usage now carries the tokens with `dollars_unmetered: true` and no dollar amount. The kernel `Budget` gains an additive `dollars_unmetered` flag (serde default false, omitted when false). It is projected into `step.completed.spend`, sticky on run totals, and ignored by dollar ceilings while tokens still count. `step.complete` rejects unmetered usage that also claims non-zero dollars. H1: BUDGET.md, SURFACE.md, DESIGN.md and the model-pricing / failure-kinds comments no longer describe the missing-price refusal or say to omit usage. F9: a live-kernel test covers the chain (unpriced under a dollar budget runs, tokens still trip a token budget, priced spend still stops the run), with kernel budget_gate tests for the flag. F1/F2/F5/F7: the preflight -> worker -> kernel contract is documented, "light enforcement" is replaced with a precise list of what is enforced, BudgetStepResolution is explained, and PreflightResult.ok is documented as "no refusal". `flows build` prints `budget_unmetered` and `flows deploy` prints warnings on success. Co-authored-by: kjgbot <kjgbot@agentrelay.dev> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Production failure
Cloud run for AgentWorkforce/burn#539 (flow deployed from agentrelay.com onboarding with
{ budget: "$8/run" }): stepplanner(cliclaude) ran, then stepplan-reviewer(clicodex, nomodel) was refused at preflight:The CLI exited 2 and the run ended
step_failed.budgetDiagnosticsrefused every non-deterministic step that had no resolvable model, and every model not inMODEL_PRICING, whenevermaxDollarswas set. Codex has no adapter default and its real model ids (gpt-5.x) aren't table keys, so every Codex step under a dollar budget was refused.Behaviour change
Pricing is light enforcement and never blocks a run.
budget_unmetered(new entry inPREFLIGHT_WARNING_KINDS) instead of thebudget_missing_pricerefusal.flows check/flows runprint it asWARNING [budget_unmetered] ...and exit 0. Warnings from check already carry into the run report'sdiagnostics, so that's where the unmetered step ids show up. No new surface was added.preflight()now stops early only on refusals, not on any diagnostic, so an unmetered step still gets its CLI/auth probes.pricedUsagereturns undefined, and the kernel'sspend.dollarsdefaults to0), so they can't tripmaxDollars. Priced steps still accrue dollars, and a crossed limit still refuses the next start withbudget_exceeded. The kernel is unchanged.Codex selects its own model). Declared Codex model ids don't need to be inMODEL_PRICING; unpriced ones warn the same way. No prices were invented.maxDollars) produces no pricing diagnostics at all. Before, declared unpriced models were refused even then.budget_missing_priceis no longer emitted anywhere, so it's removed from the refusal taxonomy (failure-kinds.ts), fromAuthoredFlowExecutionErrorCode, and from thedirect-run/authored-worker-stepmappings. Heads-up for consumers that switch on that literal.docs/BUDGET.md,docs/SURFACE.md,adapters/base.ts,model-pricing.ts,worker-spend.ts.What still refuses
budget_syntax_invalidand malformed budgets,model_unknown(project allowlist),cli_*/model_unavailableprobes, and every other existing refusal kind. A genuinely exceeded budget still stops the run in the kernel (budget_exceeded).Rollout
Cloud picks this up only after a flows release (
.github/workflows/publish.yml:@relayflows/sdk,@relayflows/surface,runtime-linux-x64/runtime-darwin-arm64,relayflows; the Cloud runtime artifact comes from.github/workflows/cloud-runtime-artifact.yml) and a new Cloud sandbox runtime snapshot. The current Cloud snapshotrelay-orchestrator-sdk-12.1.0-relayfile-v0.10.60-runtime-4.1.52-...(runtime hashf8fb54fa08bebf26) still has the refusing preflight. This PR releases, publishes and deploys nothing.Test plan
npm run typecheck(tsc + type-tests) andnpm run typecheck:tests: cleannpx vitest run tests/budget-preflight.test.ts tests/preflight.test.ts tests/model-pricing.test.ts tests/budget-attribution.test.ts tests/cli.test.ts: 5 files, 130/130 passed. New or changed cases:$8/runclaude + model-less codex (the burn#539 shape):ok, onebudget_unmeteredwarning on the codex step, both CLIs probedok, still probedgpt-5.2-codex: warning onlybudget_syntax_invalidstill refuses before probingbudget_unmeteredKernel over-budget semantics:
sh ops/cargo.sh test -p relayflowd --test budget_gategives 3 passed (crossing_completion_is_durable_and_next_step_is_refused,daily_windows_reset_and_exact_limits_do_not_refuse,deterministic_spend_and_wallclock_limit_gate_parallel_batch_starts)Manual smoke:
node dist/cli.js check burn539.flow.yaml(claude + model-less codex,budget: "$8/run") printedWARNING [budget_unmetered] Step "plan-reviewer" is unmetered (Codex selects its own model); it does not count toward the dollar budget.,CHECK PASSED, exit 0Full
npm testlocally: 1641 passed, 24 failed, 18 skipped. The failures are local-environment only:.flow.tsRerunning the 9 failing files with
RELAYFLOWD_BINset: 15 failed / 90 passed on this branch, and the same 15 fail onmain(ff8f778). CI is the authoritative run.🤖 Generated with Claude Code
Note
Medium Risk
Changes budget admission and spend accounting semantics for dollar-budgeted flows; consumers switching on
budget_missing_pricemust handlebudget_unmeteredwarnings instead.Overview
Dollar budgets no longer refuse runs when a step cannot be priced. Missing or unlisted models (including model-less Codex steps) emit preflight warning
budget_unmeteredinstead of refusalbudget_missing_price, which is removed from error taxonomies and CLI mappings.Preflight only fails on refusals, so unmetered steps still get CLI/auth probes. Runtime spend journals
0.000000dollars for unpriced steps while preserving reported tokens for token budgets; priced steps and kernelbudget_exceededbehavior are unchanged.Docs (
BUDGET.md,SURFACE.md) and tests are updated for the burn#539 shape (Claude + Codex under$8/run).Reviewed by Cursor Bugbot for commit 4053402. Bugbot is set up for automated code reviews on this repo. Configure here.