Skip to content

Show both node identifiers on the admin pages, and link the private one - #379

Merged
Babissimo merged 3 commits into
mainfrom
feat/admin-node-id-and-ref
Sep 14, 2026
Merged

Babissimo merged 3 commits into
mainfrom
feat/admin-node-id-and-ref

Conversation

@Babissimo

Copy link
Copy Markdown
Contributor

What

The admin pages show the node_ref and the node_id side by side, and a node's
link points at <node_id>.retnode.com again.

#349 moved every public feed onto node_ref, and the admin dashboard reads
those feeds — /api/radar/nodes, /api/radar/analytics, /api/custody/status
— so it was left holding only half an identity. Three things followed from
that, all of them visible to an operator:

  • Every node link resolved to nothing. RetnodeLink was handed the ref and
    built <node_ref>.retnode.com; a node's site is named after its node_id.
  • The Contact cell on every node card read "—". Contacts come from
    /api/admin/node-contacts, keyed on node_id, and were being looked up by
    ref.
  • The location-privacy toggle on those cards wrote rows that nothing reads.
    /api/admin/nodes/{node_id}/location-privacy accepts any string by design,
    so a ref was stored happily and never consulted; publication resolves privacy
    by node_id.

Three columns headed "Node ID" were also showing refs.

How

GET /api/admin/node-refs serves {node_ref: node_id} for the fleet — the
inverse of the boundary, in one place, gated on require_admin. It covers the
registry plus ids the caller already holds, resolved through owner_identity,
so mirrored and synthetic nodes appear under the handle they publish as.

useNodeIds() fetches it once per page. The ref stays the label, the listing
key and the node-page address (the per-node analytics route behind it takes a
public identity); the node_id becomes the link target, a column of its own,
and the key for the contact and privacy joins. Either identifier matches in the
search box. Where no id resolves, the id reads "—", nothing links, and the
privacy control says why rather than offering a write.

Pages touched: Network Health, Node Management, Chain of Custody.

Worth a reviewer's attention

The gate is a no-op where this ships. Prod and staging both set
AUTH_ALLOW_ANONYMOUS_ADMIN=1 with no OAuth configured, so require_admin
admits every caller and this mapping is public there. Joined against the public
ref-keyed feeds it recovers exactly the linkage #349 withholds.

This is not a new class of exposure — /api/admin/node-contacts already hands
anonymous callers node_ids alongside personal contact details — but it widens
it from contact-bearing nodes to the whole registry and serves the join
outright. Closing the door is tracked as ClickUp 86cb1emcx (urgent), which now
carries a comment about this route. Shipping knowingly rather than holding the
feature dark in every deployed environment.

A test was green over one of the bugs. NodeContact.test.tsx keyed its
node-listing fixture on node_ids, which is not the shape the page reads, so
the contact lookup passed in CI while showing "—" to every admin. The fixtures
are now shaped like the feed.

Verification

  • backend: full suite green (test_mlat_history excluded locally for time;
    CI runs it). New coverage in test_node_refs.py::TestRefToIdMap and
    test_admin_routes.py::TestNodeRefs, including the admin gate.
  • dashboard: 62 tests green, typecheck clean, lint clean (9 pre-existing
    warnings, none in the touched files).
  • pre-commit run --all-files: passed with everything staged.
  • contracts/nodes-v1.openapi.yaml: regenerated, no drift — the route is not
    under /v1/nodes.

Not yet verified on a live environment; the new columns want a look on
admin.retina.fm after deploy.

🤖 Generated with Claude Code

Babissimo and others added 3 commits September 14, 2026 15:32
The publication boundary (D16) leaves the admin pages with only half an
identity: they are built on /api/radar/nodes and /api/radar/analytics, both
keyed on node_ref and carrying no node_id at all. An admin needs the private id
to say which box an operator is looking at, to reach the node's own site, and
to join the node_id-keyed admin routes beside it.

ref_to_id_map is the inverse of the boundary, so it is served from exactly one
place, gated on require_admin. It widens the registry with ids the caller
already holds, resolved through owner_identity, so a mirrored node and a
synthetic one appear under the handle they publish as; a registry row wins over
a mirrored ref, as it does on the way out.

Worth knowing where this lands: prod and staging both run
AUTH_ALLOW_ANONYMOUS_ADMIN with no OAuth, so require_admin admits every caller
there and this mapping is effectively public until ClickUp 86cb1emcx closes
that door. The gate is the right one; the environment is what makes it a
no-op, and /api/admin/node-contacts already hands out node_ids the same way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The admin pages showed the node_ref alone under a column headed "Node ID", and
handed that ref to RetnodeLink, which builds <label>.retnode.com — so every
node link pointed at a hostname that resolves to nothing. A node's site is
named after its node_id, which the public feeds no longer carry.

