Files
felhom-controller/controller/internal/notify/r329_severity_contract_test.go
T
admin bef39598d0
gates / gates (push) Successful in 24s
v0.256.0: the box sends its own sentence in the household's language (R-558 Part B)
MinAgent: 0.131.0 (unchanged). Needs hub v0.118.0+, which shipped first and
tolerates a box that sends none of this - every box in the fleet is that box
until this release reaches it.

The hub writes a household's e-mails in their language now, but about a third
of those mails carry a sentence the BOX composed, naming a drive, an app or a
number. The hub cannot translate one. So the box sends it twice.

- message_customer on POST /api/v1/event, omitempty. A HUNGARIAN household
  sends nothing extra at all, so its payload stays byte-for-byte what every box
  sends today and the hub's fallback path keeps being the one production
  exercises rather than a branch nobody takes.
- 19 producers render both sentences from ONE bundle key. `message` stays
  Hungarian always: it is what the operator is mailed and what the hub logs.
- customer.language bootstraps a new box - stored choice, then config, then
  Hungarian. The config value is NEVER written into settings.json: that would
  record a choice the household never made.

The Hungarian did not move, measured twice: the wire golden from the slice-2
base commit, and the Go parity gate over all 19 new keys.

Three guards had to learn the change and one caught me: the test seam now
carries the new field; the R-329 severity register reported two dynamic sites
as no longer existing the moment they moved off PushEvent (the walk now checks
36 severity literals, up from 20); and TestConfigLanguageIsWiredInMain reads
main.go, because cmd/ is gitignored and ripgrep does not.

A mistake, named: the first pass dropped displayName from three producers,
which would have mailed customers "Alkalmazás telepítve: %!s(MISSING)". Caught
reading the diff; now pinned by a test that refuses %!/MISSING/%s/%d in either
language.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
2026-09-18 16:54:20 +02:00

227 lines
10 KiB
Go

package notify
import (
"fmt"
"go/ast"
"go/parser"
"go/token"
"io/fs"
"path/filepath"
"sort"
"strconv"
"strings"
"testing"
)
// R-329 — the hub's severity vocabulary, pinned by an AST walk over the WHOLE controller.
//
// THE DEFECT THIS EXISTS TO KILL. `NotifyAppStartFailures` emitted severity `"warn"`. The hub accepts
// only {info, warning, error, critical} and **silently coerces** anything else to `info` at ingest,
// after which `severityNotifies` drops it and NEITHER the operator nor the customer leg runs. So the
// dashboard showed the app down, the hub stored the event, and no mail left the building.
//
// **This is the SECOND time.** `DiskAlertKind.Severity`'s own doc comment records that it emitted
// `"warn"` until v0.215.0, and that every Figyelmeztetés-level disk alert the product ever produced
// was delivered to nobody. A comment recorded the lesson; nothing enforced it; it happened again.
// **A comment is not a guard** — this is the guard.
//
// THE LAYER. This sits at the EMITTER, which is where the defect lives: the hub's coercion is
// deliberate and correct (a lost alarm is worse than a mis-routed one), so nothing downstream can
// detect a bad word — by design, the bad word ceases to exist at the hub's front door. The only
// place the mistake is still visible is where it is written.
//
// WHY AN AST WALK AND NOT grep. `strings.Contains` over source cannot tell an emitted severity from
// the word "warn" in a comment, a healthcheck status vocabulary (`internal/monitor`,
// `internal/selftest` both legitimately use "warn"), or a `case "warn":` in an unrelated switch. The
// 2026-08-23 sweep found nine such hits and exactly one real defect.
//
// RED-PROOF (observed, see REPORT.md): put `"warn"` back at notifier.go's app_start_failed emit and
// this fails naming the file, line, call and value.
// hubSeverityVocabulary is the hub's set, verbatim. Sources, both named so a reader can check rather
// than trust: felhom.eu/hub/internal/api/handler.go (the ingest switch) and
// felhom.eu/hub/internal/notify/dispatcher.go (severityNotifies).
var hubSeverityVocabulary = map[string]bool{
"info": true, "warning": true, "error": true, "critical": true,
}
// severityArg names the functions that take a hub severity, and which argument it is.
var severityArg = map[string]int{
"emit": 1, // (eventType, severity, message, details)
"PushEvent": 1, // (eventType, severity, message, details)
// v0.256.0 (R-558): the key-based producers. Registered here in the SAME commit that introduced
// them — the walk finds severities by FUNCTION NAME, so a new push helper that is not listed is
// a set of call sites this contract silently stops checking. It caught its own omission: the
// conversion moved two dynamic sites off PushEvent and the register immediately reported them as
// no longer existing, which is the "a stale register entry is also a failure" half doing its job.
"pushEventMsg": 1, // (eventType, severity, key, details, args...)
"pushEventMsgSuffix": 1, // (eventType, severity, key, suffix, details, args...)
"pushEventBoth": 1, // (eventType, severity, message, messageCustomer, details)
}
// knownDynamicSeveritySites are the call sites that pass a VARIABLE rather than a literal, so this
// walk cannot check their value. Each was traced by hand on 2026-08-23 and every reachable value was
// in the vocabulary. **They are listed so that a NEW dynamic site cannot appear unnoticed** — the
// honest limit of an AST check is that it cannot follow a variable, and an unlisted limit is not a
// limit, it is a hole.
var knownDynamicSeveritySites = map[string]string{
"internal/notify/notifier.go:emit": `pass-through of its own severity parameter to PushEvent — the value is checked at emit's CALLERS, above`,
// v0.256.0 (R-558): three more pass-throughs, the same shape as emit. Each forwards its OWN
// severity parameter to pushEventBoth without inspecting it, so the value is checked at their
// callers — which is where the walk's 36 literals are found.
"internal/notify/notifier.go:PushEvent": `pass-through of its own severity parameter to pushEventBoth — checked at PushEvent's callers`,
"internal/notify/notifier.go:pushEventMsg": `pass-through of its own severity parameter to pushEventBoth — checked at pushEventMsg's callers`,
"internal/notify/notifier.go:pushEventMsgSuffix": `pass-through of its own severity parameter to pushEventBoth — checked at pushEventMsgSuffix's callers`,
"internal/notify/notifier.go:NotifyControllerUpdated": `local var: "info", or "error" when the update failed`,
"internal/notify/notifier.go:NotifyDRCompleted": `local var: "info", or "warning" when failCount > 0`,
"internal/notify/notifier.go:NotifyAgentChannelDown": `internal/channelhealth's classifier — every severity literal in checker.go is "warning" or "error"`,
"internal/notify/notifier.go:NotifyDiskHealthDegraded": `DiskAlertKind.Severity() — pinned behaviourally by TestR329_DiskAlertSeveritiesStillSatisfyTheContract`,
"cmd/controller/main.go:main": `fillwatch.Band.Severity() in the SetNotify closure — pinned by TestR329_FillwatchNeverEmitsTheEmptySeverity`,
}
func walkControllerFiles(t *testing.T, fn func(path string, fset *token.FileSet, f *ast.File)) {
t.Helper()
root, err := filepath.Abs("../..") // the controller module root
if err != nil {
t.Fatal(err)
}
fset := token.NewFileSet()
err = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if d.IsDir() {
if d.Name() == "vendor" || d.Name() == ".git" || d.Name() == "node_modules" {
return filepath.SkipDir
}
return nil
}
if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
return nil
}
f, perr := parser.ParseFile(fset, path, nil, parser.ParseComments)
if perr != nil {
return fmt.Errorf("parse %s: %w", path, perr)
}
rel, _ := filepath.Rel(root, path)
fn(rel, fset, f)
return nil
})
if err != nil {
t.Fatal(err)
}
}
// calleeName returns the bare function name for `foo(...)` and `x.foo(...)`.
func calleeName(call *ast.CallExpr) string {
switch fn := call.Fun.(type) {
case *ast.Ident:
return fn.Name
case *ast.SelectorExpr:
return fn.Sel.Name
}
return ""
}
func TestR329_EveryEmittedSeverityIsInTheHubVocabulary(t *testing.T) {
var checked int
var dynamic []string
walkControllerFiles(t, func(rel string, fset *token.FileSet, f *ast.File) {
// Attribute each call to the FuncDecl that encloses it, by walking declarations rather than
// trusting Inspect's visit order — a closure inside main() must read as "main", and a call in
// a var block must not inherit the previous function's name.
for _, decl := range f.Decls {
fd, isFunc := decl.(*ast.FuncDecl)
enclosing := "(file scope)"
var scope ast.Node = decl
if isFunc {
enclosing = fd.Name.Name
scope = fd
}
inspectForSeverity(t, rel, enclosing, scope, fset, &checked, &dynamic)
}
})
// The walk must actually have found call sites — a guard that silently examines nothing is the
// "instrument that can drop results" failure, and it would pass forever.
if checked < 15 {
t.Fatalf("only %d severity literals examined — the AST walk is not reaching the call sites; "+
"fix this test before trusting a green from it", checked)
}
t.Logf("checked %d severity literals across the controller", checked)
// The dynamic sites are a stated limit, and the list is the mechanism that keeps it stated.
sort.Strings(dynamic)
seen := map[string]bool{}
for _, d := range dynamic {
if seen[d] {
continue
}
seen[d] = true
if _, known := knownDynamicSeveritySites[d]; !known {
t.Errorf("NEW dynamic severity call site %q — this walk cannot check a variable's value. "+
"Trace every value it can take; if all are in the hub vocabulary, add it to "+
"knownDynamicSeveritySites with the reason. Do not delete this check.", d)
}
}
for known := range knownDynamicSeveritySites {
if !seen[known] {
t.Errorf("registered dynamic site %q no longer exists — remove it from "+
"knownDynamicSeveritySites so the register stays honest", known)
}
}
}
// The vocabulary itself must match the hub's, and DiskAlertKind.Severity — the function whose doc
// comment records the first occurrence — must still satisfy it.
func TestR329_DiskAlertSeveritiesStillSatisfyTheContract(t *testing.T) {
for _, k := range []DiskAlertKind{
DiskAlertWarn, DiskAlertFailSelfReported, DiskAlertFailSectors,
DiskAlertFailTemperature, DiskAlertFailWorsened,
} {
if got := k.Severity(); !hubSeverityVocabulary[got] {
t.Errorf("DiskAlertKind(%d).Severity() = %q, not in the hub vocabulary", k, got)
}
}
// And the one that regressed: a warning must be a *warning*, not "warn" and not "info".
if got := DiskAlertWarn.Severity(); got != "warning" {
t.Errorf("DiskAlertWarn.Severity() = %q, want \"warning\"", got)
}
}
// inspectForSeverity checks every severity-taking call inside one declaration.
func inspectForSeverity(t *testing.T, rel, enclosing string, scope ast.Node, fset *token.FileSet, checked *int, dynamic *[]string) {
t.Helper()
ast.Inspect(scope, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
idx, wanted := severityArg[calleeName(call)]
if !wanted || len(call.Args) <= idx {
return true
}
lit, isLit := call.Args[idx].(*ast.BasicLit)
if !isLit || lit.Kind != token.STRING {
*dynamic = append(*dynamic, rel+":"+enclosing)
return true
}
val, err := strconv.Unquote(lit.Value)
if err != nil {
t.Errorf("%s: unparseable severity literal %s", fset.Position(lit.Pos()), lit.Value)
return true
}
*checked++
if !hubSeverityVocabulary[val] {
t.Errorf("%s: %s(...) emits severity %q, which is NOT in the hub's vocabulary "+
"{info, warning, error, critical}.\n"+
" The hub COERCES it to \"info\" at ingest, SILENTLY, and severityNotifies then "+
"drops \"info\" — so this event is stored and emailed to NOBODY, on either leg.\n"+
" This exact mistake shipped once before (DiskAlertKind.Severity, fixed v0.215.0).",
fset.Position(lit.Pos()), calleeName(call), val)
}
return true
})
}