Uh oh!
There was an error while loading. Please reload this page.
fix: share internal boot state across onboarding - #1920
Merged
Conversation
Contributor
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
Disabled knowledge base sources:
WalkthroughAdds a new boolean Vars field Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as "UI / Client"
participant Resolver as "VarsResolver"
participant Service as "InternalBootStateService"
participant Array as "ArrayService"
participant Disks as "DisksService"
UI->>Resolver: query vars
Resolver->>Service: getBootedFromFlashWithInternalBootSetup()
Service->>Array: getArray() / read boot disk
Array-->>Service: bootDisk info
Service->>Disks: getInternalBootDevices(bootDisk) (if FLASH)
Disks-->>Service: devices[]
Service-->>Resolver: boolean bootedFromFlashWithInternalBootSetup
Resolver-->>UI: vars { ..., bootedFromFlashWithInternalBootSetup }
UI->>UI: hide/disable internal-boot step (if true)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can use oxc to improve the quality of JavaScript and TypeScript code reviews.Add a configuration file to your project to customize how CodeRabbit runs oxc. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:c9abf6f218
ℹ️ 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 (@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 (@codex) address that feedback".
Comment on lines
+24
to
+25
| const bootedFromFlashWithInternalBootSetup = | ||
| await this.internalBootStateService.getBootedFromFlashWithInternalBootSetup(); |
There was a problem hiding this comment.
vars() now blocks on internalBootStateService.getBootedFromFlashWithInternalBootSetup(), but this probe can throw (it reaches DisksService.getInternalBootDevices(), which calls disk discovery methods that are not guarded for all failures). Because there is no try/catch or fallback here, a transient disk-inspection error will fail the entire vars query (and serverState queries that include vars) instead of returning the existing vars payload.
Useful? React with 👍 / 👎.
| }; | ||
| } | ||
| if (await this.isBootedFromFlashWithInternalBootSetup()) { |
There was a problem hiding this comment.
The new isBootedFromFlashWithInternalBootSetup() check runs before the method’s try/catch, so if internal-boot state lookup throws (for example while reading array/disk state), createInternalBootPool() rejects with a GraphQL error instead of returning the structured OnboardingInternalBootResult failure response this mutation otherwise guarantees.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@## main #1920 +/- ##
==========================================
+ Coverage 50.99% 51.04% +0.05%
==========================================
Files 1024 1025 +1 Lines 70569 70640 +71 Branches 7717 7738 +21 ==========================================
+ Hits 35985 36061 +76 + Misses 34458 34453 -5
Partials 126 126 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@api/src/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.spec.ts`:
- Around line 243-247: The test is asserting the full error string which couples
it to wording; instead assert behavior and a meaningful substring: keep checking
result for ok: false and code: 3 (e.g., via toMatchObject or separate expects
against result) and replace the exact-string check on result.output with a
substring assertion (e.g., expect(result.output).toContain('boot is already
configured') or 'internal boot'), referencing the test variable result in
onboarding-internal-boot.service.spec.ts.
In
`@api/src/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.ts`:
- Around line 351-357: The current pre-check calls
this.isBootedFromFlashWithInternalBootSetup() outside the surrounding try/catch
so any error from that lookup can escape and crash the mutation; move the
boot-state lookup into the guarded path by invoking
this.isBootedFromFlashWithInternalBootSetup() inside the existing try/catch (or
wrap it in its own try/catch) and return the same { ok:false, code:3, output:...
} response on a successful check while ensuring any thrown error is caught and
handled by the function's error handling.
In `@api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts`:
- Around line 24-30: The vars() resolver currently awaits
internalBootStateService.getBootedFromFlashWithInternalBootSetup() which can
throw and cause the whole vars query to fail; wrap that call in a try/catch
inside vars() and on error set a safe default (e.g., null or false) for
bootedFromFlashWithInternalBootSetup and optionally log the error, then continue
returning the merged object (id: 'vars', ...(getters.emhttp().var ?? {}),
bootedFromFlashWithInternalBootSetup) so failures in internalBootStateService do
not surface as fatal resolver errors.
In `@web/src/store/server.ts`:
- Around line 1072-1074: The current guard using "typeof
data?.bootedFromFlashWithInternalBootSetup !== 'undefined'" prevents clearing
stale reportedBootedFromFlashWithInternalBootSetup values; change the check to
use "'bootedFromFlashWithInternalBootSetup' in data" and when that property
exists assign reportedBootedFromFlashWithInternalBootSetup.value =
data.bootedFromFlashWithInternalBootSetup so the value can be set to
undefined/false by mutateServerStateFromApi mapping; update the code around the
existing reportedBootedFromFlashWithInternalBootSetup assignment in server.ts
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a76466e7-5467-4496-b5a1-3e9733144beb
📒 Files selected for processing (22)
api/generated-schema.graphqlapi/src/unraid-api/graph/resolvers/disks/disks.module.tsapi/src/unraid-api/graph/resolvers/disks/internal-boot-notification.service.spec.tsapi/src/unraid-api/graph/resolvers/disks/internal-boot-notification.service.tsapi/src/unraid-api/graph/resolvers/disks/internal-boot-state.service.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.spec.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.tsapi/src/unraid-api/graph/resolvers/vars/vars.model.tsapi/src/unraid-api/graph/resolvers/vars/vars.resolver.spec.tsapi/src/unraid-api/graph/resolvers/vars/vars.resolver.tsweb/__test__/components/Onboarding/OnboardingInternalBootStep.test.tsweb/__test__/components/Onboarding/OnboardingModal.test.tsweb/src/components/Onboarding/OnboardingModal.vueweb/src/components/Onboarding/graphql/getInternalBootContext.query.tsweb/src/components/Onboarding/graphql/getInternalBootStepVisibility.query.tsweb/src/components/Onboarding/steps/OnboardingInternalBootStep.vueweb/src/components/Registration.standalone.vueweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsweb/src/store/server.fragment.tsweb/src/store/server.tsweb/types/server.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Eli Bosley (elibosley)
commented
Mar 16, 2026
MemberAuthor
CodeRabbit (@coderabbitai) review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Uh oh!
There was an error while loading. Please reload this page.
Eli Bosley (elibosley) pushed a commit
that referenced
this pull request
Mar 18, 2026
🤖 I have created a release *beep* *boop* --- ## [4.30.0](v4.29.2...v4.30.0) (2026-03-18) ### Features * add internal boot step to onboarding flow ([#1881](#1881)) ([337aecc](337aecc)) * Add TPM licensing availability to registration ([#1908](#1908)) ([aa162eb](aa162eb)) * add UPS power ([#1874](#1874)) ([b531aed](b531aed)) * **api:** alert when usb boot has internal boot target ([#1898](#1898)) ([b94df47](b94df47)) * **api:** expose internal boot devices in array GraphQL ([#1894](#1894)) ([0736709](0736709)) * docker overview ([#1855](#1855)) ([9ef1cf1](9ef1cf1)) * **docker:** add update actions to container context menu ([#1867](#1867)) ([4ca3e06](4ca3e06)) * **docker:** disable containers page file modification ([#1870](#1870)) ([aaa0372](aaa0372)) * issues/1597: Temperature Monitoring - Thanks @MitchellThompkins ([a1be458](a1be458)) * New Crowdin updates ([#1809](#1809)) ([a7b3f07](a7b3f07)) * New Crowdin updates ([#1883](#1883)) ([14a8fa8](14a8fa8)) * **onboarding:** add new onboarding flows for Unraid OS ([#1746](#1746)) ([15bd747](15bd747)) * registration and trial actions use Account app ([#1928](#1928)) ([c2c0425](c2c0425)) * share internal boot state ([#1921](#1921)) ([8e4d44d](8e4d44d)) * **web:** show TPM move control for trial licenses ([#1911](#1911)) ([d00fb63](d00fb63)) ### Bug Fixes * Add dedicated TPM license move option ([#1909](#1909)) ([36c56f7](36c56f7)) * allow free USB targets in onboarding internal boot setup ([#1903](#1903)) ([298da54](298da54)) * API key key display truncation ([#1890](#1890)) ([b12f75c](b12f75c)) * **api:** harden PHP wrapper args for newer PHP versions ([#1901](#1901)) ([849f177](849f177)) * **api:** prevent flash notification startup fd exhaustion ([#1893](#1893)) ([4b231ad](4b231ad)) * clear stale onboarding modal session state ([#1904](#1904)) ([23f7836](23f7836)) * consistently clear onboarding draft ([#1916](#1916)) ([199d803](199d803)) * correct graphql-api.log timestamp formatting ([#1918](#1918)) ([243c5a8](243c5a8)) * **deps:** pin dependencies ([#1878](#1878)) ([db88eb8](db88eb8)) * **docker:** change "visit" to "webui" & use correct link ([#1863](#1863)) ([cab0880](cab0880)) * **docker:** improve start/stop UX with visual feedback ([#1865](#1865)) ([c084e25](c084e25)) * **docker:** remove aggressive caching to ensure data correctness ([#1864](#1864)) ([1c1bae8](1c1bae8)) * **docker:** sync template mappings in organizer to prevent false orphan warnings ([#1866](#1866)) ([38a6f0c](38a6f0c)) * onboarding internal-boot warning panel contrast and semantics ([#1927](#1927)) ([bb6f241](bb6f241)) * **onboarding:** add explicit EFI loader path for flash entry ([#1926](#1926)) ([429b438](429b438)) * **onboarding:** extend onboarding refresh timeout ([#1925](#1925)) ([e2a5f44](e2a5f44)) * **onboarding:** persist installed plugins in summary ([#1915](#1915)) ([07f4ebd](07f4ebd)) * **onboarding:** refine storage boot setup UX ([#1900](#1900)) ([1108d0a](1108d0a)) * polish onboarding flow ([#1902](#1902)) ([8742cac](8742cac)) * preserve registration device limits after refresh ([#1905](#1905)) ([234bfc7](234bfc7)) * prevent onboarding on API errors ([#1917](#1917)) ([540d6f9](540d6f9)) * remap TPM guid prefix to 01 ([#1924](#1924)) ([5360b5b](5360b5b)) * Return null for corrupted/invalid API key files and add Connect fixtures test ([#1886](#1886)) ([013e6c5](013e6c5)) * share internal boot state across onboarding ([#1920](#1920)) ([f9b293f](f9b293f)) * too many file descriptors with thousands of notifications ([#1887](#1887)) ([7956987](7956987)) * Treat onboarding patch updates as completed ([#1884](#1884)) ([d03b25e](d03b25e)) * unify onboarding internal boot state refresh ([#1923](#1923)) ([d3032c1](d3032c1)) * **web:** refresh internal boot onboarding state ([#1913](#1913)) ([1ca2129](1ca2129)) * **web:** stop showing callback errors after successful key installs ([#1892](#1892)) ([45f1402](45f1402)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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 freeto 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
varsand use it to hide/block onboarding internal boot setup when the system is still booted from flash but internal boot is already configuredValidation
pnpm lintinapi/pnpm type-checkinapi/pnpm lintinweb/pnpm type-checkinweb/Summary by CodeRabbit
New Features
Improvements