Repository navigation
feat(cloud-agent-next): hold Kilo under memory pressure and explain restarts - #7290
Conversation
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental commit correctly moves Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (11 files)
Fix these issues in Kilo Cloud Previous Review Summaries (3 snapshots, latest commit 1398bb6)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 1398bb6)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental documentation changes correctly scope Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (37 files)
Fix these issues in Kilo Cloud Previous review (commit 58fef8b)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental documentation changes correctly scope Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 5104e36)Status: 4 Issues Found | Recommendation: Address before merge Executive SummaryNo critical or blocking defects: the memory-hold state machine, execution-clock union accounting, workload-cgroup memory.min changes, and the mobile safe-copy move verified clean. The findings are documentation/spec inconsistencies with the same PR's Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (37 files)
Reviewed by deepseek-v4.1-flash · Input: 81.4K · Output: 20.7K · Cached: 1.2M Review guidance: REVIEW.md from base branch |
…der pressure - Report per-child memory stats (tools vs. Kilo server: current, peak, anon, file, shmem) in control.workload records. - Set memory.min = 1 GiB on each Kilo scope group and server cgroup so reclaim at the shared workload cap takes tools and page cache first. It is optional: a refused write keeps the hard caps and emits a failed control.workload "protection" record, which reaches the native logs. - Sample the shared workload cgroup at every watchdog check. While it reclaims at its cap (usage within 256 MiB of memory.max and the memory.events max counter rising), a hang restart (no health answer, reconnect budget, or failed activity observation) is held: Kilo keeps running and the decision is made again at each check, for at most 10 minutes per episode. The hold ends as recovered once Kilo delivers events and its activity is observable, or with the restart after two quiet samples or at expiry. Hold start/end are logged. - While held, the runtime's native executions pause their no-progress clocks (overlap with a user wait counts once), the 120 s prompt delivery deadline does not run, and each turn on the runtime gets a non-fatal notice.
…e busy on reconnect - Add agent_unresponsive for a turn, or interrupted native work, failed by a Kilo hang restart (including one at the end of a memory hold); other Kilo restarts stay agent_restarted. Telemetry classifies it like agent_restarted. - Send readable text in cloud.message.failed `error` for control-plane reasons (live and on reconnect); `reason` keeps the raw code. The reason-to-text table moves out of callbacks into failure-messages.ts, and Kilo-written reasons pass through unchanged. - When Kilo's last root status is busy/retry, an accepted message has settled since, and none is accepted now, the connected event carries sessionStatus idle, so a reload after a hang or kill no longer applies the stale busy. Native work without a Cloud message keeps Kilo's replayed status. - Mobile shows the new texts as safe copy instead of the generic failure line; the safe list moves to its own module (max-lines).
…tarts A hang restart for exhausted activity tracking capacity is not Kilo failing to answer, so it must not say Kilo was not responding.
The activity-capacity carve-out and the dead-event-stream path were described with the wrong reason in the spec and the control-plane doc.
58fef8b to
1398bb6
Compare
The restart attribution and the memory hold each decided whether the sandbox was at its memory cap, with different rules. The attribution compared one runtime's scope usage with the shared cap at 95%, which misses a cap filled by sibling scopes and fires on reclaimable page cache after a large build. The hold is now the only owner: a hang restart that ends an expired hold (the shared workload kept reclaiming at its cap for the whole window) fails the turn as sandbox_out_of_memory. Other hang restarts stay agent_unresponsive or agent_restarted. isWorkloadAtCap and its 95% fraction are removed; the per-scope snapshot stays on the restart log line as diagnostic data only.
Summary
Keeps Kilo alive under memory pressure in
workspace_*sessions, and tells the user what happened when a control-plane failure ends a turn.Memory protection and hold (wrapper)
control.workloadstats now split memory between tools and the Kilo server (current, peak, anon, file, shmem).memory.min= 1 GiB, so reclaim at the shared workload cap takes tools and page cache first. It is optional: if the kernel refuses the write, the hard caps stay and a failedcontrol.workloadprotectionrecord goes to the native logs.memory.maxand thememory.eventsmaxcounter rose since the previous sample.errornotice. Nothing acts on it.kilo_memory_hold_started/kilo_memory_hold_ended.Failure text and stale busy (Worker, wrapper, mobile)
New reason
agent_unresponsivefor a turn, or interrupted native work, failed by a Kilo hang restart. Other restarts stayagent_restarted, including activity-capacity ones. Telemetry classifies it likeagent_restarted.cloud.message.failederrornow carries readable text for control-plane reasons, live and on reconnect, for example "Kilo was not responding and was restarted".reasonkeeps the raw code. The text table moved from callbacks tofailure-messages.ts, and Kilo-written reasons pass through unchanged.On reconnect,
connectednow carriessessionStatus: idlewhen all three hold:busy/retry;Previously, a reload after a hang or kill re-applied Kilo's stale
busy. Native work without a Cloud message keeps Kilo's replayed status.Mobile adds the new texts to its safe copy, so it no longer shows the generic "The response failed." The safe list moved to its own module because of
max-lines..specs/cloud-agent-session.mdgains the hold and failure-text rules.docs/control-plane.mdandDEBUG.mdare updated.Failure UX, both effects
Deploy order: deploy the Worker before the wrapper image. An old Worker accepts
agent_unresponsive, because the outcome reason is free text. Until it is updated, that reason gets anunknown/unclassifiedreport and the raw code as callbackerrorMessage.Verification
No manual test was run. A real check needs a sandbox driven into reclaim at its memory cap. That would show whether
memory.mintakes effect on the container kernel and whethermemory.eventsmaxbehaves as expected there. Neither has been verified on a real sandbox.Visual Changes
agent_restarted,no_progress, …) above the composer and in the bubble tooltipThe hold notice is posted, but the SDK currently turns non-fatal
errorevents into a no-opwarning, so web and mobile do not show it yet. The web delivery badge still says "Failed to deliver:" for an accepted message; that is unchanged.Reviewer Notes
Execution, in the newexecution-clock.ts. An execution that opens during a hold starts paused.activity_observationfault does not latch (onFaultreturns false), and the activity keeps reading snapshots, so a recovered Kilo clearsunhealthySince.abort_unconfirmedandactivity_capacitystill latch and never hold.settledflag on the root status, not timestamps.memory.highon tools: without swap it throttles instead of a clean OOM.memory.minalready makes the kernel pick a tool.cli_sessions_v2.status, which Kilo writes through session ingest, can still show busy after a hang. Left as a follow-up.memory.minalso applies to the legacy plane, which shares the cgroup code. It does not protect against reclaim triggered above the shared workload cgroup.services/cloud-agent-next: typecheck, lint, duplication ratchet, Worker unit (5982), integration (652), wrapper (1003).apps/mobile: typecheck, lint,src/components/agents(4933).format:check,lintandtypecheck --changes-only.sandbox-control-v2-*files I did not touch; they passed alone and in the final full run.