Skip to content

Add configurable agent tool budgets (Max Steps per Task, Max Auto-Continues) - #245

Open
ianu82 wants to merge 3 commits into
stagingfrom
tool-budget-settings
Open

Add configurable agent tool budgets (Max Steps per Task, Max Auto-Continues)#245
ianu82 wants to merge 3 commits into
stagingfrom
tool-budget-settings

Conversation

@ianu82

@ianu82 ianu82 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

What

Anton's built-in loop budgets (max_tool_rounds=25, max_continuations=3) are tuned for an interactive CLI, where stopping to ask "want me to continue?" is cheap. In Cowork that same check-in is a dead end whenever nobody is watching — channels, scheduled work, evals — and real knowledge-work tasks hit it routinely. In our GDPval benchmark runs, tasks stopped mid-way at anton's defaults (0/3 completed); with more headroom the same tasks finished cleanly (3/3, and the heaviest needed ~50 tool calls).

This PR makes both budgets proper Cowork settings:

  • Max Steps per Task — default 50 (bounds 5–500)
  • Max Auto-Continues — default 5 (bounds 0–25)

The defaults apply to every user immediately (unset settings resolve to field defaults). Values are editable via the settings API today, and a Settings-UI section ships separately in the desktop app (mindsdb/cowork PR).

How it works

One overlay line per field in the anton-harness bridge: the DB-authoritative value is set on the AntonSettings object each conversation, exactly like act_first and the memory flags. No anton changes — the fields already exist on anton's CoreSettings; Cowork just runs them higher.

Deliberate decisions reviewers should know about

  • ANTON_MAX_TOOL_ROUNDS / ANTON_MAX_CONTINUATIONS are NOT synced from .env (a NOTE in migrations.py explains why, and a regression test locks it in). Anton's own settings accept any int, so a stale CLI line like ANTON_MAX_TOOL_ROUNDS=1000 is valid for anton but out of bounds here — and sync_env_vars_to_db raises on the first invalid key, which would 400 every credential push / token refresh. This is the same class of bug that got models excluded from the table (ENG-739). Please don't re-add the mapping.
  • Cowork sessions now take budgets from the DB, always. Anyone who tuned these via ~/.cowork/.env moves onto the DB default (50/5) until they set a value through the settings API/UI. The .env lines still work for the standalone anton CLI.
  • Anton harness only. Hermes reads named fields and ignores these; the field descriptions say so.
  • The connector prober keeps anton's conservative defaults — it passes no settings object, by design.

Testing

tests/test_agent_budget_settings.py (9 tests): defaults, string→int coercion, bounds rejection, settings-service round trip (with shared-DB cleanup), the env-sync exclusion guard, overlay compatibility against the pinned anton, and a tripwire that fails if anton ever raises its own defaults above Cowork's. Full suite on this branch (rebased onto staging): 678 passed, 1 skipped; the one deselected failure (test_comments_layer.py::test_serve_injects_only_with_flag) fails identically on clean staging.

The change was also adversarially reviewed (independent review agents attacking the diff); the .env-sync exclusion above is the direct result of a blocker that review caught and reproduced.

🤖 Generated with Claude Code

ianu82 and others added 2 commits July 28, 2026 13:50
…tions=5)

Long knowledge-work tasks routinely need 30-50 tool rounds; at anton's
CLI defaults (25/3) the agent stops mid-task and asks permission to
continue, which is a dead end in unattended contexts. Expose both knobs
as user settings with Cowork-appropriate defaults, overlay them onto
AntonSettings in the harness bridge, and map the ANTON_ env vars for
first-boot .env seeding. The connector prober intentionally keeps
anton's conservative defaults (it passes no settings object).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tests

Adversarial review findings: mapping ANTON_MAX_TOOL_ROUNDS in
_ENV_TO_SETTING re-created the ENG-739 re-pin bug with a harder failure
mode — anton CoreSettings accepts any int, so a stale CLI line like
ANTON_MAX_TOOL_ROUNDS=1000 in the shared ~/.cowork/.env fails
UserSettings' bounds inside sync_env_vars_to_db, which raises before
bulk_upsert and would 400 every credential push / token refresh.
Budgets now enter the DB only via explicit writes (UI / API); a
regression test locks the exclusion in. Tests are isolated from the
developer's environment (ANTON_* env vars, ~/.anton/.env) and clean up
the shared test DB in finally. Field descriptions note the knobs apply
to the Anton harness (hermes ignores them).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

No PR environment for this pull request

Add the deploy label and push to create one. It is torn down when the label is removed or the PR closes, so any URL you saw here earlier is gone.

Updated on every push to this PR.

@ianu82
ianu82 requested a review from ea-rus July 28, 2026 13:38
@alecantu7
alecantu7 self-requested a review July 28, 2026 18:37

@alecantu7 alecantu7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at abbac6b7 alongside the client half (mindsdb/cowork#514), checking both against each other and against the pinned anton. tests/test_agent_budget_settings.py11 passed. The full suite doesn't run in my environment (my venv has an older antonModuleNotFoundError: anton.publish_access on an unrelated test file), so I leaned on CI: run-unit-tests is green.

The implementation is about as small as this could be and the decisions are well-reasoned. Things I verified rather than took on trust:

  • Field names and defaults line up with the pinned anton. anton/core/settings.py on origin/main has max_tool_rounds: int = 25 and max_continuations: int = 3, so the overlay targets real fields and the "Cowork runs them higher" framing is accurate. The tripwire test that fails if anton ever raises its own defaults above ours is a nice touch.
  • The .env-sync exclusion is correct, and the stated failure mode is real. sync_env_vars_to_db validates every mapped key in a loop and raise ValueError(...) before it reaches bulk_upsert, so a single stale ANTON_MAX_TOOL_ROUNDS=1000 really would fail the whole call — and every credential push / token refresh that goes through it. The NOTE plus the regression test is the right way to lock that in; agreed on not re-adding the mapping.
  • max_continuations=0 genuinely stops the retry loopsession.py:2682 is if continuation >= self._max_continuations:, so 0 takes the exhausted branch on the first check.
  • Client/server contract matches exactly: cowork#514's BUDGET_FIELDS and its inline hints carry the same names, bounds (5–500, 0–25) and defaults (50, 5).

The thing I'd want stated in the PR before this ships is the downside case, because the caps are currently load-bearing for several open bugs.

The evidence here (GDPval 0/3 → 3/3, heaviest task ~50 calls) measures the upside on tasks that can finish. What it doesn't measure is what these numbers do to tasks that can't — and the round cap is precisely the mechanism that ends those today. Worst case per user request goes from (1+3) × 25 = 100 tool rounds to (1+5) × 50 = 300 — a 3× rise in the ceiling, on a change that applies to every user immediately.

Three cases from the record where the cap was the thing that stopped the burn:

  • Mynda Treacy (ENG-1081 family): hosted web, 190k context, TPM-429 throttling, and the session terminated by hitting the 25-round cap. At 50 she spends roughly twice as long inside the throttle for the same non-result — and per the note below she's in the population that can't turn it down.
  • salesteam@kiranam.com: ~5M tokens on a dashboard build blocked by an Azure-SQL environment wall. No number of rounds finishes that task; the cap bounded the bill.
  • ENG-296 / ENG-716: the completion-verifier loop is the continuation loop at session.py:2682 that max_continuations sizes. ENG-716 is structural and open (the prompt already says "never ask and act in the same turn"; the loop overrides it), so raising this widens a known-buggy loop rather than a clean one.

None of that argues against the change — the interactive-CLI-vs-unattended-work reasoning is right, and 25/3 is clearly too tight for channels and scheduled work. It argues for the PR saying out loud what the new worst case costs, so whoever sees the next runaway-spend case knows this landed. Inline I've suggested one concrete mitigation worth considering (a lower default on the surface where users have no control) and two small copy/description fixes.

# Overlaid onto AntonSettings per conversation (anton_harness.harness);
# anton's own CLI defaults (25/3) are lower — Cowork deliberately runs
# with more headroom so long tasks finish without a mid-task check-in.
max_tool_rounds: int = Field(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth considering a different default for the hosted surface, because the escape hatch this PR assumes doesn't exist there.

I checked cowork's current staging rather than going from memory: the only two Settings entry points are onNavigate('settings:backend') and onNavigate('settings:agent') in Sidebar.jsx (lines 845 / 856), and both sit inside the {!host.isWeb && (…)} footer block at line 816. So on the hosted web build Settings is unreachable entirely — and #514's Advanced Settings section lives inside that same view, so it ships desktop-only by construction.

That means hosted users get 50/5 immediately with no UI control at all (only a raw PUT /settings/{key} from a browser console, which isn't a user affordance). It's also the surface where the cost lands on us rather than on a BYOK user's own key, and where ENG-878's 500k TPM ceiling bites hardest — a longer-running session is more 429-exposed, not less, and cache reads count at full weight.

Options, roughly in order of effort: leave it and note the asymmetry in the PR; pick a more conservative hosted default via app settings; or unhide just this section on web in #514. I'd at least not let it ship silently — the users least able to tune this are the ones whose bill we pay.

Separately, a description nit: "Applies to the Anton agent" is true but buries the surprising part. Because this field has a non-None default, the overlay at harness.py:419 fires on every conversation, so ANTON_MAX_TOOL_ROUNDS in ~/.cowork/.env is now dead for Cowork sessions even for users who never touch the DB setting. That's called out in the PR body; it belongs in the field description too, since that's what surfaces in the API.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this shouldn't ship silently — and thanks for checking the sidebar gating against staging rather than assuming. Went with your middle option in b3537b8: the defaults now come from app settings (COWORK_DEFAULT_MAX_TOOL_ROUNDS / COWORK_DEFAULT_MAX_CONTINUATIONS, mirroring the channels_harness pattern), so hosted deployments get an env lever to run more conservative values for users who never set the per-user setting — no per-user rows touched, desktop/BYOK unchanged at 50/5. Whether hosted should actually dial it down (and to what) is a deploy-config decision that can now be made without a code change; flagging ENG-878's TPM ceiling in that discussion makes sense. Also added the env-var-precedence note to both field descriptions as suggested.

Comment thread cowork/common/settings/user_settings.py Outdated
description=(
"When the agent stops but its work looks unfinished, Cowork can send "
"it back to complete the job — this caps how many times. Raise it "
"for hands-off thoroughness; set 0 to always stop at the first "

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy accuracy: "set 0 to always stop at the first draft" isn't quite what 0 does. At max_continuations=0, session.py:2682 takes the budget-exhausted branch on the first check and appends a SYSTEM turn instructing the agent to summarize what was accomplished, identify the blocker, and suggest next steps — so the user still gets one extra LLM round producing a diagnosis, not a bare first draft.

Closer: "set 0 to stop after the first attempt (you'll still get a summary of what's missing)". Small, but this PR is otherwise precise about copy and the same string ships in #514's UI, so it's worth getting right in both places.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — at 0 the budget-exhausted branch still produces a diagnosis turn, not a bare draft. Fixed the copy in both field descriptions to your wording ("set 0 to stop after the first attempt — you'll still get a summary of what's missing") and mirrored it in the #514 UI subtitle.

"coding_provider", "coding_model",
"memory_enabled", "memory_mode",
"episodic_memory", "proactive_dashboards", "act_first",
"max_tool_rounds", "max_continuations",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth a comment here noting that these two differ in kind from everything above them in this tuple. The other entries (memory_enabled, act_first, the provider/model pairs) are meaningfully nullable — db_val is None means "user hasn't chosen, leave anton's value alone". These two are int with non-None defaults, so getattr can never return None and the overlay is unconditional: anton's own 25/3 are now unreachable in Cowork, by design.

That's the intent and the PR body says so, but the shared loop makes it read like the same opt-in overlay as its neighbours. One line (# non-nullable: always overrides anton's own defaults) saves the next reader the trip through UserSettings.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added — the two entries now carry a comment marking them as non-nullable always-override, in contrast to the opt-in nullable neighbours.

Comment thread cowork/migrations.py
"ANTON_EPISODIC_MEMORY": "episodic_memory",
"ANTON_PROACTIVE_DASHBOARDS": "proactive_dashboards",
"ANTON_ACT_FIRST": "act_first",
# NOTE: ANTON_MAX_TOOL_ROUNDS / ANTON_MAX_CONTINUATIONS are deliberately

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with this, and I verified the mechanism rather than just the reasoning: sync_env_vars_to_db builds updates, then loops svc._validate_key(key) + UserSettings.model_validate({key: value}) and re-raises as ValueError — all before bulk_upsert. So one out-of-bounds .env line fails the entire call, not just its own key, and every caller in the credential-push / token-refresh path takes it with it. The NOTE explains the why well enough that nobody should re-add the mapping by accident, and test_budget_env_vars_not_synced_from_dotenv makes it stick.

One thought for the ENG-739 comparison: models were excluded for a related-but-different reason (stale .env clobbering a picker choice). Since the same NOTE now covers two distinct exclusion rationales, it might be worth a short paragraph at the top of _ENV_TO_SETTING stating the general rule — "a key belongs here only if every value anton's own settings accept is also valid for UserSettings" — so the next field gets classified without re-deriving it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the general rule as a paragraph at the top of _ENV_TO_SETTING: a key belongs in the table only if every anton-CLI-valid value is also UserSettings-valid (the sync raises on first failure), and re-syncing a stale line can never override an in-product choice (ENG-739). Thanks for verifying the raise-before-bulk_upsert mechanism independently.

… comments

- Budget defaults now come from app settings (COWORK_DEFAULT_MAX_TOOL_ROUNDS /
  COWORK_DEFAULT_MAX_CONTINUATIONS): hosted deployments have no Settings UI
  (sidebar entries are desktop-only) and pay for inference themselves, so they
  get an env lever to run more conservative defaults without touching
  per-user rows. Desktop/BYOK behavior unchanged (50/5).
- Field descriptions now state that these replace the ANTON_* env vars for
  Cowork sessions, and the max_continuations=0 copy matches what 0 actually
  does (stop after the first attempt, with a summary of what's missing).
- Overlay-loop comment marks the two budget entries as non-nullable /
  always-override, unlike their opt-in neighbours.
- _ENV_TO_SETTING now opens with the general inclusion rule so the next field
  gets classified without re-deriving ENG-739 + the budget exclusion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@alecantu7 alecantu7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at b3537b8. tests/test_agent_budget_settings.py → 12 passed (was 11).

The deployment lever is the right shape and it's implemented on the existing idiom rather than a new one — I checked, and channels_harness at user_settings.py:285 already uses the identical default_factory=lambda: get_app_settings()… pattern, so "mirroring the channels_harness pattern" is verifiably true rather than a claim.

The thing I most expected to break, doesn't: switching defaultdefault_factory makes UserSettings.model_fields[key].default return PydanticUndefined, and SettingService._to_response reads model_fields[key] — but only for .title and .description, with the value coming from getattr(settings, key). So the settings API response is unaffected. get_app_settings is @lru_cached, so the per-instantiation cost of the factory is nil and the tests' cache_clear() is the correct lever (the hasattr guard is belt-and-braces, but harmless).

Also good:

  • Tests are properly env-proofed now — the autouse fixture deletes COWORK_DEFAULT_* and clears the cache both before and after, so test_defaults_are_50_and_5 can't be flipped by whatever a developer happens to have exported.
  • test_hosted_deployments_can_lower_defaults_via_env also asserts an explicit per-user value still wins, which is the half that actually matters for the lever's semantics.
  • The _ENV_TO_SETTING inclusion rule and the non-nullable comment in the harness both landed, and the copy fix is consistent with #514.

One interlock worth naming explicitly, because it's load-bearing and currently implicit: excluding the budgets from _ENV_TO_SETTING is what keeps the hosted lever effective. The lever only applies to users with no per-user row, so any path that seeds a row — an env sync, a migration backfill — would silently pin those users and make COWORK_DEFAULT_* unreachable for them. Those two decisions were made for different reasons and now depend on each other; worth a line in the _ENV_TO_SETTING rule paragraph saying so, so nobody "helpfully" adds the budgets back later and quietly disables the deployment control.

The one thing still open is that the lever is inert as shipped — detail inline. Short version: nothing in deployment/ sets it, so merging this still ships 50/5 to hosted, which is the exposure the lever was added to address.

# these values. Hosted deployments (where inference cost lands on the
# operator and the Settings UI is unreachable — sidebar entries are
# desktop-only) can lower them without touching per-user rows.
default_max_tool_rounds: int = Field(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lever is real but nothing pulls it yet, so as shipped this PR doesn't change hosted behaviour — hosted still gets 50/5, which is the exposure it was added for.

The wiring exists and it's a one-line change: deployment/cowork-server/values.yaml carries the base COWORK_* env list (COWORK_SERVER_HOST, COWORK_LISTEN_PORT, COWORK_TENANCY_MODE, COWORK_MASTER_KEY) and the per-env files add overrides in the same shape — COWORK_IDENTITY_ENFORCE is set that way in values-prod.yaml / values-staging.yaml / values-dev.yaml. Neither COWORK_DEFAULT_MAX_TOOL_ROUNDS nor COWORK_DEFAULT_MAX_CONTINUATIONS appears anywhere under deployment/.

So the remaining decision is a number, not code: what should hosted actually run? Worth settling here rather than after the fact, since ENG-878's TPM ceiling means the cost of getting it wrong shows up as user-visible 429s and not just spend. Three ways to close it, any of which is fine by me:

  1. Set it in values-prod.yaml + values-staging.yaml in this PR (a value below 50/5 chosen deliberately).
  2. Add it commented-out with the intended value, so the next operator sees the control exists.
  3. Merge as-is and open a follow-up that owns the number — but then say in the PR body that hosted ships at 50/5 for now, so it's a recorded decision rather than an oversight.

Fair caveat: COWORK_CHANNELS_HARNESS isn't in deployment/ either, so undiscoverable levers are the existing norm here — I'm not asking you to fix that pattern, just noting this particular one is a cost and rate-limit control, which raises the stakes on it being findable.

# inference, no Settings UI on web) can lower them via env without
# touching per-user rows.
max_tool_rounds: int = Field(
default_factory=lambda: get_app_settings().default_max_tool_rounds,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified this doesn't have the failure mode I was worried about, recording it so nobody re-derives it: default_factory means UserSettings.model_fields["max_tool_rounds"].default is now PydanticUndefined rather than 50. SettingService._to_response does read model_fields[key], but only .title / .description, and takes the value from getattr(settings, key) — so GET /settings/ still reports the resolved number and is_set still comes from the DB rows. No behaviour change in the API surface, and #514's capability probe (presence of the key in the response) is unaffected.

One consequence worth a comment here rather than a change: because get_app_settings is @lru_cached, the deployment default is captured on the first call for the life of the process, so changing COWORK_DEFAULT_* needs a restart. That's normal for env config and matches channels_harness, but it's the kind of thing someone debugging "I changed the env and nothing happened" will want stated.

@alecantu7 alecantu7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction and an addendum, both from something I didn't know when I wrote the earlier reviews: ENG-932 is planned work to enable Settings on web (hide only the Backend section rather than all of Settings). It's in Backlog, not started, but it means the "hosted users can't reach Settings" premise I gave you is temporary.

What that retires: my framing of the asymmetry as "hosted users have no way to lower this." Once ENG-932 lands they will. So please don't write that reasoning into the code as a permanent justification — the comments in app_settings.py and user_settings.py currently say the Settings UI is unreachable and sidebar entries are desktop-only, which is true today and won't be after ENG-932. Same trap I already walked you into on #514, so I'd rather flag it twice than let it rot: state the durable reason instead ("deployment-level default for users who never set the per-user value") and leave the web-UI status out of it.

What it does not retire: the need for the lever at all. Defaults apply to everyone who never touches a setting, and that's the large majority of users on any surface — so COWORK_DEFAULT_* stays the right control and "what number should hosted run" is still the open question from my last review.

What it makes worse, and this is new: the lever sets a default, but nothing sets a ceiling. The bounds on max_tool_rounds (le=500) and max_continuations (le=25) are hardcoded in UserSettings and identical for every deployment. So the moment ENG-932 exposes Settings on web, any hosted user can raise their own budgets to 500 steps × 25 auto-continues — 10× the new default, on operator-paid inference, with no deployment-level way to prevent it. Today that's unreachable because the section is hidden; after ENG-932 it's two clicks. Detail inline.

None of this blocks the merge — the code is right and the change is inert on this axis until ENG-932. But the ceiling is much cheaper to add now, while you're already in these fields, than to retrofit after hosted users have rows at 500.

# these values. Hosted deployments (where inference cost lands on the
# operator and the Settings UI is unreachable — sidebar entries are
# desktop-only) can lower them without touching per-user rows.
default_max_tool_rounds: int = Field(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addendum to my earlier comment here, given ENG-932 (enable Settings on web) is planned: consider a deployment-level max alongside the default, not just a default.

As it stands default_max_tool_rounds bounds what a deployment defaults to, but UserSettings.max_tool_rounds is le=500 and max_continuations is le=25 for everyone — hardcoded, no per-deployment override. So after ENG-932 a hosted user can set 500 × 25 on our inference bill, ~10× whatever conservative default you pick here, and the operator has no lever to stop it. Desktop/BYOK is unaffected either way; there the user is spending their own money and should be allowed the full range.

The shape that fits what's already here: add max_max_tool_rounds / max_max_continuations (names welcome) to AppSettings, then clamp on write rather than in the field bounds — SettingService.upsert_setting already validates through UserSettings.model_validate and raises ValueError → 400, so rejecting an over-ceiling value there gives the client a clean error, and #514's inline range hint could read the effective max from the settings response instead of the hardcoded BUDGET_FIELDS spec.

If that's more than you want in this PR, entirely reasonable — but worth a line in the PR body or a follow-up ticket saying the ceiling is deliberately deferred, and worth flagging on ENG-932 as a precondition, since that's the ticket that actually opens the door. Retrofitting after hosted users have rows at 500 means either breaking their saved value or grandfathering it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants