From 291cec34d6e6041b25cd7b9ad2e7d0d3ae40d146 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Sun, 16 Aug 2026 19:45:43 +0000 Subject: [PATCH 01/10] feat: add observe-only memory watchdog to the workspace template The workspace pod's cgroup sets memory.oom.group=1, so a cgroup OOM kills every process in the container as a group - the IDE, every tmux session and every long-running agent, together. Victim selection via oom_score_adj cannot help for the same reason, and memory.high or a child cgroup would need a writable /sys/fs/cgroup, which the unprivileged-by-default stance rules out. The only remaining strategy is to never reach memory.max. script-memory-watchdog.sh samples the cgroup's genuinely unreclaimable memory and publishes the headroom left before something has to die. It runs as a coder_script daemon (there is no supervisor in this pod) with a pid-file guard so an agent restart cannot leave two of them running. It ships in observe mode: it measures, publishes headroom to the Coder UI, and logs what it would have done, but sets no RLIMIT_DATA ceilings and sends no signals. The ceilings and tier thresholds were derived from role and an 8 GiB budget rather than from measurement, and are to be set from the calibration CSV the watchdog is now collecting. Arming it is a one-line change to WATCHDOG_MODE in env.tf. Two things the implementation depends on, both covered by script-memory-watchdog-test.sh: - Headroom is memory.max minus the unreclaimable fields of memory.stat only. memory.current and the memory.stat kernel roll-up read 92% and 42% of the limit on a pod whose true unreclaimable share is 23%, so either would make the watchdog fire permanently on an idle container. - Nothing in the --type=ptyHost subtree is ever signalled. 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. Also adds a weekly cron coder_script for a vscode-server GC; the script it calls lands separately in the dotfiles repo and the resource is a no-op until then. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 7 +- DESIGN.md | 8 + .../homelab-workspace/coder-agent.tf | 22 + .../kubernetes/homelab-workspace/configmap.tf | 1 + .../homelab-workspace/deployment.tf | 5 + templates/kubernetes/homelab-workspace/env.tf | 15 + .../script-memory-watchdog-test.sh | 444 +++++++++++ .../script-memory-watchdog.sh | 735 ++++++++++++++++++ .../kubernetes/homelab-workspace/scripts.tf | 50 ++ 9 files changed, 1286 insertions(+), 1 deletion(-) create mode 100755 templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh create mode 100644 templates/kubernetes/homelab-workspace/script-memory-watchdog.sh create mode 100644 templates/kubernetes/homelab-workspace/scripts.tf diff --git a/CLAUDE.md b/CLAUDE.md index 63023529..792a0c5a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,11 +62,14 @@ Quick orientation map — for what each piece is *for* and the decisions behind | `terraform.tf` | Provider requirements/versions (Renovate-managed) | | `main.tf` | `coder_workspace`/`coder_workspace_owner` data sources, shared labels/path locals | | `parameters.tf` | User-facing `coder_parameter` inputs + sanitization locals | -| `coder-agent.tf` | `coder_agent` resource: startup script, `coder stat` metadata | +| `coder-agent.tf` | `coder_agent` resource: startup script, `coder stat` metadata, `resources_monitoring` | | `deployment.tf` / `configmap.tf` | Kubernetes Pod spec, volumes, ConfigMap | | `env.tf` | `coder_env` resources exposed to the agent | +| `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). **Ships in observe-only mode**: it measures and logs, and sets no limits and sends no signals until `WATCHDOG_MODE=enforce` | +| `script-memory-watchdog-test.sh` | Fixture tests for the watchdog's arithmetic and process selection. Run it by hand (`./script-memory-watchdog-test.sh`); nothing in CI runs it | **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. @@ -79,6 +82,8 @@ 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_*` regex allowlist is the only thing stopping `system_packages`/`preferred_nodes` from injecting shell metacharacters into the init container — any new list-type parameter must go through the same decode-then-validate 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 92% and 42% of the limit while true `U` is 23%, so either substitution makes the watchdog fire permanently on an idle container. Its thresholds are absolute bytes, not percentages, because the page cache a workload needs is a property of the workload rather than of the limit — which also means the 4 GiB memory parameter needs its own numbers, and the script logs a warning when it detects that mismatch. +- 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`. - 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`'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 f50763a3..dd138204 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -69,6 +69,14 @@ 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. + +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. + +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. + +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 ships in an observe-only mode — measuring and logging, changing nothing — so that the thresholds at which it acts get set from a week of this workload's own data rather than from a guess. 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. + ## Outcomes targeted - One operator can keep dependencies current and ship template/image changes at low ongoing effort, without a fleet of environments to maintain. diff --git a/templates/kubernetes/homelab-workspace/coder-agent.tf b/templates/kubernetes/homelab-workspace/coder-agent.tf index 4adccdbf..e495d311 100644 --- a/templates/kubernetes/homelab-workspace/coder-agent.tf +++ b/templates/kubernetes/homelab-workspace/coder-agent.tf @@ -49,4 +49,26 @@ resource "coder_agent" "main" { interval = 60 timeout = 1 } + metadata { + display_name = "Memory Headroom" + key = "6_memory_headroom" + # Published by the memory watchdog (see scripts.tf). This is the honest + # number: bytes left before something in the pod has to die. "Memory Usage" + # above reads ~63% on a pod whose true unreclaimable share is ~23%, because + # it counts page cache the kernel will hand straight back. + script = "cat $${HOME}/.local/state/vscode-memory-watchdog/headroom 2>/dev/null || echo '-'" + interval = 60 + timeout = 1 + } + + # A coarse backstop, not the real signal. Coder computes + # (memory.current - inactive_file) / memory.max, which is still inflated by + # active_file, so the threshold is 95 rather than 90: at 90 this pod would + # alert continuously while sitting at 23% unreclaimable and zero PSI. + resources_monitoring { + memory { + enabled = true + threshold = 95 + } + } } diff --git a/templates/kubernetes/homelab-workspace/configmap.tf b/templates/kubernetes/homelab-workspace/configmap.tf index b17c0f49..8a51470a 100644 --- a/templates/kubernetes/homelab-workspace/configmap.tf +++ b/templates/kubernetes/homelab-workspace/configmap.tf @@ -9,6 +9,7 @@ resource "kubernetes_config_map_v1" "workspace_scripts" { data = { agent_startup_script = file("${path.cwd}/script-agent-startup.sh") + memory_watchdog_script = file("${path.cwd}/script-memory-watchdog.sh") prepare_workspace_script = file("${path.cwd}/script-prepare-workspace.sh") workspace_init_script = coder_agent.main.init_script } diff --git a/templates/kubernetes/homelab-workspace/deployment.tf b/templates/kubernetes/homelab-workspace/deployment.tf index 48236f9a..5a39d058 100644 --- a/templates/kubernetes/homelab-workspace/deployment.tf +++ b/templates/kubernetes/homelab-workspace/deployment.tf @@ -131,6 +131,11 @@ resource "kubernetes_deployment_v1" "deployment" { name = "coder-scripts" sub_path = "agent_startup_script" } + volume_mount { + mount_path = "/memory-watchdog.sh" + name = "coder-scripts" + sub_path = "memory_watchdog_script" + } volume_mount { mount_path = "/workspace-init.sh" name = "coder-scripts" diff --git a/templates/kubernetes/homelab-workspace/env.tf b/templates/kubernetes/homelab-workspace/env.tf index 4b7449db..a1b4fcb0 100644 --- a/templates/kubernetes/homelab-workspace/env.tf +++ b/templates/kubernetes/homelab-workspace/env.tf @@ -3,3 +3,18 @@ resource "coder_env" "welcome_message" { name = "HOMEBREW_PREFIX" 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. +# +# Deliberately left at "observe". The ceilings and tier thresholds in +# script-memory-watchdog.sh were derived from role and an 8 GiB budget, not from +# measurement - too low kills a healthy extension host mid-edit, too high makes +# the mechanism inert. Flip this only once the numbers have been set from the +# calibration data the watchdog is collecting. +resource "coder_env" "memory_watchdog_mode" { + agent_id = coder_agent.main.id + name = "WATCHDOG_MODE" + value = "observe" +} diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh new file mode 100755 index 00000000..bc9cd39c --- /dev/null +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh @@ -0,0 +1,444 @@ +#!/bin/bash +# +# Fixture tests for script-memory-watchdog.sh. +# +# Run it directly; it needs nothing but bash and a writable TMPDIR: +# +# ./script-memory-watchdog-test.sh +# +# CI does not run this - there is no test stage in this repo (see TESTING.md). +# 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. +# +# 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 +# exclusion is paired with the mutation that must flip it. +set -uo pipefail + +SELF_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" +WORK="$(mktemp -d)" +trap 'rm -rf "${WORK}"' EXIT + +PASS=0 +FAIL=0 + +ok() { + PASS=$((PASS + 1)) + printf ' ok %s\n' "$1" +} + +bad() { + FAIL=$((FAIL + 1)) + printf ' FAIL %s\n' "$1" +} + +assert_eq() { + local want=$1 got=$2 what=$3 + if [[ $want == "$got" ]]; then + ok "${what}" + else + bad "${what}: want '${want}', got '${got}'" + fi +} + +# assert_protected +assert_protected() { + local pid=$1 want=$2 what=$3 got=no + [[ -n ${PROTECTED[$pid]:-} ]] && got=yes + if [[ $got == "$want" ]]; then + ok "${what}" + else + bad "${what}: protected=${got}, expected ${want}" + 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} +} + +# --------------------------------------------------------------------------- # +# fixtures +# --------------------------------------------------------------------------- # + +# 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() { + local dir=$1 max=$2 psi_full10=$3 + mkdir -p "$dir" + cat >"${dir}/memory.stat" <<'EOF' +anon 1979584512 +file 4198756352 +kernel 1693286400 +kernel_stack 2195456 +pagetables 10711040 +sec_pagetables 0 +percpu 13536 +sock 4096 +shmem 0 +unevictable 0 +slab_reclaimable 1676590856 +slab_unreclaimable 3337768 +slab 1679928624 +workingset_refault_file 57094899 +pgscan_direct 78917443 +EOF + printf '%s\n' "$max" >"${dir}/memory.max" + printf '%s\n' "7872479232" >"${dir}/memory.current" + cat >"${dir}/memory.pressure" < +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' \ + "$pid" "$comm" "$ppid" >"${d}/stat" + printf '0 %s 0 0 0 0 0\n' "$((rss / 4096))" >"${d}/statm" + cat >"${d}/limits" <<'EOF' +Limit Soft Limit Hard Limit Units +Max data size unlimited unlimited bytes +EOF + local arg + : >"${d}/cmdline" + for arg in "$@"; do + printf '%s\0' "$arg" >>"${d}/cmdline" + done +} + +# A representative tree. 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 +# +- 40 server-main.js +# | +- 41 extensionHost +# | | +- 44 tsserver.js +# | | +- 45 yaml-language-server +# | +- 42 fileWatcher +# | +- 43 ptyHost <- excised, with its whole subtree +# | +- 50 bash +# | +- 51 tmux: server +# | +- 52 claude +# | +- 53 node (the hog) +# +- 60 claude (outside the tree) +build_tree() { + local dir=$1 ptyhost_arg=${2:---type=ptyHost} + local srv="/home/coder/.vscode-server/cli/servers/Stable-abc/server/out/server-main.js" + mkdir -p "$dir" + add_proc "$dir" 1 0 coder 14208 ./coder agent + # A decoy with a lower pid that merely *mentions* both marker strings, and a + # ptyHost flag inside a larger argument. Naive substring matching would elect + # it as the server root and mis-scope every decision that follows. + add_proc "$dir" 3 1 bash 3000000 bash -c \ + "tail -f ${srv}.log --type=extensionHost-ish" + add_proc "$dir" 40 1 node 300000000 /usr/bin/node "$srv" --host localhost + add_proc "$dir" 41 40 node 1500000000 /usr/bin/node "$srv" bootstrap-fork --type=extensionHost + add_proc "$dir" 44 41 node 2000000000 /usr/bin/node /home/coder/.vscode-server/extensions/ms-ts/tsserver.js + add_proc "$dir" 45 41 node 400000000 /usr/bin/node /home/coder/.vscode-server/extensions/redhat/yaml-language-server + add_proc "$dir" 42 40 node 250000000 /usr/bin/node "$srv" bootstrap-fork --type=fileWatcher + add_proc "$dir" 43 40 node 100000000 /usr/bin/node "$srv" bootstrap-fork "$ptyhost_arg" + 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 + add_proc "$dir" 53 51 node 3000000000 node -e "const a=[];setInterval(()=>a.push(Buffer.alloc(1)),1)" + add_proc "$dir" 60 1 claude 700000000 claude +} + +load_watchdog() { + WATCHDOG_SOURCE_ONLY=1 \ + WATCHDOG_CGROUP_DIR="$1" \ + WATCHDOG_PROC_DIR="$2" \ + WATCHDOG_STATE_DIR="${WORK}/state" \ + WATCHDOG_MODE=observe \ + . "${SELF_DIR}/script-memory-watchdog.sh" + mkdir -p "${WORK}/state" +} + +scan_fixture() { + read_cgroup_memory + read_cgroup_pressure + read_process_table + SERVER_TREE=() + if find_server_root; then + subtree_of "$SERVER_PID" SERVER_TREE + read_rss "${!SERVER_TREE[@]}" + fi + compute_protected +} + +candidate_pids() { + select_candidates "$1" + local row pid out="" + for row in "${CANDIDATES[@]}"; do + pid=${row#* } + out+="${pid%% *} " + done + printf '%s' "${out% }" +} + +# --------------------------------------------------------------------------- # +# 1. the measurement +# --------------------------------------------------------------------------- # + +test_measurement() { + printf 'measurement\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + local pdir="${WORK}/proc1" + build_tree "$pdir" + load_watchdog "${WORK}/cg" "$pdir" + read_cgroup_memory + + # Hand-computed from the fixture, which is the real pod's memory.stat: + # 1979584512 + 0 + 0 + 3337768 + 2195456 + 10711040 + 0 + 13536 + 4096 + assert_eq 1995846408 "$M_U" "U excludes slab_reclaimable and page cache" + assert_eq 6594088184 "$M_H" "H = memory.max - U" + + # 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" + + 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" + + write_cgroup "${WORK}/cg" max 0.00 + read_cgroup_memory + assert_eq 2 "$?" "an unlimited cgroup is reported, not treated as huge headroom" +} + +# --------------------------------------------------------------------------- # +# 2. process selection - the part that can hurt the operator +# --------------------------------------------------------------------------- # + +test_selection() { + printf 'selection\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + local pdir="${WORK}/proc2" + build_tree "$pdir" + load_watchdog "${WORK}/cg" "$pdir" + scan_fixture + + assert_eq 40 "$SERVER_PID" "server root found by argv element, not by substring" + assert_eq 10 "${#SERVER_TREE[@]}" "server tree spans every descendant, ptyHost included" + if [[ -n ${SERVER_TREE[3]:-} ]]; then + bad "a process that merely mentions the server path joined the tree" + else + ok "a process that merely mentions the server path stays out of the tree" + fi + role_of 3 + assert_eq other "$ROLE" "a --type=extensionHost-ish argument is not read as a role flag" + role_of 43 + assert_eq ptyHost "$ROLE" "the real --type=ptyHost argument is" + + # 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. + # 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_protected 1 yes "pid 1 is protected" + assert_protected 60 yes "a claude outside the tree is protected" + + assert_eq "44 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 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 +} + +# --------------------------------------------------------------------------- # +# 3. the mutation that must flip the result +# +# Without this, "the hog was not selected" is unfalsifiable - it would pass just +# as happily against a watchdog that selects nothing at all. +# --------------------------------------------------------------------------- # + +test_selection_is_falsifiable() { + printf 'selection is falsifiable\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 + load_watchdog "${WORK}/cg" "$pdir" + scan_fixture + + 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" + else + bad "L4 still ignores the hog, so the ptyHost assertion proves nothing" + 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" +} + +# --------------------------------------------------------------------------- # +# 4. the tier ladder +# --------------------------------------------------------------------------- # + +# 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" +} + +test_tiers() { + printf 'tier ladder\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" +} + +# --------------------------------------------------------------------------- # +# 5. observe mode really is inert +# --------------------------------------------------------------------------- # + +test_observe_mode_is_inert() { + printf 'observe mode\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + local pdir="${WORK}/proc4" + 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 + + # 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" + else + ok "no ceiling is ever proposed for the ptyHost fork" + fi +} + +# --------------------------------------------------------------------------- # + +main() { + test_measurement + test_selection + test_selection_is_falsifiable + test_tiers + test_observe_mode_is_inert + printf '\n%d passed, %d failed\n' "$PASS" "$FAIL" + ((FAIL == 0)) +} + +main "$@" diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh new file mode 100644 index 00000000..16f86928 --- /dev/null +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh @@ -0,0 +1,735 @@ +#!/bin/bash +# +# Memory watchdog for the workspace pod. +# +# 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. +# +# Dependencies are deliberately tiny, because this runs before (and without) the +# operator's dotfiles: bash 4.4+, /proc, /sys/fs/cgroup, /usr/bin/sleep, coreutils +# mv/rm/mkdir, and - in enforce mode only - /usr/bin/prlimit. No brew, no mise, no +# python3, no flock, no awk. Measurement and process enumeration use bash builtins +# so a scan forks nothing. +# +# 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. +# +# 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. +set -uo pipefail + +# --------------------------------------------------------------------------- # +# configuration +# --------------------------------------------------------------------------- # + +CGROUP_DIR="${WATCHDOG_CGROUP_DIR:-/sys/fs/cgroup}" +PROC_DIR="${WATCHDOG_PROC_DIR:-/proc}" +STATE_DIR="${WATCHDOG_STATE_DIR:-${HOME:-/home/coder}/.local/state/vscode-memory-watchdog}" +MODE="${WATCHDOG_MODE:-observe}" +ONESHOT="${WATCHDOG_ONESHOT:-0}" + +GIB=1073741824 +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_IDLE="${WATCHDOG_INTERVAL_IDLE:-10}" +INTERVAL_BUSY="${WATCHDOG_INTERVAL_BUSY:-2}" +CALIBRATION_EVERY="${WATCHDOG_CALIBRATION_EVERY:-6}" # cycles between CSV rows + +# Tier thresholds, in absolute bytes of headroom H. Absolute rather than a +# percentage of memory.max, because the page cache a workload needs to make +# forward progress is a property of the workload, not of the limit. +# +# NOTE: unvalidated starting points, derived from role and an 8 GiB budget rather +# than from measurement. Replacing them with numbers taken from calibration.csv +# is the entire reason this ships in observe mode first. +T_L1="${WATCHDOG_T_L1:-3221225472}" # 3.00 GiB +T_L2="${WATCHDOG_T_L2:-2147483648}" # 2.00 GiB +T_L3="${WATCHDOG_T_L3:-1342177280}" # 1.25 GiB +T_L4="${WATCHDOG_T_L4:-805306368}" # 0.75 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}" +COOLDOWN="${WATCHDOG_COOLDOWN:-180}" +PROJECTION_HORIZON="${WATCHDOG_PROJECTION_HORIZON:-60}" # seconds + +# Soft RLIMIT_DATA ceilings by role, in bytes. Hard limits are never touched, so +# an inheriting shell restores itself with `ulimit -d unlimited`. +# +# NOT AGREED. Enforce mode must not be turned on until these are set from +# calibration data: too low kills a healthy extension host mid-edit, too high +# makes the mechanism inert. +declare -gA CEILING=( + [serverMain]=1610612736 # 1.50 GiB + [extensionHost]=3221225472 # 3.00 GiB + [tsserver]=3758096384 # 3.50 GiB + [languageServer]=1073741824 # 1.00 GiB + [fileWatcher]=1073741824 # 1.00 GiB +) + +# 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 " + +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_RSS=() CHILDREN=() +declare -gA SERVER_TREE=() PROTECTED=() +declare -ga PIDS=() CANDIDATES=() +SERVER_PID="" +TIER=L0 +ROLE=other +PREV_TIER=L0 +PREV_AT=0 +PREV_U=0 +PREV_REFAULT=0 +PREV_PGSCAN=0 +CYCLE=0 + +# --------------------------------------------------------------------------- # +# measurement - reads only, sets M_*/P_* globals, decides nothing +# --------------------------------------------------------------------------- # + +# 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 +# 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%. +# +# Returns 0 on success, 1 on read failure, 2 if the cgroup has no memory limit. +read_cgroup_memory() { + local key val + local anon=0 shmem=0 unevictable=0 slab_unreclaimable=0 kernel_stack=0 + local pagetables=0 sec_pagetables=0 percpu=0 sock=0 + + M_FILE=0 + M_SLAB_RECLAIMABLE=0 + M_REFAULT_FILE=0 + M_PGSCAN_DIRECT=0 + + while read -r key val; do + case "$key" in + anon) anon=$val ;; + shmem) shmem=$val ;; + unevictable) unevictable=$val ;; + slab_unreclaimable) slab_unreclaimable=$val ;; + kernel_stack) kernel_stack=$val ;; + pagetables) pagetables=$val ;; + sec_pagetables) sec_pagetables=$val ;; + percpu) percpu=$val ;; + sock) sock=$val ;; + file) M_FILE=$val ;; + slab_reclaimable) M_SLAB_RECLAIMABLE=$val ;; + workingset_refault_file) M_REFAULT_FILE=$val ;; + pgscan_direct) M_PGSCAN_DIRECT=$val ;; + esac + done <"${CGROUP_DIR}/memory.stat" || return 1 + + M_ANON=$anon + M_SHMEM=$shmem + M_UNEVICTABLE=$unevictable + M_SLAB_UNRECLAIMABLE=$slab_unreclaimable + M_KERNEL_STACK=$kernel_stack + M_PAGETABLES=$pagetables + M_SEC_PAGETABLES=$sec_pagetables + M_PERCPU=$percpu + M_SOCK=$sock + + # sec_pagetables is zero outside nested virtualisation but is genuinely + # unreclaimable when present, so it is counted rather than assumed away. + M_U=$((anon + shmem + unevictable + slab_unreclaimable + + kernel_stack + pagetables + sec_pagetables + percpu + sock)) + + local raw="" + read -r raw <"${CGROUP_DIR}/memory.max" || return 1 + [[ $raw == "max" ]] && return 2 + [[ $raw =~ ^[0-9]+$ ]] || return 1 + M_MAX=$raw + + M_CURRENT=0 + read -r M_CURRENT <"${CGROUP_DIR}/memory.current" 2>/dev/null + + M_H=$((M_MAX - M_U)) + return 0 +} + +# memory.pressure "full avg10", scaled by 100 so it compares as an integer. +read_cgroup_pressure() { + local kind field + M_PSI_CENTI=0 + while read -r kind field _; do + [[ $kind == "full" ]] || continue + field=${field#avg10=} + [[ $field == *.* ]] || field="${field}.00" + [[ ${field%%.*} =~ ^[0-9]+$ && ${field##*.} =~ ^[0-9]+$ ]] || continue + M_PSI_CENTI=$((10#${field%%.*} * 100 + 10#${field##*.})) + done <"${CGROUP_DIR}/memory.pressure" 2>/dev/null + return 0 +} + +# Fills PIDS / P_COMM / P_CMD / CHILDREN for one scan. +read_process_table() { + PIDS=() + P_COMM=() + P_CMD=() + P_RSS=() + CHILDREN=() + + local entry pid line rest comm ppid + local -a argv + + for entry in "${PROC_DIR}"/[0-9]*; do + pid=${entry##*/} + read -r line <"$entry/stat" 2>/dev/null || continue + + # /proc//stat is "pid (comm) state ppid ...", and comm may contain + # spaces and parentheses, so split on the *last* ") " rather than tokenise. + rest=${line##*') '} + [[ $rest == "$line" ]] && continue + comm=${line#*'('} + comm=${comm%%') '*} + rest=${rest#* } # drop state + ppid=${rest%% *} + [[ $ppid =~ ^[0-9]+$ ]] || continue + + argv=() + mapfile -d '' -t argv <"$entry/cmdline" 2>/dev/null + PIDS+=("$pid") + P_COMM[$pid]=$comm + P_CMD[$pid]="${argv[*]}" + CHILDREN[$ppid]+=" $pid" + done + return 0 +} + +read_rss() { + local pid res + for pid in "$@"; do + read -r _ res _ <"${PROC_DIR}/${pid}/statm" 2>/dev/null || continue + P_RSS[$pid]=$((res * PAGE_SIZE)) + done + return 0 +} + +# --------------------------------------------------------------------------- # +# selection - pure over the tables above; produces sets, takes no action +# --------------------------------------------------------------------------- # + +# Sets SERVER_PID to the pid of the remote server entrypoint, or "". +# +# Matching is on a whole argv element, not on a substring of the joined command +# line. Anything that merely mentions the path - a grep over the server's log, an +# editor, a shell running a script that names it - would otherwise be mistaken +# for the root, and the root is what scopes every subsequent decision. A match +# whose comm is `node` wins outright; anything else is only a fallback. +find_server_root() { + local pid arg fallback="" + local -a argv + SERVER_PID="" + for pid in "${PIDS[@]}"; do + [[ ${P_CMD[$pid]} == *"/.vscode-server/"*"out/server-main.js"* ]] || continue + argv=() + mapfile -d '' -t argv <"${PROC_DIR}/${pid}/cmdline" 2>/dev/null + for arg in "${argv[@]}"; do + [[ $arg == *"/.vscode-server/"*"out/server-main.js" ]] || continue + if [[ ${P_COMM[$pid]:-} == "node" ]]; then + SERVER_PID=$pid + return 0 + fi + [[ -n $fallback ]] || fallback=$pid + break + done + done + [[ -n $fallback ]] || return 1 + SERVER_PID=$fallback + return 0 +} + +# subtree_of +subtree_of() { + local root=$1 + local -n out=$2 + local queue=("$root") cur kids k + out=() + # shellcheck disable=SC2004 + # `out` is a nameref to an associative array, so the subscript is a string + # key. Dropping the $ would make bash key it on the literal name. + out[$root]=1 + while ((${#queue[@]})); do + cur=${queue[0]} + queue=("${queue[@]:1}") + kids=${CHILDREN[$cur]:-} + # shellcheck disable=SC2086 + # kids is a space-joined list of integers this script built itself. + for k in $kids; do + [[ -n ${out[$k]:-} ]] && continue + # shellcheck disable=SC2004 + out[$k]=1 + queue+=("$k") + done + done + return 0 +} + +# The never-signal list. Every action consults this directly, so a defect in +# tree-walking still cannot route around it. +# +# 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. Errors here are one-directional by design - refusing to +# signal something that could safely have been signalled costs nothing. +is_never_signal() { + local pid=$1 + ((pid <= 1)) && return 0 + ((pid == $$)) && return 0 + ((pid == BASHPID)) && return 0 + ((pid == PPID)) && return 0 + case "${P_COMM[$pid]:-}" in + coder | claude | chezmoi | sshd | init | systemd) return 0 ;; + tmux*) return 0 ;; + esac + case "${P_CMD[$pid]:-}" in + *"coder agent"*) return 0 ;; + *"/claude"* | "claude" | "claude "*) return 0 ;; + *chezmoi*) return 0 ;; + *memory-watchdog*) return 0 ;; + esac + return 1 +} + +# Everything the watchdog must never touch: the never-signal names anywhere in +# the pod, plus every ptyHost fork inside the server tree and all its descendants. +compute_protected() { + PROTECTED=() + local pid p + local -A pty=() + + for pid in "${PIDS[@]}"; do + is_never_signal "$pid" && PROTECTED[$pid]=1 + done + + 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 + done + fi + done + return 0 +} + +# Sets ROLE. +# +# `--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 +# first, and additionally as a loose substring: over-matching ptyHost only ever +# means protecting something that could have been touched, which is the safe +# direction, whereas over-matching any other role means mis-classifying it. +role_of() { + local cmd=" ${P_CMD[$1]:-} " + case "$cmd" in + *" --type=ptyHost "* | *"--type=ptyHost"*) ROLE=ptyHost ;; + *" --type=extensionHost "*) ROLE=extensionHost ;; + *" --type=fileWatcher "*) ROLE=fileWatcher ;; + *"tsserver.js "* | *"/typescript/lib/tsserver"*) ROLE=tsserver ;; + # Looser than the rest: language servers have no common launch convention, so + # these are prefix/suffix guesses. They are only ever consulted for processes + # already inside the server tree and outside the ptyHost subtree, which is what + # keeps the blast radius of a wrong guess to one restartable helper. + *yaml-language-server* | *jsonServerMain* | *-language-server* | *languageserver*) ROLE=languageServer ;; + *"out/server-main.js "*) ROLE=serverMain ;; + *) ROLE=other ;; + esac + 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=() + + for pid in "${!SERVER_TREE[@]}"; do + [[ -n ${PROTECTED[$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") + 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[@]}") + 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 + +# Sets TIER. Called in the current shell, never in a command substitution - the +# debounce counters are state and a subshell would silently discard them. +decide_tier() { + local h=$1 psi=$2 refault=$3 proj=$4 now=$5 + + 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 + + if ((h < T_L4)); then + TIER=L4 + return 0 + fi + + TIER=L0 + if ((C_L3 >= DEBOUNCE_L3)); then + TIER=L3 + elif ((C_L2 >= DEBOUNCE_L2)) && + { ((psi >= T_PSI_CENTI)) || ((refault >= T_REFAULT_RATE)); }; then + TIER=L2 + elif ((C_L1 >= DEBOUNCE_L1)); then + TIER=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 [[ $TIER != "L0" ]] && ((proj < T_L4)); then + TIER=L3 + fi + + # Cooldown applies to the acting tiers only. L4 is exempt because by then the + # alternative to acting is an oom.group kill of everything in the pod. + if [[ $TIER == "L2" || $TIER == "L3" ]] && ((now - LAST_ACTION_AT < COOLDOWN)); then + TIER=L1 + fi + return 0 +} + +# --------------------------------------------------------------------------- # +# action - the only place that writes state or signals anything +# --------------------------------------------------------------------------- # + +LOG_LINES=0 +CSV_LINES=0 + +log_action() { + local msg="$*" stamp + printf -v stamp '%(%Y-%m-%dT%H:%M:%S%z)T' -1 + [[ $MODE == "enforce" ]] || msg="[observe] ${msg}" + printf '%s %s\n' "$stamp" "$msg" >>"${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 + LOG_LINES=0 + fi + return 0 +} + +fmt_gib() { + printf '%d.%02d GiB' "$(($1 / GIB))" "$(($1 % GIB * 100 / GIB))" +} + +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 +} + +append_calibration() { + local csv="${STATE_DIR}/calibration.csv" + if [[ ! -s $csv ]]; then + printf '%s\n' \ + "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" \ + >"$csv" + fi + printf '%s\n' "$*" >>"$csv" + ((CSV_LINES += 1)) + if ((CSV_LINES > MAX_CSV_LINES)); then + mv -f "$csv" "${csv}.1" 2>/dev/null + CSV_LINES=0 + fi + return 0 +} + +# 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 + 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 + + 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 + log_action "ceiling pid=${pid} role=${ROLE} rlimit_data=${want} was=${cur}" + done + return 0 +} + +signal_pid() { + local sig=$1 pid=$2 why=$3 + # 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})" + return 0 +} + +shed_load() { + local tier=$1 now=$2 + local row rss pid role acted=0 + local -a targets=() + + select_candidates "$tier" + ((${#CANDIDATES[@]})) || return 0 + + for row in "${CANDIDATES[@]}"; do + rss=${row%% *} + pid=${row#* } + role=${pid#* } + pid=${pid%% *} + if signal_pid TERM "$pid" "${tier} ${role} rss=${rss}"; then + acted=1 + targets+=("$pid") + [[ $tier == "L4" ]] || break + fi + 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 + + ((acted)) && LAST_ACTION_AT=$now + return 0 +} + +# --------------------------------------------------------------------------- # +# lifecycle +# --------------------------------------------------------------------------- # + +# A coder_script re-runs when the agent restarts without the pod restarting, so +# two watchdogs are otherwise entirely possible. noclobber gives an atomic O_EXCL +# create without flock, which is brew-only here. +acquire_singleton() { + local pidfile="${STATE_DIR}/watchdog.pid" + local other="" + local -a argv=() + + if ( + set -o noclobber + printf '%s\n' "$$" >"$pidfile" + ) 2>/dev/null; then + return 0 + fi + + read -r other <"$pidfile" 2>/dev/null + if [[ $other =~ ^[0-9]+$ ]] && [[ -r ${PROC_DIR}/${other}/cmdline ]]; then + mapfile -d '' -t argv <"${PROC_DIR}/${other}/cmdline" 2>/dev/null + [[ "${argv[*]}" == *memory-watchdog* ]] && return 1 + fi + + rm -f "$pidfile" + ( + set -o noclobber + printf '%s\n' "$$" >"$pidfile" + ) 2>/dev/null +} + +release_singleton() { + rm -f "${STATE_DIR}/watchdog.pid" +} + +# Returns 2 when the cgroup has no memory limit and there is nothing to protect. +scan_once() { + local now=$1 rc + + read_cgroup_memory + rc=$? + if ((rc == 2)); then + log_action "memory.max is unlimited - nothing to protect, exiting" + return 2 + fi + ((rc == 0)) || return 1 + read_cgroup_pressure + read_process_table + + # 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 + if ((PREV_AT > 0)); then + refault_rate=$(((M_REFAULT_FILE - PREV_REFAULT) / dt)) + pgscan_rate=$(((M_PGSCAN_DIRECT - PREV_PGSCAN) / dt)) + du_rate=$(((M_U - PREV_U) / dt)) + fi + local projected=$((M_H - du_rate * PROJECTION_HORIZON)) + + SERVER_TREE=() + if find_server_root; then + subtree_of "$SERVER_PID" SERVER_TREE + read_rss "${!SERVER_TREE[@]}" + fi + 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 + + 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}" + fi + + [[ -n $SERVER_PID ]] && apply_ceilings + + 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") tree=${#SERVER_TREE[@]}" + 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 +} + +warn_if_thresholds_oversized() { + local raw="" + read -r raw <"${CGROUP_DIR}/memory.max" 2>/dev/null + [[ $raw =~ ^[0-9]+$ ]] || return 0 + # An 8 GiB pod rests at U ~1.9 GiB and so has ~6 GiB of headroom. A 4 GiB pod + # has ~2 GiB and would sit permanently at L1/L2 against these absolute + # thresholds. That is a calibration problem, not something to paper over with a + # clamp, so say it once, loudly, in the log the operator will actually read. + ((T_L1 * 2 > raw)) && + log_action "WARNING memory.max=${raw} is small relative to T_L1=${T_L1}; the ladder will sit low permanently - recalibrate before enabling enforce mode" + return 0 +} + +main() { + mkdir -p "$STATE_DIR" || exit 1 + + if ! acquire_singleton; then + printf 'memory-watchdog: another instance is already running\n' >&2 + exit 0 + fi + trap 'release_singleton; exit 0' HUP INT TERM + trap release_singleton EXIT + + log_action "started mode=${MODE} pid=$$ cgroup=${CGROUP_DIR} thresholds L1=${T_L1} L2=${T_L2} L3=${T_L3} L4=${T_L4}" + warn_if_thresholds_oversized + + local now interval + while :; do + now=${WATCHDOG_NOW:-$EPOCHSECONDS} + scan_once "$now" + (($? == 2)) && break + ((CYCLE += 1)) + + [[ $ONESHOT == "1" ]] && break + + interval=$INTERVAL_IDLE + [[ $PREV_TIER == "L0" ]] || interval=$INTERVAL_BUSY + /usr/bin/sleep "$interval" + done + return 0 +} + +# Sourcing with WATCHDOG_SOURCE_ONLY=1 exposes the functions to the test harness +# without starting the loop. +if [[ ${WATCHDOG_SOURCE_ONLY:-0} != "1" ]]; then + main "$@" +fi diff --git a/templates/kubernetes/homelab-workspace/scripts.tf b/templates/kubernetes/homelab-workspace/scripts.tf new file mode 100644 index 00000000..e2f9b82e --- /dev/null +++ b/templates/kubernetes/homelab-workspace/scripts.tf @@ -0,0 +1,50 @@ +# Agent-side scripts. There is no systemd and no supervisor in this pod - PID 1 +# 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. +# +# setsid --fork detaches the watchdog from the agent's script runner, so this +# resource completes immediately and start_blocks_login stays honest. The +# consequence is that an agent restart without a pod restart leaves the previous +# watchdog running - which is what the script's pid-file guard is for. +resource "coder_script" "memory_watchdog" { + agent_id = coder_agent.main.id + display_name = "Memory watchdog" + icon = "/icon/memory.svg" + run_on_start = true + start_blocks_login = false + script = <<-EOT + set -u + state_dir="$${HOME}/.local/state/vscode-memory-watchdog" + mkdir -p "$${state_dir}" + /usr/bin/setsid --fork /bin/bash /memory-watchdog.sh \ + >"$${state_dir}/boot.log" 2>&1 + echo "memory watchdog started in $${WATCHDOG_MODE:-observe} mode; state in $${state_dir}" + EOT +} + +# Weekly garbage collection of ~/.vscode-server, which grows without bound and +# inflates the dentry/inode slab. +# +# The split is deliberate: the logic operates on a personal directory and lives +# in the operator's dotfiles repo, while the schedule has to live here because +# coder_script's cron is the only scheduler this pod has. Missing script => no-op, +# so this resource is safe before the dotfiles side lands. +resource "coder_script" "vscode_server_gc" { + agent_id = coder_agent.main.id + display_name = "vscode-server GC" + icon = "/icon/code.svg" + # Coder's cron is 6-field (seconds first), not the usual 5. Sundays at 04:00. + cron = "0 0 4 * * 0" + script = <<-EOT + set -u + gc="$${HOME}/.local/bin/vscode-server-gc" + if [ -x "$${gc}" ]; then + "$${gc}" + else + echo "no $${gc}; skipping" + fi + EOT +} From 64e3834573e30a05f5ac8535e3cfda0d16af864a Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Sun, 16 Aug 2026 20:01:15 +0000 Subject: [PATCH 02/10] fix: drop resources_monitoring from workspace agent Alerting and notification routing are deliberately not wired on this cluster. resources_monitoring is a notify-on-threshold mechanism, and it evaluates (memory.current - inactive_file) / memory.max, which is inflated by page cache -- so it would notify on an idle workspace. The Memory Headroom metadata tile already surfaces the honest number without pushing anything at anyone. --- CLAUDE.md | 2 +- templates/kubernetes/homelab-workspace/coder-agent.tf | 10 ---------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 792a0c5a..c21f97bf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,7 +62,7 @@ Quick orientation map — for what each piece is *for* and the decisions behind | `terraform.tf` | Provider requirements/versions (Renovate-managed) | | `main.tf` | `coder_workspace`/`coder_workspace_owner` data sources, shared labels/path locals | | `parameters.tf` | User-facing `coder_parameter` inputs + sanitization locals | -| `coder-agent.tf` | `coder_agent` resource: startup script, `coder stat` metadata, `resources_monitoring` | +| `coder-agent.tf` | `coder_agent` resource: startup script, `coder stat` metadata | | `deployment.tf` / `configmap.tf` | Kubernetes Pod spec, volumes, ConfigMap | | `env.tf` | `coder_env` resources exposed to the agent | | `scripts.tf` | `coder_script` resources — the memory watchdog daemon and the weekly `vscode-server` GC schedule | diff --git a/templates/kubernetes/homelab-workspace/coder-agent.tf b/templates/kubernetes/homelab-workspace/coder-agent.tf index e495d311..4f0a1924 100644 --- a/templates/kubernetes/homelab-workspace/coder-agent.tf +++ b/templates/kubernetes/homelab-workspace/coder-agent.tf @@ -61,14 +61,4 @@ resource "coder_agent" "main" { timeout = 1 } - # A coarse backstop, not the real signal. Coder computes - # (memory.current - inactive_file) / memory.max, which is still inflated by - # active_file, so the threshold is 95 rather than 90: at 90 this pod would - # alert continuously while sitting at 23% unreclaimable and zero PSI. - resources_monitoring { - memory { - enabled = true - threshold = 95 - } - } } From 0d9d2e2f28f7f840a4c25440b50c2151ba1fa99c Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Mon, 17 Aug 2026 00:08:17 +0000 Subject: [PATCH 03/10] fix: select watchdog targets by executable path, not by process name The watchdog had never been run against a real VS Code process tree. A live capture falsified two assumptions it was built on. comm is MainThread for every node process in the server tree, never node, because V8 renames its main thread with prctl(PR_SET_NAME). Root selection preferred comm=node and fell back to the first /proc-glob match, so on a real tree the preference never fired and the fallback decided everything - electing whichever matching process happened to sort first, which against a fixture carrying a plausible decoy is a `cat` of the server path. Selection now ignores comm entirely and keys on argv[0], the path VS Code launched the process with. Every root is collected rather than one elected, because --reconnection-grace-time keeps a disconnected server alive for eight hours, and a pty host is only excised inside a tree that was discovered. A provisioned workspace also has two unrelated node installations: VS Code's bundled one under ~/.vscode-server, and mise's on PATH, which is what repo tooling and long-running agent sessions run on. Anything asking "is this node" would have classified an agent session spawned by an extension - a child of the extension host, not of the pty host - as a sheddable editor helper. A process is now eligible only if its own binary lives under ~/.vscode-server. Native extension helpers such as terraform-ls get a role of their own. They are shed at L2 like any other restartable helper but given no RLIMIT_DATA ceiling: V8 turns ENOMEM into a fatal heap OOM the editor offers to recover from, while a Go runtime turns it into an abrupt abort with no such affordance. Also from the live runs: negative headroom projections printed as "-27.-79 GiB", and observe mode re-logged every proposed ceiling every cycle, burying the tier transitions the log exists to record. Two comments asserted that python3, flock and awk were unavailable in the image; all three are present, and the real constraint is only that PATH is brew-shadowed. The fixtures are transcribed from a captured tree rather than hand-written, and comm tolerance, the operator's runtime and multiple concurrent servers each have an assertion paired with the mutation that flips it. --- CLAUDE.md | 13 +- DESIGN.md | 4 +- .../script-memory-watchdog-test.sh | 345 +++++++++++++++--- .../script-memory-watchdog.sh | 197 ++++++++-- 4 files changed, 480 insertions(+), 79 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c21f97bf..5bffb45c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,6 +32,12 @@ tflint --config=../../../.tflint.hcl CI (`.github/workflows/lint.yaml`) runs the same checks per file-type via reusable workflows in `ppat/github-workflows`, scoped to changed files on PRs, or everything on `workflow_dispatch`/schedule. +One job there is not a linter: `watchdog-tests` runs `script-memory-watchdog-test.sh` and fails the build on the first failed assertion. It is repo-local rather than a reusable workflow because `ppat/github-workflows` has nothing for "execute a test script", and the suite needs only bash and a writable `TMPDIR`: + +```bash +./templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh +``` + There is no local way to build/publish the image or push the Coder template — see [TESTING.md](TESTING.md) for how a change actually gets exercised (including the `test_mode` flow), and the **Release flow** section below for how it ships for real. ## Commit messages @@ -68,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). **Ships in observe-only mode**: it measures and logs, and sets no limits and sends no signals until `WATCHDOG_MODE=enforce` | -| `script-memory-watchdog-test.sh` | Fixture tests for the watchdog's arithmetic and process selection. Run it by hand (`./script-memory-watchdog-test.sh`); nothing in CI runs it | +| `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-tests` job in `.github/workflows/lint.yaml` | **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. @@ -81,8 +87,9 @@ 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_*` regex allowlist is the only thing stopping `system_packages`/`preferred_nodes` from injecting shell metacharacters into the init container — any new list-type parameter must go through the same decode-then-validate step. +- `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 thresholds are absolute bytes, not percentages, because the page cache a workload needs is a property of the workload rather than of the limit — which also means the 4 GiB memory parameter needs its own numbers, and the script logs a warning when it detects that mismatch. +- **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`. - 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`'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 dd138204..1e1b3c64 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -75,7 +75,9 @@ The two obvious fixes are both unreachable from inside this container. Throttlin 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. -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 ships in an observe-only mode — measuring and logging, changing nothing — so that the thresholds at which it acts get set from a week of this workload's own data rather than from a guess. 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. +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. + +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 — so that the thresholds at which it acts get set from a week of this workload's own data rather than from a guess. That default is a workspace parameter rather than a constant, because the thresholds are absolute byte counts sized for the larger pod, and the same setting that suits it sits permanently near the first tier on a smaller one. 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. ## Outcomes targeted diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh index bc9cd39c..94862861 100755 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh @@ -6,15 +6,22 @@ # # ./script-memory-watchdog-test.sh # -# CI does not run this - there is no test stage in this repo (see TESTING.md). -# 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. +# CI runs it too, in the `watchdog-tests` job of .github/workflows/lint.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. # # 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 # exclusion is paired with the mutation that must flip it. +# +# The fixtures are transcribed from a live workspace, not written from a reading +# of how VS Code ought to behave. That distinction is not stylistic: the first +# 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. set -uo pipefail SELF_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" @@ -118,44 +125,117 @@ EOF done } -# A representative tree. The hog is three levels below ptyHost, exactly like a -# tmux session or an agent started from a VS Code integrated terminal. +# 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. +# +# 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 -# +- 40 server-main.js -# | +- 41 extensionHost -# | | +- 44 tsserver.js -# | | +- 45 yaml-language-server -# | +- 42 fileWatcher -# | +- 43 ptyHost <- excised, with its whole subtree -# | +- 50 bash -# | +- 51 tmux: server -# | +- 52 claude -# | +- 53 node (the hog) -# +- 60 claude (outside the tree) +# +- 30 bash -l +# | +- 31 sh +# | +- 32 code- command-shell (the CLI, not the server) +# | +- 33 code- agent host +# | +- 34 sh .../bin/code-server --start-server +# | +- 40 server-main.js <- the root +# | +- 41 extensionHost +# | | +- 44 tsserver.js +# | | +- 45 yaml languageserver.js +# | | +- 46 terraform-ls (native, not node) +# | | +- 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 +# | +- 50 bash +# | +- 51 tmux: server +# | +- 52 claude +# | +- 53 node (the hog) +# +- 60 claude (outside the tree) +# +# 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. build_tree() { - local dir=$1 ptyhost_arg=${2:---type=ptyHost} - local srv="/home/coder/.vscode-server/cli/servers/Stable-abc/server/out/server-main.js" + local dir=$1 ptyhost_arg=${2:---type=ptyHost} nc=${3:-MainThread} + local vsc="/home/coder/.vscode-server" + local sdir="${vsc}/cli/servers/Stable-abc123/server" + local srv="${sdir}/out/server-main.js" + local ext="${vsc}/extensions" + # 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" add_proc "$dir" 1 0 coder 14208 ./coder agent - # A decoy with a lower pid that merely *mentions* both marker strings, and a - # ptyHost flag inside a larger argument. Naive substring matching would elect - # it as the server root and mis-scope every decision that follows. + # Mentions both marker strings, and carries a ptyHost-like flag inside a larger + # argument. Substring matching over the joined command line would elect it. add_proc "$dir" 3 1 bash 3000000 bash -c \ "tail -f ${srv}.log --type=extensionHost-ish" - add_proc "$dir" 40 1 node 300000000 /usr/bin/node "$srv" --host localhost - add_proc "$dir" 41 40 node 1500000000 /usr/bin/node "$srv" bootstrap-fork --type=extensionHost - add_proc "$dir" 44 41 node 2000000000 /usr/bin/node /home/coder/.vscode-server/extensions/ms-ts/tsserver.js - add_proc "$dir" 45 41 node 400000000 /usr/bin/node /home/coder/.vscode-server/extensions/redhat/yaml-language-server - add_proc "$dir" 42 40 node 250000000 /usr/bin/node "$srv" bootstrap-fork --type=fileWatcher - add_proc "$dir" 43 40 node 100000000 /usr/bin/node "$srv" bootstrap-fork "$ptyhost_arg" + # Harder: the server path appears here as a whole argv element. Only argv[0] + # separates this from the real thing. + add_proc "$dir" 4 1 cat 3000000 cat "$srv" + + add_proc "$dir" 30 1 bash 8000000 /bin/bash -l + add_proc "$dir" 31 30 sh 3000000 sh + add_proc "$dir" 32 31 code-abc123 60000000 \ + "${vsc}/code-abc123" command-shell --reconnection-grace-time 28800 \ + --cli-data-dir "${vsc}/cli" --parent-process-id 31 + add_proc "$dir" 33 32 code-abc123 40000000 \ + "${vsc}/code-abc123" --cli-data-dir "${vsc}/cli" agent host + add_proc "$dir" 34 32 sh 3000000 sh "${sdir}/bin/code-server" \ + --connection-token=remotessh --start-server --enable-remote-auto-shutdown + + 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" \ + --dns-result-order=ipv4first "${sdir}/out/bootstrap-fork" \ + --type=extensionHost --transformURIs --useHostProxy=false + add_proc "$dir" 44 41 "$nc" 2000000000 "${sdir}/node" \ + "${ext}/ms-vscode.typescript/lib/tsserver.js" --useInferredProjectPerProjectRoot + add_proc "$dir" 45 41 "$nc" 400000000 "${sdir}/node" \ + "${ext}/redhat.vscode-yaml-1.24.0/dist/languageserver.js" --node-ipc --clientProcessId=41 + add_proc "$dir" 46 41 terraform-ls 800000000 \ + "${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 + # image without dotfiles; mise's node and VS Code's bundled node are unrelated + # installations, and the operator's agent sessions run on the former. Both of + # these are children of the extension host and neither is under ptyHost, so + # nothing about tree position saves them - only argv[0] does. + add_proc "$dir" 47 41 "$nc" 2500000000 "$mnode" \ + "${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" \ + "${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 - add_proc "$dir" 53 51 node 3000000000 node -e "const a=[];setInterval(()=>a.push(Buffer.alloc(1)),1)" + add_proc "$dir" 53 51 "$nc" 3000000000 node -e "const a=[];setInterval(()=>a.push(Buffer.alloc(1)),1)" add_proc "$dir" 60 1 claude 700000000 claude } +# A second, stale server left behind by --reconnection-grace-time, on a different +# commit. Its ptyHost subtree must be excised too, which only happens if it is +# discovered as a root in its own right. +add_second_server() { + local dir=$1 + local sdir="/home/coder/.vscode-server/cli/servers/Stable-def456/server" + add_proc "$dir" 70 1 MainThread 200000000 "${sdir}/node" \ + "${sdir}/out/server-main.js" --connection-token=remotessh --start-server + add_proc "$dir" 71 70 MainThread 90000000 "${sdir}/node" \ + "${sdir}/out/bootstrap-fork" --type=ptyHost --logsPath /home/coder/.vscode-server/data/logs/x + add_proc "$dir" 72 71 bash 10000000 /bin/bash -l + add_proc "$dir" 73 70 MainThread 500000000 "${sdir}/node" \ + "${sdir}/out/bootstrap-fork" --type=fileWatcher +} + load_watchdog() { WATCHDOG_SOURCE_ONLY=1 \ WATCHDOG_CGROUP_DIR="$1" \ @@ -170,11 +250,7 @@ scan_fixture() { read_cgroup_memory read_cgroup_pressure read_process_table - SERVER_TREE=() - if find_server_root; then - subtree_of "$SERVER_PID" SERVER_TREE - read_rss "${!SERVER_TREE[@]}" - fi + build_server_tree compute_protected } @@ -209,6 +285,12 @@ test_measurement() { 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" + # 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" + read_cgroup_pressure assert_eq 0 "$M_PSI_CENTI" "psi full avg10 parses as 0" @@ -233,17 +315,53 @@ test_selection() { load_watchdog "${WORK}/cg" "$pdir" scan_fixture - assert_eq 40 "$SERVER_PID" "server root found by argv element, not by substring" - assert_eq 10 "${#SERVER_TREE[@]}" "server tree spans every descendant, ptyHost included" - if [[ -n ${SERVER_TREE[3]:-} ]]; then - bad "a process that merely mentions the server path joined the tree" - else - ok "a process that merely mentions the server path stays out of the tree" - fi + # The regression this whole fixture exists for: on a real tree no process has + # comm=node, so any rule that preferred or required it fell through to picking + # the first /proc-glob match - which is decoy pid 4, not the server. + 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" + + local d + for d in 3 4; do + if [[ -n ${SERVER_TREE[$d]:-} ]]; then + bad "decoy pid ${d} joined the tree" + else + 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" + else + ok "ancestor pid ${d} stays out of the tree" + fi + done + role_of 3 assert_eq other "$ROLE" "a --type=extensionHost-ish argument is not read as a role flag" role_of 43 assert_eq ptyHost "$ROLE" "the real --type=ptyHost argument is" + role_of 40 + assert_eq serverMain "$ROLE" "the root reads as serverMain" + role_of 41 + 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 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; @@ -265,11 +383,11 @@ test_selection() { assert_protected 1 yes "pid 1 is protected" assert_protected 60 yes "a claude outside the tree is protected" - assert_eq "44 45 42" "$(candidate_pids L2)" \ + 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 45 40 42" "$(candidate_pids L4)" \ + 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. @@ -299,6 +417,12 @@ test_selection_is_falsifiable() { build_tree "$pdir" --type=notThePtyHost 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 @@ -312,6 +436,138 @@ test_selection_is_falsifiable() { assert_protected 51 yes "tmux is still protected by name with the subtree rule disabled" } +# --------------------------------------------------------------------------- # +# 2b. 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". +# --------------------------------------------------------------------------- # + +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" + build_tree "$pdir" + load_watchdog "${WORK}/cg" "$pdir" + scan_fixture + + role_of 47 + 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 + + # 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. + # 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 + + # 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 +} + +# --------------------------------------------------------------------------- # +# 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. +# --------------------------------------------------------------------------- # + +test_comm_is_not_a_criterion() { + printf 'comm is not a selection criterion\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + local comm n=0 + for comm in MainThread node something-nobody-predicted; do + n=$((n + 1)) + local pdir="${WORK}/proc-comm-${n}" + build_tree "$pdir" --type=ptyHost "$comm" + 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" + 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. +# --------------------------------------------------------------------------- # + +test_two_servers() { + printf 'two concurrent servers\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + local pdir="${WORK}/proc5" + build_tree "$pdir" + add_second_server "$pdir" + load_watchdog "${WORK}/cg" "$pdir" + 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" + else + bad "the second server's tree is not managed at all" + fi +} + # --------------------------------------------------------------------------- # # 4. the tier ladder # --------------------------------------------------------------------------- # @@ -434,7 +690,10 @@ test_observe_mode_is_inert() { main() { test_measurement test_selection + test_operator_runtime_is_never_a_helper test_selection_is_falsifiable + test_comm_is_not_a_criterion + test_two_servers test_tiers test_observe_mode_is_inert 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 16f86928..de910c69 100644 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh @@ -10,11 +10,16 @@ # reachable without privileged:true. See DESIGN.md. The only remaining strategy # is to never reach memory.max, which is what this does from userspace. # -# Dependencies are deliberately tiny, because this runs before (and without) the -# operator's dotfiles: bash 4.4+, /proc, /sys/fs/cgroup, /usr/bin/sleep, coreutils -# mv/rm/mkdir, and - in enforce mode only - /usr/bin/prlimit. No brew, no mise, no -# python3, no flock, no awk. Measurement and process enumeration use bash builtins -# so a scan forks nothing. +# 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. +# +# 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. # # Modes: # observe (default) - measure, publish headroom, log what it *would* have done. @@ -80,6 +85,11 @@ PROJECTION_HORIZON="${WATCHDOG_PROJECTION_HORIZON:-60}" # seconds # NOT AGREED. Enforce mode must not be turned on until these are set from # calibration data: too low kills a healthy extension host mid-edit, too high # makes the mechanism inert. +# +# 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=( [serverMain]=1610612736 # 1.50 GiB [extensionHost]=3221225472 # 3.00 GiB @@ -90,7 +100,7 @@ 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 " +L2_ROLES=" tsserver languageServer fileWatcher extensionHelper " MAX_LOG_LINES="${WATCHDOG_MAX_LOG_LINES:-20000}" MAX_CSV_LINES="${WATCHDOG_MAX_CSV_LINES:-50000}" @@ -100,7 +110,7 @@ MAX_CSV_LINES="${WATCHDOG_MAX_CSV_LINES:-50000}" # globals without having to restate them. declare -gA P_COMM=() P_CMD=() P_RSS=() CHILDREN=() declare -gA SERVER_TREE=() PROTECTED=() -declare -ga PIDS=() CANDIDATES=() +declare -ga PIDS=() CANDIDATES=() SERVER_ROOTS=() SERVER_PID="" TIER=L0 ROLE=other @@ -242,33 +252,74 @@ read_rss() { # selection - pure over the tables above; produces sets, takes no action # --------------------------------------------------------------------------- # -# Sets SERVER_PID to the pid of the remote server entrypoint, or "". +# 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). +# +# Three things about a real server tree that a plausible-looking implementation +# gets wrong, all three confirmed against a live workspace: +# +# - comm is NOT "node". Every node process in the server tree - server-main.js, +# ptyHost, extensionHost, fileWatcher, node-based language servers - reports +# comm=MainThread, because V8 renames its main thread with prctl(PR_SET_NAME). +# Anything that keys off comm=="node" is keying off a state that never occurs. # -# Matching is on a whole argv element, not on a substring of the joined command -# line. Anything that merely mentions the path - a grep over the server's log, an -# editor, a shell running a script that names it - would otherwise be mistaken -# for the root, and the root is what scopes every subsequent decision. A match -# whose comm is `node` wins outright; anything else is only a fallback. -find_server_root() { - local pid arg fallback="" +# - argv[0] is the signal that does hold. VS Code launches the server as +# `/node /out/server-main.js ...`, and argv[0] keeps +# the interpreter's real path. Requiring it to live under /.vscode-server/ (or, +# for a future launcher outside that tree, to be named node) is what keeps a +# `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. +# +# Matching is always on a whole argv element, never on a substring of the joined +# command line. +find_server_roots() { + local pid arg exe local -a argv + SERVER_ROOTS=() SERVER_PID="" + for pid in "${PIDS[@]}"; do [[ ${P_CMD[$pid]} == *"/.vscode-server/"*"out/server-main.js"* ]] || continue argv=() mapfile -d '' -t argv <"${PROC_DIR}/${pid}/cmdline" 2>/dev/null - for arg in "${argv[@]}"; do + ((${#argv[@]} > 1)) || continue + exe=${argv[0]##*/} + [[ ${argv[0]} == *"/.vscode-server/"* || $exe == node || $exe == node[0-9]* ]] || continue + # From argv[1]: the server path appearing as argv[0] would mean the .js file + # is itself being executed as the program, which is not how it is launched. + for arg in "${argv[@]:1}"; do [[ $arg == *"/.vscode-server/"*"out/server-main.js" ]] || continue - if [[ ${P_COMM[$pid]:-} == "node" ]]; then - SERVER_PID=$pid - return 0 - fi - [[ -n $fallback ]] || fallback=$pid + SERVER_ROOTS+=("$pid") break done done - [[ -n $fallback ]] || return 1 - SERVER_PID=$fallback + + ((${#SERVER_ROOTS[@]})) || return 1 + 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. +build_server_tree() { + local root pid + local -A one=() + SERVER_TREE=() + find_server_roots || return 1 + for root in "${SERVER_ROOTS[@]}"; do + subtree_of "$root" one + for pid in "${!one[@]}"; do + SERVER_TREE[$pid]=1 + done + done + read_rss "${!SERVER_TREE[@]}" return 0 } @@ -298,6 +349,31 @@ subtree_of() { return 0 } +# 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. +# +# 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. +# +# 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. +is_vscode_binary() { + local argv0=${P_CMD[$1]:-} + argv0=${argv0%% *} + [[ $argv0 == *"/.vscode-server/"* ]] +} + # The never-signal list. Every action consults this directly, so a defect in # tree-walking still cannot route around it. # @@ -315,6 +391,10 @@ is_never_signal() { coder | claude | chezmoi | sshd | init | systemd) return 0 ;; tmux*) return 0 ;; esac + # Names are a second line of defence, not the first - is_vscode_binary is. They + # are also deliberately loose: `*/claude*` will match a path that merely + # contains /claude, which over-protects rather than under-protects, and that is + # the direction to err in. Nothing here should be load-bearing on its own. case "${P_CMD[$pid]:-}" in *"coder agent"*) return 0 ;; *"/claude"* | "claude" | "claude "*) return 0 ;; @@ -335,6 +415,14 @@ compute_protected() { is_never_signal "$pid" && PROTECTED[$pid]=1 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. + for pid in "${!SERVER_TREE[@]}"; do + is_vscode_binary "$pid" || PROTECTED[$pid]=1 + done + for pid in "${!SERVER_TREE[@]}"; do if [[ ${P_CMD[$pid]:-} == *"--type=ptyHost"* ]]; then subtree_of "$pid" pty @@ -355,6 +443,8 @@ compute_protected() { # direction, whereas over-matching any other role means mis-classifying it. role_of() { local cmd=" ${P_CMD[$1]:-} " + local argv0=${P_CMD[$1]:-} + argv0=${argv0%% *} case "$cmd" in *" --type=ptyHost "* | *"--type=ptyHost"*) ROLE=ptyHost ;; *" --type=extensionHost "*) ROLE=extensionHost ;; @@ -366,7 +456,28 @@ role_of() { # keeps the blast radius of a wrong guess to one restartable helper. *yaml-language-server* | *jsonServerMain* | *-language-server* | *languageserver*) ROLE=languageServer ;; *"out/server-main.js "*) ROLE=serverMain ;; - *) ROLE=other ;; + *) + # 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. + if [[ $argv0 == *"/.vscode-server/extensions/"* ]]; then + ROLE=extensionHelper + else + ROLE=other + fi + ;; esac return 0 } @@ -474,8 +585,18 @@ 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() { - printf '%d.%02d GiB' "$(($1 / GIB))" "$(($1 % GIB * 100 / GIB))" + local v=$1 sign="" + if ((v < 0)); then + sign="-" + v=$((-v)) + fi + printf '%s%d.%02d GiB' "$sign" "$((v / GIB))" "$((v % GIB * 100 / GIB))" } publish_headroom() { @@ -500,6 +621,13 @@ 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 @@ -529,8 +657,15 @@ apply_ceilings() { 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=() + fi return 0 } @@ -592,8 +727,10 @@ shed_load() { # --------------------------------------------------------------------------- # # A coder_script re-runs when the agent restarts without the pod restarting, so -# two watchdogs are otherwise entirely possible. noclobber gives an atomic O_EXCL -# create without flock, which is brew-only here. +# 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. acquire_singleton() { local pidfile="${STATE_DIR}/watchdog.pid" local other="" @@ -649,11 +786,7 @@ scan_once() { fi local projected=$((M_H - du_rate * PROJECTION_HORIZON)) - SERVER_TREE=() - if find_server_root; then - subtree_of "$SERVER_PID" SERVER_TREE - read_rss "${!SERVER_TREE[@]}" - fi + build_server_tree compute_protected decide_tier "$M_H" "$M_PSI_CENTI" "$refault_rate" "$projected" "$now" From d5020773f12083d2e2d4d7177a7cdf1618d5e662 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Mon, 17 Aug 2026 00:08:17 +0000 Subject: [PATCH 04/10] feat: make the memory watchdog mode a workspace parameter The mode was hardcoded in env.tf, so changing it meant a template release. It is now a mutable coder_parameter defaulting to observe, offering only the two valid modes, and passed through the same validate-then-use step as the list parameters - an unrecognised value falls back to the inert mode rather than reaching the agent. Per-workspace rather than per-template because the tier thresholds are absolute byte counts sized for an 8 GiB pod; the same setting sits permanently near L1 on a 4 GiB one. --- templates/kubernetes/homelab-workspace/env.tf | 14 +++++---- .../homelab-workspace/parameters.tf | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/templates/kubernetes/homelab-workspace/env.tf b/templates/kubernetes/homelab-workspace/env.tf index a1b4fcb0..de4306b7 100644 --- a/templates/kubernetes/homelab-workspace/env.tf +++ b/templates/kubernetes/homelab-workspace/env.tf @@ -8,13 +8,15 @@ resource "coder_env" "welcome_message" { # headroom and logs what it would have done; "enforce" additionally sets # RLIMIT_DATA ceilings and sheds load. # -# Deliberately left at "observe". The ceilings and tier thresholds in -# script-memory-watchdog.sh were derived from role and an 8 GiB budget, not from -# measurement - too low kills a healthy extension host mid-edit, too high makes -# the mechanism inert. Flip this only once the numbers have been set from the -# calibration data the watchdog is collecting. +# 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. resource "coder_env" "memory_watchdog_mode" { agent_id = coder_agent.main.id name = "WATCHDOG_MODE" - value = "observe" + value = local.validated_watchdog_mode } diff --git a/templates/kubernetes/homelab-workspace/parameters.tf b/templates/kubernetes/homelab-workspace/parameters.tf index c3215313..6170cc5f 100644 --- a/templates/kubernetes/homelab-workspace/parameters.tf +++ b/templates/kubernetes/homelab-workspace/parameters.tf @@ -61,8 +61,39 @@ data "coder_parameter" "system_packages" { type = "list(string)" } +data "coder_parameter" "memory_watchdog_mode" { + name = "memory_watchdog_mode" + + default = "observe" + display_name = "Memory Watchdog" + description = "What the memory watchdog is allowed to do when the pod runs low on unreclaimable-memory headroom" + 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" + } + option { + name = "Enforce" + 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" + } +} + locals { + # Coder already constrains this to the two option values server-side, but it + # reaches the agent as an environment variable and from there a shell, and it + # is the single switch that decides whether the watchdog may signal processes. + # So it goes through the same validate-then-use step as the list parameters + # 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 + ) ? data.coder_parameter.memory_watchdog_mode.value : "observe" + validated_system_packages = (data.coder_parameter.system_packages.value != "") ? [ for str in jsondecode(data.coder_parameter.system_packages.value) : str if length(regexall("[^a-zA-Z0-9-]", str)) == 0 From cf0f2a7d8aaead468a5aa25b8ae2047a7ae18ce9 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Mon, 17 Aug 2026 00:08:17 +0000 Subject: [PATCH 05/10] ci(github-actions): run the memory watchdog tests in CI The suite guards the two rules that can cost the operator work - which processes count as the editor's, and which of those may be signalled - and nothing ran it, so it could rot silently while staying green in the one place it was consulted. Repo-local rather than a reusable workflow because ppat/github-workflows has nothing for "execute a test script"; the shellcheck workflow only lints. It needs bash and a writable TMPDIR, builds fixtures under mktemp -d, touches no network and finishes in seconds, so it is cheap to leave on and has nothing in it to go flaky. --- .github/workflows/lint.yaml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index e986c42a..bf6b637f 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -37,6 +37,9 @@ jobs: - '**/.terraform-version' - '**/.terraform.lock.hcl' - '.tflint.hcl' + watchdog: + - templates/kubernetes/homelab-workspace/script-memory-watchdog*.sh + - .github/workflows/lint.yaml yaml: - '**.yaml' git_ref: ${{ github.head_ref || github.ref }} @@ -136,6 +139,29 @@ jobs: git_ref: ${{ github.head_ref || github.ref }} tf_dirs: ${{ needs.terraform-dirs.outputs.terraform_dirs }} + # The one job here that runs a test rather than a linter. It is repo-local on + # purpose: ppat/github-workflows has no reusable workflow for "execute a test + # script", and the shellcheck workflow only lints. The suite needs nothing but + # bash and a writable TMPDIR, builds its fixtures under mktemp -d, touches no + # network and finishes in a couple of seconds - so it is cheap enough to leave + # on and has nothing in it to go flaky. + watchdog-tests: + needs: [detect-changes] + if: ${{ github.event_name != 'pull_request' || fromJSON(needs.detect-changes.outputs.results).watchdog_any_changed == 'true' }} + runs-on: ubuntu-24.04 + timeout-minutes: 5 + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 1 + persist-credentials: false + ref: ${{ github.head_ref || github.ref }} + + - name: Run memory watchdog fixture tests + # Exits non-zero when any assertion fails, which is what fails the build. + run: ./templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh + yaml: needs: [detect-changes] if: ${{ github.event_name != 'pull_request' || fromJSON(needs.detect-changes.outputs.results).yaml_any_changed == 'true' }} From 890e76b573c41735f9255209331bf68b097a4400 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Mon, 17 Aug 2026 02:33:00 +0000 Subject: [PATCH 06/10] ci(github-actions): move watchdog fixture tests out of lint.yaml The watchdog suite guards process-selection safety, not code style, so lumping it into lint.yaml as watchdog-tests buried a safety-critical check where nobody browsing linters would expect to find it, and left it exposed to being pruned by anyone tidying that file. Give it its own repo-local workflow, following the same reasoning that kept it repo-local before: ppat/github-workflows has no reusable "run this test script" workflow. lint.yaml's shared detect-changes job exists to serve many linters off one change matrix; a single test job has no need of that machinery, so this instead follows release.yaml's simpler direct pull_request paths filter, scoped to the watchdog script and to changes to this workflow file itself - the same two-path trigger the job had under lint.yaml. --- .github/workflows/lint.yaml | 26 ---------------------- .github/workflows/test.yaml | 43 +++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 26 deletions(-) create mode 100644 .github/workflows/test.yaml diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index bf6b637f..e986c42a 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -37,9 +37,6 @@ jobs: - '**/.terraform-version' - '**/.terraform.lock.hcl' - '.tflint.hcl' - watchdog: - - templates/kubernetes/homelab-workspace/script-memory-watchdog*.sh - - .github/workflows/lint.yaml yaml: - '**.yaml' git_ref: ${{ github.head_ref || github.ref }} @@ -139,29 +136,6 @@ jobs: git_ref: ${{ github.head_ref || github.ref }} tf_dirs: ${{ needs.terraform-dirs.outputs.terraform_dirs }} - # The one job here that runs a test rather than a linter. It is repo-local on - # purpose: ppat/github-workflows has no reusable workflow for "execute a test - # script", and the shellcheck workflow only lints. The suite needs nothing but - # bash and a writable TMPDIR, builds its fixtures under mktemp -d, touches no - # network and finishes in a couple of seconds - so it is cheap enough to leave - # on and has nothing in it to go flaky. - watchdog-tests: - needs: [detect-changes] - if: ${{ github.event_name != 'pull_request' || fromJSON(needs.detect-changes.outputs.results).watchdog_any_changed == 'true' }} - runs-on: ubuntu-24.04 - timeout-minutes: 5 - steps: - - name: Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - fetch-depth: 1 - persist-credentials: false - ref: ${{ github.head_ref || github.ref }} - - - name: Run memory watchdog fixture tests - # Exits non-zero when any assertion fails, which is what fails the build. - run: ./templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh - yaml: needs: [detect-changes] if: ${{ github.event_name != 'pull_request' || fromJSON(needs.detect-changes.outputs.results).yaml_any_changed == 'true' }} diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml new file mode 100644 index 00000000..c1114cf8 --- /dev/null +++ b/.github/workflows/test.yaml @@ -0,0 +1,43 @@ +--- +# yamllint disable rule:line-length +name: test + +# Repo-local rather than a reusable workflow because ppat/github-workflows has +# nothing for "execute a test script" - the shellcheck workflow only lints, +# and lint.yaml's detect-changes/job-per-linter structure exists to share one +# change-detection matrix across many linters, which a single test job has no +# need of. This guards process-selection safety in the memory watchdog (which +# processes count as the editor's, and which of those may be signalled) via a +# fixture suite, not a linter, so it does not belong in lint.yaml. + +on: + pull_request: + paths: + - .github/workflows/test.yaml + - templates/kubernetes/homelab-workspace/script-memory-watchdog*.sh + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + watchdog: + runs-on: ubuntu-24.04 + timeout-minutes: 5 + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 1 + persist-credentials: false + ref: ${{ github.head_ref || github.ref }} + + - name: Run memory watchdog fixture tests + # Exits non-zero when any assertion fails, which is what fails the build. + # Needs nothing but bash and a writable TMPDIR: builds its fixtures under + # mktemp -d, touches no network, finishes in a couple of seconds. + run: ./templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh From 5a019502ff773ed00f8e57ae0ce9e24c826c6635 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Mon, 17 Aug 2026 02:33:26 +0000 Subject: [PATCH 07/10] test: deliberately break an assertion to prove test.yaml can fail Temporary commit to verify the moved watchdog job still fails the build when a fixture assertion breaks. Reverted in the next commit. --- .../kubernetes/homelab-workspace/script-memory-watchdog-test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh index 94862861..3657823c 100755 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh @@ -319,7 +319,7 @@ test_selection() { # comm=node, so any rule that preferred or required it fell through to picking # the first /proc-glob match - which is decoy pid 4, not the server. 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 41 "$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" From 9d623f2646b3c7f303fbbe67ca9fab6ea096ab41 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Mon, 17 Aug 2026 02:33:48 +0000 Subject: [PATCH 08/10] Revert "test: deliberately break an assertion to prove test.yaml can fail" This reverts commit 5a019502ff773ed00f8e57ae0ce9e24c826c6635. --- .../kubernetes/homelab-workspace/script-memory-watchdog-test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh index 3657823c..94862861 100755 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh @@ -319,7 +319,7 @@ test_selection() { # comm=node, so any rule that preferred or required it fell through to picking # the first /proc-glob match - which is decoy pid 4, not the server. assert_eq MainThread "${P_COMM[40]}" "the fixture encodes the real comm value" - assert_eq 41 "$SERVER_PID" "server root found with comm=MainThread, not comm=node" + 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" From b91ad696d5d23d0f78afe8f8dac3288c2d5353c8 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Mon, 17 Aug 2026 02:34:09 +0000 Subject: [PATCH 09/10] docs: point the fixture suite's CI reference at test.yaml Stale after moving the job out of lint.yaml's watchdog-tests. --- .../homelab-workspace/script-memory-watchdog-test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh index 94862861..d72b0167 100755 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh @@ -6,8 +6,8 @@ # # ./script-memory-watchdog-test.sh # -# CI runs it too, in the `watchdog-tests` job of .github/workflows/lint.yaml, -# which fails the build on the first failed assertion. It exists so that the two +# 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. From e5eb198f8a4fd42dff6bb7512f17704b8363c129 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Mon, 17 Aug 2026 02:34:53 +0000 Subject: [PATCH 10/10] docs: fix CLAUDE.md references to the moved watchdog test job Two spots still pointed at the old watchdog-tests job in lint.yaml. --- CLAUDE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5bffb45c..69ec8493 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,7 +32,7 @@ tflint --config=../../../.tflint.hcl CI (`.github/workflows/lint.yaml`) runs the same checks per file-type via reusable workflows in `ppat/github-workflows`, scoped to changed files on PRs, or everything on `workflow_dispatch`/schedule. -One job there is not a linter: `watchdog-tests` runs `script-memory-watchdog-test.sh` and fails the build on the first failed assertion. It is repo-local rather than a reusable workflow because `ppat/github-workflows` has nothing for "execute a test script", and the suite needs only bash and a writable `TMPDIR`: +A separate workflow, `.github/workflows/test.yaml`, runs the one thing here that's a test rather than a linter: its `watchdog` job runs `script-memory-watchdog-test.sh` and fails the build on a failed assertion. It's repo-local rather than a reusable workflow because `ppat/github-workflows` has nothing for "execute a test script", and the suite needs only bash and a writable `TMPDIR`: ```bash ./templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh @@ -75,7 +75,7 @@ Quick orientation map — for what each piece is *for* and the decisions behind | `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-tests` job in `.github/workflows/lint.yaml` | +| `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` | **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.