feat: move onboarding wizard persistence server-side#1984
Draft
Ajit-Mehrotra wants to merge 36 commits intomainfrom
Draft
feat: move onboarding wizard persistence server-side#1984Ajit-Mehrotra wants to merge 36 commits intomainfrom
Ajit-Mehrotra wants to merge 36 commits intomainfrom
Conversation
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 a server-owned onboarding wizard: GraphQL schema/types and mutations, durable tracker draft/navigation/internal-boot state with service APIs (save/clear), backend normalization/merge logic, frontend refactor to prop-driven local wizard state (removes Pinia draft store), new boot-configuration summary component, tests and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Onboarding Modal)
participant GraphQL as GraphQL API
participant Service as OnboardingService
participant Tracker as Tracker (onboarding-tracker.json)
Client->>GraphQL: mutation SaveOnboardingDraft(input)
GraphQL->>Service: saveOnboardingDraft(input)
Service->>Tracker: saveDraft(normalizedPayload)
Tracker-->>Service: persist OK
Service-->>GraphQL: return success
GraphQL-->>Client: mutation result (true)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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)
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 |
![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: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts (1)
563-583:⚠️ Potential issue | 🟠 MajorDon't return from the auto-open close path before clearing wizard state.
This branch calls
markCompleted()and exits beforeclearWizardState(), so an incomplete onboarding session keeps its saveddraft/navigation/internalBootStateafter the user closes it. Because the new close flow also routesSAVE_FAILUREexits through this method, a transient draft-save failure can both suppress future auto-open and preserve stale wizard state for the next manual reopen.Suggested direction
-public async closeOnboarding(): Promise<void> { +public async closeOnboarding(reason?: CloseOnboardingReason): Promise<void> { const trackerStateResult = await this.onboardingTracker.getStateResult(); if (trackerStateResult.kind === 'error') { throw trackerStateResult.error; } const state = trackerStateResult.state; const currentVersion = this.onboardingTracker.getCurrentVersion(); const shouldAutoOpen = this.isVersionSupported(currentVersion) && !state.completed; if (state.forceOpen) { await this.onboardingTracker.setForceOpen(false); } - if (shouldAutoOpen) { + if (reason === CloseOnboardingReason.SAVE_FAILURE) { + await this.onboardingTracker.clearWizardState(); + return; + } + + if (shouldAutoOpen) { await this.onboardingTracker.markCompleted(); - return; } await this.onboardingTracker.clearWizardState(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts` around lines 563 - 583, The closeOnboarding method returns immediately after awaiting onboardingTracker.markCompleted(), which prevents onboardingTracker.clearWizardState() from running and leaves stale draft/navigation/internalBootState; change the flow in closeOnboarding so that when shouldAutoOpen is true you still await onboardingTracker.markCompleted() and then await onboardingTracker.clearWizardState() (i.e., remove the early return and ensure both await calls execute), keeping the existing handling of state.forceOpen (onboardingTracker.setForceOpen(false)) and error handling from getStateResult/getCurrentVersion/isVersionSupported intact.
🧹 Nitpick comments (13)
web/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vue (1)
316-322: Code duplication: reuseclearOnboardingDraftStoragefromonboardingStorageCleanup.ts.This function duplicates the logic already present in
web/src/components/Onboarding/store/onboardingStorageCleanup.ts(lines 17-32). Both remove the'onboardingDraft'key and any keys containing that substring. Import and reuse the existing helper instead of re-implementing it.♻️ Proposed refactor
import { useOnboardingModalStore } from '~/components/Onboarding/store/onboardingModalVisibility'; import { useOnboardingStore } from '~/components/Onboarding/store/onboardingStatus'; +import { clearOnboardingDraftStorage } from '~/components/Onboarding/store/onboardingStorageCleanup'; import { RegistrationState } from '~/composables/gql/graphql';Then replace the local function:
-const clearLegacyOnboardingDraftStorage = () => { - if (typeof window === 'undefined') return; - - localStorage.removeItem('onboardingDraft'); - const keysToRemove = Object.keys(localStorage).filter((key) => key.includes('onboardingDraft')); - keysToRemove.forEach((key) => localStorage.removeItem(key)); -}; +const clearLegacyOnboardingDraftStorage = clearOnboardingDraftStorage;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vue` around lines 316 - 322, The function clearLegacyOnboardingDraftStorage duplicates logic already implemented in clearOnboardingDraftStorage (in onboardingStorageCleanup.ts); replace the local implementation by importing and calling clearOnboardingDraftStorage instead. Locate the local function clearLegacyOnboardingDraftStorage in OnboardingAdminPanel.standalone.vue, remove its body/definition, add an import for clearOnboardingDraftStorage from web/src/components/Onboarding/store/onboardingStorageCleanup.ts, and invoke clearOnboardingDraftStorage wherever clearLegacyOnboardingDraftStorage was used so the shared helper is reused.web/src/components/Onboarding/components/bootConfigurationSummary/buildBootConfigurationSummaryViewModel.ts (1)
1-6: Use the repo's.jsESM suffix on this import.This new TS utility is importing another TS module without the
.jssuffix, which is out of step with the repo's ESM convention.As per coding guidelines, `TypeScript imports use `.js` extensions for ESM compatibility`.Suggested fix
import type { OnboardingBootMode, OnboardingInternalBootDevice, OnboardingInternalBootDraft, OnboardingPoolMode, -} from '@/components/Onboarding/onboardingWizardState'; +} from '@/components/Onboarding/onboardingWizardState.js';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/components/bootConfigurationSummary/buildBootConfigurationSummaryViewModel.ts` around lines 1 - 6, The import of TypeScript types (OnboardingBootMode, OnboardingInternalBootDevice, OnboardingInternalBootDraft, OnboardingPoolMode) in buildBootConfigurationSummaryViewModel.ts is missing the repo's required ESM .js suffix; update the import statement to include the .js extension (e.g., change the module specifier to end with ".js") so the file imports the same module with the .js ESM suffix while keeping the same named type imports.web/__test__/components/Onboarding/OnboardingPluginsStep.test.ts (1)
132-134: Avoid order-coupled assertions forselectedIdsunless ordering is required behavior.These checks currently require exact array ordering, which can make tests brittle if set/merge order changes while behavior stays correct. Prefer order-insensitive matching plus size checks.
As per coding guidelines: "Test component behavior and output, not implementation details."
Also applies to: 160-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/Onboarding/OnboardingPluginsStep.test.ts` around lines 132 - 134, The test currently asserts ordering by calling props.onComplete with selectedIds array literal; change it to assert order-insensitively by checking props.onComplete was called with an object whose selectedIds contains the same items regardless of order and has the expected length — e.g., replace the exact array equality check around selectedIds in the OnboardingPluginsStep.test (the expectation using props.onComplete) with an assertion that compares sets or uses expect.arrayContaining plus a length/size check so the test no longer depends on array order.api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts (1)
12-26: Prefer a wizard-state factory instead of a shared object fixture.
emptyWizardStateis reused by reference across tests. If any path mutates it, cases can bleed into each other and mask regressions. A factory (createEmptyWizardState()) keeps each test isolated.Also applies to: 59-59, 85-85, 103-103, 121-121, 139-139, 157-157, 175-175, 193-193, 211-211, 229-229, 247-247, 265-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts` around lines 12 - 26, Replace the shared mutable fixture emptyWizardState with a factory function createEmptyWizardState() that returns a fresh object copy (containing currentStepId, visibleStepIds, draft, internalBootState) so tests don’t share references; update all usages of emptyWizardState in the spec (including the other occurrences noted) to call createEmptyWizardState() before each test or within each test to ensure isolation, and remove direct imports/usages of the shared object.web/src/components/Onboarding/composables/internalBoot.ts (1)
12-12: IncludeslotCountin mutation diagnostics payload for full context.
slotCountwas added toInternalBootSelection, butbuildMutationVariables()still drops it. Including it will make error diagnostics complete when troubleshooting failed applies.♻️ Suggested diff
const buildMutationVariables = (selection: InternalBootSelection) => ({ poolName: selection.poolName, + slotCount: selection.slotCount, devices: selection.devices, bootSizeMiB: selection.bootSizeMiB, updateBios: selection.updateBios, reboot: false, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/composables/internalBoot.ts` at line 12, buildMutationVariables currently omits the new InternalBootSelection.slotCount so mutation diagnostics lack full context; update the buildMutationVariables function to include selection.slotCount in the returned variables/diagnostics payload (and any related diagnostics object) so that slotCount is passed through alongside existing fields from InternalBootSelection.web/src/components/Onboarding/store/onboardingModalVisibility.ts (1)
46-57: Use the passed close reason instead of a truthy branch.
closeModal()now accepts a reason, but every truthy value is serialized asSAVE_FAILURE. If another close reason is added later, callers can pass it and silently send the wrong enum to GraphQL.Suggested refactor
export type CloseOnboardingReason = 'SAVE_FAILURE'; +const CLOSE_ONBOARDING_REASON_MAP: Record< + CloseOnboardingReason, + GraphqlCloseOnboardingReason +> = { + SAVE_FAILURE: GraphqlCloseOnboardingReason.SAVE_FAILURE, +}; const closeModal = async (reason?: CloseOnboardingReason) => { if (!canDisplayOnboardingModal.value) { return false; } await closeOnboardingMutation({ input: reason ? { - reason: GraphqlCloseOnboardingReason.SAVE_FAILURE, + reason: CLOSE_ONBOARDING_REASON_MAP[reason], } : undefined, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/store/onboardingModalVisibility.ts` around lines 46 - 57, The current closeModal implementation always sends GraphqlCloseOnboardingReason.SAVE_FAILURE whenever a truthy reason is passed, which misreports the actual CloseOnboardingReason; update closeModal to serialize and pass the actual reason argument to closeOnboardingMutation (map CloseOnboardingReason -> GraphqlCloseOnboardingReason as needed) instead of the hardcoded SAVE_FAILURE, and keep undefined when no reason is provided so the correct enum value is sent to the GraphQL API.api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts (1)
1942-2004: Cover theplugins.selectedIdsbranch in this new draft-persistence case.
saveOnboardingDraft()also mapsdraft.plugins.selectedIds, but this test only exercisescoreSettings,internalBoot,navigation, andinternalBootState. A regression in the plugins branch would still pass here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts` around lines 1942 - 2004, The test for saveOnboardingDraft misses exercising the plugins.selectedIds mapping branch, so extend the spec in onboarding.service.spec.ts to include a plugins.selectedIds value in the input draft and assert it is persisted via onboardingTrackerMock.saveDraft; specifically, add a draft.plugins.selectedIds array (e.g., ['plugin-a','plugin-b']) to the call to service.saveOnboardingDraft and include the same field in the expected object passed to onboardingTrackerMock.saveDraft to verify saveOnboardingDraft (the mapping logic around plugins.selectedIds) is covered.web/__test__/components/Onboarding/OnboardingBootConfigurationSummary.test.ts (1)
82-149: This case doesn't actually hit the missing-label fallback path.Both fixtures still provide
deviceName, so this only verifies the normal formatted-label branch. If the raw-id fallback regresses, this test still passes. Either rename the case or omitdeviceNamefrom at least one device.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/Onboarding/OnboardingBootConfigurationSummary.test.ts` around lines 82 - 149, The test title claims to exercise the "missing-label fallback" but both devices include deviceName, so update the test to actually hit the fallback by removing deviceName from at least one created device (use createBootDevice without the deviceName arg or pass undefined for the deviceName for one entry) or alternatively rename the test to reflect that it covers the normal label branch; locate the call to buildBootConfigurationSummaryViewModel in OnboardingBootConfigurationSummary.test.ts and modify the devices array used in that test (the createBootDevice calls) so one device lacks a deviceName, ensuring buildBootConfigurationSummaryViewModel exercises the raw-id fallback path.web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue (1)
11-17: Keep the new wizard-state import in the repo’s ESM form.This local TypeScript import is missing the
.jssuffix, so it diverges from the project’s ESM import convention.Suggested fix
import { type OnboardingBootMode, type OnboardingInternalBootDevice, type OnboardingInternalBootDraft, type OnboardingInternalBootSelection, type OnboardingPoolMode, -} from '@/components/Onboarding/onboardingWizardState'; +} from '@/components/Onboarding/onboardingWizardState.js';As per coding guidelines, 'TypeScript imports use
.jsextensions for ESM compatibility'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue` around lines 11 - 17, Update the import path in OnboardingInternalBootStep.vue to use the ESM-compatible `.js` suffix for the local module; change the module specifier for the imported types (OnboardingBootMode, OnboardingInternalBootDevice, OnboardingInternalBootDraft, OnboardingInternalBootSelection, OnboardingPoolMode) from '@/components/Onboarding/onboardingWizardState' to the same path with a trailing '.js' so the TypeScript/ESM resolver uses the correct module form.web/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.ts (2)
10-10: Use the repository ESM import form here.This new local TypeScript import drops the
.jssuffix, so it no longer matches the repo’s ESM import convention.Suggested fix
-import type { OnboardingInternalBootDraft } from '~/components/Onboarding/onboardingWizardState'; +import type { OnboardingInternalBootDraft } from '~/components/Onboarding/onboardingWizardState.js';As per coding guidelines, 'TypeScript imports use
.jsextensions for ESM compatibility'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.ts` at line 10, The TypeScript import for OnboardingInternalBootDraft should follow the repository ESM convention by including the .js extension; update the import statement that references OnboardingInternalBootDraft (from '~/components/Onboarding/onboardingWizardState') to use the ESM form with the .js suffix so the module resolution matches the repo's ESM pattern.
121-135: Have the step stub emit fromprops.initialDraft.The real standalone component re-hydrates
OnboardingInternalBootStepthrough itsinitialDraftprop, but this stub always clonesconfigureDraftState.valueinstead. That means these tests can still pass even if the parent stops wiring the persisted draft back into the child.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.ts` around lines 121 - 135, The stubbed cloneDraft currently reads from configureDraftState.value; change it to read and emit from props.initialDraft instead so the tests validate that OnboardingInternalBootStep is re-hydrated via its initialDraft prop. Update the cloneDraft function to use props.initialDraft as the source (falling back to the same defaults: bootMode 'usb', skipped true) and preserve the existing defensive cloning logic for selection/devices (handling undefined vs null), then ensure the stub emits that cloned object where the test expects the component to emit initialDraft updates.web/src/components/Onboarding/onboardingWizardState.ts (1)
66-89: KeepcloneOnboardingWizardDraft()semantics-preserving.This helper rewrites
undefinedplugins.selectedIdsandinternalBoot.selection.devicesto[]. For a function namedclone, that is a surprising value change and can turn an untouched draft into an explicit empty selection during round-trips.Suggested fix
export const cloneOnboardingWizardDraft = (draft: OnboardingWizardDraft): OnboardingWizardDraft => ({ coreSettings: draft.coreSettings ? { ...draft.coreSettings } : undefined, plugins: draft.plugins ? { - selectedIds: draft.plugins.selectedIds ? [...draft.plugins.selectedIds] : [], + selectedIds: draft.plugins.selectedIds ? [...draft.plugins.selectedIds] : undefined, } : undefined, internalBoot: draft.internalBoot ? { ...draft.internalBoot, selection: draft.internalBoot.selection === undefined ? undefined : draft.internalBoot.selection === null ? null : { ...draft.internalBoot.selection, devices: draft.internalBoot.selection.devices ? draft.internalBoot.selection.devices.map((device) => ({ ...device })) - : [], + : undefined, }, } : undefined, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/onboardingWizardState.ts` around lines 66 - 89, The cloneOnboardingWizardDraft function currently converts undefined plugins.selectedIds and undefined internalBoot.selection.devices into empty arrays, changing semantics; update cloneOnboardingWizardDraft so it preserves undefined vs [] vs null exactly as in the input: for plugins.selectedIds, if draft.plugins.selectedIds is undefined keep undefined, if null keep null, otherwise shallow-copy the array; similarly for internalBoot.selection.devices, if selection is undefined keep undefined, if null keep null, otherwise if devices is undefined keep undefined, if null keep null, else shallow-copy devices; adjust references in cloneOnboardingWizardDraft (plugins.selectedIds and internalBoot.selection.devices handling) accordingly.api/generated-schema.graphql (1)
1513-1514: Consider returning the updated wizard snapshot here.This mutation persists inputs that feed server-derived
currentStepIdandvisibleStepIds, so aBooleanresponse forces the caller to refetch or navigate on stale assumptions. ReturningOnboardingWizardorOnboardingwould make the transition response the new source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/generated-schema.graphql` around lines 1513 - 1514, Change the saveOnboardingDraft mutation to return the updated onboarding snapshot instead of a Boolean: update the GraphQL schema signature for saveOnboardingDraft(input: SaveOnboardingDraftInput!): Boolean! to return the appropriate object type (e.g., OnboardingWizard or Onboarding) and update the corresponding resolver (saveOnboardingDraft resolver function) to load/derive the new currentStepId and visibleStepIds after persisting the draft and return the fully populated OnboardingWizard/Onboarding object; also update any client-facing docs and typegen generation so clients receive the new snapshot as the single source of truth.
🤖 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/generated-schema.graphql`:
- Around line 1292-1300: Update the GraphQL field descriptions for
decryptionPassword and decryptionKeyfile to document the backend unlocking
constraints: state that decryptionPassword must contain only printable‑ASCII
characters (0x20–0x7E) and that decryptionPassword and decryptionKeyfile are
mutually exclusive (clients must not send both), and keep the existing note that
decryptionKeyfile accepts a data URL or raw base64; mirror the exact validation
semantics implemented in the ArrayService/unlock resolver (see array.service.ts
unlock handling) so the schema descriptions match the backend behavior.
- Around line 1043-1047: The id fields for OnboardingWizardInternalBootDevice
and OnboardingWizardInternalBootDeviceInput are declared as String! but must use
the PrefixedID scalar to preserve Node/Disk cross-server semantics; update the
GraphQL type definitions for OnboardingWizardInternalBootDevice.id and
OnboardingWizardInternalBootDeviceInput.id to PrefixedID! (and ensure the
PrefixedID scalar is available/imported in the schema if not already) so
caching/normalization and existing Node semantics are preserved.
In `@api/src/unraid-api/config/onboarding-tracker.service.spec.ts`:
- Around line 105-141: The test must also assert that clearWizardState()
persists the new state to disk rather than only updating in-memory cache: after
calling await tracker.clearWizardState(), add an assertion that the mocked write
function was called (e.g., mockWriteFile or equivalent) with the tracker file
path and a serialized payload containing the preserved fields (completed,
completedAtVersion, forceOpen) merged with createEmptyWizardState(); locate this
in the OnboardingTrackerService test block using the tracker variable and the
clearWizardState() call and check the write invocation arguments to ensure the
durable file was rewritten.
In `@api/src/unraid-api/config/onboarding-tracker.service.ts`:
- Around line 515-571: The read/merge/write sequences in methods like saveDraft
and the reset/clear block use getStateResult, merge into updatedState and call
writeTrackerState+/syncConfig without synchronization, causing lost updates
under concurrent requests; introduce a per-service async mutex/queue (e.g., a
private AsyncMutex or Promise-queue on the service) and wrap the entire critical
section that calls getStateResult, computes nextDraft/updatedState, calls
writeTrackerState, and then syncConfig inside a single exclusive lock; apply the
same locking around any other mutating methods that touch draft, navigation,
internalBootState or call writeTrackerState to ensure serialization of
mutations.
- Around line 499-513: saveDraft() and clearWizardState() are operating on the
override-composed view (getStateResult()/onboardingOverrides.getState()) and
thus persist temporary override fields; change these mutators to read and update
the underlying persisted tracker state (the internal base state object used for
persistence, e.g., this.state or the method that returns the raw persisted
state) instead of using getStateResult()/override view, then apply overrides
only when producing the public view (getCachedState()/getStateResult()). Update
saveDraft(), clearWizardState(), and the other mutators in the same region
(around lines 539-568) to: 1) fetch the raw persisted state, 2) mutate
draft/navigation/internalBootState on that persisted state, 3) write/save that
persisted state, and 4) return the override-composed public view by calling the
existing getCachedState()/getStateResult() as before.
In `@api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts`:
- Around line 370-380: The model currently allows slotCount >2 and devices
arrays longer than the UI supports; add validation to enforce the 1–2 slot UI
constraint: on the slotCount field (symbol: slotCount) add a `@Max`(2) decorator
to complement the existing `@Min`(1), and on the devices field (symbol: devices)
add an `@ArrayMaxSize`(2) (and optionally `@ArrayMinSize`(1) if you want to require
at least one when present) alongside the existing `@ValidateNested` and `@Type`
decorators so persisted drafts cannot contain more than two devices.
In `@api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts`:
- Around line 163-174: Replace the brittle log-string assertion with an
assertion that the resolver actually forwards the reason to the service: keep
the spy on logger.warn if desired, but assert that
onboardingService.closeOnboarding was called with the
CloseOnboardingReason.SAVE_FAILURE argument (e.g., check the first call's first
parameter) when resolver.closeOnboarding({ reason:
CloseOnboardingReason.SAVE_FAILURE }) is invoked; reference the
resolver.closeOnboarding invocation and onboardingService.closeOnboarding mock
to locate the code to change.
In `@docs/onboarding-internal-boot.md`:
- Around line 36-43: The example under "selection" uses devices as string IDs
but the persisted shape is an array of objects; update the example for
selection.devices to be objects with properties id, sizeBytes, and deviceName
(matching the tracker model and summary builder expectations) so consumers see
the correct JSON contract for selection, e.g., replace the string array with
objects containing id, sizeBytes, and deviceName.
In `@web/__test__/components/Onboarding/OnboardingInternalBootStep.test.ts`:
- Around line 539-540: The test sets vm.poolMode = 'dedicated' but uses await
flushPromises() instead of awaiting Vue's DOM update; replace the
flushPromises() call with await nextTick() (import nextTick from 'vue' if not
already) so the component's computed properties and DOM have updated before
asserting the recomputed options after vm.poolMode is toggled; locate the test
block using vm.poolMode = 'dedicated' in OnboardingInternalBootStep.test and
change the synchronization barrier accordingly.
In
`@web/src/components/Onboarding/components/bootConfigurationSummary/buildBootConfigurationSummaryViewModel.ts`:
- Around line 162-169: The validation currently requires
hasFiniteNumber(selection.bootSizeMiB) for all pool modes, which incorrectly
marks dedicated drafts incomplete; update the conditional in
buildBootConfigurationSummaryViewModel so that the bootSizeMiB check is skipped
when poolMode === 'dedicated' (i.e., only require
hasFiniteNumber(selection.bootSizeMiB) for non-dedicated modes), and make the
analogous change in the second validation block around the code handling the
non-dedicated/dedicated branches (the block referenced by the reviewer for lines
196-208) so that dedicated paths do not depend on bootSizeMiB or the
bootReserved-related checks. Ensure you reference poolMode,
selection.bootSizeMiB, and the helper hasFiniteNumber when editing the
condition.
In `@web/src/components/Onboarding/ONBOARDING_WIZARD.md`:
- Around line 53-73: Update the ONBOARDING_WIZARD.md example to match the actual
tracker behavior: show the "draft" field as an empty object (e.g., "draft": {})
and remove the always-present empty sections for "coreSettings", "plugins", and
"internalBoot" so they are only present when written; reference the durable
tracker object "onboarding-tracker.json" and the keys "draft", "coreSettings",
"plugins", and "internalBoot" to make clear the web treats omitted vs
present-but-empty sections differently.
In `@web/src/components/Onboarding/OnboardingModal.vue`:
- Around line 72-74: The modal chrome (stepper and top-right close) is not
blocked by isSavingTransition, allowing race conditions during
saveOnboardingDraft/saveDraft; update the modal lock logic so the chrome checks
a combined lock (e.g., create a new computed like isModalLocked = computed(() =>
isInternalBootLocked.value || isSavingTransition.value) or extend
isInternalBootLocked to include isSavingTransition) and wire that computed into
the stepper navigation and close-button handlers (prevent clicks and
hide/disable controls when true) so no step changes or closes can occur while
isSavingTransition is true.
In
`@web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue`:
- Around line 187-209: toInternalBootSelection currently converts
draft.selection into an InternalBootSelection even when the draft has been
switched to USB/skipped; update toInternalBootSelection (and the other similar
conversion at the second occurrence) to first check draft.bootMode and
draft.skipped and return null unless bootMode === 'internal' and skipped !==
true, so that handleConfigureStepComplete() will not produce an apply payload
for internal storage when the user chose USB or skipped; locate the conversion
logic in toInternalBootSelection and the duplicate conversion and add the early
guard using draft.bootMode/draft.skipped before validating selection fields.
- Around line 61-63: The hard-coded string standaloneSummaryInvalidMessage
should be localized: replace the literal with a call to the i18n translator
(useI18n()/t) and reference a new translation key (e.g.
onboarding.standalone.summaryInvalid) used by the component where
standaloneSummaryInvalidMessage is currently defined; add corresponding entries
to the locale files for supported languages. Ensure the component imports/uses
useI18n() and that any code paths showing standaloneSummaryInvalidMessage now
call t('onboarding.standalone.summaryInvalid') instead of the English literal so
non-English users see the translated message.
In `@web/src/components/Onboarding/steps/OnboardingCoreSettingsStep.vue`:
- Around line 111-114: The watcher and initialization currently treat
props.initialDraft as an all-or-nothing source (e.g., setting serverName.value =
props.initialDraft.serverName ?? ''), which overwrites activation/API precedence
for missing fields; instead, in the props.initialDraft watcher and in
onCoreSettingsResult merge fields individually: for each field (serverName,
serverDescription, timezone, theme, language, useSsh) only assign when the draft
has that specific property (e.g., if (props.initialDraft.serverName !==
undefined) serverName.value = props.initialDraft.serverName), otherwise leave
the existing reactive value intact so activation/API values remain prioritized;
update all occurrences including the initialization block using
props.initialDraft and the blocks referenced around onCoreSettingsResult to
follow this field-by-field merge pattern.
In `@web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue`:
- Around line 714-719: When building selectedDraftDevices for the
back-navigation snapshot, preserve saved devices that no longer exist in
selectedDeviceById by falling back to the previously persisted draft devices:
iterate over devices (the sliced selectedDevices.value), and for each deviceId
use selectedDeviceById.value.get(deviceId) if present, otherwise look up the
matching entry in props.initialDraft.selection.devices and include that entry
(as an OnboardingInternalBootDevice) so handleBack() does not truncate or drop
saved metadata; update the mapping logic that produces selectedDraftDevices to
perform this fallback merge while keeping the existing type guards and slotCount
truncation.
- Around line 742-744: The current assignment uses || which treats an empty
string as falsy and overwrites a deliberately blank draftSelection.poolName;
change the logic to use nullish coalescing so only null/undefined fall back:
when setting poolName.value prefer draftSelection?.poolName if it is defined
(even if empty string) and otherwise fall back to the poolMode-based default and
then to data.poolNameDefault (using ?? for that fallback as well). Update the
expression that sets poolName.value (referencing poolName.value,
draftSelection?.poolName, poolMode.value, data.poolNameDefault) to use ??
instead of || so an intentionally blank poolName is preserved on hydrate.
In `@web/src/components/Onboarding/steps/OnboardingPluginsStep.vue`:
- Around line 73-76: buildSelectedPluginsFromDraft currently treats an explicit
empty selectedIds array as “use defaults”; change it so only a missing/undefined
draft or undefined selectedIds falls back to defaultSelectedPluginIds, while an
explicit [] becomes an empty Set (i.e., use draft?.selectedIds === undefined ?
defaultSelectedPluginIds : draft.selectedIds). Update
buildSelectedPluginsFromDraft to implement that conditional and locate any other
helper with the same pattern (the analogous conversion around lines referenced
in the review) and apply the same fix so an explicit empty selection is
preserved.
In `@web/src/components/Onboarding/steps/OnboardingSummaryStep.vue`:
- Around line 486-488: The hard-coded banner message stored in
bootConfigurationInvalidMessage bypasses vue-i18n and must be localized: replace
the constant string with a reactive localized message using the component's i18n
instance (e.g., const { t } = useI18n() and set bootConfigurationInvalidMessage
to t('onboarding.summary.bootConfigurationInvalidMessage') or call t(...) inline
where the banner is rendered), update references to
bootConfigurationInvalidMessage accordingly, and add the corresponding
translation key ("onboarding.summary.bootConfigurationInvalidMessage") to your
locale files so non-English sessions show the translated text; ensure
applyReadinessTimedOut usage remains unchanged.
---
Outside diff comments:
In `@api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts`:
- Around line 563-583: The closeOnboarding method returns immediately after
awaiting onboardingTracker.markCompleted(), which prevents
onboardingTracker.clearWizardState() from running and leaves stale
draft/navigation/internalBootState; change the flow in closeOnboarding so that
when shouldAutoOpen is true you still await onboardingTracker.markCompleted()
and then await onboardingTracker.clearWizardState() (i.e., remove the early
return and ensure both await calls execute), keeping the existing handling of
state.forceOpen (onboardingTracker.setForceOpen(false)) and error handling from
getStateResult/getCurrentVersion/isVersionSupported intact.
---
Nitpick comments:
In `@api/generated-schema.graphql`:
- Around line 1513-1514: Change the saveOnboardingDraft mutation to return the
updated onboarding snapshot instead of a Boolean: update the GraphQL schema
signature for saveOnboardingDraft(input: SaveOnboardingDraftInput!): Boolean! to
return the appropriate object type (e.g., OnboardingWizard or Onboarding) and
update the corresponding resolver (saveOnboardingDraft resolver function) to
load/derive the new currentStepId and visibleStepIds after persisting the draft
and return the fully populated OnboardingWizard/Onboarding object; also update
any client-facing docs and typegen generation so clients receive the new
snapshot as the single source of truth.
In
`@api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts`:
- Around line 12-26: Replace the shared mutable fixture emptyWizardState with a
factory function createEmptyWizardState() that returns a fresh object copy
(containing currentStepId, visibleStepIds, draft, internalBootState) so tests
don’t share references; update all usages of emptyWizardState in the spec
(including the other occurrences noted) to call createEmptyWizardState() before
each test or within each test to ensure isolation, and remove direct
imports/usages of the shared object.
In `@api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts`:
- Around line 1942-2004: The test for saveOnboardingDraft misses exercising the
plugins.selectedIds mapping branch, so extend the spec in
onboarding.service.spec.ts to include a plugins.selectedIds value in the input
draft and assert it is persisted via onboardingTrackerMock.saveDraft;
specifically, add a draft.plugins.selectedIds array (e.g.,
['plugin-a','plugin-b']) to the call to service.saveOnboardingDraft and include
the same field in the expected object passed to onboardingTrackerMock.saveDraft
to verify saveOnboardingDraft (the mapping logic around plugins.selectedIds) is
covered.
In
`@web/__test__/components/Onboarding/OnboardingBootConfigurationSummary.test.ts`:
- Around line 82-149: The test title claims to exercise the "missing-label
fallback" but both devices include deviceName, so update the test to actually
hit the fallback by removing deviceName from at least one created device (use
createBootDevice without the deviceName arg or pass undefined for the deviceName
for one entry) or alternatively rename the test to reflect that it covers the
normal label branch; locate the call to buildBootConfigurationSummaryViewModel
in OnboardingBootConfigurationSummary.test.ts and modify the devices array used
in that test (the createBootDevice calls) so one device lacks a deviceName,
ensuring buildBootConfigurationSummaryViewModel exercises the raw-id fallback
path.
In `@web/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.ts`:
- Line 10: The TypeScript import for OnboardingInternalBootDraft should follow
the repository ESM convention by including the .js extension; update the import
statement that references OnboardingInternalBootDraft (from
'~/components/Onboarding/onboardingWizardState') to use the ESM form with the
.js suffix so the module resolution matches the repo's ESM pattern.
- Around line 121-135: The stubbed cloneDraft currently reads from
configureDraftState.value; change it to read and emit from props.initialDraft
instead so the tests validate that OnboardingInternalBootStep is re-hydrated via
its initialDraft prop. Update the cloneDraft function to use props.initialDraft
as the source (falling back to the same defaults: bootMode 'usb', skipped true)
and preserve the existing defensive cloning logic for selection/devices
(handling undefined vs null), then ensure the stub emits that cloned object
where the test expects the component to emit initialDraft updates.
In `@web/__test__/components/Onboarding/OnboardingPluginsStep.test.ts`:
- Around line 132-134: The test currently asserts ordering by calling
props.onComplete with selectedIds array literal; change it to assert
order-insensitively by checking props.onComplete was called with an object whose
selectedIds contains the same items regardless of order and has the expected
length — e.g., replace the exact array equality check around selectedIds in the
OnboardingPluginsStep.test (the expectation using props.onComplete) with an
assertion that compares sets or uses expect.arrayContaining plus a length/size
check so the test no longer depends on array order.
In
`@web/src/components/Onboarding/components/bootConfigurationSummary/buildBootConfigurationSummaryViewModel.ts`:
- Around line 1-6: The import of TypeScript types (OnboardingBootMode,
OnboardingInternalBootDevice, OnboardingInternalBootDraft, OnboardingPoolMode)
in buildBootConfigurationSummaryViewModel.ts is missing the repo's required ESM
.js suffix; update the import statement to include the .js extension (e.g.,
change the module specifier to end with ".js") so the file imports the same
module with the .js ESM suffix while keeping the same named type imports.
In `@web/src/components/Onboarding/composables/internalBoot.ts`:
- Line 12: buildMutationVariables currently omits the new
InternalBootSelection.slotCount so mutation diagnostics lack full context;
update the buildMutationVariables function to include selection.slotCount in the
returned variables/diagnostics payload (and any related diagnostics object) so
that slotCount is passed through alongside existing fields from
InternalBootSelection.
In `@web/src/components/Onboarding/onboardingWizardState.ts`:
- Around line 66-89: The cloneOnboardingWizardDraft function currently converts
undefined plugins.selectedIds and undefined internalBoot.selection.devices into
empty arrays, changing semantics; update cloneOnboardingWizardDraft so it
preserves undefined vs [] vs null exactly as in the input: for
plugins.selectedIds, if draft.plugins.selectedIds is undefined keep undefined,
if null keep null, otherwise shallow-copy the array; similarly for
internalBoot.selection.devices, if selection is undefined keep undefined, if
null keep null, otherwise if devices is undefined keep undefined, if null keep
null, else shallow-copy devices; adjust references in cloneOnboardingWizardDraft
(plugins.selectedIds and internalBoot.selection.devices handling) accordingly.
In
`@web/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vue`:
- Around line 316-322: The function clearLegacyOnboardingDraftStorage duplicates
logic already implemented in clearOnboardingDraftStorage (in
onboardingStorageCleanup.ts); replace the local implementation by importing and
calling clearOnboardingDraftStorage instead. Locate the local function
clearLegacyOnboardingDraftStorage in OnboardingAdminPanel.standalone.vue, remove
its body/definition, add an import for clearOnboardingDraftStorage from
web/src/components/Onboarding/store/onboardingStorageCleanup.ts, and invoke
clearOnboardingDraftStorage wherever clearLegacyOnboardingDraftStorage was used
so the shared helper is reused.
In `@web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue`:
- Around line 11-17: Update the import path in OnboardingInternalBootStep.vue to
use the ESM-compatible `.js` suffix for the local module; change the module
specifier for the imported types (OnboardingBootMode,
OnboardingInternalBootDevice, OnboardingInternalBootDraft,
OnboardingInternalBootSelection, OnboardingPoolMode) from
'@/components/Onboarding/onboardingWizardState' to the same path with a trailing
'.js' so the TypeScript/ESM resolver uses the correct module form.
In `@web/src/components/Onboarding/store/onboardingModalVisibility.ts`:
- Around line 46-57: The current closeModal implementation always sends
GraphqlCloseOnboardingReason.SAVE_FAILURE whenever a truthy reason is passed,
which misreports the actual CloseOnboardingReason; update closeModal to
serialize and pass the actual reason argument to closeOnboardingMutation (map
CloseOnboardingReason -> GraphqlCloseOnboardingReason as needed) instead of the
hardcoded SAVE_FAILURE, and keep undefined when no reason is provided so the
correct enum value is sent to the GraphQL API.
🪄 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: 85c4cd98-aec2-4268-8c11-0fbad37d73a2
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (50)
api/generated-schema.graphqlapi/src/unraid-api/config/onboarding-tracker.model.tsapi/src/unraid-api/config/onboarding-tracker.service.spec.tsapi/src/unraid-api/config/onboarding-tracker.service.tsapi/src/unraid-api/graph/resolvers/customization/activation-code.model.tsapi/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.tsapi/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.tsapi/src/unraid-api/graph/resolvers/customization/onboarding.service.tsapi/src/unraid-api/graph/resolvers/mutation/mutation.model.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding.model.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.tsdocs/onboarding-internal-boot-port-differences.mddocs/onboarding-internal-boot.mdweb/__test__/components/Onboarding/OnboardingBootConfigurationSummary.test.tsweb/__test__/components/Onboarding/OnboardingCoreSettingsStep.test.tsweb/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.tsweb/__test__/components/Onboarding/OnboardingInternalBootStep.test.tsweb/__test__/components/Onboarding/OnboardingModal.test.tsweb/__test__/components/Onboarding/OnboardingNextStepsStep.test.tsweb/__test__/components/Onboarding/OnboardingPluginsStep.test.tsweb/__test__/components/Onboarding/OnboardingSummaryStep.test.tsweb/__test__/components/Onboarding/onboardingStorageCleanup.test.tsweb/__test__/store/onboardingContextData.test.tsweb/components.d.tsweb/src/components/Onboarding/ONBOARDING_WIZARD.mdweb/src/components/Onboarding/OnboardingModal.vueweb/src/components/Onboarding/UPGRADE_ONBOARDING.mdweb/src/components/Onboarding/components/bootConfigurationSummary/OnboardingBootConfigurationSummary.vueweb/src/components/Onboarding/components/bootConfigurationSummary/buildBootConfigurationSummaryViewModel.tsweb/src/components/Onboarding/composables/internalBoot.tsweb/src/components/Onboarding/graphql/closeOnboarding.mutation.tsweb/src/components/Onboarding/graphql/onboardingBootstrap.query.tsweb/src/components/Onboarding/graphql/saveOnboardingDraft.mutation.tsweb/src/components/Onboarding/onboardingWizardState.tsweb/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vueweb/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vueweb/src/components/Onboarding/steps/OnboardingCoreSettingsStep.vueweb/src/components/Onboarding/steps/OnboardingInternalBootStep.vueweb/src/components/Onboarding/steps/OnboardingLicenseStep.vueweb/src/components/Onboarding/steps/OnboardingNextStepsStep.vueweb/src/components/Onboarding/steps/OnboardingOverviewStep.vueweb/src/components/Onboarding/steps/OnboardingPluginsStep.vueweb/src/components/Onboarding/steps/OnboardingSummaryStep.vueweb/src/components/Onboarding/store/onboardingContextData.tsweb/src/components/Onboarding/store/onboardingDraft.tsweb/src/components/Onboarding/store/onboardingModalVisibility.tsweb/src/components/Onboarding/store/onboardingStorageCleanup.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.ts
💤 Files with no reviewable changes (4)
- docs/onboarding-internal-boot-port-differences.md
- web/src/components/Onboarding/store/onboardingStorageCleanup.ts
- web/src/components/Onboarding/UPGRADE_ONBOARDING.md
- web/src/components/Onboarding/store/onboardingDraft.ts
api/generated-schema.graphql
Outdated
Comment on lines
+1043
to
+1047
| type OnboardingWizardInternalBootDevice { | ||
| id: String! | ||
| sizeBytes: BigInt! | ||
| deviceName: String! | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 \
--iglob '*.ts' --iglob '*.tsx' --iglob '*.js' --iglob '*.jsx' --iglob '*.graphql' \
'\bOnboardingWizardInternalBootDevice(Input)?\b|\bassignableDisks\b|\bCreateInternalBootPoolInput\b|\bPrefixedID\b'
rg -n -C2 \
--iglob '*.ts' --iglob '*.tsx' --iglob '*.js' --iglob '*.jsx' \
'\bOnboardingWizardInternalBootDevice\b|\bkeyFields\b|\btypePolicies\b|\bdataIdFromObject\b'Repository: unraid/api
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read lines around 1043-1047 in the generated schema
echo "=== Lines 1043-1047 in generated-schema.graphql ==="
sed -n '1040,1050p' api/generated-schema.graphql
echo ""
echo "=== Lines 1639-1643 in generated-schema.graphql ==="
sed -n '1636,1646p' api/generated-schema.graphqlRepository: unraid/api
Length of output: 590
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find files related to onboarding and internal boot
echo "=== Files with 'internal-boot' or 'onboarding' in name ==="
find . -type f -name '*internal-boot*' -o -name '*onboarding*' | grep -E '\.(ts|js|graphql)$' | head -20
echo ""
echo "=== Search for OnboardingWizardInternalBootDevice model ==="
rg -l 'OnboardingWizardInternalBootDevice' --type ts --type tsx --type js
echo ""
echo "=== Search for assignableDisks construction ==="
rg -n 'assignableDisks' --type ts --type tsx --type js -A 5 | head -50Repository: unraid/api
Length of output: 1595
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for OnboardingWizardInternalBootDevice model definition
echo "=== Search for OnboardingWizardInternalBootDevice ==="
rg -n 'OnboardingWizardInternalBootDevice' --type ts --type js | head -30
echo ""
echo "=== Search for assignableDisks construction ==="
rg -n 'assignableDisks' --type ts --type js -A 3 | head -80Repository: unraid/api
Length of output: 25734
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the OnboardingWizardInternalBootDevice definition from the model
echo "=== OnboardingWizardInternalBootDevice model definition ==="
sed -n '438,450p' api/src/unraid-api/graph/resolvers/customization/activation-code.model.ts
echo ""
echo "=== OnboardingWizardInternalBootDeviceInput model definition ==="
sed -n '346,360p' api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts
echo ""
echo "=== Check if Disk type is used elsewhere ==="
rg -n 'class Disk' --type ts | head -5
echo ""
echo "=== Look for internal boot resolver to see where these are created ==="
rg -n 'OnboardingWizardInternalBoot' --type ts -B 2 -A 5 api/src/unraid-api/graph/resolvers/onboarding/ | head -80Repository: unraid/api
Length of output: 8587
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the Disk model to see if it uses PrefixedID
echo "=== Disk model definition ==="
sed -n '69,100p' api/src/unraid-api/graph/resolvers/disks/disks.model.ts
echo ""
echo "=== Search for where OnboardingWizardInternalBootDevice is created ==="
rg -n 'new OnboardingWizardInternalBootDevice\|OnboardingWizardInternalBootDevice\s*\{' --type ts -B 3 -A 5 | head -100
echo ""
echo "=== Check internal-boot service ==="
head -100 api/src/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.tsRepository: unraid/api
Length of output: 1145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the first 150 lines of the internal boot service
echo "=== onboarding-internal-boot.service.ts ==="
head -150 api/src/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.ts
echo ""
echo "=== Search more broadly for assignableDisks ==="
rg -n 'assignableDisks' --type ts -B 2 -A 8 api/src/unraid-api | head -150Repository: unraid/api
Length of output: 20964
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the Node interface to see what id type it uses
echo "=== Check Node interface/base class ==="
rg -n 'class Node\|interface Node' --type ts -A 10 api/src/unraid-api/graph/resolvers/disks/ | head -50
echo ""
echo "=== Check Disk id field ==="
sed -n '69,75p' api/src/unraid-api/graph/resolvers/disks/disks.model.ts
echo ""
echo "=== Search for Node definition ==="
rg -n 'export.*class Node' --type ts -A 5 | head -50
echo ""
echo "=== Check if Disk uses PrefixedID anywhere ==="
rg -n 'PrefixedID' --type ts api/src/unraid-api/graph/resolvers/disks/Repository: unraid/api
Length of output: 93
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find Node definition
echo "=== Search for Node class definition ==="
rg -n 'export.*Node' --type ts -B 2 -A 8 | grep -A 10 "class Node\|interface Node" | head -40
echo ""
echo "=== Check Disk extends Node ==="
rg -n 'extends Node' --type ts api/src/unraid-api/graph/resolvers/disks/disks.model.ts -A 15 | head -30
echo ""
echo "=== Search graphql.model for Node ==="
fd 'graphql.model' --type ts -x cat {} \; | head -100
echo ""
echo "=== Look for `@Field` id in Disk ==="
rg -n '@Field.*id' --type ts api/src/unraid-api/graph/resolvers/disks/disks.model.ts -B 1Repository: unraid/api
Length of output: 1550
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check where OnboardingWizardInternalBootDevice is instantiated
echo "=== Search for OnboardingWizardInternalBootDevice instantiation ==="
rg -n 'OnboardingWizardInternalBootDevice\s*\{|new OnboardingWizardInternalBootDevice' --type ts -B 3 -A 10 | head -100
echo ""
echo "=== Check OnboardingWizardInternalBootDraft structure ==="
rg -n 'class OnboardingWizardInternalBootDraft\|class OnboardingWizardInternalBootSelection' --type ts -A 15 api/src/unraid-api/graph/resolvers/customization/activation-code.model.ts
echo ""
echo "=== Check web side - how devices are mapped ==="
grep -n 'id.*device' web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue | head -20Repository: unraid/api
Length of output: 1492
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for where OnboardingWizardInternalBootDevice/OnboardingWizardInternalBootSelection are constructed
echo "=== Search for OnboardingWizardInternalBootSelection creation ==="
rg -n 'OnboardingWizardInternalBootSelection\|OnboardingWizardInternalBoot' --type ts api/src/unraid-api/graph/resolvers/customization/activation-code.model.ts -B 1 -A 5 | head -100
echo ""
echo "=== Search in activation-code.resolver for how devices are mapped ==="
rg -n 'devices\|Device' --type ts api/src/unraid-api/graph/resolvers/customization/activation-code.resolver.ts | head -30
echo ""
echo "=== Check activation-code service ==="
head -300 api/src/unraid-api/graph/resolvers/customization/activation-code.service.ts | tail -100Repository: unraid/api
Length of output: 119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search more broadly for where these types are constructed
echo "=== Search for 'selection.*devices' or similar patterns ==="
rg -n 'selection.*devices\|devices.*=' --type ts api/src/unraid-api/graph/resolvers/customization/ -B 2 -A 3 | head -80
echo ""
echo "=== Check activation-code resolver for onboarding ==="
grep -n 'onboarding\|wizard' --type ts -i api/src/unraid-api/graph/resolvers/customization/activation-code.resolver.ts 2>/dev/null | head -20
echo ""
echo "=== Look at activation-code.service.ts for device mapping ==="
grep -n 'devices\|Device' api/src/unraid-api/graph/resolvers/customization/activation-code.service.ts | head -30Repository: unraid/api
Length of output: 113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for where disk objects are mapped to OnboardingWizardInternalBootDevice
echo "=== Search all files for OnboardingWizardInternalBootSelection instantiation ==="
rg -n 'devices:' --type ts api/src/unraid-api/graph/resolvers/customization/activation-code.service.ts -B 5 -A 5
echo ""
echo "=== Look at entire activation-code.service.ts file size and content ==="
wc -l api/src/unraid-api/graph/resolvers/customization/activation-code.service.ts
echo ""
echo "=== Read relevant parts of activation-code.service.ts ==="
sed -n '1,100p' api/src/unraid-api/graph/resolvers/customization/activation-code.service.tsRepository: unraid/api
Length of output: 267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find files related to activation code
echo "=== Find activation-code files ==="
find api/src -name '*activation*' -type f
echo ""
echo "=== Search all TypeScript files for where device objects become OnboardingWizardInternalBootDevice ==="
rg -n 'devices\s*=\|devices:\s*\[' --type ts api/src/unraid-api/graph/resolvers/ -B 3 -A 8 | head -150
echo ""
echo "=== More specific: search for pattern where Disk is mapped ==="
rg -n 'map.*device\|map.*Disk' --type ts api/src/unraid-api/graph/resolvers/customization/ -B 2 -A 5 | head -100Repository: unraid/api
Length of output: 340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look at activation-steps.util.ts
echo "=== Read activation-steps.util.ts ==="
cat api/src/unraid-api/graph/resolvers/customization/activation-steps.util.ts
echo ""
echo "=== Search for where OnboardingWizardInternalBootDevice is created ==="
rg -n 'OnboardingWizardInternalBootDevice\|OnboardingWizardInternalBootSelection' --type ts -B 5 -A 8 api/src/unraid-api/Repository: unraid/api
Length of output: 2983
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search more carefully for OnboardingWizardInternalBootDevice
echo "=== Find all mentions of OnboardingWizardInternalBootDevice ==="
rg 'OnboardingWizardInternalBootDevice' --type ts api/src/unraid-api
echo ""
echo "=== Search for OnboardingWizardInternalBootSelection ==="
rg 'OnboardingWizardInternalBootSelection' --type ts api/src/unraid-api -B 3 -A 10Repository: unraid/api
Length of output: 10468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Now let's check how the OnboardingWizardInternalBootDeviceInput id is used
echo "=== Check CreateInternalBootPoolInput usage ==="
rg -n 'CreateInternalBootPoolInput' --type ts api/src/unraid-api -B 2 -A 10 | head -60
echo ""
echo "=== Check if there's any resolver that maps between Disk and OnboardingWizardInternalBootDevice ==="
rg -n 'OnboardingWizardInternalBootDevice\|createInternalBootPool' --type ts api/src/unraid-api/graph/resolvers/onboarding/ -B 3 -A 8 | head -100Repository: unraid/api
Length of output: 6631
Use PrefixedID for the id field in both types.
These types expose Disk.id, which is defined as PrefixedID in the base Node class and carries cross-server semantics. The id fields in OnboardingWizardInternalBootDevice (lines 1043–1047) and OnboardingWizardInternalBootDeviceInput (lines 1639–1643) should use PrefixedID instead of String! to preserve that semantics and enable proper GraphQL id caching/normalization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/generated-schema.graphql` around lines 1043 - 1047, The id fields for
OnboardingWizardInternalBootDevice and OnboardingWizardInternalBootDeviceInput
are declared as String! but must use the PrefixedID scalar to preserve Node/Disk
cross-server semantics; update the GraphQL type definitions for
OnboardingWizardInternalBootDevice.id and
OnboardingWizardInternalBootDeviceInput.id to PrefixedID! (and ensure the
PrefixedID scalar is available/imported in the schema if not already) so
caching/normalization and existing Node semantics are preserved.
Comment on lines
+1292
to
+1300
| """ | ||
| Optional password used to unlock encrypted array disks when starting the array | ||
| """ | ||
| decryptionPassword: String | ||
|
|
||
| """ | ||
| Optional keyfile contents used to unlock encrypted array disks when starting the array. Accepts a data URL or raw base64 payload. | ||
| """ | ||
| decryptionKeyfile: String |
Contributor
There was a problem hiding this comment.
Document the unlock-input constraints in the schema.
The backend already rejects requests that send both fields together and only accepts printable-ASCII passwords, but neither rule is exposed here. Please reflect those constraints in the source field descriptions so clients do not discover them only at runtime. See api/src/unraid-api/graph/resolvers/array/array.service.ts:34-106 and api/src/unraid-api/graph/resolvers/array/array.service.ts:129-180.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/generated-schema.graphql` around lines 1292 - 1300, Update the GraphQL
field descriptions for decryptionPassword and decryptionKeyfile to document the
backend unlocking constraints: state that decryptionPassword must contain only
printable‑ASCII characters (0x20–0x7E) and that decryptionPassword and
decryptionKeyfile are mutually exclusive (clients must not send both), and keep
the existing note that decryptionKeyfile accepts a data URL or raw base64;
mirror the exact validation semantics implemented in the ArrayService/unlock
resolver (see array.service.ts unlock handling) so the schema descriptions match
the backend behavior.
Comment on lines
+515
to
+571
| const currentStateResult = await this.getStateResult(); | ||
| if (currentStateResult.kind === 'error') { | ||
| throw currentStateResult.error; | ||
| } | ||
|
|
||
| const currentState = currentStateResult.state; | ||
| const updatedState: TrackerState = { | ||
| completed: currentState.completed, | ||
| completedAtVersion: currentState.completedAtVersion, | ||
| forceOpen: currentState.forceOpen, | ||
| draft: {}, | ||
| navigation: {}, | ||
| internalBootState: { | ||
| applyAttempted: false, | ||
| applySucceeded: false, | ||
| }, | ||
| }; | ||
|
|
||
| await this.writeTrackerState(updatedState); | ||
| this.syncConfig(); | ||
|
|
||
| return this.getCachedState(); | ||
| } | ||
|
|
||
| async saveDraft(input: SaveOnboardingDraftInput): Promise<PublicTrackerState> { | ||
| const currentStateResult = await this.getStateResult(); | ||
| if (currentStateResult.kind === 'error') { | ||
| throw currentStateResult.error; | ||
| } | ||
|
|
||
| const currentState = currentStateResult.state; | ||
| const nextDraft: OnboardingDraft = { | ||
| ...currentState.draft, | ||
| ...(input.draft ?? {}), | ||
| }; | ||
|
|
||
| const updatedState: TrackerState = { | ||
| completed: currentState.completed, | ||
| completedAtVersion: currentState.completedAtVersion, | ||
| forceOpen: currentState.forceOpen, | ||
| draft: nextDraft, | ||
| navigation: input.navigation | ||
| ? { | ||
| ...currentState.navigation, | ||
| ...input.navigation, | ||
| } | ||
| : currentState.navigation, | ||
| internalBootState: input.internalBootState | ||
| ? { | ||
| ...currentState.internalBootState, | ||
| ...input.internalBootState, | ||
| } | ||
| : currentState.internalBootState, | ||
| }; | ||
|
|
||
| await this.writeTrackerState(updatedState); | ||
| this.syncConfig(); |
Contributor
There was a problem hiding this comment.
Serialize the tracker's read/merge/write mutations.
These methods do a full read/merge/write of onboarding-tracker.json with no lock. Two overlapping requests will read the same base state and the last write wins, dropping one request's draft, navigation, or internalBootState update even though each individual file replacement is atomic. Please guard the tracker's mutating paths behind a per-service async mutex/queue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/src/unraid-api/config/onboarding-tracker.service.ts` around lines 515 -
571, The read/merge/write sequences in methods like saveDraft and the
reset/clear block use getStateResult, merge into updatedState and call
writeTrackerState+/syncConfig without synchronization, causing lost updates
under concurrent requests; introduce a per-service async mutex/queue (e.g., a
private AsyncMutex or Promise-queue on the service) and wrap the entire critical
section that calls getStateResult, computes nextDraft/updatedState, calls
writeTrackerState, and then syncConfig inside a single exclusive lock; apply the
same locking around any other mutating methods that touch draft, navigation,
internalBootState or call writeTrackerState to ensure serialization of
mutations.
Comment on lines
+714
to
+719
| const devices = selectedDevices.value | ||
| .slice(0, slotCount.value) | ||
| .filter((device): device is string => typeof device === 'string' && device.length > 0); | ||
| const selectedDraftDevices = devices | ||
| .map((deviceId) => selectedDeviceById.value.get(deviceId)) | ||
| .filter((device): device is OnboardingInternalBootDevice => device !== undefined); |
Contributor
There was a problem hiding this comment.
Don’t drop saved devices when building the back-navigation snapshot.
selectedDevices is restored from props.initialDraft.selection.devices, but selectedDraftDevices is rebuilt only from the current eligible-device lookup. If one of those disks is no longer in selectedDeviceById, handleBack() silently persists a shorter devices array and loses the previously saved metadata without any user edit.
Suggested fix
const devices = selectedDevices.value
.slice(0, slotCount.value)
.filter((device): device is string => typeof device === 'string' && device.length > 0);
+ const persistedDevicesById = new Map<string, OnboardingInternalBootDevice>();
+ for (const device of props.initialDraft?.selection?.devices ?? []) {
+ persistedDevicesById.set(device.id, device);
+ }
const selectedDraftDevices = devices
- .map((deviceId) => selectedDeviceById.value.get(deviceId))
+ .map((deviceId) => selectedDeviceById.value.get(deviceId) ?? persistedDevicesById.get(deviceId))
.filter((device): device is OnboardingInternalBootDevice => device !== undefined);📝 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 devices = selectedDevices.value | |
| .slice(0, slotCount.value) | |
| .filter((device): device is string => typeof device === 'string' && device.length > 0); | |
| const selectedDraftDevices = devices | |
| .map((deviceId) => selectedDeviceById.value.get(deviceId)) | |
| .filter((device): device is OnboardingInternalBootDevice => device !== undefined); | |
| const devices = selectedDevices.value | |
| .slice(0, slotCount.value) | |
| .filter((device): device is string => typeof device === 'string' && device.length > 0); | |
| const persistedDevicesById = new Map<string, OnboardingInternalBootDevice>(); | |
| for (const device of props.initialDraft?.selection?.devices ?? []) { | |
| persistedDevicesById.set(device.id, device); | |
| } | |
| const selectedDraftDevices = devices | |
| .map((deviceId) => selectedDeviceById.value.get(deviceId) ?? persistedDevicesById.get(deviceId)) | |
| .filter((device): device is OnboardingInternalBootDevice => device !== undefined); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue` around
lines 714 - 719, When building selectedDraftDevices for the back-navigation
snapshot, preserve saved devices that no longer exist in selectedDeviceById by
falling back to the previously persisted draft devices: iterate over devices
(the sliced selectedDevices.value), and for each deviceId use
selectedDeviceById.value.get(deviceId) if present, otherwise look up the
matching entry in props.initialDraft.selection.devices and include that entry
(as an OnboardingInternalBootDevice) so handleBack() does not truncate or drop
saved metadata; update the mapping logic that produces selectedDraftDevices to
perform this fallback merge while keeping the existing type guards and slotCount
truncation.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
api/src/unraid-api/config/onboarding-tracker.service.ts (2)
515-533:⚠️ Potential issue | 🟠 MajorSerialize the tracker’s read/merge/write mutations.
These paths still do a full read/merge/write with no lock. Two overlapping requests can read the same base state and the last write wins, dropping
draft,navigation, orinternalBootStateupdates. Please guard allwriteTrackerState()mutators behind a per-service async mutex/queue.Also applies to: 539-575
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/config/onboarding-tracker.service.ts` around lines 515 - 533, The read/merge/write sequence using getStateResult and writeTrackerState is not serialized, allowing concurrent requests to clobber draft/navigation/internalBootState; protect all mutating flows (the block that reads currentStateResult, builds updatedState, and calls writeTrackerState, and the similar flow later around the other writeTrackerState calls) with a per-service async mutex/queue so only one mutation runs at a time. Implement a single instance mutex (e.g., an AsyncLock or promise-queue) on the OnboardingTrackerService and wrap the getStateResult → build updatedState → writeTrackerState sequence (including the other similar sequence referenced) inside the mutex/queue critical section to ensure serialized reads and writes.
499-513:⚠️ Potential issue | 🟠 MajorMutate the persisted base state, not the override-composed view.
saveDraft()still buildsupdatedStatefromgetStateResult(), so override-onlycompleted/forceOpenvalues can be written intoonboarding-tracker.json.clearWizardState()has the opposite problem in the override branch: it only clearsthis.statein memory and leaves the file untouched. Both methods should read/write the persisted tracker state and apply overrides only when returning the public view.Also applies to: 539-555
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/config/onboarding-tracker.service.ts` around lines 499 - 513, clearWizardState and saveDraft must mutate and persist the underlying stored tracker state (the base state written to onboarding-tracker.json) rather than only modifying the in-memory override-composed view; update both methods so they: load the persisted base state (use the same routine getStateResult()/persist routine used elsewhere), apply the changes to that base state (clear draft/navigation/internalBootState in clearWizardState; update draft in saveDraft), write the updated base state back to disk, and then return the public view by composing that persisted state with onboardingOverrides.getState() (as getCachedState() currently does).
🤖 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/config/onboarding-tracker.service.ts`:
- Around line 312-316: The current merge of baseState and overrideState in
onboarding-tracker.service.ts overwrites completed and completedAtVersion with
hardcoded defaults when overrideState.onboarding omits them; update the merge so
completed and completedAtVersion fall back to baseState values instead of
false/undefined (e.g., use overrideState.onboarding.completed ??
baseState.onboarding.completed and overrideState.onboarding.completedAtVersion
?? baseState.onboarding.completedAtVersion) while still allowing forceOpen to be
overridden by overrideState.onboarding.forceOpen.
- Around line 154-163: The returned object currently clamps slotCount using
parsedSlotCount but leaves devices untouched via
normalizeBootDeviceArray(candidate.devices), which can produce inconsistent
state (e.g., slotCount: 2 with >2 devices); compute the effective slotCount
first (using parsedSlotCount and the same Math.max/Math.min logic), then call
normalizeBootDeviceArray(candidate.devices) and truncate/slice the resulting
devices array to the effective slotCount before returning it so devices.length
never exceeds the clamped slotCount; reference parsedSlotCount, slotCount,
devices, and normalizeBootDeviceArray in the onboarding-tracker.service.ts
return block to implement this change.
In `@web/__test__/components/Onboarding/onboardingWizardState.test.ts`:
- Line 3: The import of normalizeOnboardingWizardDraft lacks the ".js" extension
which breaks ESM; update the import statement that references
normalizeOnboardingWizardDraft to include the .js extension (e.g., change the
module specifier to end with "/onboardingWizardState.js") so the TypeScript test
resolves to the compiled JavaScript output.
In `@web/src/components/Onboarding/onboardingWizardState.ts`:
- Around line 124-132: The normalization currently accepts any finite numbers
for slotCount and bootSizeMiB; change the checks around parsedSlotCount and
parsedBootSizeMiB (the variables derived from candidate.slotCount and
candidate.bootSizeMiB) to only allow positive integers by validating
Number.isInteger(parsedSlotCount) && parsedSlotCount > 0 and similarly for
parsedBootSizeMiB; if the validation fails, return undefined for those fields
(leave poolName and devices handling via normalizeString and
normalizeInternalBootDevices unchanged).
- Around line 78-79: normalizeStringArray and the other normalization helpers
currently convert missing or malformed inputs into explicit empty values (e.g.,
[] or null), which loses the distinction between "omitted" and "explicitly
empty"; change them to preserve omitted/missing state by returning undefined
when the input is undefined, null, or not the expected type, and only return an
array (possibly empty) when the original value was actually an Array. Update
normalizeStringArray and the comparable helpers for selectedIds, devices, and
internalBoot.selection so they check for Array/isExpectedType first, return the
filtered array if present, and otherwise return undefined (not [] or null),
ensuring a normalize/clone round-trip does not manufacture values that will be
persisted later.
---
Duplicate comments:
In `@api/src/unraid-api/config/onboarding-tracker.service.ts`:
- Around line 515-533: The read/merge/write sequence using getStateResult and
writeTrackerState is not serialized, allowing concurrent requests to clobber
draft/navigation/internalBootState; protect all mutating flows (the block that
reads currentStateResult, builds updatedState, and calls writeTrackerState, and
the similar flow later around the other writeTrackerState calls) with a
per-service async mutex/queue so only one mutation runs at a time. Implement a
single instance mutex (e.g., an AsyncLock or promise-queue) on the
OnboardingTrackerService and wrap the getStateResult → build updatedState →
writeTrackerState sequence (including the other similar sequence referenced)
inside the mutex/queue critical section to ensure serialized reads and writes.
- Around line 499-513: clearWizardState and saveDraft must mutate and persist
the underlying stored tracker state (the base state written to
onboarding-tracker.json) rather than only modifying the in-memory
override-composed view; update both methods so they: load the persisted base
state (use the same routine getStateResult()/persist routine used elsewhere),
apply the changes to that base state (clear draft/navigation/internalBootState
in clearWizardState; update draft in saveDraft), write the updated base state
back to disk, and then return the public view by composing that persisted state
with onboardingOverrides.getState() (as getCachedState() currently does).
🪄 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: 35f93c1b-57ab-4062-9142-8f9105d7657b
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (14)
api/generated-schema.graphqlapi/src/unraid-api/config/onboarding-tracker.service.spec.tsapi/src/unraid-api/config/onboarding-tracker.service.tsapi/src/unraid-api/graph/resolvers/customization/activation-code.model.tsapi/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.tsapi/src/unraid-api/graph/resolvers/customization/onboarding.service.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding.model.tsweb/__test__/components/Onboarding/OnboardingModal.test.tsweb/__test__/components/Onboarding/onboardingWizardState.test.tsweb/src/components/Onboarding/OnboardingModal.vueweb/src/components/Onboarding/graphql/onboardingBootstrap.query.tsweb/src/components/Onboarding/onboardingWizardState.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/components/Onboarding/graphql/onboardingBootstrap.query.ts
- api/src/unraid-api/config/onboarding-tracker.service.spec.ts
| @@ -0,0 +1,63 @@ | |||
| import { describe, expect, it } from 'vitest'; | |||
|
|
|||
| import { normalizeOnboardingWizardDraft } from '~/components/Onboarding/onboardingWizardState'; | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the test file exists and read the import statement
cat -n web/__test__/components/Onboarding/onboardingWizardState.test.ts | head -20Repository: unraid/api
Length of output: 893
🏁 Script executed:
# Check if the source file exists
find . -name "onboardingWizardState.ts" -o -name "onboardingWizardState.js"Repository: unraid/api
Length of output: 110
🏁 Script executed:
# Check other test files to see how they import TypeScript modules
rg "from.*components.*onboarding" --max-count=10 -iRepository: unraid/api
Length of output: 22737
🏁 Script executed:
# Check other imports in test files that import from components folder
rg "from '~/" web/__test__ -A0 | head -30Repository: unraid/api
Length of output: 2181
🏁 Script executed:
# Check for ESLint or import-related linting configuration
find . -name ".eslintrc*" -o -name "eslint.config.*" -o -name "nuxt.config.*" | head -5Repository: unraid/api
Length of output: 125
🏁 Script executed:
# Look for tsconfig or build configuration
find . -name "tsconfig.json" -o -name "vite.config.*" | head -5Repository: unraid/api
Length of output: 248
🏁 Script executed:
# Check if there are any other test files that import from onboardingWizardState
rg "from.*onboardingWizardState" -B0 -A0Repository: unraid/api
Length of output: 1962
🏁 Script executed:
# Check the ESLint configuration for import rules
cat -n ./web/eslint.config.mjs | head -100Repository: unraid/api
Length of output: 3306
🏁 Script executed:
# Check the Nuxt config to understand module resolution
cat -n ./web/nuxt.config.ts | head -100Repository: unraid/api
Length of output: 106
🏁 Script executed:
# Find nuxt config with different extensions
find . -maxdepth 2 -name "nuxt.config.*" 2>/dev/nullRepository: unraid/api
Length of output: 36
🏁 Script executed:
# Check the actual pattern of .js imports more carefully
# Look for files that DO use .js extension in imports
rg "from '[^']*\.js'" web/src web/__test__ --max-count=20Repository: unraid/api
Length of output: 3435
🏁 Script executed:
# Check if there's a build or package.json that indicates ESM handling
cat -n web/package.json | head -40Repository: unraid/api
Length of output: 2257
🏁 Script executed:
# Check if test files have a different import pattern - are they excluding .js?
# Look at test files that import TypeScript modules
rg "from '~/components" web/__test__ -A0 | grep -v ".js'" | head -20Repository: unraid/api
Length of output: 2615
🏁 Script executed:
# Compare: in web/src files, do they use .js when importing from components?
rg "from '~/components" web/src -A0 | head -30Repository: unraid/api
Length of output: 3989
🏁 Script executed:
# Final check: verify if this specific file has any special handling or if this is a systemic issue
# Look at the actual file structure to confirm
stat web/src/components/Onboarding/onboardingWizardState.tsRepository: unraid/api
Length of output: 98
Add .js extension to this TypeScript import for ESM compatibility.
TypeScript imports in this codebase must include the .js extension to reference the compiled JavaScript output.
Suggested fix
-import { normalizeOnboardingWizardDraft } from '~/components/Onboarding/onboardingWizardState';
+import { normalizeOnboardingWizardDraft } from '~/components/Onboarding/onboardingWizardState.js';📝 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
| import { normalizeOnboardingWizardDraft } from '~/components/Onboarding/onboardingWizardState'; | |
| import { normalizeOnboardingWizardDraft } from '~/components/Onboarding/onboardingWizardState.js'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/__test__/components/Onboarding/onboardingWizardState.test.ts` at line 3,
The import of normalizeOnboardingWizardDraft lacks the ".js" extension which
breaks ESM; update the import statement that references
normalizeOnboardingWizardDraft to include the .js extension (e.g., change the
module specifier to end with "/onboardingWizardState.js") so the TypeScript test
resolves to the compiled JavaScript output.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1984 +/- ##
==========================================
+ Coverage 52.34% 52.74% +0.39%
==========================================
Files 1033 1038 +5
Lines 71626 72694 +1068
Branches 8146 8400 +254
==========================================
+ Hits 37495 38342 +847
- Misses 34006 34226 +220
- Partials 125 126 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Purpose: make the API the source of truth for onboarding draft state, navigation state, and server-computed step visibility so resume works without browser persistence. - Before: the API only knew broad onboarding completion/open state, while the web owned draft progress and step flow in browser state. - Problem: refreshes and resumes depended on client storage, visible-step logic lived in the wrong layer, and there was no typed server contract for saving wizard progress. - Now: the API persists nested draft state in onboarding-tracker.json, computes visible steps on read, exposes a typed GraphQL contract for saving draft transitions, and logs save-failure close reasons. Files changed in this commit: - api/src/unraid-api/config/onboarding-tracker.model.ts: expanded the tracker model to include wizard step ids, nested draft sections, navigation state, and internal-boot operational state. - api/src/unraid-api/config/onboarding-tracker.service.ts: added normalization/defaulting for the new wizard shape, persisted draft/navigation/internalBootState, preserved non-draft tracker state during saves, and cleared wizard state on reset/completion. - api/src/unraid-api/graph/resolvers/customization/activation-code.model.ts: added GraphQL enums and object types for the server-owned onboarding wizard payload returned to the web. - api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts: added live visible-step computation, current-step fallback resolution, wizard payload building, and mapping from GraphQL input to tracker format. - api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts: introduced typed GraphQL inputs for nested wizard draft saves plus close-onboarding reason metadata. - api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts: added saveOnboardingDraft and allowed closeOnboarding to accept a reason so save-failure exits can be logged. - api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts: surfaced saveOnboardingDraft on the onboarding mutations type. - api/generated-schema.graphql: regenerated schema shape to include the new wizard fields, enums, inputs, and mutation. - api/src/unraid-api/cli/generated/graphql.ts: regenerated API-side GraphQL types for the new wizard schema. - api/src/unraid-api/cli/generated/index.ts: refreshed generated exports to match the new GraphQL type output.
- Purpose: make the web app consume server-owned onboarding wizard state while keeping only transient in-memory edits between step transitions. - Before: onboarding used a persisted Pinia draft store and client-owned step flow, so browser storage controlled resume behavior and stale draft keys could come back. - Problem: the client had too much ownership over wizard progress, save failures were not the durability boundary, and loading/error behavior around step transitions was inconsistent. - Now: the modal hydrates from server bootstrap data, persists only on step transitions, disables controls while a save is in flight, shows step-level save errors, and removes the legacy draft store entirely. Files changed in this commit: - web/src/components/Onboarding/OnboardingModal.vue: replaced the persisted draft-store flow with local in-memory wizard state, server hydration, save-on-transition logic, save-failure handling, and the close-on-failure escape path. - web/src/components/Onboarding/onboardingWizardState.ts: introduced shared client-side types/helpers for nested wizard draft state and internal-boot state cloning. - web/src/components/Onboarding/graphql/onboardingBootstrap.query.ts: expanded bootstrap data to include the server-owned wizard payload. - web/src/components/Onboarding/graphql/saveOnboardingDraft.mutation.ts: added the new mutation used for transition-time persistence. - web/src/components/Onboarding/graphql/closeOnboarding.mutation.ts: updated the close mutation shape so the client can send save-failure reason metadata. - web/src/components/Onboarding/store/onboardingContextData.ts: exposed the server-provided wizard payload from the shared onboarding bootstrap query. - web/src/components/Onboarding/store/onboardingModalVisibility.ts: threaded close reasons through the existing modal-close mutation without changing global Pinia persistence behavior. - web/src/components/Onboarding/store/onboardingStorageCleanup.ts: narrowed browser cleanup to best-effort deletion of legacy onboarding draft keys and the old hidden-modal session flag. - web/src/components/Onboarding/store/onboardingDraft.ts: removed the old persisted onboarding draft store entirely. - web/src/components/Onboarding/steps/OnboardingCoreSettingsStep.vue: switched to prop-driven draft hydration, returned draft snapshots on complete/back, and locked the form while transition saves run. - web/src/components/Onboarding/steps/OnboardingPluginsStep.vue: switched plugin selection to prop/callback-driven draft snapshots and surfaced transition-save loading/error state. - web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue: removed draft-store coupling, returned internal-boot draft snapshots, and respected transition-save locking for navigation. - web/src/components/Onboarding/steps/OnboardingSummaryStep.vue: consumed the server-backed nested draft/internal-boot state instead of reading from the old flat draft store. - web/src/components/Onboarding/steps/OnboardingNextStepsStep.vue: continued the flow from the new server-backed summary state without relying on the removed draft store. - web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue: moved standalone internal-boot flow state to local refs and aligned it with the new draft/internal-boot types. - web/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vue: updated legacy cleanup wording to describe deleting old browser draft keys rather than reviving the removed store model. - web/src/components/Onboarding/composables/internalBoot.ts: widened the internal-boot selection type so the new summary/standalone flows can carry slot count through the shared apply path. - web/src/composables/gql/gql.ts: regenerated GraphQL document mapping for the new onboarding operations. - web/src/composables/gql/graphql.ts: regenerated web GraphQL types/enums for the wizard payload and save/close inputs. - web/src/composables/gql/index.ts: refreshed generated GraphQL exports for the updated documents and types.
- Purpose: lock in the new server-owned onboarding behavior with direct API and web coverage. - Before: tests mainly covered the older completion/open flow and client-owned draft behavior, leaving the new save-on-transition path under-tested. - Problem: without new coverage, regressions in nested draft merges, step fallback logic, and save-failure UX would be easy to miss. - Now: the test suite verifies tracker persistence semantics, wizard visibility/current-step resolution, mutation behavior, modal save failures, and legacy-key cleanup. Files changed in this commit: - api/src/unraid-api/config/onboarding-tracker.service.spec.ts: added coverage for empty wizard defaults, nested partial draft merges, navigation persistence, and internal-boot state persistence. - api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts: added wizard-payload coverage for visible-step computation, current-step fallback, and saveOnboardingDraft mapping. - api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts: covered saveOnboardingDraft delegation and close-on-save-failure logging. - api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts: updated resolver expectations so the shared onboarding response shape includes the new wizard payload. - web/__test__/components/Onboarding/OnboardingModal.test.ts: covered server-hydrated wizard state, transition persistence, and blocked navigation with a save-failure close path. - web/__test__/components/Onboarding/OnboardingCoreSettingsStep.test.ts: updated the step tests for prop-driven draft hydration and transition-time save state. - web/__test__/components/Onboarding/OnboardingPluginsStep.test.ts: updated plugin-step expectations for callback-driven draft snapshots and disabled/loading state during saves. - web/__test__/components/Onboarding/OnboardingInternalBootStep.test.ts: aligned internal-boot step coverage with the new prop/callback draft flow. - web/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.ts: updated standalone internal-boot tests to use local draft refs instead of the removed store. - web/__test__/components/Onboarding/OnboardingSummaryStep.test.ts: updated summary expectations for nested draft/internal-boot props. - web/__test__/components/Onboarding/OnboardingNextStepsStep.test.ts: kept completion/reboot/shutdown coverage working against the new modal/data flow. - web/__test__/components/Onboarding/onboardingStorageCleanup.test.ts: verified that legacy onboarding draft keys are deleted and not recreated by the removed store. - web/__test__/store/onboardingContextData.test.ts: updated bootstrap-store coverage for the new wizard payload shape.
- Purpose: make the repository documentation match the new server-owned onboarding implementation. - Before: the docs described a client-owned draft store, hardcoded step flow, and an old internal-boot port-differences document that no longer matched the code. - Problem: anyone reading the old docs would implement against the wrong architecture and misunderstand how bootstrap, persistence, and legacy cleanup now work. - Now: the docs describe the server-owned bootstrap contract, tracker-backed wizard state, save-on-transition model, visible-step computation, internal-boot behavior, and exact legacy-key cleanup behavior. Files changed in this commit: - web/src/components/Onboarding/ONBOARDING_WIZARD.md: added the new source-of-truth onboarding architecture guide covering bootstrap fields, tracker shape, save rules, step visibility, current-step fallback, failure handling, and legacy cleanup semantics. - web/src/components/Onboarding/UPGRADE_ONBOARDING.md: removed the obsolete upgrade-era onboarding doc that described the deleted client-draft flow. - docs/onboarding-internal-boot.md: added focused documentation for how the internal-boot step now fits into the server-owned wizard model. - docs/onboarding-internal-boot-port-differences.md: removed the old port-differences note because it was documenting a previous transitional implementation rather than the current shipped architecture.
- Purpose: make transition-time saves visually obvious beyond the button spinner. - Before: the primary button showed loading and the step controls were disabled, but the rest of the step stayed visually static during the save. - Problem: users could miss that the wizard was actively persisting state, especially on Back/Skip where the button itself did not have its own spinner treatment. - Now: the active step shows a compact loading overlay while transition saves are in flight, while preserving the existing full-modal loading state for bootstrap and closing. Files changed in this commit: - web/src/components/Onboarding/OnboardingModal.vue: added a compact step-level loading overlay that appears only during transition saves and reuses the existing onboarding loading component/text.
- Purpose: Move transition-save loading feedback out of the modal shell and into the active onboarding steps so the state reads as part of the step interaction itself. - Before: On step transitions, the modal rendered a generic overlay above the current step while `saveOnboardingDraft` was in flight. Primary forward buttons already showed loading, but Back and Skip only became disabled, which made the save state feel modal-owned instead of step-owned. - Problem: The overlay obscured the step content, made Back and Skip feel less explicit, and did not match the product intent of showing loading within the step the user just interacted with. - What changed: Remove the modal-level transition overlay from `OnboardingModal.vue` and keep the modal responsible only for full-modal loading states such as bootstrap and close. Add compact inline onboarding loading blocks directly above the footer in the Overview, Core Settings, Plugins, Internal Boot, License, and Summary steps. Extend the License and Summary step props to understand `isSavingStep`, then disable their interactive controls while a transition save is running. Keep the existing button-level loading states where they already existed, so forward actions still feel responsive while Back and Skip now gain the same visual context through the inline step loading block. - How it works: The modal still owns `isSavingTransition` and passes it through as `isSavingStep`, but each step now renders its own compact `OnboardingLoadingState` when that flag is true. Because the loading block is rendered inside the active step, the user sees the save feedback in the same place as the buttons they just used. License and Summary now also treat transition saves as a busy state, which prevents conflicting interactions during the save window without reintroducing the old overlay.
- Purpose: Make transition-save loading states feel like true step replacements instead of supplemental content appended near the footer. - Before: The previous follow-up moved loading into each step, but it rendered as a compact block above the footer. That was better than the old modal overlay, yet it still visually competed with the existing step content and read like an extra message at the bottom. - Problem: Users could still perceive the step as mostly active because the main body remained visible. The loading state did not fully communicate that the current step had handed off control while the transition save was in flight. - What changed: Update the Overview, Core Settings, Plugins, Internal Boot, License, and Summary steps so `isSavingStep` swaps the normal step body for a centered `OnboardingLoadingState`. Remove the footer-level loading blocks that were added in the prior commit. Preserve the existing busy-state wiring and button disabling logic so interaction safety stays unchanged while the presentation becomes clearer. - How it works: Each affected step now gates its main card with `v-if` / `v-else`: when `isSavingStep` is true, the step renders only the centered loading state inside the step container. When the transition save completes, the normal step body returns or the wizard advances, which makes the loading state read as a full-step handoff instead of a footer status message.
- Purpose: Make every onboarding exit path clear the durable server-owned wizard draft so users do not come back to stale progress after leaving the modal. - Before: `completeOnboarding()` already cleared the tracker because completion resets wizard state, so dashboard exits and successful next-step finishes were mostly fine. The explicit modal close path only changed visibility/completion flags and could leave the saved draft, navigation step, and internal-boot operational state behind. The reboot and shutdown flows could also leave stale wizard state if `completeOnboarding()` failed before the power action continued. - Problem: Users who intentionally leave onboarding could reopen the wizard and see old draft data from a prior session even though they had already chosen to exit. That made the server-owned draft behave more like sticky abandoned state than a draft tied to an active onboarding run. - What changed: Add `OnboardingTrackerService.clearWizardState()` to clear `draft`, `navigation`, and `internalBootState` while preserving completion metadata and force-open semantics. Update `OnboardingService.closeOnboarding()` so non-completing close flows always wipe the wizard state after they finish their visibility bookkeeping. Update the final Next Steps reboot/shutdown path so if `completeOnboarding()` fails but we still proceed with the power action, the web explicitly falls back to `closeOnboarding()` first to clear the server draft. Add API and web tests covering the new clear-on-close behavior and the reboot/shutdown fallback path. - How it works: Completion exits still clear state through `markCompleted()`, which resets the tracker as part of completion. Non-completion exits now use the new tracker clear helper, which preserves `completed`, `completedAtVersion`, and `forceOpen` while zeroing the wizard-specific state. When reboot/shutdown continues after a completion error, the web now invokes the close mutation as a best-effort draft-clearing fallback before issuing the power command.
- Purpose: make the Core Settings step wait for its initial live server baseline before showing editable fields when no server draft exists. - Before: the onboarding modal bootstrap decided whether the wizard opened, but the Core Settings form could still render before GET_CORE_SETTINGS_QUERY and onboarding status finished settling. - Problem: users could briefly see browser-seeded defaults without clear context, and a failed live query had no explicit retry path or explanation of which values were winning. - Change: add a full-step loading state while the initial live baseline is still pending, add a retryable warning when the live query fails without a draft, and keep the form editable with safe defaults after the warning appears. - How it works: the step now waits on onboarding status plus GET_CORE_SETTINGS_QUERY only when there is no draft, treats saved-transition loading separately, and refetches the core settings query when the retry button is pressed. - Tests: extend the Core Settings component suite to cover the blocked-loading path, retry-on-query-failure behavior, and the updated timezone/identity precedence once onboarding status settles. - Docs: update the onboarding wizard documentation with step-local query behavior and a concrete precedence explanation for server identity and timezone selection, including fresh-setup browser-timezone preference versus returning-setup server-timezone preference.
- Purpose: remove the Core Settings live-baseline gating follow-up because it is unnecessary for this PR and broadens scope beyond the server-owned onboarding work. - Before: the follow-up commit added a new full-step loading gate for GET_CORE_SETTINGS_QUERY, a retryable warning state when that query failed without a draft, and extra documentation and tests describing that behavior. - Problem: those changes were a UX refinement on top of the onboarding rewrite rather than a required part of the server-side persistence migration, so they made the PR noisier and harder to review. - Change: restore the previous Core Settings rendering behavior, remove the added retry warning and query-blocking gate, and drop the matching doc/test additions. - How it works: the step now goes back to using only the existing transition-save loading state, while the docs return to describing the shipped server-owned wizard flow without the extra step-query guidance from that follow-up. - Verification: reran the focused Core Settings component test suite and web type-check after the revert to confirm the onboarding web package is back in a clean state.
- Purpose of the change Persist selected internal boot device metadata in the server-owned onboarding draft so every review surface can render the same boot summary without re-querying disk labels. - How behavior was before The internal boot step only saved device ids in the draft. The onboarding Summary step compensated by running a separate internal boot context query and enriching labels after mount. The standalone internal boot summary did not do that enrichment, so it showed raw ids only. - Why that was a problem The onboarding Summary step flickered from a raw serial/id label to a richer label a moment later. The two summary experiences disagreed with each other. The draft was not the real source of truth for what the user selected, which made resume and review behavior feel inconsistent. - What the new change accomplishes Selected devices are now stored as objects with id, sizeBytes, and deviceName across tracker state, GraphQL input/output, generated types, and web draft state. Both onboarding summaries and the boot-drive warning dialog now render from the saved draft immediately. The shared boot summary presentation is reused by both the full onboarding Summary step and the standalone internal boot wizard. - How it works Adds typed internal boot device models to the tracker, GraphQL schema/models, generated API and web GraphQL types, and local onboarding draft state. Updates the internal boot step to save device metadata directly from the already-fetched assignable disk context while still sending bootSizeMiB to cmdMakeBootable for emcmd operations. Extracts a shared boot summary helper and component, formats sizeBytes into the existing metric GB/TB labels, removes the Summary-step label-enrichment query path, and expands focused API and web tests to cover the new persistence and rendering flow.
- Purpose: make the standalone internal boot wizard behave like the onboarding summary flow while the boot apply is running, and make storage boot start in the expected hybrid mode by default. - Before: the standalone summary kept introducing a separate post-confirm section, reboot and shutdown actions could appear while createInternalBootPool was still in flight, and fresh storage-boot setups defaulted to dedicated mode. - Why that was a problem: the UI implied the user could power off the system before the internal boot apply finished, the standalone flow felt inconsistent with onboarding, and the default pool mode pushed users into the less common dedicated path. - What changed: the standalone summary now keeps the existing summary card in place, renders logs underneath it, hides all footer actions until the apply finishes, and only shows reboot or shutdown once the locked result state has been reached. - How it works: the standalone footer now gates locked actions on confirmationState === 'result', the summary/result content stays inside the same card, the internal boot step initializes poolMode to 'hybrid' for fresh storage drafts, and focused Vitest coverage now asserts both the in-flight action lock and the hybrid default.
- Purpose: capture the Core Settings source-of-truth rules in one place so the field defaults are understandable without re-reading the component logic. - Before: the precedence for server name, description, timezone, theme, language, and SSH had been discussed and partially documented during other changes, but the explanation was buried and not easy to find later. - Why that was a problem: the onboarding bootstrap and live Core Settings query serve different roles, and without an explicit precedence table it was easy to misread when browser values, activation metadata, draft values, or live server values should win. - What changed: ONBOARDING_WIZARD.md now has a dedicated Core Settings Precedence section that explains when bootstrap versus GET_CORE_SETTINGS_QUERY load, what each source is responsible for, and which source wins for each editable field. - How it works: the doc now mirrors the current OnboardingCoreSettingsStep behavior, including the fresh-setup versus non-fresh-setup timezone rule, the activation-versus-server identity rule, and the supporting live values used for hostname and current IP displays.
- Purpose: make the internal boot pool mode dropdown match the current product default. - Before: hybrid mode was selected by default, but the dropdown still listed dedicated first and hybrid second. - Why that was a problem: the select menu looked inconsistent and implied dedicated was still the primary path even after the default had been switched to hybrid. - What changed: the dropdown options now render hybrid first and dedicated second. - How it works: the pool mode item list in OnboardingInternalBootStep now builds the hybrid option before the dedicated option, without changing any underlying saved values or validation rules.
- Purpose: shrink the onboarding server-side migration by removing duplicated GraphQL draft schema surface while keeping the useful wizard shell fields typed. - Before: the onboarding draft shape was modeled repeatedly in GraphQL input classes, GraphQL output classes, generated schema/codegen output, and web field selections. - Why that was a problem: every draft tweak created synchronized edits across multiple layers, inflated the PR diff, and made the contract brittle to nullable/optional edge cases. - What this change accomplishes: only onboarding draft payloads now cross the GraphQL boundary as JSON, while currentStepId, visibleStepIds, navigation, and internalBootState remain explicitly typed. - How it works: the API now accepts and returns draft JSON directly, the tracker normalizes draft data on write and read, the web modal normalizes unknown JSON into typed local state, and focused regression tests cover the new JSON boundary.
- Purpose: fix the API CI failure caused by stale onboarding tracker expectations after the server-owned wizard state expansion. - Before: api-config tracker tests still expected only completed, completedAtVersion, and forceOpen in the public tracker result. - Why that was a problem: the tracker public state now also includes draft, navigation, and internalBootState, so the old deep-equality assertions failed in CI. - What this change accomplishes: the API config tests now assert the full current tracker contract, including the default empty wizard state fields. - How it works: the three onboarding tracker expectations in api-config.test.ts now include empty draft and navigation objects plus the default internal boot apply flags.
- Purpose: bring the generated API GraphQL types in sync with the onboarding draft JSON boundary changes already on the branch. - Before: the committed generated file still included the removed nested onboarding draft GraphQL types and enums. - Why that was a problem: the generated CLI GraphQL types no longer matched the current schema and resolver models on this branch. - What this change accomplishes: the generated API GraphQL output now reflects the simplified onboarding draft contract. - How it works: the stale onboarding draft input/output types and enums are removed from the generated graphql.ts file to match the current schema.
Ajit-Mehrotra
force-pushed
the
codex/server-side-onboarding-wizard
branch
from
712d594 to
a0bf3b1
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue (2)
301-363:⚠️ Potential issue | 🟠 MajorHandle thrown apply errors before leaving the dialog in
saving.This path only has a
try/finally. IfapplyInternalBootSelection()rejects,confirmationStatestays'saving'andapplyAttemptedstays true, which leaves the dialog locked with no result or recovery path.🤖 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 301 - 363, handleSummaryContinue currently uses try/finally around applyInternalBootSelection but no catch, so if applyInternalBootSelection throws the dialog remains stuck in confirmationState='saving' and applyAttempted is not properly handled. Add a catch block for the await applyInternalBootSelection(...) inside handleSummaryContinue that (1) sets internalBootState.value = { applyAttempted: true, applySucceeded: false }, (2) clears or records the error via addLog/addLogs (use the caught error to populate summaryT('logs.internalBootReturnedError') or a generic error message), (3) set resultSeverity/resultTitle/resultMessage to show an error result, and (4) set confirmationState.value = 'result' so the dialog can recover; keep the existing finally to clearInterval(progressTimer).
64-78:⚠️ Potential issue | 🟠 MajorOnly lock the standalone flow after a successful apply.
isLockedflips true as soon asapplyAttemptedis set. On a failed apply, the result screen still enters the locked branch, hides close/edit controls, and shows reboot/shutdown actions even thoughapplySucceededis false.Also applies to: 566-599
🤖 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 64 - 78, The flow is locking as soon as applyAttempted is set; change the isLocked computed (and the same logic in the duplicated block around lines 566–599) to only become true when the apply actually succeeded (e.g., depend on internalBootState.value.applySucceeded — optionally combined with applyAttempted as applyAttempted && applySucceeded) so failed applies do not hide close/edit controls or show reboot/shutdown actions; update the computed named isLocked (and any duplicate computed) accordingly and leave canReturnToConfigure, confirmationState, and resultSeverity checks unchanged.api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts (1)
505-525:⚠️ Potential issue | 🟠 MajorPreserve close reasons instead of always marking the wizard completed.
OnboardingService.closeOnboarding()still has no way to inspect the new close reason. Any incomplete session that lands in theshouldAutoOpenbranch will callmarkCompleted(), so a transient save-failure close or other cleanup-only close can suppress future auto-open instead of just clearing wizard state.
♻️ Duplicate comments (9)
api/src/unraid-api/config/onboarding-tracker.service.spec.ts (1)
105-142:⚠️ Potential issue | 🟡 MinorAssert that
clearWizardState()persists to disk.This test only verifies the returned in-memory state. Add an assertion that
mockAtomicWriteFilewas called to ensure the durable cleanup path is tested.Suggested assertion addition
await expect(tracker.clearWizardState()).resolves.toEqual({ completed: true, completedAtVersion: '7.1.0', forceOpen: true, ...createEmptyWizardState(), }); + + expect(mockAtomicWriteFile).toHaveBeenCalledTimes(1); + const writtenState = JSON.parse(String(mockAtomicWriteFile.mock.calls[0]?.[1])); + expect(writtenState).toMatchObject({ + completed: true, + completedAtVersion: '7.1.0', + forceOpen: true, + ...createEmptyWizardState(), + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/config/onboarding-tracker.service.spec.ts` around lines 105 - 142, The test currently only checks the returned in-memory value from OnboardingTrackerService.clearWizardState(); add an assertion that the persistence path was exercised by verifying mockAtomicWriteFile was called (e.g. after await tracker.clearWizardState()) and that it was invoked with the expected serialized state containing preserved fields (completed, completedAtVersion, forceOpen) plus the empty wizard payload. Reference the clearWizardState method on OnboardingTrackerService, the test's mockAtomicWriteFile spy, and the existing expected object used in the toEqual assertion when adding this check.web/src/components/Onboarding/onboardingWizardState.ts (2)
183-205:⚠️ Potential issue | 🟡 MinorClone function manufactures arrays for undefined values.
cloneOnboardingWizardDraftcreates empty arrays ([]) forselectedIds(line 187) anddevices(line 202) even when the original draft had these asundefined. This loses the distinction between "field was never set" and "field was explicitly set to empty", which could cause unintended state to be persisted on round-trip.Suggested fix
plugins: draft.plugins ? { - selectedIds: draft.plugins.selectedIds ? [...draft.plugins.selectedIds] : [], + selectedIds: draft.plugins.selectedIds ? [...draft.plugins.selectedIds] : undefined, } : undefined, ... devices: draft.internalBoot.selection.devices ? draft.internalBoot.selection.devices.map((device) => ({ ...device })) - : [], + : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/onboardingWizardState.ts` around lines 183 - 205, The cloneOnboardingWizardDraft helper currently converts undefined arrays into empty arrays; change the checks for plugins.selectedIds and internalBoot.selection.devices to explicitly test for undefined (e.g., draft.plugins.selectedIds === undefined) and return undefined in that case, otherwise return a shallow copy (e.g., [...draft.plugins.selectedIds] or mapped devices). Update the branches inside cloneOnboardingWizardDraft (symbols: cloneOnboardingWizardDraft, plugins.selectedIds, internalBoot.selection.devices, internalBoot.selection) so undefined is preserved, while null/empty arrays remain correctly cloned.
130-132:⚠️ Potential issue | 🟡 MinorRestrict
slotCountandbootSizeMiBto positive integers.The current validation accepts any finite number, including negative values and fractions. These should be constrained to positive integers to prevent invalid draft data from being normalized and persisted.
- slotCount: Number.isFinite(parsedSlotCount) ? parsedSlotCount : undefined, + slotCount: + Number.isInteger(parsedSlotCount) && parsedSlotCount > 0 ? parsedSlotCount : undefined, devices: normalizeInternalBootDevices(candidate.devices), - bootSizeMiB: Number.isFinite(parsedBootSizeMiB) ? parsedBootSizeMiB : undefined, + bootSizeMiB: + Number.isInteger(parsedBootSizeMiB) && parsedBootSizeMiB > 0 ? parsedBootSizeMiB : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/onboardingWizardState.ts` around lines 130 - 132, The slotCount and bootSizeMiB assignments currently accept any finite number; change their validation to only allow positive integers by replacing Number.isFinite checks with Number.isInteger(parsedSlotCount) && parsedSlotCount > 0 and Number.isInteger(parsedBootSizeMiB) && parsedBootSizeMiB > 0 respectively (keep the fallback to undefined), so slotCount and bootSizeMiB are only set when parsedSlotCount/parsedBootSizeMiB are positive whole numbers.web/src/components/Onboarding/components/bootConfigurationSummary/buildBootConfigurationSummaryViewModel.ts (1)
162-171:⚠️ Potential issue | 🟠 MajorDon't require
bootSizeMiBfor dedicated pool mode.The validation at line 166 requires
bootSizeMiBfor all storage selections, butbootReservedis only used for non-dedicated pools (lines 196-208). A dedicated pool draft withoutbootSizeMiBwould incorrectly returnINCOMPLETE_STORAGE_SELECTION.Suggested fix
if ( !isKnownPoolMode(poolMode) || !hasPositiveInteger(selection.slotCount) || devices.length === 0 || - !hasFiniteNumber(selection.bootSizeMiB) || typeof selection.updateBios !== 'boolean' || - (poolMode === 'hybrid' && !hasNonEmptyString(selection.poolName)) + (poolMode === 'hybrid' && + (!hasNonEmptyString(selection.poolName) || !hasFiniteNumber(selection.bootSizeMiB))) ) { return { kind: 'invalid', reason: 'INCOMPLETE_STORAGE_SELECTION' }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/components/bootConfigurationSummary/buildBootConfigurationSummaryViewModel.ts` around lines 162 - 171, The validation currently requires selection.bootSizeMiB for all pool modes causing dedicated pools to be marked INCOMPLETE; update the condition in buildBootConfigurationSummaryViewModel (the block using isKnownPoolMode, hasPositiveInteger(selection.slotCount), hasFiniteNumber(selection.bootSizeMiB), and poolMode checks) so that hasFiniteNumber(selection.bootSizeMiB) is only enforced when the poolMode is not 'dedicated' (i.e., require bootSizeMiB for non-dedicated/hybrid flows where bootReserved is used, but allow missing bootSizeMiB for poolMode === 'dedicated').web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue (2)
741-744:⚠️ Potential issue | 🟠 MajorKeep an intentionally blank pool name on hydrate.
||treats''as missing here, so a saved hybrid draft with a blankpoolNameis replaced with the default and then written back as if the user changed it. Use nullish coalescing for this fallback chain instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue` around lines 741 - 744, The hydrate logic for poolName uses || which treats an intentionally empty string as missing and overwrites saved blank values; update the fallback chain in the assignment to use nullish coalescing (??) so draftSelection?.poolName is only replaced when null or undefined (refer to poolName.value and draftSelection, and keep poolMode.value logic intact), and still fall back to data.poolNameDefault when appropriate.
713-719:⚠️ Potential issue | 🟠 MajorPreserve saved devices when rebuilding the back snapshot.
selectedDraftDevicesis rebuilt only fromselectedDeviceById, so a previously saved disk that no longer appears in the latest eligibility response gets dropped fromselection.deviceson Back. That silently loses persisted device metadata without any user edit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue` around lines 713 - 719, selectedDraftDevices is currently rebuilt only from selectedDeviceById which drops previously saved devices that no longer appear in the latest eligibility response; change the rebuild logic in the block that computes selectedDraftDevices to merge existing persisted devices from selection.devices with the devices derived from selectedDeviceById (keep the devices from selection.devices that are missing from selectedDeviceById by id), so saved device metadata is preserved when reconstructing selectedDraftDevices (refer to symbols: selectedDraftDevices, selectedDeviceById, selectedDevices, slotCount, and selection.devices).web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue (1)
187-209:⚠️ Potential issue | 🟠 MajorIgnore stale storage selections when the draft is USB/skipped.
toInternalBootSelection()will still build an apply payload wheneverdraft.selectionis present. If a stale selection survives after the user switches back to USB, this flow can still apply the old disks.🤖 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 187 - 209, toInternalBootSelection currently builds a payload whenever draft.selection exists, which can re-apply stale disks if the user switched to USB or skipped storage; update toInternalBootSelection to early-return null when the draft indicates USB/skipped storage (e.g. if draft.bootSource === 'usb' or draft.skipStorage === true) before validating or mapping selection, keeping the rest of the validation logic intact (function name: toInternalBootSelection, local variable: selection, mapping: devices.map).web/src/components/Onboarding/steps/OnboardingCoreSettingsStep.vue (1)
62-69:⚠️ Potential issue | 🟠 MajorMerge
initialDraftfield-by-field instead of treating it as complete.A truthy
{}or partially populated draft now bypasses the activation/API/browser precedence and backfills missing fields with defaults like'','white','en_US', or the browser timezone. The next save can then overwrite live settings that the draft never owned.Also applies to: 110-114, 190-198, 227-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/steps/OnboardingCoreSettingsStep.vue` around lines 62 - 69, The current code treats props.initialDraft as a complete override which can inadvertently shadow activation/API/browser precedence when initialDraft is an empty object or missing fields; update each initialization (e.g., selectedTimeZone, serverName, serverDescription, selectedTheme, selectedLanguage, useSsh and the other occurrences at the referenced blocks) to merge field-by-field by checking whether the draft explicitly provides that property (use hasOwnProperty or the 'in' operator) and only use draft[field] when present, otherwise fall back to the existing precedence/defaults (e.g., resolveInitialTimeZone() or TRUSTED_DEFAULT_PROFILE.*); apply this pattern to all similar initializations so missing fields in initialDraft do not overwrite computed/default values.api/src/unraid-api/config/onboarding-tracker.service.ts (1)
539-578:⚠️ Potential issue | 🟠 MajorSerialize tracker mutations to prevent lost updates.
saveDraft()performs a read-merge-write without any locking. Two concurrent requests will read the same base state and the last write wins, silently dropping one request'sdraft,navigation, orinternalBootStateupdate. Consider guarding mutations with an async mutex.Example using async-mutex
import { Mutex } from 'async-mutex'; // In the class: private readonly writeMutex = new Mutex(); async saveDraft(input: SaveOnboardingDraftInput): Promise<PublicTrackerState> { return this.writeMutex.runExclusive(async () => { const currentStateResult = await this.getStateResult(); // ...rest of existing logic }); }Apply the same pattern to
clearWizardState()and other mutating methods.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/config/onboarding-tracker.service.ts` around lines 539 - 578, saveDraft currently does an unsafe read-merge-write (getStateResult -> writeTrackerState) and can lose updates under concurrency; add an async mutex (e.g., a class field writeMutex from async-mutex) and wrap the entire mutation sequence in writeMutex.runExclusive so the read (getStateResult), merge, write (writeTrackerState) and syncConfig happen atomically; apply the same pattern to clearWizardState and any other methods that call writeTrackerState or mutate the tracker state to ensure serialized writes while still using normalizeDraft/normalizeNavigation/normalizeInternalBootState as before.
🧹 Nitpick comments (3)
api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts (1)
21-25: Remove unused imports.
OnboardingWizardBootModeandOnboardingWizardPoolModeare imported but not used in this file. This was flagged by static analysis.import type { OnboardingDraft } from '@app/unraid-api/config/onboarding-tracker.model.js'; -import { - OnboardingWizardBootMode, - OnboardingWizardPoolMode, - OnboardingWizardStepId, -} from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js'; +import { OnboardingWizardStepId } from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts` around lines 21 - 25, Remove the unused imports OnboardingWizardBootMode and OnboardingWizardPoolMode from the import statement in onboarding.model.ts; keep only the actually used symbol OnboardingWizardStepId (imported from `@app/unraid-api/graph/resolvers/customization/activation-code.model.js`) so the file no longer imports unused identifiers flagged by static analysis.web/src/components/Onboarding/OnboardingModal.vue (1)
352-357: Consider more specific error messaging for save failures.The error message uses
t('onboarding.nextSteps.completionFailed')as a fallback, which may not accurately describe a draft-save failure (vs. completion failure). A dedicated i18n key likeonboarding.draft.saveFailedwould be clearer.🤖 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 352 - 357, The catch block in OnboardingModal.vue sets saveTransitionError using t('onboarding.nextSteps.completionFailed'), which is misleading for a draft-save failure; update the catch to use a dedicated i18n key such as t('onboarding.draft.saveFailed') with an appropriate fallback message ("We couldn't save your onboarding draft right now."), and ensure the i18n key is added to the translations; references: saveTransitionError, the catch block around the save/draft save logic, and t('onboarding.nextSteps.completionFailed').api/src/unraid-api/config/onboarding-tracker.service.ts (1)
499-513:clearWizardStatein override branch only clears in-memory state.When an override is active, this method updates
this.statebut never persists to disk, while the non-override branch does persist. This asymmetry may be intentional for testing overrides, but could lead to confusion if callers expect the clear to survive a restart.Consider adding a comment documenting this intentional behavior, or aligning both branches to persist when appropriate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/config/onboarding-tracker.service.ts` around lines 499 - 513, The override branch in clearWizardState (which checks onboardingOverrides.getState()) only updates this.state and returns getCachedState() without persisting to disk; either add a clear comment above that branch documenting that this is intentional (in-memory-only for overrides) or modify the branch to call the same persistence logic used in the non-override branch (mirror the disk-write/save function used elsewhere in clearWizardState) so the cleared state is persisted across restarts; update the comment or call the same persistence helper to keep behavior consistent.
🤖 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/steps/OnboardingSummaryStep.vue`:
- Around line 75-76: The helpers and application logic for internal boot need to
ignore stale selections when the draft isn't in storage mode: update computed
properties like draftInternalBoot and internalBootSelection (and any derived
hasInternalBootSelection checks) to return null/empty unless
props.draft.bootMode === 'storage' and props.draft.skipped !== true, and guard
the applyInternalBootSelection call so it only runs when those same conditions
hold; search for internal-boot logic around internalBootSelection,
hasInternalBootSelection and applyInternalBootSelection and add the bootMode ===
'storage' && skipped !== true gate before exposing or applying the selection.
---
Outside diff comments:
In
`@web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue`:
- Around line 301-363: handleSummaryContinue currently uses try/finally around
applyInternalBootSelection but no catch, so if applyInternalBootSelection throws
the dialog remains stuck in confirmationState='saving' and applyAttempted is not
properly handled. Add a catch block for the await
applyInternalBootSelection(...) inside handleSummaryContinue that (1) sets
internalBootState.value = { applyAttempted: true, applySucceeded: false }, (2)
clears or records the error via addLog/addLogs (use the caught error to populate
summaryT('logs.internalBootReturnedError') or a generic error message), (3) set
resultSeverity/resultTitle/resultMessage to show an error result, and (4) set
confirmationState.value = 'result' so the dialog can recover; keep the existing
finally to clearInterval(progressTimer).
- Around line 64-78: The flow is locking as soon as applyAttempted is set;
change the isLocked computed (and the same logic in the duplicated block around
lines 566–599) to only become true when the apply actually succeeded (e.g.,
depend on internalBootState.value.applySucceeded — optionally combined with
applyAttempted as applyAttempted && applySucceeded) so failed applies do not
hide close/edit controls or show reboot/shutdown actions; update the computed
named isLocked (and any duplicate computed) accordingly and leave
canReturnToConfigure, confirmationState, and resultSeverity checks unchanged.
---
Duplicate comments:
In `@api/src/unraid-api/config/onboarding-tracker.service.spec.ts`:
- Around line 105-142: The test currently only checks the returned in-memory
value from OnboardingTrackerService.clearWizardState(); add an assertion that
the persistence path was exercised by verifying mockAtomicWriteFile was called
(e.g. after await tracker.clearWizardState()) and that it was invoked with the
expected serialized state containing preserved fields (completed,
completedAtVersion, forceOpen) plus the empty wizard payload. Reference the
clearWizardState method on OnboardingTrackerService, the test's
mockAtomicWriteFile spy, and the existing expected object used in the toEqual
assertion when adding this check.
In `@api/src/unraid-api/config/onboarding-tracker.service.ts`:
- Around line 539-578: saveDraft currently does an unsafe read-merge-write
(getStateResult -> writeTrackerState) and can lose updates under concurrency;
add an async mutex (e.g., a class field writeMutex from async-mutex) and wrap
the entire mutation sequence in writeMutex.runExclusive so the read
(getStateResult), merge, write (writeTrackerState) and syncConfig happen
atomically; apply the same pattern to clearWizardState and any other methods
that call writeTrackerState or mutate the tracker state to ensure serialized
writes while still using
normalizeDraft/normalizeNavigation/normalizeInternalBootState as before.
In
`@web/src/components/Onboarding/components/bootConfigurationSummary/buildBootConfigurationSummaryViewModel.ts`:
- Around line 162-171: The validation currently requires selection.bootSizeMiB
for all pool modes causing dedicated pools to be marked INCOMPLETE; update the
condition in buildBootConfigurationSummaryViewModel (the block using
isKnownPoolMode, hasPositiveInteger(selection.slotCount),
hasFiniteNumber(selection.bootSizeMiB), and poolMode checks) so that
hasFiniteNumber(selection.bootSizeMiB) is only enforced when the poolMode is not
'dedicated' (i.e., require bootSizeMiB for non-dedicated/hybrid flows where
bootReserved is used, but allow missing bootSizeMiB for poolMode ===
'dedicated').
In `@web/src/components/Onboarding/onboardingWizardState.ts`:
- Around line 183-205: The cloneOnboardingWizardDraft helper currently converts
undefined arrays into empty arrays; change the checks for plugins.selectedIds
and internalBoot.selection.devices to explicitly test for undefined (e.g.,
draft.plugins.selectedIds === undefined) and return undefined in that case,
otherwise return a shallow copy (e.g., [...draft.plugins.selectedIds] or mapped
devices). Update the branches inside cloneOnboardingWizardDraft (symbols:
cloneOnboardingWizardDraft, plugins.selectedIds, internalBoot.selection.devices,
internalBoot.selection) so undefined is preserved, while null/empty arrays
remain correctly cloned.
- Around line 130-132: The slotCount and bootSizeMiB assignments currently
accept any finite number; change their validation to only allow positive
integers by replacing Number.isFinite checks with
Number.isInteger(parsedSlotCount) && parsedSlotCount > 0 and
Number.isInteger(parsedBootSizeMiB) && parsedBootSizeMiB > 0 respectively (keep
the fallback to undefined), so slotCount and bootSizeMiB are only set when
parsedSlotCount/parsedBootSizeMiB are positive whole numbers.
In
`@web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue`:
- Around line 187-209: toInternalBootSelection currently builds a payload
whenever draft.selection exists, which can re-apply stale disks if the user
switched to USB or skipped storage; update toInternalBootSelection to
early-return null when the draft indicates USB/skipped storage (e.g. if
draft.bootSource === 'usb' or draft.skipStorage === true) before validating or
mapping selection, keeping the rest of the validation logic intact (function
name: toInternalBootSelection, local variable: selection, mapping: devices.map).
In `@web/src/components/Onboarding/steps/OnboardingCoreSettingsStep.vue`:
- Around line 62-69: The current code treats props.initialDraft as a complete
override which can inadvertently shadow activation/API/browser precedence when
initialDraft is an empty object or missing fields; update each initialization
(e.g., selectedTimeZone, serverName, serverDescription, selectedTheme,
selectedLanguage, useSsh and the other occurrences at the referenced blocks) to
merge field-by-field by checking whether the draft explicitly provides that
property (use hasOwnProperty or the 'in' operator) and only use draft[field]
when present, otherwise fall back to the existing precedence/defaults (e.g.,
resolveInitialTimeZone() or TRUSTED_DEFAULT_PROFILE.*); apply this pattern to
all similar initializations so missing fields in initialDraft do not overwrite
computed/default values.
In `@web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue`:
- Around line 741-744: The hydrate logic for poolName uses || which treats an
intentionally empty string as missing and overwrites saved blank values; update
the fallback chain in the assignment to use nullish coalescing (??) so
draftSelection?.poolName is only replaced when null or undefined (refer to
poolName.value and draftSelection, and keep poolMode.value logic intact), and
still fall back to data.poolNameDefault when appropriate.
- Around line 713-719: selectedDraftDevices is currently rebuilt only from
selectedDeviceById which drops previously saved devices that no longer appear in
the latest eligibility response; change the rebuild logic in the block that
computes selectedDraftDevices to merge existing persisted devices from
selection.devices with the devices derived from selectedDeviceById (keep the
devices from selection.devices that are missing from selectedDeviceById by id),
so saved device metadata is preserved when reconstructing selectedDraftDevices
(refer to symbols: selectedDraftDevices, selectedDeviceById, selectedDevices,
slotCount, and selection.devices).
---
Nitpick comments:
In `@api/src/unraid-api/config/onboarding-tracker.service.ts`:
- Around line 499-513: The override branch in clearWizardState (which checks
onboardingOverrides.getState()) only updates this.state and returns
getCachedState() without persisting to disk; either add a clear comment above
that branch documenting that this is intentional (in-memory-only for overrides)
or modify the branch to call the same persistence logic used in the non-override
branch (mirror the disk-write/save function used elsewhere in clearWizardState)
so the cleared state is persisted across restarts; update the comment or call
the same persistence helper to keep behavior consistent.
In `@api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts`:
- Around line 21-25: Remove the unused imports OnboardingWizardBootMode and
OnboardingWizardPoolMode from the import statement in onboarding.model.ts; keep
only the actually used symbol OnboardingWizardStepId (imported from
`@app/unraid-api/graph/resolvers/customization/activation-code.model.js`) so the
file no longer imports unused identifiers flagged by static analysis.
In `@web/src/components/Onboarding/OnboardingModal.vue`:
- Around line 352-357: The catch block in OnboardingModal.vue sets
saveTransitionError using t('onboarding.nextSteps.completionFailed'), which is
misleading for a draft-save failure; update the catch to use a dedicated i18n
key such as t('onboarding.draft.saveFailed') with an appropriate fallback
message ("We couldn't save your onboarding draft right now."), and ensure the
i18n key is added to the translations; references: saveTransitionError, the
catch block around the save/draft save logic, and
t('onboarding.nextSteps.completionFailed').
🪄 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: 9ca9b7c0-ccd2-4122-95f6-d6815c5199ab
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (52)
api/generated-schema.graphqlapi/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/config/onboarding-tracker.model.tsapi/src/unraid-api/config/onboarding-tracker.service.spec.tsapi/src/unraid-api/config/onboarding-tracker.service.tsapi/src/unraid-api/graph/resolvers/customization/activation-code.model.tsapi/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.tsapi/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.tsapi/src/unraid-api/graph/resolvers/customization/onboarding.service.tsapi/src/unraid-api/graph/resolvers/mutation/mutation.model.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding.model.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.tsdocs/onboarding-internal-boot-port-differences.mddocs/onboarding-internal-boot.mdweb/__test__/components/Onboarding/OnboardingBootConfigurationSummary.test.tsweb/__test__/components/Onboarding/OnboardingCoreSettingsStep.test.tsweb/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.tsweb/__test__/components/Onboarding/OnboardingInternalBootStep.test.tsweb/__test__/components/Onboarding/OnboardingModal.test.tsweb/__test__/components/Onboarding/OnboardingNextStepsStep.test.tsweb/__test__/components/Onboarding/OnboardingPluginsStep.test.tsweb/__test__/components/Onboarding/OnboardingSummaryStep.test.tsweb/__test__/components/Onboarding/onboardingStorageCleanup.test.tsweb/__test__/components/Onboarding/onboardingWizardState.test.tsweb/__test__/store/onboardingContextData.test.tsweb/components.d.tsweb/src/components/Onboarding/ONBOARDING_WIZARD.mdweb/src/components/Onboarding/OnboardingModal.vueweb/src/components/Onboarding/UPGRADE_ONBOARDING.mdweb/src/components/Onboarding/components/bootConfigurationSummary/OnboardingBootConfigurationSummary.vueweb/src/components/Onboarding/components/bootConfigurationSummary/buildBootConfigurationSummaryViewModel.tsweb/src/components/Onboarding/composables/internalBoot.tsweb/src/components/Onboarding/graphql/closeOnboarding.mutation.tsweb/src/components/Onboarding/graphql/onboardingBootstrap.query.tsweb/src/components/Onboarding/graphql/saveOnboardingDraft.mutation.tsweb/src/components/Onboarding/onboardingWizardState.tsweb/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vueweb/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vueweb/src/components/Onboarding/steps/OnboardingCoreSettingsStep.vueweb/src/components/Onboarding/steps/OnboardingInternalBootStep.vueweb/src/components/Onboarding/steps/OnboardingLicenseStep.vueweb/src/components/Onboarding/steps/OnboardingNextStepsStep.vueweb/src/components/Onboarding/steps/OnboardingOverviewStep.vueweb/src/components/Onboarding/steps/OnboardingPluginsStep.vueweb/src/components/Onboarding/steps/OnboardingSummaryStep.vueweb/src/components/Onboarding/store/onboardingContextData.tsweb/src/components/Onboarding/store/onboardingDraft.tsweb/src/components/Onboarding/store/onboardingModalVisibility.tsweb/src/components/Onboarding/store/onboardingStorageCleanup.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.ts
💤 Files with no reviewable changes (4)
- web/src/components/Onboarding/store/onboardingStorageCleanup.ts
- docs/onboarding-internal-boot-port-differences.md
- web/src/components/Onboarding/UPGRADE_ONBOARDING.md
- web/src/components/Onboarding/store/onboardingDraft.ts
✅ Files skipped from review due to trivial changes (11)
- api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts
- web/components.d.ts
- web/test/store/onboardingContextData.test.ts
- web/src/components/Onboarding/graphql/onboardingBootstrap.query.ts
- web/src/components/Onboarding/ONBOARDING_WIZARD.md
- api/src/unraid-api/config/onboarding-tracker.model.ts
- web/test/components/Onboarding/OnboardingBootConfigurationSummary.test.ts
- web/src/components/Onboarding/graphql/saveOnboardingDraft.mutation.ts
- docs/onboarding-internal-boot.md
- web/test/components/Onboarding/onboardingWizardState.test.ts
- web/src/components/Onboarding/steps/OnboardingLicenseStep.vue
🚧 Files skipped from review as they are similar to previous changes (16)
- web/test/components/Onboarding/onboardingStorageCleanup.test.ts
- web/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vue
- api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts
- web/src/components/Onboarding/store/onboardingContextData.ts
- web/test/components/Onboarding/OnboardingCoreSettingsStep.test.ts
- api/src/unraid-api/config/api-config.test.ts
- api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts
- web/src/components/Onboarding/components/bootConfigurationSummary/OnboardingBootConfigurationSummary.vue
- web/src/components/Onboarding/graphql/closeOnboarding.mutation.ts
- api/src/unraid-api/graph/resolvers/customization/activation-code.model.ts
- web/src/components/Onboarding/steps/OnboardingPluginsStep.vue
- web/test/components/Onboarding/OnboardingPluginsStep.test.ts
- api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts
- web/test/components/Onboarding/OnboardingInternalBootStandalone.test.ts
- web/src/composables/gql/gql.ts
- web/test/components/Onboarding/OnboardingSummaryStep.test.ts
- Purpose: keep onboarding draft and wizard-state writes grounded in the persisted tracker file even when testing overrides are active. - Before: saveDraft() and part of clearWizardState() operated on the override-composed public view, so temporary override fields like forceOpen/completed could leak into onboarding-tracker.json or be treated differently from other mutators. - Why that was a problem: override mode is meant to be an in-memory testing layer, not a way to silently rewrite the real persisted completion metadata when a user advances through the wizard. - What this change accomplishes: draft saves and wizard clears now mutate only the persisted base tracker state while still returning the override-composed public view to callers. - How it works: add an internal persisted-state read path, route saveDraft() and clearWizardState() through it, and extend tracker specs to assert that override-backed reads no longer change the on-disk completion/forceOpen fields.
- Purpose: keep partial onboarding overrides additive instead of resetting unspecified completion fields to default values. - Before: a partial override like forceOpen-only would silently force completed=false and completedAtVersion=undefined in the public tracker view. - Why that was a problem: override mode is used for testing and debugging, and unspecified override fields should inherit the persisted tracker state rather than changing effective completion state. - What this change accomplishes: partial overrides now preserve the base tracker completion metadata while still overriding the fields they explicitly set. - How it works: merge override onboarding fields against baseState instead of hardcoded false/undefined defaults, and add a regression spec that proves a forceOpen-only override keeps the persisted completed/completedAtVersion values.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
api/src/unraid-api/config/onboarding-tracker.service.ts (3)
157-163:⚠️ Potential issue | 🟡 MinorCap
devicesto the normalizedslotCount.Line 159 clamps
slotCount, but Line 162 still keeps every valid device. That can persist inconsistent selections such asslotCount: 2with 3+ devices.✂️ Proposed fix
const candidate = value as Record<string, unknown>; const parsedSlotCount = Number(candidate.slotCount); const parsedBootSize = Number(candidate.bootSizeMiB); + const slotCount = Number.isFinite(parsedSlotCount) + ? Math.max(1, Math.min(2, parsedSlotCount)) + : undefined; return { poolName: normalizeString(candidate.poolName), - slotCount: Number.isFinite(parsedSlotCount) - ? Math.max(1, Math.min(2, parsedSlotCount)) - : undefined, - devices: normalizeBootDeviceArray(candidate.devices), + slotCount, + devices: normalizeBootDeviceArray(candidate.devices).slice(0, slotCount ?? 2), bootSizeMiB: Number.isFinite(parsedBootSize) ? Math.max(0, parsedBootSize) : undefined, updateBios: normalizeBoolean(candidate.updateBios, false), poolMode: normalizePoolMode(candidate.poolMode),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/config/onboarding-tracker.service.ts` around lines 157 - 163, The devices array returned from this object must be capped to the normalized slotCount to avoid inconsistent selections; after computing the normalized slotCount (the value currently produced by the slotCount expression) call normalizeBootDeviceArray(candidate.devices) and then trim or slice the resulting array to at most that slotCount (if slotCount is defined) before returning it. Concretely, compute a single normalizedSlotCount variable using Number.isFinite(parsedSlotCount) ? Math.max(1, Math.min(2, parsedSlotCount)) : undefined, call normalizeBootDeviceArray(candidate.devices) to get devices, and if normalizedSlotCount is a number, set devices = devices.slice(0, normalizedSlotCount) so the returned devices length never exceeds the clamped slotCount.
494-517:⚠️ Potential issue | 🟠 MajorSerialize these tracker mutations.
Lines 495-513 and Lines 520-555 still do an unguarded read/merge/write against the same tracker file. Two overlapping requests can both read the same base state, and the later write wins, dropping one request’s
draft,navigation, orinternalBootStatechange.🔒 Sketch
export class OnboardingTrackerService implements OnApplicationBootstrap { private readonly logger = new Logger(OnboardingTrackerService.name); private readonly trackerPath = path.join(PATHS_CONFIG_MODULES, TRACKER_FILE_NAME); private state: TrackerState = {}; private currentVersion?: string; private bypassActive = false; private readonly versionFilePath: string; + private mutationQueue: Promise<void> = Promise.resolve(); + + private async runSerializedMutation<T>(fn: () => Promise<T>): Promise<T> { + const previous = this.mutationQueue; + let release!: () => void; + this.mutationQueue = new Promise<void>((resolve) => { + release = resolve; + }); + await previous; + try { + return await fn(); + } finally { + release(); + } + }async saveDraft(input: SaveOnboardingDraftInput): Promise<PublicTrackerState> { - const currentStateResult = await this.getPersistedStateResult(); - if (currentStateResult.kind === 'error') { - throw currentStateResult.error; - } - - const currentState = currentStateResult.state; - const nextDraft = normalizeDraft({ - ...currentState.draft, - ...(input.draft ?? {}), - }); - - const updatedState: TrackerState = { - completed: currentState.completed, - completedAtVersion: currentState.completedAtVersion, - forceOpen: currentState.forceOpen, - draft: nextDraft, - navigation: normalizeNavigation( - input.navigation - ? { - ...currentState.navigation, - ...input.navigation, - } - : currentState.navigation - ), - internalBootState: normalizeInternalBootState( - input.internalBootState - ? { - ...currentState.internalBootState, - ...input.internalBootState, - } - : currentState.internalBootState - ), - }; - - await this.writeTrackerState(updatedState); - this.syncConfig(); - - return this.getCachedState(); + return this.runSerializedMutation(async () => { + const currentStateResult = await this.getPersistedStateResult(); + if (currentStateResult.kind === 'error') { + throw currentStateResult.error; + } + + const currentState = currentStateResult.state; + const nextDraft = normalizeDraft({ + ...currentState.draft, + ...(input.draft ?? {}), + }); + + const updatedState: TrackerState = { + completed: currentState.completed, + completedAtVersion: currentState.completedAtVersion, + forceOpen: currentState.forceOpen, + draft: nextDraft, + navigation: normalizeNavigation( + input.navigation + ? { + ...currentState.navigation, + ...input.navigation, + } + : currentState.navigation + ), + internalBootState: normalizeInternalBootState( + input.internalBootState + ? { + ...currentState.internalBootState, + ...input.internalBootState, + } + : currentState.internalBootState + ), + }; + + await this.writeTrackerState(updatedState); + this.syncConfig(); + + return this.getCachedState(); + }); }Also applies to: 519-558
312-316:⚠️ Potential issue | 🟠 MajorFall back to
baseStatewhen an override omits completion fields.Lines 314-316 currently fall back to hardcoded defaults instead of the persisted values. An override that only sets
forceOpenwill make the public state look incomplete and clearcompletedAtVersion.🩹 Proposed fix
if (overrideState?.onboarding !== undefined) { return { ...baseState, - completed: overrideState.onboarding.completed ?? false, - completedAtVersion: overrideState.onboarding.completedAtVersion ?? undefined, - forceOpen: overrideState.onboarding.forceOpen ?? false, + completed: overrideState.onboarding.completed ?? baseState.completed, + completedAtVersion: + overrideState.onboarding.completedAtVersion ?? baseState.completedAtVersion, + forceOpen: overrideState.onboarding.forceOpen ?? baseState.forceOpen, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/config/onboarding-tracker.service.ts` around lines 312 - 316, The current merge of baseState and overrideState uses hardcoded defaults for onboarding fields which clobbers persisted values; update the object construction in the function that builds the public onboarding state so each field falls back to baseState's onboarding values when overrideState.onboarding omits them. Specifically, replace completed, completedAtVersion, and forceOpen assignments to use nullish-coalescing with baseState.onboarding (e.g. completed: overrideState.onboarding.completed ?? baseState.onboarding.completed, completedAtVersion: overrideState.onboarding.completedAtVersion ?? baseState.onboarding.completedAtVersion, and forceOpen: overrideState.onboarding.forceOpen ?? baseState.onboarding.forceOpen ?? false) so partial overrides (like only forceOpen) don’t clear persisted completion data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@api/src/unraid-api/config/onboarding-tracker.service.ts`:
- Around line 157-163: The devices array returned from this object must be
capped to the normalized slotCount to avoid inconsistent selections; after
computing the normalized slotCount (the value currently produced by the
slotCount expression) call normalizeBootDeviceArray(candidate.devices) and then
trim or slice the resulting array to at most that slotCount (if slotCount is
defined) before returning it. Concretely, compute a single normalizedSlotCount
variable using Number.isFinite(parsedSlotCount) ? Math.max(1, Math.min(2,
parsedSlotCount)) : undefined, call normalizeBootDeviceArray(candidate.devices)
to get devices, and if normalizedSlotCount is a number, set devices =
devices.slice(0, normalizedSlotCount) so the returned devices length never
exceeds the clamped slotCount.
- Around line 312-316: The current merge of baseState and overrideState uses
hardcoded defaults for onboarding fields which clobbers persisted values; update
the object construction in the function that builds the public onboarding state
so each field falls back to baseState's onboarding values when
overrideState.onboarding omits them. Specifically, replace completed,
completedAtVersion, and forceOpen assignments to use nullish-coalescing with
baseState.onboarding (e.g. completed: overrideState.onboarding.completed ??
baseState.onboarding.completed, completedAtVersion:
overrideState.onboarding.completedAtVersion ??
baseState.onboarding.completedAtVersion, and forceOpen:
overrideState.onboarding.forceOpen ?? baseState.onboarding.forceOpen ?? false)
so partial overrides (like only forceOpen) don’t clear persisted completion
data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f68a483f-f097-41d5-baf4-305431d7a6a6
📒 Files selected for processing (2)
api/src/unraid-api/config/onboarding-tracker.service.spec.tsapi/src/unraid-api/config/onboarding-tracker.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/config/onboarding-tracker.service.spec.ts
- Purpose: keep persisted internal-boot selections internally consistent after tracker normalization. - Before: slotCount was clamped to the supported 1-2 disk range, but devices could still keep more entries than the effective slotCount. - Why that was a problem: malformed tracker data could normalize into contradictory state like slotCount=2 with 3 devices, forcing downstream code to guess which field was authoritative. - What this change accomplishes: normalized internal-boot selections now truncate devices to the effective clamped slot count. - How it works: compute the normalized slotCount first, slice the normalized device array to that size, and add a regression spec covering oversized persisted selections.
- Purpose: preserve the difference between untouched draft fields and explicit empty selections after moving the wizard draft to a JSON boundary. - Before: web normalization and cloning eagerly converted omitted arrays and malformed optional state into [] or null. - Why that was a problem: untouched sections could look explicitly cleared, plugin defaults could be reapplied after a user chose none, and later saves could manufacture state the user never set. - What this change accomplishes: optional arrays now stay undefined unless they were actually provided, while explicit empty selections remain intentional. - How it works: tighten draft normalization and cloning, update plugin selection hydration to treat undefined as default-only and [] as explicit none, and add focused regression coverage.
- Purpose: keep the JSON-backed wizard draft boundary aligned with the actual internal boot domain constraints. - Before: the web draft normalizer accepted any finite numbers for and , including fractional, negative, and unsupported values. - Why that was a problem: malformed persisted draft data could survive into local wizard state and get round-tripped back through the UI. - What this change accomplishes: now only keeps supported integer values, while keeps only non-negative integers and still allows . - How it works: replace the generic finite-number check with field-specific normalizers and add regression coverage for invalid counts and zero-sized boot selections.
- Purpose: keep internal boot summary and apply behavior aligned with the user's current top-level boot intent. - Before: stale storage selections could remain in draft state after switching to USB or skipped boot mode, and parts of the summary/apply flow would still treat that stale data as active. - Why that was a problem: users could see invalid summary states or risk applying an outdated storage configuration that no longer matched the current wizard selection. - What this change accomplishes: stale storage selections are now ignored unless the draft is actively in storage mode and not skipped. - How it works: gate both apply conversion helpers on the current boot mode, make the shared boot summary renderer prefer the current USB intent over leftover storage data, and add focused regression coverage in the onboarding and standalone summary flows.
- Purpose: prevent the onboarding stepper from racing an in-flight draft save during step transitions. - Before: Continue and Back transitions were guarded by the save flag, but clicking the stepper could still move the modal while the draft save mutation was still pending. - Why that was a problem: users could navigate the stepper mid-save and temporarily desynchronize the local modal step from the server-owned transition being persisted. - What this change accomplishes: stepper clicks are now ignored while a transition save is in flight, while the close path remains available as an escape hatch. - How it works: short-circuit goToStep when isSavingTransition is true and add a modal regression test that holds the save promise open and verifies the stepper does nothing until the save resolves.
- Purpose: keep the internal boot documentation aligned with the persisted draft shape used by the server-owned onboarding flow. - Before: the doc still showed selection.devices as string IDs even though the tracker now stores device objects. - Why that was a problem: readers would be pointed at the wrong JSON contract when debugging or reasoning about resume behavior. - What this change accomplishes: the example now matches the persisted device shape. - How it works: refresh the saved-shape example and add a short note explaining that device metadata is persisted so the summary views can render stable labels without re-fetching disk metadata.
- Purpose: keep the onboarding architecture docs aligned with the sparse tracker draft model used by the server-owned wizard. - Before: the docs implied that draft sections were always present as empty objects, which no longer matches the real tracker behavior. - Why that was a problem: omitted sections and explicit empty values have different semantics in this flow, so the old example taught the wrong mental model. - What this change accomplishes: the docs now show both an empty sparse tracker and a realistic non-empty saved draft example. - How it works: replace the preinitialized draft example with as the default shape, explain the sparse-section rules, and add a second example that shows persisted core settings, plugin selections, and internal-boot state.
- Purpose: keep the onboarding boot summary flows fully localized instead of mixing new English-only warning text into translated sessions. - Before: the onboarding Summary invalid-config banner and the standalone internal-boot invalid-summary message were hard-coded strings. - Why that was a problem: non-English sessions could see untranslated warning copy inside otherwise localized onboarding modals. - What this change accomplishes: both warning messages now resolve through vue-i18n and share the onboarding summary translation namespace. - How it works: replace the hard-coded strings with translated lookups and add the corresponding English locale keys so other locales safely fall back to en_US until translated.
- Purpose: clean up dead onboarding model imports left behind after the draft boundary was simplified. - Before: onboarding.model.ts still imported enum types that were no longer referenced after moving the draft payload to JSON. - Why that was a problem: the unused imports triggered code-scanning noise and made the file look more coupled than it really is. - What this change accomplishes: the resolver model now imports only the GraphQL types it actually uses. - How it works: delete the unused onboarding draft enum imports and normalize the remaining import formatting.
- Purpose: make the internal boot step test wait on the correct Vue reactivity boundary. - Before: the test used flushPromises after toggling poolMode even though that path depends on Vue reactive updates, not a promise chain. - Why that was a problem: the assertion barrier was broader than necessary and slightly more timing-dependent than the behavior under test. - What this change accomplishes: the test now waits on nextTick when the reactive pool-mode toggle changes the selectable-device state. - How it works: import nextTick, replace the two reactive waits in that scenario, and keep the rest of the test flow unchanged.
- Purpose: make onboarding query and transition states easier to reason about by keeping shell concerns in the modal and step-visible failures inside the active step. - Before: steps mixed best-effort query fallbacks with step-specific loaders, the modal rendered a separate top save-failure banner, and closeOnboarding carried an optional SAVE_FAILURE GraphQL reason used only for extra metadata logging. - Why that was a problem: users could see overlapping modal and step errors at the same time, summary could continue with partial assumptions, and the close mutation/schema carried extra onboarding-only plumbing that did not affect behavior. - What changed: added a shared strict step query gate, made query-backed steps block on required data with embedded loading/error states, moved transition-save failures into the step surface, and removed the close-reason enum/input plumbing from web and API. - How it works: the modal now owns only shell loading and exit confirmation, each step owns its own query and submit failures, closeOnboarding is a plain close again, and the updated onboarding tests plus regenerated GraphQL types cover the stricter flow.
- Purpose: simplify onboarding step state ownership by removing the extra query-gate wrapper layer and using one shared embedded blocking UI for step failures. - Before: query-backed steps rendered through OnboardingStepQueryGate while save-transition failures used a separate embedded treatment, so step state was still split across multiple abstractions. - Problem: the extra wrapper made the code harder to read, hid the real step state flow, and kept two different step-level error systems alive. - Change: add OnboardingStepBlockingState, inline loading/query-error branches directly in the affected steps, delete OnboardingStepQueryGate and its test, and keep the shared copy in locale/component registration files aligned. - How it works: each step now reads directly as save failure -> blocking card, query loading -> loading state, query error -> blocking card with retry/exit, ready -> normal content.
- Purpose: make the save-failure blocking state tell users the right next step after onboarding cannot continue. - Before: the message told users to exit setup and reopen onboarding later, which no longer matches the desired guidance. - Problem: that copy implied onboarding was the preferred recovery path instead of pointing people back to the Unraid dashboard. - Change: replace the shared onboarding.stepSaveFailure.description string in en.json. - How it works: every step that renders the shared save-failure blocking state now shows the new dashboard-focused message automatically.
- Purpose: make the standalone internal boot result message use the same normal alert styling as the rest of onboarding. - Before: the result state used hand-rolled border and background classes, which produced a washed-out warning panel that did not match the standard UAlert presentation. - Problem: the standalone flow looked visually inconsistent and the warning state was harder to read next to the rest of the wizard UI. - Change: replace the custom result panel markup with a standard UAlert and compute the matching success, warning, and error alert props. - How it works: the standalone summary now maps result severity to normal alert color and icon props while still rendering the BIOS follow-up note inside the alert description when needed.
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
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
2 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
This PR moves the onboarding wizard from browser-owned draft persistence to a server-owned flow.
The main change is that onboarding progress, step visibility, and saved draft data now live on the server instead of in browser
localStoragewhich is not used at all. The web app bootstraps from server state, keeps only transient in-memory step edits while the user is actively editing, and persists draft state only on step transitions.The PR also includes follow-up UX and internal-boot improvements that build on that server-owned flow.
Why
Before this change:
onboardingDraftpathThat created a few problems:
What changed
Server-owned onboarding state
ONBOARDING_BOOTSTRAP_QUERYBrowser draft removal
localStorageonboardingDraftdata is deleted rather than migratedGraphQL draft boundary simplification
draftnow crosses the GraphQL boundary as JSON instead of a large nested GraphQL input/output treecurrentStepId,visibleStepIds,navigation, andinternalBootStateremain explicitly typed GraphQL fieldsExit and completion behavior
Core settings bootstrap and precedence
Internal boot draft and summary improvements
idsizeBytesdeviceNameBehavioral changes
After this PR:
Continue,Back, andSkipNotable implementation details
GB/TBdisplay rules even though rawsizeBytesare stored in the draftTesting
Validated with focused API and web coverage across the server-owned onboarding flow, including:
Notes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Sparse draft semantics now preserve
undefinedversus explicit empty selections so untouched sections do not get materialized as cleared state.Numeric draft normalization now only keeps supported integer slot counts and non-negative MiB sizes, while still allowing a boot size of
0.Stale internal-boot storage selections are now ignored whenever the current draft has moved back to USB or skipped mode, so summary rendering and apply behavior stay aligned.
The onboarding stepper now ignores clicks while a transition save is in flight, preventing backward navigation races without trapping users in the modal.
Internal boot docs now show the persisted device-object shape used in the tracker, instead of the old string-id example.
The onboarding architecture docs now describe the tracker draft as sparse by default and include a realistic non-empty saved example.
The two new boot-configuration warning banners now go through vue-i18n and fall back to en_US instead of being hard-coded English strings.
Removed the last unused onboarding GraphQL model imports left over from the draft-to-JSON refactor.
The remaining internal-boot test now uses nextTick for the pool-mode reactivity case instead of flushPromises.
Final onboarding state ownership
The final branch state also includes a follow-up cleanup to simplify onboarding loading and error ownership:
closeOnboardingsave-failure reason plumbing was removed; closing onboarding is a plain close again, and save failures rely on the existing frontend/server logs instead of extra GraphQL metadataThis leaves the wizard with a simpler ownership model:
OnboardingStepQueryGatewrapper entirely; the remaining query-backed steps now branch directly on save failure, query loading, query error, or ready content while sharing the same embedded blocking-state component.