Skip to content

fix(cloud-agent-next): budget a preparation attempt for its own steps - #8819

Open
iscekic wants to merge 7 commits into
mainfrom
drain/preparation-attempt-budget
Open

iscekic wants to merge 7 commits into
mainfrom
drain/preparation-attempt-budget

Conversation

@iscekic

@iscekic iscekic commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Evidence

Evidence: partially demonstrated — the deadline behaviour is observed on the real control-plane Durable Object in the Workers runtime under Miniflare, with a scripted provider adapter standing in for the container provider; no production provider is contacted, so a real container create and snapshot save are not part of the observation.

  • src/shared/control-plane-timers.test.ts asserts that one preparation attempt's deadline is at least the sum of the step bounds the attempt runs (sandbox create at the repository startup bound, one retried create, wrapper first connect, wrapper hello, snapshot restore, clone fallback, workspace save, and both bounds of the retried Kilo runtime start and session import), and that the attempt budget stays real when the development divisor shrinks the step timers. 19 tests pass locally.
  • On this VM the real Durable Object armed an enrolled, repository-backed preparation route with a deadline equal to that sum, read from the route row it persists, and larger than the earlier derivation by exactly the workspace save plus one runtime and one session bound.
  • The session.prepare frame for that route carried capture, so the workspace save the budget now covers is part of this attempt's sequence.
  • A single preparation attempt that included a capture plus one failed Kilo runtime start and one failed session probe reached ready, showing those steps retry inside the attempt the budget wraps.
  • The failure branch still holds: with the attempt deadline moved into the past, the alarm failed the route with preparation_timeout.
  • test/integration/sandbox-control-v2-routes.test.ts and test/integration/control-plane-session-v2-transport.test.ts pass locally (90 tests), and the full workers integration suite passed in an earlier session (638 tests); the transport suite keeps asserting that the attempt budget is not scaled.
  • Re-run on this VM at the current head, on the real Durable Object in the Workers runtime under Miniflare with a scripted provider adapter: a repository-backed route (snapshot_key eligible) prepared under the default timers armed its attempt deadline 2 210 020 ms after the attempt started, which is the step sum plus 20 ms of preparation time, and 548 s longer than the earlier 27.7-minute derivation; no development divisor was set.
  • On the same head, with that route's attempt deadline moved into the past, the Durable Object alarm failed the route with preparation_timeout and the session received { state: 'failed', reason: 'preparation_timeout' }, so the failure branch still fires at the widened deadline.
  • Committed suites on this VM at the current head: control-plane timers unit (19 tests); test/integration/sandbox-control-v2-routes.test.ts and test/integration/control-plane-session-v2-transport.test.ts (90 tests).

Change

  • services/cloud-agent-next: one preparation attempt's deadline is derived from the bounds of every step that attempt runs in sequence, so the attempt can stay preparing until its last bounded step finishes, and a step that stays inside its own bound is not cut off by the attempt deadline.
  • The sum now also covers the workspace save that a repository-backed route performs inside preparation, and counts the retried create plus one retry each of the Kilo runtime start and the session import.
  • Setup commands are bounded per command by their own inactivity and hard timeouts; their count comes from route configuration, so the attempt deadline is their aggregate bound.
  • Slower failure paths are unchanged: a step that hangs fails on its own bound, a permanent create failure fails preparing routes at once, and a lost wrapper is caught by the heartbeat and reconnect window before the attempt deadline.
  • The deadline is built from named external-work constants, so the derivation stays correct when a step bound changes, and it stays free of worker-only imports because this module is bundled into the wrapper with only src/shared. docs/control-plane.md now describes every derived value in terms of its owner instead of restating minutes.

Production prerequisites

None.

Proxy answers

