docs: correct and evidence the memory watchdog's kill-path claims - #882
Merged
Conversation
role_of returned `other` for three real VS Code helpers on the live tree, and
compute_policed skips `other`, so they had no budget, no dwell clock, no signal
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 - launch shapes are
each extension's own choice, so the list is a permanent race against a
vendor-controlled vocabulary that fails silently, one extension at a time. The
default inside the tree is inverted instead: on VS Code's own runtime,
unprotected and unnamed becomes role `treeHelper`. Two things buy off the risk
that creates rather than assuming it away:
- treeHelper is in a new UNKNOWN_ROLES tier, 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, not as
silence. The axis is calibration, not blast radius.
- 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 release that renames
--type=extensionHost yields a process that is unmanaged and loud rather
than one carrying a guessed 512 MiB budget against the 685 MB it holds.
Sweep-log identity for the role is <extension>/<script>, because a dozen
extensions all ship a file called server.js and `server` answers nothing.
Four smaller corrections, each with the assertion that pins it:
- extensionHost's declared budget was 1024 MiB against its own 1069 MiB
resting 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 the one always in force: no effective budget
changes at any pod size, and the flag now flips off at 16 GiB. The suite
asserts the invariant (no declared budget below its own floor) rather
than the flag on one pod.
- disarming a role now finishes an escalation already in flight and starts
no new one. A drill found the opposite by accident: the breaker tripped
between SIGTERM and SIGKILL and the process was left alive, over budget
and half-signalled, recorded as neither killed nor spared. record_kill is
not called again - it is the same kill - and the line says after_disarm.
- the "nothing policed" warning is per episode of blindness rather than a
one-shot latch over a lifetime high-water mark, so a blind spot arriving
on day two of a pod's life is no longer silent.
- `2>/dev/null` now precedes the input redirect it silences on every
per-process /proc read. The old order opened the file first, so a process
exiting mid-sweep produced 160 of the 161 lines in boot.log. Harmless,
but it is noise that would hide a real error. memory.stat/memory.max are
deliberately left alone: a missing cgroup file is worth seeing.
Not done, deliberately: budgets stay per-process rather than per-role-aggregate.
The only multi-process role here is tsserver (two, at 112 + 141 MB against
768 MiB each); a role total makes "which sibling dies" arbitrary; and it does
not compose with a dwell clock keyed per pid:starttime, which is what separates
load from drift. The cheap half is done instead - role_pss_mb and role_peak_mb
in the hourly census - so the evasion, and a stale RESTING_ROLE, become visible
as data. RESTING_ROLE stays hand-measured: a watchdog that re-derived its floor
from observation would raise the limit it enforces exactly when the thing it
enforces against grew.
Every new assertion is paired with the mutation that flips it red, and all
eight were run: removing the treeHelper branch (10 fail), the direct-child
guard (3), the argv[0] condition (5), arming treeHelper under enforce (3),
reverting the declared budget (2), restoring the old disarm ordering (2), the
one-shot visibility latch (2), and the identity breadcrumb (3).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197hA8JPwGMX8wufiQiy6oz
TESTING.md's drill account is replaced with a verified one: verbatim actions.log lines from the 2026-08-21 disposable-workspace drill, the dwell-vs-age distinction between its two kills, the negative space (untouched detached tool-call stand-in and in-budget helper), what the compressed thresholds do and don't cost the result, and what the drill did not exercise. It also checks PR #865's merged claim that a "739 MB helper" drill was "verified end to end... in Loki" against the retained Loki record. Neither an event=kill/disarmed line nor the string 739 appears anywhere in that record; the only kill/disarm lines near the PR's merge time are the already-documented fixture-suite stdout leak, and no workspace named test ever emitted one. The claim is not corroborated, though a seam-respecting drill would also leave no trace, so the record cannot rule the description in or out - only narrow what is checkable. calibration.csv's du_bytes_per_s column is an instantaneous per-sample rate, not a drift rate: naively averaged it overstates drift roughly fourfold (19.8 kB/s vs the 18.4 MB/hour a long census window gives). Documented next to the column's declaration in the script and as a CLAUDE.md gotcha, since nothing else guards a reader from the same misreading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197hA8JPwGMX8wufiQiy6oz
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Three corrections to this project's evidentiary record about the memory watchdog, stacked on #881 (touches the same
CLAUDE.md/DESIGN.mdregions, so based on that branch rather thanmainto avoid a guaranteed conflict).1. A documented claim the append-only record does not corroborate
PR #865's body claims a live drill killed "a drifted 739 MB helper... three times", disarmed on the third, and states "the daemon's own lines from the test workspace are in Loki". Querying the full retained Loki record (
{namespace="coder"} |= "component=memory-watchdog", 2026-07-01 through today):739never appears, anywhere, ever.event=kill/event=would-kill/event=refused/event=disarmedline exists from any workspace namedtest, at any time.event=note reason=fixture-leak, naming fixture pids 41/61 at 1907/1583 MB — the already-documented fixture-suite stdout leak, not a drill result.event=budget role=extensionHost budget_mb=1069 pod_share_mb=512 floored=1(also cited as drill evidence) is not distinguishing: every disposabletest_modeworkspace since has printed that exact line at startup regardless of any kill, becausefloored=1was a constant at that pod size — the bug fix: govern the in-tree helpers the watchdog could not name #881 fixes.This doesn't prove the drill didn't happen — a drill run through the fixture harness's required
WATCHDOG_STDOUT_PATHseam would correctly leave nothing in Loki, indistinguishable from one that never ran.TESTING.mdnow says exactly this: what's checkable, what was checked, and that it doesn't corroborate the specific claim.#865's body is also edited in place (not just referenced from here): a correction is inserted directly under its
## Evidencesection, and a forward pointer added where the same claim is repeated under## Sweep log, and action lines that leave the pod. This follows the operator's standing rule that PR bodies are living records, corrected in place rather than via comment, even once merged — and #865 itself already sets this precedent internally (see its## Where acute protection now sits: "which is why an earlier revision of this description claimed the opposite from a single old-pod sample").2. A real observed result written down nowhere
A drill run today (2026-08-21) on a disposable
watchdog-drillworkspace did exercise the full kill path, and its Loki lines were re-fetched and matched against the pod's localactions.log.TESTING.mdnow carries the verbatimevent=kill/event=disarmedlines, the dwell-bound-vs-age-bound distinction between the two kills, asweep.logexcerpt showing a dip resets the dwell clock, the negative space (an untouchedsetsid-detached stand-in for a Bash tool call and an untouched in-budget helper), what compressing the thresholds costs the result (production code paths and arithmetic, not production timings), and what wasn't exercised (production timings, the VS Code tree,observemode, the global breaker,event=refused).3. A misleading column in the watchdog's own output
calibration.csv'sdu_bytes_per_sis an instantaneous per-sample rate, not a drift rate — naively averaged it reads ~68 MB/hour on the live pod, vs ~18.4 MB/hour computed from the hourly census over a 26.1h window, a 3.7x overstatement driven by short GC/allocation spikes. Documented at the column's declaration inscript-memory-watchdog.shand as aCLAUDE.mdgotcha, matching the existingU-computation entry.Not done
DESIGN.md: checked fix: govern the in-tree helpers the watchdog could not name #881's diff first, and neither the Loki-verification claim nor the drill narrative lives there — only inTESTING.mdand feat: police helper-process drift instead of chasing OOM spikes #865's body (now corrected).Verification
pre-commit run --all-files— green (baseline at the base commit checked first, also green).Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com