Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-container-entrypoint.sh` | The workspace container's `command`. Wipes `/tmp` and `exec`s Coder's generated `/workspace-init.sh` — the wipe must precede the agent, see the gotcha below |
| `script-memory-watchdog.sh` | Userspace memory watchdog — see [DESIGN.md](DESIGN.md#design-tensions-and-decisions). It bounds the **standing population of restartable helpers** (per-role PSS budgets, ten-minute dwell, per-role circuit breaker) and records every per-process sweep. It does **not** try to prevent an acute OOM. `memory_watchdog_mode` selects `observe` / `enforce` (helpers — the default) / `enforce-all` (helpers + editor) |
| `script-memory-watchdog.sh` | Userspace memory watchdog — see [DESIGN.md](DESIGN.md#design-tensions-and-decisions). It bounds the **standing population of restartable helpers** (per-role PSS budgets, ten-minute dwell, per-role circuit breaker) and records every per-process sweep. It does **not** try to prevent an acute OOM. `memory_watchdog_mode` selects `observe` / `enforce` (helpers — the default) / `enforce-all` (helpers + editor + the uncalibrated `treeHelper` role) |
| `script-memory-watchdog-test.sh` | Fixture tests for the watchdog's arithmetic, process selection, budgets, dwell and circuit breaker. Run by hand (`./script-memory-watchdog-test.sh`) and by the `watchdog` job in `.github/workflows/test.yaml`. `kill` is shadowed by a function throughout — the fixture pids are real pids in whatever container runs the suite |
| `script-vscode-server-gc.sh` | Weekly GC of `~/.vscode-server` (interrupted downloads, superseded server versions/extensions, orphaned CLI binaries — see the script's own header for the exact signal per class, and the `coder_script.vscode_server_gc` comment in `scripts.tf` for why it's template-owned rather than dotfiles-owned) |

Expand All @@ -92,8 +92,14 @@ Things that look arbitrary in the code but are load-bearing (full reasoning in [
- `parameters.tf`'s `local.validated_*` allowlist is the only thing stopping `system_packages`/`preferred_nodes` from injecting shell metacharacters into the init container — any new parameter whose value reaches a shell must go through the same validate-then-use step.
- `script-memory-watchdog.sh` computes headroom as `memory.max − U`, where `U` sums only the *unreclaimable* fields of `memory.stat` (`anon`, `shmem`, `unevictable`, `slab_unreclaimable`, `kernel_stack`, `pagetables`, `sec_pagetables`, `percpu`, `sock`). Do not "simplify" it to `memory.current` or to `memory.stat`'s `kernel` roll-up: on the live pod those read 96% and 42% of the limit while true `U` is 28%. Nothing acts on this number any more — it is pod-level context for the per-process rows and the honest figure published in the workspace UI.
- **The watchdog is a drift policer, not an OOM preventer, and the difference is measured.** The graded L1–L4 shedding ladder that used to be here was removed, not tuned: the recorded kills are 70–220 MB/s spikes that go from idle to dead inside a minute, a live reproduction climbed the ladder correctly and logged `no-candidates` because the runaway was not in the tree it managed, and the entire editor tree it could shed is ~0.7 GiB — five seconds of that growth. Before re-adding anything reactive, establish that a poll loop can see the event at all. What the loop *is* good at is MB-per-minute growth in the standing population, which is what it now does.
- **Budgets are per role, in PSS, and never below a role's measured *resting* size — not its fresh size.** Fresh tree: extension host 471 MB PSS, serverMain 160 MB, ptyHost 36 MB, file watcher 34 MB. Resting tree (reconnected, idle, 8 GiB pod): extension host **713 MB**, file watcher 88 MB, language server 54 MB, tree total 1093 MB. A uniform 512 MB budget would sit 40 MB above where the extension host *starts* and 200 MB below where it *lives*, which is the same class of defect as the earlier `RLIMIT_DATA` ceiling that landed below what an idle file watcher already held — "calibrated against fresh, deployed against resting" has now appeared twice in this design, so `RESTING_ROLE` holds the resting figures and nothing else. Each budget is `max(min(role budget, memory.max / 8), resting × 1.5)`; on the 8 GiB pod that makes the extension host 1069 MiB rather than the 1024 MiB share, and the derivation records which budgets the floor lifted (`floored=1`), because that means the pod is too small to bound the role at its intended share. The tests assert the *property* — no budget at or below resting, at any pod size — rather than the arithmetic. Override one role with `WATCHDOG_BUDGET_<role>`; PSS (`smaps_rollup`) is the comparison, not RSS and not `VmData`.
- **Budgets are per role, in PSS, and never below a role's measured *resting* size — not its fresh size.** Fresh tree: extension host 471 MB PSS, serverMain 160 MB, ptyHost 36 MB, file watcher 34 MB. Resting tree (reconnected, idle, 8 GiB pod): extension host **713 MB**, file watcher 88 MB, language server 54 MB, tree total 1093 MB. A uniform 512 MB budget would sit 40 MB above where the extension host *starts* and 200 MB below where it *lives*, which is the same class of defect as the earlier `RLIMIT_DATA` ceiling that landed below what an idle file watcher already held — "calibrated against fresh, deployed against resting" has now appeared twice in this design, so `RESTING_ROLE` holds the resting figures and nothing else. Each budget is `max(min(role budget, memory.max / 8), resting × 1.5)`, and the derivation records which budgets the floor lifted (`floored=1`), which is documented to mean the pod is too small to bound that role at its intended share. **That reading only holds while every declared budget already clears its own resting floor, and it did not: `extensionHost` was declared at 1024 MiB against a 1069 MiB floor, so `floored=1` was true at 4, 8, 16 and 64 GiB alike — a property of the constants reported as a property of the pod.** The declared number is now 1069 MiB, the one that was always in force; no effective budget changed at any pod size, and the flag now flips off at 16 GiB. The tests assert the *properties* — no budget at or below resting, and no declared budget below its own floor, at any pod size — rather than the arithmetic. Override one role with `WATCHDOG_BUDGET_<role>`; PSS (`smaps_rollup`) is the comparison, not RSS and not `VmData`.
- **A kill needs ten minutes of continuous over-budget dwell, and three kills of one role inside an hour disarm that role.** The dwell is what separates drift from load — a language server that balloons while indexing and hands the memory back must survive. The breaker is what stops the failure that would make this actively harmful: kill the extension host → VS Code restarts it → it reloads every extension → it exceeds again → kill, a loop that arrives looking exactly like the watchdog working. It disarms and reports rather than widening its own budget, because a mechanism that raises the limit it is enforcing has stopped enforcing.
- **Inside the server tree, an unrecognised process is *governed*, not invisible — that default is inverted on purpose, and its risk is bought off by arming rather than by hope.** `role_of` returned `other` for three real helpers on the live tree, and `compute_policed` skips `other`, so they had no budget, no dwell clock and no warning: a `tamasfe.even-better-toml` `server.js` at **280 MB PSS** (already above the 256 MiB `languageServer` budget a matching pattern would have given it), tsserver's `typingsInstaller.js` at 61 MB, and the built-in `markdown-language-features` server at 54 MB. Widening the pattern list is the easy fix and the wrong one: the set of launch shapes is the union of every extension's own choices, so a pattern list is a permanent race against a vendor-controlled vocabulary that fails *silently*. So anything in the tree, on VS Code's own runtime, unprotected and unnamed becomes role `treeHelper`. Two things make that safe rather than merely bold: (1) `treeHelper` is in `UNKNOWN_ROLES`, armed **only** under `enforce-all`, because its 512 MiB budget is a reasoned guess and no member has been measured at rest — under `enforce` (the template default) a coverage gap arrives as `event=would-kill armed=no` in the container log instead of as silence; (2) a **direct child of a server root is excluded structurally** — core forks (`fileWatcher`, `extensionHost`, `ptyHost`) are exactly the direct children of `server-main.js`, and no extension helper is, so a VS Code upgrade that renames `--type=extensionHost` makes that process unmanaged and loud rather than managed on a 512 MiB budget it holds 685 MB against. Its sweep-log identity is `<extension>/<script>`, because a dozen extensions all ship a file called `server.js` and `server` answers nothing in a post-mortem.
- **Per-process, never per-role-aggregate — and that hole is a resourced decision, not an oversight.** Each pid is compared against `BUDGET[$role]` alone, so N same-role processes can each sit just under budget indefinitely. Not fixed, for three reasons: the only multi-process role here is `tsserver` (two of them, 112 + 141 MB against 768 MiB *each*); a role-level total makes "which sibling dies" arbitrary when three each contributed a third; and it does not compose with a dwell clock keyed per `pid:starttime`, which is what separates load from drift. What *is* done is the cheap half — `role_pss_mb` in the hourly census — so the evasion becomes visible as data before anyone designs for it.
- **Disarming a role finishes the kill it already started; it does not begin a new one.** A drill found the opposite by accident: the breaker tripped between a process's SIGTERM and its SIGKILL, the disarm branch short-circuited ahead of the escalation, and the process was left alive, over budget and half-signalled — recorded as neither killed nor spared, and undocumented either way. The escalation check now precedes the disarm check, `record_kill` is deliberately *not* called again (it is the same kill, already counted), and the line carries `after_disarm=yes`. Completing one escalation cannot start the loop the breaker exists to stop.
- **`RESTING_ROLE` is a hand measurement with a review trigger, never a derived one.** A watchdog that re-derived its own floor from what it observed would raise the limit it is enforcing exactly when the thing it enforces against grew — the move the breaker already refuses, one level removed. The trigger is `role_peak_mb` in the census and `peak_pss` in the summary file: when a role's peak sits persistently above its constant while never dwelling over budget, a human re-measures. Checked at the time of writing: the live extension host read 685 MB at 27.6 h uptime, i.e. *below* the recorded 713 MB, so the reference is not stale and the drift is not monotonic — V8 hands memory back.
- **The "nothing policed" warning is per episode of blindness, not per process lifetime.** It was a one-shot latch set at the warmup sweep whether or not it warned, testing a lifetime high-water mark — so a watchdog that policed something once in its first half hour could never warn again however blind it went. A pod lives for days and the interesting blind spot is the one that arrives on day two, when an upgrade changes a launch shape. The clock now restarts when blindness starts and re-arms when anything is policed again.
- **`2>/dev/null` must precede the input redirect it is meant to silence, not follow it.** `read -r line <"$entry/stat" 2>/dev/null || continue` opens the file *before* stderr is redirected, so a process exiting between the `/proc` listing and the read makes bash print its own error to inherited stderr; `|| continue` still handles it, so the bug is pure noise — 160 of 161 lines of `boot.log` — that would hide a real error. `release_singleton` already carried a comment showing this was understood. Every per-process `/proc` read now orders it correctly; `memory.stat`/`memory.max` deliberately do not, because a missing cgroup file is a genuine error worth seeing.
- **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-<commit>/server/`, and mise's on `PATH`, which is what repo tooling and the operator's agent sessions run on. (There is no `/usr/bin/node`, and nothing named `node` on `PATH` at all without dotfiles.) `comm` is `MainThread` for every node process in a real tree, never `node`, because V8 renames its main thread; nothing may key off it. `script-memory-watchdog-test.sh` asserts this three ways, each paired with the mutation that flips it.
- **Helpers spawned by an agent session are the second policed population, and the walk that finds them stops at two boundaries: a shell, and a change of session id.** MCP servers live nowhere near `~/.vscode-server`, so the tree-scoped selection could not see them — the largest single offender ever measured was 1.66 GB of python. The session rule is measured, not assumed: on the live workspace a session root has `sid` = its login shell's session, while every Bash tool call has `pgid == sid == its own pid`, because Claude Code detaches each one. That is what keeps an in-flight build out of the policed set even when the tool call's shell has exec'd itself away, which the shell test alone would miss. If Claude Code ever stops detaching, the walk polices *nothing* rather than the wrong thing, and the visibility warning in `actions.log` says so.
- **Identity guards are absolute; the two positional rules are not, and the distinction is deliberate.** `pid 1`, the coder agent, tmux, `claude` session roots, agent payloads and the watchdog's own kin may never be signalled by anything. The ptyHost subtree and "does not run VS Code's own binary" bound the *editor* selection only, so that an MCP server is policed the same whether its session was started with `coder ssh` or in a VS Code terminal — sparing one set because of which terminal it came from would make the mechanism miss half its cases silently. Everything the ptyHost rule genuinely protects (shells, multiplexers, sessions, tool calls) is still covered by identity guards and by the shell/session boundaries.
Expand Down
Loading
Loading