orchestrator: Add CheckpointWalk, the concrete BootWatch - #471
Merged
Merged
Conversation
chrysh
added a commit
to 9elements/openprot
that referenced
this pull request
Sep 17, 2026
BmcBootReader dispatches BmcSignal::Bl1 to a GpioBootMonitor bound at bring-up to pin 42 (bank EH, bit 10, active-high). BMC_DEVICE holds the device table constant: one checkpoint, 500 ms window. The crate is host-testable against a mock GPIO port; 3 tests cover Booted/Booting dispatch and the device-table shape. The platform driver constructs the reader and hands it to CheckpointWalk at wiring time. Depends on OpenPRoT#471 (CheckpointWalk). Assisted-by: Claude (anthropic.com)
chrysh
force-pushed
the
add-checkpoint-walk
branch
2 times, most recently
from
September 21, 2026 15:40
63eeada to
e35be75
Compare
chrysh
force-pushed
the
add-checkpoint-walk
branch
4 times, most recently
from
September 21, 2026 16:57
8d37503 to
5c08162
Compare
chrysh
marked this pull request as ready for review
September 21, 2026 17:06
chrysh
force-pushed
the
add-checkpoint-walk
branch
from
September 21, 2026 17:07
5c08162 to
5ebf70d
Compare
Collaborator
|
@JesseMelon eyes on this. |
rusty1968
approved these changes
Sep 21, 2026
rusty1968
reviewed
Sep 21, 2026
rusty1968
reviewed
Sep 21, 2026
rusty1968
reviewed
Sep 21, 2026
Collaborator
CheckpointWalk<R, P> walks a device's BootCheckpoints in declaration order, polling an EvidenceReader at each step and judging per-checkpoint windows against caller-injected now_millis. Read errors are treated as silence (Booting), lapsed windows are timeouts with no last-chance read. The QEMU runtime itest (scenarios 1-2) now uses CheckpointWalk directly instead of the hand-rolled checkpoint loop, separating walk judgment from BootWatchdogs scheduling. Scenarios 3-5 (BootWatchdogs multiplexing and commit watchdog) are unchanged. Assisted-by: Claude
Add a one-line doc on CheckpointWalk documenting that an unarmed or
post-terminal poll returns Waiting { deadline_millis: u64::MAX }. Replace
Box::leak with a const in single_checkpoint_walk_completes_in_one_poll,
collapse a needless rebind, drop the "Decision 3:" section header, and
improve an assertion message.
In the QEMU test, update the module doc to name the actual falsifiable
claim (DeadlineExceeded re-poll proves the timeout path) and trim doc
comments that were unnecessarily opinionated about the walk/watchdog
wiring decision.
Assisted-by: Claude
WalkVerdict::Failed carries a checkpoint name and a FailureCause
(TimedOut / DeviceRetriable / DeviceFatal). Until now the driver
collapsed every failure to Event::Timeout, losing the cause. Add
BootFailureKind to the SM crate (mirrors FailureCause without
coupling SM to capabilities) and Event::BootFailed { id, checkpoint,
kind }. The driver now maps Failed to BootFailed instead of Timeout.
Both events enter recovery identically today. The checkpoint and kind
fields let the SM skip retries on DeviceFatal or log the failing
checkpoint without a second round-trip.
Adds two deterministic SM tests (boot_failed_awaited_enters_recovering,
boot_failed_stale_id_ignored) alongside the existing Timeout ones.
The property test fuzzer exercises BootFailed via a coin-flip on the
Timeout arm.
Assisted-by: Claude
chrysh
force-pushed
the
add-checkpoint-walk
branch
from
September 21, 2026 20:18
5ebf70d to
ba31b0f
Compare
chrysh
added a commit
to 9elements/openprot
that referenced
this pull request
Sep 23, 2026
The walk rounds Duration to whole milliseconds via as_millis(), so a timeout below 1 ms silently becomes zero and the checkpoint expires immediately. Tighten the const assert from "not zero" to "at least 1 ms" so the real rule is enforced at build time. Addresses rusty's review comment on OpenPRoT#471. Assisted-by: Claude (Anthropic)
chrysh
added a commit
to 9elements/openprot
that referenced
this pull request
Sep 23, 2026
CheckpointWalk::new was the only constructor in the walk family that validated at runtime (non-empty assert). DeviceConfig::new already checks non-empty and unique-named at build time, so take &'static DeviceConfig<S, P> and drop the runtime assert. Addresses rusty's review comment on OpenPRoT#471. Assisted-by: Claude (Anthropic)
chrysh
added a commit
to 9elements/openprot
that referenced
this pull request
Sep 23, 2026
CheckpointWalk::new was the only constructor in the walk family that validated at runtime (non-empty assert). DeviceConfig::new already checks non-empty and unique-named at build time, so take &'static DeviceConfig<S, P> and drop the runtime assert. Addresses rusty's review comment on OpenPRoT#471. Assisted-by: Claude (Anthropic)
rusty1968
pushed a commit
that referenced
this pull request
Sep 23, 2026
The walk rounds Duration to whole milliseconds via as_millis(), so a timeout below 1 ms silently becomes zero and the checkpoint expires immediately. Tighten the const assert from "not zero" to "at least 1 ms" so the real rule is enforced at build time. Addresses rusty's review comment on #471. Assisted-by: Claude (Anthropic)
rusty1968
pushed a commit
that referenced
this pull request
Sep 23, 2026
CheckpointWalk::new was the only constructor in the walk family that validated at runtime (non-empty assert). DeviceConfig::new already checks non-empty and unique-named at build time, so take &'static DeviceConfig<S, P> and drop the runtime assert. Addresses rusty's review comment on #471. Assisted-by: Claude (Anthropic)
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
2 participants
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.
Based on #475 (probe rename). Review the last 3 commits only.
Summary
CheckpointWalk<R, P>, the concreteBootWatchthat walks a device'sBootCheckpoints in declaration order, polling anEvidenceReaderat eachstep and judging per-checkpoint windows against caller-injected
now_millis.Event::BootFailed { id, checkpoint, kind }to the SM, preservingthe walk's failure cause (TimedOut / DeviceRetriable / DeviceFatal) across
the driver boundary instead of collapsing every failure to
Timeout.BootFailureKindis diagnostic metadata for now; failure-policydifferentiation (e.g. skipping retries on
DeviceFatal) is a follow-up.CheckpointWalkdirectly,separating walk judgment from
BootWatchdogsscheduling.Read errors are silenced (
Booting), lapsed windows are timeouts with nolast-chance read. 17 host unit tests cover happy path, timeout, device faults,
read errors, arm/re-arm, deadline arithmetic, and construction rejection.