Skip to content

feat(gauntlet): name leftover CPU-heavy processes before the layers and on failure (GH #158) - #632

Merged
wasikarn merged 15 commits into
developfrom
claude/stray-proc-sensor
Oct 26, 2026
Merged

wasikarn merged 15 commits into
developfrom
claude/stray-proc-sensor

Conversation

@wasikarn

@wasikarn wasikarn commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

Turns the background-flood mistake, which has now happened twice, into a committed check and a committed rule (repeated-mistake doctrine in CLAUDE.md). Refs #158.

Why

Twice, a stress run typed into the Bash tool's zsh left its workers running:

  • 2026-10-05: 289 processes at load about 248 on 12 cores. wait $pids does not word-split in zsh.
  • 2026-10-25: 4 yes processes at 100% CPU for about 4 h. kill $(jobs -p) under zsh -c found no jobs.

The load failed every session's timing rows. PR #581's push failed again and again, and it took hours before anyone traced the cause to stray processes.

Prior work checked

What

  • scripts/_lib/stray-procs.sh: stray_procs_report groups your own processes older than GAUNTLET_STRAY_MIN_SECS (1800 s) by command name. Any group whose %CPU sums to GAUNTLET_STRAY_MIN_CPU (80) or more is listed on stderr, with its count, summed CPU, oldest age and up to 8 PIDs, plus the load average.
    • Catches both incident shapes: one process at full CPU, and a flood of many processes at a few percent each. 43 idle SetStoreUpdateService copies sum to about 0 and stay quiet.
    • Warn-only: it never kills anything and always returns 0, so a set -e caller isn't aborted. If ps fails, it prints one stray-process check skipped (ps failed) line. If the load can't be read, the warning says load unknown.
    • Bash 3.2, and it parses [[dd-]hh:]mm:ss etime (macOS has no etimes).
  • run-gauntlet.sh: calls it before the layers start, and again on failure. The second call sits above the failed-at:/failing: lists, so the last line is still the failing: list (GH gauntlet: print which test files failed #387).
  • repo-gotchas.md: adds the rule half: run background-load scripts with bash <file>, collect pids+=($!), kill them in a trap ... EXIT, and confirm no strays are left. The check only catches a stray after the fact, so the rule stays.
  • env-vars.md: documents the two thresholds.
  • No version bump: per branching-model.md, branches don't bump the version; merge-pr.sh does it at merge time.

Known limits

  • Only processes 30+ min old count. The default started at 600 s, but the first live run then named a busy 13-minute headless claude run, which is real work, so it went up to 1800 s.
  • A respawning loop can slip through. For example, a few low-CPU parents plus many short-lived children, each younger than the threshold.
  • Grouping uses the first word of the command. A path with spaces groups under its first word, such as Google for Chrome. It also merges an app's own processes: 6 Chrome renderers at 15% each are named as one group at 90%. That is still real CPU load on the timing rows, but it isn't a leftover stress run, so the warning says "check them" and never kills anything.
  • Departs from the doctrine's "fails" and ratchet wording. The check never fails a run, because a stray may be a peer session's live work, and the request allowed "fails or warns". A ratchet doesn't fit runtime machine state: the mistake lives in ad hoc Bash-tool commands, not committed files.

Review

  • First fresh-context review of c3e6a1b1: FIX FIRST (comment on this PR).
    • The per-process 80% rule missed the 2026-10-05 flood shape.
    • A ps failure was silent.
    • The CPU boundary and the "before the layers" placement weren't tested.
    • All of these are fixed in 34060467.
  • Second review of 34060467: FIX FIRST. The failure-path call had no test, so deleting it left both suites green. Fixed in 93e5c204, and that deletion now fails test-gauntlet-failed-summary.sh.
  • Third review of 93e5c204: 0 hard Standards violations, and the Spec's "wrong" items were all PR-body claims, corrected here.
  • The two threshold knobs were flagged as scope creep. They stay because they're small, documented and tested, and the test unsets them so an operator's own settings can't change its results.

Verified

  • tests/scripts/test-gauntlet-stray-procs.sh: 21 rows, all fed from fixture ps rows. They cover:
    • the 2 yes group
    • the 1-day etime
    • the age boundary (30:00 named, 29:59 not)
    • the CPU boundary (80.0 named, 79.9 not)
    • a 30-worker flood at 4% each
    • 43 idle copies
    • the ps failure diagnostic and load unknown
    • the env thresholds and the non-numeric fallback
    • set -e safety
    • stderr-only output
    • the call placed before the layers
  • Mutations: each of these 12 makes the test fail, and the code was restored after each:
    • remove the age check, or change its < to <=
    • remove the CPU check, or change its < to <=
    • max instead of sum
    • drop the days from etime
    • group by PID
    • warning to stdout
    • silent ps failure
    • empty load
    • default back to 600 s
    • call moved after the layers
  • test-gauntlet-failed-summary.sh: stubs the report with a line that prints, so moving the failure-path call below the lists would fail its "last line is the failing: list" row.
  • Live runs:
    • It named Finder spinning at 99.7% for 6 days on a push that then failed a timing row at load 11.28.
    • The current code names duetexpertd, a macOS system daemon, at 97.9% CPU for about 6 days. That is real CPU load on this machine, and it is why system daemons that run under your uid can show up in the report.
  • audit.sh and the gauntlet: audit.sh exits 0, and the pre-push gauntlet passed all layers on 34060467.

…nd on failure (GH #158)

A stress run whose cleanup failed under the Bash tool's zsh left 4 yes
processes at 100% CPU for ~4 h on 2026-10-25; every push's timing rows
failed until someone spotted them. scripts/_lib/stray-procs.sh lists the
operator's own processes older than 600 s at 80%+ CPU, with the load,
on stderr. run-gauntlet.sh calls it before the layers and again on
failure, above the failed-at:/failing: lists so the last line is still
the failing list. Warn-only: it never kills and never fails the run.

repo-gotchas.md gets the prose half (bash file, explicit PIDs, check
for strays after), since the sensor only catches a stray after the fact.
The first live run named a busy headless claude run at 13 min, which is
work, not a stray. The strays this targets ran for hours.
…_* env

env-vars.md invites setting knobs in settings.json; a raised
GAUNTLET_STRAY_MIN_SECS would turn the boundary rows red on that
operator's pre-push.
…sensor

# Conflicts:
#	.claude-plugin/marketplace.json
#	.claude-plugin/plugin.json
…sensor

# Conflicts:
#	.claude-plugin/marketplace.json
#	.claude-plugin/plugin.json
…sensor

# Conflicts:
#	.claude-plugin/marketplace.json
#	.claude-plugin/plugin.json
…sensor

# Conflicts:
#	.claude-plugin/marketplace.json
#	.claude-plugin/plugin.json
@wasikarn

Copy link
Copy Markdown
Owner Author

Fresh-context review: FIX FIRST @ c3e6a1b

Two-axis review (mattpocock-skills:code-review). A separate subagent ran each axis against merge-base eef3ec2.

Spec (MISSING=0 CREEP=1 WRONG=1)

Standards (HARD=1 JUDGEMENT=3)

  • Hard: the check fails silently when ps fails. The rule is that a fail-open sensor still prints a diagnostic, as gauntlet-lock.sh:14-15 does.
  • Judgement:
    • _sp_num copies _gl_num without its one-line reason.
    • No fixture row sits at exactly 80% CPU, so the CPU boundary isn't pinned.
    • The wiring row only counts the calls, so it doesn't guard the "before the layers" placement.
  • Checked and clean: home paths, no new gate, /usr/bin/grep, safe under set -e, Bash 3.2, the env-vars row, the header shape.

All of these except the knobs will be fixed on this branch, followed by a new review.

…fails

Review of c3e6a1b (FIX FIRST):
- Spec: an 80%-per-process rule missed the 2026-10-05 flood (289
  workers at ~4% each on 12 cores). Processes are now grouped by
  command name and a group whose %CPU sums to the threshold is named;
  43 idle SetStoreUpdateService copies still sum to ~0.
- Standards: a failed ps now prints one 'check skipped' line instead
  of nothing, and a failed load reader prints 'load unknown'.
- _sp_num carries the reason it shares with _gl_num.
- Tests pin the 80% boundary (80.0 named, 79.9 not), the low-CPU
  flood, the idle group, and that the first call runs before the
  layers start.
@wasikarn

Copy link
Copy Markdown
Owner Author

Fresh-context review: FIX FIRST @ 3406046

Second two-axis review (mattpocock-skills:code-review). New subagents ran it against merge-base eef3ec2, without seeing round one.

Spec (MISSING=1 CREEP=1 WRONG=1). All three are about the PR text, not the code:

  • Missing: the PR body didn't record the peer-PR check.
  • Wrong: the PR body was out of date: the ps diagnostic, the per-group sum, 21 rows.
  • Both are fixed in the body now.
  • Creep: the two threshold knobs. Kept, as in round one.
  • Fixture check of the incidents:
    • 4 yes at 99.8%, 4 h old: named, 399.2%.
    • 289 workers at 4.1%, 45 min old: named, 1184.9%.
    • Respawning short-lived children: silent. That's now listed under Known limits.

Standards (HARD=1 JUDGEMENT=3)

  • Hard: the failure-path stray_procs_report call has no test. Deleting run-gauntlet.sh:223 leaves both suites green, though a comment says test-gauntlet-failed-summary.sh covers it.
  • Judgement:
    • Use grep -m1 instead of | head -1 under pipefail. Will fix.
    • _sp_num duplicates _gl_num. Kept: 3 lines, with the shared reason cited.
    • The test fakes ps and load through private helper overrides rather than PATH shims. Kept: it's a seam choice.
  • Note: command paths with spaces group under their first word. This only affects the label and is listed under Known limits.

…list

Review of 3406046 (FIX FIRST): deleting the failure-path
stray_procs_report call left both suites green. The failed-summary
test now checks the stub's line is printed, above the failing: list;
deleting the call or moving it below the lists fails it. The wiring
row reads with grep -m1 instead of | head -1 under pipefail.
@wasikarn

Copy link
Copy Markdown
Owner Author

Fresh-context review: MERGE @ 93e5c20

Third two-axis review (mattpocock-skills:code-review). New subagents ran it against merge-base eef3ec2.

Standards (HARD=0 JUDGEMENT=5). Every hard rule checked was clean: home paths, banned commands, no new gate, fail-open sensors that still print a diagnostic, /usr/bin/grep, the pipefail rule, house style, docs. Both suites pass (21/0 and 9/0). Judgement calls, none blocking:

  • _sp_num duplicates _gl_num.
  • The incident story is told in 3 places.
  • The set -e row covers only the ps-failure path; dropping || load="" isn't caught.
  • The tests stub private helpers, so the real ps flags are never exercised.
  • Three test cases inline what run() already does.

Spec (MISSING=2 CREEP=1 WRONG=2). Both incident shapes are named from fixture rows:

  • 4 yes: 398.0%.
  • 289 workers at about 4% each: 1199.3%.

Both "wrong" items were PR-body claims, now corrected: the check isn't silent on this machine (it names duetexpertd at 97.9% for 6 days), and grouping merges an app's own processes. "Missing" is warn-only rather than fail, with no ratchet, and the 30-minute detection delay. Both are by design under "fails or warns", and both are now stated in the body. Creep: the two threshold knobs, kept.

Verdict: MERGE. No hard violation and no code defect remain after three rounds. The open judgement calls are test-seam improvements (the set -e load path, the real ps call) and can follow separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant