feat(onboarding): force reboot after internal boot setup and lock down wizard#1966
Open
Ajit-Mehrotra wants to merge 7 commits intomainfrom
Open
feat(onboarding): force reboot after internal boot setup and lock down wizard#1966Ajit-Mehrotra wants to merge 7 commits intomainfrom
Ajit-Mehrotra wants to merge 7 commits intomainfrom
Conversation
…n wizard - When internal boot configuration is applied during onboarding, the system must reboot to finalize setup regardless of success or failure - Previously, users could escape the reboot path via the X button, back navigation, browser back, keyboard shortcut, or URL bypass - This left the system in a potentially broken state if internal boot was partially configured but never finalized with a reboot Changes: - Add `internalBootApplyAttempted` flag to draft store that persists to localStorage and engages lockdown once the Summary step begins applying internal boot configuration - Hide X close button on the onboarding modal when lockdown is active - Hide back button on all steps when lockdown is active - Block browser back navigation (popstate) when lockdown is active - Block backward stepper clicks and exit intent when lockdown is active - Remove keyboard shortcut (Ctrl+Alt+Shift+O) and URL bypass (?onboarding=bypass) entirely as this feature has not shipped yet - Force reboot button on Next Steps even when internal boot apply fails, with messaging: "Internal boot timed out but is likely setup on your server. Please reboot your system to finalize setup." - Show BIOS manual update warning when BIOS update was selected but internal boot pool creation failed - Make completeOnboarding() failure non-blocking for the reboot path so users are never stuck (can't close, can't go back, can't reboot) - Apply the same lockdown to the standalone internal boot wizard: X hidden, popstate blocked, "Close" becomes "Reboot", failure messaging included - Update tests across 5 test files with new lockdown behavior coverage
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "Onboarding UI"
participant Store as "onboardingDraftStore"
participant Composable as "internalBoot composable"
participant Browser as "window.history"
UI->>Store: setInternalBootApplyAttempted(true)
UI->>UI: enter locked state (isInternalBootLocked)
UI->>Browser: push history / block back navigation
Note right of UI: Back/Close/Exit intents suppressed while locked
UI->>Composable: applyInternalBootSelection()
alt apply success
Store->>UI: set internalBootApplySucceeded(true)
UI->>UI: proceed to normal completion/close
else apply failure/timeout
Store-->>UI: internalBootApplyAttempted remains true
UI->>UI: show Reboot & Shutdown actions (confirm dialog)
UI->>Composable: submitInternalBootReboot() or submitInternalBootShutdown()
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1966 +/- ##
==========================================
+ Coverage 52.03% 52.08% +0.05%
==========================================
Files 1030 1031 +1
Lines 71349 71461 +112
Branches 8057 8094 +37
==========================================
+ Hits 37125 37222 +97
- Misses 34100 34114 +14
- Partials 124 125 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/__test__/components/Onboarding/OnboardingModal.test.ts (1)
484-490: Exercise the locked-exit path instead of rechecking initial render state.This case never triggers the close path, so it won’t catch regressions in the
@update:model-valueguard or the lockedpopstatehandling—it only reasserts that the X button is hidden. Emit the dialog close event or dispatch aPopStateEvent, then assert the exit dialog still does not open. As per coding guidelines, "Test component interactions such as clicks and inputs."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/Onboarding/OnboardingModal.test.ts` around lines 484 - 490, The test currently only asserts the X button is hidden but never exercises the locked-exit path; modify the test to simulate the close attempt by either emitting the component close event (emit 'update:model-value' with false on the mounted component via wrapper.vm.$emit or wrapper.setProps) or dispatching a PopStateEvent to window, then assert that the exit confirmation still does not appear (text does not contain 'Exit onboarding?') while onboardingDraftStore.internalBootApplyAttempted.value remains true; keep the use of mountComponent(), the 'Close onboarding' button selector, and the same final assertion to validate the guarded close behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/Onboarding/OnboardingModal.vue`:
- Around line 457-461: The popstate handler handlePopstate is currently
preventing programmatic history navigations used by closeModal when
isInternalBootLocked is true; add a short-circuit so handlePopstate ignores
popstate events triggered by our own closeModal (e.g., check a new sentinel like
a boolean flag isProgrammaticClose or a specific history.state marker set by
closeModal), and ensure closeModal sets and clears that sentinel around its
window.history.go(...) call; keep the existing behavior for real user
back/forward events by only bypassing the isInternalBootLocked guard when that
sentinel or state marker is present.
In `@web/src/components/Onboarding/steps/OnboardingSummaryStep.vue`:
- Around line 932-935: The current call to applyInternalBootSelection is
discarding the concrete error output and conflating the generic failed branch
with the timeout-like message; update the options passed to
applyInternalBootSelection so returnedError is a function that accepts the
backend output (e.g., returnedError: (output) => /* use output in the
message/log via summaryT/t */) and ensure failed maps to a distinct message (not
the timeout/likely-setup copy) so real backend errors are preserved in the
Summary console and logs; reference applyInternalBootSelection, returnedError,
failed, summaryT and t when making the change.
---
Nitpick comments:
In `@web/__test__/components/Onboarding/OnboardingModal.test.ts`:
- Around line 484-490: The test currently only asserts the X button is hidden
but never exercises the locked-exit path; modify the test to simulate the close
attempt by either emitting the component close event (emit 'update:model-value'
with false on the mounted component via wrapper.vm.$emit or wrapper.setProps) or
dispatching a PopStateEvent to window, then assert that the exit confirmation
still does not appear (text does not contain 'Exit onboarding?') while
onboardingDraftStore.internalBootApplyAttempted.value remains true; keep the use
of mountComponent(), the 'Close onboarding' button selector, and the same final
assertion to validate the guarded close behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ba423f79-c5e4-4dc8-b9bb-8dcbd5a88483
📒 Files selected for processing (12)
web/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.tsweb/__test__/components/Onboarding/OnboardingModal.test.tsweb/__test__/components/Onboarding/OnboardingNextStepsStep.test.tsweb/__test__/components/Onboarding/OnboardingSummaryStep.test.tsweb/__test__/store/onboardingModalVisibility.test.tsweb/src/components/Onboarding/OnboardingModal.vueweb/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vueweb/src/components/Onboarding/steps/OnboardingNextStepsStep.vueweb/src/components/Onboarding/steps/OnboardingSummaryStep.vueweb/src/components/Onboarding/store/onboardingDraft.tsweb/src/components/Onboarding/store/onboardingModalVisibility.tsweb/src/locales/en.json
💤 Files with no reviewable changes (1)
- web/test/store/onboardingModalVisibility.test.ts
… boot setup - Previously only a reboot button was available after internal boot configuration; some users may prefer to shut down instead (e.g., to physically relocate the server or swap drives before the next boot) - Add a smaller "Shutdown" text button next to the primary "Reboot" CTA, mirroring the "Skip Setup" / "Get Started" visual pattern - Extract shared `submitBootCommand()` helper in internalBoot.ts that accepts 'reboot' | 'shutdown', eliminating code duplication - Add shutdown confirmation dialog with same warnings (flash drive, BIOS boot order) as the reboot dialog - Shutdown path uses same `finishOnboarding()` flow: completeOnboarding mutation → cleanup → shutdown, with same resilience (proceeds even if completeOnboarding fails) - Add shutdown button to standalone internal boot wizard as well - Add 4 new i18n keys for shutdown UI - Add 6 new tests covering shutdown visibility, confirmation flow, and resilience across both wizard and standalone components
…nd log messages
Issue 1 — Popstate lock traps programmatic closeModal():
- closeModal() uses window.history.go() for manual sessions, which fires
a popstate event; the lockdown guard intercepted this and called
history.forward(), trapping the user
- Add isProgrammaticHistoryExit ref that closeModal sets before calling
history.go(), allowing handlePopstate to skip the lock for our own
programmatic navigations
Issue 2 — Error messages lost in Summary console logs:
- The returnedError and failed messages in SummaryStep were remapped to
the generic "timed out" i18n key, which dropped the specific error
output from the technical log stream
- Restore original summaryT('logs.internalBootReturnedError', { output })
and summaryT('logs.internalBootFailed') so the expandable console keeps
specific error details
- The generic "timed out but likely setup" message correctly shows only
on the Next Steps page alert, not in the detailed logs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 410109faa1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| type="button" | ||
| data-testid="internal-boot-standalone-reboot" | ||
| class="bg-primary hover:bg-primary/90 rounded-md px-4 py-2 text-sm font-semibold text-white transition-colors" | ||
| @click="submitInternalBootReboot()" |
There was a problem hiding this comment.
In the locked standalone flow, the power buttons now call submitInternalBootReboot()/submitInternalBootShutdown() directly without first running cleanupOnboardingStorage(). Since internalBootApplyAttempted is persisted, this leaves the onboarding draft in localStorage after the power action and can cause later onboarding sessions to reopen in a locked, non-dismissible state (close/back/popstate are blocked off that flag). Mirror the Next Steps flow by clearing onboarding storage before submitting either power command.
Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/Onboarding/OnboardingModal.vue (1)
427-449:⚠️ Potential issue | 🟠 MajorDismiss or block an already-open exit dialog when the lock turns on.
Right now the lock only prevents opening the exit dialog. If
showExitConfirmDialogis alreadytruewheninternalBootApplyAttemptedflips, nothing closes it, andhandleExitConfirm()still runscloseModal(), which lets the user escape the reboot path after the lock is supposed to be active.Suggested fix
const handleExitIntent = () => { if (isClosingModal.value || isInternalBootLocked.value) { return; } showExitConfirmDialog.value = true; }; +watch(isInternalBootLocked, (locked) => { + if (locked) { + showExitConfirmDialog.value = false; + } +}); + const handleExitConfirm = async () => { + if (isInternalBootLocked.value) { + showExitConfirmDialog.value = false; + return; + } showExitConfirmDialog.value = false; isClosingModal.value = true; try { await closeModal(); } finally {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/OnboardingModal.vue` around lines 427 - 449, When the internal boot lock (isInternalBootLocked) becomes true we must close or block any already-open exit confirmation and prevent confirm actions; add a watcher on isInternalBootLocked that sets showExitConfirmDialog.value = false when it flips true, and update handleExitConfirm to no-op (return early) if isInternalBootLocked.value is true to avoid calling closeModal while locked; also ensure handleExitCancel similarly respects isInternalBootLocked (no-op) so the dialog cannot be interacted with once the lock is active.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@web/src/components/Onboarding/OnboardingModal.vue`:
- Around line 427-449: When the internal boot lock (isInternalBootLocked)
becomes true we must close or block any already-open exit confirmation and
prevent confirm actions; add a watcher on isInternalBootLocked that sets
showExitConfirmDialog.value = false when it flips true, and update
handleExitConfirm to no-op (return early) if isInternalBootLocked.value is true
to avoid calling closeModal while locked; also ensure handleExitCancel similarly
respects isInternalBootLocked (no-op) so the dialog cannot be interacted with
once the lock is active.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d30f4191-b78b-44c6-ac2c-70375ca6befc
📒 Files selected for processing (2)
web/src/components/Onboarding/OnboardingModal.vueweb/src/components/Onboarding/steps/OnboardingSummaryStep.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/components/Onboarding/steps/OnboardingSummaryStep.vue
…irmation to standalone
- The wizard flow (OnboardingNextStepsStep) had two nearly identical UModal
blocks for reboot and shutdown confirmation, while the standalone flow
had no confirmation dialog at all (direct submit on click)
- This inconsistency meant users could accidentally trigger a reboot or
shutdown in the standalone wizard with a single click
Changes:
- Extract InternalBootConfirmDialog.vue shared component that accepts
action ('reboot' | 'shutdown'), failed state, and emits confirm/cancel
- Replace both UModal blocks in OnboardingNextStepsStep with single
shared dialog instance using pendingPowerAction ref
- Wire shared dialog into standalone component so reboot/shutdown buttons
now show confirmation before proceeding
- Remove dead i18n keys (confirmShutdown.description, confirmShutdown.confirm)
that duplicated confirmReboot equivalents
- Update standalone tests to click through confirmation dialog
- Standalone flow did not call cleanupOnboardingStorage() before form
submit, so the internalBootApplyAttempted flag would persist in
localStorage if the reboot/shutdown failed — causing a permanent
lockout on page refresh
- Now calls cleanupOnboardingStorage() before submitting the form in
the standalone confirm handler, matching the wizard flow behavior
- Change finishOnboarding signature from { reboot?: boolean; shutdown?:
boolean } to { action?: 'reboot' | 'shutdown' } to make the options
mutually exclusive by type
- Update confirmation dialog warning text to be generic ("fully
restarted") instead of reboot-specific, so it reads correctly for
both reboot and shutdown paths
- Update failure description to say "Restarting" instead of "Rebooting"
…d shutdown - The confirmation dialog now shows identical content for both reboot and shutdown — same description, same warning, same confirm button text - Only the title differs: "Confirm Reboot" vs "Confirm Shutdown" - Removed `failed` prop from InternalBootConfirmDialog since the failure context is already shown via alert banners on the Next Steps page - Removed unused `failureDescription` i18n key and `internalBootFailed` computed from standalone component
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/Onboarding/components/InternalBootConfirmDialog.vue`:
- Around line 25-29: The computed text keys currently hard-code reboot messages;
update the computed properties (e.g., description and the similar warning
computed around lines 46-50) to select message keys based on props.action (or
switch to a neutral shared key) instead of always using 'confirmReboot.*'; for
example, choose 'onboarding.nextSteps.confirmShutdown.*' when props.action ===
'shutdown' and 'onboarding.nextSteps.confirmReboot.*' otherwise, ensuring both
the failure/normal branches use the action-aware key selection.
In
`@web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue`:
- Around line 60-68: The current handleConfirmPowerAction treats any
non-'shutdown' value (including null) as reboot; update handleConfirmPowerAction
so it first reads and clears pendingPowerAction.value, calls
cleanupOnboardingStorage(), then explicitly handle only 'shutdown' and 'reboot'
cases (call submitInternalBootShutdown() for 'shutdown' and
submitInternalBootReboot() for 'reboot'); for any other value (including null)
do nothing/no-op to avoid issuing an unintended reboot. Use the existing
pendingPowerAction.value, submitInternalBootShutdown, submitInternalBootReboot,
and cleanupOnboardingStorage identifiers to locate and update the logic.
In `@web/src/components/Onboarding/steps/OnboardingNextStepsStep.vue`:
- Around line 62-67: The failure computed currently flips as soon as a selection
exists because setInternalBootSelection() initializes
internalBootApplySucceeded=false; change the internalBootFailed computed to
require an actual apply attempt by checking
draftStore.internalBootApplyAttempted (e.g. internalBootApplyAttempted === true
&& !draftStore.internalBootApplySucceeded) instead of only selection !== null,
and keep biosUpdateMissed derived from that internalBootFailed computed
(biosUpdateMissed should remain internalBootFailed.value &&
(draftStore.internalBootSelection?.updateBios ?? false)); update references to
internalBootFailed and ensure setInternalBootSelection and any state
initialization still set internalBootApplyAttempted appropriately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1d1e5a28-09ad-4be5-91c9-d4789846c8b8
📒 Files selected for processing (5)
web/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.tsweb/src/components/Onboarding/components/InternalBootConfirmDialog.vueweb/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vueweb/src/components/Onboarding/steps/OnboardingNextStepsStep.vueweb/src/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/locales/en.json
Comment on lines
+25
to
+29
| const description = computed(() => | ||
| props.failed | ||
| ? t('onboarding.nextSteps.confirmReboot.failureDescription') | ||
| : t('onboarding.nextSteps.confirmReboot.description') | ||
| ); |
Contributor
There was a problem hiding this comment.
The shutdown confirmation still uses reboot-only messaging.
The failed description and warning are hard-wired to confirmReboot.*. On the shutdown path that tells users about restarting, which is misleading in the final forced-action step. Please branch those strings on action, or move them to neutral keys shared by both actions.
Also applies to: 46-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/Onboarding/components/InternalBootConfirmDialog.vue`
around lines 25 - 29, The computed text keys currently hard-code reboot
messages; update the computed properties (e.g., description and the similar
warning computed around lines 46-50) to select message keys based on
props.action (or switch to a neutral shared key) instead of always using
'confirmReboot.*'; for example, choose 'onboarding.nextSteps.confirmShutdown.*'
when props.action === 'shutdown' and 'onboarding.nextSteps.confirmReboot.*'
otherwise, ensuring both the failure/normal branches use the action-aware key
selection.
Comment on lines
+60
to
+68
| const handleConfirmPowerAction = () => { | ||
| const action = pendingPowerAction.value; | ||
| pendingPowerAction.value = null; | ||
| cleanupOnboardingStorage(); | ||
| if (action === 'shutdown') { | ||
| submitInternalBootShutdown(); | ||
| } else { | ||
| submitInternalBootReboot(); | ||
| } |
Contributor
There was a problem hiding this comment.
Do not treat a missing pending action as reboot.
pendingPowerAction is nullable, but anything other than 'shutdown' falls through to submitInternalBootReboot(). A stray/null confirm here becomes a destructive reboot request; guard null and branch explicitly on both valid actions.
Suggested guard
const handleConfirmPowerAction = () => {
const action = pendingPowerAction.value;
pendingPowerAction.value = null;
+ if (!action) {
+ return;
+ }
cleanupOnboardingStorage();
if (action === 'shutdown') {
submitInternalBootShutdown();
- } else {
+ } else if (action === 'reboot') {
submitInternalBootReboot();
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleConfirmPowerAction = () => { | |
| const action = pendingPowerAction.value; | |
| pendingPowerAction.value = null; | |
| cleanupOnboardingStorage(); | |
| if (action === 'shutdown') { | |
| submitInternalBootShutdown(); | |
| } else { | |
| submitInternalBootReboot(); | |
| } | |
| const handleConfirmPowerAction = () => { | |
| const action = pendingPowerAction.value; | |
| pendingPowerAction.value = null; | |
| if (!action) { | |
| return; | |
| } | |
| cleanupOnboardingStorage(); | |
| if (action === 'shutdown') { | |
| submitInternalBootShutdown(); | |
| } else if (action === 'reboot') { | |
| submitInternalBootReboot(); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue`
around lines 60 - 68, The current handleConfirmPowerAction treats any
non-'shutdown' value (including null) as reboot; update handleConfirmPowerAction
so it first reads and clears pendingPowerAction.value, calls
cleanupOnboardingStorage(), then explicitly handle only 'shutdown' and 'reboot'
cases (call submitInternalBootShutdown() for 'shutdown' and
submitInternalBootReboot() for 'reboot'); for any other value (including null)
do nothing/no-op to avoid issuing an unintended reboot. Use the existing
pendingPowerAction.value, submitInternalBootShutdown, submitInternalBootReboot,
and cleanupOnboardingStorage identifiers to locate and update the logic.
- internalBootFailed computed was true as soon as a selection existed because internalBootApplySucceeded starts false, causing the failure alerts to briefly show before the apply even ran - Now requires internalBootApplyAttempted to be true before reporting failure, so alerts only appear after the apply has actually been attempted - Update tests to set internalBootApplyAttempted = true when testing failure states
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue (1)
59-68:⚠️ Potential issue | 🟠 MajorGuard against null action to prevent unintended reboot.
Unlike the fix in
OnboardingNextStepsStep.vue(lines 161-167), this handler doesn't guard against a nullaction. IfpendingPowerActionis somehow cleared before the handler executes, theelsebranch will triggersubmitInternalBootReboot()unintentionally. Additionally,cleanupOnboardingStorage()is called unconditionally even when no valid action exists.Proposed fix
const handleConfirmPowerAction = () => { const action = pendingPowerAction.value; pendingPowerAction.value = null; + if (!action) { + return; + } cleanupOnboardingStorage(); if (action === 'shutdown') { submitInternalBootShutdown(); - } else { + } else if (action === 'reboot') { submitInternalBootReboot(); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue` around lines 59 - 68, The handler handleConfirmPowerAction currently reads pendingPowerAction.value, clears it, and always calls cleanupOnboardingStorage() and either submitInternalBootShutdown() or submitInternalBootReboot(), which can trigger an unintended reboot when action is null; modify handleConfirmPowerAction so it first reads const action = pendingPowerAction.value and if action is null/undefined simply return (do not call cleanupOnboardingStorage or any submit*), otherwise then call cleanupOnboardingStorage() and call submitInternalBootShutdown() only when action === 'shutdown' else submitInternalBootReboot(); ensure you reference the pendingPowerAction reactive, handleConfirmPowerAction, cleanupOnboardingStorage, submitInternalBootShutdown, and submitInternalBootReboot symbols when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue`:
- Around line 59-68: The handler handleConfirmPowerAction currently reads
pendingPowerAction.value, clears it, and always calls cleanupOnboardingStorage()
and either submitInternalBootShutdown() or submitInternalBootReboot(), which can
trigger an unintended reboot when action is null; modify
handleConfirmPowerAction so it first reads const action =
pendingPowerAction.value and if action is null/undefined simply return (do not
call cleanupOnboardingStorage or any submit*), otherwise then call
cleanupOnboardingStorage() and call submitInternalBootShutdown() only when
action === 'shutdown' else submitInternalBootReboot(); ensure you reference the
pendingPowerAction reactive, handleConfirmPowerAction, cleanupOnboardingStorage,
submitInternalBootShutdown, and submitInternalBootReboot symbols when making the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ed132c93-2443-4740-abdc-dd9e67669fc8
📒 Files selected for processing (5)
web/__test__/components/Onboarding/OnboardingNextStepsStep.test.tsweb/src/components/Onboarding/components/InternalBootConfirmDialog.vueweb/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vueweb/src/components/Onboarding/steps/OnboardingNextStepsStep.vueweb/src/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (3)
- web/src/components/Onboarding/components/InternalBootConfirmDialog.vue
- web/src/locales/en.json
- web/test/components/Onboarding/OnboardingNextStepsStep.test.ts
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
1 participant
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.
Summary
When internal boot configuration is applied during onboarding, the system must reboot (or shut down) to finalize setup — regardless of whether the apply succeeded or failed. Previously, users could escape the reboot path via the X button, back navigation, browser back, a keyboard shortcut (
Ctrl+Alt+Shift+O), or a URL bypass (?onboarding=bypass). This left the system in a potentially broken state if internal boot was partially configured but the user never rebooted.Changes
Lockdown mechanism
internalBootApplyAttemptedflag to the onboarding draft store (persisted to localStorage)truewhen the Summary step begins applying internal boot, before the API call — this engages the lockdown immediatelyisInternalBootLockedcomputedWizard lockdown (OnboardingModal.vue)
handleExitIntent,goToPreviousStep— early return when lockedgoToStep— blocks backward stepper clicks when lockedhandlePopstate— callswindow.history.forward()to neutralize browser back, with anisProgrammaticHistoryExitguard so the modal's owncloseModal()history navigation isn't blockedCtrl+Alt+Shift+Oand URL parameter?onboarding=bypassentirely (feature hasn't shipped)Forced reboot/shutdown on success or failure (OnboardingNextStepsStep.vue)
showRebootButtonnow checksinternalBootSelection !== nullinstead ofinternalBootApplySucceeded— reboot shows regardless of outcomeupdateBioswas selected but apply failed — instructs user to manually update BIOS boot ordercompleteOnboarding()failure non-blocking for the reboot/shutdown path — wraps in try/catch so users are never stuckStandalone lockdown (OnboardingInternalBoot.standalone.vue)
updateBiosselectedLog message preservation (SummaryStep + composable)
returnedErrorandfailedcallbacks inapplyInternalBootSelectionpreserve the original error output in the user-visible log streami18n
en.jsonfor failure messaging, BIOS warnings, shutdown button, and dialog descriptionsTest plan
Files changed (13)
store/onboardingDraft.tsinternalBootApplyAttemptedflag + setter + persistencestore/onboardingModalVisibility.tsbypassOnboarding()OnboardingModal.vuecomposables/internalBoot.tsreturnedError/failedcallbackssteps/OnboardingSummaryStep.vueinternalBootApplyAttemptedbefore applysteps/OnboardingNextStepsStep.vuefinishOnboardingstandalone/OnboardingInternalBoot.standalone.vuelocales/en.jsonSummary by CodeRabbit
New Features
Improvements
Removed