Repository navigation
fix(stage0): land #162 — let stage-1 roll back a hung image instead of recovering (rebased) - #167
Conversation
A test-boot image that hangs is reset by the watchdog, and stage-1 increments boot_attempts before every jump. When boot_attempts reaches max_attempts, stage-1's eos_boot_policy_select() rolls back to the other slot (docs/architecture.md, docs/secure_boot_chain.md). But stage-0's ebldr_recovery_triggered() tested the same condition first -- watchdog reset with boot_attempts >= max_attempts -- and entered recovery, so the rollback never ran and the device sat in recovery with a good image in the other slot. Drop that clause from the stage-0 trigger. Exhausted attempts are a rollback condition, owned by the stage-1 policy, which still falls through to recovery itself when there is no valid slot to roll back to. The recovery pin, FORCE_RECOVERY and FACTORY_RESET triggers are unchanged. test_watchdog_rollback: new suite running the real stage-0 trigger check (stage0/recovery_entry.c compiled for the host) and then the real boot policy, with slot validity mocked at the slot-manager boundary: a hung test boot of B after a watchdog reset rolls back to A; with no valid alternate the policy still refuses (stage-1 enters recovery); the explicit recovery triggers still fire. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…esolution only; contributor commits unchanged as first parent)
Reviewer's GuideRemoves stage-0's premature recovery path for watchdog-exhausted boot attempts so stage-1 can roll back a hung image, while preserving recovery for explicit triggers and unavailable fallback images. Adds an integrated host regression test with mocked board and slot-manager boundaries, and registers it alongside the existing stage-0 boot-log test. Sequence diagram for watchdog rollback in stage-1sequenceDiagram
participant Stage0
participant Stage1
participant SlotManager
Stage0->>Stage0: ebldr_recovery_triggered(bctl)
Stage0-->>Stage1: return false
Stage1->>Stage1: eos_boot_policy_select()
alt fallback slot is valid
Stage1->>SlotManager: select fallback slot
SlotManager-->>Stage1: rollback target
else no valid slot
Stage1-->>Stage0: enter recovery
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
You've used all your free Kodus-paid PR reviews 🎁Your trial is still active — this just means the PR reviews we cover during the trial are used up. Connect your own AI key to keep Kody reviewing — unlimited reviews, on any plan (Free included). Want more trial reviews to finish evaluating before adding a key? Talk to our founders. 😎 |
|
Caution Review failedThe pull request is closed. Walkthrough
Merge Risk:
 |
PR Summary by QodoLet stage-1 roll back after watchdog boot attempts are exhausted
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by QodoView medium (1)
1. Damaged stage-one prevents recovery
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| if (reason == EOS_RESET_WATCHDOG && | ||
| bctl->boot_attempts >= bctl->max_attempts) | ||
| return true; |
There was a problem hiding this comment.
1. Damaged stage-one prevents recovery 🐞 Bug ☼ Reliability
ebldr_recovery_triggered() no longer uses exhausted watchdog attempts as a recovery escape, even when stage-one verification is disabled. If attempts are exhausted and the stage-one image is subsequently damaged, stage-zero jumps to it without checking it, so neither stage-one rollback nor its recovery fallback can run.
Agent Prompt
## Issue description
Removing the exhausted-watchdog recovery trigger makes recovery depend on stage one running, but builds with stage-one verification disabled can jump to a damaged stage-one image.
## Fix Focus Areas
- stage0/recovery_entry.c[27-31]
- stage0/jump_stage1.c[74-76]
- stage0/jump_stage1.c[126-140]
## Recommended Fix
Add a persistent stage-one progress marker: clear it before the stage-zero jump, set it when stage one reaches boot selection, and retain a watchdog recovery escape when attempts are exhausted and the previous stage-one run never reached that marker. Continue handing watchdog resets that reached stage one to its rollback policy.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /* Exhausted boot attempts are not a recovery trigger. An image that | ||
| * hangs is reset by the watchdog until boot_attempts reaches | ||
| * max_attempts, and that is the condition on which stage-1's | ||
| * eos_boot_policy_select() rolls back to the other slot. Diverting it | ||
| * to recovery here meant the rollback never ran. Stage-1 still enters | ||
| * recovery itself when there is no valid slot to roll back to. */ |
There was a problem hiding this comment.
Removing this check leaves devices rebooting forever when both slots contain valid images that hang. eos_boot_policy_select() switches slots and clears boot_attempts whenever the limit is reached. It does not remember that the alternate image already failed, so the two images keep getting fresh attempts instead of reaching recovery. Previously, stage-0 stopped this loop.
Keep the first rollback, but enter recovery if the fallback also exhausts its attempts. Add a test for that sequence.
Prompt To Fix With AI
This is a comment left during a code review.
Path: stage0/recovery_entry.c
Line: 27-32
Comment:
**Both images keep rebooting**
Removing this check leaves devices rebooting forever when both slots contain valid images that hang. `eos_boot_policy_select()` switches slots and clears `boot_attempts` whenever the limit is reached. It does not remember that the alternate image already failed, so the two images keep getting fresh attempts instead of reaching recovery. Previously, stage-0 stopped this loop.
Keep the first rollback, but enter recovery if the fallback also exhausts its attempts. Add a test for that sequence.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Linked issues
#161 A hung new image goes to recovery after a watchdog reset instead of rolling back to the good slot · 2 of 2 met
| # | Requirement | Status | Notes |
|---|---|---|---|
| 1 | A hung new image with a valid good slot in the other slot rolls back to that slot. | ✅ Met | Stage-0 no longer diverts exhausted attempts to recovery; the stage-1 policy rolls back to a valid alternate. The new test covers a hung test boot of B rolling back to A. |
| 2 | If no valid slot is available for rollback, the device enters recovery. | ✅ Met | The policy returns EOS_ERR_NO_IMAGE when the alternate is invalid, and stage-1 enters recovery when no boot target is selected. The new test covers this case. |
Sourcery assessment
Needs a human reviewer. After the watchdog-attempt limit, stage-0 now hands control to stage-1, whose policy may change the active slot and boot-control rollback state. Those changes can survive reverting this patch, but they are bounded and repairable; if no slot is valid, stage-1 still enters recovery.
Lands the work of #162 (by @Greeva48) on current master.
Why a separate PR: #162 is from a fork branch. It was approved on substance on 2026-11-01 and is blocked only by a tests/CMakeLists.txt anchor conflict: #158's merge added a test registration at the same anchor where #162 adds its own. Maintainer edits are enabled on #162, but the GitHub API does not permit pushing commits to fork branches (verified 2026-11-02: git-data writes to the fork return 404), so the rebase could not be done in #162's own branch.
What this PR is: a merge commit whose FIRST PARENT is #162's head (43279bf) unchanged — the contributor's commits land exactly as authored — and whose second parent is current master (09fbc81). The only new content is the tests/CMakeLists.txt conflict resolution, which keeps both test registrations (test_stage0_boot_log from master, test_watchdog_rollback from #162). No other file differs from the combination of #162 + master.
Fixes #161 (via #162's change).
On merge, #162 will be closed as superseded by this PR with a pointer here.
Summary by Sourcery
Fix watchdog handling so exhausted boot attempts reach stage-1 rollback policy while retaining recovery fallback behavior.
Bug Fixes:
Tests:
Summary by CodeRabbit
Add a stopping condition for a failed fallback before merging.
Findings
Fix with agent prompt
Summary
Stage-0 now leaves exhausted boot attempts to stage-1 instead of entering recovery immediately.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Watchdog reset after attempt limit] --> B[Stage-0 continues to stage-1] B --> C{Alternate image valid?} C -->|No| D[Enter recovery] C -->|Yes| E[Switch slots and clear attempts] E --> F[Run alternate image] F -->|Also hangs until attempt limit| AReviews (1) · Last reviewed commit: "Merge PR #162 head with master — maintai..." · Reviewed by Greptile