fix(agents): apply CPU cgroup limit on Linux (NanoCpus) + fix resource config UI (#1126) - #1128
Conversation
…#1126) Backend (the confirmed root cause): every container-creation site passed `cpu_count` to docker-py, which maps to the Windows-only HostConfig.CpuCount and leaves NanoCpus=0 on Linux — so NO agent ever got a CPU cgroup limit (created, recreated, or system). Memory was unaffected (`mem_limit` is the correct Linux param). Replaced with `nano_cpus = cores * 1e9` (Linux CFS quota) at all three sites: crud.py (create), lifecycle.py (recreate), system_agent_service.py (system agent). Frontend: - ResourceModal: pre-select the EFFECTIVE value (`cpu ?? current_cpu`, `memory ?? current_memory`) instead of the DB override (null when unset), so the dialog reflects what the agent actually runs with; the null option becomes an explicit "Inherit default". Adds a load-error banner. - useAgentSettings: `loadResourceLimits` records `error` (surfaced in the modal) instead of silently swallowing; `updateResourceLimits` returns a success boolean. - AgentHeader: show the configured core count + max memory alongside the live CPU/MEM gauges while running (was only shown when stopped). - AgentDetail.saveResourceLimits: gate the restart on the container actually reaching a stopped state (poll via fetchAgent, 30s cap) instead of a fixed 1s sleep, skip restart if the save failed, and report restart failures to the user instead of leaving the agent stopped silently. Verified live: PUT cpu + restart → HostConfig.NanoCpus = cores*1e9 (0 → 1e9 for 1 core, → 2e9 for 2 cores), label trinity.cpu and the cgroup agree, memory unchanged. UI files compile (vue/compiler-sfc). Optional capacity-pressure hint: deferred (issue marks it drop-if-not-easy). Related to #1126 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| class="w-full px-3 py-2 border border-gray-300 dark:border-gray-600 rounded-lg shadow-sm focus:ring-action-primary-500 focus:border-action-primary-500 bg-white dark:bg-gray-700 text-gray-900 dark:text-white" | ||
| > | ||
| <option :value="null">Default ({{ resourceLimits.current_memory || '4g' }})</option> | ||
| <option value="">Inherit default ({{ resourceLimits.current_memory || '4g' }})</option> |
There was a problem hiding this comment.
Minor UX: with :value="memory ?? current_memory", the "Inherit default" option can never appear selected. Picking it emits null ("" || null), the parent sets memory=null, and the binding re-resolves to null ?? current_memory → the dropdown snaps to the concrete effective option (e.g. "4 GB"). The inherit intent is still stored correctly (save sends null → clears the override), so this is cosmetic — just note the option visually "bounces" rather than sticking. Fine to leave; flagging for awareness.
🔎 Code review (high-effort, recall-biased) — no blocking bugsThe headline backend fix is correct and live-verified (NanoCpus 0→1e9→2e9, labels+cgroup agree). Frontend is sound. No correctness bugs survived verification. Verified
Non-blocking note
✅ Ready to merge. Reviewed with the |
|
Resolve by running |
✅ Live-tested against a real instance (browser UI, agent
|
| AC | Result |
|---|---|
| Header shows configured cores + max memory while running | ✅ 0.2% / 2 cores and 48 MB / 4G rendered next to the live gauges |
| Dialog pre-selects effective CPU/memory | ✅ CPU→"2 Cores", Mem→"4 GB"; tracked dynamically (showed "1" after a change) — no blank/Default-only state |
| Reliable apply/restart | ✅ Changed CPU via dialog → Save → full stop→poll-until-stopped→start flow ran → terminal toast "Agent restarted with new resource limits." |
CPU cgroup applied (NanoCpus non-zero) |
✅ 0 → 1e9 → 2e9, matches configured cores via docker inspect |
| Labels + DB agree after apply | ✅ label.cpu tracked 2→1→2 in lockstep with the cgroup |
| No JS errors | ✅ zero page errors throughout |
Agent restored to its original 2 CPU / 4g afterward.
⚠️ One behavioral caveat for the reviewer
The restart logic (waitForAgentStatus poll + startAgent) runs client-side in saveResourceLimits. If the user closes the tab / navigates away in the ~seconds between Save and the restart completing, the agent is left stopped (the stop already fired server-side, but the follow-up start never runs). I hit this first as a test artifact (closed the browser 3s after Save) before re-running with the page kept open.
This isn't a regression — the prior code was equally browser-driven (stop → 1s sleep → start), so the same window existed and was actually wider before. But now that the flow is explicit, it's worth a conscious decision: acceptable as-is, or should the stop+start be moved server-side (e.g. a single backend "apply resources & restart" endpoint) so it survives the tab closing? Out of scope for this fix; flagging for the call.
Not live-triggerable (code-verified only)
- Load-error banner (
resourceLimits.error) — needs a forced GET failure. - "Agent did not stop within 30s" timeout branch.
Both are straightforward (v-if banner; bounded poll returning false), but couldn't be exercised without fault injection.
vybe
left a comment
There was a problem hiding this comment.
Validated via /validate-pr: nano_cpus fix verified at all three container-creation sites (matches the canonical creation points in architecture.md); AgentHeader resourceLimits prop pre-exists so the new running-state display is safe; restart flow now status-gated instead of sleep-gated. CI green. Approving.
) (#1227) A GitHub source repo whose template.yaml declares a fractional/Kubernetes-style resources block (cpu: "0.5", memory: "512Mi") aborted agent creation deep in container-create with an opaque `ValueError: invalid literal for int() with base 10: '0.5'`, after the MCP key was already minted — leaving an orphaned mcp_api_keys row per attempt (#1126/#1128 added the unguarded int(cpu) at three sites). - Add normalize_cpu/normalize_memory + canonical VALID_CPU/VALID_MEMORY in services/agent_service/capabilities.py (stdlib-only, the anti-drift home for container spec). routers/settings.py now imports these instead of duplicating the lists, so the API and the create paths can't drift. - crud.py create: normalize/validate config.resources BEFORE any side effect and raise a clear HTTP 400 on invalid input; write canonical values back so labels + limits use them. Roll back the agent-scoped MCP key in the failure path so a failed create leaves no orphan row. - lifecycle.py recreate + system_agent_service.py: same guard at their nano_cpus/mem_limit sites. - tests/unit/test_resource_normalization.py: pins the helpers (valid set, fractional/k8s rejection with actionable message, case-folding, default fallback, int()-castability, drift guard). Related to #1197 Co-authored-by: Eugene Vyborov <1073874+vybe@users.noreply.github.com>
Summary
Fixes the per-agent CPU/memory resource configuration end-to-end. The headline is the confirmed backend bug (DevOps inspection in the issue): CPU limits were never enforced on Linux for any agent.
Backend — CPU limit never applied (root cause)
Every container-creation site passed
cpu_countto docker-py, which maps to the Windows-onlyHostConfig.CpuCountand leavesNanoCpus=0on Linux — so the cgroup was unconstrained for every agent (created, recreated, and system). Memory was fine becausemem_limitis the correct Linux parameter (consistent with the bug being CPU-only).Replaced
cpu_count=int(cores)→nano_cpus=int(cores) * 1_000_000_000(Linux CFS quota →HostConfig.NanoCpus) at all three sites:services/agent_service/crud.py(agent creation)services/agent_service/lifecycle.py(restart/recreate)services/system_agent_service.py(system agent)Frontend
ResourceModal.vue— selects now pre-select the effective value (cpu ?? current_cpu,memory ?? current_memory) instead of the DB override (which isnulluntil explicitly set, hence the blank/Default-only dialog). The null option becomes an explicit "Inherit default (Xg)". Adds a load-error banner.useAgentSettings.js—loadResourceLimitsrecordserror(rendered in the modal) instead of swallowing toconsole.error;updateResourceLimitsreturns a success boolean.AgentHeader.vue— shows the configured core count + max memory next to the live CPU/MEM gauges while running (previously only when stopped), so live usage reads against the ceiling.AgentDetail.saveResourceLimits— gates the restart on the container actually reaching a stopped state (poll viafetchAgent, 30s cap) instead of a fixed 1s sleep; skips the restart if the save didn't persist; reports restart failures to the user instead of silently leaving the agent stopped.Acceptance criteria
HostConfig.NanoCpusmatching the configured count (verifieddocker inspect).Default-only).Verification (live)
NanoCpus=0,CpuCount=2(ineffective)NanoCpus=1000000000,label.cpu=1, Mem unchangedNanoCpus=2000000000,label.cpu=2Memory limit (
HostConfig.Memory) correct throughout. UI files compile viavue/compiler-sfc. Test agent restored to its original 2 CPU / 4g.Related to #1126
🤖 Generated with Claude Code