Carry the node_ref across the detection mirror

[Babissimo]

Why

The test droplet is publishing a fleet that is half fiction. Measured 2026-09-14:

test-api /api/radar/nodes    → 50 published, all synth-*, zero real
test-api /api/test/dashboard → {'total': 60, 'active': 59, 'synthetic': 50, 'real': 10}

Ten real nodes are connected and active, and not one of them reaches the map.
test-map.retina.fm shows the 50 synthetic nodes and none of the real ones.

services/node_refs.public_identity is fail-closed by design: a real node with
no registry row resolves to None and its entry is dropped, because publishing
the private id as a fallback is the exact failure the boundary exists to
prevent. Dropping them is correct given the inputs. The gap is upstream: the
test droplet receives real nodes from prod over the detection mirror, those
nodes have no registry row locally, and nothing carries their ref across.

This was the final piece of the node_ref publication plan. It was written and
reviewed alongside #349 but was not part of that merge, which is why the
feature is absent from main.

The change

• detection_mirror.build_batch attaches node_ref to an entry when
  node_refs.ref_for(node_id) resolves.
• BulkNodeEntry gains node_ref: NodeRef | None, using the canonical
  annotation with its ^(nde|sim)[0-9a-z]{12}$ pattern rather than a loose
  str. An unpatterned field long enough to hold a node id would let a caller
  inject one onto the public wire through the very fallback meant to keep it
  off.
• Both ingest branches call _record_mirrored_ref. A ref arriving against an
  entry the server already holds moves neither known nor changed, so
  recording it only at registration would do nothing until a restart, which is
  precisely the rolling-upgrade case this is for.
• A ref already claimed by another node is refused rather than published twice.
• owner_identity becomes ref_for(node_id) or _mirrored_ref(node_id), so a
  real registry row always wins.

One subtlety worth a reviewer's eye: the fallback is applied in
owner_identity, not public_identity. It reaches the public path only
because the latter delegates to the former. If either function is restructured,
that delegation is what makes this work.

Verification

ruff check and ruff format --check are clean across all seven files.

I did not run the backend tests locally: this worktree has no venv, and
borrowing another checkout's resolves imports to the wrong tree through its
editable-install paths. The three test files ship with the change and CI runs
the full matrix, so that is the gate. The tests cover the public path including
the fail-closed case: test_a_mirrored_ref_is_published_when_the_registry_has_none,
test_without_one_the_node_is_still_dropped,
test_the_registry_wins_over_a_mirrored_ref, and
test_a_mirrored_node_reaches_the_published_feed.

Before merging

The third commit, "Sort the NodeRef import into the first-party block", is a
correction to the commit below it and should be squashed into it rather than
merged as its own step. It exists separately only because the amend was
unavailable to me. The placement it fixes fails ruff's I001, so the branch
would have gone red in CI without it.

Note routes/radar.py deliberately publishes "node_ref": None for the default
pipeline. That is correct: the default pipeline is a process-wide fallback with
no registry row, so it genuinely has no ref, and the key is present so consumers
read an explicit null rather than a missing field. It should not be "fixed" by
substituting its node_id.

There is no ClickUp ticket for this yet.

🤖 Generated with Claude Code

[Babissimo]

Merge blocker: the trust model here rests on RADAR_API_KEY, and it is not set everywhere

45ce1ed7's own message notes the mirrored ref is "only as trusted as the API
key the bulk endpoint is gated on". That gate is conditional:

# backend/routes/radar.py, ingest_detections_bulk
if RADAR_API_KEY and x_api_key != RADAR_API_KEY:
    raise HTTPException(status_code=401, ...)

With RADAR_API_KEY unset the endpoint accepts anyone. With this PR applied,
that includes accepting an attacker-chosen node_ref for an arbitrary
node_id, which _mirrored_ref will then publish for any node lacking a local
registry row. The NodeRef pattern constrains the shape of the value, not who
gets to supply it.

Measured today, presence only:

environment	RADAR_API_KEY
retina-prod	set
retina-staging	no such line in backend/.env
retina-test	not checked

Staging matches ticket 86cb93r0d, which already records it unset there.
Nobody had connected that ticket to this feature; they are the same exposure.
Staging's practical risk is low today because it runs only synthetic nodes,
which short-circuit in is_synthetic_node before the mirrored lookup is
reached.

