Repository navigation
diag(cloud-agent-next): quiet control workload stats and attribute memory on restart - #7249
Merged
Merged
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up commit resolves every previously reported workload-diagnostics finding, and the new cooldown/hysteresis, per-scope snapshot attribution, and closed Files Reviewed (13 files)
Previous Review Summaries (3 snapshots, latest commit b600ea7)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit b600ea7)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit b352f96)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit c573dff)Status: 1 Issues Found | Recommendation: Address before merge Executive SummaryThe workload-stats emission state machine is correct and unit-tested, but its immediate-emission triggers have no cooldown, so a scope under sustained CPU/memory pressure can still emit a stats record once per 1s sweep. Overview
Issue Details (click to expand)WARNING
Files Reviewed (4 files)
Reviewed by deepseek-v4.1-flash · Input: 123.5K · Output: 27.7K · Cached: 2.5M Review guidance: REVIEW.md from base branch |
eshurakov
changed the title
…ment sweep The one-second cgroup sweep emitted a `control.workload` stats record on almost every tick because its values include monotonic cpu/io counters and fluctuating `memory.current`. Under any real load that is ~1 record per second per scope, flooding the diagnostic upload buffer and Axiom. Split the two jobs the sweep was doing. The sweep still runs every second to migrate new server/tool processes into their cgroups, where the short cadence is enforcement. Stats now sample every `WORKLOAD_STATS_INTERVAL_MS` (30s) and emit immediately when an actionable counter changes: a new cpu throttle, a new `memory.events` max/oom count, or the first crossing of 90% of the group memory cap. OOM and migration events keep their existing immediate emissions. Verification: wrapper typecheck and oxlint clean; `bun test src/control` passes. Added `decideWorkloadStatsEmission` unit coverage for the cadence gate and each immediate trigger.
A health-probe restart only said `agent_restarted`, and the workload stats recorded the parent scope alone, so triage could not tell whether Kilo grew or tools plus page cache filled the shared headroom. Stats and snapshot: - `readWorkloadStats` now reads the `memory.stat` anon/file/shmem split and reports `pressureAvailable`, confirming whether the kernel exposes `memory.pressure` (PSI). Without PSI, triage falls back to `memory.events` and `memory.stat`. - The `control.workload` stats record adds `anonBytes`/`fileBytes`/`shmemBytes` and `toolCurrentBytes`/`toolPeakBytes` and `serverCurrentBytes`/`serverPeakBytes`, so a full headroom is traceable to the tools subgroup or the Kilo server and to page cache versus process memory. - `createOwnedProcessScope` publishes a `WorkloadSnapshot` on every sweep, not only on the 30s stats cadence, so a restart decision sees the current state. Restart attribution: - A `health_probe_false` restart attaches the latest snapshot under the restarting log line's `workload` key. - When the snapshot is at the cap, the turn fails with `sandbox out of memory: <current>/<cap> GiB` instead of `agent_restarted`, so triage can classify it as the user's workload. Verification: wrapper and service typecheck clean; oxlint and oxfmt clean; `bun test src/control src/control-plane` green apart from the pre-existing sandbox cert-refusal process test; added coverage for the memory.stat split, absent PSI, the cap reason, the restart diagnostic snapshot, and the turn outcome reason.
…art attribution Address review on the workload diagnostics. - Event-driven stats (throttle/memory-event increases, first pressure crossing) now share `WORKLOAD_STATS_EVENT_COOLDOWN_MS` (10s), and the pressure signal releases only below 80% (hysteresis). A scope parked at its limit can no longer re-emit on every one-second sweep, matching the PR's own goal. - The restart snapshot now comes from the runtime's own process scope rather than a wrapper-global sink, so a restart is not attributed to a sibling runtime's memory. The snapshot is still compared against the shared parent cap, which is the only aggregate limit available. - The sandbox-out-of-memory outcome is now the closed reason `sandbox_out_of_memory` instead of a free-form string, gated to health-probe restarts and mapped in the failure classifier and callback message table, so run-state classification stays `wrapper_disconnected` instead of regressing to `unclassified`. The native `session_outcome` diagnostic keeps it too. - DEBUG.md states the 95% at-cap fraction, the 10s event cooldown, and the pressure release fraction. Verification: wrapper and service typecheck clean; oxlint and oxfmt clean; `bun test src/control src/control-plane` green apart from the pre-existing sandbox cert-refusal process test; `vitest run src/telemetry src/shared src/control-plane/session` green. Updated the cadence, hysteresis, cap, health diagnostic, outcome, and classification tests.
eshurakov
force-pushed
the
kilo/young-willow-xg9
branch
from
October 8, 2026 08:29
b600ea7 to
5272bcc
Compare
iscekic
approved these changes
Oct 8, 2026
This was referenced Oct 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three related changes to control-workload diagnostics.
1. Sample stats off the containment sweep
WORKLOAD_SWEEP_INTERVAL_MS = 1000drivessweepManagedonce per second and every sweep emitted acontrol.workload/workloadPhase=statsrecord. The reporter dedups by value, but the record carries monotonic cumulative counters and a fluctuatingmemory.current, so under load it emitted ~1 record/sec/scope and drowned lifecycle, lease, and shutdown records.The 1s sweep still migrates new server/tool PIDs into their cgroups (enforcement). Stats now sample every
WORKLOAD_STATS_INTERVAL_MS(30s), and sooner on an actionable change (a new cpu throttle,memory.eventsmax/oom count, or the first crossing of 90% of the group cap) bounded byWORKLOAD_STATS_EVENT_COOLDOWN_MS(10s) so monotonic counters cannot re-emit on every sweep. The pressure signal latches at 90% and releases only below 80% (hysteresis). OOM and migration events keep their immediate emissions.2. Attribute memory and name OOM restarts
readWorkloadStatsreads thememory.statanon/file/shmem split and reportspressureAvailable, confirming whether the kernel exposesmemory.pressure(PSI). Without PSI, triage falls back tomemory.eventsandmemory.stat.control.workloadstats record addsanonBytes/fileBytes/shmemBytesandtoolCurrentBytes/toolPeakBytesandserverCurrentBytes/serverPeakBytes, so a full headroom is traceable to the tools subgroup vs the Kilo server and to page cache (file, e.g. Docker storage I/O) vs process memory (anon).createOwnedProcessScopepublishes a per-scopeWorkloadSnapshoton every sweep (not just the 30s cadence), and each runtime reads its own scope, so a restart is never attributed to a sibling runtime's memory.health_probe_falserestart attaches the latest snapshot under the restarting log line'sworkloadkey.sandbox_out_of_memoryinstead ofagent_restarted, so run-state classification stayswrapper_disconnectedrather than regressing tounclassified. The callback message is "The sandbox ran out of memory"; the numeric current/cap detail remains in the diagnostic snapshot and stats record.3. Docs
DEBUG.mdstates the 30s/10s cadences, the 90%/95%/80% fractions, the new memory fields, and thesandbox_out_of_memoryreason.Verification
pnpm -C wrapper run typecheckandpnpm run typecheck(service) — cleanoxlintandoxfmt— cleanbun test src/control src/control-plane— green apart from the pre-existingcontrol-plane/main-cert-refusal.test.tsprocess test that fails in this sandbox (spawned child never exits; untouched here)vitest run src/telemetry src/shared src/control-plane/session— green