Skip to content

efi/preinstall: reordered some preinstall check actions - #470

Merged
ernestl merged 2 commits into
canonical:masterfrom
ernestl:ernestl/auto-before-manual-fix-actions
Nov 27, 2025
Merged

ernestl merged 2 commits into
canonical:masterfrom
ernestl:ernestl/auto-before-manual-fix-actions

Conversation

@ernestl

@ernestl ernestl commented Nov 21, 2025 •

Copy link
Copy Markdown
Member

Reordered some error-kinds actions according in accordance with prioritization guideline and use new helper to insert ActionProceed in accordance to the guideline.

This proposal is a result of discussions around action order that happened during review of Using the Snapd Error Kinds and Actions APIs.

It was also suggested that we do not need manual actions at all, given the current manual actions all have automatic PPI based counterparts, however I though to keep them in place for the moment while its still early days, so that we have an additional path to achieve actions.

Also corrected a few spelling mistakes.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR reorders action sequences in the preinstall check error handling system to prioritize automatic PPI-based actions over manual firmware settings actions. The PR also corrects multiple spelling and grammatical errors throughout the codebase.

Key changes:

  • Automatic TPM actions (via PPI) are now listed before manual reboot-to-firmware-settings actions
  • Added documentation explaining the rationale for action ordering
  • Fixed numerous spelling errors in comments and error messages

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
efi/preinstall/errors.go Corrected spelling errors in comments and error messages throughout the file
efi/preinstall/checks_context.go Reordered action sequences to prioritize automatic actions and added documentation explaining the ordering rationale
efi/preinstall/actions.go Fixed typos in action constant documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ernestl
ernestl force-pushed the ernestl/auto-before-manual-fix-actions branch 2 times, most recently from 6f7a2c3 to d63796c Compare November 24, 2025 12:18
@ernestl ernestl changed the title efi/preinstall: moved manual after automatic preinstall check actions efi/preinstall: reordered some preinstall check actions Nov 24, 2025
@ernestl
ernestl force-pushed the ernestl/auto-before-manual-fix-actions branch 2 times, most recently from 13d9d11 to 9140439 Compare November 24, 2025 15:36
@ernestl ernestl changed the title efi/preinstall: reordered some preinstall check actions efi/preinstall: reordered some preinstall check error-kind actions Nov 24, 2025
@ernestl ernestl changed the title efi/preinstall: reordered some preinstall check error-kind actions efi/preinstall: reordered some preinstall check actions Nov 24, 2025
@ernestl
ernestl force-pushed the ernestl/auto-before-manual-fix-actions branch from 9140439 to 4709c58 Compare November 24, 2025 17:55
@ernestl
ernestl force-pushed the ernestl/auto-before-manual-fix-actions branch from 4709c58 to d5d7d5d Compare November 24, 2025 19:38
@ernestl
ernestl requested a review from ZeyadYasser November 25, 2025 18:50
@chrisccoulson

Copy link
Copy Markdown
Contributor

I think this looks ok to me, but I am a bit confused about the user experience. Whilst I understand how ordering the returned errors will be presented (that's something I'm going to work on before Christmas), I'm still a bit unsure how this will be presented - ie, is the intention to only present the first action, and how are the others presented if there is more than one?

@ernestl

ernestl commented Nov 26, 2025

Copy link
Copy Markdown
Member Author

I think this looks ok to me, but I am a bit confused about the user experience. Whilst I understand how ordering the returned errors will be presented (that's something I'm going to work on before Christmas), I'm still a bit unsure how this will be presented - ie, is the intention to only present the first action, and how are the others presented if there is more than one?

Hi @chrisccoulson, thanks for having a look. The basic UX was captured here: Using the Snapd Error Kinds and Actions APIs. (Requires your review).

  • The installer handles a single error at a time, from the top down.
  • For each error, it finds the associated actions (explicitly provided by secboot via snapd API or implicit as required for some special cases). Here the order of the actions as provided by secboot matters, and the installer will display the list of actions as options in that order. The user will pick the action they prefer from the list and press a button to kick it off.

@ZeyadYasser ZeyadYasser left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, Thank you!

@ernestl
ernestl merged commit de0b33d into canonical:master Nov 27, 2025
2 checks passed
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.

4 participants