[JanZachmann]

Closes #205

Summary

omnect-os moves from the scripted initramfs to omnect-os-init, an init written in Rust (omnect/meta-omnect#697). The init writes the factory-reset result to /run/omnect-device-service/omnect-os-initramfs.json in a new JSON format. This PR makes omnect-device-service read this format and report it to the twin.

• Reads the key factory_reset. A result under the old key factory-reset is unusable: it is logged as an error and reported as null. A null under the old key is a normal boot.
• The status codes have the omnect-os-init names: 0 success, 1 invalid, 2 error, 3 config error, and the new 4 warning (the reset worked, but a partition needed a second mkfs). The status is a u32, as omnect-os-init writes it. A status code this version does not know is reported as it is, and the raw result is logged as a warning.
• error and context are optional, paths defaults to an empty list. data_wiped is required: a result without it is unusable and reported as null, because a missing value must not read as a safe abort.
• Reports data_wiped: on error it tells a safe abort from a failure after data was already wiped.
• Reports error and context as null instead of leaving them out: twin reports are merge patches, so a missing key would keep the value of a previous result.
• The factory_reset feature version is 4, because the reported result changed. The crate version is 0.46.1.
• Mode 4 is no longer accepted: the init does not support it, so a request with mode 4 would come back as Invalid, a reset that never runs.
• The test files have the real omnect-os-init output. testfiles/positive/contract holds copies of the status files that omnect-os-init writes for every factory reset result (its tests/contract, fix: keep the behaviour of the scripted initramfs, remove a stale update validation config omnect-os-init#40); a test parses each one and compares every field.
• The README says that a result from before feature version 4 stays in the twin until the next reset, and that preserve accepts only the reported keys.
• README and update_validation.md link the initramfs docs in omnect-os-init (docs/usage.md) instead of meta-omnect and the legacy scripts. The README documents the result fields, and that preserve is mandatory, as the parser already requires.
• update_validation.md says that omnect-os-init deletes update_validation_conf.json on every boot without update validation (fix: keep the behaviour of the scripted initramfs, remove a stale update validation config omnect-os-init#40), so a config written by hand is gone after the next reboot.

Reason

The parser did not match the omnect-os-init format: it read the key factory-reset and required an error string. So the result of the init was unusable and never reached the twin. See #205 for the full list of differences.

omnect-os-init tests that it writes exactly the contract files, and this repo tests that it parses them. So a format change on one side fails a test once the copies are updated.

omnect-ui reads the published result and must follow the new format and the removed mode 4: omnect/omnect-ui#132. The local web service keeps the channel FactoryResetV1 and the route /factory-reset/v1, although the payload changed and mode 4 is rejected. An omnect-ui without omnect/omnect-ui#132 gets no signal about this. The only safeguard is that this PR, omnect/meta-omnect#697 and omnect/omnect-ui#132 reach a release together; omnect-ui#132 also requires ods >=0.46.1.

[JanZachmann]

FYI @mlilien

[mlilien]

Review notes; inline comments below. Two points that cannot be anchored inline:

• src/web_service.rs (FactoryResetV1): the published FactoryResetResult payload changes incompatibly (error can be null, new data_wiped, status 4 / 4294967295), but the publish channel stays FactoryResetV1, and /factory-reset/v1 now rejects mode 4. An omnect-ui build without omnect/omnect-ui#132 can't detect the change and breaks on error: null / status: 4. The twin feature version was bumped to 4; should the local channel/endpoint get a V2 as well?
• Commit 827a8c2: the Signed-off-by trailer has no angle brackets around the email (Signed-off-by: Jan Zachmann 50990105+...@users.noreply.github.com), so a DCO check will treat it as unsigned.

[JanZachmann]

@mlilien on the two points in the review summary:

• V2 channel / endpoint: No V2. omnect-ui#132 checks the ods version (>=0.46.1), and this PR, feat: factory reset feature version 4, wcs service-info probe, drop reset mode 4 omnect-ui#132 and feat(initramfs): replace the scripted initramfs with omnect-os-init meta-omnect#697 go into one release. An old UI that only knows V1 would get no result at all with a V2 channel, so renaming it only replaces a parse error with a missing result.
• Signed-off-by of 827a8c2: The repo has only a CLA check, no DCO check, and allows only squash merge, so the trailers of branch commits do not reach main. I will not rewrite the pushed history; the squash commit gets a correct trailer.

[mlilien]

The local web service still publishes on FactoryResetV1 ("FactoryResetResult") and serves /factory-reset/v1, but the payload changed: error can be null, data_wiped is new, status 4 now means Warning, and mode 4 is rejected. Only the twin feature version was bumped. An omnect-ui without #132 gets no signal that the format changed. If the joint release is the only safeguard, could the description say so explicitly? Otherwise, should the channel or route get a new version?

[JanZachmann]

@mlilien on the review summary: the description now says it explicitly. The channel FactoryResetV1 and the route /factory-reset/v1 stay, and the only safeguard is that this PR, omnect/meta-omnect#697 and omnect/omnect-ui#132 reach a release together; omnect-ui#132 also requires ods >=0.46.1.

[JoergZeidler]

Review of the factory-reset result contract change. Findings below reference file:line; none land on a line this diff touches (web_service.rs is unchanged and connect_web_service is outside the diff), so they are in the body rather than inline.

Contract verified against omnect-os-init at 9083856 (#40): key factory_reset, status codes 0..=4 as u32, field shapes (error/context optional, paths default, data_wiped required), and the six testfiles/positive/contract/ files are byte-for-byte copies of its tests/contract/. README claims about paths, the mandatory /etc/omnect/factory-reset.d/, and data_wiped match the producer. Mode 4 removal is consistent.

Machine checks on the PR head: cargo fmt -- --check passes; cargo audit passes (only pre-existing allowed warnings). clippy and the test suite could not run locally — azure-iot-sdk-sys needs the azure-iot-sdk-dev pkg-config library, absent here; they run in Concourse.

---

Medium (decision) — the web-service factory-reset surface changed its contract, but the README "Local web service" section was not updated.
The published channel FactoryResetResult (PublishChannel::FactoryResetV1, src/web_service.rs:58,70) now carries the new result shape, and the route /factory-reset/v1 (src/web_service.rs:184) now rejects mode 4. The README diff updated only the twin "Report factory reset result" section; the "Local web service" section is unchanged. The v1/V1 label stays on an incompatible payload, so an omnect-ui without omnect/omnect-ui#132 gets no signal and the only safeguard is a coordinated release (as the PR body notes).

• Decision: accept the coordinated release as the safeguard, or move the channel and route to v2 so an old consumer fails loudly instead of misreading v1?
• At minimum, update the README "Local web service" section for the new payload and the rejected mode 4.

Low (decision) — web service drops the last reset result on the next boot; the twin keeps it.
src/twin/factory_reset.rs:149 (connect_web_service) publishes the full self.report, so on a boot with no reset it sends {keys} with no result and the consumer sees nothing. connect_twin instead omits the key, so the twin's merge patch keeps the last result (as newly documented). After a reset and a later unrelated reboot, the twin still shows the result but omnect-ui no longer does. The publish logic is pre-existing, but the PR documents the twin-keeps behaviour without noting the web service differs — please confirm it is intended and consider a one-line note in the "Local web service" section.

Low — new tests use unwrap() where the convention prefers expect("reason").
e.g. src/twin/factory_reset.rs:408,416,421 and the loops in factory_reset_status_test. The new new_factory_reset helper already uses expect(...), and sibling tests use unwrap() throughout, so this only matches the existing style — raise only if the team wants to tighten test style here.

Minor — test coverage gap. No test exercises connect_web_service for the unusable result, which is exactly where the web-service/twin asymmetry (Low finding above) lives. Otherwise the parser error paths, no-reset/legacy cases, unknown-status round-trip, twin_report null-vs-omit, and the mode-4/missing-preserve rejections are all covered.

[JanZachmann]

@JoergZeidler on your review:

• v2 for channel and route: We keep v1 and accept the joint release as the safeguard; the description says so. omnect-ui#132 also requires ods >=0.46.1. With a v2 channel an old omnect-ui would not fail loudly: it would get no factory reset status at all, so v2 only changes a parse error into a missing result.
• README "Local web service": updated in 2c472d0. The factory reset route links to Trigger factory reset for mode and preserve, and the status entry links to the twin result format.
• Web service drops the last result: intended. omnect-ui shows the result dialog whenever the published status has a result and the browser session has not acknowledged it yet (App.vue, sessionStorage); the ack marker of the backend is not used for this. If ods published the last result on every boot, every new browser session would show the dialog for an old reset again. The twin keeps the last one for the cloud. The status entry in the README now says that no result is published on a later boot while the twin keeps it (2c472d0).
• unwrap() in tests: replaced by expect in this file (733e3f6).
• Test for connect_web_service: I did not add one. In test builds web_service::publish does nothing, and the payload is serde_json::to_value(&self.report), so there is nothing to check beyond what twin_report_test covers for an absent result. A test would need a test hook in publish first.

Your review is set to "changes requested" with an empty body. If anything beyond these points is open, please say which.

[JoergZeidler]

Round-2 review after the feedback commits. All substantive findings from the first review are resolved: the README "Local web service" section now documents the published payload and the web-service/twin result asymmetry, and the tests use expect(...) instead of unwrap(). Contract still matches omnect-os-init #40. cargo fmt -- --check passes; clippy and tests run in Concourse. Only an optional minor test-coverage item remains (connect_web_service with an unusable result).