Each page now joins through the admin ref map: the ref stays the label and the
listing key, the node_id becomes the link target and a column of its own, and
either one matches in the search box, since an operator arrives holding
whichever identifier their last conversation used.

The same missing join was quietly breaking two other things on the node cards.
Contacts are keyed on node_id, so looking them up by ref left every card
reading "nobody reported one"; the location-privacy override is keyed on
node_id too, and its route accepts any string, so an admin toggling privacy
there wrote a row under the ref that nothing ever consults. Both now address
the node by its id, and the control says so rather than offering a write when
no id can be resolved.

The contact test fixtures were keyed on node_ids, which is what let the first
of those pass while it was broken in front of users; they are now shaped like
the feed the page actually reads.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bullet describes the one-way trip and stops there, so a reader looking for
the mapping concludes it is nowhere. It is served, admin-only, and the
dashboard depends on it. The filename was wrong too: node_refs.py, plural.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 14, 2026

Copy link
Copy Markdown

Claude finished @Babissimo's task in 2m 49s —— View job


Claude finished reviewing this PR

Todo list

Review

This is a clean, well-scoped fix for the three bugs #349 introduced (dead node links, empty Contact cells, and privacy-toggle writes to a key nothing reads). I read the diff against origin/main in full; no functional bugs found.

Backend (backend/services/node_refs.py, backend/routes/admin.py)

  • ref_to_id_map correctly layers registry rows (_reverse) over caller-supplied node_ids resolved via owner_identity, using setdefault so a registry row always wins over a mirrored/synthetic entry — matches the new TestRefToIdMap cases (registry wins, mirrored resolves, synthetic maps to itself, handle-less node is dropped).
  • GET /api/admin/node-refs is correctly gated on require_admin, calls ref_to_id_map with the currently connected node ids (read under connected_nodes_lock, released before the call — consistent with the existing _mirrored_ref convention of reading state.connected_nodes unlocked).
  • The route is under /api/admin, not /v1/nodes, so no OpenAPI contract regen is needed — matches the PR's claim and the unchanged contracts/nodes-v1.openapi.yaml.
  • Calling the synchronous, DB-backed ref_to_id_map/_refresh() directly from an async def route handler blocks the event loop, but this is the same pattern every other call site in this module already uses (id_for_ref, public_identity, public_name in admin.py/radar.py/analytics.py/custody.py/output.py), and the module's own docstring explains why (executor threads have no loop to await on; the TTL cache keeps this cheap). Not a new issue.
  • The docstring on the route is upfront that require_admin is a no-op wherever AUTH_ALLOW_ANONYMOUS_ADMIN=1 is set with no OAuth, and that this route then makes the ref↔id join public. That's flagged in the PR description too, with a tracking ticket (ClickUp 86cb1emcx) — a known, disclosed tradeoff rather than an oversight, so I'm not raising it as a new finding.

Frontend

  • useNodeIds() distinguishes "not fetched yet" (null) from "fetched, empty" ({}), and every consumer (NodeManagementPage, NetworkHealthPage, CustodyPage) reads it correctly.
  • RetnodeLink/retnodeUrl now accept null/undefined and degrade to a plain label with no link — verified against NodeIdentity.test.tsx's "offers no link for a ref it cannot resolve" case.
  • NodeManagementPage's NodeLocationPrivacy correctly distinguishes "still loading the id map" (renders "Loading…") from "resolved, no id for this ref" (unresolved, renders an explanatory message and never calls the privacy API) — and its useEffect depends on nodeId, so it fires correctly once the id resolves after mount.
  • Contact lookups now correctly join on node_id (contacts[nodeId]) instead of node_ref, fixing the bug described in the PR body; NodeContact.test.tsx's fixtures were reshaped to match the actual feed shape it was silently passing against before.
  • Table column counts and colSpan values for empty-state rows were double-checked (NetworkHealthPage's 9-column table/colSpan={9}) and are consistent.
  • No leftover un-migrated node_id reads in the touched pages (checked via grep).

Tests / verification

  • I was not able to run pytest or the dashboard test/lint suite myself — this sandbox doesn't have the backend .venv or a way to invoke python3/npm (commands required approval that isn't available in this headless run). I reviewed the new and changed tests by reading them against the implementation instead, and they exercise the cases described in the PR body (admin gate, mirrored/synthetic/no-handle resolution, both-identifier display, link target, contact join, privacy-control gating).

No blocking issues found. Nice, thorough PR — the docstrings pull their weight and the "worth a reviewer's attention" callouts in the description are accurate and complete.
· Branch: feat/admin-node-id-and-ref

@Babissimo
Babissimo merged commit 6b830c7 into main Sep 14, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant