admin
c21bcf84f7
New shared scripts/instructions_gate.py, registered in controller_gates.py and agent_gates.py, never copied into a sibling repo (the reuse_refs_check.py precedent). 20 fixture tests, all asserting the effect: exit code AND that the message names the file and the reason. It is a consistency gate, not a budget gate, and the failure message says so. A /context reading measured the instruction files at 15k tokens against 869k free in a 1M window -- space is not the constraint, and a future reader must not re-derive the wrong reason. The 200-line ceiling is adherence guidance; a file nobody can hold in their head is where contradictions hide, and five were found here. Checks run against effective text (HTML comments stripped, because they are stripped before injection): the line ceiling; every .claude/rules/*.md declares paths: or an explicit unconditional: true; no component version literal; no TEMPORARY block carrying a past date; and the workspace-root CLAUDE.md is byte-identical to its versioned copy -- the live file sits outside any git repo, so that copy is its only version-controlled record. Two traps recorded so they are not reintroduced: a bare \d+\.\d+\.\d+ matches the first three octets of every IPv4 (the gate excludes dotted quads, or it fails on 192.168.0.180 in the agent's own file); and unconditional: true is NOT a Claude Code feature but this project's own marker. Workspace-root CLAUDE.md 208 -> 182 lines (142 effective), copy kept identical. The nine-instance invariant table moved into the felhom-testing skill, which triggers when writing or reviewing a test; all three directive bullets stayed in the core. felhom.eu/CLAUDE.md got surgical corrections only and is knowingly still over the ceiling at 227 effective lines -- closing it needs the restructure R-229 defers, said plainly rather than quietly absorbed. CONTEXT.md gains standing ruling S-35. OPEN-ITEMS.md gains R-229. Docs only -- no Go, no version bump, nothing built or deployed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJc8sAGRWmavP3rMtdpkr2
7.8 KiB
name, description
| name | description |
|---|---|
| felhom-testing | Felhom testing doctrine — use when writing or reviewing ANY Go test in felhom-controller, felhom-agent, or the felhom.eu hub, and for EVERY correctness or security fix (the red-proof is mandatory there). Triggers - "write a test", "add tests", reviewing a diff that changes logic, fixing a bug, hardening a guard, or validating a fix live. Contains the non-hollow rules, the companion red-proof procedure, seam locations, and the green-gate command. |
Felhom testing doctrine
Non-hollow rule (the cardinal one)
A test must assert the effect, not the absence of error. Wrong: HTTP 200 came back, err == nil,
"function ran". Right: the stored row has the expected value, the rendered HTML contains the badge,
the state file transitioned, the fake recorded the exact command. If deleting the fix wouldn't fail
the test, the test is hollow.
Companion red-proof (mandatory for every correctness/security fix)
Prove the test detects the bug it guards against:
- Temporarily restore the PRE-FIX shape (revert the fixed line, or model the old predicate inline).
- Run the test → it must FAIL, with the wrong value visible in the failure message.
- Restore the fix → test passes.
git diffclean. - Record the red-proof outcome in
REPORT.md(what failed, with what value).
In-tree exemplars (verified):
felhom.eu/hub/internal/notify/dispatcher_test.goTestSeverityNotifies(~L27–49) — models the pre-fixwarning||errorpredicate inline and asserts the fix routes what it dropped.felhom.eu/hub/internal/api/event_test.goTestHandleEvent_CriticalPreserved— asserts the STORED severity; its red-proof was run by reverting the one-line switch (stored"info"→ FAIL).
Seams over shell-outs
Never let a unit test touch docker/pct/real /dev. Every repo's REUSE.md §4 lists its seams and
existing fakes — inject there:
- controller:
diskAgent(mockAgent),quiesce.Backend/Stacks,channelhealth.Probe/Sink,selfupdate.AgentSwapper,offboxRunner,bootstrap.PullFunc. - agent:
proxmox.Runner(mockRunner),storage.HostOps/HostReader, localapiOptionsfakes, Server seam funcs (reresolveWipe,deviceDurableID— override in tests, no real /dev). - hub:
Dispatcher.sendEmailFn,mailrelay.Sender,mailRateLimiter.now(clock),monitor.EventNotifyFunc, provider interfaces on the api Handler.
Test harness conventions: real store on t.TempDir() DB (hub/internal/api/host_test.go
newTestHandler pattern); t.Cleanup for teardown; table-driven where natural; pure
classifier functions get fixture tables (agent classifyClaim style).
What every test suite should also cover
- Negative cases: the 400/401/403/refusal paths, not just the happy path (e.g. unknown event_type → 400 AND nothing stored).
- Idempotency: re-running the op is a clean no-op where the contract says so (registry add, intent set, mount ensure).
- Fail-safe direction: for guards, ambiguity must refuse (agent claim/role classifiers are the canon — any read error ⇒ most-protected verdict).
Green gate (run before every commit that touches Go code)
go build ./... && go vet ./... && go test ./...
Run it in the module dir: felhom.eu/hub/, felhom-controller/controller/, felhom-agent/ root.
No known flakes. The long-standing agent TestGenerateRecoveryCode_EntropyAndFormat ~1/5 failure
was fixed in agent v0.93.0 (2026-07-21) — and it was never a flaky test. It was a real defect the
test was correctly detecting and everyone had been told to re-run past: the EFF wordlist contains four
hyphenated entries, so a recovery code could come out reading as 11 words instead of 10. If it fails
now, it is a regression. The lesson generalises: "known flake, just re-run it" is a diagnosis, and
it needs the same evidence as any other one. A test that fails at a stable, explainable rate is
usually telling the truth about a rare input, not misbehaving.
A comment asserting an invariant needs a test pinning it — the nine instances
The three directives live in the workspace-root CLAUDE.md and apply always. This is the evidence
behind them: nine shipped guarantees the code did not provide, each surviving review because the
comment read as settled. Read this table when you are about to trust a comment, or write one.
| # | Comment | What it claimed | What the code did |
|---|---|---|---|
| 1 | EffectiveProtected |
a stack was protected | it was not — the samba false alarm |
| 2 | newestArchiveOn |
"errors degrade to unknown, never to no-backup" | the (time,bool) signature made that impossible (R-88 Part 2) |
| 3 | R-97a operator-only | the event "cannot be routed to a customer" | only configuration stopped it; fixed by a real operatorOnlyEvents register |
| 4 | classifyRunStates I1 |
"StateStopped means deliberately stopped by the user" | quiesce stops stacks the same way — a failed restart was silent (F-CRIT-1) |
| 5 | inflight.go |
"a caller that cannot acquire DEFERS" | the backup caller recorded a failure and paged the operator (F-A1) |
| 6 | quiesce.go |
the agent's 409 prevents "a spurious failure" | on the start path it produced one (F-A1) |
| 7 | recovery_unit.go B2 refusal (R-181) |
"the previous unit is untouched and NOTHING was deleted" | nothing deleted held; untouched was measured false — the floor was checked ONLY in captureAllRecoveryUnits, while the two dump legs wrote the bulk into the same tree first and unguarded, so a 182,272 B tar became 2,147,666,432 B under a manifest that had not moved |
| 8 | ResolveManagedFloor (R-216) |
"never push a controller past the agent it depends on" | it compared the box's agent against the golden's MinAgent while serving a floor that could point elsewhere. Measured live 2026-08-05: golden 0.192.0/MinAgent 0.113.0, floor 0.200.0, agent 0.120.0 → served, and the box landed on a controller needing 0.125.0. Its customer was then told their correct recovery code was wrong. The first entry where the false invariant was a GUARD, not a comment alone. Fixed hub v0.97.0 |
| 9 | escrow/recover.go header (R-224) |
"The errors below are DISTINCT on purpose", naming three situations | there were four. A failed FETCH fell through the local-api handler's default into the wrong-code answer, so an unreachable hub was reported to the customer as a bad recovery code. Measured live 2026-08-05: 0.0556 s hub-firewalled and 0.0299 s agent-stopped, against ~1.0 s for a genuine unseal. AND A GREEN TEST NAMED IT AND DID NOT PREVENT IT — TestRecoverOffsiteRepoPassword_FetchErrorIsDistinct asserted the package's error string, one layer below where the merge happened, and a string is not something a caller can branch on. Mechanism asserted, consequence unpinned. Fixed agent v0.126.0 + controller v0.202.0 |
Three of these (4, 5/6 and 7) were found on live hardware, not by review or unit tests. #4 had a
green, red-proofed suite over a production path broken two independent ways. #7 survived a full green
suite plus three of its own red-proofs, because every one asserted the mechanism inside
captureAllRecoveryUnits and none asserted the consequence across the whole backup run. The test
that would have caught it is the one #7's fix ships: fingerprint the tree before and after, and
compare.
Live validation doctrine (after unit-land)
Exercise the SERVER-SIDE PIPELINE a real user triggers, end-to-end — never hand-set state around it
(the F9 lesson). Invoking the exact endpoint the UI invokes is an acceptable proxy when a browser
isn't available; the residual is client-side rendering only — SAY which method was used. Test crash
behavior with kill -9 / OOM, never docker kill (containers' restart policy masks the difference).
Don't echo API keys/tokens into logs or REPORT — extract into shell vars, print lengths only.