Skip to content

Orchestrator: gracefully gate components the platform can't recover - #473

Open
rusty1968 wants to merge 2 commits into
OpenPRoT:mainfrom
rusty1968:orch-sm-recovery-unavail
Open

rusty1968 wants to merge 2 commits into
OpenPRoT:mainfrom
rusty1968:orch-sm-recovery-unavail

Conversation

@rusty1968

Copy link
Copy Markdown
Collaborator

Summary

The platform always knows when it's out of recovery slots; until now it had no way to tell the orchestrator except by returning an actuation error, which locked the whole platform regardless of policy.

  • Adds Event::RecoveryUnavailable(ComponentId) so the platform driver can authoritatively report "no recovery source left" for a component, instead of the only existing option (EffectError) unconditionally locking the whole platform via EffectFailed.
  • Routes it through the same exhaust_recovery/gate_by_policy arm the retry-cap exhaustion path already uses, so an Isolable/Cascading component is gated and skipped (platform boots degraded) while Required still reports and locks — the two exhaustion paths can never diverge.
  • max_retry stays as a liveness backstop for non-progressing restores; RecoveryUnavailable is immediate and authoritative, and doesn't consult or bump the retry count.
  • Documents the verdict-vs-error contract on Platform::execute/Effect::RecoverComponent, wires the new event into the random_event property-test fuzzer, and updates orchestrator-machine.md's transition table and diagram.

Supersedes #445 — that branch forked before main picked up b18b78c/5060c36, which independently fixed the walk-hang and stale-verdict issues raised in that PR's review; this branch is rebased on top of those fixes rather than reintroducing them.

Test plan

  • 6 new unit tests mirroring the existing retry-cap-exhaustion suite (isolable skips, required locks, cascading, short-circuits-the-cap, wrong-target dropped, off-chain dropped)
  • 3 existing property tests now exercise RecoveryUnavailable via the randomized fuzzer
  • Full suite green (100 tests) via local cargo harness — bazelisk test //services/orchestrator/sm:orchestrator_sm_test blocked here by a sandbox network restriction on an unrelated external crate fetch

rusty1968 and others added 2 commits September 17, 2026 11:44
Add Event::RecoveryUnavailable so a platform driver out of recovery
sources routes through FailurePolicy (Isolable/Cascading gate-and-skip,
Required locks) instead of EffectFailed's unconditional lockdown.
Shares the exhaust_recovery arm with the retry-cap path so the two can
never disagree.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wire RecoveryUnavailable into the random_event() property-test fuzzer
and document its transitions in orchestrator-machine.md, alongside the
existing retry-cap exhaustion path it shares exhaust_recovery with.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@chrysh

chrysh commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

The code is right, but in case we want to add another one for regression testing:

All six new tests deliver RecoveryUnavailable while the machine is in Recovering. A stale one arriving after the machine has moved on (walk completed, state Ready) falls through the supervising handler's wildcard and is discarded, which is the right behavior but currently unpinned. Suggested test, drops in next to recovery_unavailable_off_chain_dropped:

/// A stale `RecoveryUnavailable` arriving after the machine left
/// `Recovering` (e.g. the walk already completed) is silently discarded:
/// no effects, no state change.
#[test]
fn recovery_unavailable_outside_recovering_is_discarded() {
    let (effects, state) = drive(
        passive_required(&[C0, C1]),
        &[
            BOOT,
            Event::VerificationPassed(C0),
            Event::VerificationPassed(C1),
            // Machine is now Ready.
            Event::RecoveryUnavailable(C0), // stale: not in Recovering
        ],
    );
    assert_eq!(state, State::Ready);
    assert!(!effects.contains(&Effect::AssertReset(C0)));
    assert!(!effects.contains(&Effect::ReportIsolated(C0)));
    assert!(!effects.contains(&Effect::ReportRecoveryFailed(C0)));
    assert!(!effects.contains(&Effect::LatchLockdown));
}

@chrysh chrysh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one nit

chrysh added a commit to 9elements/openprot that referenced this pull request Sep 21, 2026
Source exhaustion is a verdict, not a fault: it travels on the Ok side
as RestoreOutcome::SourceExhausted so the platform driver can report
Event::RecoveryUnavailable and the component is gated per policy,
instead of the whole platform locking. Matches the verdict-vs-error
contract in OpenPRoT#473.

Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Christina Quast <christina.quast@9elements.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants