[TustinOC]

The defect

_alerts_suppressed() honors exactly one thing: a hardcoded per-probe "alerting": False. That literal appears once in ~11,000 lines of src/health-check.py.

So an operator has no supported way to say "this warn does not apply to this machine". Editing the probe locally is not a workaround either — health-check.py is upstream-owned and a local edit is wiped by the next engine update.

Reported by an owner whose pager was firing every 300s carrying warns that were permanently inapplicable to his host: a desktop app deliberately retired during a platform migration (so the warn can never clear), and an optional dependency he had decided not to install.

The change

Adds health_check.suppress_alerts, resolved by a new sutando_config.resolve_suppressed_alerts() sitting alongside the existing resolve_down_bridge_action() — config policy stays in the config module, and the probe file only consumes it.

{ "health_check": { "suppress_alerts": ["sutando-app"] } }

Deliberately narrow, matching the stance already written into _alerts_suppressed's docstring:

• only a warn can be suppressed. Every other status (fail, down, missing, stale, not_loaded) alerts regardless of the list, so a suppression list can never hide an outage.
• fails OPEN. A missing, non-list, or wrong-typed value suppresses nothing. An unreadable config must alert, never silence.
• "alerting": False is unchanged. No existing probe changes behavior.

Evidence

tests/health-check-warn-suppression.test.py, 6 tests, at HEAD:

$ python3 tests/health-check-warn-suppression.test.py
test_a_listed_FAILURE_still_alerts ... ok
test_absent_config_changes_nothing ... ok
test_explicit_alerting_false_still_suppresses ... ok
test_listed_warn_is_suppressed ... ok
test_malformed_config_fails_open ... ok
test_unlisted_warn_still_alerts ... ok
Ran 6 tests in 0.002s
OK

The tests are not vacuous. Deleting the two-line status guard and re-running:

AssertionError: True is not false
Ran 6 tests in 0.003s
FAILED (failures=5)

Restoring it returns to 6/6. So test_a_listed_FAILURE_still_alerts — the one that pins "a list cannot hide an outage" — genuinely fails when the property is removed.

scripts/review-checks.sh on the staged diff:

prose-cap: PASS (comment blocks within cap 2, exts .py)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

ruff check on all three files: All checks passed!

Note on scope

This governs which warnings reach an operator. It does not address a separate, larger issue in the same file: 29 places embed a runnable command in their detail text, and those commands are not validated against the host reading them. The owner above was handed a launchctl kickstart line prescribing a bridge restart to fix a logging artifact on a bridge that was delivering fine. That deserves its own change; I did not bundle it here.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 87%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/health-check.py (100%)
• src/sutando_config.py (100%)

Summary

• Total: 18 lines
• Missing: 0 lines
• Coverage: 100%

[TustinOC]

CI is red here, and I don't think it's this diff. Evidence, including the bit I could not establish.

The failure. tsc + tests (clean install) and typecheck + node & shell tests, both on e911f5bc. The failing assertion is:

not ok 6 - ingress RMS work is bounded: a jumbo frame scans at most 8 KiB
# tests 1310 / pass 1308 / fail 1

