feat(server): say at startup when alerting is unconfigured (#107) #108

Closed
mathias wants to merge 1 commits from agent/107-loud-unconfigured-alerting into main
Owner

Closes #107. Ten lines plus a test, from the #100 post-mortem.

What it fixes

buildNotifier() returns a nil interface when BRAIN_NTFY_URL is unset, and every consumer skips a nil notifier silently. Correct for an unconfigured homelab; wrong for an alert whose entire value is that someone finds out.

The cost was that "configured and working" and "never configured" were byte-identical in behaviour — no error, no warning, only a message that never arrives, which nobody notices because nobody is waiting for an alert they have never seen. Two alerts sat dark in production on exactly that (#100), and both were found by firing a real event and watching nothing happen, not by reading anything.

Now:

WARN alerting DISABLED: BRAIN_NTFY_URL unset — brain_edit_root governance
     rewrites and capture audit degradation will not notify

WARN, not fatal — deliberately

Brain carries a stronger sibling rule: empty-credential-must-refuse-to-start-not-switch-modes — an empty credential must be a startup error, never a silent mode switch. That rule is about a control being removed (if token == "" { skip auth }). Here observability degrades and no gate is bypassed, and refusing to boot would break the unconfigured-homelab case the nil notifier exists to support.

What the rule does demand, and what this provides, is that the mode switch stops being silent.

Scope narrowed after checking

buildAuditSink already announces its own fallback (capture audit: slog sink (BRAIN_LOKI_URL unset)), so loki's absence was never silent. Only the notifier needed this — one warning, not a sweep.

Tests

Red-first against a stub: unconfigured names the variable and both alerts it disables; configured produces nothing.

alertingWarnings is pure with an injected getenv rather than reading the environment directly — the reason this gap survived a review is that nothing about it was observable, so the fix is tested rather than asserted.

task check exit 0.

Risk: LOW — logging only, no behaviour change to any alert path.

Closes #107. Ten lines plus a test, from the #100 post-mortem. ## What it fixes `buildNotifier()` returns a nil interface when `BRAIN_NTFY_URL` is unset, and every consumer skips a nil notifier silently. Correct for an unconfigured homelab; wrong for an alert whose entire value is that someone finds out. The cost was that **"configured and working" and "never configured" were byte-identical in behaviour** — no error, no warning, only a message that never arrives, which nobody notices because nobody is waiting for an alert they have never seen. Two alerts sat dark in production on exactly that (#100), and both were found by firing a real event and watching nothing happen, not by reading anything. Now: ``` WARN alerting DISABLED: BRAIN_NTFY_URL unset — brain_edit_root governance rewrites and capture audit degradation will not notify ``` ## WARN, not fatal — deliberately Brain carries a stronger sibling rule: *`empty-credential-must-refuse-to-start-not-switch-modes`* — an empty credential must be a startup error, never a silent mode switch. That rule is about a **control being removed** (`if token == "" { skip auth }`). Here observability degrades and no gate is bypassed, and refusing to boot would break the unconfigured-homelab case the nil notifier exists to support. What the rule does demand, and what this provides, is that the mode switch stops being silent. ## Scope narrowed after checking `buildAuditSink` already announces its own fallback (`capture audit: slog sink (BRAIN_LOKI_URL unset)`), so loki's absence was never silent. Only the notifier needed this — one warning, not a sweep. ## Tests Red-first against a stub: unconfigured names the variable *and* both alerts it disables; configured produces nothing. `alertingWarnings` is pure with an injected `getenv` rather than reading the environment directly — the reason this gap survived a review is that nothing about it was observable, so the fix is tested rather than asserted. `task check` exit 0. **Risk:** LOW — logging only, no behaviour change to any alert path.
mathias added 1 commit 2026-08-22 14:10:57 +00:00
feat(server): say at startup when alerting is unconfigured (#107)
CI / Lint / Test / Vet (pull_request) Successful in 31s
CI / Mirror to GitHub (pull_request) Skipped
1ff6af4a40
buildNotifier returns a nil interface when BRAIN_NTFY_URL is unset, and
every consumer skips a nil notifier silently. That is right for an
unconfigured homelab and wrong for an alert whose entire value is that
someone finds out: "configured and working" and "never configured" were
byte-identical in behaviour — no error, no warning, only a message that
never arrives, which nobody notices because nobody is waiting for an alert
they have never seen.

Two alerts were dark in production for exactly that reason (#100): the
governance-rewrite alert from #97, and the audit-degradation alert that
reports the audit trail is down. Both were found by firing a real event
and watching nothing happen, not by reading anything.

WARN rather than fatal, deliberately. The brain rule
`empty-credential-must-refuse-to-start-not-switch-modes` says an empty
credential must be a startup error rather than a silent mode switch — but
that is about a control being removed. Here observability degrades, no gate
is bypassed, and refusing to boot would break the unconfigured-homelab case
the nil notifier exists to support. What that rule demands and this
provides is that the mode switch stops being silent.

buildAuditSink already announces its own fallback ("capture audit: slog
sink (BRAIN_LOKI_URL unset)"), so only the notifier needed this.

alertingWarnings is pure and env-injected: the reason this gap survived is
that nothing about it was observable, so the fix is tested rather than
asserted.

Refs #107, #100, #97.
Author
Owner

Superseded by #140 — same code, rebased onto current main (this branch predated the tertiary-LLM router and CLAUDE_INGEST_SKIP_PROJECTS features and was never CI-tested; merging as-is would have silently reverted both). Closing in favor of #140.


Created via git-mcp on behalf of @mathias

Superseded by #140 — same code, rebased onto current `main` (this branch predated the tertiary-LLM router and `CLAUDE_INGEST_SKIP_PROJECTS` features and was never CI-tested; merging as-is would have silently reverted both). Closing in favor of #140. --- _Created via git-mcp on behalf of @mathias_
mathias closed this pull request 2026-09-23 12:59:42 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.