Repository navigation
fix(cloud-agent-next): defer preparation timeout while a runtime replacement is in flight - #6586
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental head Files Reviewed (7 files)
Previous Review Summaries (5 snapshots, latest commit 5667954)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 5667954)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (7 files)
Reviewed the full current PR (history was rewritten, so the incremental diff against the previous SHA was not usable). The previously reported deferral re-validation, deferral-budget cap, budget reset, and retired-attach test coverage are all present and correct in this head; the new finding is in the deferral's Fix these issues in Kilo Cloud Previous review (commit 22dbe6e)Status: No Issues Found | Recommendation: Merge Executive SummaryIncremental review of the code changed since the previous review: the new native-runtime-keyed deferral-budget reset ( Files Reviewed (3 files)
Previous review (commit 6661da5)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe new deferral-budget reset keys on the wrapper incarnation, which a directory-native retirement does not change, so a queued head can still terminalize with Overview
Issue Details (click to expand)WARNING
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous review (commit fdd2b5b)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Incremental review of 190 changed lines across 3 files since the previous review; all three previously reported findings are resolved in the current head and no new issues were introduced. Previous review (commit 890963e)Status: 3 Issues Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (7 files)
Verified: Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
05f36c1 to
fdd2b5b
Compare
|
kilo-review — independent audit of the published diff. Status: 1 Issues
|
|
bot: Accepted. Pushed 5b0fa6b (kwf What changed:
|
5b0fa6b to
6661da5
Compare
b0b6adc to
22dbe6e
Compare
3b2e04c to
5667954
Compare
…acement is in flight #6586
eb8d81d to
438544c
Compare
Changelog for users
Changelog for maintainers
services/cloud-agent-next/src/sandbox-session/SandboxSession.ts:3519— accepted: the deferral-budget reset there keys on the wrapper incarnation, which a directory-native retirement leaves unchanged.recordNativeRuntimenow clearsreplacementWaitsthrough the newclearRuntimeReplacementWaits(epoch)when it binds the native runtime, so an in-place rebind resets the budget; the wrapper-identity reset inrecordRuntimestill covers wrapper replacements.services/cloud-agent-next/src/sandbox-session/SandboxSession.ts:4598— accepted: the deferral retired the live attach proof intoretiredAttachby overwrite, and a dispatched attach carries noattachmentEpochuntil its result arrives, so the overwrite could drop the epoch pool below the native-runtime fence's epoch and the replacement's in-place rebind was refused as a stale result. The retire now goes through the newretireAttachProof(attach, previous), which carries the higher of the two epochs into the single slot; the other attach-retirement sites use the same helper.preparation_timeouteven while a control-plane runtime replacement was in flight. After:awaitRuntimeReplacementOrFailasks the control plane forruntimeReplacementInFlight; while true the head stays queued, takes a newpreparationAttemptId, retires any bound attach intoretiredAttach, extends its delivery deadline, spends one unit of a boundedreplacementWaitsbudget, and re-arms the existing 5 s queue retry. When false, or once the budget is spent, the existing terminal path runs unchanged.SandboxControl.getStatusandstatusForPhysicalaccept an optionalsessionIdand reportruntimeReplacementInFlightwhile the session still holds a native retirement fence; the RPC forwards the session id. An absent session id or a transport failure reports no replacement, so the probe adds no failure mode.holdsNativeRetirementFenceForSessionextracts the fence check: a completed-and-delivered receipt releases the fence, and a mismatched wrapper lifetime does not hold it.nextAttachmentEpochcountsretiredAttachepochs when it mints the next attach epoch, so an attach minted after a deferral advances past the epoch the native-runtime fence holds instead of re-minting it and looking like a stale result. A fresh preparation attempt identity is required because the control plane binds an acquisition id to its original deadline and rejects a changed deadline for the same id.SandboxSessionand its delivery-deadline call sites. The risk is a head that never terminalizes if the fence never clears; the bound is the control-plane retirement lifecycle, which prunes receipts once the allocation stops, plus thereplacementWaitsbudget.awaiting-runtime-replacement-delivers-queuesuite covers defer-then-deliver, the re-arm as a new acquisition, attach retirement intoretiredAttach, budget exhaustion, the in-place-rebind budget reset, and the epoch pool surviving a second deferral; the fence helper and thestartup-timeout-fails-waiting-queueterminal path are also covered.E2E proof
Evidence collected 2026-09-22. The appended log excerpts are the run's evidence for both scenarios and replace the truncated excerpt cited earlier. Limitation: the live terminal-path escalation did not complete during verification, so that path stays proved by the deterministic
startup-timeout-fails-waiting-queuetest.Owner request
E2E proof — log excerpts