test(ingestor): pin clientRxCoverage.sources through LoadConfig (#278)

[dborup-agent]

Relates to #278

Follow-ups from the review of #274 (#265): close the LoadConfig test gap for clientRxCoverage.sources, warn at startup when one allowlisted name matches more than one MQTT source, and list the hardcoded drop-log limits as configurable values.

Plan

Each item gets a test that goes red first, then the code or docs change, then at least one mutant that turns the test red again.

#	Item	Change	Test
1	Pin the JSON key through LoadConfig	Test only. The struct tag is already correct; the test makes a typo in it visible	TestLoadConfigClientRxCoverageSources
2	Warn on duplicate source names	checkClientRxSources counts matches per allowlisted name and logs one WARNING that lists every name matching more than one mqttSources[].name (case-insensitive)	TestCheckClientRxSourcesDuplicateName, TestCheckClientRxSourcesUniqueNamesNoDuplicateWarning
3	Docs (AGENTS rule 8)	docs/client-rx-coverage.md: list the 10-minute re-log interval and the 10-line cap under "Configurable values (future customizer)", and add a note on duplicate names to the matching rules	TestClientRxCoverageDocListsDropLogLimits (reads the section and checks it against the Go constants)

Details

1. LoadConfig round trip. The test writes a config.json with two mqttSources (a, b) and "clientRxCoverage": { "enabled": true, "sources": ["a"] }, then loads it through the real LoadConfig. It asserts that Sources == ["a"], that a is allowed and that b is rejected. Before this, every allowlist test built ClientRxCoverageConfig directly, so json:"sourcez" left the suite green while the documented key did nothing. That fails open: every source would be accepted.

The server does not read sources. cmd/server's ClientRxCoverageConfig has only Enabled, so there is no server-side key to pin, and this PR does not touch cmd/server. As a check, a server started with the same clientRxCoverage block still reports clientRxCoverage: true (see the E2E results below).

2. Duplicate names. mqttSources[].name is not required to be unique and matching is case-insensitive. So "sources": ["auth"] with sources named auth and Auth admits both brokers, which quietly widens a trust boundary. At startup, checkClientRxSources now logs, for example:

[client-rx] WARNING: clientRxCoverage.sources name(s) match more than one configured mqttSources[].name (case-insensitive): AUTH matches 2 sources ("auth", " Auth ") — coverage is accepted from every one of them; give each source a unique name

This is a warning only, with no behaviour change. Source names are printed with %q, so an odd config value cannot forge log lines. Duplicated names that are not on the allowlist are not reported, because they do not affect the allowlist. The function signature and the existing unknown-name warning are unchanged.

3. Docs. The interval and the cap are now listed with their constant names (clientRxSourceWarnInterval, clientRxSourceWarnMax) and values. The doc test builds the expected text from the constants, so changing either constant without updating the docs turns the test red.

Tests

• Red before: on origin/master with only the new test file, TestCheckClientRxSourcesDuplicateName fails (no warning line) and TestClientRxCoverageDocListsDropLogLimits fails (neither value is listed). TestLoadConfigClientRxCoverageSources passes on master because the tag is correct; it fails under mutant M1.
• Mutants, each run on its own against TestLoadConfig*|TestCheckClientRxSources*|TestClientRxCoverage*|TestClientRxSource*, then reverted:

Mutant	Red tests
M1: struct tag sources → sourcez	TestLoadConfigClientRxCoverageSources (and only that test)
M2: duplicate warning not logged	TestCheckClientRxSourcesDuplicateName
M3: duplicate threshold > 1 → > 2	TestCheckClientRxSourcesDuplicateName
M4: interval line removed from the docs	TestClientRxCoverageDocListsDropLogLimits
M5: clientRxSourceWarnInterval 10 → 15 min, docs unchanged	TestClientRxCoverageDocListsDropLogLimits
M6: clientRxSourceWarnMax 10 → 20, docs unchanged	TestClientRxCoverageDocListsDropLogLimits

Scope and invariants

• The diff touches cmd/ingestor/client_rx_sources.go, the new cmd/ingestor/client_rx_sources_278_test.go and docs/client-rx-coverage.md. Nothing under cmd/server, public/ or .github.
• No new map[string]interface{} and no colours.
• scripts/check-xss-sinks.sh --diff origin/master is clean (no public/ changes).
• Fork guards are unchanged: 9 in deploy.yml, 1 in release-fast-path.yml.
• Perf: the check runs once at startup and loops over allowlist entries × configured sources, both a handful of items. No hot path is touched.
• Customizer (rule 8): the two log limits are ingestor settings, so if they become configurable they would be config keys rather than customizer controls. This is noted in the doc.

🤖 Generated with Claude Code

[dborup-agent]

Rapport — CS-pve-agent3 PR#290 #278 — head 3db1111

Status: All three follow-ups are implemented and each is tested red before and green after, with mutants. The draft PR is open and CI passed on the first attempt (no re-runs).

Evidence tags: [T] test I ran, [A] analysis / code reading, [K] command output. Branch base: origin/master 81b13b20. Three commits, all authored and committed by dborup <kontakt@meshview.dk> [K]: 7480cc6b tests, 0257da39 fix, 3db11114 docs.

Requirement 1 — pin clientRxCoverage.sources through LoadConfig

Change	Test only. The struct tag json:"sources,omitempty" is already correct [A]
Test	TestLoadConfigClientRxCoverageSources writes JSON with two mqttSources (a, b) and "clientRxCoverage": { "enabled": true, "sources": ["a"] }, then calls the real LoadConfig. It asserts Sources == ["a"], that a is allowed and that b is rejected [T]
Red before	Passes on master because the tag is correct. Under mutant M1 it is red [T]
Mutant	M1: tag sources → sourcez. Only this test fails, which confirms the gap from the #274 review is now closed [K]
Server	Not applicable. cmd/server's ClientRxCoverageConfig has only Enabled, so the server does not read sources and has nothing to pin [A]. A server started with {"clientRxCoverage": {"enabled": true, "sources": ["a"]}} reports clientRxCoverage: true in /api/config/client [K]

Requirement 2 — warn at startup on duplicate source names

Change	checkClientRxSources counts the matching mqttSources[].name per allowlisted name (trimmed, case-insensitive). When a name matches more than one source, it logs one WARNING listing the entry, the count and the matching source names (quoted with %q). Warning only. The signature, the return value and the unknown-name warning are unchanged [A]
Tests	TestCheckClientRxSourcesDuplicateName: sources auth, Auth, legacy, LEGACY with allowlist AUTH. Expects exactly one warning naming AUTH, "auth", " Auth " and the count 2, no mention of the non-allowlisted duplicate legacy, and no unknown names. TestCheckClientRxSourcesUniqueNamesNoDuplicateWarning: unique names produce no warning [T]
Red before	TestCheckClientRxSourcesDuplicateName fails on master: "expected exactly one warning line, got 0" [T]
Mutants	M2: warning block disabled → red. M3: threshold > 1 → > 2 → red [K]

Requirement 3 — docs (AGENTS rule 8)

Change	docs/client-rx-coverage.md: "Configurable values (future customizer)" now lists clientRxSourceWarnInterval (10 minutes) and clientRxSourceWarnMax (10 lines), and notes that they would become ingestor config keys, not customizer controls. The matching rules under "Restricting which MQTT sources may contribute" also mention the duplicate-name warning [A]
Test	TestClientRxCoverageDocListsDropLogLimits reads that section and requires `clientRxSourceWarnInterval`, 10 minutes and `clientRxSourceWarnMax`, 10 lines, with both values built from the Go constants [T]
Red before	Fails on master: neither entry is present [T]
Mutants	M4: doc entry removed → red. M5: interval constant changed to 15 minutes without updating the docs → red. M6: cap constant changed to 20 without updating the docs → red [K]

Each mutant was applied on its own, run against TestLoadConfig*|TestCheckClientRxSources*|TestClientRxCoverage*|TestClientRxSource*, and reverted. In every case only the new test for that item failed [K].

Local verification [K]

Command	Result
cd cmd/ingestor && go vet ./... && go test -count=1 -timeout 30m ./...	ok (777 s)
cd cmd/server && go vet ./... && go test -count=1 -timeout 30m ./...	ok (728 s)
sh test-all.sh	220 passed, 0 failed (220 files)
node test-frontend-helpers.js	707 passed, 0 failed
gofmt -l on the touched Go files	clean

E2E ran against a local Go server on a scratch copy of e2e-fixture.db, prepared as in CI: freshen, the inline seed SQL from deploy.yml, corescope-migrate, then seeds 2073, 199 and 245. The server was stopped by pid and the port checked free afterwards [K].

• Default config (coverage off, as in CI): test-node-reach-e2e.js OK, test-issue-1630-reach-mobile-e2e.js 7/0, test-reach-rank-e2e.js 14/0. test-node-reach-coverage-e2e.js and test-rx-coverage-mobile-nav-e2e.js SKIP (coverage disabled), as in CI [T]
• Config with enabled: true and sources: ["a"]: test-rx-coverage-mobile-nav-e2e.js 3/0, test-node-reach-e2e.js OK [T]. test-node-reach-coverage-e2e.js fails with query failed because the fixture has no client_* tables. Those are created by the ingestor, not by corescope-migrate, and CI never runs this test with coverage on. This is the same fixture limitation reported in the feat(ingestor): accept client RX coverage only from configured sources (#265) #274 review and is unrelated to this PR, which does not touch cmd/server or public/ [K][A]

Always-checks [K]

• Diff: cmd/ingestor/client_rx_sources.go, the new cmd/ingestor/client_rx_sources_278_test.go, and docs/client-rx-coverage.md. Nothing under cmd/server, public/ or .github, so cmd/server stays read-only.
• No new map[string]interface{} (0 added lines, test file included).
• No hardcoded colours. The only hex-like matches in the diff are the #278 / #265 issue references.
• bash scripts/check-xss-sinks.sh --diff origin/master: no public/ changes, exit 0.
• Fork guards unchanged: 9 in deploy.yml, 1 in release-fast-path.yml.
• No closing keywords in the commits, title or body. The body starts with "Relates to Follow-ups to #274: pin the clientRxCoverage.sources JSON key, warn on duplicate source names, list log limits as configurable #278".

CI per job

Run 37400330950, attempt 1, head 3db11114 [K]:

Job	Result
✅ Go Build & Test	pass (19m17s)
🎭 Playwright E2E Tests	pass (20m46s), first attempt; the #271 flake did not appear
🏗️ Build & Publish Docker Image	pass (51s)
📦 Release Artifacts	skipping (fork guard / tag-only)
🚀 Deploy Staging	skipping (fork guard / push-only)
📝 Publish Badges & Summary	skipping (fork guard / push-only)

Remaining / notes

• Startup call not executed end to end. The new warning lives in checkClientRxSources, which main() already calls once after ResolvedSources(). main is not unit-tested, so that call site has no test of its own [A].
• No live MQTT run. The duplicate warning is verified on captured log output, not against real brokers.
• Allowlist duplicates are not de-duplicated. If the allowlist itself repeats a name (["auth", "AUTH"]) and that name matches several sources, it shows up twice in the one warning line. This is cosmetic and was left as is.
• No browser validation. This PR makes no UI changes.
• No merge, ready-for-review, or issue state change.

[adminopenclaw8-sketch]

Review — CS-Macmini PR#290 — head 3db1111

Dom: APPROVE med nits

Independent, read-only review. Verified on a git archive of the head and of the merged tree
(git merge-tree --write-tree origin/master 3db11114 → e0c6adf3, clean merge against
origin/master 4f1de049; PR base is 81b13b20). git ls-remote showed 3db11114 before and
after the review — the branch did not move. Evidence tags: [T] test I ran, [A] code reading,
[K] command output.

Findings

ID	Severity	Where	Finding
N1	nit	cmd/ingestor/client_rx_sources.go:117	The allowlist entry want is interpolated with %s, not %q, so a config value with an embedded newline splits the warning and forges an extra [client-rx] coverage restricted to … line [T]. Pre-existing on master (the state line already joins allow unquoted, as does the unknown-name warning) — the PR adds a third unquoted site. The PR body's "Source names are printed with %q, so an odd config value cannot forge log lines" is true of mqttSources[].name only, not of the allowlist entry. Operator-controlled input, so impact is low.
N2	nit	cmd/ingestor/client_rx_sources_278_test.go:44	Two surviving mutants in the new message: strings.Join(ambiguous, "; ") → " " and len(matched) → literal 2 both keep the suite green [K]. No test exercises more than one ambiguous name, nor a match count other than 2.
N3	nit	cmd/ingestor/client_rx_sources.go:132	With clientRxCoverage.enabled: false the state line says "no coverage is ingested at all" and the new WARNING two lines later still says "coverage is accepted from every one of them" [T]. Same shape as the existing unknown-name warning, so this is consistency, not a regression.
N4	nit	cmd/ingestor/client_rx_sources.go:104	Disclosed by the author: a duplicated allowlist entry ("sources": ["auth","AUTH"], a plausible copy/paste) repeats the whole clause in one line — auth matches 2 sources ("auth", "Auth"); AUTH matches 2 sources ("auth", "Auth") [T]. ClientRxCoverageSources() does not de-duplicate (pre-existing; the unknown list can repeat the same way).
O1	observation	fixture	test-node-reach-coverage-e2e.js fails with coverage enabled against the CI fixture (query failed, no client_* tables) — reproduced exactly as reported [T]. Pre-existing and not attributable here: cmd/server and cmd/migrate are byte-identical to origin/master in the merged tree [K].

No blocking finding. Nothing in N1–N4 changes behaviour: checkClientRxSources is warning-only and
its return value is unchanged (unknown is appended on len(matched) == 0, exactly the old
!found) [A].

Requirements

1 — pin clientRxCoverage.sources through LoadConfig. Met.
TestLoadConfigClientRxCoverageSources goes through the real LoadConfig and asserts Sources == ["a"], a allowed, b rejected; t.Setenv("MQTT_BROKER","") is necessary isolation, since
MQTT_BROKER replaces mqttSources with a single source named env [A][T]. The test passes on
master because the tag is already correct, so red-before exists only under a mutant — correct for a
pinning test, and my own MUT-A kills it [K]. I also confirmed LoadConfig does no uniqueness
validation, so the new warning is reachable [A], and that a server started with
{"clientRxCoverage":{"enabled":true,"sources":["a"]}} still reports clientRxCoverage: true —
the extra key does not break cmd/server's narrower struct [T].

2 — warn on duplicate source names. Met. Red before on origin/master with only the new test
file added: expected exactly one warning line, got 0 [T]. The break on first match is gone, so
all matches are counted; names are %q-quoted; non-allowlisted duplicates (legacy/LEGACY) are
correctly not reported [T]. main.go:79 passes cfg.ResolvedSources(), which does not de-duplicate,
so duplicates do reach the check [A]. The warning's claim is accurate: I verified
ClientRxSourceAllowed accepts auth, " Auth " and AUTH under the allowlist AUTH [T].

3 — docs (AGENTS rule 8). Met. Red before on master (neither entry present) [T]. Both constants
are listed by name and value under ## Configurable values (future customizer), the anchor
#restricting-which-mqtt-sources-may-contribute resolves to the heading at line 37 [K], and the
doc test derives the expected text from the Go constants, so a constant change without a doc change
is red (my MUT-E) [K]. Rule 8 only requires the hardcoded values be tracked; calling them ingestor
config keys rather than customizer controls is a fair reading [A].

Tests I ran

On the merged tree, all green [T][K]:

Command	Result
cmd/ingestor: go vet ./..., gofmt -l on the touched files	clean
cmd/ingestor: go test -count=1 -timeout 40m ./...	ok, 104.9s
cmd/server: go vet ./... && go test -count=1 -timeout 40m ./...	ok, 39.1s
sh test-all.sh	222 passed, 0 failed (222 files)
node test-frontend-helpers.js	709 passed, 0 failed
Targeted: TestLoadConfigClientRxCoverageSources|TestCheckClientRxSources*|TestClientRxCoverage*|TestClientRxSource*	all pass

Red-before, on origin/master with only client_rx_sources_278_test.go added:
TestCheckClientRxSourcesDuplicateName FAIL, TestClientRxCoverageDocListsDropLogLimits FAIL
(both entries missing), TestLoadConfigClientRxCoverageSources PASS (tag already correct) [T].

E2E against a local Go server on a scratch copy of e2e-fixture.db, prepared as in CI
(tools/freshen-fixture.sh, the inline #1486 seed SQL from deploy.yml, then
corescope-migrate). Port taken from 13800 after checking it was free; the server was stopped by
the pid resolved from the listening port, and the port verified free afterwards [K].

• Coverage off (CI default): test-node-reach-e2e.js OK; test-reach-rank-e2e.js 14/0;
  test-issue-1630-reach-mobile-e2e.js 7/0; test-rx-coverage-mobile-nav-e2e.js and
  test-node-reach-coverage-e2e.js SKIP (coverage disabled) [T]
• Coverage on, sources: ["a"]: test-rx-coverage-mobile-nav-e2e.js 3/0;
  test-node-reach-e2e.js OK; test-node-reach-coverage-e2e.js FAIL — see O1 [T]

Mutants I ran

Six of my own, each applied alone and reverted, run against
TestLoadConfigClientRxCoverageSources|TestCheckClientRxSources*|TestClientRxCoverage*|TestClientRxSource*
[K]:

Mutant	Result
MUT-A: struct tag json:"sources" → json:"source" (config.go:171)	killed — only TestLoadConfigClientRxCoverageSources
MUT-B: restore break after the first match	killed — TestCheckClientRxSourcesDuplicateName
MUT-C: drop strings.TrimSpace(src.Name) from the match	killed — TestCheckClientRxSourcesDuplicateName
MUT-D: drop %q quoting of matched source names	killed — TestCheckClientRxSourcesDuplicateName
MUT-E: clientRxSourceWarnMax 10 → 11, docs unchanged	killed — TestClientRxCoverageDocListsDropLogLimits
MUT-F: strings.Join(ambiguous, "; ") → " "	survived (N2)
MUT-G: len(matched) → literal 2 in the message	survived (N2)

MUT-B is the core of the fix and MUT-A the core of the pinning test; both die. The two survivors are
message shape only, not behaviour.

Edge cases I tried

Six probes in a throwaway test file, removed afterwards [T]:

1. Two ambiguous names at once → one warning line, both clauses, "; " separator — correct, but untested (N2).
2. Ambiguous and unknown in the same config → two distinct WARNING lines, unknown == ["typo"], neither line mentions the other's name — correct, untested.
3. Both duplicate-named sources really are allowed → the warning's claim holds.
4. Duplicated allowlist entries → N4.
5. Allowlist value with an embedded newline → N1 (also red on origin/master, so pre-existing).
6. MQTT_BROKER set → mqttSources replaced by a single env source, so a JSON allowlist becomes "unknown"; pre-existing, out of scope here.

Scope and always-checks [K]

• Diff is exactly 3 files: cmd/ingestor/client_rx_sources.go (+16/−6), the new
  cmd/ingestor/client_rx_sources_278_test.go (+121), docs/client-rx-coverage.md (+10).
• cmd/server read-only: sources byte-identical to origin/master in the merged tree; cmd/migrate
  and public/ identical too. The one write added (log.Printf) is in cmd/ingestor.
• No new map[string]interface{} on any added line, tests included.
• No hardcoded colours: the only hex-shaped matches in the diff are the #278 / #265 issue refs.
• bash scripts/check-xss-sinks.sh --diff origin/master at the head: exit 0 (no public/ changes).
• Fork guards: 9 in deploy.yml, 1 in release-fast-path.yml.
• No closing keywords in the three commit messages, the title, or the body (the body opens with
  "Relates to Follow-ups to #274: pin the clientRxCoverage.sources JSON key, warn on duplicate source names, list log limits as configurable #278").
• All three commits authored and committed by dborup <kontakt@meshview.dk>.
• Perf: startup-only, O(allowlist × sources) over a handful of entries; the early break it loses
  cost nothing measurable.

CI per job

Run 37400330950, attempt 1, head 3db11114 — no re-runs [K]:

Job	Conclusion
Go Build & Test	success (19m17s)
Playwright E2E Tests	success (20m46s)
Build & Publish Docker Image	success (51s)
Release Artifacts / Deploy Staging / Publish Badges & Summary	skipped (guards)

Only one run exists for this branch, so nothing was re-run to get green. The known flakes #256
(Hash Stats sort) and #267 (backfill write-hold) did not appear.

Not verified

• main() end to end: the new warning's call site is exercised by no test, as the author states.
• No live MQTT broker; the duplicate warning is checked on captured log output only.
• No browser/visual validation (no UI change) and no full Playwright suite — only the reach and
  coverage E2Es, against plain public/ rather than the coverage-instrumented copy CI builds.
• No staging or production access, and no server API key was used.
• The fixture prep I replicated is what deploy.yml actually contains at this head: freshen, the
  single #1486 seed step, then corescope-migrate. I found no "seed 2073" or "seed 199" steps in
  the workflow to replicate.
• CI judged from per-job conclusions via the API, not by reading the job logs line by line.