Repository navigation
orchestrator: Confirm or revert a device's trial image - #586
Merged
Merged
Conversation
This was referenced Oct 19, 2026
chrysh
force-pushed
the
wire-trial-boot
branch
6 times, most recently
from
October 20, 2026 11:00
aa93d43 to
3562f2f
Compare
chrysh
force-pushed
the
wire-trial-boot
branch
6 times, most recently
from
October 20, 2026 18:46
a2f6d41 to
0bc41d3
Compare
`DeviceTrialBoot` merged as a trait and nothing ever wired it: no `BoardCapabilities` slot, no implementor, no call site. So a test image a device was given stayed on trial forever, whatever the boot did, and the SOW P3 row "ensure known bootable partition" rested on the trait existing. The gate follows the `ConfirmSelfTrial` pattern the eRoT uses for itself, one chain position out. `Effect::ConfirmDeviceTrial` fires where the commit window closes on a successful boot, which is the `pending_commit` guard in the `BootConfirmed` arm of `Ready`. `Effect::RevertDeviceTrial` fires at the two sites that close the window with no boot behind it: a newer update superseding the image, and recovery putting the running image under suspicion. Both sites use `pending_commit.take()`, so the component id reaches the effect in the same statement that clears the field. The board picks an implementor through `BoardCapabilities::DeviceTrial`. A device that keeps its own trial record binds `()`, the usual case under `SvnFloorBinding::SelfManaged`. `full_update` covers confirm and revert with exact counts: one confirm and no reverts when the device boots what it was given, one revert and no confirms when the device takes the update and never comes back, neither when no update is ever offered. An `assert!(confirms > 0)` would also pass with `()` behind the seam. `is_pending` has no caller: settling a trial left open by an orchestrator that rebooted mid-update is undecided, the same way it is undecided for an unclaimed image of the eRoT itself. In GAPS.md. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
`Ready` accepted every `UpdateRequest`, and entry to `Updating` closes an open trial on the way in. So a device that had activated a test image and had not confirmed it lost that image as soon as any other component was offered an update: the trial reverted, and the `BootConfirmed` arriving afterwards found no window to close, leaving the device running an image the slot metadata calls unconfirmed. The next reset boots the old image. A device may confirm late, with nothing failing anywhere. `Ready` now answers `Super` for a request naming a component other than the one on trial, which falls through to the arm that already refuses an update the platform is too busy to take. The source gets `ReportUpdateDeferred` and the trial is untouched. Superseding the image under trial stays allowed: the update replaces that image, so the trial closes without keeping it. One test asserted the opposite. `an_update_for_a_sibling_reverts_the_open_trial` asserted the behaviour removed here, and is replaced by `an_update_for_another_component_is_refused_while_a_trial_is_open`. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
The components form a chain with depends_on edges, not a tree, so two of them are not siblings in any structural sense. DSP0267 and the rest of the tree say component. Renames the three recovery re-walk tests and the boot-confirm window test, and the prose in GAPS.md and orchestrator-model.md that reads the same way. The sibling wording in the HAL and peripheral drivers means the analogous other port and stays. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
The orchestrator timer already uses arm and disarm in the watchdog sense. What revert does to a device is clear the marker that makes the next reset boot the test image. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
chrysh
force-pushed
the
wire-trial-boot
branch
from
October 20, 2026 18:54
0bc41d3 to
8d5c567
Compare
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.
DeviceTrialBootmerged as a trait (#460) and nothing ever wired it: noBoardCapabilitiesslot, no implementor, no call site. So an image a devicewas given stayed on trial forever, whatever its boot did, and SOW P3's "ensure
known bootable partition" rested on the trait existing rather than on anything
running.
The gate is the eRoT's own
ConfirmSelfTrialpattern, one chain position out.Effect::ConfirmDeviceTrialis emitted where the commit window closes onproof, which is the
pending_commitguard inReady'sBootConfirmedarm,the guard #507 added so a sibling's boot cannot consume another's window.
Effect::RevertDeviceTrialis emitted wherever that window closes onsuspicion instead, which is exactly the two places
pending_commitis voidedwithout a boot behind it: a newer update superseding the image, and recovery
putting the running one under suspicion. Taking the id with
take()ratherthan clearing it means the emit and the void cannot drift apart.
BoardCapabilities::DeviceTrialis the seam.()is the no-op for a devicethat keeps its own trial record, which is the usual case under
SvnFloorBinding::SelfManaged, where the eRoT moves no slot metadata.What proves it
full_updatecloses the trial both ways, with exact counts rather than "atleast one": one confirm and no reverts when the device boots what it was
given, one revert and no confirms when it takes the update and never comes
back, neither when no update is ever offered. Exact because a no-op
()behind the seam would sail through a looser check.
Four state-machine tests cover the arcs the scenarios cannot reach cheaply: a
proven boot confirms only the component that booted, a superseding update
reverts the image it replaces, recovery reverts an uncommitted trial, and an
update aimed at a sibling closes whatever trial was open. That last one is the
cross-component case, since the commit window is one and not one per
component.
Worth a look in review
is_pendinghas no caller. It exists for an orchestrator that rebootedmid-update and has to read a device's open trial from stored state. Settling
that is the downstream twin of the eRoT's unclaimed-trial question and is
undecided the same way, so it is in GAPS.md rather than guessed at here.
The
Sinkcapacity derivation insm/src/sink.rsgained a paragraph. Thebound is unchanged and still has slack, but it is derived by enumerating
emission sites, and this adds three, so the comment now says why none of them
raises the worst case.
A sibling's update is now refused while a trial is open
Second commit, a behaviour change worth its own look.
Readyaccepted everyUpdateRequest, andUpdatingentry closes an open trial on the way in. So adevice that had activated an image and was still proving it lost that image
the moment any other component was offered an update: the trial reverted, the
BootConfirmedthat followed found no window to close, and the device ran animage its slot metadata calls unconfirmed. The next reset boots the old one.
A device slow to confirm is ordinary, so the race needed nothing to go wrong.
Readynow answersSuperfor a request naming a different component, whichfalls through to the arm that already refuses an update the platform is too
busy to take. Superseding the open trial's own image stays allowed.
Two things reviewers should weigh:
Readywhere it previously sawacceptance, so
ReportUpdateDeferredreaches sources in a state it neverdid before.
BootConfirmedand before the agent's 0x22, a sibling's update voids theoutstanding floor advance, because
Updatingentry clearsawaiting_floor_advanceunconditionally. orchestrator: Move the SVN floor advance to the update agent's ask #568 sanctioned that for asame-target update, where the SVN belongs to the image being replaced; for
a sibling the rationale does not hold. Recorded as a question rather
than widened into this PR, and it touches orchestrator: Commit the eRoT's own floor through the same effect #581.
Tested:
bazel test --config=virt_ast10x0 //target/ast10x0/tests/integration/...passes,
bazel test //services/...passes,./pw presubmitisgreen, and the commit builds standalone.