20260827 - Tolerate a node with no location set - #17
Merged
Merged
Conversation
retina-node is about to ship the receiver and transmitter geometry null until an owner picks a tower, rather than defaulting to a plausible site that nothing downstream can distinguish from a real choice. This side has to tolerate that first. `_require` raises on a null, so shipping the new default before this lands would make every unsited node report a malformed config and never register - worse than the bug being fixed. `_optional` reads the seven geometry keys, `is_located` says whether the node knows where it is, and an unsited node holds registration rather than asserting a position nobody chose. A wrongly typed coordinate still raises: optional means "may be absent", not "may be anything". is_located is all-or-nothing. The bistatic solution needs the whole geometry, and a missing value becomes NaN downstream rather than an error, so a partial set would look like a working node that silently associates nothing. tx_name is excluded because a name is a label, not a position. Zero is a real coordinate throughout. Reported through the status document's `detail` rather than as a NodeState, because being unsited does not change what the node is doing - everything else about it is normal. Holding is an interim. Once the spec carries a nullable geometry, an unsited node should register with explicit nulls so the fleet can count nodes nobody has configured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 of stopping nodes from telling the server a location nobody chose. (86cba5qt4)
Important
Must be released before retina-node's null default (offworldlabs/retina-node#TBD). Today's
_requireraises on a null geometry, so shipping the new default first would make every unsited node report a malformed config and never register — worse than the bug being fixed.Why
retina-node/config/default.ymlships a complete, plausible geometry (Greenwich Observatory / Crystal Palace) and config-merger writes it on first boot. Nothing downstream can tell an unconfigured node from a configured one, so nodes have registered claiming to sit in south-east London and the server's data is poisoned with positions nobody chose.The fix is for the node to ship no location until an owner picks a tower. This side has to tolerate that before the default changes.
What
_optionalreads the seven geometry keys. An unset geometry is the ordinary state of a node whose owner has not reached the tower step, not a malformed config. A wrongly typed coordinate still raises: optional means "may be absent", not "may be anything".is_locatedsays whether the node knows where it is. All-or-nothing, because the bistatic solution needs the whole geometry and a missing value becomes NaN downstream rather than an error, so a partial set would look like a working node that silently associates nothing.tx_nameis excluded: a name is a label, not a position.detail.Two deliberate choices worth a reviewer's eye
detail, not aNodeState. Being unsited does not change what the node is doing; everything else about it is normal. A state would have to sit somewhere in the precedence chain and would misreport a node that is otherwise healthy.The hold is in
__main__.py, notwire/.wire/answers "can this payload be built"; the service layer answers "should we send". Those coincide today but diverge in Phase 2, where the payload becomes buildable with nulls and we will want to send it. Phase 2's change here is then a five-line deletion. Note this is asymmetric with consent, which is enforced inwire/registration.py— deliberately, because a missing consent record should never be sendable, whereas an unsited node will become sendable.Phase 2, once the spec carries a nullable geometry
An unsited node should register with explicit nulls rather than holding, so the fleet can count nodes nobody has configured and prompt their owners. That needs the spec revision (in progress with the server author) and the server accepting an unsited node rather than refusing. Marked as interim in the code comment.
Testing
6 new collect tests, 3 new service tests.
tools/check.sh --trackedpasses all five gates against a clean copy of tracked files.Verified end to end on owl-ded9 with the full stack running unsited (patched into the running containers, node restored afterwards, config checksums confirmed identical):
detailmessage,registered: false, and made zero registration attemptscapture,network,process,saveNot verified: this ran as patched files in a running container, not from a built image via Mender.
🤖 Generated with Claude Code