fix(loader): one duplicate node_id should not disable the entire trusted-agents feed

[pstayets]

Problem

Load() treats a single duplicate node_id as fatal for the whole list — it returns the error before the parsed roster is committed:

if other, exists := idx[a.NodeID]; exists {
    return fmt.Errorf("duplicate node_id %d ... %q and %q", ...)  // aborts all N entries
}
...
byNode = idx ; all = doc.Agents   // never reached

So one bad row discards the entire feed on every hourly refresh, and each daemon falls back to its embedded snapshot. Worse, if the embedded list itself carries the duplicate (a binary built from a colliding feed), init() hits the same error and sets byNode to an empty map — booting with zero trusted agents. TestEmbeddedListLoads doesn't catch that; it only checks byNode != nil.

This is not hypothetical — it is happening on main right now: pilot-mom and pilot-director share node_id 243113, which takes down the full 438-entry list. (The companion PR fixes that data; this PR makes the loader resilient to the next one.)

Change

On a duplicate node_id, drop every entry for that node_id and keep loading the rest of the list — instead of failing the whole load. Fourteen lines in data.go, mirroring how the loop already continues past node_id==0 and empty-hostname entries.

Why "drop all", not "keep first"

The repo's own tests document the security contract: an ambiguous node_id must not be trusted, and a colliding pin must not "silently win" (TestLoad_DuplicateWithPinsRejected, FuzzLoad's fail-closed invariant). "Keep the first entry" would violate that — it lets the first pin win on an ambiguous binding. Dropping all entries for the duplicated id preserves the exact property: the ambiguous node stays untrusted, while the other 437 good entries still load.

That is why the two duplicate tests keep their security assertions unchanged — only their "returns an error" expectation flips:

test	before	after
TestLoadDuplicateNodeID	err ≠ nil and node 1 untrusted	err == nil and node 1 untrusted
TestLoad_DuplicateWithPins…	err ≠ nil and neither pin trusted	err == nil and neither pin trusted

Scope

Duplicate handling only. Malformed-JSON and bad-pin paths still return an error (unchanged). SetForTest's panic-on-dup is unchanged.

Verification

• gofmt -l clean · go vet clean · go build ./... clean
• go test ./... green (full suite), including TestLoad_BadPinRejected, TestLoad_ErrorBranch, zero/empty-hostname filters
• extra throwaway test (not committed): a triplicated node_id is voided while unique neighbors before and after it survive
• FuzzLoad ~1.2M executions over 12s — no panics, invariants hold

🤖 Generated with Claude Code

https://claude.ai/code/session_0142ryqVGEmJN66VZtCC7wFD

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!