retina-test is the environment that actually exercises this path, since it
is the one receiving mirrored real nodes from prod, and it is the one I could
not check. That is the answer this PR needs before it merges: read
/opt/retina-server/backend/.env on that droplet, and if RADAR_API_KEY is
unset, set it before merging rather than after.

Worth linking 86cb93r0d to whichever ticket is raised for the underlying
regression.

[claude]

Claude finished @Babissimo's task in 1m 40s —— View job

---

Review: Carry the node_ref across the detection mirror (re-review after push)

• Gather context (PR description, CLAUDE.md, changed files, prior review comments)
• Understand the request (re-review triggered by new commits pushed to this PR)
• Diff origin/main...HEAD and re-check the collision-guard bug flagged in the previous review
• Review the new commit's docstring corrections and its test
• Post final review

The collision-guard bug from the previous review is fixed. Commit 5b8353d ("Let the guard be the only writer of a mirrored ref") removes "node_ref": entry.node_ref from the dict literal at backend/routes/radar.py:236-241 (the registration/re-registration branch), leaving _record_mirrored_ref (backend/routes/radar.py:177-192) as the sole writer on both branches. I checked that node_registration.register_node (called right after, backend/routes/radar.py:249) never touches state.connected_nodes, so there's no re-ordering hazard between the guard's write and registration. The new test test_a_ref_the_registry_gives_to_another_node_is_refused (backend/tests/test_radar_routes.py:410-417) exercises exactly the scenario the bug allowed: a colliding ref arriving at first contact (registration), monkeypatching node_refs.id_for_ref to report ownership by a different node, and asserting the entry's node_ref stays unset. This matches the previously-suggested fix precisely.

The commit also tightens two docstrings that had overclaimed:

• _record_mirrored_ref's docstring now correctly scopes the guard to the local registry only (two mirrored nodes both claiming an unregistered ref aren't caught) — this is the same caveat raised as a minor point in the prior review, now addressed.
• _mirrored_ref's lock-free-read justification in backend/services/node_refs.py no longer claims entries are "replaced wholesale"; it now correctly states a concurrent writer either replaces the entry or assigns the single node_ref key, either of which a lock-free read observes atomically.

Nothing else changed in this push besides this fix and its test, so the rest of the design from the original review stands: fail-closed publication is preserved, the registry still wins over a mirrored ref (owner_identity at backend/services/node_refs.py:200), and BulkNodeEntry.node_ref uses the canonical NodeRef-patterned type rather than a loose str.

Not re-verified here: I could not execute pytest or ruff in this environment (no venv available, consistent with what the PR author reported), so this is a static read of the diff, same limitation as before. The new/changed tests read as correct and exercise the right scenarios by inspection.

Outstanding, not a code issue: the merge-blocker raised by the PR author (RADAR_API_KEY unset on retina-test, the environment that actually exercises the mirrored-ref path) is an infrastructure/config question, not something this diff can fix — worth confirming before merge as the author noted, but out of scope for this review.

[Babissimo]

Correction to the "Before merging" note above: it says the third commit wants
squashing. There are now two such commits, both corrections to b9ab45a6
rather than steps in their own right:

• 09a637bb Sort the NodeRef import into the first-party block
• 5b8353d6 Let the guard be the only writer of a mirrored ref

Both should be folded into b9ab45a6, leaving the two-commit history the work
was written as. They are separate only because --amend was not available to
me; neither represents a change of design.

The merge blocker above still stands: RADAR_API_KEY on retina-test.

[Babissimo]

Merge blocker resolved: retina-test has RADAR_API_KEY set

Checked directly, presence only:

environment	RADAR_API_KEY
retina-prod	set
retina-test	set
retina-staging	no such line in backend/.env

retina-test is the environment that actually receives mirrored real nodes, and
its bulk endpoint is gated, so the trust model this change depends on holds
where the path is used. The blocker I raised above is cleared.

Staging remains unset, which is ticket 86cb93r0d and not a blocker here: it
runs only synthetic nodes, and those short-circuit in is_synthetic_node
before the mirrored lookup is reached. It becomes one the day staging carries a
real node, so the ticket is worth keeping open on that basis rather than as
tidiness.

One adjacent thing checked, since retina-test is deliberately a free-for-all
deploy target: just deploy-test rsyncs with --delete, but backend/.env is
an excluded path and the recipe deliberately omits --delete-excluded, so an
ad-hoc deploy cannot strip the key off the droplet. The gate survives the
deploy pattern that environment is for.