feat(api): expose internal boot devices in array GraphQL#1894
Merged
Ajit-Mehrotra merged 6 commits intomainfrom Mar 10, 2026
Merged
feat(api): expose internal boot devices in array GraphQL#1894Ajit-Mehrotra merged 6 commits intomainfrom
Ajit-Mehrotra merged 6 commits intomainfrom
Conversation
- Purpose: align array boot detection with Unraid/webgui by honoring \ entries whose type is \, and expose that distinction in GraphQL. - Before: the API parser collapsed internal boot semantics into existing disk categories, so \ could miss internal boot devices and clients could not distinguish boot-pool entries from flash or cache disks. - Problem: this diverged from the source-of-truth in emhttp/webgui, caused incorrect \ results for internal boot systems, and hid the boot role from generated client types. - Change: add \ to \, preserve \ from \ in the slots parser, and select \ using the same boot-disk precedence as webgui with legacy flash fallback. - How: update the array schema and generated GraphQL artifacts, carry non-enumerated fs status needed for boot selection, add parser and array tests, and keep cache/disk grouping separate from boot-only entries.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughA new BOOT disk type is introduced to the Unraid API's disk type system, along with boot device detection and selection logic. The changes add infrastructure to parse boot disks from the configuration, enumerate all detected boot devices, and select the active boot disk through a multi-step fallback algorithm, exposing both as GraphQL fields. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
elibosley
approved these changes
Mar 9, 2026
Member
elibosley
left a comment
elibosley
left a comment
There was a problem hiding this comment.
This looks safe - just make sure you test for a case where there is no internal boot disk
![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: 1
🧹 Nitpick comments (1)
api/src/store/state-parsers/slots.ts (1)
100-106: Non-enumerablefsStatusis intentionally internal-only but carries subtle risks.Defining
fsStatusas non-enumerable means:
- Direct access (
disk.fsStatus) works ✓- The
selectBootDiskfunction can read it ✓- Object spread (
{...disk}) loses the propertyJSON.stringifyomits itObject.keys()excludes itThis appears intentional (internal boot selection without GraphQL exposure), but consider adding a brief code comment explaining why non-enumerable was chosen, to prevent future confusion when someone attempts to spread or serialize disk objects.
📝 Suggested documentation
+ // fsStatus is non-enumerable to keep it internal for boot selection + // without exposing it through GraphQL or object serialization Object.defineProperties(result, { fsStatus: { value: slot.fsStatus ?? null, enumerable: false, writable: true, }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/store/state-parsers/slots.ts` around lines 100 - 106, The fsStatus property is defined as non-enumerable via Object.defineProperties(result, { fsStatus: { ... } }) which is intentional for internal-only use (readable by selectBootDisk and direct access) but will be omitted by object spread, JSON.stringify and Object.keys; add a brief inline comment immediately above the Object.defineProperties call (referencing fsStatus, selectBootDisk and result) that states why fsStatus is non-enumerable, that it must remain internal-only, and warns about spread/serialization omissions to prevent future confusion.
🤖 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/array/array.model.ts`:
- Around line 132-133: The property fsStatus is declared without a GraphQL
decorator so it won't be exposed; add the `@Field` decorator to the fsStatus
property (e.g. `@Field`(() => String, { nullable: true })) and ensure Field is
imported from `@nestjs/graphql`, placing the decorator directly above the fsStatus
declaration in the same class (the property named fsStatus in array.model.ts) so
it becomes part of the generated GraphQL schema.
---
Nitpick comments:
In `@api/src/store/state-parsers/slots.ts`:
- Around line 100-106: The fsStatus property is defined as non-enumerable via
Object.defineProperties(result, { fsStatus: { ... } }) which is intentional for
internal-only use (readable by selectBootDisk and direct access) but will be
omitted by object spread, JSON.stringify and Object.keys; add a brief inline
comment immediately above the Object.defineProperties call (referencing
fsStatus, selectBootDisk and result) that states why fsStatus is non-enumerable,
that it must remain internal-only, and warns about spread/serialization
omissions to prevent future confusion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e0799012-0036-4d5f-b7dc-8306c9a47bdc
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (7)
api/generated-schema.graphqlapi/src/__test__/core/modules/array/get-array-data.test.tsapi/src/__test__/store/state-parsers/slots.test.tsapi/src/core/modules/array/get-array-data.tsapi/src/store/state-parsers/slots.tsapi/src/unraid-api/graph/resolvers/array/array.model.tsweb/src/composables/gql/graphql.ts
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1894 +/- ##
==========================================
+ Coverage 49.73% 49.83% +0.09%
==========================================
Files 1021 1021
Lines 69966 70037 +71
Branches 7383 7421 +38
==========================================
+ Hits 34799 34901 +102
+ Misses 35044 35013 -31
Partials 123 123 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Purpose: expose all detected boot devices while keeping a single selected boot entry for compatibility. - Before: the API only returned one boot disk and could not represent mirrored internal boot members. - Problem: internal boot can expose multiple type="Boot" entries in disks.ini, but clients had no way to inspect the full boot set. - Change: add array.bootDevices, deprecate array.boot, and select array.boot from the active /boot-mounted boot member when multiple boot disks exist. - Details: preserve boot-specific runtime metadata from disks.ini parsing, update array boot selection logic, add tests, and regenerate GraphQL-facing types/schema artifacts.
- Purpose: make root and api type-check runs deterministic in a clean workspace. - Before: api type-check could run before @unraid/shared had built dist artifacts, which broke workspace package resolution. - Problem: unresolved @unraid/shared imports cascaded into many false type errors across API models and tests. - Change: add a pretype-check step that builds @unraid/shared before running tsc in the API package. - Details: this keeps pnpm type-check green without changing runtime behavior or broadening TypeScript path hacks.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/composables/gql/graphql.ts`:
- Around line 263-264: Docstring for the Disk discriminant is incorrect: it says
“Array” but the exported enum discriminant is DATA. Update the property comment
for "type: ArrayDiskType" (and any related JSDoc) to reflect the actual enum
value, e.g., "Data (DATA) - used to differentiate Boot / Cache / Flash /
Parity", or rename the enum/discriminant if you intended "Array" instead of
"DATA"; ensure the text and the enum member (DATA on ArrayDiskType) are
consistent so client typings match runtime values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 00933214-ba48-44f0-9b79-cac8ccff008b
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (8)
api/generated-schema.graphqlapi/package.jsonapi/src/__test__/core/modules/array/get-array-data.test.tsapi/src/core/modules/array/get-array-data.tsapi/src/store/state-parsers/slots.tsapi/src/unraid-api/graph/resolvers/array/array.model.tsapi/src/unraid-api/graph/resolvers/array/array.service.spec.tsweb/src/composables/gql/graphql.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/array/array.model.ts
Ajit-Mehrotra
changed the title
- Purpose: keep the existing array.boot field as a supported part of the contract. - Before: array.boot was marked deprecated even though it still represents the active boot disk clients care about. - Problem: the deprecation suggested clients should stop using array.boot entirely, which did not match the intended API design. - Change: remove the deprecation and update the field description to state that boot returns the active boot disk. - Details: update the source GraphQL model and sync the generated schema and client type artifacts in api and web.
- Purpose: address the remaining CodeRabbit review notes on the internal boot API changes. - Before: the public disk type docs still said Array even though the enum value is DATA, and the internal-only boot-selection metadata did not explain why it stays off the GraphQL schema. - Problem: client typings were misleading and the internal fsStatus/fsMountpoint fields looked like accidental omissions. - Change: update the disk type description to reference Data (DATA) and document that fsStatus/fsMountpoint are runtime-only helpers kept out of GraphQL and object serialization. - Details: sync the source model, slots parser comment, and generated API/web GraphQL artifacts with the clarified behavior.
Contributor
Author
|
addressing coderabbit, testing briefly, then should be good for another round of PR review |
This reverts commit fb3b50a.
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
![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: bce81a6c1c
ℹ️ 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".
| disks.filter((disk) => disk.type === ArrayDiskType.BOOT); | ||
|
|
||
| const getUsbBootDevices = (disks: ArrayDisk[]): ArrayDisk[] => | ||
| disks.filter((disk) => disk.type === ArrayDiskType.FLASH && disk.fsMountpoint === '/boot'); |
There was a problem hiding this comment.
Filtering USB boot devices to disk.fsMountpoint === '/boot' drops legacy flash boot detection when disks.ini does not include fsMountpoint (the parser sets it to null in api/src/store/state-parsers/slots.ts), so bootDevices becomes empty and array.boot resolves to undefined even though a FLASH disk is present. This is a regression from the previous behavior (which selected the flash disk by type) and will surface on environments that only report fsStatus for flash entries.
Useful? React with 👍 / 👎.
pujitm
approved these changes
Mar 10, 2026
Ajit-Mehrotra
added a commit
that referenced
this pull request
Mar 10, 2026
## Summary Update the array boot API to understand Unraid internal boot devices using the same runtime signal as the webgui, and expose the full boot device set to clients. This change adds support for `type="Boot"` entries from `disks.ini`, introduces `BOOT` as a public `ArrayDiskType`, and adds `array.bootDevices` so mirrored/redundant internal boot members can be represented explicitly. ## Why Unraid now supports booting from storage devices instead of only USB flash devices. Before this PR: - the API only treated `FLASH` as boot - `array.boot` was a single disk chosen from `FLASH` - internal boot members from `disks.ini` were not represented correctly - clients had no way to inspect multiple boot devices in mirrored internal boot setups That meant the API no longer matched the webgui/runtime behavior for internal boot. ## What Changed ### Boot detection and selection - Preserve `type="Boot"` entries from `disks.ini` in the slots parser. - Expose those entries as `ArrayDiskType.BOOT`. - Build `array.bootDevices` from: - all `BOOT` entries for internal boot - otherwise, the mounted `/boot` `FLASH` entry for legacy USB boot - Select `array.boot` from `bootDevices` using the active/runtime member: - prefer the entry mounted at `/boot` - otherwise fall back to the first present boot device using the existing selection order ### GraphQL contract updates - Add `BOOT` to `ArrayDiskType` - Add `array.bootDevices: [ArrayDisk!]!` - Mark `array.boot` as deprecated but keep it supported - Update generated GraphQL types/schema artifacts in both `api` and `web` ### Parser/runtime details - Keep `fsStatus` and `fsMountpoint` on parsed disk objects as internal helper fields so boot selection can distinguish: - the active `/boot` member in mirrored internal boot setups - the mounted USB boot device in legacy flash boot setups ### Validation and tests - Add parser coverage for `type="Boot"` slots - Add array boot selection tests for: - internal boot preferred over flash - mirrored boot selection preferring the `/boot` mounted member - fallback behavior when only flash boot is present - ignoring non-mounted flash rows for legacy USB boot detection ## Behavior After This PR - Internal boot systems expose boot members as `type: BOOT` - `array.bootDevices` returns the full boot set - `array.boot` remains available and returns the selected active boot disk - Legacy USB boot still works, using the mounted `/boot` flash entry - Mirrored internal boot setups can now be represented without conflating boot devices with cache devices <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for detecting and enumerating multiple boot devices, enabling better identification of all available boot options on the system. * Boot disk selection now prioritizes internal boot devices over legacy USB boot entries with improved fallback logic. * **Tests** * Added comprehensive test coverage for boot device detection and selection scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
3 participants
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
Update the array boot API to understand Unraid internal boot devices using the same runtime signal as the webgui, and expose the full boot device set to clients.
This change adds support for
type="Boot"entries fromdisks.ini, introducesBOOTas a publicArrayDiskType, and addsarray.bootDevicesso mirrored/redundant internal boot members can be represented explicitly.Why
Unraid now supports booting from storage devices instead of only USB flash devices.
Before this PR:
FLASHas bootarray.bootwas a single disk chosen fromFLASHdisks.iniwere not represented correctlyThat meant the API no longer matched the webgui/runtime behavior for internal boot.
What Changed
Boot detection and selection
type="Boot"entries fromdisks.iniin the slots parser.ArrayDiskType.BOOT.array.bootDevicesfrom:BOOTentries for internal boot/bootFLASHentry for legacy USB bootarray.bootfrombootDevicesusing the active/runtime member:/bootGraphQL contract updates
BOOTtoArrayDiskTypearray.bootDevices: [ArrayDisk!]!array.bootas deprecated but keep it supportedapiandwebParser/runtime details
fsStatusandfsMountpointon parsed disk objects as internal helper fields so boot selection can distinguish:/bootmember in mirrored internal boot setupsValidation and tests
type="Boot"slots/bootmounted memberBehavior After This PR
type: BOOTarray.bootDevicesreturns the full boot setarray.bootremains available and returns the selected active boot disk/bootflash entrySummary by CodeRabbit
New Features
Tests