The owner's proxy gave these answers in the owner's place.

  1. Question: Gate defect: The harness public-text gate refuses every write in this session: DRAIN_MASK_EXPLOIT=1 is set, but this side-PR work item has an empty findings list, so the gate throws "The work item file has no findings" before judging anything. Two writes are still required to finish: adding the janitor:trust label and replacing the PR body (Evidence + Production prerequisites None + proxy answers). Nothing is wrong with the PR itself: CI is green on the head, kilobot reviewed it, zero unresolved threads. How should this be unblocked?
    Answer: Runner ignores the mask for side-PR items
    Confidence: 0.82 · Precedent: Never accept blocked as an end state; treat a blocked item as a workflow bug and fix it (session 55c9c80332a7: "blocked is not an acceptable end state"; session 2f71af3a787e: "a work item must converge"). Fix the class (session 00eb2a1d3e4b).
  2. Question: Body write: The janitor:trust label is now on the pull request. The remaining write, the pull request body, still fails: the gate rejects every judged write because the mask flag is on while this side-PR work item has an empty findings list, and the runner fix cannot be applied from this VM (the runner process owns that env and the ledger file, neither is reachable here). With the label done, only the body write is left. Which fallback do you want?
    Answer: Neither handoff nor leaving it. Treat this as a workflow bug. A side-PR work item with an empty findings list must not trip the mask gate. Fix that class of bug in the workflow repo by pushing directly to main, and let the fleet's auto-pull and restart pick it up. Then retry the body write in this session. Keep the body true to the diff: E2E evidence, Change, Production prerequisites: None, and the proxy answers. While you wait, finish the rest of the loop: kilobot threads, CI, labels. Do not end with the body incomplete.
    Confidence: 0.50 · Precedent: Never accept blocked as an end state (session 55c9c80332a7: "blocked is not an acceptable end state"; 2f71af3a787e: "a work item must converge"); assume the owner is away (dfa302467be0)
  3. Question: Setup budget: Kilobot's remaining WARNING: the attempt budget does not include setup commands, each bounded by an 8-minute hard timeout with up to 20 configured commands, so a route whose setup commands together take longer than the attempt deadline still fails as preparation_timeout while every command stayed inside its own bound. Which policy should this PR encode?
    Answer: Keep the deadline as setup's aggregate stop (Recommended)
    Confidence: 0.65 · Precedent: Speed and least work: choose the simplest design that reaches the goal (session a30dc8dc131a, "Simplest please")
  4. Question: Stuck review: the review of the documentation head never ran, because the review worker failed after preparing the session and its provider check stayed in progress; nothing on this VM could restart it without a new commit. Everything else on the pull request was already complete: CI green, labels correct, body sections present, all review threads resolved. Which recovery do you want?
    Answer: Push an empty Conventional Commit (chore: re-trigger review) so the new head re-creates the review; keep the existing end-to-end proof because no code changed; address any new remark, then end handled. Record the stuck provider check as an internal finding for its own work item; do not fix it in this pull request.
    Confidence: 0.60 · Precedent: Speed and least work (session 7f3470e0ba6f); blocked is not an acceptable end state (session 55c9c80332a7); assume the human is away (session dfa302467be0)

One preparation attempt runs the sandbox create, the wrapper's first connect
and hello, the snapshot restore with its clone fallback, the Kilo runtime start
and the session import in sequence. Each step keeps its own bound, so the
attempt deadline is derived from their sum instead of a fixed 12 minutes: a
shorter deadline fails a slow but valid preparation as preparation_timeout
while every step stayed inside its own budget. The step bounds, a permanent
create failure and the reconnect window still fail a stuck preparation earlier.
@iscekic iscekic added the drain Work item from the findings drain label Oct 23, 2026
Comment thread services/cloud-agent-next/src/shared/control-plane-timers.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change is limited to the setup-command rationale in control-plane-timers.ts and the matching preparation-deadline paragraph in docs/control-plane.md; both now state the deliberate design (per-command bounds, attempt deadline as the aggregate stop) and match the code, so the previously carried setup-command warning is resolved by documented intent.

Assumptions: [INFERENCE] Verified by code reading only: SETUP_COMMAND_HARD_TIMEOUT_MS = 8 min and the default runSetup wiring in wrapper/src/control-plane/prepare.ts, preparation_timeout failure paths in src/control-plane/sandbox/routes.ts, and Limits.MAX_SETUP_COMMANDS = 20. No suite was executed in this read-only session.

Files Reviewed (2 files)
  • services/cloud-agent-next/src/shared/control-plane-timers.ts - 0 issues
  • services/cloud-agent-next/docs/control-plane.md - 0 issues
Previous Review Summaries (6 snapshots, latest commit fa3b6d1)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit fa3b6d1)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The workspace save (captureMs) and the doubled kilo_runtime/kilo_session bounds are now in the attempt budget and the stale minute values are gone from the docs, but the budget still cannot outlast the setup commands whose per-command hard timeouts it does not count.

