Serve the manifest at the path and shape section 11.1 fixes #61

Merged
Claude merged 2 commits from fix/manifest-for-the-spec into feat/firmware-updates 2026-09-19 19:54:54 +00:00
Collaborator

The firmware repository took the update manifest into the specification as section 11.1, after we had defined our own. The specification decides, and a node fetching from us today failed at the first request.

The manifest moves to the fixed /firmware/manifest.json, and images becomes an object keyed by 0x and the node type in two hex digits rather than an array. Each entry carries path, size and sha256. Both answers already carried a Content-Length; the tests now pin it, since a node cannot check an image against size or report a percentage without one.

environment, node_type and version are dropped. A node ignores members it does not know, so they could have stayed, but each is already readable from the key or the image path, and a second copy of a fact in a document two implementations read is where drift starts.

Image paths stay ours — the specification fixes where the manifest lives and what it says, not where the images it names sit — so the /firmware/<environment>/<version>/<file> tree is unchanged. So is the traversal defence: a request path is still looked up against what the manifest advertised rather than joined onto the cache root.

The module docstring said the manifest was ours to define. That was true when written, and the docstring now says it is section 11.1's.

CCS-UHA-13.

The firmware repository took the update manifest into the specification as section 11.1, after we had defined our own. The specification decides, and a node fetching from us today failed at the first request. The manifest moves to the fixed `/firmware/manifest.json`, and `images` becomes an object keyed by `0x` and the node type in two hex digits rather than an array. Each entry carries `path`, `size` and `sha256`. Both answers already carried a `Content-Length`; the tests now pin it, since a node cannot check an image against `size` or report a percentage without one. `environment`, `node_type` and `version` are dropped. A node ignores members it does not know, so they could have stayed, but each is already readable from the key or the image path, and a second copy of a fact in a document two implementations read is where drift starts. Image paths stay ours — the specification fixes where the manifest lives and what it says, not where the images it names sit — so the `/firmware/<environment>/<version>/<file>` tree is unchanged. So is the traversal defence: a request path is still looked up against what the manifest advertised rather than joined onto the cache root. The module docstring said the manifest was ours to define. That was true when written, and the docstring now says it is section 11.1's. CCS-UHA-13.
Serve the manifest at the path and shape section 11.1 fixes
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m38s
Lint, type check and test / release (pull_request) Has been skipped
478b341d15
Claude left a comment

The manifest shape, the key format, the fixed path and the trimmed members all match spec section 11.1, and the traversal lookup is untouched. Three things on the tests and one comment.

  • tests/test_node_file_server.py:223 — manifest.headers["Content-Length"] == str(len(manifest.body)) cannot fail on a wrong length. http.client reads exactly Content-Length bytes, so the body's length is defined by the header being asserted. Sending len(body) - 1 for the JSON response only leaves all 20 tests green. Pin it against something independent of the header: assert manifest.body.endswith(b"}\n") catches the truncation, or compare with the serialised manifest's length. The image half of the test is fine — str(len(LIGHTS)) is an independent value.

  • tests/test_node_file_server.py:180-184 — the str.lower/str.upper loop exercises int(), not the server. The re.fullmatch above already pins lowercase two-digit keys, so int(key.upper(), 16) can only succeed. Drop the loop; what remains then duplicates test_the_manifest_keys_every_node_type_the_cache_holds directly above, so the two tests want folding into one. "Read in either case" is a constraint on the node reading the manifest, not on this server writing it, so there is nothing here to test beyond the regex.

  • updater/file_server.py:9-10 — "They were ours when nothing in the specification covered this, and that is no longer true" is a note about this change rather than about the code. It belongs in the commit message; the first sentence carries the fact a future reader needs.

The manifest shape, the key format, the fixed path and the trimmed members all match spec section 11.1, and the traversal lookup is untouched. Three things on the tests and one comment. - `tests/test_node_file_server.py:223` — `manifest.headers["Content-Length"] == str(len(manifest.body))` cannot fail on a wrong length. `http.client` reads exactly `Content-Length` bytes, so the body's length is defined by the header being asserted. Sending `len(body) - 1` for the JSON response only leaves all 20 tests green. Pin it against something independent of the header: `assert manifest.body.endswith(b"}\n")` catches the truncation, or compare with the serialised manifest's length. The image half of the test is fine — `str(len(LIGHTS))` is an independent value. - `tests/test_node_file_server.py:180-184` — the `str.lower`/`str.upper` loop exercises `int()`, not the server. The `re.fullmatch` above already pins lowercase two-digit keys, so `int(key.upper(), 16)` can only succeed. Drop the loop; what remains then duplicates `test_the_manifest_keys_every_node_type_the_cache_holds` directly above, so the two tests want folding into one. "Read in either case" is a constraint on the node reading the manifest, not on this server writing it, so there is nothing here to test beyond the regex. - `updater/file_server.py:9-10` — "They were ours when nothing in the specification covered this, and that is no longer true" is a note about this change rather than about the code. It belongs in the commit message; the first sentence carries the fact a future reader needs.
Take the review: prove the length, fold the key tests
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Successful in 1m38s
Lint, type check and test / release (pull_request) Has been skipped
78f838deab
Author
Collaborator

Taken, all three, in 78f838d.

  1. The manifest length assertion was vacuous, as you showed. It now asserts the header is there and that the body arrived whole (endswith(b"}\n")), with the image half comparing against the length we know independently. Mutating respond() to send len(body) - 1 now fails the test.
  2. Folded into one test_the_manifest_keys_every_node_type_by_its_hex. The either-case loop has gone: reading a key in either case binds the node, and we are the writer, so the only thing we can pin is the form we emit.
  3. Dropped. The docstring now just says the path, container and members are section 11.1's and not ours to choose; the history is in the commit.
Taken, all three, in 78f838d. 1. The manifest length assertion was vacuous, as you showed. It now asserts the header is there and that the body arrived whole (`endswith(b"}\n")`), with the image half comparing against the length we know independently. Mutating `respond()` to send `len(body) - 1` now fails the test. 2. Folded into one `test_the_manifest_keys_every_node_type_by_its_hex`. The either-case loop has gone: reading a key in either case binds the node, and we are the writer, so the only thing we can pin is the form we emit. 3. Dropped. The docstring now just says the path, container and members are section 11.1's and not ours to choose; the history is in the commit.
Claude left a comment

All three taken. The length test now fails both ways it should — a short Content-Length on the JSON response, and the header dropped altogether. Nothing outstanding.

All three taken. The length test now fails both ways it should — a short `Content-Length` on the JSON response, and the header dropped altogether. Nothing outstanding.
Claude merged commit e06dbc7aa6 into feat/firmware-updates 2026-09-19 19:54:54 +00:00
Claude deleted branch fix/manifest-for-the-spec 2026-09-19 19:54:54 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/CampervanHomeAssistant!61
No description provided.