20260815 - Make the mock answer like the server, not like the spec - #13
Merged
Merged
Conversation
The mock validated against the generated models and answered the status codes
the contract declares. That made it a model of the *contract*, and the contract
is not what a node meets. Comparing it against Tower-Finder's open node-API work
(#175-#180, on top of the wire models in #167) turned up five places where the
implementation departs from `node-ingest-v1.yml`, every one of them reachable by
a healthy node:
1. Schema failures are 422, not 400, in FastAPI's `{"detail": [...]}` shape
where `detail` is a *list*. The spec declares no 422 anywhere. It is what a
node meets for a bad node_id, an unknown key, or mismatched parallel arrays.
2. 401 has two shapes. PUT /nodes/config catches its own dependency and
restores `{"error": "unauthorized"}`; detection and heartbeat let FastAPI
render `{"detail": "unauthorized"}`. Same condition, two bodies.
3. 409 is narrower than "the version does not match". A version the server
issued and has since superseded is accepted with `config_stale`, because
node_configs is append-only and the frame is still interpretable. Only a
version it never issued is a 409.
4. 413 is `{"error": "too_large"}`, not the spec's `payload_too_large`.
5. Registration is refused with 403, never 429, by a limiter of 5 an hour and
20 a day per node_id that spends its allowance before it knows whether the
device is real.
So the four handlers are now transcriptions of routes/node_register.py,
node_config.py and node_stream.py, with validate_config, the refusal taxonomy
and both rate limiters transcribed from the services they call. Where the server
and the spec disagree this file follows the server, and says so at the point it
does. config_stale and streaming_allowed are derived per response rather than
latched, because the server holds no such flags — it compares the reported
version against the active one, and reads streaming off the node's status.
Three guards had to be hand-written because a JSON Schema cannot express them
and our generated models therefore cannot carry them: a bool or numeric string
where a number belongs, a non-finite value, and the parallel-array length rule.
Without them the mock was *laxer* than production on the hot path, which is the
wrong direction to err in — a frame the mock accepts and the server 422s is a
frame silently dropped on a real node.
Nothing under retina_telemetry/ changed. The service already handled all five:
422 and 413 fall to Kind.INVALID and are not retried, both 401 shapes are keyed
off the status rather than the body, the superseded-version ack drives a config
resend, and Retry-After on a 403 already wins over our own backoff.
Two defects did surface, both in verification tooling rather than the service:
- probe_wire.py compared an enum member to a string. NodeHealth.blah2 is
required-and-nullable, so the generator gives its enum a None member and
cannot derive it from str; it is a plain Enum, where Blah2.up == "up" is
False. adsb is optional, keeps no None member, and stays a StrEnum that
compares fine. The check had been failing against a valid "up" since v1.1.1
was adopted. Payloads were never affected: to_wire dumps with mode="json".
- pip's 15s default read timeout against the node's uplink, measured at
4.9-13.6s to pypi. It killed live-probe outright and killed the service
container in live-stalled while the script still exited 0 and reported
"reached stalled: NO". Raised to --timeout 60 --retries 10 in all six.
`stalled` is no longer shipped ahead of confirmation. NodeState on
Tower-Finder's main carries all six values, and live-stalled.sh reached it on
Owl: streaming, blah2 stopped, eight beats of stalled with blah2 "down", blah2
restarted, back to streaming with seq resuming 12 -> 30. Every stalled beat was
accepted, so the heartbeat-400 path that would have silenced a node with a dead
radar is closed at both ends.
Verified by the full unit suite through tools/check.sh --tracked, and by all six
live scripts against a real Owl node: probe (both stages), service, failures,
stress, stalled and edges. live-service now shows config_version 1 after a
register-then-PUT, which is the resend path the old mock structurally could not
produce, and live-edges confirms 0 frames carrying inf/nan with detections
truncated at the spec's 512.
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.
The mock validated against the generated models and answered the status codes the contract declares. That made it a model of the contract, and the contract is not what a node meets.
Comparing it against Tower-Finder's open node-API work (offworldlabs/retina-server#175–#180, on top of the wire models in #167) turned up five places where the implementation departs from
node-ingest-v1.yml, every one reachable by a healthy node.The five divergences
{"detail": [...]}shape wheredetailis a list. The spec declares no 422 anywherenode_id, an unknown key, mismatched parallel arraysPUT /nodes/configrestores{"error": "unauthorized"}; detection and heartbeat let FastAPI render{"detail": "unauthorized"}config_stale, sincenode_configsis append-only and the frame is still interpretable. Only a never-issued version is a 409{"error": "too_large"}, notpayload_too_largenode_idthat spends its allowance before it knows whether the device is realWhat changed
The four handlers are now transcriptions of
routes/node_register.py,node_config.pyandnode_stream.py, withvalidate_config, the refusal taxonomy and both rate limiters transcribed from the services they call. Where the server and the spec disagree this file follows the server, and says so at the point it does.config_staleandstreaming_allowedare now derived per response rather than latched, because the server holds no such flags — it compares the reported version against the active one and reads streaming off the node's status.Three guards had to be hand-written because a JSON Schema cannot express them, so our generated models cannot carry them: a bool or numeric string where a number belongs, a non-finite value, and the parallel-array length rule. Without them the mock was laxer than production on the hot path — a frame the mock accepts and the server 422s is a frame silently dropped on a real node.
No service code changed
Nothing under
retina_telemetry/was touched. The service already handled all five: 422 and 413 fall toKind.INVALIDand are not retried, both 401 shapes are keyed off the status rather than the body, the superseded-version ack drives a config resend, andRetry-Afteron a 403 already wins over our own backoff.Two defects found, both in verification tooling
tools/probe_wire.pycompared an enum member to a string.NodeHealth.blah2is required-and-nullable, so the generator gives its enum aNonemember and cannot derive it fromstr— a plainEnum, whereBlah2.up == "up"is False.adsbis optional, keeps noNonemember, and stays aStrEnumthat compares fine. Failing against a valid"up"since v1.1.1 was adopted. Payloads were never affected (to_wiredumps withmode="json").live-probeoutright, and killed the service container inlive-stalledwhile the script still exited 0 and reportedreached stalled: NO. Raised to--timeout 60 --retries 10in all six scripts.stalledis confirmed at both endsNodeStateon Tower-Finder'smaincarries all six values, andlive-stalled.shreached it on Owl:streaming→ blah2 stopped → eight beats ofstalledwithblah2: "down"→ blah2 restarted →streaming, seq resuming 12 → 30. Everystalledbeat was accepted, closing the heartbeat-400 path that would have silenced a node with a dead radar.Verification
tools/check.sh --trackedgreen, and all six live scripts against a real Owl node:live-probelive-servicelive-failureslive-stresslive-stalledreached stalled: YES, radar restoredlive-edgeslive-servicenow showsconfig_version: 1after a register-then-PUT — the resend path the old mock structurally could not produce.Still open, not addressed here
Retry-Afterobedience. We honour the server's ~300s header over our own backoff, which is correct per spec, but against 5/hour and 20/day it exhausts the daily budget in ~4 hours while awaiting Mender acceptance. A design question for the server author (their ticket 86cb5dcra), not a client fix.live-stalled.shexits 0 on a run that never happened, andlive-edges.shleaves a remotefake_blah2.pythat silently serves the next run from the wrong mode.CLAUDE.mdstill describesstalledas unconfirmed and 409 as firing on any mismatch.🤖 Generated with Claude Code