Assumptions: [INFERENCE] Setup commands can run on repository-backed container routes (the wrap phase does not filter by repoKey), and no caller extends the route's attempt deadline when setup commands are configured; both were verified by reading prepare.ts and routes.ts, not by a live slow-provider run. Timings are derived by code reading.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/src/shared/control-plane-timers.ts 111-113, 115-125 The new comment states setup commands need no term because "this deadline is their aggregate bound", but the aggregate of their own bounds is far larger than the sum: each command is bounded by SETUP_COMMAND_HARD_TIMEOUT_MS = 8 min (wrapper/src/control-plane/prepare.ts:78) and up to Limits.MAX_SETUP_COMMANDS = 20 commands are allowed (src/schema.ts:13), so up to 160 minutes of setup can each stay inside its own bound against the 36.8-minute attempt budget. A route with roughly four commands totaling 20 minutes of valid work pushes a bound-worst-case preparation past the deadline, so preparation_timeout still fails a preparation whose steps were all inside their bounds - the failure this PR removes. Summary-only because the active thread on line 115 already covers setup commands; docs/control-plane.md repeats the "outlasts its own steps" list without setup commands. Consider bounding setup as a single step in the sum (and asserting it in control-plane-timers.test.ts) or stating that setup is the one step the deadline may cut off.
Files Reviewed (3 files)
  • services/cloud-agent-next/src/shared/control-plane-timers.ts - 1 issue (carried forward from the line-115 thread; setup-command gap still present)
  • services/cloud-agent-next/src/shared/control-plane-timers.test.ts - 0 issues (invariant now covers capture and both retried steps; it still mirrors the implementation's step list, so it cannot catch omitted steps)
  • services/cloud-agent-next/docs/control-plane.md - 0 issues (stale 16.5/12-minute values removed; the Preparation paragraph inherits the setup omission above)

Fix these issues in Kilo Cloud

Previous review (commit eba464c)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The retried create is now counted, but ROUTE_PREPARATION_MS still omits the workspace snapshot capture (5 min 10 s) and the doubled runtime/session retry bounds that run inside the same repository preparation, so the preparation_timeout this PR targets remains reachable; the control-plane docs also keep the old 12-minute attempt and 16.5-minute backstop values.

Assumptions: [INFERENCE] The snapshot capture runs before session.ready for every repoKey route on cloudflare-containers (verified by reading prepare.ts, repository-launch.ts and the container provider, not by a live slow-provider run). Setup commands have no bound of their own, so the attempt deadline is their only stop. Timings are derived by code reading.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/src/shared/control-plane-timers.ts 106 The budget sums create, connect, hello, restore, clone, runtime start and session import, but the same attempt also runs the workspace snapshot capture (timers.captureMs, 5 min 10 s) for exactly the repository-backed container routes this budget covers, runs kilo_runtime/kilo_session up to twice under STEP_RETRY_ATTEMPTS = 2, and runs unbounded setup commands. Capture alone puts a valid slow preparation at about 32.8 min against the 27.7-minute budget, so the failure this PR targets is still reachable with every step inside its own bound.

SUGGESTION

File Line Issue
services/cloud-agent-next/docs/control-plane.md 117, 126, 858, 868, 918 The docs still state a 16.5-minute queued backstop, a 12-minute preparation attempt and a 12-minute route-preparation timer. With routePreparationMs now 27.7 minutes the queued backstop derives to 32.2 minutes, so these tables contradict the updated route-state section. Summary-only because those lines are outside the diff.
Files Reviewed (6 files)
  • services/cloud-agent-next/src/shared/control-plane-timers.ts - 1 issue
  • services/cloud-agent-next/src/shared/control-plane-timers.test.ts - 0 issues (invariant mirrors the implementation's step list, so it cannot catch the omitted steps)
  • services/cloud-agent-next/docs/control-plane.md - 1 issue (summary-only, stale numeric values)
  • services/cloud-agent-next/src/control-plane/sandbox/routes.ts - 0 issues (comment only)
  • services/cloud-agent-next/src/control-plane/sandbox/sandbox-do.ts - 0 issues (comment only; repository create widening verified)
  • services/cloud-agent-next/test/integration/control-plane-session-v2-transport.test.ts - 0 issues

Fix these issues in Kilo Cloud

Previous review

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The incremental commit correctly counts one retried create plus its pause in the preparation budget, matching the retry path in reduceAllocation; the remaining issue is stale numeric timer values in the control-plane docs.

Assumptions: [INFERENCE] The attempt deadline is set after startAttempt's grant/policy work, so that work does not consume attempt time. A retried create is bounded by the same providerCreateMs (5 minutes for repository-backed cloudflare-containers), and the budget covers exactly one retry; further retries intentionally consume the remaining attempt time and are capped by the route deadline. Verified by code reading, not a live slow-provider run.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
services/cloud-agent-next/docs/control-plane.md 117, 126, 858, 868, 918, 1365 The docs still state a 12-minute preparation attempt and a 16.5-minute queued backstop, and the timer table still lists a 2-minute create. With routePreparationMs now 27.7 minutes (two creates plus the 10-second retry pause and the remaining steps) and the queued backstop 32.2 minutes, these values contradict the updated route-state section. Summary-only because those lines are outside the diff.
Files Reviewed (2 files)
  • services/cloud-agent-next/src/shared/control-plane-timers.ts - retry now counted; no new issues
  • services/cloud-agent-next/src/shared/control-plane-timers.test.ts - invariant updated to match; no issues

Fix these issues in Kilo Cloud

Previous review (commit 922606d)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The new commit correctly removes the worker-only deadlines.ts import by restating the 5-minute repository create bound, but ROUTE_PREPARATION_MS still does not cover a retried create, and the doc timer tables keep the old 12/16.5-minute values.

Assumptions: [INFERENCE] A create that reaches createDeadlineAt while the route deadline remains is retried by reduceTick with a new full providerCreateMs (control-plane-timers.ts:85 restates DEADLINE_MS.repositoryStartup, verified to still be 5 minutes in src/sandbox-control/deadlines.ts:3); verified by code reading, not a live slow-provider run. The wrapper build context (Dockerfile*:COPY src/shared) confirms the import removal was necessary for the wrapper bundle.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/src/shared/control-plane-timers.ts 85 The budget counts one create. reduceTick retries a timed-out create with a fresh full create bound while routeRetryAllowed still sees route time left, and the route deadline is not extended, so one attempt can need up to 27.5 min (two creates plus the other steps) against the 22.5-minute budget and still fail as preparation_timeout.

SUGGESTION

File Line Issue
services/cloud-agent-next/docs/control-plane.md 117, 126, 858, 868, 918, 1365 The docs still state a 12-minute preparation attempt and a 16.5-minute queued backstop, and the timer table still lists a 2-minute create call. With routePreparationMs now 22.5 min and the queued backstop 27 min, these values contradict the updated route-state section. Summary-only because those lines are outside the diff.
Files Reviewed (1 file)
  • services/cloud-agent-next/src/shared/control-plane-timers.ts - 1 issue (carried forward; import removal and restated 5-minute bound verified correct)

Fix these issues in Kilo Cloud

Previous review (commit 9c9404c)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The repository-create bound is now widened correctly, but ROUTE_PREPARATION_MS still counts only one create while a timed-out create is retried with a full bound inside the same unchanged route deadline, and the doc timer tables keep the old 12/16.5-minute values.

Assumptions: [INFERENCE] A create that reaches createDeadlineAt while the route deadline remains is retried by reduceTick with a new full providerCreateMs; the retry path was verified by code reading, not by a live slow-provider run.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/src/shared/control-plane-timers.ts 84 The budget counts one create. reduceTick retries a timed-out create with a fresh full create bound while routeRetryAllowed still sees route time left, and the route deadline is not extended, so one attempt can need up to 27.5 min (two creates plus the other steps) against the 22.5-minute budget and still fail as preparation_timeout.

SUGGESTION

File Line Issue
services/cloud-agent-next/docs/control-plane.md 117, 126, 858, 868, 918, 1365 The docs still state a 12-minute preparation attempt and a 16.5-minute queued backstop, and the timer table still lists a 2-minute create call. With routePreparationMs now 22.5 min and the queued backstop 27 min, these values contradict the updated route-state section. Summary-only because those lines are outside the diff.
Files Reviewed (3 files)
  • services/cloud-agent-next/src/shared/control-plane-timers.ts - 1 issue
  • services/cloud-agent-next/src/shared/control-plane-timers.test.ts - 0 issues (now asserts the widened repository create bound)
  • services/cloud-agent-next/docs/control-plane.md - 1 issue (targeted check, summary-only)

Fix these issues in Kilo Cloud

Previous review (commit 68b9409)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The derived preparation budget still under-counts the sandbox-create step on the repository-startup path (5 min, not 2 min), so the preparation_timeout failure this PR targets can still occur, and docs/control-plane.md keeps three stale copies of the old 12-minute / 16.5-minute values.

Assumptions: [INFERENCE] The repository-startup override (DEADLINE_MS.repositoryStartup) applies to every production repository route on cloudflare-containers, as inferred from sandbox-do.ts:2128-2138; the workflow was not exercised in a live environment during this review.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/src/shared/control-plane-timers.ts 95 ROUTE_PREPARATION_MS uses the 2-minute PROVIDER_CREATE_MS, but applyEvent raises the create bound to DEADLINE_MS.repositoryStartup (5 min) for repository routes on cloudflare-containers. Real worst case is 22.5 min (5 create + 5 first connect + 0.5 hello + 2 restore + 6 clone + 2 runtime + 2 session) against the 19.5-minute attempt, so a valid slow preparation can still fail as preparation_timeout. The new invariant test reads the same 2-minute value, so it cannot detect this.

SUGGESTION

File Line Issue
services/cloud-agent-next/docs/control-plane.md 117, 126, 858, 868, 918, 1365 The route-state paragraph was updated, but the doc still states a 12-minute preparation attempt and a 16.5-minute queued backstop. With routePreparationMs now 19.5 min and queuedBackstopMs derived to 24 min (90 s + attempt + 3 min), these tables contradict the changed section. Summary-only because those lines are outside the diff.
Files Reviewed (6 files)
  • services/cloud-agent-next/src/shared/control-plane-timers.ts - 1 issue
  • services/cloud-agent-next/src/shared/control-plane-timers.test.ts - 0 issues (encodes the same 2-minute create bound as the implementation)
  • services/cloud-agent-next/docs/control-plane.md - 1 issue (summary-only)
  • services/cloud-agent-next/src/control-plane/sandbox/routes.ts - 0 issues
  • services/cloud-agent-next/src/control-plane/sandbox/sandbox-do.ts - 0 issues
  • services/cloud-agent-next/test/integration/control-plane-session-v2-transport.test.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 32.3K · Output: 4.1K · Cached: 305.3K

Review guidance: REVIEW.md from base branch main

…tion attempt

A repository-backed container allocation creates its sandbox under the wider
repository startup bound, which is the create path every preparation attempt
with a repository takes. Use that bound in the attempt budget and assert it in
the invariant test, so a valid slow repository preparation cannot outrun the
attempt deadline while each step stays inside its own bound.
Comment thread services/cloud-agent-next/src/shared/control-plane-timers.ts Outdated
…y imports

This module is bundled into the wrapper with only src/shared, so it cannot
import the worker's other sources. State the repository create bound locally
with the source it mirrors named, and let the invariant test compare it with
that source instead.
…dget

A transient create failure pauses and creates again inside the same
preparation attempt, so the attempt budget carries one retried create as well.
Further retries stay bounded by the attempt deadline, which remains what stops
a create that keeps failing.
@iscekic iscekic self-assigned this Oct 23, 2026
@iscekic iscekic added janitor:trust Janitor finding: trust boundary hardening merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. labels Oct 24, 2026
Comment thread services/cloud-agent-next/src/shared/control-plane-timers.ts
@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. and removed merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Oct 24, 2026
@iscekic iscekic removed merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Oct 24, 2026
@iscekic

iscekic commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

bot: On the setup-command warning in the summary for d608195: rejected as a defect, and the reasoning now lives in the code and the docs.

  • Each setup command keeps its own inactivity and hard timeout, so a single stuck command fails with its own reason instead of silently consuming the attempt.
  • How many commands run, and how long each valid one takes, is route configuration. Counting the maximum configuration in the sum would put the attempt deadline and the queued backstop in the hours, and a stuck preparation would hold a session for that long before anything surfaces; a bounded stop is the better failure here.
  • The invariant this change establishes therefore covers every step that runs a fixed number of times inside one attempt. Setup is bounded where it belongs: the attempt deadline is its aggregate stop, explicitly stated in the module comment and in the route-state section of docs/control-plane.md.

No timer value changed for this point; the commit adds the policy text only.

@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. labels Oct 24, 2026
@iscekic iscekic added merge:blocked:insufficient-e2e Verification does not cover the change merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head and removed merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Oct 24, 2026
@iscekic iscekic added merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:insufficient-e2e Verification does not cover the change and removed merge:blocked:insufficient-e2e Verification does not cover the change merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Oct 24, 2026

This branch has not been deployed

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

Labels

drain Work item from the findings drain janitor:trust Janitor finding: trust boundary hardening merge:bot Bot-owned review; merge when ready once readiness passes and a review approves.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant