[bug] Two autonomous entry points still start model work with no budget pool — #184 promised an admission check at every one

[serge-ivo]

The promise this misses

#184 ("Autonomous work budget", closed 2026-08-04) scoped itself explicitly:

Admission check at every autonomous entry point (cron, webhook, delivery, delegation) and on retry/replay.

and named the chokepoint:

Natural chokepoint for both: the BYOK/Workers-AI call path that already writes ai_usage.

The check did not land at that chokepoint. It landed in reserve(), which is reached only by callers that were handed a pool. Two autonomous entry points therefore start unattended model work with no pool, no reservation, and no account-ceiling check — the same escape #374 documented and fixed for the browser Loop, still open on other paths.

(This is orthogonal to #485. #485 turned the account ceilings observe-only; these paths would be uncovered either way, and that is precisely why they are worth naming separately.)

Verified coverage of the budget today

accountUsageSince has exactly one enforcement caller — workers/api/src/lib/delegation-budget-store.ts:305 inside reserve(). (lib/connectors/supervision.ts:334,638 and routes/budget.ts:221,284 read it only to report.) reserve() has exactly two callers: workflows/agent-loop.ts:88 and workflows/coding-session.ts:453. Grepped, not assumed.

Gap 1 — MCP coding_loop_start runs up to 50 unbudgeted iterations

workers/mcp/src/instance-tools/coding.ts:43-140. The tool runs the whole loop inline in the MCP server:

• :84 — POST /v1/instances/${id}/chat with the objective
• :105 — while (state.running && state.iteration < state.maxIterations)
• :112 — POST /v1/instances/${id}/loop-decide
• :139 — POST /v1/instances/${id}/chat with the returned instruction

Neither endpoint touches the budget. /loop-decide is workers/api/src/routes/instances-chat.ts:134-165: it authorizes the instance, validates the body, calls runLoopDecide, returns. No openBudget, no reserve, no accountUsageSince. /chat reaches AgentDO and never calls reserve() either.

So this is BYOK Claude deciding, and BYOK Claude acting, up to max_iterations (zod .max(50), :30) times, on the far side of every budget guard — the identical shape #374 described for the browser Loop ("/loop-decide reserves nothing and settles nothing. An N-iteration browser Loop is N unbudgeted BYOK Claude calls plus N Engine turns"), which was fixed by routing that button through POST /loop → loopDriverFor → the Pilot.

The bound that does exist is the iteration cap and the dry_run preview at :65-77, which honestly states the spend commitment. That makes this a bounded leak, not an unbounded one — but it is bounded by a number the caller chooses, not by the account's pool.

Gap 2 — POST /v1/instances/:id/coding/sessions/:sid/run starts a Pilot with no pool

workers/api/src/routes/coding.ts:625-641 creates the CODING_SESSION workflow with params: { instanceId, userId, sessionId, repoId, runnerNode, cloneUrl, branch, token, tokenUsername, goal, driverId } — no budgetId, no depth. Both are optional on the params type (workers/api/src/workflows/coding-session-params.ts:47,49).

The Pilot then skips the budget entirely, workers/api/src/workflows/coding-session.ts:451-452:

const budgetId = event.payload.budgetId ?? null;
if (!budgetId) return decideCodingAction(env, userId, p, { kind: "coding", instanceId });

The comment three lines above (:445-448) states the intent this defeats: "The LLM call is the one place this loop spends money, so it is the one place the budget has to sit. Delegated runs (#159) previously reached the Pilot with a pool id and never drew on it — unbounded spend on exactly the path a supervisor can trigger without a human watching." The delegated path was fixed; the direct one still has no pool to pass.

Reachability: no console or agents/coder/web caller greps out (the Coding tab's Loop goes through POST /loop since #374), and no MCP caller — MCP only uses capture/message/end/restart. It is a documented owner-authenticated API route (it is in the route table in CLAUDE.md), so an agent holding a session token, a script, or a future client reaches it.

Adjacent, worth noting but NOT a defect

lib/pipeline-run-start.ts:83 opens a pool only when pipelineOpensDelegations(pipeline). A cron- or webhook-triggered pipeline whose steps are plain ai_generate therefore runs unbudgeted. That looks deliberate (a pipeline's step count is declared and finite, unlike a loop's), but #184's scope named cron and webhook explicitly, so the owner should confirm it is a choice rather than an oversight.

What to do — cheapest first

1. Gap 2 is a two-line fix: openBudget(c.env, uid, instanceId) in the /run handler and pass budgetId + depth: 0 into the workflow params, exactly as lib/delegate-instance.ts:72 and routes/tools.ts:957 already do. Everything downstream already handles it.
2. Gap 1: make coding_loop_start a thin wrapper over the durable path — POST /v1/instances/:id/loop and then poll /loop/:runId — instead of reimplementing the loop in the MCP DO. It inherits the pool, the driver claim ([docs] Reconcile runtime docs with relay-only pags up #208) and merge authority ([safety] Nothing decides whether an agent may merge to main — three unattended merges already happened #314) for free, and the tool stops being a second orchestrator that can drift from lib/loop-orchestrator.ts. It also stops holding an MCP request open for 50 model calls.
   • Cheaper interim if that is too big: open a pool at the start of the tool and reserve/settle per iteration. Rejected as the primary fix — it is a third implementation of loop budgeting, and [bug] The Coding tab's Loop is still a browser poll — so merge authority, the driver claim and the spend budget all miss it #374's lesson was that the answer is one driver, not more guards bolted onto each copy.
3. The invariant that stops this recurring: a source test asserting that every path which drives a model N times without a human in the loop passes a budgetId. Today the rule is enforced by whichever call sites remembered.

Acceptance criteria

• Starting a Pilot via POST …/coding/sessions/:sid/run creates a delegation_budgets row and each decide increments delegations_used.
• coding_loop_start produces an agent_loop_runs row with a budget_id, and coding_loop_status reports iterations from that row rather than from MCP-DO memory.
• With BUDGET_ENFORCE=1 and a tripped account ceiling, both paths stop with the account_ceiling message instead of continuing.
• A test enumerates the autonomous entry points and fails when one is added without a pool.

Regression risk

Gap 2's fix makes a previously unbounded path bounded — a long unattended session that used to run freely can now stop at the per-tree limit (50 delegations / $5 charged). That is the intended behaviour, but it will read as a new failure the first time it happens; the message from reserve() already names which limit fired. Gap 1's fix changes coding_loop_start from synchronous-transcript to start-and-poll, which is a breaking change to the tool's return shape — the description at coding.ts:44 promises "Runs the WHOLE loop before returning", and workers/mcp/src/instance-tools.test.ts:734,811 assert against the current shape. Decide that deliberately; I would take it, because holding an MCP call open across 50 BYOK model calls is its own problem.

Found while auditing #485/#478. Related: #374 (same class, browser Loop), #184 (the promise), #159 (the delegated Pilot fix).

[serge-ivo]

Fixed in f79fb29, deployed (API + MCP + host all green; CI run 31558635412 including e2e).

Both gaps closed, plus a third of the same shape that the ticket did not name.

Gap 2 — POST …/coding/sessions/:sid/run

openBudget(c.env, uid, instanceId) before the workflow create, budgetId + depth: 0 into the params, and the pool id returned alongside workflowId. Two lines, as you said.

Gap 3 (new) — the Overseer's delegation to the Pilot

routes/coding-brains.ts:167 creates CODING_SESSION with no budgetId either — found while writing AC 4's guard, which failed on it. Same one-line fix. Worth naming separately because it is the least supervised of the three: a model decides to start it, not a human pressing a button.

Gap 1 — MCP coding_loop_start

Took your primary fix, not the interim: the tool now starts the durable run through POST /v1/instances/:id/loop and returns a run id. It inherits the pool, the single-driver claim (#208), merge authority (#314), a run row requestCancel can act on, and dispatch by the declared driver (#210) — so on a Coder it now drives the Engine instead of talking to the chat about the repo.

coding_loop_status reads GET /loop/:runId (or lists runs) instead of the Map in the MCP DO, and coding_loop_stop cancels the real run — with run_id optional, defaulting to the newest running one, so the old single-argument call shape still works and now names what it stopped. Both were ungated ("none" in the contract table) while they read and mutated Worker memory; they are now read and write respectively.

Taken deliberately, as you flagged: the return shape is a breaking change (run id, not transcript). instance-tools.test.ts:734,811 were rewritten rather than kept.

One thing worth knowing that the ticket did not mention: start_instance_loop / check_instance_loop / stop_instance_loop in composition.ts were already exactly the thin wrapper Gap 1 asks for. So coding_loop_* was a second implementation of tools that already existed durably. I rewired rather than deleted — AC 2 asks for coding_loop_start to keep working — but the six tools now do the same thing under two names, and collapsing them is a separate call worth its own ticket rather than an unticketed deletion.

AC 4 — the invariant

workers/api/src/lib/autonomous-budget.test.ts. Every CODING_SESSION / AGENT_LOOP / JOB_APPLY .create( site is read out of the source and must pass a budgetId; exemptions are compared exactly, so removing one fails too. Non-vacuous: without the two fixes above, routes/coding.ts and routes/coding-brains.ts are the offender list. The MCP half asserts no MCP tool references /loop-decide — there is one loop driver and it is on the server.

The existing pipeline-budget.test.ts guard (which enumerates openBudget call sites from the other direction) was updated with the two new sites and why they open a pool.

Two exemptions, one of which is a real remaining gap

• mode: "watch" (coding-brains.ts:83, coding.ts:584) — not a loop. workflows/coding-watch.ts polls for the engine to finish and makes exactly one copilotSummary call. Pooling it would add a delegation_budgets row per instruction and buy nothing.
• JOB_APPLY (routes/instances-apply.ts:189) — a real gap, recorded rather than hidden. The apply loop drives BYOK Claude for as many steps as the ATS takes and reserve() has no caller anywhere inside it, so unlike the coding sites this is missing wiring, not a missing argument. That is a separable change and wants its own issue; I did not widen scope into it.

Your "adjacent, not a defect" note about pipeline-run-start.ts:83 is untouched and still needs an owner's confirmation.

Verified

• All three coding entry points pass a pool (source guard + coding.contract.test.ts drives /run end to end: the delegation_budgets INSERT is issued, bound to the caller and rooted at their instance, and the id that reaches CODING_SESSION.create is the same one, with depth: 0).
• A /run with no objective opens no pool and creates no workflow.
• coding_loop_start calls /loop and makes no /loop-decide or /chat call; a 409 from /loop is reported as an error, not as a started run, and writes no completed audit event.
• Live: both deploys green, /health OK on API and MCP (135 tools, unchanged), openapi.yaml serves the new /run spend note, both routes answer 401 unauthenticated.
• Not verified live end-to-end: starting a real Pilot run needs an owner session plus a machine on pags up; I have no PAGS credentials in this environment. The budget row creation is asserted by driving the handler, not by observing production D1.

Note on the framing

No limit was tightened. BUDGET_ENFORCE is still observe-only and the per-tree defaults are unchanged — this only makes the spend accounted for and cancellable, which was the point.