diff --git a/codeframe/core/react_agent.py b/codeframe/core/react_agent.py index dc1e19ba..5a786bc8 100644 --- a/codeframe/core/react_agent.py +++ b/codeframe/core/react_agent.py @@ -55,6 +55,16 @@ # Reason string emitted when a stall timeout triggers a blocker — used to # set the correct BlockerOrigin ("system") vs agent-generated blockers. _REASON_STALL_DETECTED = "stall_detected" +_REASON_COST_CAP_EXCEEDED = "cost_cap_exceeded" + +#: Verification-failure reasons that mean "a blocker was created", so the run is +#: BLOCKED rather than FAILED. Anything not listed falls through to FAILED — +#: which would leave a blocker attached to a failed run. +_BLOCKED_REASONS = frozenset({ + "escalated_to_blocker", + _REASON_STALL_DETECTED, + _REASON_COST_CAP_EXCEEDED, +}) # Map tool names to agent phases for progress reporting. _TOOL_PHASE_MAP = { @@ -136,6 +146,8 @@ def __init__( self.workspace = workspace self.llm_provider = llm_provider self.max_iterations = max_iterations + self._max_cost_usd: Optional[float] = None + self._prior_task_cost_usd: float = 0.0 self.max_verification_retries = max_verification_retries self._stall_timeout_s = stall_timeout_s self._stall_action = stall_action @@ -205,6 +217,10 @@ def run(self, task_id: str) -> AgentStatus: packager = TaskContextPackager(self.workspace) context = packager.load_context(task_id) + self._max_cost_usd = self._resolve_cost_cap() + if self._max_cost_usd is not None: + self._prior_task_cost_usd = self._load_prior_task_cost(task_id) + # Adaptive budget based on task complexity adaptive = self._calculate_adaptive_budget(context) if adaptive != self.max_iterations: @@ -259,7 +275,7 @@ def run(self, task_id: str) -> AgentStatus: self._emit_stream_completion(task_id) return AgentStatus.COMPLETED - if reason == "escalated_to_blocker" or reason == _REASON_STALL_DETECTED: + if reason in _BLOCKED_REASONS: self._emit(EventType.AGENT_FAILED, { "task_id": task_id, "reason": "blocked", @@ -464,6 +480,21 @@ def _react_loop(self, system_prompt: str) -> AgentStatus: ) return AgentStatus.BLOCKED + # Spend cap (#911). Checked before each LLM call: 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". + over = self._cost_cap_message() + if over is not None: + self._verbose_print(f"[ReactAgent] {over}") + # BLOCKED rather than FAILED — the work is not wrong, it needs a + # human decision (raise the cap, or stop), which is what a + # blocker is for. + self._create_text_blocker( + f"Task stopped after {iterations} iteration(s). {over}", + _REASON_COST_CAP_EXCEEDED, + ) + return AgentStatus.BLOCKED + self._verbose_print(f"[ReactAgent] Iteration {iterations + 1}/{self.max_iterations}") self._emit(EventType.AGENT_ITERATION_STARTED, { "task_id": self._current_task_id, @@ -770,6 +801,19 @@ def _run_final_verification( ] for _turn in range(max_fix_turns): + # The correction loop spends too. Without this a run could sit + # at $4.99 under a $5 cap, fail verification, and then spend + # max_verification_retries * max_fix_turns more calls — a cap + # the correction loop ignores is not a cap (#911 review). + over = self._cost_cap_message() + if over is not None: + self._verbose_print(f"[ReactAgent] Verification fixes stopped. {over}") + self._create_text_blocker( + f"Verification fixes stopped. {over}", + _REASON_COST_CAP_EXCEEDED, + ) + return (False, _REASON_COST_CAP_EXCEEDED) + response = self.llm_provider.complete( messages=fix_messages, purpose=Purpose.CORRECTION, @@ -1001,6 +1045,120 @@ def _run_lint_on_file(self, rel_path: str) -> str: return "" + def _tokens_recorded(self) -> int: + """Total tokens this run has actually spent.""" + return sum( + r["input_tokens"] + r["output_tokens"] for r in self._token_records + ) + + def _models_used(self) -> set[str]: + """Model names seen in this run's token records.""" + return {r["model"] for r in self._token_records} + + def _cost_cap_message(self) -> Optional[str]: + """Why spending must stop, or None to continue. + + The single place any spending loop asks "may I make another call?" — + the main ReAct loop and the verification self-correction loop both go + through it, because a cap the correction loop ignores is not a cap + (#911 review). + """ + cap = self._max_cost_usd + if cap is None: + return None + + # A cap we cannot measure is not a cap. MetricsTracker.calculate_cost + # returns $0.00 for models it has no pricing for, so with e.g. + # --llm-provider openai the guard below would see $0.00 forever and + # never fire — the same silently-inert control this issue is about, one + # layer down. Refuse before spending rather than pretend (#911 review). + spent = self._prior_task_cost_usd + self._estimate_total_cost() + + # A cap we cannot measure is not a cap. `calculate_cost` returns $0.00 + # for models it has no pricing for, so the check below would see $0.00 + # forever and never fire — the same silently-inert control this issue is + # about, one layer down. + # + # Detected by *outcome* (tokens recorded but zero cost) rather than by + # MODEL_PRICING membership: the table holds three keys, so a membership + # test false-fires on ordinary Anthropic models like the default + # claude-haiku-4-5 and blocks a legitimately-priced run (#911 review). + if spent == 0.0 and self._tokens_recorded() > 0: + return ( + f"A cost cap of ${cap:.2f} is configured, but spend cannot be " + f"measured for {', '.join(sorted(self._models_used())) or 'this model'} " + "— no pricing data. Remove the cap, or use a model with known pricing." + ) + + if spent >= cap: + return ( + f"Estimated spend ${spent:.4f} reached the configured cap of " + f"${cap:.2f}. Raise 'Max cost per task (USD)' in Settings to continue." + ) + return None + + def _load_prior_task_cost(self, task_id: str) -> float: + """Spend already recorded against this task by earlier runs. + + The setting is "max cost per *task*", and ``_estimate_total_cost`` only + sums this agent instance's in-memory records. Without this, answering + the blocker and resuming would hand the task a fresh full budget every + time — a cap trivially bypassed by clicking resume (#911 review). + """ + conn = None + try: + from codeframe.platform_store.repositories.token_repository import ( + TokenRepository, + ) + + # Same per-workspace connection the write path uses + # (`_persist_token_usage`). `Database()` is control-plane and needs + # an explicit db_path — calling it bare raised TypeError straight + # into the except below, so this method silently returned 0.0 and + # the resume bypass it exists to close stayed wide open. The + # repository sets the Row factory it needs on this connection. + conn = sqlite3.connect(str(self.workspace.db_path)) + summary = TokenRepository(sync_conn=conn).get_task_token_summary(task_id) + return float(summary.get("total_cost_usd") or 0.0) + except Exception as exc: # pragma: no cover - best effort + logger.debug("Could not read prior spend for task %s: %s", task_id, exc) + return 0.0 + finally: + if conn is not None: + try: + conn.close() + except Exception: + pass + + def _resolve_cost_cap(self) -> Optional[float]: + """The configured per-task spend cap, or None when unset. + + Read from the same ``.codeframe/config.yaml`` the Settings page writes + (``max_cost_usd``). Before #911 nothing read it: a user could set a $5 + cap and get no cost limiting at all, while the sibling ``max_turns`` + control genuinely worked — so an inert cap was indistinguishable from a + working one. + """ + from codeframe.core.config import load_environment_config + + try: + env_config = load_environment_config(self.workspace.repo_path) + except Exception as exc: # pragma: no cover - config read is best effort + logger.warning("Could not read the cost cap: %s", exc) + return None + + cap = getattr(env_config, "max_cost_usd", None) if env_config else None + if cap is None: + return None + try: + cap = float(cap) + except (TypeError, ValueError): + logger.warning("Ignoring non-numeric max_cost_usd: %r", cap) + return None + # 0 would stop before the first call; treat it as "no cap" rather than + # bricking every run, and let validation reject negatives upstream. + return cap if cap > 0 else None + def _calculate_adaptive_budget(self, context: TaskContext) -> int: """Calculate iteration budget based on task complexity. diff --git a/codeframe/platform_store/repositories/base.py b/codeframe/platform_store/repositories/base.py index e6936eba..8ac03246 100644 --- a/codeframe/platform_store/repositories/base.py +++ b/codeframe/platform_store/repositories/base.py @@ -44,6 +44,17 @@ def __init__( if sync_conn is None and async_conn is None: raise ValueError("At least one connection (sync or async) must be provided") + if sync_conn is not None: + # Repository methods read columns by name (row["total_tokens"]), so + # they need a Row factory. `Database` sets this on its own + # connection, which is why they work when reached that way — a + # caller passing a bare `sqlite3.connect()` got tuples and a + # `TypeError: tuple indices must be integers` from the first named + # access (#911 review). Set it here so every caller is correct by + # construction. sqlite3.Row still supports integer indexing and + # iteration, so tuple-style access keeps working. + sync_conn.row_factory = sqlite3.Row + self.conn = sync_conn # For backward compatibility self._async_conn = async_conn self._database = database # Reference to parent Database instance diff --git a/tests/core/test_cost_cap_enforced_911.py b/tests/core/test_cost_cap_enforced_911.py new file mode 100644 index 00000000..20889dac --- /dev/null +++ b/tests/core/test_cost_cap_enforced_911.py @@ -0,0 +1,329 @@ +"""The 'Max cost per task (USD)' setting actually caps spend (#911). + +Settings → Agent rendered the control, the API persisted ``max_cost_usd``, and a +full-repo grep found the field only in ``config.py``, ``ui/models.py``, +``settings_v2.py`` and the frontend — no runtime, ``react_agent``, ``conductor`` +or adapter read it. A user who set a $5 cap got **zero** cost limiting, and the +sibling ``max_turns`` control genuinely worked, so an inert cap was +indistinguishable from a working one. + +For a paid product a spend cap that silently does nothing is worse than no cap. +""" + +import pytest + +from codeframe.core.config import ( + EnvironmentConfig, + load_environment_config, + save_environment_config, +) +from codeframe.core.react_agent import ReactAgent +from codeframe.core.workspace import create_or_load_workspace + +pytestmark = pytest.mark.v2 + + +@pytest.fixture +def workspace(tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + return create_or_load_workspace(repo) + + +def _agent(workspace) -> ReactAgent: + """An agent with no LLM: every test here stops before a call is made.""" + return ReactAgent(workspace=workspace, llm_provider=None) + + +def _set_cap(workspace, cap) -> None: + save_environment_config(workspace.repo_path, EnvironmentConfig(max_cost_usd=cap)) + + +# --------------------------------------------------------------------------- +# The setting is read at all +# --------------------------------------------------------------------------- + + +def test_the_cap_is_read_from_the_config_the_settings_page_writes(workspace): + """Settings → Agent PUTs to the same file this reads.""" + _set_cap(workspace, 5.0) + + assert _agent(workspace)._resolve_cost_cap() == 5.0 + + +def test_no_cap_configured_means_no_limit(workspace): + save_environment_config(workspace.repo_path, EnvironmentConfig()) + + assert _agent(workspace)._resolve_cost_cap() is None + + +def test_a_zero_cap_is_treated_as_unset(workspace): + """0 would stop before the first call and brick every run.""" + _set_cap(workspace, 0) + + assert _agent(workspace)._resolve_cost_cap() is None + + +def test_a_malformed_cap_does_not_break_the_run(workspace, monkeypatch): + """A hand-edited config must not take the agent down.""" + import codeframe.core.config as config_module + + monkeypatch.setattr( + config_module, + "load_environment_config", + lambda p: EnvironmentConfig(max_cost_usd="five dollars"), # type: ignore[arg-type] + ) + + assert _agent(workspace)._resolve_cost_cap() is None + + +# --------------------------------------------------------------------------- +# The cap actually stops the run +# --------------------------------------------------------------------------- + + +def test_exceeding_the_cap_blocks_the_run(workspace, monkeypatch): + """The headline behaviour: spend over the cap terminates the loop.""" + _set_cap(workspace, 1.0) + agent = _agent(workspace) + + # Pretend a prior iteration already spent past the cap. + monkeypatch.setattr(agent, "_estimate_total_cost", lambda: 1.25) + created: list[tuple[str, str]] = [] + monkeypatch.setattr( + agent, "_create_text_blocker", lambda text, reason: created.append((text, reason)) + ) + + from codeframe.core.react_agent import AgentStatus + + agent._max_cost_usd = agent._resolve_cost_cap() + status = agent._react_loop("do the thing") + + assert status == AgentStatus.BLOCKED + assert created, "no blocker was created" + text, reason = created[0] + assert reason == "cost_cap_exceeded" + assert "1.25" in text and "1.00" in text, text + + +def test_the_llm_is_never_called_once_the_cap_is_reached(workspace, monkeypatch): + """Stopping *before* the next call is the point — an after-the-fact check + would spend another call's worth every time.""" + _set_cap(workspace, 1.0) + + calls: list[str] = [] + + class ExplodingProvider: + def complete(self, *args, **kwargs): + calls.append("called") + raise AssertionError("the LLM was called after the cap was reached") + + agent = ReactAgent(workspace=workspace, llm_provider=ExplodingProvider()) + monkeypatch.setattr(agent, "_estimate_total_cost", lambda: 99.0) + monkeypatch.setattr(agent, "_create_text_blocker", lambda text, reason: None) + agent._max_cost_usd = agent._resolve_cost_cap() + + agent._react_loop("do the thing") + + assert calls == [] + + +def test_spend_under_the_cap_does_not_block(workspace, monkeypatch): + """The guard must not stop runs that are within budget.""" + _set_cap(workspace, 10.0) + agent = _agent(workspace) + + monkeypatch.setattr(agent, "_estimate_total_cost", lambda: 0.5) + blocked: list[str] = [] + monkeypatch.setattr( + agent, "_create_text_blocker", lambda text, reason: blocked.append(reason) + ) + + class OneShotProvider: + def complete(self, *args, **kwargs): + raise RuntimeError("stop here — we only needed to get past the cap check") + + agent.llm_provider = OneShotProvider() + agent._max_cost_usd = agent._resolve_cost_cap() + # `_react_loop` is normally entered from `run()`, which sets this up. + agent._current_task_id = "task-1" + + with pytest.raises(RuntimeError, match="stop here"): + agent._react_loop("do the thing") + + assert "cost_cap_exceeded" not in blocked + + +# --------------------------------------------------------------------------- +# No UI control persists a value nothing reads +# --------------------------------------------------------------------------- + + +def test_the_persisted_setting_round_trips_to_the_enforcer(workspace): + """End to end from the API's write path to the agent's read path.""" + config = load_environment_config(workspace.repo_path) or EnvironmentConfig() + config.max_cost_usd = 2.5 # what PUT /api/v2/settings does + save_environment_config(workspace.repo_path, config) + + assert _agent(workspace)._resolve_cost_cap() == 2.5 + + +# --------------------------------------------------------------------------- +# Review findings: every spending loop, per-task, and measurability +# --------------------------------------------------------------------------- + + +def test_the_verification_fix_loop_also_respects_the_cap(workspace, monkeypatch): + """A run can sit just under the cap, fail verification, then spend + max_verification_retries * max_fix_turns more calls. A cap the correction + loop ignores is not a cap. + + Drives the real `_run_final_verification` — asserting only that + `_cost_cap_message()` returns something would test the helper, not the + loop's use of it, and would pass with the guard deleted. + """ + from codeframe.core import gates as core_gates + from codeframe.core.react_agent import _REASON_COST_CAP_EXCEEDED + + _set_cap(workspace, 1.0) + + calls: list[str] = [] + + class ExplodingProvider: + def complete(self, *args, **kwargs): + calls.append("called") + raise AssertionError("spent past the cap during verification fixes") + + agent = ReactAgent(workspace=workspace, llm_provider=ExplodingProvider()) + agent._max_cost_usd = agent._resolve_cost_cap() + agent._current_task_id = "task-1" + monkeypatch.setattr(agent, "_estimate_total_cost", lambda: 5.0) + + # Gates fail, so verification enters the LLM fix loop. + monkeypatch.setattr( + core_gates, + "run", + lambda *a, **k: core_gates.GateResult( + passed=False, + checks=[ + core_gates.GateCheck( + name="pytest", + status=core_gates.GateStatus.FAILED, + output="1 failed", + ) + ], + ), + ) + monkeypatch.setattr(agent, "_try_quick_fix", lambda summary: False) + + blockers: list[str] = [] + monkeypatch.setattr( + agent, "_create_text_blocker", lambda text, reason: blockers.append(reason) + ) + + ok, reason = agent._run_final_verification("do the thing") + + assert calls == [], "the LLM was called after the cap was reached" + assert ok is False + assert reason == _REASON_COST_CAP_EXCEEDED + assert _REASON_COST_CAP_EXCEEDED in blockers + + +def test_prior_spend_on_the_task_counts_toward_the_cap(workspace, monkeypatch): + """The setting is per *task*. Without this, answering the blocker and + resuming hands the task a fresh full budget every time.""" + _set_cap(workspace, 5.0) + agent = _agent(workspace) + agent._max_cost_usd = agent._resolve_cost_cap() + + # This run has spent almost nothing; an earlier run spent most of the cap. + monkeypatch.setattr(agent, "_estimate_total_cost", lambda: 0.10) + agent._prior_task_cost_usd = 4.95 + + message = agent._cost_cap_message() + + assert message is not None, "prior spend was ignored; the cap resets on resume" + assert "5.05" in message + + +def test_prior_spend_is_read_from_the_persisted_task_total(workspace): + """Writes a real usage row and reads it back. + + The first version of this test only asserted `== 0.0` for an unknown task, + so it passed whether the read worked or raised — and it did raise: + `Database()` needs an explicit db_path, and the broad `except` swallowed the + TypeError, leaving prior spend permanently 0.0. + """ + import sqlite3 + + from codeframe.lib.metrics_tracker import MetricsTracker + from codeframe.platform_store.repositories.token_repository import TokenRepository + + conn = sqlite3.connect(str(workspace.db_path)) + try: + tracker = MetricsTracker(db=TokenRepository(sync_conn=conn)) + tracker.record_token_usage_sync( + task_id="task-prior", + agent_id="react", + project_id=1, + model_name="claude-sonnet-4-5", + input_tokens=1_000_000, + output_tokens=0, + ) + conn.commit() + finally: + conn.close() + + prior = _agent(workspace)._load_prior_task_cost("task-prior") + + assert prior > 0.0, "the persisted spend was not read back" + assert prior == pytest.approx(3.0, rel=0.01) # $3.00/MTok input + + +def test_an_unknown_task_has_no_prior_spend(workspace): + assert _agent(workspace)._load_prior_task_cost("no-such-task") == 0.0 + + +def test_a_cap_on_an_unpriced_model_refuses_rather_than_never_firing( + workspace, monkeypatch +): + """calculate_cost returns $0.00 for unknown models, so with e.g. + --llm-provider openai the guard would see $0.00 forever and never fire — + the same silently-inert control this issue is about, one layer down.""" + _set_cap(workspace, 1.0) + agent = _agent(workspace) + agent._max_cost_usd = agent._resolve_cost_cap() + + agent._token_records.append( + {"model": "gpt-4o", "input_tokens": 100000, "output_tokens": 50000, + "call_type": "execution", "iteration": 1} + ) + + message = agent._cost_cap_message() + + assert message is not None, "an unmeasurable cap silently allowed unbounded spend" + assert "gpt-4o" in message + assert "pricing" in message + + +def test_a_priced_model_under_the_cap_still_runs(workspace): + """The measurability guard must not block ordinary Anthropic runs.""" + _set_cap(workspace, 100.0) + agent = _agent(workspace) + agent._max_cost_usd = agent._resolve_cost_cap() + + agent._token_records.append( + {"model": "claude-sonnet-4-5", "input_tokens": 1000, "output_tokens": 500, + "call_type": "execution", "iteration": 1} + ) + + assert agent._cost_cap_message() is None + + +def test_the_cap_reason_maps_to_blocked_not_failed(): + """A cap hit during verification creates a blocker, so the run must be + BLOCKED. Any reason not in this set falls through to FAILED, which would + leave a blocker attached to a failed run.""" + from codeframe.core.react_agent import _BLOCKED_REASONS, _REASON_COST_CAP_EXCEEDED + + assert _REASON_COST_CAP_EXCEEDED in _BLOCKED_REASONS diff --git a/web-ui/src/app/settings/page.tsx b/web-ui/src/app/settings/page.tsx index 9968c0dc..5794a529 100644 --- a/web-ui/src/app/settings/page.tsx +++ b/web-ui/src/app/settings/page.tsx @@ -335,6 +335,11 @@ function AgentSettingsForm({ if (!Number.isNaN(v)) onMaxCostChange(v); }} /> +

+ 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. +