From da3192cf4d5ac747792cb6f6492bfa425a313424 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Tue, 18 Aug 2026 00:07:15 +0000 Subject: [PATCH 1/3] feat: police helper-process drift instead of chasing OOM spikes The memory watchdog was built to prevent a cgroup OOM by shedding VS Code processes before the limit. That premise did not survive measurement: - every recorded kill is a 70-220 MB/s spike, idle to dead inside a minute, with an agent session or node named as the victim, never a VS Code process; - a poll loop cannot win that race - a biggest-RSS killer beats the kernel only at a 0.3s interval and is killed by oom.group when it loses; - the whole editor tree the ladder could shed is ~0.7 GiB, five seconds of that growth, and in a live spike the ladder climbed correctly and then logged no-candidates because the runaway was not in the tree it managed. So the graded L1-L4 ladder and the RLIMIT_DATA ceilings are removed, and the measurement they were built on is kept. What replaces them is the thing a poll loop is actually good at: bounding the standing population of restartable helpers, which the operator has been policing by hand for months (a python MCP server held 1.66 GB at one of the kills). The goal is runway, not rescue. What it now does: - per-role PSS budgets, each clamped between a share of the pod's memory.max and 1.5x the role's measured resting size, so a budget can never land below what a role demonstrably needs - the defect that reached enforce-readiness twice before; - a kill needs ten minutes of continuous over-budget dwell, so a language server that balloons while indexing and hands the memory back survives; - three kills of one role inside an hour disarm that role, with a loud log line. The kill loop, not the wrong kill, is what would make this harmful; - a second policed population: helpers an agent session spawned, chiefly MCP servers, which live nowhere near ~/.vscode-server. The walk stops at shells and at a change of session id - measured, because Claude Code detaches every Bash tool call into its own session, which keeps in-flight work out even when the tool call's shell has exec'd itself away; - a durable per-process sweep log with identity breadcrumbs and argv redaction. The previous version computed this table every cycle and threw it away, which is why every post-mortem in this investigation was unanswerable. Identity guards stay absolute; the two positional rules (ptyHost subtree, "not VS Code's own binary") now bound the editor selection only, so an MCP server is treated the same whether its session came from coder ssh or a VS Code terminal. memory_watchdog_mode gains a third value: observe / enforce (helpers, the new default) / enforce-all (adds the extension host and server, which restart visibly). Exercised on the test workspace as well as by fixtures: in enforce mode the watchdog killed a drifted 739 MB helper three times as its supervisor respawned it, disarmed the role on the third, and left the fourth incarnation, a detached 400 MB tool call and both session roots untouched. --- CLAUDE.md | 23 +- DESIGN.md | 16 +- TESTING.md | 9 + .../homelab-workspace/coder-agent.tf | 11 + templates/kubernetes/homelab-workspace/env.tf | 19 +- .../homelab-workspace/parameters.tf | 17 +- .../script-memory-watchdog-test.sh | 1366 +++++++-------- .../script-memory-watchdog.sh | 1491 +++++++++-------- .../kubernetes/homelab-workspace/scripts.tf | 6 +- 9 files changed, 1584 insertions(+), 1374 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c7541c97..6f5bb2c0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,8 +74,8 @@ Quick orientation map — for what each piece is *for* and the decisions behind | `scripts.tf` | `coder_script` resources — the memory watchdog daemon and the weekly `vscode-server` GC schedule | | `variables.tf` | `workspace_image`, `test_mode` — both supplied by the release workflow | | `script-agent-startup.sh` / `script-prepare-workspace.sh` | Scripts run on agent/workspace startup | -| `script-memory-watchdog.sh` | Userspace memory watchdog — see [DESIGN.md](DESIGN.md#design-tensions-and-decisions). **Defaults to observe-only mode**: it measures and logs, and sets no limits and sends no signals unless the `memory_watchdog_mode` parameter is switched to `enforce` | -| `script-memory-watchdog-test.sh` | Fixture tests for the watchdog's arithmetic and process selection. Run by hand (`./script-memory-watchdog-test.sh`) and by the `watchdog` job in `.github/workflows/test.yaml` | +| `script-memory-watchdog.sh` | Userspace memory watchdog — see [DESIGN.md](DESIGN.md#design-tensions-and-decisions). It bounds the **standing population of restartable helpers** (per-role PSS budgets, ten-minute dwell, per-role circuit breaker) and records every per-process sweep. It does **not** try to prevent an acute OOM. `memory_watchdog_mode` selects `observe` / `enforce` (helpers — the default) / `enforce-all` (helpers + editor) | +| `script-memory-watchdog-test.sh` | Fixture tests for the watchdog's arithmetic, process selection, budgets, dwell and circuit breaker. Run by hand (`./script-memory-watchdog-test.sh`) and by the `watchdog` job in `.github/workflows/test.yaml`. `kill` is shadowed by a function throughout — the fixture pids are real pids in whatever container runs the suite | **Image** (`images/homelab-workspace/Dockerfile`): three build stages — `base` (minimal bootstrap deps) → `system-base` (`unminimize` + full interactive toolset) → final stage (env vars into `/etc/environment`, fixed-UID/GID `coder` user, `USER coder`). All `apt`-touching `RUN` steps use BuildKit cache mounts — match that pattern when adding packages. @@ -87,13 +87,18 @@ Things that look arbitrary in the code but are load-bearing (full reasoning in [ - `deployment.tf`'s `system` volume is an `empty_dir`, rebuilt from the image on every pod start — a fix to anything under `/usr`, `/etc`, `/var` must go in the image or the init script, not be treated as a one-time patch. - The Dockerfile writes shared env vars to `/etc/environment` rather than using `ENV`, because `PATH` needs to be extended by a script running after the image is built, not fixed at build time. -- `parameters.tf`'s `local.validated_*` allowlist is the only thing stopping `system_packages`/`preferred_nodes` from injecting shell metacharacters into the init container — any new parameter whose value reaches a shell must go through the same validate-then-use step. `memory_watchdog_mode` follows it too: Coder constrains the value server-side, but it is the single switch deciding whether the watchdog may signal processes, so an unrecognised value falls back to the inert `observe` rather than being passed through. -- `script-memory-watchdog.sh` computes headroom as `memory.max − U`, where `U` sums only the *unreclaimable* fields of `memory.stat` (`anon`, `shmem`, `unevictable`, `slab_unreclaimable`, `kernel_stack`, `pagetables`, `sec_pagetables`, `percpu`, `sock`). Do not "simplify" it to `memory.current` or to `memory.stat`'s `kernel` roll-up: on the live pod those read 92% and 42% of the limit while true `U` is 23%, so either substitution makes the watchdog fire permanently on an idle container. Its tier thresholds and its `RLIMIT_DATA` ceilings are **derived from the pod's own `memory.max`**, not written into the script — the workspace is offered at 4 and 8 GiB, and a ladder fixed for one sits permanently on its first rung in the other. The rungs are multiples of a single critical reserve (`memory.max / 10`, clamped to 384 MiB…1 GiB); at 8 GiB this reproduces the hand-tuned numbers it replaced. A pod too small for the ladder to fit inside refuses to enforce and logs why. Set any `WATCHDOG_T_L*` or `WATCHDOG_CEILING_` in the environment to override one value without replacing the derivation. -- **The watchdog decides what is a VS Code process by executable path — `argv[0]` under `~/.vscode-server/` — never by whether something "is node".** A provisioned workspace has two unrelated node installations: VS Code's bundled one under `~/.vscode-server/cli/servers/Stable-/server/`, and mise's on `PATH`, which is what repo tooling and the operator's agent sessions run on. (There is no `/usr/bin/node`, and nothing named `node` on `PATH` at all without dotfiles.) Matching on `comm`, on a basename, or on a loose cmdline substring would classify an agent session spawned by an extension — a child of the extension host, and *not* under ptyHost — as a sheddable editor helper. `comm` in particular is `MainThread` for every node process in a real tree, never `node`, because V8 renames its main thread; nothing may key off it. `script-memory-watchdog-test.sh` asserts this three ways, each paired with the mutation that flips it. -- The watchdog never signals anything in the `--type=ptyHost` subtree. Tree membership alone is *not* a safe kill criterion: tmux sessions and agent runs started from a VS Code integrated terminal are descendants of the server tree through ptyHost, so a tree-wide kill would take the operator's work with it. The exclusion is asserted, together with the mutation that must flip it, in `script-memory-watchdog-test.sh`. -- **The never-signal guards match `comm`, `argv[0]`'s basename, and whole path segments of argv elements — never a substring of the joined command line.** Loose substrings over-matched twice: `*/claude*` protected an unrelated process because a scratchpad path contained `/claude`, and `*memory-watchdog*` protected *every* process in a test harness because the harness's own directory path contained it, leaving two full runs green while asserting nothing. The watchdog's own identity is now structural — its pid, ancestors and descendants — rather than a name at all. Each guard records which rule claimed a process, and the tests assert every rule is individually reachable; a guard nothing can trigger is untested, not correct. -- **`RLIMIT_DATA` accounts `VmData`, not RSS, and on a V8 process the two differ by an order of magnitude** — measured on the live 4 GiB workspace at rest, the extension host was 497 MB resident against 1004 MB of data, the file watcher 66 MB resident against 622 MB. A ceiling reasoned about as though it bounded RSS is therefore far tighter than intended: the first derived file-watcher ceiling was *below* what an idle file watcher already held, which in enforce mode would have killed it on its next allocation and again on every restart. Every ceiling is now `max(derived, observed data + 2 × reserve)`, i.e. a growth allowance rather than an absolute size, and a role that could only be capped above `memory.max` is reported instead of capped. -- **The graded rungs do not engage against the failure this pod actually has** — measured, not assumed. See [DESIGN.md](DESIGN.md#design-tensions-and-decisions): the recorded OOM kills are 40–90 second spikes, and a live reproduction on the test workspace took the container from idle to `OOMKilled` in 43 seconds while the watchdog, running in enforce mode throughout, never left `L0` and logged no action at all. Do not tune the debounce to "fix" this without first asking whether a poll loop can see the event at all; the lever that did work was the `RLIMIT_DATA` cap, which is preventive and needs no sampling. +- `parameters.tf`'s `local.validated_*` allowlist is the only thing stopping `system_packages`/`preferred_nodes` from injecting shell metacharacters into the init container — any new parameter whose value reaches a shell must go through the same validate-then-use step. +- `script-memory-watchdog.sh` computes headroom as `memory.max − U`, where `U` sums only the *unreclaimable* fields of `memory.stat` (`anon`, `shmem`, `unevictable`, `slab_unreclaimable`, `kernel_stack`, `pagetables`, `sec_pagetables`, `percpu`, `sock`). Do not "simplify" it to `memory.current` or to `memory.stat`'s `kernel` roll-up: on the live pod those read 96% and 42% of the limit while true `U` is 28%. Nothing acts on this number any more — it is pod-level context for the per-process rows and the honest figure published in the workspace UI. +- **The watchdog is a drift policer, not an OOM preventer, and the difference is measured.** The graded L1–L4 shedding ladder that used to be here was removed, not tuned: the recorded kills are 70–220 MB/s spikes that go from idle to dead inside a minute, a live reproduction climbed the ladder correctly and logged `no-candidates` because the runaway was not in the tree it managed, and the entire editor tree it could shed is ~0.7 GiB — five seconds of that growth. Before re-adding anything reactive, establish that a poll loop can see the event at all. What the loop *is* good at is MB-per-minute growth in the standing population, which is what it now does. +- **Budgets are per role, in PSS, and never below a role's measured resting size.** Fresh-tree measurements: extension host 471 MB PSS, serverMain 160 MB, ptyHost 36 MB, file watcher 34 MB. A uniform 512 MB budget would sit 40 MB above where the extension host starts (and 256 MB below its floor outright), which is the same class of defect as the earlier `RLIMIT_DATA` ceiling that landed *below* what an idle file watcher already held. Each budget is `max(min(role budget, memory.max / 8), resting × 1.5)`, so the pod share can never push a budget under what the role demonstrably needs, and the tests assert that property at every pod size rather than asserting the arithmetic. Override one role with `WATCHDOG_BUDGET_`; PSS (`smaps_rollup`) is the comparison, not RSS and not `VmData`. +- **A kill needs ten minutes of continuous over-budget dwell, and three kills of one role inside an hour disarm that role.** The dwell is what separates drift from load — a language server that balloons while indexing and hands the memory back must survive. The breaker is what stops the failure that would make this actively harmful: kill the extension host → VS Code restarts it → it reloads every extension → it exceeds again → kill, a loop that arrives looking exactly like the watchdog working. It disarms and reports rather than widening its own budget, because a mechanism that raises the limit it is enforcing has stopped enforcing. +- **The watchdog decides what is a VS Code process by executable path — `argv[0]` under `~/.vscode-server/` — never by whether something "is node".** A provisioned workspace has two unrelated node installations: VS Code's bundled one under `~/.vscode-server/cli/servers/Stable-/server/`, and mise's on `PATH`, which is what repo tooling and the operator's agent sessions run on. (There is no `/usr/bin/node`, and nothing named `node` on `PATH` at all without dotfiles.) `comm` is `MainThread` for every node process in a real tree, never `node`, because V8 renames its main thread; nothing may key off it. `script-memory-watchdog-test.sh` asserts this three ways, each paired with the mutation that flips it. +- **Helpers spawned by an agent session are the second policed population, and the walk that finds them stops at two boundaries: a shell, and a change of session id.** MCP servers live nowhere near `~/.vscode-server`, so the tree-scoped selection could not see them — the largest single offender ever measured was 1.66 GB of python. The session rule is measured, not assumed: on the live workspace a session root has `sid` = its login shell's session, while every Bash tool call has `pgid == sid == its own pid`, because Claude Code detaches each one. That is what keeps an in-flight build out of the policed set even when the tool call's shell has exec'd itself away, which the shell test alone would miss. If Claude Code ever stops detaching, the walk polices *nothing* rather than the wrong thing, and the visibility warning in `actions.log` says so. +- **Identity guards are absolute; the two positional rules are not, and the distinction is deliberate.** `pid 1`, the coder agent, tmux, `claude` session roots, agent payloads and the watchdog's own kin may never be signalled by anything. The ptyHost subtree and "does not run VS Code's own binary" bound the *editor* selection only, so that an MCP server is policed the same whether its session was started with `coder ssh` or in a VS Code terminal — sparing one set because of which terminal it came from would make the mechanism miss half its cases silently. Everything the ptyHost rule genuinely protects (shells, multiplexers, sessions, tool calls) is still covered by identity guards and by the shell/session boundaries. +- **The never-signal guards match `comm`, `argv[0]`'s basename, and whole path segments of argv elements — never a substring of the joined command line.** Loose substrings over-matched twice: `*/claude*` protected an unrelated process because a scratchpad path contained `/claude`, and `*memory-watchdog*` protected *every* process in a test harness because the harness's own directory path contained it, leaving two full runs green while asserting nothing. The watchdog's own identity is structural — its pid, ancestors and descendants — rather than a name at all. Each guard records which rule claimed a process, and the tests assert every rule is individually reachable; a guard nothing can trigger is untested, not correct. +- **The per-process sweep log is a deliverable, not decoration.** `~/.local/state/vscode-memory-watchdog/sweep.log` (plus `sweep.latest`, `summary`, `headroom`, `top`, `calibration.csv`, `actions.log`) records every policed process and every unmanaged one above 32 MB, with a stable identity breadcrumb (an MCP server's module, not `python3`) and argv secrets redacted at the point of writing. The previous version computed this table every cycle and threw it away, which is why every post-mortem in this investigation was unanswerable. There is no metrics path out of the pod today — the cluster's log agent tails container stdout, which these files are not — so do not delete the local log on the assumption that Prometheus has it. +- Watchdog state keys — the dwell clock and the SIGTERM/SIGKILL escalation — are keyed on `pid:starttime`, never on pid alone. A recycled pid must not inherit another process's history and be killed for it. +- `parameters.tf`'s `memory_watchdog_mode` also goes through the `local.validated_*` treatment: Coder constrains the value server-side, but it is the single switch deciding whether the watchdog may signal processes, so an unrecognised value falls back to the inert `observe` rather than being passed through. - Adding a package/tool has three possible homes, and picking the wrong one is a real mistake, not a style choice — route by the rule in [DESIGN.md](DESIGN.md#where-the-workspace-environment-comes-from): universal + stable → image (`Dockerfile`); occasionally-needed + apt-only + too heavy to bake in → the template's `system_packages` parameter; personal, fast-moving, or not an apt package → the operator's dotfiles (a *different* repo — see below), never this one. - `deployment.tf` mounts `/tmp` on its own ephemeral Longhorn volume, not the node's root filesystem and not the NFS-backed home PVC - see [DESIGN.md](DESIGN.md#design-tensions-and-decisions) for why both of those are wrong for it. Its lifecycle is per-Pod, the same as the `system` volume, so it is *not* wiped by a container-only restart within a live Pod - `script-agent-startup.sh` wipes it explicitly on every agent start instead. Anything relying on `/tmp` persisting across an agent restart was already wrong before this (the same was true for free when it was the container's writable overlay). - `deployment.tf`'s Deployment `metadata.name` (`local.workload_name` in `main.tf`) is not cosmetic: the cluster's Prometheus resolves pod → ReplicaSet → Deployment via an existing `kube_pod_owner` recording rule and exposes the result as a `workload` label with no other join needed, so whatever this Deployment is named *is* the identity CPU/memory/PSI/OOM metrics get attributed to. Don't revert it to an opaque identifier (e.g. the workspace UUID) without re-breaking that attribution — see [DESIGN.md](DESIGN.md#design-tensions-and-decisions). diff --git a/DESIGN.md b/DESIGN.md index 71ef6e89..9ce8d062 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -71,19 +71,21 @@ The rule that ties the layers together: a package or tool belongs in the *lowest - *The prefix is `coder-workspace-`, not just `coder-`*, matching the `app.kubernetes.io/part-of` value already used in `main.tf`'s `common_labels`. A bare `coder-` prefix isn't enough to unambiguously mean "workspace": the same Kubernetes namespace also holds the `coder` control-plane Deployment itself and other `coder`-prefixed infra (e.g. a CloudNativePG cluster named `coder-db-`) that a naive `workload=~"coder-.+"` match would also catch. - *Renaming a workspace already relocates its home directory* (the `home` volume's `sub_path` is `data.coder_workspace.me.name`), so coupling the Deployment name to the workspace name too doesn't introduce a new class of rename hazard — it's already priced in. A rename recreates the Deployment (the pod restarts anyway) and needs a fresh `coder-workspace--` home subdirectory, exactly as it needed a fresh `sub_path` before this change. -**A userspace memory watchdog, because the kernel's own mechanisms are out of reach.** The workspace pod has a hard memory limit, and a memory-hungry editor server can walk it into a cgroup OOM. The kill itself would be tolerable; its blast radius is not. `memory.oom.group` is set to `1` by the kubelet, so a cgroup OOM kills *every process in the container as a group* — the IDE, every tmux session, and every long-running agent, together. That also rules out the usual mitigation: with `oom.group = 1`, nudging `oom_score_adj` cannot make one process die instead of all of them, because there is no victim selection left to influence. +**A userspace memory watchdog that bounds the standing population of helper processes.** The workspace pod has a hard memory limit and no way to enforce anything below it: `/sys/fs/cgroup` is mounted read-only, `cgroup.subtree_control` is empty, the cgroup namespace is private and the workspace user has no capabilities, so `memory.high` and a child cgroup both need `privileged: true` or a read-write host mount — exactly what *Unprivileged by default* above exists to prevent. `memory.oom.group` is `1`, so a cgroup OOM still kills every process in the container together, and `oom_score_adj` cannot influence a victim selection that no longer happens. -The two obvious fixes are both unreachable from inside this container. Throttling with `memory.high`, or confining the editor to a child cgroup, would need a writable `/sys/fs/cgroup` — but it is mounted read-only, `cgroup.subtree_control` is empty, the cgroup namespace is private, and the workspace user has no capabilities. Getting either would mean `privileged: true` or a read-write host mount of `/sys/fs/cgroup`, which is exactly what *Unprivileged by default* above exists to prevent. Raising the limit was also considered and rejected: it moves the wall rather than removing it, and the pod is already large for a single-operator homelab. +[`script-memory-watchdog.sh`](templates/kubernetes/homelab-workspace/script-memory-watchdog.sh) does **not** try to prevent that OOM, and the earlier version of this section, which said it did, was wrong on the evidence. The recorded kills are spikes: 70–220 MB/s, idle to dead inside a minute, with an agent session or `node` named as the victim in the kernel log and never a VS Code process. A poll loop cannot win that race — a generic biggest-RSS killer beats the kernel only at a 0.3 s interval, loses at 0.5 s, and while `oom.group = 1` is killed by the very event it lost. The graded shedding ladder that used to live here was exercised against a real spike, climbed correctly, and logged `no-candidates`: the runaway was not in the tree it managed, and the whole editor tree it *could* have shed is ~0.7 GiB, or five seconds of that growth rate. The ladder has been removed rather than tuned, and the measurement it did well — unreclaimable memory, pressure, refault and reclaim rates — has been kept. -What is left is to never reach the limit in the first place, which is what [`script-memory-watchdog.sh`](templates/kubernetes/homelab-workspace/script-memory-watchdog.sh) does. It samples how much genuinely unreclaimable memory the cgroup holds, and — as the editor's helper processes grow — lowers their *soft* `RLIMIT_DATA` so that one of them fails its own allocation and restarts, instead of the kernel taking down the whole container. Lowering another same-uid process's soft limit needs no privilege, and leaving the hard limit alone means any shell that inherits the ceiling can lift it again. +What a poll loop is good at is growth measured in MB per *minute*, and that drift is the real, daily problem: the operator was policing it by hand for months, repeatedly killing VS Code to save agent sessions, with a python MCP server holding 1.66 GB at one of the kills. So the watchdog now keeps the resting population of **restartable helpers** inside per-role budgets. The goal is runway rather than rescue: when a spike does arrive, it starts from as much free memory as the pod can offer. Every process it may signal has a supervisor — VS Code respawns its own forks and language servers, an agent session respawns its MCP servers — so a wrong kill costs a reload, not a session, and that asymmetry is what licenses being aggressive. -Which processes it may touch is settled by executable path, not by name or role heuristics: only a process whose own binary lives under `~/.vscode-server` counts as the editor's. That boundary is doing more work than it appears to. A provisioned workspace carries two unrelated node installations — VS Code's bundled one, which arrives with the server download, and the operator's from mise, which is what repo tooling and long-running agent sessions run on — and a rule that asked "is this node" instead of "whose binary is this" would classify an agent session spawned by an extension as an editor helper and shed it. The watchdog exists to stop the operator's work being collateral damage, so a detection rule that makes it the target would be a self-defeating one. Terminal descendants are excluded on top of that, by excising the editor's pty host and everything beneath it. +**Budgets are per role and anchored on measurement, because a uniform number is wrong in both directions.** On a fresh tree the extension host is already 471 MB PSS, serverMain 160 MB and the file watcher 34 MB, so a single 512 MB budget would sit 40 MB above where the extension host starts and below where it spends its life, while being far too generous for a file watcher. Each role therefore gets a budget clamped between a share of the pod's own `memory.max` and one-and-a-half times its measured resting size, and the resting floor wins when they conflict — a budget below resting usage is not a conservative budget, it is a kill loop written down, which is the defect that reached enforce-readiness twice under the previous design. PSS is the quantity compared, not RSS and not `VmData`: `VmData` was only ever relevant because `RLIMIT_DATA` accounts it, and on a V8 process it runs an order of magnitude above what the pod actually pays. -The trade is that this is a userspace daemon in a pod with no supervisor, doing something the kernel would do better if it were allowed to. It is therefore built to be deletable in one step if the constraint ever lifts, and it defaults to an observe-only mode — measuring and logging, changing nothing. Its measurement deliberately disagrees with every stock memory reading, including Coder's own: page cache and reclaimable slab make this pod look near death while it is idle, and a watchdog that believed them would fire constantly. That disagreement is the point of the thing, so the honest number is surfaced next to the misleading one in the workspace UI rather than replacing it. +Two things then keep aggression from becoming harm. A process must be over budget **continuously for ten minutes** before anything happens to it, so a language server that balloons while indexing and hands the memory back is treated as load rather than drift. And a role that has to be killed three times inside an hour is not drifting — its budget is wrong for this workload — so the watchdog **disarms itself for that role**, logs why, and leaves the number to a human. It never widens its own budget: a mechanism that quietly raises the limit it enforces is a mechanism that has stopped enforcing. The kill loop, not the wrong kill, is the failure mode that would make this actively harmful, and it arrives looking exactly like the watchdog working. -Everything it acts on is derived from the pod's own `memory.max` rather than fixed in the script, because the workspace is offered in more than one size and a ladder tuned for the larger one sits permanently on its first rung in the smaller. The rungs are multiples of a single critical reserve — a tenth of the limit, floored and capped — so the 8 GiB pod keeps the numbers that were reasoned about for it, the 4 GiB pod gets the same shape scaled down, and a pod too small for the ladder to fit inside at all refuses to enforce and says why instead of shedding the editor continuously. Frequency is part of that: each rung fires at most once per excursion and only recovery re-arms it, because an editor that dies every fifteen minutes gets the watchdog switched off, and a watchdog that is switched off protects nothing. +**What it may touch is decided structurally, and there are two populations rather than one.** Inside the VS Code tree, only a process whose own binary lives under `~/.vscode-server` counts as the editor's: a provisioned workspace carries two unrelated node installations — VS Code's bundled one and the operator's from mise — and a rule that asked "is this node" instead of "whose binary is this" would classify an agent session spawned by an extension as a sheddable helper. The second population is what an agent session spawned directly, chiefly MCP servers, which live nowhere near `~/.vscode-server` and which the tree-scoped selection could not see at all. That walk descends from each session root and stops at two boundaries: any shell or multiplexer, and any change of session id. The second is measured, not assumed — on the live workspace a session root has `sid` equal to its login shell's session while every Bash tool call has `pgid == sid == its own pid`, because Claude Code detaches each one — and it is what keeps an in-flight build out of the policed set even when the tool call's shell has exec'd itself away. Identity guards (`pid 1`, the agent, tmux, the session roots themselves, agent payloads, the watchdog's own kin) are absolute; the two *positional* rules — the pty host's subtree, and "does not run VS Code's binary" — bound the editor selection only, so that an MCP server is treated the same whether its session was started with `coder ssh` or in a VS Code terminal. -**What the watchdog does not address, established by measurement rather than assumed.** Every memcg OOM kill recorded for this workspace in the kernel's own log names a Claude Code session as the victim — never a VS Code process, and at the largest one VS Code was not running at all. The kills are also spikes rather than growth: a single session went from a few hundred megabytes to 7.3 GiB of anonymous RSS inside ninety seconds, and a reproduction on the test workspace took the container from idle to `OOMKilled` in fifty-two seconds. Two consequences follow, and they are the reason this section is worth reading before extending the thing. First, no sampling interval this daemon can afford will reliably see such an event in time — it is a preventive limit that helps, not a reactive one. Second, the editor tree it manages is roughly 0.7 GiB, so shedding all of it buys seconds against a runaway of that size; the ladder is a brake, not the answer. What *does* convert that failure into a survivable one is the same `RLIMIT_DATA` lever pointed at the runaway itself: capped, the identical allocation fails inside its own process with an ordinary `RangeError` and the container is untouched. Extending the ceilings beyond the editor tree is therefore the obvious next question, and it is deliberately left open here because it decides what may happen to the operator's own long-running sessions. +**It records every sweep, and that is a deliverable rather than decoration.** Each per-process sweep is appended to a rotating local log with role, PSS, RSS, age, budget, how long the process has been over it, which guard claimed it, and a stable identity breadcrumb — the MCP server's module name rather than `python3` — with secrets in argv redacted at the point of writing. Processes it does *not* manage are recorded too, because on the evidence of every OOM in this investigation that is where the memory actually was. The previous version computed this table on every cycle and discarded it, which is why every post-mortem here has been unanswerable. There is no metrics path out of the pod today (the cluster's log agent tails container stdout, which this file is not), so a durable local log is the floor and metrics are a later, additive question. + +The trade is unchanged in shape: this is a userspace daemon in a pod with no supervisor, doing crudely what the kernel would do properly if it were allowed to, and it is built to be deleted in one step if that ever changes. It measures deliberately against every stock reading, including Coder's own — page cache and reclaimable slab make this pod look near death while it is idle — so the honest number is published beside the misleading one in the workspace UI rather than replacing it, next to the largest helper as a share of its budget. What remains unaddressed is the acute spike, and honestly so: that is the kernel's job, and it would do it far better with `singleProcessOOMKill` enabled at the kubelet, which was checked while writing this and is **not** in effect — `memory.oom.group` reads `1` on both a long-lived workspace and a pod created minutes earlier. ## Outcomes targeted diff --git a/TESTING.md b/TESTING.md index 00d369cd..c8ce683f 100644 --- a/TESTING.md +++ b/TESTING.md @@ -23,6 +23,15 @@ Both modes run the identical sequence of stages; only what each stage is permitt Because all three stages run for real in dry-run — just scoped away from production — a passing PR is a meaningful signal that a live release would also succeed, not a guess based on static checks alone. +## The memory watchdog is the one part with runtime behaviour + +Everything else here is declarative and is covered by the stages above. `script-memory-watchdog.sh` decides at runtime whether to kill a process, so it gets two things neither lint nor a template push can provide: + +- **Fixtures** — `./templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh`, also run by the `watchdog` job in `.github/workflows/test.yaml`. The suite is built around negative assertions paired with the mutation that must flip them: "it did not kill the agent session" proves nothing unless removing one rule makes it kill the agent session. `kill` is shadowed by a function throughout, because the fixture pids name real processes in whatever container runs the suite. +- **A live drill on the `test` workspace**, which has disposable storage and can be wrecked freely. Fixtures cannot answer whether a real process tree classifies correctly, whether a supervisor really does respawn what was killed, or whether the circuit breaker stops a loop rather than joining it. The drill that has been run: a stand-in session root with an over-budget helper that a supervisor respawns, a second helper inside its budget, and a detached tool call larger than both. In `enforce` mode with the dwell shortened, the watchdog killed the drifted helper three times, disarmed that role on the third with the loop message, and left the fourth incarnation, the tool call, and the session roots untouched. + +Neither replaces the other, and the live one is where every defect that mattered in this component has been found. + ## After merge Merging to `main` is what flips the pipeline into live mode — there's no separate promotion step afterward. The dry-run pass on the PR is the actual release gate. diff --git a/templates/kubernetes/homelab-workspace/coder-agent.tf b/templates/kubernetes/homelab-workspace/coder-agent.tf index 4f0a1924..5e57f998 100644 --- a/templates/kubernetes/homelab-workspace/coder-agent.tf +++ b/templates/kubernetes/homelab-workspace/coder-agent.tf @@ -60,5 +60,16 @@ resource "coder_agent" "main" { interval = 60 timeout = 1 } + metadata { + display_name = "Largest Helper" + key = "7_largest_helper" + # The biggest restartable helper as a share of its budget, also from the + # watchdog. This tile is the point of the drift half of the design: the + # operator used to obtain this number by running ps and then killing things + # by hand, and a number nobody can see is a number nobody acts on. + script = "cat $${HOME}/.local/state/vscode-memory-watchdog/top 2>/dev/null || echo '-'" + interval = 60 + timeout = 1 + } } diff --git a/templates/kubernetes/homelab-workspace/env.tf b/templates/kubernetes/homelab-workspace/env.tf index de4306b7..d722adf7 100644 --- a/templates/kubernetes/homelab-workspace/env.tf +++ b/templates/kubernetes/homelab-workspace/env.tf @@ -4,17 +4,16 @@ resource "coder_env" "welcome_message" { value = local.homebrew_directory } -# The switch that arms the memory watchdog. "observe" measures, publishes -# headroom and logs what it would have done; "enforce" additionally sets -# RLIMIT_DATA ceilings and sheds load. +# The switch that arms the memory watchdog. "observe" measures and records what +# it would have done; "enforce" kills helpers that have been over budget for ten +# minutes; "enforce-all" adds the two editor roles whose restart the operator can +# see. # -# Set from a mutable workspace parameter rather than hardcoded here, because the -# right value is a per-workspace judgement: the tier thresholds are absolute -# bytes sized for an 8 GiB pod, so the same setting that is right there sits -# permanently near L1 on a 4 GiB one. It defaults to "observe" and should stay -# there until the ceilings and thresholds have been set from the calibration data -# the watchdog collects - too low kills a healthy extension host mid-edit, too -# high makes the mechanism inert. +# Set from a mutable workspace parameter rather than hardcoded here, because it +# is the one control that decides whether the watchdog may signal anything, and +# because turning it off has to be a parameter change rather than a code change +# on a bad day. The budgets themselves are derived from the pod's own memory.max, +# so this value does not have to be reconsidered per pod size. resource "coder_env" "memory_watchdog_mode" { agent_id = coder_agent.main.id name = "WATCHDOG_MODE" diff --git a/templates/kubernetes/homelab-workspace/parameters.tf b/templates/kubernetes/homelab-workspace/parameters.tf index 6170cc5f..187716aa 100644 --- a/templates/kubernetes/homelab-workspace/parameters.tf +++ b/templates/kubernetes/homelab-workspace/parameters.tf @@ -64,21 +64,26 @@ data "coder_parameter" "system_packages" { data "coder_parameter" "memory_watchdog_mode" { name = "memory_watchdog_mode" - default = "observe" + default = "enforce" display_name = "Memory Watchdog" - description = "What the memory watchdog is allowed to do when the pod runs low on unreclaimable-memory headroom" + description = "What the memory watchdog may do about a helper process that has been over its budget for ten minutes" icon = "/icon/memory.svg" mutable = true option { name = "Observe only" value = "observe" - description = "Measure, publish headroom and log what it would have done. Sets no limits and sends no signals" + description = "Measure, record every sweep, and log the kill it would have made. Sends no signals" } option { - name = "Enforce" + name = "Enforce (helpers)" value = "enforce" - description = "Also cap helper processes with RLIMIT_DATA and shed load as headroom falls. Do not enable before the thresholds have been set from calibration data" + description = "Also kill drifted helpers: language servers, the file watcher, native extension helpers, and MCP servers an agent session spawned. Each restarts invisibly" + } + option { + name = "Enforce (helpers and editor)" + value = "enforce-all" + description = "Also kill the VS Code extension host and server. These restart visibly, so they are armed separately" } } @@ -91,7 +96,7 @@ locals { # below, and anything unrecognised falls back to the inert mode rather than to # whatever was supplied. validated_watchdog_mode = contains( - ["observe", "enforce"], data.coder_parameter.memory_watchdog_mode.value + ["observe", "enforce", "enforce-all"], data.coder_parameter.memory_watchdog_mode.value ) ? data.coder_parameter.memory_watchdog_mode.value : "observe" validated_system_packages = (data.coder_parameter.system_packages.value != "") ? [ diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh index 7ec6277d..cebd2512 100755 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh @@ -8,13 +8,12 @@ # # CI runs it too, in the `watchdog` job of .github/workflows/test.yaml, which # fails the build on the first failed assertion. It exists so that the two -# things in the watchdog that can actually hurt the operator - the -# unreclaimable-memory arithmetic and the process-selection rules - can be -# changed with evidence rather than hope. +# things in the watchdog that can actually hurt the operator - which processes it +# is willing to kill, and when - can be changed with evidence rather than hope. # # The important cases here are the negative ones. A test that asserts "the -# watchdog did not signal the memory hog" proves nothing unless the same fixture, -# with the ptyHost marker removed, produces the opposite result - so each +# watchdog did not signal the agent session" proves nothing unless the same +# fixture, with one rule removed, produces the opposite result - so each # exclusion is paired with the mutation that must flip it. # # The fixtures are transcribed from a live workspace, not written from a reading @@ -22,6 +21,11 @@ # version of this file assumed `comm` would be `node` for the server processes, # it is `MainThread` on every real tree, and the whole suite was green while the # detection it was guarding picked the wrong process. +# +# `kill` is shadowed by a function throughout. The fixture pids are small +# integers - 1, 40, 41 - which in this container name real processes, and a +# harness that sent a real SIGTERM to pid 41 to prove it would have sent one is +# not a harness anybody should run. set -uo pipefail SELF_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" @@ -30,6 +34,7 @@ trap 'rm -rf "${WORK}"' EXIT PASS=0 FAIL=0 +SIGNALS="" ok() { PASS=$((PASS + 1)) @@ -50,6 +55,24 @@ assert_eq() { fi } +assert_contains() { + local hay=$1 needle=$2 what=$3 + if [[ $hay == *"$needle"* ]]; then + ok "$what" + else + bad "${what}: '${needle}' not found" + fi +} + +assert_absent() { + local hay=$1 needle=$2 what=$3 + if [[ $hay == *"$needle"* ]]; then + bad "${what}: '${needle}' is present and should not be" + else + ok "$what" + fi +} + # assert_protected assert_protected() { local pid=$1 want=$2 what=$3 got=no @@ -61,23 +84,33 @@ assert_protected() { fi } -# Seeds the debounce state the watchdog carries between samples. These are -# globals of the sourced script, which shellcheck cannot see assigned here. -# shellcheck disable=SC2034 -reset_tier_state() { - C_L1=0 - C_L2=0 - C_L3=0 - LAST_ACTION_AT=${1:-0} - RUNG_FIRED=() - IN_EXCURSION=0 - EXCURSIONS=0 +# assert_armed +assert_armed() { + local role=$1 want=$2 what=$3 got=no + role_is_armed "$role" && got=yes + assert_eq "$want" "$got" "$what" +} + +# assert_policed +assert_policed() { + local pid=$1 want=$2 what=$3 + assert_eq "$want" "${POLICED[$pid]:-no}" "$what" +} + +# The stand-in for the kill builtin. Every signal the watchdog believes it sent +# is recorded here and nothing leaves this process. +# shellcheck disable=SC2317,SC2329 # called indirectly, from the sourced watchdog +kill() { + SIGNALS+="${1#-}:$2 " + return 0 } # --------------------------------------------------------------------------- # # fixtures # --------------------------------------------------------------------------- # +UPTIME_S=100000 + # memory.stat as read from the real 8 GiB workspace pod at rest, trimmed to the # fields the watchdog reads plus the ones it must be careful to ignore. write_cgroup() { @@ -109,19 +142,23 @@ EOF } # add_proc +# +# Every process is old by default; the ones whose age matters set it explicitly +# with set_age. stat's field 22, starttime, is written for real because every +# piece of state the watchdog carries between sweeps is keyed on pid:starttime - +# a recycled pid must not inherit another process's dwell clock. add_proc() { local dir=$1 pid=$2 ppid=$3 comm=$4 rss=$5 shift 5 local d="${dir}/${pid}" mkdir -p "$d" - printf '%s (%s) S %s 0 0 0 -1 4194560 0 0 0 0 0 0 0 0 20 0 1 0 0\n' \ + # stat fields 5, 6 and 7 are pgrp, session and tty. The session is the one the + # helper walk keys on, so it is written for real; everything shares session 1 + # unless set_sid says otherwise. + printf '%s (%s) S %s 1 1 0 -1 4194560 0 0 0 0 0 0 0 0 20 0 1 0 100\n' \ "$pid" "$comm" "$ppid" >"${d}/stat" - # statm: size resident shared text lib data dt. `data` is what RLIMIT_DATA - # accounts, and on a real V8 process it is several times resident - see - # set_proc_data, and read_rss() in the watchdog for the live measurements. - # Defaulting it to resident keeps the fixtures honest about which field is - # being read without asserting a relationship that does not hold. printf '0 %s 0 0 0 %s 0\n' "$((rss / 4096))" "$((rss / 4096))" >"${d}/statm" + set_pss "$dir" "$pid" "$rss" cat >"${d}/limits" <<'EOF' Limit Soft Limit Hard Limit Units Max data size unlimited unlimited bytes @@ -133,25 +170,51 @@ EOF done } -# set_proc_data -set_proc_data() { - local dir=$1 pid=$2 rss=$3 data=$4 - printf '0 %s 0 0 0 %s 0\n' "$((rss / 4096))" "$((data / 4096))" >"${dir}/${pid}/statm" +# set_pss . PSS is what every budget is compared against, +# so it is the fixture knob the drift tests turn. +set_pss() { + local dir=$1 pid=$2 bytes=$3 + cat >"${dir}/${pid}/smaps_rollup" < . Claude Code detaches every Bash tool call into +# its own session; this is how a fixture says so. +set_sid() { + local dir=$1 pid=$2 sid=$3 + local -a f + read -r -a f <"${dir}/${pid}/stat" + f[4]=$sid + f[5]=$sid + printf '%s\n' "${f[*]}" >"${dir}/${pid}/stat" +} + +# set_age +set_age() { + local dir=$1 pid=$2 age=$3 line rest + read -r line <"${dir}/${pid}/stat" + rest=${line% *} + printf '%s %s\n' "$rest" "$(((UPTIME_S - age) * 100))" >"${dir}/${pid}/stat" +} + +write_uptime() { + printf '%s.00 %s.00\n' "$UPTIME_S" "$((UPTIME_S * 4))" >"${1}/uptime" } # A representative tree, transcribed from `ps -eo pid,ppid,comm,args` on a live -# workspace with a VS Code server attached. Argument shapes, argv[0] paths and - -# the part that matters most - the `comm` values are what that capture showed, -# not what a reading of the VS Code source would suggest. +# workspace with a VS Code server attached, and extended with the second +# population this watchdog now polices: the helpers an agent session spawns. # # The `comm` values are load-bearing. Every node process in a real server tree # reports MainThread, because V8 renames its main thread with prctl(PR_SET_NAME). # An earlier version of this fixture wrote `node`, and that one wrong string hid # a root-selection bug that only a real tree could expose. # -# The hog is three levels below ptyHost, exactly like a tmux session or an agent -# started from a VS Code integrated terminal. -# # 1 coder agent # +- 30 bash -l # | +- 31 sh @@ -166,12 +229,19 @@ set_proc_data() { # | | +- 47 claude-code cli.js (mise's node, not VS Code's) # | | +- 48 an extension task (mise's node, not VS Code's) # | +- 42 fileWatcher -# | +- 43 ptyHost <- excised, whole subtree +# | +- 43 ptyHost <- positional protection # | +- 50 bash # | +- 51 tmux: server -# | +- 52 claude +# | +- 52 claude (a session in a VS Code terminal) +# | | +- 54 python MCP server <- policed # | +- 53 node (the hog) -# +- 60 claude (outside the tree) +# +- 60 claude (a session under `coder ssh`) +# +- 61 python MCP server <- policed +# +- 62 node MCP server <- policed +# +- 63 bash -c (a tool call) <- boundary +# | +- 64 a build the session is running <- never policed +# +- 65 claude (a child session) +# +- 66 python MCP server of the child <- policed # # Decoys 3 and 4 both sort before 40 in /proc glob order, which is how the root # used to be picked when no process had comm=node - i.e. always, on a real tree. @@ -184,6 +254,7 @@ build_tree() { # The operator's node, from mise, on PATH. Nothing to do with VS Code's. local mnode="/home/coder/.local/share/mise/installs/node/22.14.0/bin/node" mkdir -p "$dir" + write_uptime "$dir" add_proc "$dir" 1 0 coder 14208 ./coder agent # Mentions both marker strings, and carries a ptyHost-like flag inside a larger # argument. Substring matching over the joined command line would elect it. @@ -205,14 +276,14 @@ build_tree() { add_proc "$dir" 40 34 "$nc" 300000000 "${sdir}/node" "$srv" \ --connection-token=remotessh --start-server --enable-remote-auto-shutdown - add_proc "$dir" 41 40 "$nc" 1500000000 "${sdir}/node" \ + add_proc "$dir" 41 40 "$nc" 500000000 "${sdir}/node" \ --dns-result-order=ipv4first "${sdir}/out/bootstrap-fork" \ --type=extensionHost --transformURIs --useHostProxy=false - add_proc "$dir" 44 41 "$nc" 2000000000 "${sdir}/node" \ + add_proc "$dir" 44 41 "$nc" 400000000 "${sdir}/node" \ "${ext}/ms-vscode.typescript/lib/tsserver.js" --useInferredProjectPerProjectRoot - add_proc "$dir" 45 41 "$nc" 400000000 "${sdir}/node" \ + add_proc "$dir" 45 41 "$nc" 120000000 "${sdir}/node" \ "${ext}/redhat.vscode-yaml-1.24.0/dist/languageserver.js" --node-ipc --clientProcessId=41 - add_proc "$dir" 46 41 terraform-ls 800000000 \ + add_proc "$dir" 46 41 terraform-ls 300000000 \ "${ext}/hashicorp.terraform-2.40.0-linux-x64/bin/terraform-ls" serve # Two processes the extension host spawned that run the *operator's* node, not # VS Code's. There is no /usr/bin/node and nothing named node on PATH in this @@ -224,15 +295,44 @@ build_tree() { "${ext}/anthropic.claude-code-2.1.232/resources/claude-code/cli.js" --ide add_proc "$dir" 48 41 "$nc" 600000000 "$mnode" \ "${ext}/hverlin.mise-vscode-1.6.0/dist/taskRunner.js" --cwd /home/coder/code - add_proc "$dir" 42 40 "$nc" 250000000 "${sdir}/node" \ + add_proc "$dir" 42 40 "$nc" 40000000 "${sdir}/node" \ "${sdir}/out/bootstrap-fork" --type=fileWatcher add_proc "$dir" 43 40 "$nc" 100000000 "${sdir}/node" \ "${sdir}/out/bootstrap-fork" "$ptyhost_arg" --logsPath "${vsc}/data/logs/20260816T162519" add_proc "$dir" 50 43 bash 20000000 /bin/bash -l add_proc "$dir" 51 50 "tmux: server" 5000000 tmux new -s work add_proc "$dir" 52 51 claude 900000000 claude + # Deliberately inside its budget: this one is in the fixture to prove it is + # *reachable*, and a second process over budget would make every assertion + # about which pid was killed depend on hash iteration order. + add_proc "$dir" 54 52 python3 300000000 \ + /usr/bin/python3 -m mcp_server_terminal --stdio add_proc "$dir" 53 51 "$nc" 3000000000 node -e "const a=[];setInterval(()=>a.push(Buffer.alloc(1)),1)" + + # A session started with `coder ssh`, i.e. a child of the agent rather than of + # the editor, and the helpers it spawned. This is the population the + # tree-scoped selection could not see at all. add_proc "$dir" 60 1 claude 700000000 claude + add_proc "$dir" 61 60 python3 1660000000 \ + /home/coder/.local/share/mise/installs/python/3.13/bin/python3 \ + -m homelab_mcp.server --transport stdio + add_proc "$dir" 62 60 MainThread 200000000 \ + /home/coder/.local/share/mise/installs/node/22.14.0/bin/node \ + /home/coder/.local/share/npm/mcp-server-github/dist/index.js + add_proc "$dir" 63 60 bash 3000000 /bin/bash -c "cargo build --release" + add_proc "$dir" 64 63 cargo 2000000000 cargo build --release + # Measured on the live workspace: a session root has sid = the login shell's + # session, and every Bash tool call has pgid = sid = its own pid. The build + # below therefore has two independent reasons not to be policed, and the tests + # remove them one at a time. + set_sid "$dir" 63 63 + set_sid "$dir" 64 63 + # The shape that makes the session rule matter on its own: a tool call whose + # shell exec'd itself away, so there is no shell left in the chain at all. + add_proc "$dir" 67 60 python3 1500000000 /usr/bin/python3 ./scripts/train.py + set_sid "$dir" 67 67 + add_proc "$dir" 65 60 claude 400000000 claude --child-session + add_proc "$dir" 66 65 python3 300000000 /usr/bin/python3 -m mcp_server_fetch } # A second, stale server left behind by --reconnection-grace-time, on a different @@ -251,37 +351,49 @@ add_second_server() { } load_watchdog() { + local mode=${3:-observe} WATCHDOG_SOURCE_ONLY=1 \ WATCHDOG_CGROUP_DIR="$1" \ WATCHDOG_PROC_DIR="$2" \ WATCHDOG_STATE_DIR="${WORK}/state" \ - WATCHDOG_MODE=observe \ + WATCHDOG_MODE="$mode" \ . "${SELF_DIR}/script-memory-watchdog.sh" mkdir -p "${WORK}/state" - # The thresholds and the RLIMIT_DATA ceilings are derived from memory.max on - # the first successful scan rather than being constants, so a harness that - # skipped this would be testing a watchdog with an empty ladder - which is not - # a state the real thing is ever in, and which silently passes any assertion - # about *not* acting. - read_cgroup_memory && derive_limits "$M_MAX" + SIGNALS="" + # Budgets are derived from memory.max on the first cycle rather than being + # constants, so a harness that skipped this would be testing a watchdog with no + # budgets at all - which is not a state the real thing is ever in, and which + # silently passes any assertion about not killing anything. + read_cgroup_memory && derive_budgets "$M_MAX" + read_cgroup_pressure } scan_fixture() { - read_cgroup_memory - read_cgroup_pressure + read_uptime read_process_table build_server_tree compute_protected + compute_policed + read_usage "${PIDS[@]}" } -candidate_pids() { - select_candidates "$1" - local row pid acc="" - for row in "${CANDIDATES[@]}"; do - pid=${row#* } - acc+="${pid%% *} " - done - printf '%s' "${acc% }" +sweep_at() { + sweep_once "$1" + SWEEPS=$((SWEEPS + 1)) +} + +# Replaces the watchdog's role classifier with the one this file used before a +# live tree was consulted: keyed on the joined command line rather than on +# argv[0]. Defined here rather than inline so that the tests below can call +# role_of before the mutation exists. +mutate_role_of_to_cmdline() { + # shellcheck disable=SC2317,SC2329 # invoked indirectly, via the sourced watchdog + role_of() { + case " ${P_CMD[$1]:-} " in + *"/.vscode-server/extensions/"*) ROLE=extensionHelper ;; + *) ROLE=other ;; + esac + } } # --------------------------------------------------------------------------- # @@ -304,16 +416,14 @@ test_measurement() { # The whole reason this formula exists: the naive readings disagree by 4x. assert_eq 23 "$((M_U * 100 / M_MAX))" "U is 23% of the limit" assert_eq 91 "$((M_CURRENT * 100 / M_MAX))" "memory.current is 91% of the limit" + assert_eq ok "$PRESSURE" "and the pod is labelled comfortable, because it is" - # Found on a live pod: the projection term is routinely negative, and bash - # division truncates toward zero, so a naive formatter prints "-27.-79 GiB". assert_eq "6.14 GiB" "$(fmt_gib 6594088184)" "headroom formats as GiB" - assert_eq "-1.50 GiB" "$(fmt_gib -1610612736)" "a negative projection formats with one sign" - assert_eq "0.00 GiB" "$(fmt_gib 0)" "zero formats without a sign" + assert_eq "-1.50 GiB" "$(fmt_gib -1610612736)" "a negative value formats with one sign" + assert_eq "512 MiB" "$(fmt_mib 536870912)" "budgets format as MiB" read_cgroup_pressure assert_eq 0 "$M_PSI_CENTI" "psi full avg10 parses as 0" - write_cgroup "${WORK}/cg" 8589934592 12.34 read_cgroup_pressure assert_eq 1234 "$M_PSI_CENTI" "psi full avg10 parses to centi-units" @@ -321,10 +431,28 @@ test_measurement() { write_cgroup "${WORK}/cg" max 0.00 read_cgroup_memory assert_eq 2 "$?" "an unlimited cgroup is reported, not treated as huge headroom" + + # PSS is the quantity every budget is compared against, and it is not RSS. + write_cgroup "${WORK}/cg" 8589934592 0.00 + load_watchdog "${WORK}/cg" "$pdir" + scan_fixture + set_pss "$pdir" 41 123456789 + read_usage 41 + assert_eq 123456512 "${P_PSS[41]}" "PSS is read from smaps_rollup" + assert_eq 499998720 "${P_RSS[41]}" "and RSS separately from statm" + assert_eq 0 "$PSS_UNAVAILABLE" "with PSS available, nothing is flagged" + + # And the fallback, which must be visible rather than silent: RSS is the larger + # number, so substituting it quietly would make every budget look tighter. + rm -f "${pdir}/41/smaps_rollup" + read_usage 41 + assert_eq 499998720 "${P_PSS[41]}" "without smaps_rollup, RSS stands in" + assert_eq 1 "$PSS_UNAVAILABLE" "and the substitution is recorded, not hidden" + set_pss "$pdir" 41 500000000 } # --------------------------------------------------------------------------- # -# 2. process selection - the part that can hurt the operator +# 2. what is policed, and what is never touched # --------------------------------------------------------------------------- # test_selection() { @@ -341,7 +469,7 @@ test_selection() { assert_eq MainThread "${P_COMM[40]}" "the fixture encodes the real comm value" assert_eq 40 "$SERVER_PID" "server root found with comm=MainThread, not comm=node" assert_eq "40" "${SERVER_ROOTS[*]}" "and the decoys are not roots" - assert_eq 13 "${#SERVER_TREE[@]}" "server tree spans every descendant, ptyHost included" + assert_eq 14 "${#SERVER_TREE[@]}" "server tree spans every descendant, ptyHost included" local d for d in 3 4; do @@ -351,8 +479,6 @@ test_selection() { ok "decoy pid ${d} stays out of the tree" fi done - # The CLI and the shells above the server are ancestors, not descendants: the - # root is server-main.js, so scoping starts there and not at `code command-shell`. for d in 30 31 32 33 34; do if [[ -n ${SERVER_TREE[$d]:-} ]]; then bad "ancestor pid ${d} joined the tree" @@ -371,105 +497,155 @@ test_selection() { assert_eq extensionHost "$ROLE" "the real extension-host argv reads as extensionHost" role_of 42 assert_eq fileWatcher "$ROLE" "the real file-watcher argv reads as fileWatcher" + role_of 44 + assert_eq tsserver "$ROLE" "tsserver reads as tsserver" role_of 45 assert_eq languageServer "$ROLE" "a node language server under extensions/ keeps its role" role_of 46 assert_eq extensionHelper "$ROLE" "terraform-ls reads as a native extension helper" - # It is sheddable but never pre-emptively capped - see role_of() for why a - # ceiling that is graceful for V8 is an abrupt abort for a Go runtime. - if [[ -n ${CEILING[extensionHelper]:-} ]]; then - bad "a native extension helper was given an RLIMIT_DATA ceiling" - else - ok "a native extension helper is given no RLIMIT_DATA ceiling" - fi # ptyHost is matched loosely on purpose, unlike every other role. Reading - # something as ptyHost that is not only ever protects more than necessary; - # reading something as extensionHost that is not gets it signalled at L3. + # something as ptyHost that is not only ever protects more than necessary. # shellcheck disable=SC2034 # P_CMD is a global of the sourced watchdog P_CMD[9001]="/usr/bin/node fork --type=ptyHostSomethingNew" role_of 9001 assert_eq ptyHost "$ROLE" "an unrecognised ptyHost variant still reads as ptyHost" unset 'P_CMD[9001]' - local p - for p in 43 50 51 52 53; do - if [[ -n ${PROTECTED[$p]:-} ]]; then - ok "pid ${p} in the ptyHost subtree is protected" - else - bad "pid ${p} in the ptyHost subtree is NOT protected" - fi - done + assert_policed 40 serverMain "the server root is policed" + assert_policed 41 extensionHost "so is the extension host" + assert_policed 42 fileWatcher "so is the file watcher" + assert_policed 44 tsserver "so is tsserver" + assert_policed 46 extensionHelper "so is terraform-ls" + + assert_policed 43 no "the ptyHost fork itself is never policed" + assert_policed 50 no "nor the shell beneath it" + assert_policed 51 no "nor tmux" + assert_policed 52 no "nor a session running in a VS Code terminal" + assert_policed 53 no "nor the hog that session started" + assert_policed 47 no "nor an agent session the extension host spawned" + assert_policed 48 no "nor an extension task on the operator's node" + assert_policed 60 no "nor a session under coder ssh" + assert_policed 64 no "nor a build a session is running" + assert_policed 1 no "nor the coder agent" + assert_protected 1 yes "pid 1 is protected" - assert_protected 60 yes "a claude outside the tree is protected" - - assert_eq "44 46 45 42" "$(candidate_pids L2)" \ - "L2 offers only kill-safe helpers, heaviest first" - assert_eq "41" "$(candidate_pids L3)" "L3 offers only the extension host" - # Ordered by RSS, so serverMain (300M) precedes fileWatcher (250M). - assert_eq "44 41 46 45 40 42" "$(candidate_pids L4)" \ - "L4 offers the whole tree except the ptyHost subtree" - - # The negative assertion, stated explicitly for every tier. - local tier all - for tier in L2 L3 L4; do - all=" $(candidate_pids "$tier") " - if [[ $all == *" 53 "* ]]; then - bad "${tier} would signal the hog inside a VS Code terminal" - else - ok "${tier} never signals the hog inside a VS Code terminal" - fi - done + assert_protected 60 yes "an agent session is protected" + assert_protected 52 yes "including one inside a terminal" } # --------------------------------------------------------------------------- # -# 3. the mutation that must flip the result +# 2b. the second population: helpers an agent session spawned # -# Without this, "the hog was not selected" is unfalsifiable - it would pass just -# as happily against a watchdog that selects nothing at all. +# These are what the tree-scoped selection could not see at all, and where the +# largest single offender ever measured lived - 1.66 GB of python. The rule is +# structural: descend from a session root, stop at any shell, police what is +# left. The shell boundary is the difference between "anything a session invokes +# is fair game" as a principle and as a foot-gun, because below a shell is the +# session's in-flight work and nothing restarts that. # --------------------------------------------------------------------------- # -test_selection_is_falsifiable() { - printf 'selection is falsifiable\n' +test_claude_helpers() { + printf 'helpers spawned by an agent session\n' write_cgroup "${WORK}/cg" 8589934592 0.00 - local pdir="${WORK}/proc3" - # Same tree, but pid 43 is no longer marked as the ptyHost fork. - build_tree "$pdir" --type=notThePtyHost + local pdir="${WORK}/proc2b" + build_tree "$pdir" load_watchdog "${WORK}/cg" "$pdir" scan_fixture - # The hog runs the operator's node, so the path rule protects it too. It is - # dropped here so that this test measures the ptyHost subtree rule and only - # that; the path rule has its own mutation in the previous test. - # shellcheck disable=SC2317,SC2329 # invoked indirectly, via the sourced watchdog - is_vscode_binary() { return 0; } - compute_protected - assert_protected 53 no "without the ptyHost marker the hog loses subtree protection" - if [[ " $(candidate_pids L4) " == *" 53 "* ]]; then - ok "and L4 would then select it - the exclusion is what keeps it safe" + assert_eq 3 "${#CLAUDE_ROOTS[@]}" "every session root is found, including a child session" + assert_policed 61 claudeHelper "the python MCP server is policed" + assert_policed 62 claudeHelper "and the node one" + assert_policed 66 claudeHelper "and a child session's MCP server" + assert_policed 63 no "a tool call's shell is not" + assert_policed 64 no "and neither is what that shell is running" + assert_policed 67 no "nor a tool call whose shell exec'd itself away - the session says so" + assert_policed 65 no "a child session is a session, not a helper" + + # The identity breadcrumb is what makes the sweep log answerable later: three + # of these are `python3` or `node` and only the argument distinguishes them. + assert_eq "homelab_mcp.server" "$(identity_of 61 claudeHelper)" \ + "a python MCP server is identified by its module, not by python3" + assert_eq "index" "$(identity_of 62 claudeHelper)" \ + "and a node one by its script, not by node" + assert_eq "terraform-ls" "$(identity_of 46 extensionHelper)" \ + "a native extension helper by its binary" + + # The case the ptyHost hole exists for. A session in a VS Code terminal is the + # same thing to the operator as one under `coder ssh`; only its terminal + # differs. Its MCP server is policed, and everything that makes it a terminal + # is not. + assert_policed 54 claudeHelper "an MCP server of a session in a VS Code terminal is policed" + local p + for p in 43 50 51 52 53; do + if [[ -n ${NOT_EDITOR[$p]:-} ]]; then + ok "pid ${p} is inside the ptyHost subtree" + else + bad "pid ${p} is NOT inside the ptyHost subtree" + fi + done + if signal_pid TERM 53 other "test"; then + bad "the hog in a terminal could be signalled" else - bad "L4 still ignores the hog, so the ptyHost assertion proves nothing" + ok "the hog in a terminal is refused by the positional guard" + fi + if signal_pid TERM 52 claudeHelper "test"; then + bad "a session in a terminal could be signalled by claiming a helper role" + else + ok "a session in a terminal is refused whatever role is claimed" fi - # The name-based net still holds independently of tree position. - assert_protected 52 yes "claude is still protected by name with the subtree rule disabled" - assert_protected 51 yes "tmux is still protected by name with the subtree rule disabled" + # Falsification. All of the above would pass equally against a watchdog that + # polices nothing at all, so each rule is removed in turn and must flip a + # result. + # + # Mutation 1 - the session boundary is removed, the shell test kept. The + # exec'd-away tool call has nothing left and becomes policed; the ordinary one + # is still held by its shell. That asymmetry is the measurement of what each + # rule is doing, and why neither may be dropped as redundant. + local saved_sid=${P_SID[63]} + P_SID[63]=1 + P_SID[64]=1 + P_SID[67]=1 + compute_policed + assert_policed 67 claudeHelper "without the session rule, an exec'd-away tool call is policed" + assert_policed 64 no "while the shell rule still holds the ordinary one" + P_SID[63]=$saved_sid + P_SID[64]=$saved_sid + P_SID[67]=67 + + # Mutation 2 - shells stop being a boundary, sessions kept. The build is still + # spared, because its session differs. + # shellcheck disable=SC2317,SC2329 # invoked indirectly, via the sourced watchdog + is_shell_like() { return 1; } + compute_policed + assert_policed 64 no "with the session rule alone, a running build is still spared" + P_SID[63]=1 + P_SID[64]=1 + compute_policed + assert_policed 64 claudeHelper "and with neither rule it is policed - which is the harm both prevent" + + # Mutation 3 - session roots stop being recognised. Every helper disappears + # from the policed set, which proves the walk is what put them there. + # shellcheck disable=SC2317,SC2329 # invoked indirectly, via the sourced watchdog + is_claude_root() { return 1; } + compute_policed + assert_policed 61 no "with no session roots there are no session helpers" + assert_policed 54 no "including the one in a terminal" } # --------------------------------------------------------------------------- # -# 2b. the operator's runtime is never a VS Code helper +# 2c. the operator's runtime is never a VS Code helper # -# The failure this guards against is the one the whole design exists to prevent, -# arriving through the detection layer instead of the action layer: an agent -# session spawned by an extension is a child of the extension host, is not under -# ptyHost, and would be stamped with an RLIMIT_DATA ceiling and shed at L2/L3 by -# anything that decides "is this a VS Code helper" by asking "is this node". +# An agent session spawned by an extension is a child of the extension host, is +# not under ptyHost, and would be policed by anything that decides "is this a VS +# Code helper" by asking "is this node". # --------------------------------------------------------------------------- # test_operator_runtime_is_never_a_helper() { printf 'the operator runtime is never a VS Code helper\n' write_cgroup "${WORK}/cg" 8589934592 0.00 - local pdir="${WORK}/proc2b" + local pdir="${WORK}/proc2c" build_tree "$pdir" load_watchdog "${WORK}/cg" "$pdir" scan_fixture @@ -478,69 +654,42 @@ test_operator_runtime_is_never_a_helper() { assert_eq other "$ROLE" "an agent session under the extension host is not a helper role" role_of 48 assert_eq other "$ROLE" "nor is an extension task run on the operator's node" - # Same directory in the arguments, opposite classification - argv[0] is the - # only thing separating pid 46 from pid 47. role_of 46 assert_eq extensionHelper "$ROLE" "while the extension's own binary still is one" - assert_protected 47 yes "the agent session is protected" - assert_protected 48 yes "and so is the extension task" - local tier all p - for tier in L2 L3 L4; do - all=" $(candidate_pids "$tier") " - for p in 47 48; do - if [[ $all == *" $p "* ]]; then - bad "${tier} would signal pid ${p}, which runs the operator's node" - else - ok "${tier} never signals pid ${p}, which runs the operator's node" - fi - done - done + assert_protected 47 yes "the agent session is protected by identity" + assert_protected 48 no "the extension task is not - it is excluded by position and by role" + assert_policed 48 no "and so it is not policed" + assert_policed 47 no "nor is the agent session" # Two guards stand between these processes and a signal, and each is mutated # separately so that neither can be credited with the other's work. # - # Mutation 1 - drop the path rule, keep the name guard. The agent session - # survives on its name; the extension task has nothing left and is reachable. - # That asymmetry is the measurement of how much the path rule is doing, and why - # the name guard must not be relied on by itself. + # Mutation 1 - drop the tree-position rule that says "this does not run VS + # Code's binary". The agent session survives on its name, which is identity and + # absolute; the extension task loses its only positional protection and is left + # standing on one thing alone - that role_of keys on argv[0]. # shellcheck disable=SC2317,SC2329 # invoked indirectly, via the sourced watchdog is_vscode_binary() { return 0; } compute_protected - assert_protected 47 yes "without the path rule the agent session still has its name" - assert_protected 48 no "but the extension task has nothing left" - if [[ " $(candidate_pids L4) " == *" 48 "* ]]; then - ok "and L4 would then select it - the path rule is what prevents that" - else - bad "L4 still ignores it, so the path assertion proves nothing" - fi + compute_policed + assert_protected 47 yes "without the position rule the agent session still has its name" + assert_policed 47 no "and is still not policed" + assert_policed 48 no "the extension task is spared by role_of keying on argv[0], not by position" # Mutation 2 - additionally key roles on the joined command line instead of on # argv[0], which is what this file did before a live tree was consulted. The # extension task's arguments name the extension directory, so it is classified - # as a sheddable helper and L2 - the corroborated, everyday tier - picks it up. - # shellcheck disable=SC2317,SC2329 # invoked indirectly, via the sourced watchdog - role_of() { - case " ${P_CMD[$1]:-} " in - *"/.vscode-server/extensions/"*) ROLE=extensionHelper ;; - *) ROLE=other ;; - esac - } - if [[ " $(candidate_pids L2) " == *" 48 "* ]]; then - ok "keying roles on arguments instead of argv[0] makes L2 shed the extension task" - else - bad "the role assertion proves nothing - argv[0] keying is not what excludes it" - fi + # as a sheddable helper and policed. + mutate_role_of_to_cmdline + compute_policed + assert_policed 48 extensionHelper \ + "keying roles on arguments instead of argv[0] is what would police it" + assert_policed 47 no "while the name guard still keeps the session out" } # --------------------------------------------------------------------------- # -# 3a. comm is not a selection criterion, and must never become one again -# -# Pinning the fixture to MainThread would only trade one hardcoded assumption for -# another. What is actually required is that comm does not participate in the -# decision at all, so the same tree is built under three different comm values - -# the real one, the one the fixtures used to assume, and a value nothing has ever -# reported - and the root must come out the same every time. +# 2d. comm is not a selection criterion, and must never become one again # --------------------------------------------------------------------------- # test_comm_is_not_a_criterion() { @@ -554,18 +703,13 @@ test_comm_is_not_a_criterion() { load_watchdog "${WORK}/cg" "$pdir" scan_fixture assert_eq 40 "$SERVER_PID" "comm=${comm}: the server root is still pid 40" - assert_eq 13 "${#SERVER_TREE[@]}" "comm=${comm}: the tree is still complete" - assert_protected 53 yes "comm=${comm}: the hog under ptyHost is still protected" + assert_eq 14 "${#SERVER_TREE[@]}" "comm=${comm}: the tree is still complete" + assert_policed 53 no "comm=${comm}: the hog in a terminal is still not policed" done } # --------------------------------------------------------------------------- # -# 3b. more than one server, and none of them named `node` -# -# --reconnection-grace-time keeps a disconnected server alive for eight hours, so -# two live server trees is an ordinary state, not an exotic one. Electing a -# single root would leave the other tree's ptyHost subtree un-excised, because -# excision only runs inside the tree that was discovered. +# 2e. more than one server, and none of them named `node` # --------------------------------------------------------------------------- # test_two_servers() { @@ -578,31 +722,26 @@ test_two_servers() { scan_fixture assert_eq "40 70" "${SERVER_ROOTS[*]}" "both server roots are discovered" - assert_eq 17 "${#SERVER_TREE[@]}" "the managed tree is the union of both subtrees" - assert_protected 71 yes "the second server's ptyHost is protected" - assert_protected 72 yes "and so is the shell beneath it" - if [[ " $(candidate_pids L4) " == *" 73 "* ]]; then - ok "the second server's fileWatcher is reachable at L4" + assert_eq 18 "${#SERVER_TREE[@]}" "the managed tree is the union of both subtrees" + assert_policed 73 fileWatcher "the second server's file watcher is policed" + assert_policed 72 no "and the shell under its ptyHost is not" + if [[ -n ${NOT_EDITOR[71]:-} ]]; then + ok "the second server's ptyHost subtree is recognised" else bad "the second server's tree is not managed at all" fi } # --------------------------------------------------------------------------- # -# 3c. the guards are precise, and each of them is reachable +# 2f. the guards are precise, and each of them is reachable # # Both historical over-matches were substring matches over the joined command # line, both were found by accident, and both were invisible to a green suite - # the second one protected every process in a fixture harness because the # harness's own directory path contained the string the guard matched on, so two # full runs asserted nothing at all. The decoys below are those exact paths. -# -# Each rule is also asserted to be *reachable*: a guard that no fixture can -# trigger is not being tested, it is only being carried. # --------------------------------------------------------------------------- # -# Three processes that all run VS Code's own node, inside the tree, outside the -# ptyHost subtree - so nothing structural separates them. Only the guards do. add_decoys() { local dir=$1 local vsc="/home/coder/.vscode-server" @@ -618,23 +757,23 @@ add_decoys() { add_proc "$dir" 81 41 MainThread 600000000 "${sdir}/node" \ "${ext}/redhat.vscode-yaml-1.24.0/dist/languageserver.js" \ --config /home/coder/watchdog-live-evidence/memory-watchdog-run2/settings.json - # The case the payload rule genuinely exists for, and the reason it cannot - # simply be deleted: an agent session that an extension started with the - # editor's own interpreter. argv[0] is VS Code's node, so is_vscode_binary - # says "editor"; it is not under ptyHost, so the subtree rule never sees it. + # The case the payload rule genuinely exists for: an agent session that an + # extension started with the editor's own interpreter. argv[0] is VS Code's + # node, so is_vscode_binary says "editor"; it is not under ptyHost, so the + # positional rule never sees it. add_proc "$dir" 82 41 MainThread 900000000 "${sdir}/node" \ "${ext}/anthropic.claude-code-2.1.232/resources/claude-code/cli.js" --ide # Outside the tree, and the reason argv[0] is consulted at all: for a script # with a shebang the kernel sets comm from the *interpreter*, so a launcher on - # PATH called `claude` reports comm=bash. The name the operator knows it by - # survives only in argv[0]. + # PATH called `claude` reports comm=bash. add_proc "$dir" 83 1 bash 500000000 /home/coder/.local/bin/claude --resume + add_proc "$dir" 84 83 python3 800000000 /usr/bin/python3 -m mcp_server_git } test_guards_are_precise() { printf 'the guards are precise\n' write_cgroup "${WORK}/cg" 8589934592 0.00 - local pdir="${WORK}/proc3c" + local pdir="${WORK}/proc2f" build_tree "$pdir" add_decoys "$pdir" load_watchdog "${WORK}/cg" "$pdir" @@ -646,27 +785,17 @@ test_guards_are_precise() { assert_eq payload "${PROTECT_REASON[82]}" "and it is the payload rule that claims it" assert_protected 83 yes "a shebang launcher named claude is protected" assert_eq argv0 "${PROTECT_REASON[83]}" "by argv[0], since its comm is the interpreter" + assert_policed 84 claudeHelper "and its MCP server is policed, so the launcher counts as a session root" - # Falsification: if the two decoys were unreachable for some other reason, the - # assertions above would pass against a watchdog that selects nothing. - local l4 - l4=" $(candidate_pids L4) " - for p in 80 81; do - if [[ $l4 == *" $p "* ]]; then - ok "decoy pid ${p} is genuinely reachable, so its non-protection means something" - else - bad "decoy pid ${p} is unreachable anyway - the assertion proves nothing" - fi - done - if [[ $l4 == *" 82 "* ]]; then - bad "the claude-code payload is reachable at L4" - else - ok "the claude-code payload is not reachable at any tier" - fi + # Falsification: if the two decoys were unpoliced for some other reason, the + # assertions above would pass against a watchdog that polices nothing. + assert_policed 80 fileWatcher "decoy pid 80 is genuinely policed, so its non-protection means something" + assert_policed 81 languageServer "and so is decoy pid 81" + assert_policed 82 no "while the claude-code payload is policed by nothing" # Every guard the code can apply is applied to something here. A rule nothing # exercises is a rule nobody has established the correctness of. - local want got reasons=" " + local want reasons=" " p for p in "${!PROTECT_REASON[@]}"; do reasons+="${PROTECT_REASON[$p]} " done @@ -678,452 +807,390 @@ test_guards_are_precise() { fi done - # And the census says something usable about all of it: a managed tree with - # nothing eligible is the shape of the harness bug, whatever the cause. - got="$(census_line)" - if [[ $got == *"eligible=0" ]]; then - bad "census reports no eligible processes on a healthy tree: ${got}" + # And the watchdog's own kin, which is structural rather than named: the + # harness process is this process, so it must claim itself. + compute_watchdog_kin + if [[ -n ${WATCHDOG_KIN[$$]:-} ]]; then + ok "the watchdog recognises its own pid structurally" else - ok "census reports the tree, the guards and what is left: ${got}" + bad "the watchdog does not recognise itself" fi } # --------------------------------------------------------------------------- # -# 3d. an acting tier never acts silently +# 3. the budgets # -# The bug that hid the second over-match was not the over-match: it was that -# enforce mode logged tier=L3 with no signal line and no refusal line, a state -# that reads as "nothing needed doing". Whatever the guards do, every path out of -# an acting tier must now say what happened. -# --------------------------------------------------------------------------- # - -test_acting_tier_never_acts_silently() { - printf 'an acting tier never acts silently\n' - write_cgroup "${WORK}/cg" 8589934592 0.00 - local pdir="${WORK}/proc3d" - build_tree "$pdir" - rm -rf "${WORK}/state" - load_watchdog "${WORK}/cg" "$pdir" - scan_fixture - - # Reproduce the failure exactly: a guard that swallows the entire tree. - # shellcheck disable=SC2317,SC2329 # invoked indirectly, via the sourced watchdog - is_operator_payload() { return 0; } - compute_protected - : >"${WORK}/state/actions.log" - shed_load L3 1000 - if [[ "$(cat "${WORK}/state/actions.log")" == *"no-candidates tier=L3"* ]]; then - ok "a tier with nothing left to signal says so, with the census" - else - bad "an acting tier signalled nothing and logged nothing - the original bug" - fi - - # And the other silent path: a candidate too small to be worth the disruption. - # shellcheck disable=SC2317,SC2329 # invoked indirectly, via the sourced watchdog - is_operator_payload() { return 1; } - compute_protected - : >"${WORK}/state/actions.log" - WATCHDOG_MIN_SHED_RSS_SAVED=$MIN_SHED_RSS - MIN_SHED_RSS=999999999999 - shed_load L2 1000 - MIN_SHED_RSS=$WATCHDOG_MIN_SHED_RSS_SAVED - if [[ "$(cat "${WORK}/state/actions.log")" == *"no-worthwhile-candidate tier=L2"* ]]; then - ok "declining to shed something too small is logged, not skipped quietly" - else - bad "a tier declined to act and said nothing" - fi -} - -# --------------------------------------------------------------------------- # -# 4. the tier ladder +# The failure this section exists to prevent has happened twice in this design, +# both times the same way: a limit derived from the pod that sat below what the +# process already held at rest. In observe mode that is a log line; in enforce +# mode it is a process that dies on its next allocation, restarts, and dies +# again. So the properties asserted here are about the *relationship* between a +# budget and the measured resting size of the role, not about the arithmetic. # --------------------------------------------------------------------------- # -# step_tier -# -# Called in the current shell on purpose. The debounce counters are state carried -# between samples, and running decide_tier in a command substitution would throw -# them away - which is exactly the bug this ladder had before it was tested. -step_tier() { - decide_tier "$1" "$2" "$3" "$4" "$5" -} - -# assert_tier -assert_tier() { - step_tier "$2" "$3" "$4" "$5" "$6" - assert_eq "$1" "$TIER" "$7" +budgets_at() { + BUDGET=() + derive_budgets "$1" } -test_tiers() { - printf 'tier ladder\n' +test_budgets() { + printf 'budgets\n' write_cgroup "${WORK}/cg" 8589934592 0.00 load_watchdog "${WORK}/cg" "${WORK}/proc2" - local big=6594088184 mid=2500000000 low=1800000000 crit=1000000000 dead=700000000 - - reset_tier_state - assert_tier L0 "$big" 0 0 "$big" 1000 "idle at 6 GiB headroom is L0" - # This is the case a naive memory.current > 85% trigger gets wrong: the real - # pod sits here permanently. - assert_tier L0 "$big" 0 0 "$big" 1000 "and stays L0 while nothing changes" - - reset_tier_state - assert_tier L0 "$mid" 0 0 "$mid" 1000 "first sample below L1 does not act" - assert_tier L0 "$mid" 0 0 "$mid" 1000 "second sample below L1 does not act" - assert_tier L1 "$mid" 0 0 "$mid" 1000 "third consecutive sample is L1" - assert_tier L0 "$big" 0 0 "$big" 1000 "recovery resets the debounce" - - reset_tier_state - step_tier "$low" 0 0 "$low" 1000 - step_tier "$low" 0 0 "$low" 1000 - assert_tier L1 "$low" 0 0 "$low" 1000 \ - "below L2 without PSI or refault corroboration stays at L1" - - reset_tier_state - step_tier "$low" 1500 0 "$low" 1000 - step_tier "$low" 1500 0 "$low" 1000 - assert_tier L2 "$low" 1500 0 "$low" 1000 "below L2 with PSI >= 10 is L2" - - reset_tier_state - step_tier "$low" 0 50000 "$low" 1000 - step_tier "$low" 0 50000 "$low" 1000 - assert_tier L2 "$low" 0 50000 "$low" 1000 "or with a high refault rate" - - reset_tier_state - step_tier "$crit" 0 0 "$crit" 1000 - assert_tier L3 "$crit" 0 0 "$crit" 1000 "L3 needs two samples and no corroboration" - - reset_tier_state - assert_tier L4 "$dead" 0 0 "$dead" 1000 "L4 acts on the first sample" - - # Projection: headroom is fine-ish but falling fast enough to hit L4 inside the - # horizon. Only armed once the ladder has already left L0. - reset_tier_state - step_tier "$mid" 0 0 "$mid" 1000 - step_tier "$mid" 0 0 "$mid" 1000 - assert_tier L3 "$mid" 0 0 100000000 1000 "a 60s projection into L4 escalates to L3" - - reset_tier_state - assert_tier L0 "$big" 0 0 100000000 1000 \ - "but a spike from idle does not - the projection is disarmed at L0" - - # Cooldown holds the acting tiers back; L4 is exempt. - reset_tier_state 1000 - step_tier "$crit" 0 0 "$crit" 1010 - assert_tier L1 "$crit" 0 0 "$crit" 1010 "L3 is suppressed inside the cooldown" - reset_tier_state 1000 - assert_tier L4 "$dead" 0 0 "$dead" 1010 "L4 ignores the cooldown" + # 8 GiB: the pod the roles were measured on. The operator's 512 MiB instinct + # applies unchanged to the helpers; the extension host gets twice its measured + # resting size instead, because 512 MiB is 40 MiB above where it starts. + budgets_at 8589934592 + assert_eq 1073741824 "${BUDGET[extensionHost]}" "8 GiB: the extension host gets 1 GiB" + assert_eq 536870912 "${BUDGET[serverMain]}" "8 GiB: serverMain gets 512 MiB" + assert_eq 268435456 "${BUDGET[fileWatcher]}" "8 GiB: the file watcher gets 256 MiB" + assert_eq 536870912 "${BUDGET[claudeHelper]}" "8 GiB: an MCP server gets 512 MiB" + + # 4 GiB: the pod share would give the extension host 512 MiB, which is above + # its resting 471 MB by 40 MB - i.e. a kill loop. The resting floor overrides + # it. This is the assertion that would have caught both historical defects. + budgets_at 4294967296 + assert_eq 740818944 "${BUDGET[extensionHost]}" \ + "4 GiB: the resting floor overrides the pod share for the extension host" + assert_eq 268435456 "${BUDGET[fileWatcher]}" "4 GiB: the file watcher is unaffected" + + local max role budget resting + for max in 2147483648 4294967296 8589934592 17179869184; do + budgets_at "$max" + for role in "${!BUDGET[@]}"; do + budget=${BUDGET[$role]} + resting=${RESTING_ROLE[$role]:-0} + if ((resting == 0)) || ((budget > resting)); then + ok "memory.max=${max}: the ${role} budget is above its measured resting size" + else + bad "memory.max=${max}: the ${role} budget (${budget}) is at or below resting (${resting}) - a kill loop" + fi + if ((budget < max)); then + ok "memory.max=${max}: the ${role} budget is inside the pod" + else + bad "memory.max=${max}: the ${role} budget (${budget}) is the whole pod - inert" + fi + done + done + + # An explicit budget must win, or a number cannot be tried on a live workspace + # without editing the script under test. + BUDGET=() + WATCHDOG_BUDGET_extensionHost=268435456 derive_budgets 8589934592 + assert_eq 268435456 "${BUDGET[extensionHost]}" "an explicit budget overrides the derivation" + assert_eq 536870912 "${BUDGET[serverMain]}" "while the rest is still derived" + budgets_at 8589934592 + + # Arming is by role and by mode, because the blast radius differs: nothing in + # the helper set is visible to the operator when it restarts, and both members + # of the editor set are. + # MODE is a global of the sourced watchdog, set here to ask each mode what it + # would arm. + # shellcheck disable=SC2034 + MODE=observe + assert_armed claudeHelper no "observe mode arms nothing" + # shellcheck disable=SC2034 + MODE=enforce + assert_armed claudeHelper yes "enforce arms MCP servers" + assert_armed fileWatcher yes "enforce arms the file watcher" + assert_armed extensionHost no "enforce leaves the extension host alone" + # shellcheck disable=SC2034 + MODE=enforce-all + assert_armed extensionHost yes "enforce-all arms the extension host" + assert_armed serverMain yes "enforce-all arms serverMain" + # shellcheck disable=SC2034 + MODE=observe } # --------------------------------------------------------------------------- # -# 4b. the ladder is derived from the pod, and stays sane at every pod size +# 4. dwell: drift is killed, load is not # -# The thresholds used to be absolute bytes chosen for an 8 GiB pod, which made -# the 4 GiB workspace sit permanently on the first rung - the watchdog detected -# that itself and declined to be useful. What is asserted here is not that the -# arithmetic is what it is, but that the properties that make the ladder usable -# hold across every size the workspace is offered at, and past both ends of it. +# The whole point of a dwell requirement is that a helper which balloons while +# doing work and then hands the memory back is load, not drift. A watchdog +# without one would kill tsserver every time it indexed a large project, which is +# the "disruption every fifteen minutes" that gets the thing switched off. # --------------------------------------------------------------------------- # -# ladder_at -> "L4 L3 L2 L1" -ladder_at() { - T_L1="" T_L2="" T_L3="" T_L4="" - CEILING=() - derive_limits "$1" - printf '%s %s %s %s' "$T_L4" "$T_L3" "$T_L2" "$T_L1" -} - -test_derivation() { - printf 'the ladder is derived from the pod\n' +test_dwell() { + printf 'dwell\n' write_cgroup "${WORK}/cg" 8589934592 0.00 - load_watchdog "${WORK}/cg" "${WORK}/proc2" + local pdir="${WORK}/proc4" + build_tree "$pdir" + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" enforce - # 8 GiB is the only size the original absolute numbers were ever reasoned - # about, so the derivation has to land near them or it has thrown away the one - # piece of thinking that existed. 0.80/1.20/2.00/3.20 against 0.75/1.25/2.00/3.00. - assert_eq "858993459 1288490188 2147483647 3435973836" "$(ladder_at 8589934592)" \ - "8 GiB reproduces the hand-tuned ladder it replaces" - # 4 GiB: the case that forced this change. Resting headroom on the live 4 GiB - # workspace was 3.36 GiB, which has to be comfortably clear of L1. - assert_eq "429496729 644245093 1073741822 1717986916" "$(ladder_at 4294967296)" \ - "4 GiB scales the whole ladder down rather than sitting on it" - # 16 GiB: the objection that made the thresholds absolute in the first place - - # a plain percentage would reserve 1.6 GiB and shed with real headroom left. - assert_eq "1073741824 1610612736 2684354560 4294967296" "$(ladder_at 17179869184)" \ - "16 GiB is capped rather than scaled into absurdity" - # 1 GiB: far below anything offered. The floor holds, and the ladder stays - # ordered - the failure to design out is a rung that overtakes another. - local small - small="$(ladder_at 1073741824)" - assert_eq "402653184 603979776 1006632960 1610612736" "$small" \ - "a pod below the floor gets the floor, not a ladder of noise" - - local max l4 l3 l2 l1 - for max in 1073741824 2147483648 4294967296 8589934592 17179869184 34359738368; do - # Not via a command substitution: TOO_SMALL is state the derivation sets, and - # a subshell would discard exactly the answer being asserted on. - T_L1="" T_L2="" T_L3="" T_L4="" - CEILING=() - derive_limits "$max" - l4=$T_L4 l3=$T_L3 l2=$T_L2 l1=$T_L1 - if ((l4 < l3 && l3 < l2 && l2 < l1)); then - ok "memory.max=${max}: the rungs stay strictly ordered" - else - bad "memory.max=${max}: rungs out of order (${l4} ${l3} ${l2} ${l1})" - fi - # Either the pod has room above its own first rung, or the watchdog has - # declared the pod too small to act on. What must never happen is a pod that - # boots inside the shedding tiers while still believing it may shed - that is - # the "kills the editor every fifteen minutes" failure, reached by arithmetic - # rather than by bad luck. - if ((l1 * 2 <= max)); then - ok "memory.max=${max}: the pod has room above L1" - elif ((TOO_SMALL == 1)); then - ok "memory.max=${max}: too small for the ladder, and says so" - else - bad "memory.max=${max}: L1 (${l1}) crowds the limit and enforce is not refused" - fi - done + # The python MCP server at the size it was actually observed at: 1.66 GB + # against a 512 MiB budget. + local t=1000 + sweep_at $t + assert_eq "" "$SIGNALS" "the first sweep over budget signals nothing" + assert_eq "$t" "${OVER_SINCE[61:100]}" "but it starts the dwell clock" - T_L1="" T_L2="" T_L3="" T_L4="" - CEILING=() - derive_limits 1073741824 - assert_eq 1 "$TOO_SMALL" "a 1 GiB pod is marked too small for the ladder" - T_L1="" T_L2="" T_L3="" T_L4="" - CEILING=() - derive_limits 4294967296 - assert_eq 0 "$TOO_SMALL" "a 4 GiB pod is not" - - # Ceilings scale for the same reason and with the same failure modes: a 3 GiB - # extension-host ceiling on a 4 GiB pod is not cautious, it is unreachable. - T_L1="" T_L2="" T_L3="" T_L4="" - CEILING=() - derive_limits 8589934592 - assert_eq 1610612736 "${CEILING[serverMain]}" "8 GiB keeps the reasoned serverMain ceiling" - assert_eq 3221225472 "${CEILING[extensionHost]}" "and the extension-host one" - assert_eq 3758096384 "${CEILING[tsserver]}" "and tsserver's" - T_L1="" T_L2="" T_L3="" T_L4="" - CEILING=() - derive_limits 4294967296 - assert_eq 1610612736 "${CEILING[extensionHost]}" "4 GiB halves the extension-host ceiling" - assert_eq 536870912 "${CEILING[fileWatcher]}" "and floors the small ones rather than shrinking them to nothing" - local role - for role in serverMain extensionHost tsserver languageServer fileWatcher; do - if ((${CEILING[$role]} < 4294967296)); then - ok "4 GiB pod: the ${role} ceiling is inside the pod" - else - bad "4 GiB pod: the ${role} ceiling (${CEILING[$role]}) is the whole pod - inert" - fi - done + t=$((t + DWELL_SECONDS - 60)) + sweep_at $t + assert_eq "" "$SIGNALS" "nor does one just short of the dwell period" - # An explicit environment value must win, or the live demonstration cannot - # force a ceiling to bite without editing the script under test. - T_L1="" T_L2="" T_L3="" T_L4="" - CEILING=() - WATCHDOG_CEILING_extensionHost=268435456 derive_limits 8589934592 - assert_eq 268435456 "${CEILING[extensionHost]}" "an explicit ceiling overrides the derivation" - T_L1="" T_L2="" T_L3="" - T_L4=123456789 # as WATCHDOG_T_L4 would have left it at startup - CEILING=() - derive_limits 8589934592 - assert_eq 123456789 "$T_L4" "and an explicit rung overrides its share of the ladder" - assert_eq 3435973836 "$T_L1" "while the rest of the ladder is still derived" + t=$((t + 60)) + sweep_at $t + assert_eq "TERM:61 " "$SIGNALS" "and at the dwell period it is asked to exit" + assert_eq 1 "${KILLS[claudeHelper]:-0}" "the kill is counted against its role" + + # It ignored SIGTERM. The escalation is deliberately not an in-loop sleep: a + # process shutting down politely gets the whole grace period. + SIGNALS="" + t=$((t + KILL_GRACE - 5)) + sweep_at $t + assert_eq "" "$SIGNALS" "inside the grace period it is left alone" + t=$((t + 10)) + sweep_at $t + assert_eq "KILL:61 " "$SIGNALS" "past it, SIGKILL" + + # Load, not drift: a helper that goes over budget and comes back must survive. + SIGNALS="" + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" enforce + t=2000 + set_pss "$pdir" 44 900000000 # tsserver indexing, over its 768 MiB budget + sweep_at $t + t=$((t + 300)) + sweep_at $t + set_pss "$pdir" 44 400000000 # indexing finished, memory handed back + t=$((t + 60)) + sweep_at $t + assert_eq "" "${OVER_SINCE[44:100]:-}" "coming back under budget clears the dwell clock" + t=$((t + DWELL_SECONDS)) + sweep_at $t + assert_absent "$SIGNALS" "44" "a helper that recovered is never killed for the earlier excursion" + + # Young processes are exempt: something over budget seconds after it started is + # a spike, and spikes are the kernel's problem, not this one's. + SIGNALS="" + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" enforce + set_age "$pdir" 62 30 + set_pss "$pdir" 62 900000000 + t=3000 + sweep_at $t + t=$((t + DWELL_SECONDS + 60)) + sweep_at $t + assert_absent "$SIGNALS" "62" "a process younger than MIN_AGE is not killed for drift" + set_age "$pdir" 62 50000 + set_pss "$pdir" 62 200000000 } # --------------------------------------------------------------------------- # -# 4c. how often it acts +# 4b. the kill loop, and the circuit breaker that stops it # -# Shedding the editor is the right trade against an oom.group kill. Shedding it -# every fifteen minutes is not, because the operator switches the watchdog off -# and then it protects nothing. Correctness and frequency are both requirements. +# This is the failure mode that makes drift policing actively harmful, and it +# arrives looking exactly like the watchdog working: kill the extension host, VS +# Code restarts it, it reloads every extension, it exceeds again, kill. What +# distinguishes a drifting role from a wrongly-budgeted one is only how often the +# same role has to be killed, so that is what the breaker measures. # --------------------------------------------------------------------------- # -# LAST_ACTION_AT and RUNG_FIRED are globals of the sourced watchdog, standing in -# here for the bookkeeping shed_load would have done after a real signal. -# shellcheck disable=SC2034 -test_shedding_is_rate_limited() { - printf 'shedding is rate limited\n' +test_kill_loop_breaker() { + printf 'the kill loop breaker\n' write_cgroup "${WORK}/cg" 8589934592 0.00 - load_watchdog "${WORK}/cg" "${WORK}/proc2" - - local mid=2500000000 low=1800000000 crit=1000000000 big=6594088184 - local t=1000 + local pdir="${WORK}/proc4b" + build_tree "$pdir" + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" enforce + + local t=1000 i + for ((i = 1; i <= LOOP_KILLS; i++)); do + # A fresh incarnation of the same role, as a supervisor restart would give. + printf '%s (python3) S 60 1 1 0 -1 4194560 0 0 0 0 0 0 0 0 20 0 1 0 %s\n' \ + 61 $((i * 1000)) >"${pdir}/61/stat" + set_pss "$pdir" 61 1660000000 + sweep_at $t + t=$((t + DWELL_SECONDS)) + sweep_at $t + t=$((t + 60)) + done + assert_eq "$LOOP_KILLS" "${KILLS[claudeHelper]:-0}" "the role was killed once per incarnation" + if [[ -n ${DISARMED[claudeHelper]:-} ]]; then + ok "and after ${LOOP_KILLS} kills inside the window the role is disarmed" + else + bad "the role kept being killed - there is no circuit breaker" + fi + assert_contains "$(cat "${WORK}/state/actions.log")" "DISARMED role=claudeHelper" \ + "the breaker says so in the log rather than going quiet" + + # And it stays disarmed: the next incarnation is watched, reported, and left + # alone. + SIGNALS="" + printf '%s (python3) S 60 1 1 0 -1 4194560 0 0 0 0 0 0 0 0 20 0 1 0 99000\n' 61 \ + >"${pdir}/61/stat" + set_pss "$pdir" 61 1660000000 + sweep_at $t + t=$((t + DWELL_SECONDS + 60)) + sweep_at $t + assert_eq "" "$SIGNALS" "a disarmed role is not killed again" + assert_contains "$(cat "${WORK}/state/sweep.latest")" "disarmed" \ + "and the sweep records that it is over budget and deliberately spared" + + # Falsification: with a window short enough that the kills fall outside it, the + # same three kills must NOT disarm anything - otherwise the assertion above is + # about the count and not about the rate, and any long-lived workspace would + # eventually disarm itself. + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" enforce + # shellcheck disable=SC2034 # a global of the sourced watchdog + LOOP_WINDOW=60 + t=10000 + for ((i = 1; i <= LOOP_KILLS; i++)); do + record_kill claudeHelper $t + t=$((t + 600)) + done + if [[ -n ${DISARMED[claudeHelper]:-} ]]; then + bad "kills spread over hours still disarm the role - the breaker counts, it does not measure a rate" + else + ok "kills spread beyond the window do not disarm the role" + fi - reset_tier_state - # Reach L2 and let it fire. - step_tier "$low" 1500 0 "$low" $t - step_tier "$low" 1500 0 "$low" $t - assert_tier L2 "$low" 1500 0 "$low" $t "the first L2 of an excursion acts" - RUNG_FIRED[L2]=1 - LAST_ACTION_AT=$t - - # Still bad, well past the settle window: the old cooldown would have let L2 - # fire again every 180s for as long as the pressure lasted. - t=$((t + 600)) - assert_tier L1 "$low" 1500 0 "$low" $t \ - "and no later sample in the same excursion sheds again at L2" - - # Escalation is untouched - this is what the fixed cooldown got wrong, by - # demoting L3 to L1 for three minutes after an L2 shed. - step_tier "$crit" 0 0 "$crit" $t - assert_tier L3 "$crit" 0 0 "$crit" $t "but a worse rung still fires during the same excursion" - RUNG_FIRED[L3]=1 - LAST_ACTION_AT=$t - - # Recovery above L1 is what re-arms, not the clock. - t=$((t + 60)) - assert_tier L0 "$big" 0 0 "$big" $t "recovery above L1 ends the excursion" - step_tier "$low" 1500 0 "$low" $t - step_tier "$low" 1500 0 "$low" $t - assert_tier L2 "$low" 1500 0 "$low" $((t + 100)) "and the next excursion may shed again" - - # The settle window still separates two actions inside one excursion, so the - # ladder sees the effect of a kill before deciding it was not enough. - reset_tier_state - step_tier "$crit" 0 0 "$crit" 2000 - assert_tier L3 "$crit" 0 0 "$crit" 2000 "L3 acts" - LAST_ACTION_AT=2000 - RUNG_FIRED=() - assert_tier L1 "$crit" 0 0 "$crit" 2005 "another action 5s later is held back by the settle window" - - # L4 is exempt from all of it: by then the alternative is the whole pod. - reset_tier_state 2000 - RUNG_FIRED[L4]=1 - assert_tier L4 700000000 0 0 700000000 2001 "L4 ignores both the settle window and the rung flag" - - # An excursion is one continuous dip, not one sample below the line. - reset_tier_state - step_tier "$mid" 0 0 "$mid" 3000 - assert_eq 1 "$EXCURSIONS" "a dip below L1 opens exactly one excursion" - step_tier "$mid" 0 0 "$mid" 3010 - assert_eq 1 "$EXCURSIONS" "and staying down does not open another" - step_tier "$big" 0 0 "$big" 3020 - step_tier "$mid" 0 0 "$mid" 3030 - assert_eq 2 "$EXCURSIONS" "recovering and falling again does" + # The global breaker: a budget wrong in a way that spreads across roles. + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" enforce + t=20000 + for ((i = 1; i <= GLOBAL_LOOP_KILLS; i++)); do + record_kill "role${i}" $t + done + if [[ -n ${DISARMED[claudeHelper]:-} ]]; then + ok "enough kills across unrelated roles disarms everything" + else + bad "no global circuit breaker" + fi } # --------------------------------------------------------------------------- # -# 4d. a ceiling is never a kill order for a healthy process -# -# This is a regression test for a defect found by running the derivation against -# a live 4 GiB workspace rather than against these fixtures. RLIMIT_DATA accounts -# data_vm, not RSS, and on a V8 process the two differ by roughly an order of -# magnitude: the live file watcher held 622 MB of data while resident in 66 MB. -# The derived file-watcher ceiling for that pod was 512 MB - below what the -# process already had - so enforcing it would have killed a perfectly healthy -# file watcher on its next allocation, and again on every restart. -# -# The numbers below are that measurement, not an invention. +# 5. observe mode really is inert, and says what it would have done # --------------------------------------------------------------------------- # -# CEILING_LOGGED is a memo global of the sourced watchdog, cleared here so the -# second half of the test can observe a fresh proposal. -# shellcheck disable=SC2034 -test_ceiling_is_never_below_observed_usage() { - printf 'a ceiling is never below what the process already holds\n' - write_cgroup "${WORK}/cg" 4294967296 0.00 - local pdir="${WORK}/proc4d" +test_observe_mode() { + printf 'observe mode\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + local pdir="${WORK}/proc5b" build_tree "$pdir" - # Live 4 GiB workspace, at rest, VS Code 1.132: pid 918 fileWatcher and pid - # 907 extensionHost. - set_proc_data "$pdir" 42 67000000 637184000 - set_proc_data "$pdir" 41 509220000 1028004000 rm -rf "${WORK}/state" - load_watchdog "${WORK}/cg" "$pdir" - scan_fixture - - # The case is real only if the derived ceiling really is below observed usage. - # Without this the assertion below would pass on a pod where nothing was wrong. - if ((CEILING[fileWatcher] < P_DATA[42])); then - ok "the derived file-watcher ceiling really is below observed usage on a 4 GiB pod" - else - bad "the fixture does not reproduce the condition - the test proves nothing" - fi + load_watchdog "${WORK}/cg" "$pdir" observe - apply_ceilings + local t=1000 + sweep_at $t + t=$((t + DWELL_SECONDS)) + sweep_at $t + assert_eq "" "$SIGNALS" "observe mode signals nothing" local log log="$(cat "${WORK}/state/actions.log")" - local line want - for pid in 42 41; do - line=$(printf '%s\n' "$log" | grep "pid=${pid} " | head -1) - want=${line##*rlimit_data=} - want=${want%% *} - if [[ -n $want ]] && ((want > ${P_DATA[$pid]:-0})); then - ok "pid ${pid}: proposed ceiling ${want} is above its observed ${P_DATA[$pid]:-0} bytes of data" - else - bad "pid ${pid}: proposed ceiling '${want}' would kill it at once (data=${P_DATA[$pid]:-0})" - fi - done - - # And the growth allowance is the reserve, so the ceiling means "may grow by - # this much", not "may be this big". - line=$(printf '%s\n' "$log" | grep "pid=42 " | head -1) - want=${line##*rlimit_data=} - want=${want%% *} - assert_eq "$((P_DATA[42] + 2 * C_RESERVE))" "$want" \ - "the ceiling is observed usage plus two critical reserves" - - # A ceiling that could only be set above memory.max bounds nothing. Saying so - # is better than setting it and looking protected. - set_proc_data "$pdir" 42 67000000 4000000000 - read_rss 42 - : >"${WORK}/state/actions.log" - CEILING_LOGGED=() - apply_ceilings - if [[ "$(cat "${WORK}/state/actions.log")" == *"no-ceiling pid=42"* ]]; then - ok "a process already too large to cap is reported, not silently capped" - else - bad "a process too large to cap was handled silently" - fi + assert_contains "$log" "[observe] would-kill pid=61" \ + "but it logs the kill it would have made" + assert_contains "$log" "armed=no" "and records that the role was not armed" + assert_eq 0 "${KILLS[claudeHelper]:-0}" "a kill it did not make is not counted" + assert_contains "$(cat "${WORK}/state/sweep.latest")" "would-kill" \ + "the sweep row says would-kill rather than killed" + + # Enforce mode arms helpers but not the editor, so the extension host is + # reported and spared in exactly the same way. + SIGNALS="" + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" enforce + set_pss "$pdir" 41 2000000000 + t=5000 + sweep_at $t + t=$((t + DWELL_SECONDS)) + sweep_at $t + assert_absent "$SIGNALS" "TERM:41" "enforce mode does not kill the extension host" + assert_contains "$(cat "${WORK}/state/actions.log")" "pid=41" \ + "but it does report it as over budget" + + SIGNALS="" + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" enforce-all + t=8000 + sweep_at $t + t=$((t + DWELL_SECONDS)) + sweep_at $t + assert_contains "$SIGNALS" "TERM:41" "enforce-all does kill it" + set_pss "$pdir" 41 500000000 } # --------------------------------------------------------------------------- # -# 4e. the sample interval is chosen by rate, not by tier +# 6. the sweep log # -# Measured, not supposed. A runaway at the rate seen in production took the test -# workspace from idle to OOMKilled in 43 seconds while the watchdog ran in -# enforce mode, never left L0 and logged nothing: at a 10-second idle interval it -# had four samples in which to satisfy a three-sample debounce and climb three -# rungs. Tier cannot be the input to the interval, because tier is the lagging -# indicator of the very thing being raced. +# Every post-mortem in this investigation was unanswerable because the watchdog +# computed this table on every cycle and threw it away. The log is therefore a +# deliverable in its own right, not decoration on the killing. # --------------------------------------------------------------------------- # -# shellcheck disable=SC2034 # TIME_TO_LIMIT and PREV_TIER are the sourced globals -test_interval_is_chosen_by_rate() { - printf 'the sample interval is chosen by rate\n' - write_cgroup "${WORK}/cg" 4294967296 0.00 - load_watchdog "${WORK}/cg" "${WORK}/proc2" +test_sweep_log() { + printf 'the sweep log\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + local pdir="${WORK}/proc6" + build_tree "$pdir" + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" observe + sweep_at 1000 - TIME_TO_LIMIT=0 - PREV_TIER=L0 - assert_eq 10 "$(next_interval)" "an idle pod that is not growing polls slowly" - - PREV_TIER=L1 - assert_eq 2 "$(next_interval)" "a pod already on the ladder polls faster" - - # 3.96 GiB of headroom disappearing at 91 MB/s - the reproduced production - # rate - is 43 seconds from the limit while still reading L0. - TIME_TO_LIMIT=43 - PREV_TIER=L0 - assert_eq 1 "$(next_interval)" \ - "but a rate that reaches the limit inside the horizon overrides L0 entirely" - - # The case that must not become chatty: growth so slow it will never matter. - TIME_TO_LIMIT=3600 - PREV_TIER=L0 - assert_eq 10 "$(next_interval)" "slow growth does not spin the loop up" - - # With a 1-second interval the debounce that could not complete in the live run - # completes with time to spare: three samples is three seconds, against the 43 - # the event took. - if ((DEBOUNCE_L1 * 1 < 43 && DEBOUNCE_L3 * 1 < 43)); then - ok "at the fast interval the debounces fit inside the observed event" + local log + log="$(cat "${WORK}/state/sweep.log")" + assert_contains "$log" "pss_kb" "the log carries a header naming its columns" + assert_contains "$log" " extensionHost " "a policed process appears with its role" + assert_contains "$log" " claudeHelper " "so does an MCP server" + assert_contains "$log" "homelab_mcp.server" "with the identity that survives a restart" + assert_contains "$log" " TOTAL " "and each sweep ends with a totals row" + assert_contains "$log" "policed=" "which carries the census" + + # The processes it does NOT manage are the ones every OOM in this investigation + # actually involved, so they are recorded too - as unmanaged, not as absent. + assert_contains "$log" " unmanaged " "large unmanaged processes are recorded" + local line + line=$(printf '%s\n' "$log" | grep -m1 ' 60 ') + assert_contains "$line" "unmanaged" "including the agent session itself" + + # Small processes are not, or a sweep is a hundred rows of shells. + if printf '%s\n' "$log" | grep -q ' 31 '; then + bad "a 3 MB shell was written to the sweep log" else - bad "the debounces still cannot complete inside a 43-second event" + ok "processes below the log floor are omitted" fi + + # Secrets in argv are redacted at the point of writing, because this file is + # the one thing here that might later be shipped somewhere. + assert_absent "$log" "remotessh" "a connection token never reaches the log" + assert_eq "node --connection-token= --start" \ + "$(redact "node --connection-token=remotessh --start")" \ + "the value is replaced in place, so the shape is still visible" + assert_eq "python3 --api-key=" "$(redact "python3 --api-key=sk-live-1234")" \ + "and the same for a key on an MCP server's command line" + + # The summary file is what the operator reads first, so its numbers have to be + # the real ones. Asserted because they were not: `$((BUDGET[role]))` reads the + # *key* "role" inside an arithmetic context and every budget printed as 0M, + # which looked like a watchdog with no budgets at all. + publish_summary 1000 + local summary + summary="$(cat "${WORK}/state/summary")" + assert_contains "$summary" "claudeHelper=512M" "the summary prints real budgets" + assert_contains "$summary" "extensionHost=1024M" "including the derived ones" + assert_contains "$summary" "policed=" "and the size of the policed set" + + # The visibility check: a watchdog that manages nothing looks exactly like a + # watchdog with nothing to do, and only the log can tell them apart. + rm -rf "${WORK}/state" + local empty="${WORK}/proc6b" + mkdir -p "$empty" + write_uptime "$empty" + add_proc "$empty" 1 0 coder 14208 ./coder agent + load_watchdog "${WORK}/cg" "$empty" observe + SWEEPS=$VISIBILITY_WARMUP + sweep_at 1000 + check_visibility + assert_contains "$(cat "${WORK}/state/actions.log")" "WARNING no process has been policed" \ + "an empty policed set is reported rather than passing for health" } # --------------------------------------------------------------------------- # -# 4f. a stale pidfile does not disarm the watchdog forever +# 7. a stale pidfile does not disarm the watchdog forever # # Found on the live test workspace, not here. The pod was OOM-killed, the # watchdog died by SIGKILL without running its EXIT trap, and its pidfile @@ -1135,12 +1202,11 @@ test_interval_is_chosen_by_rate() { test_singleton_survives_a_hard_kill() { printf 'a stale pidfile does not disarm the watchdog\n' write_cgroup "${WORK}/cg" 4294967296 0.00 - local pdir="${WORK}/proc4f" + local pdir="${WORK}/proc7" build_tree "$pdir" rm -rf "${WORK}/state" load_watchdog "${WORK}/cg" "$pdir" - # A pid from the previous container that no longer exists at all. printf '99999\n' >"${WORK}/state/watchdog.pid" if acquire_singleton; then ok "a pidfile naming a dead process is taken over" @@ -1148,8 +1214,6 @@ test_singleton_survives_a_hard_kill() { bad "a dead process's pidfile locks the watchdog out" fi - # A pid that does exist in this container but is something else entirely - - # the recycled-pid case, which is the normal case after a restart. printf '44\n' >"${WORK}/state/watchdog.pid" if acquire_singleton; then ok "a pidfile naming an unrelated live process is taken over" @@ -1157,7 +1221,6 @@ test_singleton_survives_a_hard_kill() { bad "an unrelated process holding a recycled pid locks the watchdog out" fi - # An empty pidfile - what a truncated or half-written file looks like. : >"${WORK}/state/watchdog.pid" if acquire_singleton; then ok "an empty pidfile is taken over" @@ -1170,7 +1233,7 @@ test_singleton_survives_a_hard_kill() { # watchdog with no singleton guard at all. local d="${pdir}/4242" mkdir -p "$d" - printf '4242 (bash) S 1 0 0 0 -1 4194560 0 0 0 0 0 0 0 0 20 0 1 0 0\n' >"${d}/stat" + printf '4242 (bash) S 1 1 1 0 -1 4194560 0 0 0 0 0 0 0 0 20 0 1 0 100\n' >"${d}/stat" printf '/bin/bash\0%s\0' "${SELF_DIR}/script-memory-watchdog.sh" >"${d}/cmdline" printf '4242\n' >"${WORK}/state/watchdog.pid" if acquire_singleton; then @@ -1179,7 +1242,6 @@ test_singleton_survives_a_hard_kill() { ok "a live process running this same script does hold the lock" fi - # Releasing must not steal a pidfile owned by someone else. printf '4242\n' >"${WORK}/state/watchdog.pid" release_singleton if [[ -s "${WORK}/state/watchdog.pid" ]]; then @@ -1190,39 +1252,36 @@ test_singleton_survives_a_hard_kill() { } # --------------------------------------------------------------------------- # -# 5. observe mode really is inert +# 8. pid recycling +# +# Every piece of state carried between sweeps is keyed on pid:starttime. Without +# the starttime a recycled pid inherits the dwell clock of whatever held that +# number before it, and can be killed for someone else's drift. # --------------------------------------------------------------------------- # -test_observe_mode_is_inert() { - printf 'observe mode\n' +test_pid_recycling() { + printf 'pid recycling\n' write_cgroup "${WORK}/cg" 8589934592 0.00 - local pdir="${WORK}/proc4" + local pdir="${WORK}/proc8" build_tree "$pdir" rm -rf "${WORK}/state" - load_watchdog "${WORK}/cg" "$pdir" - scan_fixture - - apply_ceilings - local limits_now - limits_now="$(cat "${pdir}/41/limits")" - if [[ $limits_now == *"unlimited unlimited"* ]]; then - ok "observe mode changed no RLIMIT_DATA" - else - bad "observe mode wrote a limit" - fi - if [[ -s "${WORK}/state/actions.log" ]] && - [[ "$(cat "${WORK}/state/actions.log")" == *"[observe] ceiling"* ]]; then - ok "observe mode logged the ceilings it would have set" - else - bad "observe mode logged nothing" - fi + load_watchdog "${WORK}/cg" "$pdir" enforce - # ptyHost must never appear in the ceiling log, at any tier, in any mode: a - # soft RLIMIT_DATA there is inherited by every terminal the operator opens. - if [[ "$(cat "${WORK}/state/actions.log")" == *"pid=43"* ]]; then - bad "a ceiling was proposed for the ptyHost fork" + local t=1000 + sweep_at $t + assert_eq "$t" "${OVER_SINCE[61:100]}" "the dwell clock is keyed on pid and starttime" + + # Same pid, different process. The starttime changes, so the key changes. + printf '61 (python3) S 60 1 1 0 -1 4194560 0 0 0 0 0 0 0 0 20 0 1 0 55500\n' \ + >"${pdir}/61/stat" + t=$((t + DWELL_SECONDS)) + sweep_at $t + assert_eq "" "$SIGNALS" "a recycled pid does not inherit the previous dwell" + assert_eq "$t" "${OVER_SINCE[61:55500]}" "it starts its own clock" + if [[ -n ${OVER_SINCE[61:100]:-} ]]; then + bad "the dead process's dwell entry was left behind to grow forever" else - ok "no ceiling is ever proposed for the ptyHost fork" + ok "and the dead process's entry is pruned" fi } @@ -1231,19 +1290,18 @@ test_observe_mode_is_inert() { main() { test_measurement test_selection + test_claude_helpers test_operator_runtime_is_never_a_helper - test_selection_is_falsifiable - test_guards_are_precise - test_acting_tier_never_acts_silently test_comm_is_not_a_criterion test_two_servers - test_tiers - test_derivation - test_shedding_is_rate_limited - test_ceiling_is_never_below_observed_usage - test_interval_is_chosen_by_rate + test_guards_are_precise + test_budgets + test_dwell + test_kill_loop_breaker + test_observe_mode + test_sweep_log test_singleton_survives_a_hard_kill - test_observe_mode_is_inert + test_pid_recycling printf '\n%d passed, %d failed\n' "$PASS" "$FAIL" ((FAIL == 0)) } diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh index b9369798..c7137b6b 100644 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh @@ -1,38 +1,62 @@ #!/bin/bash # -# Memory watchdog for the workspace pod. +# Memory watchdog for the workspace pod: it bounds the *standing population* of +# restartable helper processes. # -# Why this exists: the pod's cgroup has memory.oom.group=1, so a cgroup OOM kills -# every process in the container together - the IDE, every tmux session and every -# long-running agent. /sys/fs/cgroup is mounted read-only with an empty -# cgroup.subtree_control under a private cgroup namespace, and the workspace runs -# as uid 10001 with no capabilities, so neither memory.high nor a child cgroup is -# reachable without privileged:true. See DESIGN.md. The only remaining strategy -# is to never reach memory.max, which is what this does from userspace. +# What it is for, and what it deliberately is not for. # -# Dependencies are deliberately tiny: bash 4.4+, /proc, /sys/fs/cgroup, -# /usr/bin/sleep, coreutils mv/rm/mkdir, and - in enforce mode only - -# /usr/bin/prlimit. Measurement and process enumeration use bash builtins, so a -# scan forks nothing at all; at a 2-second interval under pressure that matters. +# It is not an OOM preventer. The earlier version of this script was, and the +# premise did not survive measurement. Every memcg OOM recorded for this +# workspace was a spike - 70 to 220 MB/s, idle to dead inside a minute - and the +# victims named in the kernel log were agent sessions and node, never a VS Code +# process. A poll loop cannot win that race: a generic biggest-RSS killer only +# beats the kernel at a 0.3s interval, loses at 0.5s, and while +# memory.oom.group=1 it is killed by the very event it lost to. The graded +# shedding ladder that used to live here climbed correctly during a live spike +# and then logged `no-candidates`, because the runaway was not in the tree it +# managed. That ladder has been removed rather than tuned. +# +# What a poll loop is genuinely good at is growth measured in MB per *minute*: +# the slow drift of long-lived helpers - the extension host, language servers, +# the file watcher, and the MCP servers and other helpers an agent session +# spawns. That drift is real and was being policed by hand (repeatedly killing +# VS Code to save agent sessions); a python MCP server holding 1.66 GB was +# observed during one of the kills. Every process this script may signal is +# restarted by its own supervisor - VS Code respawns its forks and language +# servers, an agent session respawns its MCP servers - so a wrong kill costs a +# reload, not a session. That asymmetry is what licenses being aggressive. # -# awk, flock, python3, ps and pgrep do all exist in the base image - an earlier -# version of this comment claimed otherwise and was wrong. Not using them is a -# choice (no forks per scan, one language to review), not a constraint. The one -# real constraint is that the operator's PATH is shadowed by brew, so anything -# invoked here is called by absolute /usr/bin/... path and never by name. +# The goal is therefore runway, not rescue: keep the resting population small so +# that when a spike does arrive it starts from as much free memory as possible, +# and stop the operator having to do it by hand. +# +# Dependencies are deliberately tiny: bash 4.4+, /proc, /sys/fs/cgroup, +# /usr/bin/sleep, coreutils mv/rm/mkdir. Measurement and process enumeration use +# bash builtins, so a sweep forks nothing at all. The operator's PATH is shadowed +# by brew, so anything invoked here is called by absolute /usr/bin/... path and +# never by name. # -# Modes: -# observe (default) - measure, publish headroom, log what it *would* have done. -# Sets no limits and sends no signals. -# enforce - additionally refresh RLIMIT_DATA ceilings and shed load. +# Modes (the memory_watchdog_mode template parameter; the script itself falls +# back to observe when the value is anything else): +# observe - measure, publish, and log the kill it *would* have made. +# Signals nothing. +# enforce - additionally kill drifted *helpers*: language servers, the +# file watcher, native extension helpers, and helpers spawned by +# an agent session (MCP servers). This is the template default, +# because every one of them restarts without the operator +# noticing. +# enforce-all - additionally kill the extension host and the server main +# process. These are user-visible when they restart, and they +# are the two roles a wrong budget would put in a kill loop, so +# they are armed separately and on purpose. # # Test seams, exercised by script-memory-watchdog-test.sh: # WATCHDOG_CGROUP_DIR WATCHDOG_PROC_DIR WATCHDOG_STATE_DIR # WATCHDOG_MODE WATCHDOG_ONESHOT WATCHDOG_NOW WATCHDOG_SOURCE_ONLY # # Deliberately NOT using `set -e`: this is a supervisor with no supervisor of its -# own. A read that fails because a /proc entry vanished mid-scan must skip that -# entry, not take the watchdog down and leave the pod unprotected. +# own. A read that fails because a /proc entry vanished mid-sweep must skip that +# entry, not take the watchdog down and leave the pod unpoliced. set -uo pipefail # --------------------------------------------------------------------------- # @@ -46,242 +70,201 @@ MODE="${WATCHDOG_MODE:-observe}" ONESHOT="${WATCHDOG_ONESHOT:-0}" GIB=1073741824 +MIB=1048576 PAGE_SIZE=4096 - -# Sampling. The interval shortens under pressure so the ladder can outrun a -# process that allocates a gigabyte in a few seconds. -# -# INTERVAL_FAST and FAST_HORIZON exist because the original two-speed scheme was -# measured against a real event and lost. Reproduced on the test workspace, a -# runaway growing at the rate seen in production took the pod from idle to -# OOMKilled in 43 seconds; the watchdog was running in enforce mode throughout, -# never left L0, and logged no action at all. At a 10-second idle interval it -# got four samples, and a three-sample debounce cannot complete inside four -# samples that also have to cross three rungs. -# -# The fix is to key the interval on the measured rate rather than on the tier: -# once dU/dt implies the limit is reachable inside FAST_HORIZON, sample every -# second, whatever tier the ladder currently believes it is in. Tier is a lagging -# indicator of exactly the thing being raced. This changes only how often the -# rules are evaluated, not what they decide - the corroboration and debounce that -# keep L2 honest are untouched. -INTERVAL_IDLE="${WATCHDOG_INTERVAL_IDLE:-10}" -INTERVAL_BUSY="${WATCHDOG_INTERVAL_BUSY:-2}" -INTERVAL_FAST="${WATCHDOG_INTERVAL_FAST:-1}" -FAST_HORIZON="${WATCHDOG_FAST_HORIZON:-120}" # seconds to memory.max -CALIBRATION_EVERY="${WATCHDOG_CALIBRATION_EVERY:-6}" # cycles between CSV rows - -# Tier thresholds, in absolute bytes of headroom H, derived from the pod's own -# memory.max by derive_limits() below. Set any of these in the environment to -# override the derivation entirely; empty means "derive it". -T_L1="${WATCHDOG_T_L1:-}" -T_L2="${WATCHDOG_T_L2:-}" -T_L3="${WATCHDOG_T_L3:-}" -T_L4="${WATCHDOG_T_L4:-}" - -# The one number the ladder is derived from: the critical reserve C, the amount -# of headroom below which the next allocation burst can reach memory.max before -# the next sample can react. T_L4 is C, and the rungs above it are fixed -# multiples of it. +USER_HZ=100 + +# Two cadences, one loop. The cgroup sample is a handful of file reads and is +# what keeps the telemetry legible across a fast event; the sweep walks every +# process and reads smaps_rollup, and drives every decision. Drift is measured in +# MB per minute, so a decision cadence of a minute is not a compromise - it is +# the correct resolution for the thing being policed. +SAMPLE_INTERVAL="${WATCHDOG_SAMPLE_INTERVAL:-10}" +SWEEP_EVERY="${WATCHDOG_SWEEP_EVERY:-6}" # samples per sweep => 60s + +# A process must be over its budget continuously for this long before anything +# happens to it. This is the single most important number in the file, and it is +# what separates drift policing from the spike chasing that did not work: a +# language server that balloons while indexing and then hands the memory back is +# load, not drift, and must survive. Expressed in seconds so that changing the +# sweep cadence cannot silently change the policy. +DWELL_SECONDS="${WATCHDOG_DWELL_SECONDS:-600}" + +# Never signal something that has not been alive long enough to have finished +# starting up. A process that is over budget within seconds of its own start is +# a spike, which is the kernel's problem, not this one's. +MIN_AGE="${WATCHDOG_MIN_AGE:-300}" + +# SIGTERM, then SIGKILL on a later sweep if it is still there and still over. +# There is no in-loop sleep: the grace period is measured across sweeps, so a +# process that is shutting down politely is never hurried. +KILL_GRACE="${WATCHDOG_KILL_GRACE:-30}" + +# The circuit breaker. This is the counterweight to being aggressive, and it +# exists because the failure mode of drift policing is not a wrong kill - it is a +# kill *loop*: kill the extension host, VS Code restarts it, it reloads every +# extension, it exceeds again, kill. That loop arrives looking exactly like the +# watchdog working, and it is the thing that gets a watchdog switched off. # -# Why this is a clamped fraction rather than an absolute constant, having been an -# absolute constant first. The original argument for absolute bytes was that the -# page cache a workload needs for forward progress is a property of the workload, -# not of the container's limit - and that is true, but it only settles what the -# thresholds *mean*, not what values are available. Headroom is bounded above by -# memory.max, so on a pod small enough that the fixed floor exceeds the range the -# pod ever has, an absolute ladder does not become conservative, it becomes -# permanently tripped and therefore inert: the 4 GiB workspace rests at H = 3.4 -# GiB against a 3.0 GiB L1, which is one editor window away from sitting at L1 -# for the rest of the pod's life. So the fraction scales the ladder to the pod, -# while: +# So a role that has to be killed LOOP_KILLS times inside LOOP_WINDOW is not a +# drifting role, it is a role whose budget is wrong for this workload. The +# watchdog disarms itself for that role, says so, and leaves the number to a +# human. It never widens its own budget: a mechanism that quietly raises the +# limit it is enforcing is a mechanism that stops enforcing anything. +LOOP_WINDOW="${WATCHDOG_LOOP_WINDOW:-3600}" +LOOP_KILLS="${WATCHDOG_LOOP_KILLS:-3}" +# And the same idea across all roles at once, in case a budget is wrong in a way +# that spreads: too many kills in one window disarms everything. +GLOBAL_LOOP_KILLS="${WATCHDOG_GLOBAL_LOOP_KILLS:-8}" + +# Budgets, in bytes of PSS, by role. # -# - the FLOOR expresses reaction time, which really is size-independent. It is -# what the sample interval times a plausible allocation rate costs, and no -# pod is too small to need it. -# - the CAP stops a percentage from scaling into absurdity on a large pod, -# which was the correct half of the original objection: a fraction that gave -# 16 GiB a 4 GiB "critical" reserve would shed with plenty of genuine -# headroom left. +# Why PSS and not VmData. RLIMIT_DATA accounts VmData, which is why the previous +# design measured it; that mechanism is gone, and for deciding "is this process +# holding too much memory" VmData is the wrong quantity by roughly an order of +# magnitude on a V8 process (measured: file watcher 66 MB resident against 622 MB +# of data). PSS is what the pod actually pays: it counts shared pages once, +# divided among the processes sharing them, which matters here because a dozen +# node processes share one binary's file-backed pages. RSS would charge each of +# them the full share and make every helper look bigger than it is. # -# At 8 GiB this reproduces the hand-tuned ladder it replaces (0.80/1.20/2.00/3.20 -# against 0.75/1.25/2.00/3.00), which is the only calibration point that ever -# existed; at 4 GiB it gives 0.41/0.61/1.02/1.64, leaving the measured 3.4 GiB -# resting headroom two thirds of the pod clear of the first rung. -C_FRACTION_NUM="${WATCHDOG_C_NUM:-1}" -C_FRACTION_DEN="${WATCHDOG_C_DEN:-10}" -C_FLOOR="${WATCHDOG_C_FLOOR:-402653184}" # 384 MiB -C_CAP="${WATCHDOG_C_CAP:-1073741824}" # 1.00 GiB - -# Corroboration is required at L2 only. At L2 we are "at the limit"; PSI and the -# refault rate are what separate "at the limit and fine" - the normal resting -# state of this pod - from "at the limit and dying". By L3/L4 there is no time -# left to wait for a second opinion. -T_PSI_CENTI="${WATCHDOG_T_PSI_CENTI:-1000}" # memory.pressure full avg10 >= 10.00 -T_REFAULT_RATE="${WATCHDOG_T_REFAULT_RATE:-20000}" - -DEBOUNCE_L1="${WATCHDOG_DEBOUNCE_L1:-3}" -DEBOUNCE_L2="${WATCHDOG_DEBOUNCE_L2:-3}" -DEBOUNCE_L3="${WATCHDOG_DEBOUNCE_L3:-2}" -PROJECTION_HORIZON="${WATCHDOG_PROJECTION_HORIZON:-60}" # seconds - -# How rarely it acts is part of what "working" means here, not a refinement of -# it. Shedding the editor is the right trade against an oom.group kill that takes -# every tmux session and every agent with it - but an editor that dies every -# fifteen minutes gets the watchdog switched off, and a watchdog that is switched -# off protects nothing. So the ladder is rate-limited by construction rather than -# by a timer alone: +# Why not one uniform number. The operator's instinct - "none of these should +# ever exceed 512 MB, maybe 256" - is right about the helpers and wrong about the +# extension host, and the difference is measured, not argued: on a *fresh* tree +# the extension host is already 471 MB PSS, serverMain 160 MB, ptyHost 36 MB and +# the file watcher 34 MB. A uniform 512 MB budget puts the extension host 40 MB +# from its resting size before it has done any work, and 256 MB is below its +# floor outright - which is the same defect as the ceiling this file used to +# derive that sat below what an idle file watcher already held. So each role gets +# the operator's number where it is right, and a number anchored on its own +# measured resting size where it is not. # -# - SETTLE is the minimum gap between any two actions. It exists so the ladder -# can see the effect of a kill before deciding it was not enough. It does NOT -# hold back a higher rung indefinitely, which the fixed 180s cooldown it -# replaces did: that cooldown demoted L3 to L1 for three minutes after an L2 -# shed, so a fast-growing extension host could not be stopped during exactly -# the window when it most needed stopping. -# -# - Each rung then fires at most once per excursion, and an excursion only ends -# when headroom recovers above L1. Escalation is unaffected - L2, then L3, -# then L4 all remain available as things get worse - but a pod that is simply -# too small for its workload sheds one helper and one extension host and then -# stops, instead of shedding one every SETTLE seconds forever. If that is not -# enough, the answer is a bigger pod, and repeatedly killing the editor is a -# worse way of finding that out than the log line that says so. -SETTLE="${WATCHDOG_SETTLE:-30}" - -# A shed has to be worth its disruption. Killing a 40 MiB file watcher frees -# nothing, restarts a component the operator can notice, and burns the rung that -# would otherwise have been available later in the same excursion. -MIN_SHED_RSS="${WATCHDOG_MIN_SHED_RSS:-134217728}" # 128 MiB - -# Soft RLIMIT_DATA ceilings by role, as sixteenths of memory.max. Hard limits are -# never touched, so an inheriting shell restores itself with `ulimit -d -# unlimited`. -# -# These scale for the same reason the ladder does, and it matters more here: a -# 3 GiB extension-host ceiling on a 4 GiB pod is not conservative, it is inert - -# the pod dies first. The numerators are the hand-picked 8 GiB values expressed -# against that pod's limit (1.5, 3.0, 3.5, 1.0, 1.0 GiB), so an 8 GiB workspace -# gets exactly the ceilings that were reasoned about, and every other size gets -# the same shape. -# -# The floor is what keeps the scaling from turning into a different failure: a -# ceiling below what a role legitimately needs makes it die doing ordinary work, -# which is worse than no ceiling because it is constant rather than occasional. -# The cap keeps a large pod from being handed a ceiling so high that nothing -# could ever reach it. -# -# Every role here is a V8 process, and that is what makes a ceiling a reasonable -# thing to set: hitting it makes mmap return ENOMEM, V8 raises its own fatal heap -# OOM, and the editor offers "Restart Extension Host" or silently respawns the -# language server. `extensionHelper` is deliberately absent - see role_of(). -declare -gA CEILING_SIXTEENTHS=( - [serverMain]=3 - [extensionHost]=6 - [tsserver]=7 - [languageServer]=2 - [fileWatcher]=2 +# The reference column is that measurement. It is not a budget; it is the floor +# below which a budget is a kill order rather than a limit. +declare -gA BUDGET_ROLE=( + [extensionHost]=1073741824 # 1024 MiB, against 471 MB resting + [serverMain]=536870912 # 512 MiB, against 160 MB resting + [tsserver]=805306368 # 768 MiB - legitimately large on a big project + [languageServer]=268435456 # 256 MiB - yaml/json/terraform LS rest near 100 + [fileWatcher]=268435456 # 256 MiB, against 34 MB resting + [extensionHelper]=536870912 # 512 MiB - terraform-ls, gopls and the like + [claudeHelper]=536870912 # 512 MiB - MCP servers. The operator's number, + # applied exactly where his instinct is right: a + # helper that holds 1.66 GB is not doing its job + # better than one that holds 300 MB. ) -CEILING_FLOOR="${WATCHDOG_CEILING_FLOOR:-536870912}" # 512 MiB -CEILING_CAP="${WATCHDOG_CEILING_CAP:-4294967296}" # 4.00 GiB - -# Filled by derive_limits(). A role may also be pinned outright from the -# environment - WATCHDOG_CEILING_extensionHost=... - which is how the live -# demonstration forces a ceiling to bite without editing the script. -declare -gA CEILING=() - -# Roles L2 is allowed to shed. Each is restarted transparently or on demand by -# the editor, and none of them holds unsaved user state. -L2_ROLES=" tsserver languageServer fileWatcher extensionHelper " +declare -gA RESTING_ROLE=( + [extensionHost]=493879296 # 471 MB + [serverMain]=167772160 # 160 MB + [fileWatcher]=35651584 # 34 MB +) +# A budget is never allowed below the role's measured resting size times this, +# whatever the pod arithmetic says. This is the guard against the class of error +# that has already bitten this design twice. +RESTING_FACTOR_NUM="${WATCHDOG_RESTING_NUM:-3}" +RESTING_FACTOR_DEN="${WATCHDOG_RESTING_DEN:-2}" +# No single helper may be budgeted more than this share of the pod. The workspace +# is offered at 4 and 8 GiB; without it, a budget reasoned about for the larger +# pod lets one helper own a quarter of the smaller one. +POD_SHARE_DEN="${WATCHDOG_POD_SHARE_DEN:-8}" + +# Which roles each mode may signal. The split is by blast radius, not by size: +# nothing in the helper set is visible to the operator when it restarts, and both +# members of the editor set are. +HELPER_ROLES=" tsserver languageServer fileWatcher extensionHelper claudeHelper " +EDITOR_ROLES=" extensionHost serverMain " + +# Filled by derive_budgets(). +declare -gA BUDGET=() + +# Only processes at or above this are written to the sweep log, plus every +# process the watchdog is policing regardless of size. The floor keeps a sweep +# from being a hundred rows of 2 MB shells while still recording anything that +# could plausibly matter later. +SWEEP_LOG_FLOOR="${WATCHDOG_SWEEP_LOG_FLOOR:-33554432}" # 32 MiB MAX_LOG_LINES="${WATCHDOG_MAX_LOG_LINES:-20000}" MAX_CSV_LINES="${WATCHDOG_MAX_CSV_LINES:-50000}" - -# Scan state. Declared at file scope, not inside main(), so that sourcing the -# script with WATCHDOG_SOURCE_ONLY=1 gives the test harness correctly-typed -# globals without having to restate them. -declare -gA P_COMM=() P_CMD=() P_ARGV0=() P_PPID=() P_RSS=() P_DATA=() CHILDREN=() -declare -gA SERVER_TREE=() PROTECTED=() PROTECT_REASON=() WATCHDOG_KIN=() -declare -ga PIDS=() CANDIDATES=() SERVER_ROOTS=() +MAX_SWEEP_LINES="${WATCHDOG_MAX_SWEEP_LINES:-200000}" + +# Pressure labels. These drive nothing at all - no action is keyed on them - and +# exist so the telemetry says whether the pod was comfortable at the time. The +# ladder that used to act on these numbers is gone; the numbers were the half of +# it worth keeping. +PRESSURE_LOW_DEN="${WATCHDOG_PRESSURE_LOW_DEN:-4}" # H < max/4 => low +PRESSURE_CRIT_DEN="${WATCHDOG_PRESSURE_CRIT_DEN:-8}" # H < max/8 => critical + +# Sweep state. Declared at file scope, not inside main(), so that sourcing with +# WATCHDOG_SOURCE_ONLY=1 gives the test harness correctly-typed globals. +declare -gA P_COMM=() P_CMD=() P_ARGV0=() P_PPID=() P_RSS=() P_PSS=() P_START=() P_AGE=() P_SID=() +declare -gA CHILDREN=() SERVER_TREE=() PROTECTED=() NOT_EDITOR=() PROTECT_REASON=() WATCHDOG_KIN=() +declare -gA POLICED=() CLAUDE_ROOTS=() +declare -ga PIDS=() SERVER_ROOTS=() +declare -gA OVER_SINCE=() KILLED_AT=() KILL_TIMES=() DISARMED=() KILLS=() SERVER_PID="" -TIER=L0 ROLE=other GUARD="" -PREV_TIER=L0 +PRESSURE=ok PREV_AT=0 -PREV_U=0 PREV_REFAULT=0 PREV_PGSCAN=0 +PREV_U=0 CYCLE=0 -TIME_TO_LIMIT=0 +SWEEPS=0 +UPTIME=0 H_MAX_SEEN=0 H_MIN_SEEN=0 -CALIBRATION_WARNED=0 -C_RESERVE=0 -TOO_SMALL=0 M_MAX=0 +POLICED_MAX_SEEN=0 +KILLS_TOTAL=0 +KILL_TIMES_ALL="" +LAST_KILL_AT=0 +PSS_UNAVAILABLE=0 +VISIBILITY_WARNED=0 STARTED_AT=${WATCHDOG_NOW:-$EPOCHSECONDS} -# How often each rung has acted since the watchdog started, and when it last did. -# Published in the summary file every cycle - in observe mode too, where it is -# the count of sheds enforce mode *would* have performed. That number is what -# says whether enforce mode is tolerable on a live workspace, and it can be had -# without ever signalling anything. -declare -gA ACTIONS=([L2]=0 [L3]=0 [L4]=0) -declare -gA RUNG_FIRED=() -EXCURSIONS=0 -IN_EXCURSION=0 - # --------------------------------------------------------------------------- # # derivation - one pure function of memory.max; no state, no I/O # --------------------------------------------------------------------------- # -clamp() { - local v=$1 lo=$2 hi=$3 - ((v < lo)) && v=$lo - ((v > hi)) && v=$hi - printf '%s' "$v" -} - -# Sets T_L1..T_L4 and CEILING from memory.max. Anything already set from the -# environment is left alone, so a single role or a single rung can be pinned for -# an experiment without replacing the derivation. -derive_limits() { - local max=$1 role c - - c=$(clamp $((max * C_FRACTION_NUM / C_FRACTION_DEN)) "$C_FLOOR" "$C_CAP") - C_RESERVE=$c - : "${T_L4:=$c}" - : "${T_L3:=$((c * 3 / 2))}" - : "${T_L2:=$((c * 5 / 2))}" - : "${T_L1:=$((c * 4))}" - - # Inversion: what pod size makes this formula harmful rather than imprecise? - # A small enough one. The floor is a reaction-time budget and cannot shrink - # with the pod, so below roughly 3 GiB the top of the ladder approaches - # memory.max and the pod is inside the shedding tiers from the moment it boots - # - which would mean killing the editor continuously, the one outcome that - # reliably gets a watchdog switched off for good. There is no threshold that - # fixes this, because the pod genuinely has no runway; the honest response is - # to say so and refuse to act. The workspace offers 4 and 8 GiB, both of which - # clear this comfortably, so this is a guard against a future option rather - # than a live case. - TOO_SMALL=0 - ((T_L1 * 2 > max)) && TOO_SMALL=1 - - local var - for role in "${!CEILING_SIXTEENTHS[@]}"; do - # An explicit WATCHDOG_CEILING_ wins over the derivation. - var="WATCHDOG_CEILING_${role}" +# Sets BUDGET from memory.max. An explicit WATCHDOG_BUDGET_ wins outright, +# which is how a budget is pinned for an experiment without editing the script. +# +# The two clamps pull in opposite directions and the order matters. The pod share +# is applied first because it expresses what the pod can afford; the resting +# floor is applied second because it expresses what the process demonstrably +# needs, and when those disagree the process wins. A budget below resting usage +# is not a conservative budget, it is a kill loop written down. +derive_budgets() { + local max=$1 role want share floor var + share=$((max / POD_SHARE_DEN)) + BUDGET=() + for role in "${!BUDGET_ROLE[@]}"; do + var="WATCHDOG_BUDGET_${role}" if [[ -n ${!var:-} ]]; then - CEILING[$role]=${!var} + BUDGET[$role]=${!var} continue fi - CEILING[$role]=$(clamp \ - $((max * CEILING_SIXTEENTHS[$role] / 16)) "$CEILING_FLOOR" "$CEILING_CAP") + want=${BUDGET_ROLE[$role]} + ((want > share)) && want=$share + floor=$(((${RESTING_ROLE[$role]:-0} * RESTING_FACTOR_NUM) / RESTING_FACTOR_DEN)) + ((want < floor)) && want=$floor + BUDGET[$role]=$want done return 0 } +# True when the mode may signal this role at all. +role_is_armed() { + local role=$1 + [[ -n ${DISARMED[$role]:-} ]] && return 1 + case "$MODE" in + enforce) [[ $HELPER_ROLES == *" $role "* ]] ;; + enforce-all) [[ $HELPER_ROLES == *" $role "* || $EDITOR_ROLES == *" $role "* ]] ;; + *) return 1 ;; + esac +} + # --------------------------------------------------------------------------- # # measurement - reads only, sets M_*/P_* globals, decides nothing # --------------------------------------------------------------------------- # @@ -289,12 +272,14 @@ derive_limits() { # Unreclaimable memory U. With memory.swap.max=0, all anon is unreclaimable. # # The `kernel` roll-up in memory.stat must NOT be used: it is dominated by -# slab_reclaimable (dentry/inode cache - 1.6 GiB on the real pod), which the +# slab_reclaimable (dentry/inode cache - 1.9 GiB on the real pod), which the # kernel hands back under pressure. Counting it makes an idle container look like # it is about to die. memory.current has the same defect plus the page cache, -# which is why it reads 97% here while U is 23%. +# which is why it reads 96% here while U is 28%. # -# Returns 0 on success, 1 on read failure, 2 if the cgroup has no memory limit. +# Nothing acts on this any more; it is the pod-level context every per-process +# row is read against, and the only record of what the container was doing at the +# time. Returns 0 on success, 1 on read failure, 2 if the cgroup has no limit. read_cgroup_memory() { local key val local anon=0 shmem=0 unevictable=0 slab_unreclaimable=0 kernel_stack=0 @@ -348,6 +333,10 @@ read_cgroup_memory() { read -r M_CURRENT <"${CGROUP_DIR}/memory.current" 2>/dev/null M_H=$((M_MAX - M_U)) + + PRESSURE=ok + ((M_H < M_MAX / PRESSURE_LOW_DEN)) && PRESSURE=low + ((M_H < M_MAX / PRESSURE_CRIT_DEN)) && PRESSURE=critical return 0 } @@ -365,19 +354,36 @@ read_cgroup_pressure() { return 0 } -# Fills PIDS / P_COMM / P_CMD / CHILDREN for one scan. +read_uptime() { + local raw="" + read -r raw _ <"${PROC_DIR}/uptime" 2>/dev/null + UPTIME=${raw%%.*} + [[ $UPTIME =~ ^[0-9]+$ ]] || UPTIME=0 + return 0 +} + +# Fills PIDS / P_COMM / P_CMD / P_ARGV0 / P_PPID / P_START / P_AGE / CHILDREN. +# +# starttime (stat field 22) is read for a reason that is not cosmetic: pids are +# recycled, and every piece of state this watchdog carries between sweeps - how +# long something has been over budget, whether it has already been sent a +# SIGTERM - is keyed on pid *and* starttime. Without that, a recycled pid +# inherits another process's history and can be killed for it. read_process_table() { PIDS=() P_COMM=() P_CMD=() P_ARGV0=() P_PPID=() + P_START=() + P_AGE=() + P_SID=() P_RSS=() - P_DATA=() + P_PSS=() CHILDREN=() - local entry pid line rest comm ppid - local -a argv + local entry pid line rest comm ppid start sid + local -a argv f for entry in "${PROC_DIR}"/[0-9]*; do pid=${entry##*/} @@ -389,9 +395,15 @@ read_process_table() { [[ $rest == "$line" ]] && continue comm=${line#*'('} comm=${comm%%') '*} - rest=${rest#* } # drop state - ppid=${rest%% *} + read -r -a f <<<"$rest" + ppid=${f[1]:-} [[ $ppid =~ ^[0-9]+$ ]] || continue + # f[0] is state, so f[19] is stat field 22, starttime, in USER_HZ ticks, and + # f[3] is field 6, the session id. + start=${f[19]:-0} + [[ $start =~ ^[0-9]+$ ]] || start=0 + sid=${f[3]:-0} + [[ $sid =~ ^[0-9]+$ ]] || sid=0 argv=() mapfile -d '' -t argv <"$entry/cmdline" 2>/dev/null @@ -404,32 +416,45 @@ read_process_table() { # handling this script has already been bitten by. P_ARGV0[$pid]="${argv[0]:-}" P_PPID[$pid]=$ppid + P_START[$pid]=$start + P_SID[$pid]=$sid + P_AGE[$pid]=$((UPTIME - start / USER_HZ)) + # Written with an explicit $: inside (( )) an associative-array subscript is + # a string, so `P_AGE[pid]` would look up the key "pid" and quietly read 0. + ((${P_AGE[$pid]} < 0)) && P_AGE[$pid]=0 CHILDREN[$ppid]+=" $pid" done return 0 } -# RSS, and the quantity RLIMIT_DATA actually accounts. -# -# These are not interchangeable and the difference is not small. RLIMIT_DATA -# limits mm->data_vm - private writable anonymous mappings - which for a V8 -# process is dominated by *reserved* address space rather than resident pages. -# Measured on the live 4 GiB workspace at rest: the extension host was 497 MB -# resident against 1004 MB of data, the file watcher 66 MB resident against -# 622 MB of data. A ceiling reasoned about as though it bounded RSS is therefore -# roughly an order of magnitude tighter than intended, and the first derived -# ceilings this script produced were *below* what an idle file watcher already -# held - which would have killed it on its next allocation, every time, forever. +# PSS from smaps_rollup, falling back to RSS from statm. # -# statm's sixth field is data_vm + stack_vm, so it overstates by the stack (a few -# hundred kB here). That is accepted rather than read a second file per process -# per scan: the allowance added on top is measured in hundreds of megabytes. -read_rss() { - local pid res data +# smaps_rollup walks the page tables, so it is the expensive read in this script; +# it is done once a minute for a few dozen processes and never in the cgroup +# sample. The fallback exists because smaps_rollup is absent on some kernels and +# unreadable for a process that exits mid-sweep - and when it is used, the row +# says so, because silently substituting a number that is 30% larger would make +# every budget look tighter than it is. +read_usage() { + local pid res key val got for pid in "$@"; do - read -r _ res _ _ _ data _ <"${PROC_DIR}/${pid}/statm" 2>/dev/null || continue + read -r _ res _ <"${PROC_DIR}/${pid}/statm" 2>/dev/null || continue P_RSS[$pid]=$((res * PAGE_SIZE)) - P_DATA[$pid]=$((data * PAGE_SIZE)) + got="" + if [[ -r "${PROC_DIR}/${pid}/smaps_rollup" ]]; then + while read -r key val _; do + if [[ $key == "Pss:" ]]; then + got=$val + break + fi + done <"${PROC_DIR}/${pid}/smaps_rollup" + fi + if [[ $got =~ ^[0-9]+$ ]]; then + P_PSS[$pid]=$((got * 1024)) + else + P_PSS[$pid]=${P_RSS[$pid]} + PSS_UNAVAILABLE=1 + fi done return 0 } @@ -439,7 +464,7 @@ read_rss() { # --------------------------------------------------------------------------- # # Fills SERVER_ROOTS with every remote-server entrypoint, and sets SERVER_PID to -# the first of them (used only for logging and the calibration CSV). +# the first of them (used only for logging). # # Three things about a real server tree that a plausible-looking implementation # gets wrong, all three confirmed against a live workspace: @@ -456,12 +481,12 @@ read_rss() { # `cat`, `tail -f` or `grep` over the same path from being elected as the root # of everything the watchdog then decides. # -# - there can be more than one. `--reconnection-grace-time 28800` keeps a -# disconnected server alive for eight hours, and a window on a different commit -# gets its own server. Electing one and scoping to it would leave the other -# tree not merely unmanaged but unprotected, because the ptyHost excision only -# runs inside the tree that was discovered. So every root counts and the -# managed tree is the union of their subtrees. +# - there can be more than one. `--reconnection-grace-time` keeps a disconnected +# server alive for hours, and a window on a different commit gets its own +# server. Electing one and scoping to it would leave the other tree not merely +# unmanaged but unprotected, because the ptyHost excision only runs inside the +# tree that was discovered. So every root counts and the managed tree is the +# union of their subtrees. # # Matching is always on a whole argv element, never on a substring of the joined # command line. @@ -488,12 +513,15 @@ find_server_roots() { done ((${#SERVER_ROOTS[@]})) || return 1 + # shellcheck disable=SC2034 + # Read by script-memory-watchdog-test.sh, which asserts which process is + # elected as the root - the one selection defect a fixture can catch early. SERVER_PID=${SERVER_ROOTS[0]} return 0 } -# Sets SERVER_TREE to the union of every root's descendants, and reads their RSS. -# Returns 1 when no server is running, leaving SERVER_TREE empty. +# Sets SERVER_TREE to the union of every root's descendants. Returns 1 when no +# server is running, leaving SERVER_TREE empty. build_server_tree() { local root pid local -A one=() @@ -505,7 +533,6 @@ build_server_tree() { SERVER_TREE[$pid]=1 done done - read_rss "${!SERVER_TREE[@]}" return 0 } @@ -538,26 +565,52 @@ subtree_of() { # True when the process is running a binary that VS Code itself shipped, i.e. one # under ~/.vscode-server. This is the structural boundary between "a process VS # Code started with its own runtime" and "a process that merely happens to sit -# inside the tree", and it is the primary safety rule of the whole watchdog. +# inside the tree", and it is one of the two primary safety rules here. # # A fully provisioned workspace has two unrelated node installations: VS Code's # bundled one at ~/.vscode-server/cli/servers/Stable-/server/node, which # arrives with the server download, and mise's on PATH, which is what the -# operator's repo tooling and Claude Code sessions run on. There is no -# /usr/bin/node and no node on PATH at all without dotfiles. Every process VS -# Code spawns - server-main.js, every bootstrap-fork, every node language server, -# and native helpers like terraform-ls - runs a binary under ~/.vscode-server; -# nothing the operator runs does. +# operator's repo tooling and agent sessions run on. There is no /usr/bin/node +# and no node on PATH at all without dotfiles. Every process VS Code spawns runs +# a binary under ~/.vscode-server; nothing the operator runs does. # # So keying detection on "is this node" by comm, by basename, or by a loose # cmdline match would make an agent session indistinguishable from an editor -# helper, and the watchdog would stamp RLIMIT_DATA on it and shed it at L2/L3 - -# the exact outcome this design exists to prevent, arriving one layer earlier -# than the action rules that are meant to prevent it. Path, and only path. +# helper. Path, and only path. is_vscode_binary() { [[ ${P_ARGV0[$1]:-} == *"/.vscode-server/"* ]] } +# True for the root process of an agent session. Structural: the program being +# executed is named claude, by comm (a compiled launcher) or by argv[0]'s +# basename (a shebang script, where comm is the interpreter). Never a substring +# of the joined command line - `*/claude*` once protected an unrelated process +# whose scratchpad path contained /claude-10001. +is_claude_root() { + local pid=$1 + [[ ${P_COMM[$pid]:-} == "claude" ]] && return 0 + local exe=${P_ARGV0[$pid]:-} + [[ ${exe##*/} == "claude" ]] +} + +# True for an interactive shell or a terminal multiplexer, i.e. a boundary the +# helper walk must not cross. Everything below one of these inside an agent +# session is the agent's *tool calls* - builds, test runs, `gh run watch` - which +# are in-flight work with no supervisor to restart them. They are excluded from +# policing on purpose, and that exclusion is the difference between "anything a +# session invokes is fair game" as a principle and as a foot-gun. +is_shell_like() { + local pid=$1 + local exe=${P_ARGV0[$pid]:-} + case "${P_COMM[$pid]:-}" in + bash | sh | dash | zsh | fish | ksh | tmux* | screen | sshd | ssh) return 0 ;; + esac + case "${exe##*/}" in + bash | sh | dash | zsh | fish | ksh | tmux | screen | sshd | ssh) return 0 ;; + esac + return 1 +} + # The watchdog's own process, everything that started it, and everything it # started. Structural, by pid: this replaces a `*memory-watchdog*` cmdline match # that once protected every process in a test harness because the harness lived @@ -592,10 +645,7 @@ compute_watchdog_kin() { # Matching is on whole path segments, never on a raw substring. Substrings are # what made `*/claude*` protect an unrelated process whose scratchpad path # happened to contain /claude: the segment there was `claude-10001`, which is not -# the program and does not match. The cost of being wrong is asymmetric but not -# free in either direction - over-matching protects something that could have -# been shed, which quietly turns the mechanism off, and that is exactly how the -# harness bug hid. +# the program and does not match. is_operator_payload() { local tok seg for tok in ${P_CMD[$1]:-}; do @@ -614,12 +664,6 @@ is_operator_payload() { # The never-signal list. Every action consults this directly, so a defect in # tree-walking still cannot route around it. Sets GUARD to the rule that fired, # so that a protection can be reported rather than merely happening. -# -# Tree membership is NOT a safe kill criterion: tmux sessions and long-running -# agents started from a VS Code integrated terminal are descendants of the server -# tree via ptyHost. The rules below are ordered cheapest and most certain first; -# each of them is asserted, and asserted to be individually reachable, by -# script-memory-watchdog-test.sh. is_never_signal() { local pid=$1 GUARD="" @@ -660,10 +704,12 @@ is_never_signal() { # Everything the watchdog must never touch: the never-signal rules anywhere in # the pod, plus every ptyHost fork inside the server tree and all its -# descendants. PROTECT_REASON records which rule claimed each pid, which is what -# makes an over-broad guard visible instead of silently inert. +# descendants, plus anything in the tree not running a VS Code binary. +# PROTECT_REASON records which rule claimed each pid, which is what makes an +# over-broad guard visible instead of silently inert. compute_protected() { PROTECTED=() + NOT_EDITOR=() PROTECT_REASON=() local pid p local -A pty=() @@ -677,22 +723,40 @@ compute_protected() { fi done - # Everything in the tree that is not running a VS Code binary, whatever its - # position in it. A Claude Code session spawned by an extension is a child of - # the extension host and not of ptyHost, so neither the subtree excision nor - # tree membership would save it; its executable path does. + # Two rules about *position* rather than identity, kept in their own set. The + # distinction is load-bearing and was learned the hard way: everything in + # PROTECTED is protected by what it is and may never be signalled by anything, + # whereas these two say only "this is not one of the editor's own helpers" - + # which is the correct rule for the editor walk and the wrong one for the + # second population. An MCP server belonging to a session that happens to be + # running in a VS Code terminal is inside the tree and does not run VS Code's + # binary, and it is exactly what this watchdog is meant to bound. + # + # First: anything in the tree not running a binary VS Code shipped. An agent + # session spawned by an extension is a child of the extension host and not of + # ptyHost, so tree position would not save it; its executable path does - and + # it is *also* claimed by the payload rule above, which is identity and is + # absolute. That overlap is deliberate belt-and-braces, not redundancy. for pid in "${!SERVER_TREE[@]}"; do if ! is_vscode_binary "$pid"; then - PROTECTED[$pid]=1 + NOT_EDITOR[$pid]=1 PROTECT_REASON[$pid]=${PROTECT_REASON[$pid]:-foreign-binary} fi done + # Terminals, and everything started in one. Tree membership is NOT a safe kill + # criterion: tmux sessions and long-running agent runs started from a VS Code + # integrated terminal are descendants of the server tree via ptyHost. + # + # Second: the ptyHost subtree. Shells, multiplexers and everything started in + # one are absolutely protected anyway - by comm, by argv[0], by the payload + # rule, and by the walk in compute_policed stopping at shells - so what this + # adds is the position, not the only line of defence. for pid in "${!SERVER_TREE[@]}"; do if [[ ${P_CMD[$pid]:-} == *"--type=ptyHost"* ]]; then subtree_of "$pid" pty for p in "${!pty[@]}"; do - PROTECTED[$p]=1 + NOT_EDITOR[$p]=1 PROTECT_REASON[$p]=${PROTECT_REASON[$p]:-ptyhost} done fi @@ -700,7 +764,7 @@ compute_protected() { return 0 } -# Sets ROLE. +# Sets ROLE for a process inside the VS Code server tree. # # `--type=` flags are matched as whole argv elements (P_CMD is argv joined with # spaces, so the surrounding spaces make the match exact). ptyHost is tested @@ -724,19 +788,11 @@ role_of() { *) # Native helpers shipped inside an extension - terraform-ls on the live tree, # and gopls / rust-analyzer / clangd on the same pattern. Keyed on argv[0], - # never on the joined command line: a Claude Code session launched by an - # extension has the extension's directory all over its arguments while its - # executable is mise's node, and matching the arguments would classify it as - # a sheddable helper. Checked last, so a node language server - which also - # lives under extensions/ but runs VS Code's own node - keeps its role above. - # - # It gets no RLIMIT_DATA ceiling, on purpose. The argument for a ceiling is - # that V8 turns ENOMEM into its own fatal heap OOM and the editor offers a - # restart; a Go or Rust runtime turns the same ENOMEM into an abrupt abort - # with no editor-side affordance, and there is no measured relationship - # between its working set and a number we could pick. It is still restartable - # and holds no unsaved state, so it is shed at L2 - a corroborated, debounced - # decision to kill a helper - rather than pre-emptively capped on a guess. + # never on the joined command line: an agent session launched by an extension + # has the extension's directory all over its arguments while its executable + # is mise's node, and matching the arguments would classify it as a sheddable + # helper. Checked last, so a node language server - which also lives under + # extensions/ but runs VS Code's own node - keeps its role above. if [[ $argv0 == *"/.vscode-server/extensions/"* ]]; then ROLE=extensionHelper else @@ -747,131 +803,149 @@ role_of() { return 0 } -# Fills CANDIDATES with " " rows the given tier may signal, -# heaviest resident set first. -select_candidates() { - local tier=$1 - local pid i bi - local -a sorted=() - CANDIDATES=() +# Fills POLICED with pid -> role: every process this watchdog is willing to have +# an opinion about. Two disjoint populations, found two different ways. +# +# 1. The VS Code server tree, minus the guards. Unchanged from the version that +# only knew about VS Code. +# +# 2. Helpers spawned by an agent session - the MCP servers and other long-lived +# children that the tree-scoped selection could not see at all, and where the +# largest single offender ever measured (1.66 GB of python) lived. The walk +# starts at each session root and descends, but never through a shell or a +# multiplexer: below one of those is the session's *tool calls*, which are +# in-flight work that nothing will restart. The session root itself is +# protected by name; this is only about what it spawned. +# +# The positional exclusions - the ptyHost subtree, and "does not run a binary VS +# Code shipped" - are permeable to the second walk, and that is the one +# deliberate hole in the rules; it is worth stating plainly. A session started +# with `coder ssh` sits under the agent and a session started in a VS Code +# terminal sits under ptyHost, running the operator's python or node rather than +# VS Code's; they are the same thing to the operator, and sparing one set of MCP +# servers because of which terminal its session came from would make the +# mechanism miss half its cases silently. What those rules are actually for - +# shells, multiplexers, the session itself, agent payloads, and every command run +# in a terminal - is still absolutely protected, by identity, because the walk +# stops at shells and every guard in is_never_signal still applies. +compute_policed() { + POLICED=() + CLAUDE_ROOTS=() + local pid root cur kids k for pid in "${!SERVER_TREE[@]}"; do - [[ -n ${PROTECTED[$pid]:-} ]] && continue + [[ -n ${PROTECTED[$pid]:-} || -n ${NOT_EDITOR[$pid]:-} ]] && continue role_of "$pid" - [[ $ROLE == "ptyHost" ]] && continue - case "$tier" in - L2) [[ $L2_ROLES == *" $ROLE "* ]] || continue ;; - L3) [[ $ROLE == "extensionHost" ]] || continue ;; - L4) ;; - *) continue ;; - esac - CANDIDATES+=("${P_RSS[$pid]:-0} $pid $ROLE") + [[ $ROLE == "ptyHost" || $ROLE == "other" ]] && continue + POLICED[$pid]=$ROLE done - # Selection sort. The server tree is a few dozen processes at most, and this - # avoids a fork to sort(1) on every cycle. - while ((${#CANDIDATES[@]})); do - bi=0 - for ((i = 1; i < ${#CANDIDATES[@]}; i++)); do - [[ ${CANDIDATES[i]%% *} -gt ${CANDIDATES[bi]%% *} ]] && bi=$i - done - sorted+=("${CANDIDATES[bi]}") - unset 'CANDIDATES[bi]' - CANDIDATES=("${CANDIDATES[@]}") + for pid in "${PIDS[@]}"; do + is_claude_root "$pid" || continue + CLAUDE_ROOTS[$pid]=1 + done + + local -a queue=() + local sid + for root in "${!CLAUDE_ROOTS[@]}"; do + kids=${CHILDREN[$root]:-} + # shellcheck disable=SC2086 + for k in $kids; do queue+=("${k}:${P_SID[$root]:-0}"); done + done + while ((${#queue[@]})); do + cur=${queue[0]%%:*} + sid=${queue[0]#*:} + queue=("${queue[@]:1}") + # Two independent boundaries, either of which ends the walk. + # + # The session id is the stronger of the two and it is measured, not assumed: + # on the live workspace an agent session root has pgid == its own pid and + # sid == the login shell's session, while every tool call it runs has pgid + # == sid == its own pid - i.e. Claude Code detaches each Bash tool call into + # a new session so that it can kill the whole tree later. A stdio MCP server + # is spawned to talk over pipes and stays in the session it was started + # from. So "same session as the root" separates the servers from the work + # even when the tool call's shell has exec'd itself away, which the shell + # test alone would miss. + # + # If that ever stops being true, this walk polices nothing rather than + # policing the wrong thing, and check_visibility says so in the log. + [[ ${P_SID[$cur]:-0} == "$sid" ]] || continue + # And a shell is a boundary in its own right: neither it nor anything under + # it is policed, whatever session it is in. + is_shell_like "$cur" && continue + kids=${CHILDREN[$cur]:-} + # shellcheck disable=SC2086 + for k in $kids; do queue+=("${k}:${sid}"); done + # Protected by identity: not a candidate. Excluded only positionally - in + # the ptyHost subtree, or not running VS Code's own binary: still a + # candidate, for the reason given above. + [[ -n ${PROTECTED[$cur]:-} ]] && continue + is_claude_root "$cur" && continue + [[ -n ${POLICED[$cur]:-} ]] && continue + POLICED[$cur]=claudeHelper done - CANDIDATES=("${sorted[@]}") return 0 } -# --------------------------------------------------------------------------- # -# decision - a function of the numbers and the debounce counters only -# --------------------------------------------------------------------------- # - -C_L1=0 -C_L2=0 -C_L3=0 -LAST_ACTION_AT=0 -SUPPRESSED="" - -# Sets TIER. Called in the current shell, never in a command substitution - the -# debounce counters are state and a subshell would silently discard them. +# A short, stable identity for a policed process - what it *is*, as opposed to +# which pid it happens to have this time. This is the breadcrumb that makes the +# sweep log answerable after the fact ("which MCP server was that?"), and the +# label a metrics exporter would bucket on. # -# SUPPRESSED records why an acting tier was demoted, so that "the ladder reached -# L3 and nothing happened" is always accompanied by the reason. It is not -# decoration: the one class of bug this watchdog has repeatedly produced is a -# state that looks correct because the evidence of its being wrong is absent. -decide_tier() { - local h=$1 psi=$2 refault=$3 proj=$4 now=$5 - local want - SUPPRESSED="" - - if ((h < T_L1)); then ((C_L1 += 1)); else C_L1=0; fi - if ((h < T_L2)); then ((C_L2 += 1)); else C_L2=0; fi - if ((h < T_L3)); then ((C_L3 += 1)); else C_L3=0; fi - - # Excursion tracking. An excursion opens the first time headroom falls below - # L1 and closes when it comes back above it; the rung-fired flags live for - # exactly that long. Recovery is what re-arms the ladder, not the passage of - # time, because time passing does not mean the pressure went away. - if ((h < T_L1)); then - if ((IN_EXCURSION == 0)); then - IN_EXCURSION=1 - ((EXCURSIONS += 1)) - RUNG_FIRED=() - fi - elif ((IN_EXCURSION == 1)); then - IN_EXCURSION=0 - RUNG_FIRED=() - fi - - if ((h < T_L4)); then - TIER=L4 - return 0 - fi - - want=L0 - if ((C_L3 >= DEBOUNCE_L3)); then - want=L3 - elif ((C_L2 >= DEBOUNCE_L2)) && - { ((psi >= T_PSI_CENTI)) || ((refault >= T_REFAULT_RATE)); }; then - want=L2 - elif ((C_L1 >= DEBOUNCE_L1)); then - want=L1 - fi - - # Projection. Armed only once headroom is already below L1, so a momentary - # allocation spike from an idle state cannot vault the ladder. - if [[ $want != "L0" ]] && ((proj < T_L4)); then - want=L3 - fi - - # Rate limiting, in two independent parts - see SETTLE above for why the - # single 180s cooldown they replace was worse than either. - if [[ $want == "L2" || $want == "L3" ]]; then - if ((LAST_ACTION_AT > 0)) && ((now - LAST_ACTION_AT < SETTLE)); then - SUPPRESSED="settling(${want})" - want=L1 - elif [[ -n ${RUNG_FIRED[$want]:-} ]]; then - SUPPRESSED="already-fired-this-excursion(${want})" - want=L1 - fi +# Secrets are redacted here rather than at read time, because this is the only +# place a command line is written to a file that might later be shipped +# somewhere: VS Code puts --connection-token= on its own argv, and an MCP server +# can be launched with an API key in an argument. +identity_of() { + local pid=$1 role=$2 tok seg fallback + # For a helper, the interpreter is never the interesting part of the name: a + # dozen unrelated MCP servers are all `node` or `python3`, and what + # distinguishes them is the first argument that is not the interpreter and not + # a flag. For everything else argv[0] is the name, because that is the thing + # the role was decided from. + if [[ $role == "claudeHelper" || $role == "extensionHelper" ]]; then + for tok in ${P_CMD[$pid]:-}; do + seg=${tok##*/} + [[ -z $seg ]] && continue + case "$seg" in + -* | node | node[0-9]* | python | python[0-9] | python[0-9].[0-9]* | uv | uvx | npx | bun | deno) continue ;; + esac + printf '%s' "${seg%.js}" + return 0 + done fi - - TIER=$want + fallback=${P_ARGV0[$pid]:-} + [[ -n $fallback ]] || fallback=${P_COMM[$pid]:-unknown} + printf '%s' "${fallback##*/}" return 0 } +redact() { + local acc="" tok + for tok in $1; do + case "$tok" in + *token=* | *TOKEN=* | *key=* | *KEY=* | *secret=* | *SECRET=* | *password=* | *PASSWORD=*) + acc+=" ${tok%%=*}=" + ;; + *) acc+=" ${tok}" ;; + esac + done + printf '%s' "${acc# }" +} + # --------------------------------------------------------------------------- # # action - the only place that writes state or signals anything # --------------------------------------------------------------------------- # LOG_LINES=0 CSV_LINES=0 +SWEEP_LINES=0 log_action() { local msg="$*" stamp printf -v stamp '%(%Y-%m-%dT%H:%M:%S%z)T' -1 - [[ $MODE == "enforce" ]] || msg="[observe] ${msg}" + [[ $MODE == "observe" ]] && msg="[observe] ${msg}" printf '%s %s\n' "$stamp" "$msg" >>"${STATE_DIR}/actions.log" ((LOG_LINES += 1)) if ((LOG_LINES > MAX_LOG_LINES)); then @@ -881,11 +955,6 @@ log_action() { return 0 } -# The projected-headroom term goes negative whenever dU/dt is steep enough to -# exhaust the cgroup inside the horizon - a normal reading, and the one the log -# most needs to be legible for. Bash division truncates toward zero, so both -# halves of a negative value come out negative and print as "-27.-79 GiB"; the -# sign is taken off the front and applied once. fmt_gib() { local v=$1 sign="" if ((v < 0)); then @@ -895,70 +964,38 @@ fmt_gib() { printf '%s%d.%02d GiB' "$sign" "$((v / GIB))" "$((v % GIB * 100 / GIB))" } +fmt_mib() { + printf '%d MiB' "$(($1 / MIB))" +} + publish_headroom() { printf '%s free (%s)\n' "$(fmt_gib "$1")" "$2" >"${STATE_DIR}/headroom.tmp" && mv -f "${STATE_DIR}/headroom.tmp" "${STATE_DIR}/headroom" return 0 } -# How many processes the current scan found, how many of them the guards claimed, -# and - the number that matters - how many remain eligible to be signalled at -# all. A managed tree with zero eligible processes is the signature of a guard -# that has swallowed everything, and it is worth more than any assertion about a -# particular pid because it does not depend on knowing which pid to ask about. -census_line() { - local pid role protected=0 eligible=0 pty_n=0 - for pid in "${!SERVER_TREE[@]}"; do - if [[ -n ${PROTECTED[$pid]:-} ]]; then - ((protected += 1)) - [[ ${PROTECT_REASON[$pid]:-} == "ptyhost" ]] && ((pty_n += 1)) - continue - fi - role_of "$pid" - [[ $ROLE == "ptyHost" ]] && continue - ((eligible += 1)) - done - printf 'tree=%d protected=%d ptyhost=%d eligible=%d' \ - "${#SERVER_TREE[@]}" "$protected" "$pty_n" "$eligible" -} - -# The question "would enforce mode have been tolerable to live with?" answered -# from observe mode, where it costs nothing to ask. Sheds per hour of uptime is -# the number that decides whether the operator turns this on and leaves it on - -# a watchdog that is switched off protects nothing, however correct each of its -# individual decisions was. -publish_summary() { - local now=$1 h=$2 tier=$3 - local up=$((now - STARTED_AT)) - ((up > 0)) || up=1 - local total=$((${ACTIONS[L2]:-0} + ${ACTIONS[L3]:-0} + ${ACTIONS[L4]:-0})) - { - printf 'mode=%s uptime_s=%d tier=%s\n' "$MODE" "$up" "$tier" - printf 'h=%s h_min=%s h_max=%s\n' \ - "$(fmt_gib "$h")" "$(fmt_gib "$H_MIN_SEEN")" "$(fmt_gib "$H_MAX_SEEN")" - printf 'thresholds l1=%s l2=%s l3=%s l4=%s (reserve=%s of memory.max=%s)\n' \ - "$(fmt_gib "$T_L1")" "$(fmt_gib "$T_L2")" "$(fmt_gib "$T_L3")" \ - "$(fmt_gib "$T_L4")" "$(fmt_gib "$C_RESERVE")" "$(fmt_gib "$M_MAX")" - printf 'excursions=%d sheds l2=%d l3=%d l4=%d total=%d\n' \ - "$EXCURSIONS" "${ACTIONS[L2]:-0}" "${ACTIONS[L3]:-0}" "${ACTIONS[L4]:-0}" "$total" - # Per day rather than per hour: the operator's question is "not every 15 - # minutes", and an hourly rate computed over a few minutes of uptime reads - # as a huge number for one event. - printf 'shed_rate_per_day=%d.%02d\n' \ - "$((total * 86400 / up))" "$((total * 86400 * 100 / up % 100))" - printf '%s\n' "$(census_line)" - } >"${STATE_DIR}/summary.tmp" && - mv -f "${STATE_DIR}/summary.tmp" "${STATE_DIR}/summary" +# The workspace UI tile: the largest policed process as a share of its budget. +# This is the number the operator used to obtain by running ps himself, which is +# the habit this whole change exists to end. +publish_top() { + local best="none" line + if [[ -n ${TOP_ROLE:-} ]]; then + printf -v line '%s %s (%d%% of %s)' \ + "$TOP_ID" "$(fmt_mib "$TOP_PSS")" "$TOP_PCT" "$(fmt_mib "$TOP_BUDGET")" + best=$line + fi + printf '%s\n' "$best" >"${STATE_DIR}/top.tmp" && + mv -f "${STATE_DIR}/top.tmp" "${STATE_DIR}/top" return 0 } -CSV_HEADER="ts,mem_max,mem_current,u,h,anon,shmem,unevictable,slab_unreclaimable,slab_reclaimable,kernel_stack,pagetables,sec_pagetables,percpu,sock,file,psi_full_avg10_centi,refault_file_per_s,pgscan_direct_per_s,tier,server_pid,tree_procs,tree_rss,protected,eligible,t_l1,t_l4" +CSV_HEADER="ts,mem_max,mem_current,u,h,anon,shmem,unevictable,slab_unreclaimable,slab_reclaimable,kernel_stack,pagetables,sec_pagetables,percpu,sock,file,psi_full_avg10_centi,refault_file_per_s,pgscan_direct_per_s,pressure,du_bytes_per_s" append_calibration() { local csv="${STATE_DIR}/calibration.csv" first="" if [[ -s $csv ]]; then - # A file written by an earlier version has fewer columns, and appending to it - # would produce one CSV that is silently two different schemas. Rotate it. + # A file written by an earlier version has different columns, and appending + # to it would produce one CSV that is silently two schemas. Rotate it. read -r first <"$csv" 2>/dev/null if [[ $first != "$CSV_HEADER" ]]; then mv -f "$csv" "${csv}.1" 2>/dev/null @@ -976,171 +1013,293 @@ append_calibration() { return 0 } -# Proposals already written to the log, keyed by pid and value. In enforce mode a -# ceiling is set once and the process then fails the "needs lowering" test on -# every later cycle, so it is logged once. In observe mode nothing is ever set, -# so without this the same handful of lines is appended every cycle - which at a -# 10-second interval buries the tier transitions the log exists to record. -declare -gA CEILING_LOGGED=() - -# Idempotent: reads the current soft limit and only lowers what is unlimited or -# above the ceiling. Runs every cycle regardless of tier, because this is the -# proactive mechanism and it must also cover processes spawned while the pod is -# already under pressure - not only while it is idle. -apply_ceilings() { - local pid want cur floor obs - local -a f - for pid in "${!SERVER_TREE[@]}"; do - role_of "$pid" - [[ $ROLE == "ptyHost" ]] && continue - [[ -n ${PROTECTED[$pid]:-} ]] && continue - want=${CEILING[$ROLE]:-} - [[ -n $want ]] || continue - - # A ceiling must be a growth allowance above what the process already holds, - # never an absolute size derived from the pod alone. The derived value says - # what the pod can afford; this says what the process demonstrably needs - # right now. Taking the larger of the two is what stops a correctly-reasoned - # budget from being an instant kill order for a healthy process - see - # read_rss() for the measurements that made this necessary. - obs=${P_DATA[$pid]:-0} - floor=$((obs + 2 * C_RESERVE)) - ((want < floor)) && want=$floor - - # A ceiling at or above the pod's own limit cannot bound anything, and - # setting one would only look like protection. Say so instead. - if ((want >= M_MAX)); then - if [[ -z ${CEILING_LOGGED[${pid}:inert]:-} ]]; then - CEILING_LOGGED[${pid}:inert]=1 - log_action "no-ceiling pid=${pid} role=${ROLE} data=${obs} would-need=${want} memory.max=${M_MAX} (a cap this size cannot bound anything)" - fi - continue - fi - - cur="" - while read -r -a f; do - [[ ${f[0]:-} == "Max" && ${f[1]:-} == "data" && ${f[2]:-} == "size" ]] || continue - cur=${f[3]:-} - break - done <"${PROC_DIR}/${pid}/limits" 2>/dev/null - [[ -n $cur ]] || continue - if [[ $cur != "unlimited" ]]; then - [[ $cur =~ ^[0-9]+$ ]] || continue - ((cur <= want)) && continue - fi - - if [[ $MODE == "enforce" ]]; then - /usr/bin/prlimit --pid "$pid" --data="${want}:" 2>/dev/null || continue - fi - [[ -n ${CEILING_LOGGED[${pid}:${want}]:-} ]] && continue - CEILING_LOGGED[${pid}:${want}]=1 - log_action "ceiling pid=${pid} role=${ROLE} rlimit_data=${want} was=${cur}" - done - # Pids are recycled, so the memo has to be pruned or it grows without bound and - # eventually suppresses a proposal for a genuinely new process. - if ((${#CEILING_LOGGED[@]} > 512)); then - CEILING_LOGGED=() +SWEEP_HEADER=$'#ts\tsweep\tpid\tstart\trole\tstate\tpss_kb\trss_kb\tage_s\tbudget_kb\tover_s\tguard\tidentity\tcmd' + +# The per-process record. It is the point of this version of the script as much +# as the killing is: every post-mortem in this investigation was unanswerable +# because the watchdog computed exactly this table on every cycle and then threw +# it away, so "what was holding the memory ten minutes before the kill" had no +# source. It is a local file first - Prometheus and Loki can be added later and +# cannot be backfilled - and it is written whatever the mode, because observe +# mode is where the budgets get their evidence. +append_sweep() { + local dst="${STATE_DIR}/sweep.log" first="" + if [[ -s $dst ]]; then + read -r first <"$dst" 2>/dev/null + [[ $first != "$SWEEP_HEADER" ]] && mv -f "$dst" "${dst}.1" 2>/dev/null + fi + [[ -s $dst ]] || printf '%s\n' "$SWEEP_HEADER" >"$dst" + printf '%s\n' "$@" >>"$dst" + SWEEP_LINES=$((SWEEP_LINES + $#)) + if ((SWEEP_LINES > MAX_SWEEP_LINES)); then + mv -f "$dst" "${dst}.1" 2>/dev/null + SWEEP_LINES=0 fi return 0 } signal_pid() { - local sig=$1 pid=$2 why=$3 + local sig=$1 pid=$2 role=$3 why=$4 # Second, independent guard. Selection already excluded these; this exists so # that a defect in tree-walking still cannot reach a protected process. if [[ -n ${PROTECTED[$pid]:-} ]] || is_never_signal "$pid"; then log_action "REFUSED sig=${sig} pid=${pid} reason=protected (${why})" return 1 fi - [[ $MODE == "enforce" ]] && kill "-${sig}" "$pid" 2>/dev/null - role_of "$pid" - log_action "signal sig=${sig} pid=${pid} rss=${P_RSS[$pid]:-0} role=${ROLE} (${why})" + # And the positional rules, restated independently of selection: inside the + # server tree, the only thing that may be signalled without being one of the + # editor's own helpers is a helper a session spawned directly. + if [[ -n ${NOT_EDITOR[$pid]:-} && $role != "claudeHelper" ]]; then + log_action "REFUSED sig=${sig} pid=${pid} reason=${PROTECT_REASON[$pid]:-not-editor} role=${role} (${why})" + return 1 + fi + [[ $MODE == "observe" ]] || kill "-${sig}" "$pid" 2>/dev/null return 0 } -shed_load() { - local tier=$1 now=$2 - local row rss pid role acted=0 - local -a targets=() +# The circuit breaker. Called after a kill; decides whether this role has stopped +# being a drifting role and started being a loop. +record_kill() { + local role=$1 now=$2 + local t keep="" n=0 all="" na=0 - select_candidates "$tier" + KILLS[$role]=$((${KILLS[$role]:-0} + 1)) + KILLS_TOTAL=$((KILLS_TOTAL + 1)) + LAST_KILL_AT=$now - # An acting tier that signals nothing is the exact shape of the bug that hid a - # broken guard through two clean-looking test runs: enforce mode logged - # tier=L3 with no signal and no REFUSED line, a state the code is otherwise - # supposed to make impossible. It is now impossible for a different reason - - # every path out of here says something. - if ((${#CANDIDATES[@]} == 0)); then - log_action "no-candidates tier=${tier} $(census_line)" - return 0 + for t in ${KILL_TIMES[$role]:-}; do + ((now - t < LOOP_WINDOW)) || continue + keep+="${t} " + ((n += 1)) + done + keep+="${now}" + ((n += 1)) + KILL_TIMES[$role]=$keep + + for t in ${KILL_TIMES_ALL:-}; do + ((now - t < LOOP_WINDOW)) || continue + all+="${t} " + ((na += 1)) + done + all+="${now}" + ((na += 1)) + KILL_TIMES_ALL=$all + + if ((n >= LOOP_KILLS)); then + DISARMED[$role]=1 + log_action "DISARMED role=${role} reason=kill-loop kills=${n} window=${LOOP_WINDOW}s budget=$(fmt_mib "${BUDGET[$role]:-0}") - a role that has to be killed this often does not have a drift problem, it has a wrong budget; raise WATCHDOG_BUDGET_${role} or accept the size, but this watchdog will not keep restarting it" fi + if ((na >= GLOBAL_LOOP_KILLS)); then + local r + for r in "${!BUDGET[@]}"; do DISARMED[$r]=1; done + log_action "DISARMED role=all reason=global-kill-loop kills=${na} window=${LOOP_WINDOW}s - too many kills across roles for the budgets to be right; enforcement is off until the watchdog restarts" + fi + return 0 +} - for row in "${CANDIDATES[@]}"; do - rss=${row%% *} - pid=${row#* } - role=${pid#* } - pid=${pid%% *} - # Below L4, a shed has to pay for itself. Killing something small does not - # move headroom, and it spends the rung as surely as killing something big - # would - see MIN_SHED_RSS. At L4 the tree is going down regardless. - if [[ $tier != "L4" ]] && ((rss < MIN_SHED_RSS)); then - log_action "no-worthwhile-candidate tier=${tier} best=${pid} role=${role} rss=${rss} min=${MIN_SHED_RSS}" - break +# --------------------------------------------------------------------------- # +# the sweep - measure every process, decide, act, record +# --------------------------------------------------------------------------- # + +# Prunes dwell and kill bookkeeping for keys whose process is gone. Without this +# the maps grow for the life of the pod, and - worse - a recycled pid could +# inherit an entry. Keys are pid:starttime, so recycling changes the key. +prune_state() { + local key pid start + local -A live=() + for pid in "${PIDS[@]}"; do + live["${pid}:${P_START[$pid]:-0}"]=1 + done + for key in "${!OVER_SINCE[@]}"; do + [[ -n ${live[$key]:-} ]] || unset 'OVER_SINCE[$key]' + done + for key in "${!KILLED_AT[@]}"; do + [[ -n ${live[$key]:-} ]] || unset 'KILLED_AT[$key]' + done + return 0 +} + +sweep_once() { + local now=$1 + local pid role budget pss key over_since over_s state guard id cmd age + local -a rows=() + local policed_n=0 over_n=0 killed_n=0 total_pss=0 + + read_uptime + read_process_table + build_server_tree + compute_protected + compute_policed + read_usage "${PIDS[@]}" + prune_state + + TOP_ROLE="" + TOP_ID="" + TOP_PSS=0 + TOP_BUDGET=0 + TOP_PCT=0 + + for pid in "${!POLICED[@]}"; do + role=${POLICED[$pid]} + budget=${BUDGET[$role]:-0} + pss=${P_PSS[$pid]:-0} + age=${P_AGE[$pid]:-0} + key="${pid}:${P_START[$pid]:-0}" + ((policed_n += 1)) + total_pss=$((total_pss + pss)) + + id="$(identity_of "$pid" "$role")" + if ((budget > 0)) && ((pss * 100 / budget > TOP_PCT)); then + TOP_ROLE=$role + TOP_PSS=$pss + TOP_BUDGET=$budget + TOP_PCT=$((pss * 100 / budget)) + TOP_ID=$id fi - if signal_pid TERM "$pid" "${tier} ${role} rss=${rss}"; then - acted=1 - targets+=("$pid") - [[ $tier == "L4" ]] || break + + state=ok + over_s=0 + if ((budget > 0 && pss > budget)); then + ((over_n += 1)) + : "${OVER_SINCE[$key]:=$now}" + over_since=${OVER_SINCE[$key]} + over_s=$((now - over_since)) + state=over + if ((over_s >= DWELL_SECONDS)) && ((age >= MIN_AGE)); then + state=over-dwell + if [[ -n ${DISARMED[$role]:-} ]]; then + state=disarmed + elif ! role_is_armed "$role"; then + # Observe mode, or a role this mode does not arm. Report the kill that + # would have happened, then restart the dwell clock so the same process + # is reported once per dwell period rather than on every sweep for the + # rest of its life. + state=would-kill + log_action "would-kill pid=${pid} role=${role} id=${id} pss=${pss} budget=${budget} over_s=${over_s} age=${age} mode=${MODE} armed=no" + OVER_SINCE[$key]=$now + elif [[ -n ${KILLED_AT[$key]:-} ]]; then + # Already asked politely. Escalate once the grace has elapsed. + if ((now - KILLED_AT[$key] >= KILL_GRACE)); then + if signal_pid KILL "$pid" "$role" "drift ${role}"; then + state=killed-9 + log_action "kill sig=KILL pid=${pid} role=${role} id=${id} pss=${pss} budget=${budget} over_s=${over_s} (still over budget ${KILL_GRACE}s after SIGTERM)" + fi + else + state=terminating + fi + else + if signal_pid TERM "$pid" "$role" "drift ${role}"; then + KILLED_AT[$key]=$now + state=killed + ((killed_n += 1)) + log_action "kill sig=TERM pid=${pid} role=${role} id=${id} pss=${pss} budget=${budget} over_s=${over_s} age=${age} mode=${MODE}" + record_kill "$role" "$now" + fi + fi + fi + else + unset 'OVER_SINCE[$key]' + unset 'KILLED_AT[$key]' fi + + guard=${PROTECT_REASON[$pid]:-'-'} + cmd="$(redact "${P_CMD[$pid]:-}")" + rows+=("${now}"$'\t'"${SWEEPS}"$'\t'"${pid}"$'\t'"${P_START[$pid]:-0}"$'\t'"${role}"$'\t'"${state}"$'\t'"$((pss / 1024))"$'\t'"$((${P_RSS[$pid]:-0} / 1024))"$'\t'"${age}"$'\t'"$((budget / 1024))"$'\t'"${over_s}"$'\t'"${guard}"$'\t'"${id}"$'\t'"${cmd:0:160}") done - # L4 only: give the tree two seconds to exit, then SIGKILL whatever of it is - # still there. Re-verify identity from /proc rather than trusting the tables - # captured before the SIGTERM, because a pid can be recycled in between. - if [[ $tier == "L4" ]] && ((acted)); then - /usr/bin/sleep 2 - local -a argv - for pid in "${targets[@]}"; do - [[ -r ${PROC_DIR}/${pid}/cmdline ]] || continue - argv=() - mapfile -d '' -t argv <"${PROC_DIR}/${pid}/cmdline" 2>/dev/null - [[ "${argv[*]}" == *"/.vscode-server/"* ]] || continue - signal_pid KILL "$pid" "L4 escalation" - done - fi + # Everything else that is large enough to matter, policed or not. This is what + # makes the log answerable about the processes the watchdog does *not* manage - + # which, on the evidence of every OOM recorded for this workspace, is where the + # memory actually was. + for pid in "${PIDS[@]}"; do + [[ -n ${POLICED[$pid]:-} ]] && continue + pss=${P_PSS[$pid]:-0} + ((pss >= SWEEP_LOG_FLOOR)) || continue + guard=${PROTECT_REASON[$pid]:-'-'} + cmd="$(redact "${P_CMD[$pid]:-}")" + rows+=("${now}"$'\t'"${SWEEPS}"$'\t'"${pid}"$'\t'"${P_START[$pid]:-0}"$'\t'"unmanaged"$'\t'"seen"$'\t'"$((pss / 1024))"$'\t'"$((${P_RSS[$pid]:-0} / 1024))"$'\t'"${P_AGE[$pid]:-0}"$'\t'"0"$'\t'"0"$'\t'"${guard}"$'\t'"${P_COMM[$pid]:-unknown}"$'\t'"${cmd:0:160}") + done + + ((policed_n > POLICED_MAX_SEEN)) && POLICED_MAX_SEEN=$policed_n + + rows+=("${now}"$'\t'"${SWEEPS}"$'\t'"0"$'\t'"0"$'\t'"TOTAL"$'\t'"${PRESSURE}"$'\t'"$((total_pss / 1024))"$'\t'"0"$'\t'"0"$'\t'"0"$'\t'"0"$'\t'"-"$'\t'"policed=${policed_n},over=${over_n},killed=${killed_n},procs=${#PIDS[@]},tree=${#SERVER_TREE[@]},sessions=${#CLAUDE_ROOTS[@]}"$'\t'"h=${M_H},u=${M_U},psi=${M_PSI_CENTI},mode=${MODE}") + append_sweep "${rows[@]}" + printf '%s\n' "$SWEEP_HEADER" "${rows[@]}" >"${STATE_DIR}/sweep.latest.tmp" && + mv -f "${STATE_DIR}/sweep.latest.tmp" "${STATE_DIR}/sweep.latest" + + POLICED_N=$policed_n + OVER_N=$over_n + publish_top + return 0 +} - if ((acted)); then - LAST_ACTION_AT=$now - RUNG_FIRED[$tier]=1 - ACTIONS[$tier]=$((${ACTIONS[$tier]:-0} + 1)) +# The falsification the census used to provide, generalised. A watchdog that +# manages nothing looks exactly like a watchdog with nothing to do, and the only +# difference is whether anything was ever there to manage. Nothing is *wrong* +# with a workspace where VS Code is closed and no session is running - but it is +# worth one line in the log, because the alternative reading is that selection is +# broken, and that reading has been correct before. +VISIBILITY_WARMUP="${WATCHDOG_VISIBILITY_WARMUP:-30}" # sweeps + +check_visibility() { + ((VISIBILITY_WARNED)) && return 0 + ((SWEEPS >= VISIBILITY_WARMUP)) || return 0 + VISIBILITY_WARNED=1 + if ((POLICED_MAX_SEEN == 0)); then + log_action "WARNING no process has been policed in ${SWEEPS} sweeps - either nothing this watchdog manages has run, or selection is finding nothing; sweep.latest shows what was there" fi return 0 } +publish_summary() { + local now=$1 + local up=$((now - STARTED_AT)) + ((up > 0)) || up=1 + local role line="" + { + printf 'mode=%s uptime_s=%d sweeps=%d\n' "$MODE" "$up" "$SWEEPS" + printf 'h=%s h_min=%s h_max=%s pressure=%s\n' \ + "$(fmt_gib "$M_H")" "$(fmt_gib "$H_MIN_SEEN")" "$(fmt_gib "$H_MAX_SEEN")" "$PRESSURE" + printf 'policed=%d over_budget=%d sessions=%d tree=%d\n' \ + "${POLICED_N:-0}" "${OVER_N:-0}" "${#CLAUDE_ROOTS[@]}" "${#SERVER_TREE[@]}" + for role in "${!BUDGET[@]}"; do + line+="${role}=$((${BUDGET[$role]} / MIB))M" + [[ -n ${DISARMED[$role]:-} ]] && line+="(disarmed)" + line+=" " + done + printf 'budgets %s\n' "$line" + line="" + for role in "${!KILLS[@]}"; do line+="${role}:${KILLS[$role]} "; done + printf 'kills total=%d by_role=%s last=%s\n' \ + "$KILLS_TOTAL" "${line:-none}" "${LAST_KILL_AT:-0}" + # Per day rather than per hour: the operator's question is "not every + # fifteen minutes", and an hourly rate over a short uptime reads as a huge + # number for one event. + printf 'kill_rate_per_day=%d.%02d\n' \ + "$((KILLS_TOTAL * 86400 / up))" "$((KILLS_TOTAL * 86400 * 100 / up % 100))" + printf 'dwell_s=%d min_age_s=%d loop_window_s=%d loop_kills=%d pss=%s\n' \ + "$DWELL_SECONDS" "$MIN_AGE" "$LOOP_WINDOW" "$LOOP_KILLS" \ + "$((PSS_UNAVAILABLE ? 0 : 1))" + } >"${STATE_DIR}/summary.tmp" && + mv -f "${STATE_DIR}/summary.tmp" "${STATE_DIR}/summary" + return 0 +} + # --------------------------------------------------------------------------- # # lifecycle # --------------------------------------------------------------------------- # # A coder_script re-runs when the agent restarts without the pod restarting, so -# two watchdogs are otherwise entirely possible. `set -o noclobber` gives an -# atomic O_EXCL create, which is all that is needed here: /usr/bin/flock does -# exist in the image, but it would only add a fork and a held descriptor to get -# the same guarantee. -# Liveness decides, not the existence of a file. -# -# The previous version treated a failed O_EXCL create as evidence that another -# watchdog held the lock, and only then looked at whether the recorded pid was -# alive. That inverts the reliable test and the unreliable one, and it failed in -# exactly the situation this daemon exists for: the pod was OOM-killed, the -# watchdog died by SIGKILL without running its EXIT trap, and the pidfile -# survived on the NFS-backed home directory. Every restart afterwards logged -# "another instance is already running" and exited, so the first kill left the -# pod permanently unwatched - observed on the test workspace, not theorised. +# two watchdogs are otherwise entirely possible. # -# The identity check is the script's own path as a whole argv element, not a -# substring of the command line. A recycled pid in a fresh container would have -# to be running this same script for the check to match, which is precisely the -# case where refusing to start is correct. +# Liveness decides, not the existence of a file. The previous version treated a +# failed O_EXCL create as evidence that another watchdog held the lock, and only +# then looked at whether the recorded pid was alive. That inverts the reliable +# test and the unreliable one, and it failed in exactly the situation this daemon +# exists for: the pod was OOM-killed, the watchdog died by SIGKILL without +# running its EXIT trap, and the pidfile survived on the NFS-backed home +# directory. Every restart afterwards logged "another instance is already +# running" and exited - observed on the test workspace, not theorised. acquire_singleton() { local pidfile="${STATE_DIR}/watchdog.pid" local self="${BASH_SOURCE[0]}" @@ -1168,6 +1327,10 @@ acquire_singleton() { release_singleton() { local pidfile="${STATE_DIR}/watchdog.pid" owner="" + # Both traps can fire, so the second one finds no file. A redirection that + # fails is reported by the shell before `2>/dev/null` on the same command has + # taken effect, so the test is done here rather than swallowed there. + [[ -r $pidfile ]] || return 0 read -r owner <"$pidfile" 2>/dev/null # Never remove a pidfile another instance owns - that would hand the lock to a # third one while the second is still running. @@ -1175,38 +1338,35 @@ release_singleton() { return 0 } -# Returns 2 when the cgroup has no memory limit and there is nothing to protect. -scan_once() { +# Returns 2 when the cgroup has no memory limit and there is nothing to measure +# against. +cycle_once() { local now=$1 rc read_cgroup_memory rc=$? if ((rc == 2)); then - log_action "memory.max is unlimited - nothing to protect, exiting" + log_action "memory.max is unlimited - nothing to budget against, exiting" return 2 fi ((rc == 0)) || return 1 - # Derived once, from the pod's own limit, on the first successful read. - if [[ -z $T_L4 ]]; then - derive_limits "$M_MAX" - log_action "derived memory.max=$(fmt_gib "$M_MAX") reserve=$(fmt_gib "$C_RESERVE") ladder L1=$(fmt_gib "$T_L1") L2=$(fmt_gib "$T_L2") L3=$(fmt_gib "$T_L3") L4=$(fmt_gib "$T_L4")" - local r - for r in serverMain extensionHost tsserver languageServer fileWatcher; do - log_action "derived ceiling role=${r} rlimit_data=$(fmt_gib "${CEILING[$r]}")" + + if ((${#BUDGET[@]} == 0)); then + derive_budgets "$M_MAX" + local role + for role in extensionHost serverMain tsserver languageServer fileWatcher extensionHelper claudeHelper; do + log_action "budget role=${role} pss=$(fmt_mib "${BUDGET[$role]}") (pod share $(fmt_mib "$((M_MAX / POD_SHARE_DEN))"), resting reference $(fmt_mib "${RESTING_ROLE[$role]:-0}"))" done - if ((TOO_SMALL)) && [[ $MODE == "enforce" ]]; then - MODE=observe - log_action "WARNING memory.max=$(fmt_gib "$M_MAX") leaves no room above L1=$(fmt_gib "$T_L1"); a pod this size would sit in the shedding tiers permanently, so enforce mode is refused and this run is observe-only" - fi + log_action "armed roles: $( + for role in "${!BUDGET[@]}"; do role_is_armed "$role" && printf '%s ' "$role"; done + printf '(mode=%s)' "$MODE" + )" fi - read_cgroup_pressure - read_process_table + read_cgroup_pressure ((M_H > H_MAX_SEEN)) && H_MAX_SEEN=$M_H ((H_MIN_SEEN == 0 || M_H < H_MIN_SEEN)) && H_MIN_SEEN=$M_H - # Rates. Elapsed time is tracked explicitly because the sample interval is - # adaptive, so a fixed denominator would be wrong exactly when it matters. local dt=$((now - PREV_AT)) ((dt > 0)) || dt=1 local refault_rate=0 pgscan_rate=0 du_rate=0 @@ -1215,89 +1375,50 @@ scan_once() { pgscan_rate=$(((M_PGSCAN_DIRECT - PREV_PGSCAN) / dt)) du_rate=$(((M_U - PREV_U) / dt)) fi - local projected=$((M_H - du_rate * PROJECTION_HORIZON)) - # Seconds until U reaches memory.max at the currently observed rate. Zero means - # "not growing", and is the normal reading. This is what chooses the next - # interval, and it is deliberately a raw single-interval rate rather than a - # smoothed one: smoothing is what would hide the only event shape that matters. - TIME_TO_LIMIT=0 - ((du_rate > 0)) && TIME_TO_LIMIT=$((M_H / du_rate)) + append_calibration "${now},${M_MAX},${M_CURRENT},${M_U},${M_H},${M_ANON},${M_SHMEM},${M_UNEVICTABLE},${M_SLAB_UNRECLAIMABLE},${M_SLAB_RECLAIMABLE},${M_KERNEL_STACK},${M_PAGETABLES},${M_SEC_PAGETABLES},${M_PERCPU},${M_SOCK},${M_FILE},${M_PSI_CENTI},${refault_rate},${pgscan_rate},${PRESSURE},${du_rate}" + publish_headroom "$M_H" "$PRESSURE" - build_server_tree - compute_protected - - decide_tier "$M_H" "$M_PSI_CENTI" "$refault_rate" "$projected" "$now" - publish_headroom "$M_H" "$TIER" - - local tree_rss=0 pid - for pid in "${!SERVER_TREE[@]}"; do - tree_rss=$((tree_rss + ${P_RSS[$pid]:-0})) - done - - local census - census="$(census_line)" - local n_protected=${census#*protected=} - n_protected=${n_protected%% *} - local n_eligible=${census##*eligible=} - - if ((CYCLE % CALIBRATION_EVERY == 0)); then - append_calibration "${now},${M_MAX},${M_CURRENT},${M_U},${M_H},${M_ANON},${M_SHMEM},${M_UNEVICTABLE},${M_SLAB_UNRECLAIMABLE},${M_SLAB_RECLAIMABLE},${M_KERNEL_STACK},${M_PAGETABLES},${M_SEC_PAGETABLES},${M_PERCPU},${M_SOCK},${M_FILE},${M_PSI_CENTI},${refault_rate},${pgscan_rate},${TIER},${SERVER_PID:--},${#SERVER_TREE[@]},${tree_rss},${n_protected},${n_eligible},${T_L1},${T_L4}" - fi - - [[ -n $SERVER_PID ]] && apply_ceilings - publish_summary "$now" "$M_H" "$TIER" - check_calibration - - if [[ $TIER != "L0" && $TIER != "$PREV_TIER" ]]; then - log_action "tier=${TIER} h=$(fmt_gib "$M_H") u=$(fmt_gib "$M_U") psi_full10=${M_PSI_CENTI} refault/s=${refault_rate} dU/s=${du_rate} projected=$(fmt_gib "$projected") ${census}${SUPPRESSED:+ suppressed=${SUPPRESSED}}" + if ((CYCLE % SWEEP_EVERY == 0)); then + sweep_once "$now" + ((SWEEPS += 1)) + check_visibility + publish_summary "$now" fi - case "$TIER" in - L2 | L3 | L4) shed_load "$TIER" "$now" ;; - esac - PREV_AT=$now PREV_U=$M_U PREV_REFAULT=$M_REFAULT_FILE PREV_PGSCAN=$M_PGSCAN_DIRECT - PREV_TIER=$TIER return 0 } -# The previous version of this check compared T_L1 against memory.max and warned -# when the ratio looked wrong - a hardcoded rule about hardcoded numbers, which -# could only ever restate the assumption it was meant to test. Now that the -# ladder is derived, the honest check is the observation itself: has this pod, in -# its own life, ever had enough headroom to sit above the first rung? If not, the -# derivation is wrong for this workload whatever the arithmetic says, and the -# operator should see that before switching enforce on rather than after. -CALIBRATION_WARMUP="${WATCHDOG_CALIBRATION_WARMUP:-30}" # cycles - -# How long to wait before the next scan. Rate first, tier second - see -# INTERVAL_FAST for the measurement that made that ordering necessary. -next_interval() { - if ((TIME_TO_LIMIT > 0 && TIME_TO_LIMIT < FAST_HORIZON)); then - printf '%s' "$INTERVAL_FAST" - elif [[ $PREV_TIER == "L0" ]]; then - printf '%s' "$INTERVAL_IDLE" - else - printf '%s' "$INTERVAL_BUSY" - fi -} - -check_calibration() { - ((CALIBRATION_WARNED)) && return 0 - ((CYCLE >= CALIBRATION_WARMUP)) || return 0 - CALIBRATION_WARNED=1 - if ((H_MAX_SEEN < T_L1)); then - log_action "WARNING best headroom seen since start is $(fmt_gib "$H_MAX_SEEN"), below L1=$(fmt_gib "$T_L1") on memory.max=$(fmt_gib "$M_MAX") - this pod never leaves the ladder, so recalibrate or resize before enabling enforce mode" - fi - return 0 +# Rotation counts lines written by *this* process, so a daemon that restarts +# every few hours would append to a file it believes is empty and the cap would +# never be reached. One fork each at startup fixes that; there is no way to ask +# bash for a file's size. +count_lines() { + local n=0 + [[ -r $1 ]] || { + printf '0' + return 0 + } + n=$(/usr/bin/wc -l <"$1" 2>/dev/null) + printf '%s' "${n:-0}" } main() { mkdir -p "$STATE_DIR" || exit 1 + LOG_LINES=$(count_lines "${STATE_DIR}/actions.log") + CSV_LINES=$(count_lines "${STATE_DIR}/calibration.csv") + SWEEP_LINES=$(count_lines "${STATE_DIR}/sweep.log") + + case "$MODE" in + observe | enforce | enforce-all) ;; + *) + MODE=observe + ;; + esac if ! acquire_singleton; then printf 'memory-watchdog: another instance is already running\n' >&2 @@ -1306,20 +1427,18 @@ main() { trap 'release_singleton; exit 0' HUP INT TERM trap release_singleton EXIT - local now interval + local now STARTED_AT=${WATCHDOG_NOW:-$EPOCHSECONDS} - log_action "started mode=${MODE} pid=$$ cgroup=${CGROUP_DIR}" + log_action "started mode=${MODE} pid=$$ cgroup=${CGROUP_DIR} sweep_every=$((SAMPLE_INTERVAL * SWEEP_EVERY))s dwell=${DWELL_SECONDS}s" while :; do now=${WATCHDOG_NOW:-$EPOCHSECONDS} - scan_once "$now" + cycle_once "$now" (($? == 2)) && break ((CYCLE += 1)) [[ $ONESHOT == "1" ]] && break - - interval=$(next_interval) - /usr/bin/sleep "$interval" + /usr/bin/sleep "$SAMPLE_INTERVAL" done return 0 } diff --git a/templates/kubernetes/homelab-workspace/scripts.tf b/templates/kubernetes/homelab-workspace/scripts.tf index e2f9b82e..fc6f3cb4 100644 --- a/templates/kubernetes/homelab-workspace/scripts.tf +++ b/templates/kubernetes/homelab-workspace/scripts.tf @@ -2,8 +2,10 @@ # is the coder agent - so coder_script is the only thing that can start a daemon # or run something on a schedule here. -# Starts the memory watchdog. See script-memory-watchdog.sh for why a userspace -# watchdog is the only option, and DESIGN.md for the constraint that forces it. +# Starts the memory watchdog, which bounds the standing population of +# restartable helper processes. See script-memory-watchdog.sh for what it does +# and does not attempt, and DESIGN.md for why the acute OOM half of its former +# job is not one a poll loop can do. # # setsid --fork detaches the watchdog from the agent's script runner, so this # resource completes immediately and start_blocks_login stays honest. The From e3b2fab121ff1eb18feee28c2aec5104137738f4 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Tue, 18 Aug 2026 00:24:21 +0000 Subject: [PATCH 2/3] docs: correct the OOM-blast-radius framing - singleProcessOOMKill is in effect memory.oom.group is fixed for a container when the kubelet creates it, so a long-lived workspace still reads 1 while a pod created after the rollout reads 0. Sampling only the former is what produced the wrong claim; both were re-checked, and a freshly created pod reads 0. The design is unchanged - a poll loop still cannot see a 43-second event, and drift policing is still the right job for one - but the acute case now rests on the kernel, with runway as the second line rather than the only one. That kubelet setting was applied to the nodes by hand and exists in no repository, so a rebuild or the Talos migration would silently restore all-or-nothing OOM behaviour. Raised as ppat/homelab-ops-kubernetes-clusters#948 and referenced from DESIGN.md, because this template's rationale now depends on it. --- DESIGN.md | 6 +++--- .../kubernetes/homelab-workspace/script-memory-watchdog.sh | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 9ce8d062..ce119b4a 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -71,9 +71,9 @@ The rule that ties the layers together: a package or tool belongs in the *lowest - *The prefix is `coder-workspace-`, not just `coder-`*, matching the `app.kubernetes.io/part-of` value already used in `main.tf`'s `common_labels`. A bare `coder-` prefix isn't enough to unambiguously mean "workspace": the same Kubernetes namespace also holds the `coder` control-plane Deployment itself and other `coder`-prefixed infra (e.g. a CloudNativePG cluster named `coder-db-`) that a naive `workload=~"coder-.+"` match would also catch. - *Renaming a workspace already relocates its home directory* (the `home` volume's `sub_path` is `data.coder_workspace.me.name`), so coupling the Deployment name to the workspace name too doesn't introduce a new class of rename hazard — it's already priced in. A rename recreates the Deployment (the pod restarts anyway) and needs a fresh `coder-workspace--` home subdirectory, exactly as it needed a fresh `sub_path` before this change. -**A userspace memory watchdog that bounds the standing population of helper processes.** The workspace pod has a hard memory limit and no way to enforce anything below it: `/sys/fs/cgroup` is mounted read-only, `cgroup.subtree_control` is empty, the cgroup namespace is private and the workspace user has no capabilities, so `memory.high` and a child cgroup both need `privileged: true` or a read-write host mount — exactly what *Unprivileged by default* above exists to prevent. `memory.oom.group` is `1`, so a cgroup OOM still kills every process in the container together, and `oom_score_adj` cannot influence a victim selection that no longer happens. +**A userspace memory watchdog that bounds the standing population of helper processes.** The workspace pod has a hard memory limit and no way to enforce anything below it: `/sys/fs/cgroup` is mounted read-only, `cgroup.subtree_control` is empty, the cgroup namespace is private and the workspace user has no capabilities, so `memory.high` and a child cgroup both need `privileged: true` or a read-write host mount — exactly what *Unprivileged by default* above exists to prevent. Until recently `memory.oom.group` was `1`, so a cgroup OOM killed every process in the container together and `oom_score_adj` could not influence a victim selection that no longer happened. The kubelet now sets `singleProcessOOMKill`, so a container created since that change gets `memory.oom.group = 0` and an OOM kills only the offending process — verified from inside a pod created after the rollout, while a container predating it still reads `1` because the value is fixed when the container is created. That setting lives on the nodes and in no repository (`ppat/homelab-ops-kubernetes-clusters#948`), so it is a property of node state rather than a guarantee this template can rely on. -[`script-memory-watchdog.sh`](templates/kubernetes/homelab-workspace/script-memory-watchdog.sh) does **not** try to prevent that OOM, and the earlier version of this section, which said it did, was wrong on the evidence. The recorded kills are spikes: 70–220 MB/s, idle to dead inside a minute, with an agent session or `node` named as the victim in the kernel log and never a VS Code process. A poll loop cannot win that race — a generic biggest-RSS killer beats the kernel only at a 0.3 s interval, loses at 0.5 s, and while `oom.group = 1` is killed by the very event it lost. The graded shedding ladder that used to live here was exercised against a real spike, climbed correctly, and logged `no-candidates`: the runaway was not in the tree it managed, and the whole editor tree it *could* have shed is ~0.7 GiB, or five seconds of that growth rate. The ladder has been removed rather than tuned, and the measurement it did well — unreclaimable memory, pressure, refault and reclaim rates — has been kept. +[`script-memory-watchdog.sh`](templates/kubernetes/homelab-workspace/script-memory-watchdog.sh) does **not** try to prevent that OOM, and the earlier version of this section, which said it did, was wrong on the evidence. The recorded kills are spikes: 70–220 MB/s, idle to dead inside a minute, with an agent session or `node` named as the victim in the kernel log and never a VS Code process. A poll loop cannot win that race — a generic biggest-RSS killer beats the kernel only at a 0.3 s interval, loses at 0.5 s, and under the `oom.group = 1` that then applied was killed by the very event it lost. The graded shedding ladder that used to live here was exercised against a real spike, climbed correctly, and logged `no-candidates`: the runaway was not in the tree it managed, and the whole editor tree it *could* have shed is ~0.7 GiB, or five seconds of that growth rate. The ladder has been removed rather than tuned, and the measurement it did well — unreclaimable memory, pressure, refault and reclaim rates — has been kept. What a poll loop is good at is growth measured in MB per *minute*, and that drift is the real, daily problem: the operator was policing it by hand for months, repeatedly killing VS Code to save agent sessions, with a python MCP server holding 1.66 GB at one of the kills. So the watchdog now keeps the resting population of **restartable helpers** inside per-role budgets. The goal is runway rather than rescue: when a spike does arrive, it starts from as much free memory as the pod can offer. Every process it may signal has a supervisor — VS Code respawns its own forks and language servers, an agent session respawns its MCP servers — so a wrong kill costs a reload, not a session, and that asymmetry is what licenses being aggressive. @@ -85,7 +85,7 @@ Two things then keep aggression from becoming harm. A process must be over budge **It records every sweep, and that is a deliverable rather than decoration.** Each per-process sweep is appended to a rotating local log with role, PSS, RSS, age, budget, how long the process has been over it, which guard claimed it, and a stable identity breadcrumb — the MCP server's module name rather than `python3` — with secrets in argv redacted at the point of writing. Processes it does *not* manage are recorded too, because on the evidence of every OOM in this investigation that is where the memory actually was. The previous version computed this table on every cycle and discarded it, which is why every post-mortem here has been unanswerable. There is no metrics path out of the pod today (the cluster's log agent tails container stdout, which this file is not), so a durable local log is the floor and metrics are a later, additive question. -The trade is unchanged in shape: this is a userspace daemon in a pod with no supervisor, doing crudely what the kernel would do properly if it were allowed to, and it is built to be deleted in one step if that ever changes. It measures deliberately against every stock reading, including Coder's own — page cache and reclaimable slab make this pod look near death while it is idle — so the honest number is published beside the misleading one in the workspace UI rather than replacing it, next to the largest helper as a share of its budget. What remains unaddressed is the acute spike, and honestly so: that is the kernel's job, and it would do it far better with `singleProcessOOMKill` enabled at the kubelet, which was checked while writing this and is **not** in effect — `memory.oom.group` reads `1` on both a long-lived workspace and a pod created minutes earlier. +The trade is unchanged in shape: this is a userspace daemon in a pod with no supervisor, doing crudely what the kernel would do properly if it were allowed to, and it is built to be deleted in one step if that ever changes. It measures deliberately against every stock reading, including Coder's own — page cache and reclaimable slab make this pod look near death while it is idle — so the honest number is published beside the misleading one in the workspace UI rather than replacing it, next to the largest helper as a share of its budget. What remains unaddressed is the acute spike, and honestly so: that is the kernel's job, and with `singleProcessOOMKill` now set at the kubelet it does it properly — the runaway dies and the container survives. Runway is the second line of defence, not the first, and it is the line that matters if the first is ever lost: that kubelet setting is hand-applied node state rather than code, and if a rebuild or the Talos migration drops it the blast radius silently returns to all-or-nothing with nothing to say why. ## Outcomes targeted diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh index c7137b6b..7f87267f 100644 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh @@ -10,8 +10,11 @@ # workspace was a spike - 70 to 220 MB/s, idle to dead inside a minute - and the # victims named in the kernel log were agent sessions and node, never a VS Code # process. A poll loop cannot win that race: a generic biggest-RSS killer only -# beats the kernel at a 0.3s interval, loses at 0.5s, and while -# memory.oom.group=1 it is killed by the very event it lost to. The graded +# beats the kernel at a 0.3s interval, loses at 0.5s, and under the +# memory.oom.group=1 that applied at the time was killed by the very event it +# lost to. The kubelet now sets singleProcessOOMKill, so a container created +# since reads memory.oom.group=0 and an OOM takes the offending process only - +# which is the acute job done properly, by the layer that can actually do it. The graded # shedding ladder that used to live here climbed correctly during a live spike # and then logged `no-candidates`, because the runaway was not in the tree it # managed. That ladder has been removed rather than tuned. From 7cf69609f374eecad9e04b30e59b9e1b6a1eca5b Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Tue, 18 Aug 2026 01:10:08 +0000 Subject: [PATCH 3/3] feat: emit action lines to container stdout, and re-anchor budgets on resting size Two changes, both from measurement. **Action lines now leave the pod.** The cluster's log agent tails container stdout only, but PID 1 in this container is the coder agent, so /proc/1/fd/1 is that stdout: writing there needs no new infrastructure and no configuration anywhere, and the lines arrive in Loki labelled by namespace, pod and container. Only actions take that route - kills, refusals, disarms, the budgets in force, and one census line an hour. The per-process sweep stays in the local file, and a test asserts that it does: dozens of rows a minute do not belong in a log pipeline. Every line is logfmt beginning with component=memory-watchdog, so `|= "component=memory-watchdog" | logfmt` works against a very chatty stream without adding a stream label; free text is quoted into detail= by record_action rather than at each call site. The write is best-effort - one failure disables the path, records why once locally, and changes nothing else. Observe mode emits what it would have done, which is the evidence needed before arming this anywhere. **Budgets are re-anchored on resting rather than fresh measurements.** A fresh extension host is 471 MB PSS; the same process on a reconnected, idle 8 GiB workspace holds 713 MB, with the tree at 1093 MB rather than the 727 MB this design cited. "Calibrated against fresh, deployed against resting" is the error that produced a file-watcher ceiling below what an idle file watcher held, and it had crept back in. RESTING_ROLE now carries resting figures, which lifts the extension host to 1069 MiB on both pod sizes via the existing floor rule, and the derivation reports which budgets the floor lifted (floored=1) - that means the pod is too small to bound the role at its intended share, which is worth saying rather than hiding. Also: the fixture suite now refuses to run unless its stdout seam is set. The first run after the stdout path was added wrote fixture kill lines into the live workspace's container log and thus into Loki, describing kills that never happened in the format a post-mortem would trust. Nothing was signalled (kill is shadowed there), a correcting note was appended to the same stream, and a test fixture that can write to production telemetry is now a hard failure. Verified live from the test workspace: the daemon's own census and budget lines appear in Loki and parse into 18 logfmt fields, with budget_mb=1069 floored=1 for the extension host against a 512 MiB pod share. --- CLAUDE.md | 6 +- DESIGN.md | 6 +- TESTING.md | 2 +- .../script-memory-watchdog-test.sh | 170 ++++++++++++++++-- .../script-memory-watchdog.sh | 141 ++++++++++++--- 5 files changed, 282 insertions(+), 43 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6f5bb2c0..5df8cadd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -90,13 +90,15 @@ Things that look arbitrary in the code but are load-bearing (full reasoning in [ - `parameters.tf`'s `local.validated_*` allowlist is the only thing stopping `system_packages`/`preferred_nodes` from injecting shell metacharacters into the init container — any new parameter whose value reaches a shell must go through the same validate-then-use step. - `script-memory-watchdog.sh` computes headroom as `memory.max − U`, where `U` sums only the *unreclaimable* fields of `memory.stat` (`anon`, `shmem`, `unevictable`, `slab_unreclaimable`, `kernel_stack`, `pagetables`, `sec_pagetables`, `percpu`, `sock`). Do not "simplify" it to `memory.current` or to `memory.stat`'s `kernel` roll-up: on the live pod those read 96% and 42% of the limit while true `U` is 28%. Nothing acts on this number any more — it is pod-level context for the per-process rows and the honest figure published in the workspace UI. - **The watchdog is a drift policer, not an OOM preventer, and the difference is measured.** The graded L1–L4 shedding ladder that used to be here was removed, not tuned: the recorded kills are 70–220 MB/s spikes that go from idle to dead inside a minute, a live reproduction climbed the ladder correctly and logged `no-candidates` because the runaway was not in the tree it managed, and the entire editor tree it could shed is ~0.7 GiB — five seconds of that growth. Before re-adding anything reactive, establish that a poll loop can see the event at all. What the loop *is* good at is MB-per-minute growth in the standing population, which is what it now does. -- **Budgets are per role, in PSS, and never below a role's measured resting size.** Fresh-tree measurements: extension host 471 MB PSS, serverMain 160 MB, ptyHost 36 MB, file watcher 34 MB. A uniform 512 MB budget would sit 40 MB above where the extension host starts (and 256 MB below its floor outright), which is the same class of defect as the earlier `RLIMIT_DATA` ceiling that landed *below* what an idle file watcher already held. Each budget is `max(min(role budget, memory.max / 8), resting × 1.5)`, so the pod share can never push a budget under what the role demonstrably needs, and the tests assert that property at every pod size rather than asserting the arithmetic. Override one role with `WATCHDOG_BUDGET_`; PSS (`smaps_rollup`) is the comparison, not RSS and not `VmData`. +- **Budgets are per role, in PSS, and never below a role's measured *resting* size — not its fresh size.** Fresh tree: extension host 471 MB PSS, serverMain 160 MB, ptyHost 36 MB, file watcher 34 MB. Resting tree (reconnected, idle, 8 GiB pod): extension host **713 MB**, file watcher 88 MB, language server 54 MB, tree total 1093 MB. A uniform 512 MB budget would sit 40 MB above where the extension host *starts* and 200 MB below where it *lives*, which is the same class of defect as the earlier `RLIMIT_DATA` ceiling that landed below what an idle file watcher already held — "calibrated against fresh, deployed against resting" has now appeared twice in this design, so `RESTING_ROLE` holds the resting figures and nothing else. Each budget is `max(min(role budget, memory.max / 8), resting × 1.5)`; on the 8 GiB pod that makes the extension host 1069 MiB rather than the 1024 MiB share, and the derivation records which budgets the floor lifted (`floored=1`), because that means the pod is too small to bound the role at its intended share. The tests assert the *property* — no budget at or below resting, at any pod size — rather than the arithmetic. Override one role with `WATCHDOG_BUDGET_`; PSS (`smaps_rollup`) is the comparison, not RSS and not `VmData`. - **A kill needs ten minutes of continuous over-budget dwell, and three kills of one role inside an hour disarm that role.** The dwell is what separates drift from load — a language server that balloons while indexing and hands the memory back must survive. The breaker is what stops the failure that would make this actively harmful: kill the extension host → VS Code restarts it → it reloads every extension → it exceeds again → kill, a loop that arrives looking exactly like the watchdog working. It disarms and reports rather than widening its own budget, because a mechanism that raises the limit it is enforcing has stopped enforcing. - **The watchdog decides what is a VS Code process by executable path — `argv[0]` under `~/.vscode-server/` — never by whether something "is node".** A provisioned workspace has two unrelated node installations: VS Code's bundled one under `~/.vscode-server/cli/servers/Stable-/server/`, and mise's on `PATH`, which is what repo tooling and the operator's agent sessions run on. (There is no `/usr/bin/node`, and nothing named `node` on `PATH` at all without dotfiles.) `comm` is `MainThread` for every node process in a real tree, never `node`, because V8 renames its main thread; nothing may key off it. `script-memory-watchdog-test.sh` asserts this three ways, each paired with the mutation that flips it. - **Helpers spawned by an agent session are the second policed population, and the walk that finds them stops at two boundaries: a shell, and a change of session id.** MCP servers live nowhere near `~/.vscode-server`, so the tree-scoped selection could not see them — the largest single offender ever measured was 1.66 GB of python. The session rule is measured, not assumed: on the live workspace a session root has `sid` = its login shell's session, while every Bash tool call has `pgid == sid == its own pid`, because Claude Code detaches each one. That is what keeps an in-flight build out of the policed set even when the tool call's shell has exec'd itself away, which the shell test alone would miss. If Claude Code ever stops detaching, the walk polices *nothing* rather than the wrong thing, and the visibility warning in `actions.log` says so. - **Identity guards are absolute; the two positional rules are not, and the distinction is deliberate.** `pid 1`, the coder agent, tmux, `claude` session roots, agent payloads and the watchdog's own kin may never be signalled by anything. The ptyHost subtree and "does not run VS Code's own binary" bound the *editor* selection only, so that an MCP server is policed the same whether its session was started with `coder ssh` or in a VS Code terminal — sparing one set because of which terminal it came from would make the mechanism miss half its cases silently. Everything the ptyHost rule genuinely protects (shells, multiplexers, sessions, tool calls) is still covered by identity guards and by the shell/session boundaries. - **The never-signal guards match `comm`, `argv[0]`'s basename, and whole path segments of argv elements — never a substring of the joined command line.** Loose substrings over-matched twice: `*/claude*` protected an unrelated process because a scratchpad path contained `/claude`, and `*memory-watchdog*` protected *every* process in a test harness because the harness's own directory path contained it, leaving two full runs green while asserting nothing. The watchdog's own identity is structural — its pid, ancestors and descendants — rather than a name at all. Each guard records which rule claimed a process, and the tests assert every rule is individually reachable; a guard nothing can trigger is untested, not correct. -- **The per-process sweep log is a deliverable, not decoration.** `~/.local/state/vscode-memory-watchdog/sweep.log` (plus `sweep.latest`, `summary`, `headroom`, `top`, `calibration.csv`, `actions.log`) records every policed process and every unmanaged one above 32 MB, with a stable identity breadcrumb (an MCP server's module, not `python3`) and argv secrets redacted at the point of writing. The previous version computed this table every cycle and threw it away, which is why every post-mortem in this investigation was unanswerable. There is no metrics path out of the pod today — the cluster's log agent tails container stdout, which these files are not — so do not delete the local log on the assumption that Prometheus has it. +- **The per-process sweep log is a deliverable, not decoration.** `~/.local/state/vscode-memory-watchdog/sweep.log` (plus `sweep.latest`, `summary`, `headroom`, `top`, `calibration.csv`, `actions.log`) records every policed process and every unmanaged one above 32 MB, with a stable identity breadcrumb (an MCP server's module, not `python3`) and argv secrets redacted at the point of writing. The previous version computed this table every cycle and threw it away, which is why every post-mortem in this investigation was unanswerable. +- **Action lines also go to `/proc/1/fd/1`, which is the container's stdout, because PID 1 here is the coder agent.** That is the only route out of this pod — the cluster's log agent tails container stdout and nothing else — and it needs no configuration anywhere: the lines land in Loki labelled by namespace, pod and container. **Only actions** take that route (kills, refusals, disarms, the budgets in force, one census line an hour); the sweep stays local, and a test asserts that it does, because dozens of rows a minute do not belong in a log pipeline. Lines are logfmt starting with `component=memory-watchdog` so `|= "component=memory-watchdog" | logfmt` works against a very chatty stream without adding a stream label, free text is quoted into `detail=` by `record_action` rather than at call sites, and the write is best-effort — one failure disables the path, logs why once, and nothing else changes. +- **A test fixture that can write to `/proc/1/fd/1` writes into production telemetry.** This happened: the first run of the suite after the stdout path was added — before the harness set the seam — put fixture `event=kill role=extensionHost pss_mb=1907` lines into the live workspace's container log and thus into Loki, describing kills that never occurred in the exact format a post-mortem would trust. `load_watchdog` now sets `WATCHDOG_STDOUT_PATH` and the suite **exits** rather than continuing if the seam did not take. Any future test seam that fans out to a real sink deserves the same treatment. - Watchdog state keys — the dwell clock and the SIGTERM/SIGKILL escalation — are keyed on `pid:starttime`, never on pid alone. A recycled pid must not inherit another process's history and be killed for it. - `parameters.tf`'s `memory_watchdog_mode` also goes through the `local.validated_*` treatment: Coder constrains the value server-side, but it is the single switch deciding whether the watchdog may signal processes, so an unrecognised value falls back to the inert `observe` rather than being passed through. - Adding a package/tool has three possible homes, and picking the wrong one is a real mistake, not a style choice — route by the rule in [DESIGN.md](DESIGN.md#where-the-workspace-environment-comes-from): universal + stable → image (`Dockerfile`); occasionally-needed + apt-only + too heavy to bake in → the template's `system_packages` parameter; personal, fast-moving, or not an apt package → the operator's dotfiles (a *different* repo — see below), never this one. diff --git a/DESIGN.md b/DESIGN.md index ce119b4a..ff866cf3 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -77,13 +77,15 @@ The rule that ties the layers together: a package or tool belongs in the *lowest What a poll loop is good at is growth measured in MB per *minute*, and that drift is the real, daily problem: the operator was policing it by hand for months, repeatedly killing VS Code to save agent sessions, with a python MCP server holding 1.66 GB at one of the kills. So the watchdog now keeps the resting population of **restartable helpers** inside per-role budgets. The goal is runway rather than rescue: when a spike does arrive, it starts from as much free memory as the pod can offer. Every process it may signal has a supervisor — VS Code respawns its own forks and language servers, an agent session respawns its MCP servers — so a wrong kill costs a reload, not a session, and that asymmetry is what licenses being aggressive. -**Budgets are per role and anchored on measurement, because a uniform number is wrong in both directions.** On a fresh tree the extension host is already 471 MB PSS, serverMain 160 MB and the file watcher 34 MB, so a single 512 MB budget would sit 40 MB above where the extension host starts and below where it spends its life, while being far too generous for a file watcher. Each role therefore gets a budget clamped between a share of the pod's own `memory.max` and one-and-a-half times its measured resting size, and the resting floor wins when they conflict — a budget below resting usage is not a conservative budget, it is a kill loop written down, which is the defect that reached enforce-readiness twice under the previous design. PSS is the quantity compared, not RSS and not `VmData`: `VmData` was only ever relevant because `RLIMIT_DATA` accounts it, and on a V8 process it runs an order of magnitude above what the pod actually pays. +**Budgets are per role and anchored on measurement, because a uniform number is wrong in both directions.** A *fresh* extension host is 471 MB PSS, so a single 512 MB budget would sit 40 MB above where it starts while being far too generous for a 34 MB file watcher. Fresh is also the wrong baseline: measured on a reconnected, idle 8 GiB workspace, the same extension host holds **713 MB** and the tree around it 1093 MB rather than the 727 MB originally cited. "Calibrated against fresh, deployed against resting" is precisely the error that produced a file-watcher ceiling below what an idle file watcher already held, so the reference for each role is now the largest figure measured at rest. Each role gets a budget clamped between a share of the pod's own `memory.max` and one-and-a-half times that reference, and the resting floor wins when they conflict — a budget below resting usage is not a conservative budget, it is a kill loop written down. On the 8 GiB pod the floor is what sets the extension host's budget at 1069 MiB rather than the 1024 MiB share, and the watchdog reports which budgets were lifted that way, because the floor winning means the pod is too small to bound that role at the share it was meant to have. PSS is the quantity compared, not RSS and not `VmData`: `VmData` was only ever relevant because `RLIMIT_DATA` accounts it, and on a V8 process it runs an order of magnitude above what the pod actually pays. Two things then keep aggression from becoming harm. A process must be over budget **continuously for ten minutes** before anything happens to it, so a language server that balloons while indexing and hands the memory back is treated as load rather than drift. And a role that has to be killed three times inside an hour is not drifting — its budget is wrong for this workload — so the watchdog **disarms itself for that role**, logs why, and leaves the number to a human. It never widens its own budget: a mechanism that quietly raises the limit it enforces is a mechanism that has stopped enforcing. The kill loop, not the wrong kill, is the failure mode that would make this actively harmful, and it arrives looking exactly like the watchdog working. **What it may touch is decided structurally, and there are two populations rather than one.** Inside the VS Code tree, only a process whose own binary lives under `~/.vscode-server` counts as the editor's: a provisioned workspace carries two unrelated node installations — VS Code's bundled one and the operator's from mise — and a rule that asked "is this node" instead of "whose binary is this" would classify an agent session spawned by an extension as a sheddable helper. The second population is what an agent session spawned directly, chiefly MCP servers, which live nowhere near `~/.vscode-server` and which the tree-scoped selection could not see at all. That walk descends from each session root and stops at two boundaries: any shell or multiplexer, and any change of session id. The second is measured, not assumed — on the live workspace a session root has `sid` equal to its login shell's session while every Bash tool call has `pgid == sid == its own pid`, because Claude Code detaches each one — and it is what keeps an in-flight build out of the policed set even when the tool call's shell has exec'd itself away. Identity guards (`pid 1`, the agent, tmux, the session roots themselves, agent payloads, the watchdog's own kin) are absolute; the two *positional* rules — the pty host's subtree, and "does not run VS Code's binary" — bound the editor selection only, so that an MCP server is treated the same whether its session was started with `coder ssh` or in a VS Code terminal. -**It records every sweep, and that is a deliverable rather than decoration.** Each per-process sweep is appended to a rotating local log with role, PSS, RSS, age, budget, how long the process has been over it, which guard claimed it, and a stable identity breadcrumb — the MCP server's module name rather than `python3` — with secrets in argv redacted at the point of writing. Processes it does *not* manage are recorded too, because on the evidence of every OOM in this investigation that is where the memory actually was. The previous version computed this table on every cycle and discarded it, which is why every post-mortem here has been unanswerable. There is no metrics path out of the pod today (the cluster's log agent tails container stdout, which this file is not), so a durable local log is the floor and metrics are a later, additive question. +**It records every sweep, and that is a deliverable rather than decoration.** Each per-process sweep is appended to a rotating local log with role, PSS, RSS, age, budget, how long the process has been over it, which guard claimed it, and a stable identity breadcrumb — the MCP server's module name rather than `python3` — with secrets in argv redacted at the point of writing. Processes it does *not* manage are recorded too, because on the evidence of every OOM in this investigation that is where the memory actually was. The previous version computed this table on every cycle and discarded it, which is why every post-mortem here has been unanswerable. + +**Its actions also leave the pod, at no cost.** The cluster's log agent tails container stdout and nothing else, so no file here can reach Loki — but PID 1 in this container is the coder agent, which makes `/proc/1/fd/1` the container's stdout. Writing action lines there needs no new infrastructure and no configuration anywhere, and they arrive already labelled by namespace, pod and container. Only actions go that way — kills, refusals, disarms, the budgets in force, and one census line an hour; the sweep is dozens of rows a minute and stays local, which is a limit worth defending rather than a temporary compromise. Every line is logfmt beginning with `component=memory-watchdog`, so a LogQL line filter and the `logfmt` parser both work on it without anyone adding a stream label to a chatty stream, and free text is quoted at the point of writing so a sentence cannot parse into five bogus fields. The write is best-effort in both directions: a fd that cannot be written disables the path, says so once locally, and changes nothing else, because a watchdog that died because logging failed would be a worse bug than any it prevents. Observe mode emits what it *would* have done, which is the evidence anyone needs before arming this on a live workspace. The trade is unchanged in shape: this is a userspace daemon in a pod with no supervisor, doing crudely what the kernel would do properly if it were allowed to, and it is built to be deleted in one step if that ever changes. It measures deliberately against every stock reading, including Coder's own — page cache and reclaimable slab make this pod look near death while it is idle — so the honest number is published beside the misleading one in the workspace UI rather than replacing it, next to the largest helper as a share of its budget. What remains unaddressed is the acute spike, and honestly so: that is the kernel's job, and with `singleProcessOOMKill` now set at the kubelet it does it properly — the runaway dies and the container survives. Runway is the second line of defence, not the first, and it is the line that matters if the first is ever lost: that kubelet setting is hand-applied node state rather than code, and if a rebuild or the Talos migration drops it the blast radius silently returns to all-or-nothing with nothing to say why. diff --git a/TESTING.md b/TESTING.md index c8ce683f..b3d261f4 100644 --- a/TESTING.md +++ b/TESTING.md @@ -27,7 +27,7 @@ Because all three stages run for real in dry-run — just scoped away from produ Everything else here is declarative and is covered by the stages above. `script-memory-watchdog.sh` decides at runtime whether to kill a process, so it gets two things neither lint nor a template push can provide: -- **Fixtures** — `./templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh`, also run by the `watchdog` job in `.github/workflows/test.yaml`. The suite is built around negative assertions paired with the mutation that must flip them: "it did not kill the agent session" proves nothing unless removing one rule makes it kill the agent session. `kill` is shadowed by a function throughout, because the fixture pids name real processes in whatever container runs the suite. +- **Fixtures** — `./templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh`, also run by the `watchdog` job in `.github/workflows/test.yaml`. The suite is built around negative assertions paired with the mutation that must flip them: "it did not kill the agent session" proves nothing unless removing one rule makes it kill the agent session. Two of its own safety properties matter as much as its assertions: `kill` is shadowed by a function throughout, because the fixture pids name real processes in whatever container runs the suite; and every load of the watchdog redirects its stdout emission to a temporary file, with the suite exiting outright if that seam did not take — the real default is the container's own stdout, and a run without the seam has already put fixture kill lines into a live workspace's log stream. - **A live drill on the `test` workspace**, which has disposable storage and can be wrecked freely. Fixtures cannot answer whether a real process tree classifies correctly, whether a supervisor really does respawn what was killed, or whether the circuit breaker stops a loop rather than joining it. The drill that has been run: a stand-in session root with an over-budget helper that a supervisor respawns, a second helper inside its budget, and a detached tool call larger than both. In `enforce` mode with the dwell shortened, the watchdog killed the drifted helper three times, disarmed that role on the third with the loop message, and left the fourth incarnation, the tool call, and the session roots untouched. Neither replaces the other, and the live one is where every defect that mattered in this component has been found. diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh index cebd2512..7acfdaed 100755 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh @@ -350,14 +350,38 @@ add_second_server() { "${sdir}/out/bootstrap-fork" --type=fileWatcher } +# Always set explicitly, and never left at its default: the real default is +# /proc/1/fd/1, i.e. the container's own stdout. +# +# This is not hypothetical. The first run of this suite after the stdout path was +# added - before load_watchdog set the seam - wrote its fixture kill lines into +# the live workspace's container log, where they reached Loki labelled as that +# workspace and reading `event=kill role=extensionHost pss_mb=1907`. Nothing was +# signalled (kill is shadowed), but for as long as those lines are retained they +# describe kills that never happened, on a real workspace, in the exact format a +# post-mortem would trust. A test fixture that can write into production +# telemetry is a defect in the test, so the seam is asserted below rather than +# merely set. +STDOUT_FILE="" + load_watchdog() { local mode=${3:-observe} + STDOUT_FILE="${WORK}/stdout.log" + : >"$STDOUT_FILE" WATCHDOG_SOURCE_ONLY=1 \ WATCHDOG_CGROUP_DIR="$1" \ WATCHDOG_PROC_DIR="$2" \ WATCHDOG_STATE_DIR="${WORK}/state" \ + WATCHDOG_STDOUT_PATH="$STDOUT_FILE" \ WATCHDOG_MODE="$mode" \ . "${SELF_DIR}/script-memory-watchdog.sh" + # Not an assertion - a refusal. If the seam ever fails to take, every later + # test in this file writes fixture actions to the container's real stdout. + if [[ ${STDOUT_PATH:-} != "$STDOUT_FILE" ]]; then + printf 'FATAL: the stdout seam did not take (STDOUT_PATH=%s); refusing to run\n' \ + "${STDOUT_PATH:-unset}" >&2 + exit 1 + fi mkdir -p "${WORK}/state" SIGNALS="" # Budgets are derived from memory.max on the first cycle rather than being @@ -839,22 +863,46 @@ test_budgets() { load_watchdog "${WORK}/cg" "${WORK}/proc2" # 8 GiB: the pod the roles were measured on. The operator's 512 MiB instinct - # applies unchanged to the helpers; the extension host gets twice its measured - # resting size instead, because 512 MiB is 40 MiB above where it starts. + # applies unchanged to the helpers. The extension host does not get it, and + # does not even get the pod share: measured at rest on a reconnected, idle + # 8 GiB workspace it holds 713 MB, so the resting floor lifts it above the + # 1024 MiB share. That is the correction this table exists to survive - the + # first version of these numbers was calibrated against a *fresh* tree, where + # the same process is 471 MB. budgets_at 8589934592 - assert_eq 1073741824 "${BUDGET[extensionHost]}" "8 GiB: the extension host gets 1 GiB" + assert_eq 1121452032 "${BUDGET[extensionHost]}" \ + "8 GiB: the extension host is lifted above the pod share by its resting size" + assert_eq 1 "${FLOORED[extensionHost]}" "and that lift is recorded, not silent" assert_eq 536870912 "${BUDGET[serverMain]}" "8 GiB: serverMain gets 512 MiB" assert_eq 268435456 "${BUDGET[fileWatcher]}" "8 GiB: the file watcher gets 256 MiB" + assert_eq "" "${FLOORED[fileWatcher]:-}" "and is not floored - 88 MB resting is well under it" assert_eq 536870912 "${BUDGET[claudeHelper]}" "8 GiB: an MCP server gets 512 MiB" - # 4 GiB: the pod share would give the extension host 512 MiB, which is above - # its resting 471 MB by 40 MB - i.e. a kill loop. The resting floor overrides - # it. This is the assertion that would have caught both historical defects. + # 4 GiB: the pod share is 512 MiB, far below what the extension host holds at + # rest. The floor overrides it rather than issuing a standing kill order. budgets_at 4294967296 - assert_eq 740818944 "${BUDGET[extensionHost]}" \ + assert_eq 1121452032 "${BUDGET[extensionHost]}" \ "4 GiB: the resting floor overrides the pod share for the extension host" assert_eq 268435456 "${BUDGET[fileWatcher]}" "4 GiB: the file watcher is unaffected" + # The property that matters more than any of the numbers: no budget may sit + # below the size the role was measured at while idle, at any pod size, ever. + local m r + for m in 2147483648 4294967296 8589934592; do + budgets_at $m + for r in "${!RESTING_ROLE[@]}"; do + # shellcheck disable=SC2004 + # The $ is NOT unnecessary here: these are associative arrays, whose + # subscripts are strings inside (( )), so dropping it looks up the literal + # key "r" and silently reads 0. That defect shipped once already. + if ((BUDGET[$r] > RESTING_ROLE[$r])); then + ok "memory.max=${m}: ${r} is budgeted above its resting size" + else + bad "memory.max=${m}: ${r} budget ${BUDGET[$r]} is at or below resting ${RESTING_ROLE[$r]}" + fi + done + done + local max role budget resting for max in 2147483648 4294967296 8589934592 17179869184; do budgets_at "$max" @@ -1014,7 +1062,7 @@ test_kill_loop_breaker() { else bad "the role kept being killed - there is no circuit breaker" fi - assert_contains "$(cat "${WORK}/state/actions.log")" "DISARMED role=claudeHelper" \ + assert_contains "$(cat "${WORK}/state/actions.log")" "event=disarmed role=claudeHelper" \ "the breaker says so in the log rather than going quiet" # And it stays disarmed: the next incarnation is watched, reported, and left @@ -1082,9 +1130,9 @@ test_observe_mode() { assert_eq "" "$SIGNALS" "observe mode signals nothing" local log log="$(cat "${WORK}/state/actions.log")" - assert_contains "$log" "[observe] would-kill pid=61" \ + assert_contains "$log" "event=would-kill armed=no pid=61" \ "but it logs the kill it would have made" - assert_contains "$log" "armed=no" "and records that the role was not armed" + assert_contains "$log" "mode=observe" "and records the mode it was in" assert_eq 0 "${KILLS[claudeHelper]:-0}" "a kill it did not make is not counted" assert_contains "$(cat "${WORK}/state/sweep.latest")" "would-kill" \ "the sweep row says would-kill rather than killed" @@ -1171,7 +1219,7 @@ test_sweep_log() { local summary summary="$(cat "${WORK}/state/summary")" assert_contains "$summary" "claudeHelper=512M" "the summary prints real budgets" - assert_contains "$summary" "extensionHost=1024M" "including the derived ones" + assert_contains "$summary" "extensionHost=1069M" "including the ones the resting floor lifted" assert_contains "$summary" "policed=" "and the size of the policed set" # The visibility check: a watchdog that manages nothing looks exactly like a @@ -1185,10 +1233,107 @@ test_sweep_log() { SWEEPS=$VISIBILITY_WARMUP sweep_at 1000 check_visibility - assert_contains "$(cat "${WORK}/state/actions.log")" "WARNING no process has been policed" \ + assert_contains "$(cat "${WORK}/state/actions.log")" "event=warning reason=nothing-policed" \ "an empty policed set is reported rather than passing for health" } +# --------------------------------------------------------------------------- # +# 6b. the action lines reach the container's stdout +# +# The cluster's log agent tails container stdout and nothing else, and PID 1 in +# the workspace container is the coder agent - so /proc/1/fd/1 is the only route +# out of this pod, and it is free. What goes through it is deliberately limited +# to actions: the sweep is dozens of rows a minute and belongs in the local file. +# --------------------------------------------------------------------------- # + +test_stdout_emission() { + printf 'action lines reach container stdout\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + local pdir="${WORK}/proc6c" + build_tree "$pdir" + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" enforce + + # Driven through cycle_once rather than sweep_once, because the budget and + # census lines are emitted by the cycle and a test that only swept would be + # asserting on half the output. + # shellcheck disable=SC2034 # globals of the sourced watchdog + SWEEP_EVERY=1 + # Emptied so the first cycle derives its budgets exactly as the daemon does at + # startup, and emits them. + BUDGET=() + local t=1000 + cycle_once $t + # shellcheck disable=SC2034 # a global of the sourced watchdog + CYCLE=1 + t=$((t + DWELL_SECONDS)) + cycle_once $t + + local out + out="$(cat "$STDOUT_FILE")" + assert_contains "$out" "event=kill sig=TERM" "a kill is emitted to stdout" + assert_contains "$out" "event=budget role=" "so are the budgets in force" + assert_contains "$out" "floored=1" "including which of them the resting floor lifted" + assert_contains "$out" "event=census" "and a census line" + assert_contains "$out" "id=homelab_mcp.server" "with the identity breadcrumb, not just a pid" + assert_contains "$out" "top_role=" "and the census carries the standing population's largest member" + + # Queryable without anyone adding a stream label: every line is logfmt and + # starts with the same key, so `|= "component=memory-watchdog" | logfmt` works. + local line n=0 bad_lines=0 + while IFS= read -r line; do + [[ -z $line ]] && continue + n=$((n + 1)) + [[ $line == "component=memory-watchdog time="* ]] || bad_lines=$((bad_lines + 1)) + [[ $line == *" event="* ]] || bad_lines=$((bad_lines + 1)) + done <"$STDOUT_FILE" + assert_eq 0 "$bad_lines" "every emitted line is logfmt with component and event first (${n} lines)" + + # Free text is quoted, so a sentence in detail= cannot become five bogus keys. + record_action "event=warning reason=test detail=@a sentence with spaces in it@" + out="$(cat "$STDOUT_FILE")" + assert_contains "$out" 'detail="a sentence with spaces in it"' \ + "free text is quoted rather than spilling into the parse" + assert_absent "$out" "detail=a sentence" "an unquoted sentence never reaches the line" + + # The volume rule: the sweep stays local. This is the assertion that stops a + # later change from putting forty thousand rows a day into the log pipeline. + assert_absent "$out" " unmanaged " "sweep rows are not emitted to stdout" + assert_absent "$out" "pss_kb" "nor the sweep header" + assert_contains "$(cat "${WORK}/state/sweep.log")" " unmanaged " "they are in the local sweep log" + + # Observe mode emits what it *would* have done, which is the data anyone needs + # before arming this on a live workspace. + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" observe + t=5000 + sweep_at $t + t=$((t + DWELL_SECONDS)) + sweep_at $t + out="$(cat "$STDOUT_FILE")" + assert_contains "$out" "event=would-kill armed=no" "observe mode emits the kill it would have made" + assert_contains "$out" "mode=observe" "labelled with the mode, so the two cannot be confused" + assert_absent "$out" "event=kill " "and never emits a kill it did not make" + + # Best effort, always. The fd may not be writable in some contexts, and a + # watchdog that died because logging failed would be worse than any bug it + # prevents. + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" enforce + # shellcheck disable=SC2034 # a global of the sourced watchdog + STDOUT_PATH="${WORK}/no-such-dir/stdout.log" + record_action "event=kill sig=TERM pid=1234 role=claudeHelper" + assert_eq 0 "$STDOUT_OK" "an unwritable stdout path disables itself" + local log + log="$(cat "${WORK}/state/actions.log")" + assert_contains "$log" "reason=stdout-unavailable" "and says so once, locally" + assert_contains "$log" "event=kill sig=TERM pid=1234" "while the local record is unaffected" + record_action "event=kill sig=TERM pid=1235 role=claudeHelper" + assert_contains "$(cat "${WORK}/state/actions.log")" "pid=1235" "and later actions still record" + assert_eq 1 "$(grep -c "reason=stdout-unavailable" "${WORK}/state/actions.log")" \ + "the warning is not repeated on every action" +} + # --------------------------------------------------------------------------- # # 7. a stale pidfile does not disarm the watchdog forever # @@ -1300,6 +1445,7 @@ main() { test_kill_loop_breaker test_observe_mode test_sweep_log + test_stdout_emission test_singleton_survives_a_hard_kill test_pid_recycling printf '\n%d passed, %d failed\n' "$PASS" "$FAIL" diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh index 7f87267f..5b8d6995 100644 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh @@ -142,8 +142,18 @@ GLOBAL_LOOP_KILLS="${WATCHDOG_GLOBAL_LOOP_KILLS:-8}" # the operator's number where it is right, and a number anchored on its own # measured resting size where it is not. # -# The reference column is that measurement. It is not a budget; it is the floor -# below which a budget is a kill order rather than a limit. +# The reference column is that measurement, and it is the *resting* one rather +# than the fresh one, which is the correction that produced these numbers. A +# freshly started extension host is 471 MB PSS; the same extension host on a +# reconnected, idle 8 GiB workspace is 713 MB, and the tree around it 1093 MB +# rather than the 727 MB this design was first calibrated against. "Calibrated +# against fresh, deployed against resting" is the same error that produced a +# file-watcher ceiling below what an idle file watcher already held, so the +# references below are the largest resting figure measured for each role, and +# a role with no measurement gets none rather than a guess. +# +# It is not a budget; it is the floor below which a budget is a kill order +# rather than a limit. declare -gA BUDGET_ROLE=( [extensionHost]=1073741824 # 1024 MiB, against 471 MB resting [serverMain]=536870912 # 512 MiB, against 160 MB resting @@ -157,9 +167,10 @@ declare -gA BUDGET_ROLE=( # better than one that holds 300 MB. ) declare -gA RESTING_ROLE=( - [extensionHost]=493879296 # 471 MB - [serverMain]=167772160 # 160 MB - [fileWatcher]=35651584 # 34 MB + [extensionHost]=747634688 # 713 MB resting (471 MB fresh) + [serverMain]=167772160 # 160 MB resting (90 MB on a second measurement) + [fileWatcher]=92274688 # 88 MB resting (34 MB fresh) + [languageServer]=56623104 # 54 MB resting ) # A budget is never allowed below the role's measured resting size times this, # whatever the pod arithmetic says. This is the guard against the class of error @@ -178,7 +189,7 @@ HELPER_ROLES=" tsserver languageServer fileWatcher extensionHelper claudeHelper EDITOR_ROLES=" extensionHost serverMain " # Filled by derive_budgets(). -declare -gA BUDGET=() +declare -gA BUDGET=() FLOORED=() # Only processes at or above this are written to the sweep log, plus every # process the watchdog is policing regardless of size. The floor keeps a sweep @@ -186,6 +197,26 @@ declare -gA BUDGET=() # could plausibly matter later. SWEEP_LOG_FLOOR="${WATCHDOG_SWEEP_LOG_FLOOR:-33554432}" # 32 MiB +# Where the action lines also go, so that they survive the pod. +# +# The cluster's log agent tails container stdout and nothing else, so no file in +# this pod can reach Loki - but PID 1 in this container is the coder agent, and +# /proc/1/fd/1 *is* the container's stdout. Writing there needs no new +# infrastructure, no configuration anywhere, and the lines arrive already +# labelled by namespace, pod and container. +# +# Only *actions* go there: kills, refusals, disarms, the budgets in force, and a +# low-rate census line. The per-process sweep stays in the local file, because a +# few dozen rows a minute is not something to put through a log pipeline, and +# the local file remains the detailed record either way. +# +# Every write is best-effort. The fd may not be writable in every context - a +# harness, a different container layout, a future PID 1 that closes it - and a +# watchdog that died because logging failed would be a worse bug than any it +# prevents. The first failure disables the path and says so once, locally. +STDOUT_PATH="${WATCHDOG_STDOUT_PATH:-/proc/1/fd/1}" +CENSUS_EVERY="${WATCHDOG_CENSUS_EVERY:-3600}" # seconds between census lines + MAX_LOG_LINES="${WATCHDOG_MAX_LOG_LINES:-20000}" MAX_CSV_LINES="${WATCHDOG_MAX_CSV_LINES:-50000}" MAX_SWEEP_LINES="${WATCHDOG_MAX_SWEEP_LINES:-200000}" @@ -224,6 +255,8 @@ KILL_TIMES_ALL="" LAST_KILL_AT=0 PSS_UNAVAILABLE=0 VISIBILITY_WARNED=0 +STDOUT_OK=1 +LAST_CENSUS_AT=0 STARTED_AT=${WATCHDOG_NOW:-$EPOCHSECONDS} # --------------------------------------------------------------------------- # @@ -242,6 +275,7 @@ derive_budgets() { local max=$1 role want share floor var share=$((max / POD_SHARE_DEN)) BUDGET=() + FLOORED=() for role in "${!BUDGET_ROLE[@]}"; do var="WATCHDOG_BUDGET_${role}" if [[ -n ${!var:-} ]]; then @@ -251,7 +285,15 @@ derive_budgets() { want=${BUDGET_ROLE[$role]} ((want > share)) && want=$share floor=$(((${RESTING_ROLE[$role]:-0} * RESTING_FACTOR_NUM) / RESTING_FACTOR_DEN)) - ((want < floor)) && want=$floor + # The floor winning is not an error, but it is worth saying out loud: it + # means this pod is too small to bound this role at the share it was meant + # to have, and the role keeps its budget because the alternative is a kill + # loop. On the 8 GiB workspace the extension host reaches this, which is why + # it is reported rather than silently applied. + if ((want < floor)); then + want=$floor + FLOORED[$role]=1 + fi BUDGET[$role]=$want done return 0 @@ -945,11 +987,10 @@ LOG_LINES=0 CSV_LINES=0 SWEEP_LINES=0 -log_action() { - local msg="$*" stamp - printf -v stamp '%(%Y-%m-%dT%H:%M:%S%z)T' -1 - [[ $MODE == "observe" ]] && msg="[observe] ${msg}" - printf '%s %s\n' "$stamp" "$msg" >>"${STATE_DIR}/actions.log" +# Appends one line to the local action log and nothing else. Used directly only +# by emit_stdout, so that a failure to reach stdout cannot recurse into itself. +append_action_log() { + printf '%s\n' "$1" >>"${STATE_DIR}/actions.log" ((LOG_LINES += 1)) if ((LOG_LINES > MAX_LOG_LINES)); then mv -f "${STATE_DIR}/actions.log" "${STATE_DIR}/actions.log.1" 2>/dev/null @@ -958,6 +999,32 @@ log_action() { return 0 } +emit_stdout() { + ((STDOUT_OK)) || return 0 + [[ -n $STDOUT_PATH ]] || return 0 + if ! printf '%s\n' "$1" >>"$STDOUT_PATH" 2>/dev/null; then + STDOUT_OK=0 + append_action_log "component=memory-watchdog event=warning reason=stdout-unavailable path=${STDOUT_PATH} detail=@action lines are local-only from here@" + fi + return 0 +} + +# The one way an action is recorded. Arguments are logfmt pairs, and the whole +# line is logfmt - `component` first, so that a LogQL line filter +# (|= "component=memory-watchdog") and the logfmt parser both work on it without +# anyone adding a stream label. Free text goes in a detail= field and is quoted +# here rather than at each call site, because a call site that forgot would +# produce a line that parses into the wrong fields rather than an obvious error. +record_action() { + local stamp line="$*" + printf -v stamp '%(%Y-%m-%dT%H:%M:%S%z)T' -1 + line=${line//@/\"} + line="component=memory-watchdog time=${stamp} mode=${MODE} ${line}" + append_action_log "$line" + emit_stdout "$line" + return 0 +} + fmt_gib() { local v=$1 sign="" if ((v < 0)); then @@ -1046,14 +1113,14 @@ signal_pid() { # Second, independent guard. Selection already excluded these; this exists so # that a defect in tree-walking still cannot reach a protected process. if [[ -n ${PROTECTED[$pid]:-} ]] || is_never_signal "$pid"; then - log_action "REFUSED sig=${sig} pid=${pid} reason=protected (${why})" + record_action "event=refused sig=${sig} pid=${pid} role=${role} reason=protected detail=@${why}@" return 1 fi # And the positional rules, restated independently of selection: inside the # server tree, the only thing that may be signalled without being one of the # editor's own helpers is a helper a session spawned directly. if [[ -n ${NOT_EDITOR[$pid]:-} && $role != "claudeHelper" ]]; then - log_action "REFUSED sig=${sig} pid=${pid} reason=${PROTECT_REASON[$pid]:-not-editor} role=${role} (${why})" + record_action "event=refused sig=${sig} pid=${pid} role=${role} reason=${PROTECT_REASON[$pid]:-not-editor} detail=@${why}@" return 1 fi [[ $MODE == "observe" ]] || kill "-${sig}" "$pid" 2>/dev/null @@ -1090,12 +1157,12 @@ record_kill() { if ((n >= LOOP_KILLS)); then DISARMED[$role]=1 - log_action "DISARMED role=${role} reason=kill-loop kills=${n} window=${LOOP_WINDOW}s budget=$(fmt_mib "${BUDGET[$role]:-0}") - a role that has to be killed this often does not have a drift problem, it has a wrong budget; raise WATCHDOG_BUDGET_${role} or accept the size, but this watchdog will not keep restarting it" + record_action "event=disarmed role=${role} reason=kill-loop kills=${n} window_s=${LOOP_WINDOW} budget_mb=$((${BUDGET[$role]:-0} / MIB)) detail=@a role that has to be killed this often does not have a drift problem, it has a wrong budget; raise WATCHDOG_BUDGET_${role} or accept the size, but this watchdog will not keep restarting it@" fi if ((na >= GLOBAL_LOOP_KILLS)); then local r for r in "${!BUDGET[@]}"; do DISARMED[$r]=1; done - log_action "DISARMED role=all reason=global-kill-loop kills=${na} window=${LOOP_WINDOW}s - too many kills across roles for the budgets to be right; enforcement is off until the watchdog restarts" + record_action "event=disarmed role=all reason=global-kill-loop kills=${na} window_s=${LOOP_WINDOW} detail=@too many kills across roles for the budgets to be right; enforcement is off until the watchdog restarts@" fi return 0 } @@ -1178,14 +1245,14 @@ sweep_once() { # is reported once per dwell period rather than on every sweep for the # rest of its life. state=would-kill - log_action "would-kill pid=${pid} role=${role} id=${id} pss=${pss} budget=${budget} over_s=${over_s} age=${age} mode=${MODE} armed=no" + record_action "event=would-kill armed=no pid=${pid} role=${role} id=${id} pss_mb=$((pss / MIB)) budget_mb=$((budget / MIB)) over_s=${over_s} age_s=${age}" OVER_SINCE[$key]=$now elif [[ -n ${KILLED_AT[$key]:-} ]]; then # Already asked politely. Escalate once the grace has elapsed. if ((now - KILLED_AT[$key] >= KILL_GRACE)); then if signal_pid KILL "$pid" "$role" "drift ${role}"; then state=killed-9 - log_action "kill sig=KILL pid=${pid} role=${role} id=${id} pss=${pss} budget=${budget} over_s=${over_s} (still over budget ${KILL_GRACE}s after SIGTERM)" + record_action "event=kill sig=KILL pid=${pid} role=${role} id=${id} pss_mb=$((pss / MIB)) budget_mb=$((budget / MIB)) over_s=${over_s} detail=@still over budget ${KILL_GRACE}s after SIGTERM@" fi else state=terminating @@ -1195,7 +1262,7 @@ sweep_once() { KILLED_AT[$key]=$now state=killed ((killed_n += 1)) - log_action "kill sig=TERM pid=${pid} role=${role} id=${id} pss=${pss} budget=${budget} over_s=${over_s} age=${age} mode=${MODE}" + record_action "event=kill sig=TERM pid=${pid} role=${role} id=${id} pss_mb=$((pss / MIB)) budget_mb=$((budget / MIB)) over_s=${over_s} age_s=${age}" record_kill "$role" "$now" fi fi @@ -1249,11 +1316,29 @@ check_visibility() { ((SWEEPS >= VISIBILITY_WARMUP)) || return 0 VISIBILITY_WARNED=1 if ((POLICED_MAX_SEEN == 0)); then - log_action "WARNING no process has been policed in ${SWEEPS} sweeps - either nothing this watchdog manages has run, or selection is finding nothing; sweep.latest shows what was there" + record_action "event=warning reason=nothing-policed sweeps=${SWEEPS} detail=@either nothing this watchdog manages has run, or selection is finding nothing; sweep.latest shows what was there@" fi return 0 } +# One line an hour, whatever is happening. Two reasons it earns its place in a +# log pipeline that the sweep does not: it answers "was the watchdog alive and +# what was it seeing" for any window in the past, which no in-pod file can once +# the pod is gone; and it is the trend series for the standing population - the +# thing this watchdog exists to bound - at 24 lines a day rather than 40,000. +emit_census() { + local now=$1 role disarmed="" + for role in "${!DISARMED[@]}"; do disarmed+="${role} "; done + LAST_CENSUS_AT=$now + record_action "event=census sweeps=${SWEEPS} uptime_s=$((now - STARTED_AT))" \ + "policed=${POLICED_N:-0} over_budget=${OVER_N:-0} sessions=${#CLAUDE_ROOTS[@]}" \ + "tree=${#SERVER_TREE[@]} kills_total=${KILLS_TOTAL} disarmed=@${disarmed% }@" \ + "h_mb=$((M_H / MIB)) u_mb=$((M_U / MIB)) pressure=${PRESSURE} psi_full10=${M_PSI_CENTI}" \ + "top_role=${TOP_ROLE:-none} top_id=${TOP_ID:-none} top_pss_mb=$((${TOP_PSS:-0} / MIB))" \ + "top_budget_pct=${TOP_PCT:-0}" + return 0 +} + publish_summary() { local now=$1 local up=$((now - STARTED_AT)) @@ -1349,7 +1434,7 @@ cycle_once() { read_cgroup_memory rc=$? if ((rc == 2)); then - log_action "memory.max is unlimited - nothing to budget against, exiting" + record_action "event=exit reason=no-memory-limit detail=@memory.max is unlimited, so there is nothing to budget against@" return 2 fi ((rc == 0)) || return 1 @@ -1358,12 +1443,11 @@ cycle_once() { derive_budgets "$M_MAX" local role for role in extensionHost serverMain tsserver languageServer fileWatcher extensionHelper claudeHelper; do - log_action "budget role=${role} pss=$(fmt_mib "${BUDGET[$role]}") (pod share $(fmt_mib "$((M_MAX / POD_SHARE_DEN))"), resting reference $(fmt_mib "${RESTING_ROLE[$role]:-0}"))" + record_action "event=budget role=${role} budget_mb=$((${BUDGET[$role]} / MIB)) pod_share_mb=$((M_MAX / POD_SHARE_DEN / MIB)) resting_mb=$((${RESTING_ROLE[$role]:-0} / MIB)) floored=${FLOORED[$role]:-0} armed=$(role_is_armed "$role" && printf yes || printf no)" done - log_action "armed roles: $( + record_action "event=budgets_derived memory_max_mb=$((M_MAX / MIB)) dwell_s=${DWELL_SECONDS} min_age_s=${MIN_AGE} loop_window_s=${LOOP_WINDOW} loop_kills=${LOOP_KILLS} armed=@$( for role in "${!BUDGET[@]}"; do role_is_armed "$role" && printf '%s ' "$role"; done - printf '(mode=%s)' "$MODE" - )" + )@" fi read_cgroup_pressure @@ -1387,6 +1471,11 @@ cycle_once() { ((SWEEPS += 1)) check_visibility publish_summary "$now" + # The first census goes out immediately, so that a watchdog that starts and + # then dies has still said what it saw. + if ((LAST_CENSUS_AT == 0 || now - LAST_CENSUS_AT >= CENSUS_EVERY)); then + emit_census "$now" + fi fi PREV_AT=$now @@ -1432,7 +1521,7 @@ main() { local now STARTED_AT=${WATCHDOG_NOW:-$EPOCHSECONDS} - log_action "started mode=${MODE} pid=$$ cgroup=${CGROUP_DIR} sweep_every=$((SAMPLE_INTERVAL * SWEEP_EVERY))s dwell=${DWELL_SECONDS}s" + record_action "event=started pid=$$ cgroup=${CGROUP_DIR} sweep_every_s=$((SAMPLE_INTERVAL * SWEEP_EVERY)) dwell_s=${DWELL_SECONDS} stdout=${STDOUT_PATH}" while :; do now=${WATCHDOG_NOW:-$EPOCHSECONDS}