fix(agent): enforce the Max cost per task (USD) setting (#911)

[frankbria]

Closes #911.

Problem

Settings → Agent renders a Max cost per task (USD) control, PUT /api/v2/settings persists config.max_cost_usd, and a full-repo grep finds the field only in config.py, ui/models.py, settings_v2.py and the frontend. No runtime, react_agent, conductor or adapter ever read it.

A user who sets a $5 cap gets zero cost limiting — and the sibling max_turns control genuinely works, so an inert cap is indistinguishable from a working one. For a paid product a spend cap that silently does nothing is worse than no cap.

Which option

The issue allows either enforcing the field or removing it. Enforced — the data was already there: ReactAgent._estimate_total_cost() accumulates per-model cost from token records via MetricsTracker.calculate_cost. Only the read was missing.

Fix

ReactAgent._resolve_cost_cap() reads max_cost_usd from the same .codeframe/config.yaml the Settings page writes, resolved once per run alongside the adaptive iteration budget. The ReAct loop then checks accumulated spend against it before each LLM call, next to the existing stall guard.

Three decisions worth naming:

• Before the call, not after. A call's cost is not knowable until it returns, so the cap means "stop as soon as we are over", not "never exceed by a cent". Checking afterwards would spend one extra call's worth every time.
• BLOCKED, not FAILED. The stall handler set this precedent. The work is not wrong; it needs a human decision — raise the cap, or stop — which is exactly what a blocker is for and what cf blocker answer resolves. FAILED would misreport the run.
• A 0 cap is treated as unset. Enforcing it literally would stop before the first call and brick every run — a footgun for anyone who types 0 meaning "no limit". Negative values are already rejected by EnvironmentConfig.validate().

A malformed value (hand-edited YAML, a string) logs and disables the cap rather than taking the agent down.

Tests

tests/core/test_cost_cap_enforced_911.py — 8 tests:

• The cap is read from the file the Settings page writes, and round-trips from the API's write path to the agent's read path.
• Exceeding it returns AgentStatus.BLOCKED with a cost_cap_exceeded blocker naming both the spend and the cap.
• The LLM is never called once the cap is reached — asserted with a provider that raises if invoked, since "stop before the next call" is the whole point.
• Spend under the cap does not block.
• Unset / 0 / malformed all mean "no cap".

Mutation-checked: nulling the resolved cap fails the two behavioural tests. 83 tests across test_react_agent.py plus this file pass.

Scope, stated on the control itself

The cap binds the built-in ReAct engine — the default. It cannot bind the others today:

• The plan engine (core/executor.py) has no token or cost tracking at all, so there is no accumulated spend to compare against.
• Delegated adapters (Claude Code, Codex, OpenCode) bill through their own CLI and report no usage back.

Rather than leave that implicit, the Settings control now says so:

Stops the run and raises a blocker once estimated spend reaches the cap. Applies to the built-in agent; delegated engines (Claude Code, Codex, OpenCode) bill through their own CLI and are not counted.

Closing that gap is #1004 [P1.35] — it needs token accounting built in the plan engine first, which dwarfs this issue.

Acceptance criteria

• The execution path enforces max_cost_usd and aborts the run when exceeded
• A test with a low cap asserts the run terminates and the task is not marked DONE (BLOCKED, so runtime never transitions it to DONE)
• No UI control persists a value nothing reads

[github-actions]

Precision bug-hunting review in progress

Scope: concrete defects only (logic errors, null/error paths, security, data loss, contract breaks).

• Fetch PR diff and context
• Check existing comments (avoid duplicates)
• Read react_agent.py change + surrounding code / callers
• Read the new test file
• Confirm findings against real code
• Post inline comments for confirmed defects
• Post summary

View job run

[github-actions]

Claude finished @frankbria's task in 6m 57s —— View job

---

Precision bug-hunting review — 2 defects found

Scope: concrete defects only (logic errors, null/error paths, security, data loss, contract breaks). Style, architecture, naming, and test coverage are covered by the general review.

• Fetch PR diff and context
• Check existing comments (avoid duplicates)
• Read react_agent.py change + surrounding code / callers
• Read the new test file
• Confirm findings against real code
• Post inline comments for confirmed defects
• Post summary

Both findings share one root cause: _estimate_total_cost() (react_agent.py:338) does not produce the value the new cap check needs in two real cases, so spent is 0.0 and the guard never binds — i.e. the cap stays inert, the precise defect #911 set out to fix.

severity	file:line	finding
major	codeframe/core/react_agent.py:478	Cap is silently inert for every non-Anthropic model. MetricsTracker.calculate_cost returns 0.0 for any model not in the Anthropic-only MODEL_PRICING (lib/metrics_tracker.py:53), so for the supported OpenAI/Ollama/vLLM/compatible providers spent is always 0.0 and the cap never fires — undocumented, unlike the plan-engine/delegated-adapter exclusions.
minor	codeframe/core/react_agent.py:489	"Max cost per task" cap is effectively per-run. Resume builds a fresh ReactAgent with empty _token_records (adapters/builtin.py:108), and _estimate_total_cost() sums only that in-memory accumulator — not the persisted rows — so a resumed task gets a fresh budget and can exceed the cap repeatedly.

No null/error-path crashes, no injection/authz/SSRF/traversal issues, no resource leaks introduced by this diff.

[frankbria]

Cross-family adversarial review (codex) — 5 findings triaged

Fixed (3). All three were paths my own tests never reached — a reminder that mutation testing proves a test catches its fix being removed, and says nothing about code the fix never covered.

Verification self-correction spent past the cap. A run could sit at $4.99 under a $5 cap, fail verification, and then spend max_verification_retries × max_fix_turns more calls — the cap was only in _react_loop. Both loops now go through one _cost_cap_message(); a cap the correction loop ignores is not a cap.

It was a per-run cap, not per-task. _estimate_total_cost() sums only this agent instance's in-memory records, so answering the blocker and resuming handed the task a fresh full budget every time — a cap bypassed by clicking resume. Prior spend is now seeded from the persisted per-task token totals (get_task_token_summary), the same store the Costs page reports from.

An unmeasurable cap silently allowed unbounded spend. MetricsTracker.calculate_cost returns $0.00 for models it has no pricing for — only Anthropic models are priced — so with --llm-provider openai --llm-model gpt-4o the guard would see $0.00 forever and never fire. That is the same silently-inert control this issue is about, one layer down. It now refuses before spending: "a cost cap of $X is configured, but there is no pricing data for gpt-4o". Fail-closed is the right default for a money control.

While fixing the first one I found a related mismatch of my own: the guard created a blocker but returned a reason the caller mapped to FAILED, leaving a blocker attached to a failed run. The reason set is now a named _BLOCKED_REASONS constant rather than an inline tuple, so it is directly testable.

Filed, not fixed (1). Finding 1 — web resume flips run state without spawning a worker, stranding the task — is pre-existing: resume_run() never had the background execute_agent() thread that start has, for any blocked task. #911 makes it common rather than latent, since raising a cap and resuming becomes routine. Filed as #1005 [P0.26] with the reproduce steps and the suggested fix (extract the thread body once, call it from both endpoints). Bundling it here would hide a distinct bug inside a cost-cap PR.

Already handled (1). Finding 4 — other engines uncapped — is documented on the Settings control itself and tracked as #1004 [P1.35]. Codex is right that ui/models.py still describes the field globally; that reads as the intent, and #1004 is where it becomes true.

One test-quality note: my first version of the verification-loop test asserted _cost_cap_message() is not None and that no call happened — which tested the helper, not the loop's use of it, and passed with the guard deleted. Rewritten to drive the real _run_final_verification with failing gates. All three fixes now fail their tests when reverted.

[github-actions]

Claude finished @frankbria's task in 9m 10s —— View job

---

Precision bug-hunting review — 2 defects found

Scope: concrete defects only (logic errors, null/error paths, security, data loss, contract breaks, races, resource leaks). Style, architecture, naming, and test coverage are covered by the general review.

• Fetch PR diff (git diff origin/main..HEAD) and full file context
• Check existing inline comments (avoid duplicates)
• Read react_agent.py cost-cap logic + _estimate_total_cost / _resolve_cost_cap / both loops
• Read MetricsTracker.calculate_cost + MODEL_PRICING + normalize_model_name
• Read persisted-spend path (Database / TokenRepository / get_task_token_summary)
• Read the new test file + adapters/builtin.py resume path
• Confirm each candidate finding against real code
• Post inline comments for confirmed defects
• Post summary

The prior review's two findings (inert for non-Anthropic models; per-run rather than per-task) are addressed in the latest commits — but the per-task fix is non-functional, and the measurability guard over-fires. Both defeat the cap this PR exists to enforce.

severity	file:line	finding
major	codeframe/core/react_agent.py:1102	_load_prior_task_cost calls Database() with no db_path, but Database.__init__ requires one → TypeError is swallowed by the broad except → _prior_task_cost_usd is always 0.0. The resume-bypass the method exists to close is still wide open: a blocked task resumed with a raised cap gets a fresh full budget every time. The masking test only asserts == 0.0, so it passes whether the read succeeded or threw.
minor	codeframe/core/react_agent.py:1075	The "unmeasurable cap" guard hard-blocks any run whose recorded model isn't an exact MODEL_PRICING key — including stock Anthropic models (e.g. claude-opus-4-1, the shipped DEFAULT_GENERATION_MODEL = "claude-haiku-4-5") that the codebase's own CODEFRAME_CORRECTION_MODEL guidance steers users toward. A false cost_cap_exceeded blocker stops a legitimately-priced run.

Notes:

• The major finding is the headline one: commit 63d0aa0 ("count prior task spend") does not work in production. Inline comment includes a drop-in fix mirroring the proven _persist_token_usage read path (TokenRepository on workspace.db_path via MetricsTracker).
• No null/error-path crashes introduced, no injection/authz/SSRF/traversal issues, no resource leaks or unbounded loops from this diff. sqlite3 is already imported at module level and _load_prior_task_cost has a single production caller (react_agent.py:222), so the suggested replacement is self-contained.

[frankbria]

GLM review — both findings fixed, and the first one had a third layer

Major: the per-task read was non-functional, and my test masked it. Exactly right. Database() requires db_path, so the bare call raised TypeError straight into the broad except, and _prior_task_cost_usd was permanently 0.0 — the resume bypass the method exists to close stayed wide open. My test asserted == 0.0 for an unknown task, which passes whether the read succeeded or threw. A broad except plus an assertion matching the failure value is a perfect blind spot.

The test now writes a real token_usage row and asserts $3.00 comes back — and doing that surfaced a third layer neither review reached:

TypeError: tuple indices must be integers or slices, not str

get_task_token_summary reads row["total_input_tokens"], which needs row_factory = sqlite3.Row. Database sets that on its own connection — which is the only reason these repository methods work anywhere. Any caller building a bare sqlite3.connect() and handing it to a repository was broken by construction, for every repo in platform_store/, not just this one.

So the fix went into BaseRepository.__init__ rather than my call site: a repository that reads columns by name should guarantee the factory on any connection it is given, mirroring what Database already does. Backward compatible — sqlite3.Row supports integer indexing and iteration, and nothing in the codebase type-checks rows as tuples. 273 tests across tests/platform_store, tests/auth, and the agent suites pass.

Minor was worse than minor, and you were right to flag it. MODEL_PRICING holds three keys, so my membership test would have blocked runs on ordinary Anthropic models — including the shipped default claude-haiku-4-5 — with a false cost_cap_exceeded. Replaced with detection by outcome: tokens recorded but computed spend still $0.00. That is robust to pricing-table gaps for any model and cannot false-fire on a priced one, since a priced model yields cost > 0.

Thanks for the drop-in fix pointing at the _persist_token_usage read path — that was the right shape, and following it is what exposed the row-factory issue underneath.

[github-actions]

Claude finished @frankbria's task in 6m 22s —— View job

---

Precision bug-hunting review in progress

This is a synchronize — new commits claim to fix the two findings from the prior round. I'll verify those fixes actually work and hunt for any new defects the fixes introduced.

Scope: concrete defects only (logic errors, null/error paths, security, data loss, contract breaks, races, resource leaks). Style/naming/coverage are handled by the general review.

• Fetch PR diff (git diff origin/main...HEAD) and full file context
• Check existing inline comments (avoid duplicates)
• Verify the claimed per-task cost read fix (_load_prior_task_cost + BaseRepository row_factory)
• Verify the unmeasurable-cap outcome-detection fix (_unpriced_models)
• Read both ReAct + verification loops, _cost_cap_message, _resolve_cost_cap, _estimate_total_cost
• Confirm each candidate finding against real code
• Post inline comments for confirmed defects
• Post summary

[github-actions]

Claude finished @frankbria's task in 11m 5s —— View job

---

Precision bug-hunting review — 1 defect found

Scope: concrete defects only (logic errors, null/error paths, security, data loss, contract breaks, races, resource leaks). Style/naming/coverage are handled by the general review.

• Fetch PR diff (git diff origin/main...HEAD) and full file context
• Check existing inline comments (avoid duplicates)
• Verify the claimed per-task cost read fix (_load_prior_task_cost + BaseRepository row_factory)
• Verify the unmeasurable-cap outcome-detection fix
• Read both ReAct + verification loops, _cost_cap_message, _resolve_cost_cap, _estimate_total_cost
• Confirm each candidate finding against real code
• Post inline comments for confirmed defects
• Post summary

Both claimed fixes verified working. _load_prior_task_cost now opens a TokenRepository on workspace.db_path (the same connection the write path uses) and BaseRepository.__init__ sets row_factory = sqlite3.Row, so get_task_token_summary's named-column reads resolve — the resume-bypass the method exists to close is actually closed, and the test writes a real row and asserts $3.00 back. The row_factory change is safe: the only other index-access reader (conductor.py:996) uses its own raw connection and index access (works under Row too), and nothing isinstance-checks rows as tuples. The outcome-detection refactor correctly stops false-firing on priced Anthropic models.

One new narrow gap introduced by the outcome-detection refactor — the headline feature goes inert again in a specific resume case:

severity	file:line	finding
minor	codeframe/core/react_agent.py:1086	Unmeasurable detection keys off spent (prior + current), so a resumed task that switches priced→unpriced keeps non-zero spent from prior runs and the guard never fires — cap inert for the resumed run (real-money false-negative the prior membership guard would have caught).

No null/error-path crashes introduced, no injection/authz/SSRF/traversal issues, no resource leaks or unbounded loops. The connection in _load_prior_task_cost is closed in finally; summary.get(...) or 0.0 is None-safe; _BLOCKED_REASONS correctly maps cost_cap_exceeded to BLOCKED in both loops.