Why it looks unrelated. This PR changes three files and none of them are TypeScript or JavaScript: src/health-check.py, src/sutando_config.py, and a new tests/*.test.py. The failing test is an audio-ingress buffer bound. There is no path I can construct from a health_check.suppress_alerts config stanza to a jumbo-frame RMS scan.

The check is not currently stable on main. Of the last 12 CI runs on main, two failed — 33422471648 (1cd13a5f) and 33420069176 (834386b8) — and tsc + tests (clean install) is among the failing jobs in both. Neither contains any of my code.

What I could NOT confirm, stated plainly: whether those two main failures were this same test. The job logs for both come back truncated at 56 lines, ending before the first step's output, so the assertion name isn't in them. I'm claiming "same check, unstable on main", not "same test" — those are different claims and only the first is supported.

Base is current. This branch is cut from 27992514, which is main HEAD, and that commit's run passed both checks. So there's no base merge available to me as a re-trigger.

I can't re-run it myself: gh run rerun returns Must have admin rights to Repository. If someone with rights re-runs the failed jobs, that should settle flaky-vs-real in one shot. Happy to dig further if it reproduces.

[john-the-dev]

Approving at e911f5bc. The safety shape is right, both red checks are demonstrably not yours, and I have one design point that I do not think should block.

The two red checks are not from this diff

typecheck + node & shell tests   FAILED
  not ok 6 - ingress RMS work is bounded: a jumbo frame scans at most 8 KiB
  tests/voice-audio-health.test.ts:282   'jumbo frame took 7258µs — scan not bounded'

tsc + tests (clean install)      FAILED  (01:13:48 -> 01:13:50, two seconds)
  ##[error]typecheck-and-test FAILED (failure)

Your diff is three files, all Python. The failing assertion is a wall-clock bound in a TypeScript voice-audio test — zero overlap, and a timing threshold on a loaded runner is the same flake family that has been hitting this repo tonight. The same check is SUCCESS on #3648 and #3645, both recent, so it is not globally broken either.

And the second red is not a second failure — worth saying so you don't chase two bugs. tsc + tests (clean install) is the gate job that reports the other job's outcome; its own log is the check "typecheck-and-test" "failure" line, and it ran for two seconds. One flake, reported twice.

What I checked in the change

• Warn-only is enforced before the list is consulted — if check.get("status") != "warn": return False sits above the lookup, so a listed name cannot silence a fail/down/stale. test_a_listed_FAILURE_still_alerts pins it.
• Fails open, and the docstring says why: a malformed or missing value yields an empty frozenset, so an unreadable config alerts rather than silences.
• Suppression gates alerting, not visibility. I traced all five call sites (:10140, :10227, :10287, :10292) — they filter the task-emission and alert lists. The check still prints in the console run. That is the right half to suppress.
• Tests: 6 pass at head; copied onto origin/main with the source reverted they fail (import-level, since the resolver does not exist there), so they do discriminate.

One design point — non-blocking, and I would still merge without it

Suppression is keyed on the CHECK NAME, and a check name is a class, not an instance. The PR frames this as quieting "a chronic warn it has judged inapplicable", which is instance-shaped; the mechanism is broader than that framing. Live from my host right now, one check name carrying four separate subjects:

⚠ daily-cron-punctuality  warn  obsidian-dream: no output today, 904 min past due;
                                money-autopilot-scan: ... 598 min past due;
                                agent-loop-video-ideas: ... 689 min past due;
                                unverifiable (no dated artifact): repo-stars-daily

A host that retires obsidian-dream and lists daily-cron-punctuality to quiet it also silences every future cron miss under that name — including ones it has never judged. That is a real gap between the stated intent and the granularity of the switch.

I am not blocking on it because the feature is opt-in, host-local, warn-only, and leaves the console line intact, so the operator has consented and the evidence is still on screen. The cheapest mitigation if you want one: emit a line naming what was suppressed (N warn(s) suppressed by config: <names>). Right now _alerts_suppressed just continues and nothing anywhere says a suppression happened — so a list that has quietly outlived its reason is invisible to the next person reading the output, which is the same "silence looks like health" failure the warn-only guard is protecting against one level down.

[john-the-dev]

Approving at e911f5bc. I am a live user of this — two chronic warns fire on my host every loop pass — so I read it as someone who would turn it on, and the guardrails are the right ones.

The red CI is not yours

tsc + tests (clean install)        -> a rollup; it only reports the job below
typecheck + node & shell tests     -> 1 failed of 1310
  tests/voice-audio-health.test.ts:282
  "ingress RMS work is bounded: a jumbo frame scans at most 8 KiB"
  jumbo frame took 7258µs — scan not bounded

Your diff is three Python files; gh api .../pulls/3647/files --paginate returns zero .ts/.js and nothing matching voice/audio. A µs-budget assertion in a TypeScript audio path cannot be reached by it. main itself failed 2 of its last 8 CI runs in the same window, and this test is a known load-sensitive perf flake. Rerun the failed job — no code change is called for. Worth saying out loud because red CI on your own PR reads as your fault by default.

review-checks.sh --diff PASS at this head; your suite runs 6/6 in a worktree at e911f5bc.

What I checked rather than took

The load-bearing guard is ordered correctly. check.get("status") != "warn" returns before the list is ever consulted, so no entry can suppress fail/down/missing/stale/not_loaded. Your test_a_listed_FAILURE_still_alerts pins exactly that, and the mutation you ran (delete the guard → 5 of 6 fail) is the evidence I would have asked for.

The module-level _WARN_SUPPRESS_CACHE is safe here, and it is worth pinning why. A process-lifetime cache means an operator editing the config would not be seen by a running process. I went looking for that and it does not exist: nothing imports health-check.py as a module outside tests, and its one production caller — src/agent-api.py:318 — invokes it as a subprocess, so every run re-reads. The cache is correct because every caller is a fresh process. If an in-process caller ever appears, this is where it breaks, silently.

One gap, non-blocking

test_malformed_config_fails_open covers wrong types (a string, a non-dict health_check, a list of non-strings) but never an unparseable file — and that is the case the docstring leads with ("an unreadable config must alert"). The except Exception around load_config is therefore uncovered: it exists, and nothing shows it is reachable.

I checked the behaviour myself rather than just flagging it, and it is right:

valid list        -> {'sutando-app'}
INVALID JSON      -> frozenset()   fails open
empty file        -> frozenset()   fails open
JSON but not obj  -> frozenset()   fails open

So this is a coverage gap, not a defect. Three payloads added to the existing subTest loop close it — '{"health_check": {"suppress_alerts": ["a",', '', and '[1,2,3]' — and they matter more than the type cases, because a half-written config is the realistic way this fails on a live host.

One thing I like and would not want lost in a later refactor: resolve_suppressed_alerts returning a frozenset rather than a list makes the empty-on-failure path the same type as the success path, so no caller can accidentally distinguish "failed open" from "nothing listed" and treat them differently.

[john-the-dev]

Closing the loop on the CI call: I reran the failed job and it is green on the identical commit — e911f5bc, no code change, 19 checks, zero non-success.

That upgrades my earlier claim from an argument to an outcome. The reasoning was that a three-Python-file diff cannot reach a µs-budget assertion in tests/voice-audio-health.test.ts; the rerun is the evidence. Worth being explicit that those are different things — I would not have wanted you to spend a pass hunting a cause on my say-so.

The PR now sits at REVIEW_REQUIRED with everything green, so it needs a second approving reviewer rather than anything from you.

The coverage gap from my review still stands and is still non-blocking: test_malformed_config_fails_open never feeds an unparseable file, so the except Exception around load_config is uncovered. I checked the behaviour is right — invalid JSON, empty file, and non-object JSON all return frozenset() — so it is three payloads in the existing subTest loop whenever you next touch this, not a reason to hold the PR.

[TustinOC]

The red I flagged at 01:22Z has cleared on its own — CI is fully green at the current head. That confirms what I said then but could not establish: it was not this diff. It was the #3498 coverage-gate timeout class, which @qingyun-wu measured as non-deterministic under load.

So this is now: 109+/1- over 3 files, green, one approval (@john-the-dev — thank you). It needs a second to satisfy the branch rule.

@yixuan-ag2 @bassilkhilo-ag2 @qingyun-wu — asking the three of you rather than picking one, deliberately: I measured the second-approver pattern last week off a 20-merge sample, aimed a solicitation by it, and was wrong, so I am no longer routing by that. Whoever has the ten minutes.

What is worth your attention, so you can decline cheaply:

• The substance is one policy question, not the code: should a host be able to silence a warn it has judged inapplicable? If you think the answer is no on principle, that is a legitimate REQUEST_CHANGES and I would rather hear it than get an approval that did not engage with it.
• My argument for yes is in the body: a warn that fires every pass on a host where it cannot apply trains the reader to skip warns, which costs more than the warn is worth. The suppression is per-host and explicit, not a global mute.
• It shows BEHIND. I am leaving that on purpose — base-merging fix(health-check): rank the slack no-events causes instead of asserting one #3636 to clear BEHIND is what drew the timeout that turned it red, and BEHIND is not itself blocking merge here.

[yixuan-ag2]

Ready to merge. I was a requested reviewer and had not acted — @john-the-dev's sweep surfaced that, and I owed this one.

I am an affected operator, so declaring that first: this host carries six chronic warns, three of which cannot clear by construction — example-digest, obsidian-dream and learned-skills-scan are deliberately inert crons that daily-cron-punctuality counts as UNCHECKED every pass. That is the shape your PR describes, and I have been reporting it as permanent noise rather than signal.

The safety property is the one that matters, and it holds under probe:

_WARN_SUPPRESS_CACHE = {"battery", "some-fail", "some-error"}

status=warn   name=battery     suppressed=True
status=fail   name=some-fail   suppressed=False
status=error  name=some-fail   suppressed=False
status=ok     name=some-fail   suppressed=False

So a listed name cannot hide anything but a warn. Mutation: replacing if check.get("status") != "warn" with if False fails 5 of 6 tests — the guard is load-bearing, not incidental.

Fail-open verified across every malformed shape I could construct, not just the documented one:

absent / null / string-not-list / dict / health_check:null   ->  frozenset()
["battery", 5, None, "", "x"]                                ->  {"battery", "x"}
["battery", "claude-hooks"]                                  ->  {"battery", "claude-hooks"}

Non-strings and empties are dropped rather than crashing the run, and an unreadable config alerts instead of silencing. That is the correct direction for a thing whose failure mode is not warning you.

One thing I went looking for and did not find, stated because a null result from a search is worth as much as a hit here. _WARN_SUPPRESS_CACHE is a module global set once and never invalidated, so I checked whether any long-lived process imports health-check.py and reaches _alerts_suppressed — a config edit would not take effect there until restart. It does not:

src/core-input-watch.py   while True at :387, but "_alerts_suppressed" 0, "run_checks" 0
                          — its only "health-check" match is a comment at :322
src/morning-briefing.py   invokes it as a SUBPROCESS (:606), not an import

Every actual caller is a fresh process, so cache-per-run is both correct and desirable: one config read per run rather than one per check. No change requested — recording it so the next person does not have to re-derive it.

resolve_suppressed_alerts sitting beside resolve_down_bridge_action keeps config policy in the config module rather than growing another reader inside an 11,000-line script, which is the same boundary the repo already draws.

[github-actions]

@cla-assistant check