Skip to content

feat: move onboarding wizard persistence server-side#1984

Draft
Ajit-Mehrotra wants to merge 16 commits intomainfrom
codex/server-side-onboarding-wizard
Draft

feat: move onboarding wizard persistence server-side#1984
Ajit-Mehrotra wants to merge 16 commits intomainfrom
codex/server-side-onboarding-wizard

Conversation

@Ajit-Mehrotra
Copy link
Copy Markdown
Contributor

@Ajit-Mehrotra Ajit-Mehrotra commented Apr 1, 2026

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 localStorage which 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:

  • onboarding draft state was persisted in the browser via the old onboardingDraft path
  • resume behavior depended on browser state rather than a server-owned source of truth
  • the wizard could not reliably resume across sessions or devices from authoritative server state
  • server name changes would cause URL change and client-side onboardingDraft would be lost!

That created a few problems:

  • onboarding state was tied to a single browser rather than the server being configured
  • the API was not the source of truth for step visibility, current step, and saved wizard data
  • stale browser state had to be cleaned up and could drift from live server state

What changed

Server-owned onboarding state

  • added durable onboarding wizard persistence in the server tracker
  • the API now owns:
    • saved draft data
    • current step navigation state
    • internal-boot apply state
    • visible step IDs
  • the web onboarding modal now hydrates from ONBOARDING_BOOTSTRAP_QUERY
  • step transitions save through GraphQL instead of updating browser draft storage

Browser draft removal

  • removed the old browser-owned onboarding draft store
  • onboarding no longer reads wizard progress from localStorage
  • legacy onboarding browser keys are now only cleaned up best-effort
  • old onboardingDraft data is deleted rather than migrated

GraphQL draft boundary simplification

  • onboarding draft now crosses the GraphQL boundary as JSON instead of a large nested GraphQL input/output tree
  • currentStepId, visibleStepIds, navigation, and internalBootState remain explicitly typed GraphQL fields
  • the durable draft shape now lives in the tracker and web TypeScript types instead of being duplicated across GraphQL DTOs and generated schema/codegen output
  • tracker and web boundary helpers normalize the JSON draft on write/read so the runtime contract stays typed without the GraphQL schema carrying every nested draft field

Exit and completion behavior

  • modal exits now clear the server-owned onboarding draft/navigation/internal-boot state
  • dashboard exit, modal close, and internal-boot shutdown/reboot flows all clear draft state before leaving when possible
  • onboarding completion still marks the wizard complete and clears durable wizard state

Core settings bootstrap and precedence

  • the web flow now differentiates between modal bootstrap state and the live Core Settings baseline query
  • documented how Core Settings values are prioritized across:
    • saved draft values
    • activation metadata
    • browser timezone
    • live server settings
    • trusted defaults

Internal boot draft and summary improvements

  • selected internal-boot devices are now persisted as draft objects with:
    • id
    • sizeBytes
    • deviceName
  • onboarding summary and standalone internal-boot summary now render from saved draft device metadata instead of refetching device labels
  • this removes the boot-summary relabel flicker and makes both summaries consistent
  • the standalone internal-boot summary now keeps logs underneath the existing summary card instead of opening a separate section
  • standalone internal-boot actions are hidden while apply is still running and only reappear once the result state is reached
  • hybrid mode is now the default internal-boot storage mode, and the dropdown order matches that default

Behavioral changes

After this PR:

  • onboarding opens from server bootstrap state rather than browser draft state
  • draft persistence happens only on Continue, Back, and Skip
  • the active step blocks navigation while transition saves are in flight
  • closing or leaving onboarding clears the saved wizard draft instead of preserving partial state indefinitely
  • internal-boot summaries show complete device labels immediately from saved draft data
  • the standalone internal-boot wizard behaves more like the main onboarding summary flow during apply

Notable implementation details

  • the durable tracker shape now carries wizard draft, navigation state, and internal-boot apply state
  • visible steps are computed server-side instead of persisted client-side
  • the web modal keeps local in-memory draft state only while the user is editing
  • boot summary labels continue to format disk size using the existing metric GB/TB display rules even though raw sizeBytes are stored in the draft

Testing

Validated with focused API and web coverage across the server-owned onboarding flow, including:

  • onboarding tracker normalization and persistence
  • onboarding save/close/complete service behavior
  • onboarding modal bootstrap and transition-save behavior
  • internal-boot draft save/load behavior
  • onboarding and standalone internal-boot summary rendering
  • standalone internal-boot apply-state locking
  • web type-check and focused component tests
  • JSON draft boundary normalization in both API and web hydration/save paths

Notes

  • this PR intentionally does not migrate legacy browser draft data into the new server-owned wizard state
  • legacy browser onboarding keys are only cleaned up and removed
  • one intermediate Core Settings gating experiment was reverted during this branch, so the net behavior reflects the final server-owned onboarding flow rather than that temporary follow-up

Summary by CodeRabbit

  • New Features

    • Server-owned onboarding wizard with durable, persistable draft and a Save Draft action
    • New boot-configuration summary component showing devices, pool mode, and formatted sizes
    • Steps now derive visibility from server eligibility; transitions persist and block on save failures
    • Modal close can include a reason (e.g., SAVE_FAILURE) to surface save-error handling
  • Bug Fixes

    • Array start accepts optional decryption password and keyfile
  • Documentation

    • Added onboarding/internal-boot docs; removed legacy upgrade/local-draft docs

- 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.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: aaca1325-1cc9-4016-9d57-28acda4ef219

📥 Commits

Reviewing files that changed from the base of the PR and between df546ab and 91336a3.

📒 Files selected for processing (1)
  • api/src/unraid-api/config/api-config.test.ts

Walkthrough

Adds a server-owned onboarding wizard: new GraphQL types/mutations, durable tracker draft/navigation/internal-boot state and service APIs, backend normalization/merge logic, frontend refactor to local prop-driven wizard state (Pinia draft store removed), new boot-configuration summary component, and broad test/documentation updates.

Changes

Cohort / File(s) Summary
GraphQL Schema & Documents
api/generated-schema.graphql, web/src/composables/gql/graphql.ts, web/src/components/Onboarding/graphql/*
Introduced onboarding wizard types/enums (OnboardingWizard, OnboardingWizardInternalBootState, OnboardingWizardStepId, etc.), CloseOnboardingInput/CloseOnboardingReason, updated closeOnboarding(input: ...), added SaveOnboardingDraftInput and saveOnboardingDraft mutation, and extended ArrayStateInput with decryption fields. Updated bootstrap/close/save documents.
Backend: Tracker Model & Service
api/src/unraid-api/config/onboarding-tracker.model.ts, api/src/unraid-api/config/onboarding-tracker.service.ts, api/src/unraid-api/config/onboarding-tracker.service.spec.ts
TrackerState now includes optional draft, navigation, internalBootState. Service adds clearWizardState() and saveDraft() with normalization, merging, and persistence; tests added/updated for merge/clear/sanitization and empty-default behavior.
Backend: Onboarding Resolver & Models
api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts, .../onboarding.mutation.ts, .../activation-code.model.ts, .../onboarding/onboarding.model.ts
Added visible-step computation and current-step normalization; getOnboardingResponse() now returns wizard. Added GraphQL object/input types/enums and service binding for saveOnboardingDraft. closeOnboarding resolver signature changed to accept optional input and logs on save-failure reason. Resolver tests updated.
Frontend: Wizard State Module & Docs
web/src/components/Onboarding/onboardingWizardState.ts, web/src/components/Onboarding/ONBOARDING_WIZARD.md
New typed wizard state module (draft shapes, normalizer, clone/constructors) and documentation describing bootstrap flow, durable tracker format, component integration, and save semantics.
Frontend: Modal, Steps & Standalone Views
web/src/components/Onboarding/OnboardingModal.vue, web/src/components/Onboarding/steps/*, web/src/components/Onboarding/standalone/*
Modal refactored to local reactive wizard state (localDraft, localCurrentStepId, localInternalBootState). Steps converted to prop-driven drafts and async callbacks; transitions persist via SAVE_ONBOARDING_DRAFT_MUTATION. Internal-boot standalone updated to summary + confirm/apply flow; components accept isSavingStep/saveError where applicable.
Frontend: Boot Configuration Summary
web/src/components/Onboarding/components/bootConfigurationSummary/*, web/__test__/components/Onboarding/OnboardingBootConfigurationSummary.test.ts, web/components.d.ts
Added builder buildBootConfigurationSummaryViewModel and OnboardingBootConfigurationSummary.vue component, producing hidden/invalid/ready view models and rendering device rows; tests added and component globally typed.
Frontend: Store & Cleanup Changes
web/src/components/Onboarding/store/onboardingDraft.ts (deleted), web/src/components/Onboarding/store/onboardingContextData.ts, web/src/components/Onboarding/store/onboardingModalVisibility.ts, web/src/components/Onboarding/store/onboardingStorageCleanup.ts
Removed Pinia onboardingDraft store. Context store exposes wizard (replacing internalBootVisibility). closeModal accepts optional reason and sends it to closeOnboarding mutation. Legacy storage cleanup no longer resets a live Pinia store.
Frontend: GraphQL Operations & Typings
web/src/composables/gql/gql.ts, web/src/composables/gql/graphql.ts, web/src/components/Onboarding/graphql/*
Updated typed GraphQL helper overloads and generated typings to include wizard in bootstrap, updated CloseOnboarding to accept input, and added SaveOnboardingDraft mutation and corresponding types.
Tests & Fixtures
api/.../*.spec.ts, web/__test__/components/Onboarding/*, web/__test__/store/onboardingContextData.test.ts, web/__test__/components/Onboarding/onboardingWizardState.test.ts
Extensive test updates: backend tracker/resolver tests for wizard state and merging; frontend tests rewritten to pass initialDraft/draft/internalBootState props, new boot-summary tests, adjusted mutation mocks, and removed reliance on Pinia draft store.
Documentation Updates
docs/onboarding-internal-boot.md (added), docs/onboarding-internal-boot-port-differences.md (removed), web/src/components/Onboarding/UPGRADE_ONBOARDING.md (removed)
Added internal-boot and onboarding-wizard docs; removed outdated upgrade/port-differences docs.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client (Onboarding Modal)
  participant GraphQL as GraphQL API
  participant Service as OnboardingService
  participant Tracker as TrackerService (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)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 I stitched a draft, nibbled a plan,
The server now guards each saved little span.
No Pinia nest to keep me in place,
Just GraphQL routes and a durable trace.
Hop forward, brave wizard—saved with a steady pace.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: move onboarding wizard persistence server-side' clearly and concisely summarizes the main change: relocating onboarding wizard persistence from browser/client-side to server-side. It is specific, actionable, and directly reflects the primary objective of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/server-side-onboarding-wizard

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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 | 🟠 Major

Don't return from the auto-open close path before clearing wizard state.

This branch calls markCompleted() and exits before clearWizardState(), so an incomplete onboarding session keeps its saved draft / navigation / internalBootState after the user closes it. Because the new close flow also routes SAVE_FAILURE exits 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: reuse clearOnboardingDraftStorage from onboardingStorageCleanup.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 .js ESM suffix on this import.

This new TS utility is importing another TS module without the .js suffix, which is out of step with the repo's ESM convention.

Suggested fix
 import type {
   OnboardingBootMode,
   OnboardingInternalBootDevice,
   OnboardingInternalBootDraft,
   OnboardingPoolMode,
-} from '@/components/Onboarding/onboardingWizardState';
+} from '@/components/Onboarding/onboardingWizardState.js';
As per coding guidelines, `TypeScript imports use `.js` extensions 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/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 for selectedIds unless 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.

emptyWizardState is 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: Include slotCount in mutation diagnostics payload for full context.

slotCount was added to InternalBootSelection, but buildMutationVariables() 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 as SAVE_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 the plugins.selectedIds branch in this new draft-persistence case.

saveOnboardingDraft() also maps draft.plugins.selectedIds, but this test only exercises coreSettings, internalBoot, navigation, and internalBootState. 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 omit deviceName from 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 .js suffix, 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 .js extensions 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 .js suffix, 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 .js extensions 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 from props.initialDraft.

The real standalone component re-hydrates OnboardingInternalBootStep through its initialDraft prop, but this stub always clones configureDraftState.value instead. 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: Keep cloneOnboardingWizardDraft() semantics-preserving.

This helper rewrites undefined plugins.selectedIds and internalBoot.selection.devices to []. For a function named clone, 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 currentStepId and visibleStepIds, so a Boolean response forces the caller to refetch or navigate on stale assumptions. Returning OnboardingWizard or Onboarding would 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

📥 Commits

Reviewing files that changed from the base of the PR and between 160c5b1 and 56d028d.

⛔ Files ignored due to path filters (1)
  • api/src/unraid-api/cli/generated/graphql.ts is excluded by !**/generated/**
📒 Files selected for processing (50)
  • api/generated-schema.graphql
  • api/src/unraid-api/config/onboarding-tracker.model.ts
  • api/src/unraid-api/config/onboarding-tracker.service.spec.ts
  • api/src/unraid-api/config/onboarding-tracker.service.ts
  • api/src/unraid-api/graph/resolvers/customization/activation-code.model.ts
  • api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts
  • api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts
  • api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts
  • api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts
  • api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts
  • api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts
  • api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts
  • docs/onboarding-internal-boot-port-differences.md
  • docs/onboarding-internal-boot.md
  • web/__test__/components/Onboarding/OnboardingBootConfigurationSummary.test.ts
  • web/__test__/components/Onboarding/OnboardingCoreSettingsStep.test.ts
  • web/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.ts
  • web/__test__/components/Onboarding/OnboardingInternalBootStep.test.ts
  • web/__test__/components/Onboarding/OnboardingModal.test.ts
  • web/__test__/components/Onboarding/OnboardingNextStepsStep.test.ts
  • web/__test__/components/Onboarding/OnboardingPluginsStep.test.ts
  • web/__test__/components/Onboarding/OnboardingSummaryStep.test.ts
  • web/__test__/components/Onboarding/onboardingStorageCleanup.test.ts
  • web/__test__/store/onboardingContextData.test.ts
  • web/components.d.ts
  • web/src/components/Onboarding/ONBOARDING_WIZARD.md
  • web/src/components/Onboarding/OnboardingModal.vue
  • web/src/components/Onboarding/UPGRADE_ONBOARDING.md
  • web/src/components/Onboarding/components/bootConfigurationSummary/OnboardingBootConfigurationSummary.vue
  • web/src/components/Onboarding/components/bootConfigurationSummary/buildBootConfigurationSummaryViewModel.ts
  • web/src/components/Onboarding/composables/internalBoot.ts
  • web/src/components/Onboarding/graphql/closeOnboarding.mutation.ts
  • web/src/components/Onboarding/graphql/onboardingBootstrap.query.ts
  • web/src/components/Onboarding/graphql/saveOnboardingDraft.mutation.ts
  • web/src/components/Onboarding/onboardingWizardState.ts
  • web/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vue
  • web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue
  • web/src/components/Onboarding/steps/OnboardingCoreSettingsStep.vue
  • web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue
  • web/src/components/Onboarding/steps/OnboardingLicenseStep.vue
  • web/src/components/Onboarding/steps/OnboardingNextStepsStep.vue
  • web/src/components/Onboarding/steps/OnboardingOverviewStep.vue
  • web/src/components/Onboarding/steps/OnboardingPluginsStep.vue
  • web/src/components/Onboarding/steps/OnboardingSummaryStep.vue
  • web/src/components/Onboarding/store/onboardingContextData.ts
  • web/src/components/Onboarding/store/onboardingDraft.ts
  • web/src/components/Onboarding/store/onboardingModalVisibility.ts
  • web/src/components/Onboarding/store/onboardingStorageCleanup.ts
  • web/src/composables/gql/gql.ts
  • web/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

Comment on lines +1043 to +1047
type OnboardingWizardInternalBootDevice {
id: String!
sizeBytes: BigInt!
deviceName: String!
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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.graphql

Repository: 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 -50

Repository: 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 -80

Repository: 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 -80

Repository: 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.ts

Repository: 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 -150

Repository: 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 1

Repository: 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 -20

Repository: 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 -100

Repository: 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 -30

Repository: 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.ts

Repository: 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 -100

Repository: 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 10

Repository: 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 -100

Repository: 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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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 +105 to +141
it('clears wizard state while preserving completion metadata', async () => {
const config = createConfigService();
const overrides = new OnboardingOverrideService();

mockReadFile.mockImplementation(async (filePath) => {
if (String(filePath).includes('unraid-version')) {
return 'version="7.2.0"\n';
}

return JSON.stringify({
completed: true,
completedAtVersion: '7.1.0',
forceOpen: true,
draft: {
coreSettings: {
serverName: 'Tower',
},
},
navigation: {
currentStepId: 'SUMMARY',
},
internalBootState: {
applyAttempted: true,
applySucceeded: true,
},
});
});

const tracker = new OnboardingTrackerService(config, overrides);
await tracker.onApplicationBootstrap();

await expect(tracker.clearWizardState()).resolves.toEqual({
completed: true,
completedAtVersion: '7.1.0',
forceOpen: true,
...createEmptyWizardState(),
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Assert that clearWizardState() rewrites the tracker file.

This test only checks the returned in-memory state. If clearWizardState() stops persisting and only updates the cache, the test still passes even though the durable cleanup path broke.

Suggested assertion addition
         await expect(tracker.clearWizardState()).resolves.toEqual({
             completed: true,
             completedAtVersion: '7.1.0',
             forceOpen: true,
             ...createEmptyWizardState(),
         });
+        expect(mockAtomicWriteFile).toHaveBeenCalledTimes(1);
+        expect(JSON.parse(String(mockAtomicWriteFile.mock.calls[0]?.[1]))).toMatchObject({
+            completed: true,
+            completedAtVersion: '7.1.0',
+            forceOpen: true,
+            ...createEmptyWizardState(),
+        });
📝 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
it('clears wizard state while preserving completion metadata', async () => {
const config = createConfigService();
const overrides = new OnboardingOverrideService();
mockReadFile.mockImplementation(async (filePath) => {
if (String(filePath).includes('unraid-version')) {
return 'version="7.2.0"\n';
}
return JSON.stringify({
completed: true,
completedAtVersion: '7.1.0',
forceOpen: true,
draft: {
coreSettings: {
serverName: 'Tower',
},
},
navigation: {
currentStepId: 'SUMMARY',
},
internalBootState: {
applyAttempted: true,
applySucceeded: true,
},
});
});
const tracker = new OnboardingTrackerService(config, overrides);
await tracker.onApplicationBootstrap();
await expect(tracker.clearWizardState()).resolves.toEqual({
completed: true,
completedAtVersion: '7.1.0',
forceOpen: true,
...createEmptyWizardState(),
});
it('clears wizard state while preserving completion metadata', async () => {
const config = createConfigService();
const overrides = new OnboardingOverrideService();
mockReadFile.mockImplementation(async (filePath) => {
if (String(filePath).includes('unraid-version')) {
return 'version="7.2.0"\n';
}
return JSON.stringify({
completed: true,
completedAtVersion: '7.1.0',
forceOpen: true,
draft: {
coreSettings: {
serverName: 'Tower',
},
},
navigation: {
currentStepId: 'SUMMARY',
},
internalBootState: {
applyAttempted: true,
applySucceeded: true,
},
});
});
const tracker = new OnboardingTrackerService(config, overrides);
await tracker.onApplicationBootstrap();
await expect(tracker.clearWizardState()).resolves.toEqual({
completed: true,
completedAtVersion: '7.1.0',
forceOpen: true,
...createEmptyWizardState(),
});
expect(mockAtomicWriteFile).toHaveBeenCalledTimes(1);
expect(JSON.parse(String(mockAtomicWriteFile.mock.calls[0]?.[1]))).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 - 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.

Comment on lines +499 to +513
async clearWizardState(): Promise<PublicTrackerState> {
const overrideState = this.onboardingOverrides.getState();
if (overrideState?.onboarding !== undefined) {
this.state = {
...this.state,
draft: {},
navigation: {},
internalBootState: {
applyAttempted: false,
applySucceeded: false,
},
};

return this.getCachedState();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make draft mutations operate on the persisted tracker state, not the override view.

saveDraft() currently builds updatedState from getStateResult(), which is the override-composed view when OnboardingOverrideService is active. That lets temporary completed / forceOpen overrides get written back to onboarding-tracker.json, while clearWizardState() does the opposite and only clears this.state in memory in the override branch. The other mutators already keep overrides non-persistent; these two should read/write the persisted base state and apply overrides only when returning the public view.

Also applies to: 539-568

🤖 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, 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.

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();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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 +111 to 114
if (props.initialDraft) {
serverName.value = props.initialDraft.serverName ?? '';
serverDescription.value = props.initialDraft.serverDescription ?? '';
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Apply initialDraft field-by-field, not as an all-or-nothing source.

A truthy initialDraft here can mean {} or a partially populated draft from bootstrap. These branches then bypass the activation/API precedence entirely, so any missing field falls back to '' or the trusted default and can be saved back incorrectly on the next transition.

Suggested direction
 const applyPreferredIdentity = (data?: CoreSettingsIdentityData | null) => {
-  if (props.initialDraft) {
-    serverName.value = props.initialDraft.serverName ?? '';
-    serverDescription.value = props.initialDraft.serverDescription ?? '';
-    return;
-  }
-
   const activationSystem = data?.customization?.activationCode?.system;
   ...
-  serverName.value = apiServerName || activationServerName || '';
-  serverDescription.value = apiServerComment || (hasActivationComment ? activationComment : '');
+  const resolvedServerName = apiServerName || activationServerName || '';
+  const resolvedServerDescription =
+    apiServerComment || (hasActivationComment ? activationComment : '');
+
+  serverName.value = props.initialDraft?.serverName ?? resolvedServerName;
+  serverDescription.value =
+    props.initialDraft?.serverDescription ?? resolvedServerDescription;
 };

Apply the same field-level merge to timezone/theme/language/useSsh in onCoreSettingsResult and in the props.initialDraft watcher.

Also applies to: 191-198, 227-241

🤖 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 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.

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);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Comment on lines 742 to 744
poolName.value =
draftSelection?.poolName ||
(poolMode.value === 'dedicated' ? 'boot' : (data.poolNameDefault ?? 'cache'));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve an intentionally blank pool name on first hydrate.

Using || here treats '' as missing, so a saved hybrid draft with an empty poolName comes back as the default name on bootstrap. That mutates the server-owned draft before the user finishes the field.

Suggested fix
   poolName.value =
-    draftSelection?.poolName ||
+    draftSelection?.poolName ??
     (poolMode.value === 'dedicated' ? 'boot' : (data.poolNameDefault ?? 'cache'));
📝 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
poolName.value =
draftSelection?.poolName ||
(poolMode.value === 'dedicated' ? 'boot' : (data.poolNameDefault ?? 'cache'));
poolName.value =
draftSelection?.poolName ??
(poolMode.value === 'dedicated' ? 'boot' : (data.poolNameDefault ?? 'cache'));
🤖 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 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.

Comment on lines +73 to +76
const buildSelectedPluginsFromDraft = (draft?: OnboardingPluginsDraft | null) => {
const selectedIds = draft?.selectedIds ?? [];
return selectedIds.length > 0 ? new Set(selectedIds) : new Set(defaultSelectedPluginIds);
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep selectedIds: [] as “user chose no plugins.”

buildSelectedPluginsFromDraft() currently turns an explicit empty selection back into the default community-apps choice. Reopening the step after a skip silently reselects that plugin, and the next transition persists the wrong draft.

Suggested fix
 const buildSelectedPluginsFromDraft = (draft?: OnboardingPluginsDraft | null) => {
-  const selectedIds = draft?.selectedIds ?? [];
-  return selectedIds.length > 0 ? new Set(selectedIds) : new Set(defaultSelectedPluginIds);
+  if (!draft) {
+    return new Set(defaultSelectedPluginIds);
+  }
+
+  if (draft.selectedIds === undefined) {
+    return new Set(defaultSelectedPluginIds);
+  }
+
+  return new Set(draft.selectedIds);
 };

Also applies to: 137-142

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/Onboarding/steps/OnboardingPluginsStep.vue` around lines
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.

Comment on lines +486 to 488
const bootConfigurationInvalidMessage =
'This boot configuration is incomplete. Go back to Configure Boot to review it before applying changes.';
const applyReadinessTimedOut = ref(false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Localize the new invalid-configuration banner.

This is the only new summary warning that bypasses vue-i18n, so non-English onboarding sessions will see an untranslated message here.

Suggested fix
-const bootConfigurationInvalidMessage =
-  'This boot configuration is incomplete. Go back to Configure Boot to review it before applying changes.';
+const bootConfigurationInvalidMessage = computed(() =>
+  t('onboarding.summaryStep.bootConfig.invalid')
+);
📝 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 bootConfigurationInvalidMessage =
'This boot configuration is incomplete. Go back to Configure Boot to review it before applying changes.';
const applyReadinessTimedOut = ref(false);
const bootConfigurationInvalidMessage = computed(() =>
t('onboarding.summaryStep.bootConfig.invalid')
);
const applyReadinessTimedOut = ref(false);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/Onboarding/steps/OnboardingSummaryStep.vue` around lines
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.

- 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.
Comment on lines +21 to +25
import {
OnboardingWizardBootMode,
OnboardingWizardPoolMode,
OnboardingWizardStepId,
} from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js';

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused imports OnboardingWizardBootMode, OnboardingWizardPoolMode.

Copilot Autofix

AI about 11 hours ago

To fix unused imports, remove only the specific symbols that are not used anywhere in the file, keeping the rest of the import statement intact. This eliminates dead code and makes the file clearer without changing runtime behavior (since unused TypeScript type imports are erased at compile time).

In this case, the best fix is to edit api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts and update the import from @app/unraid-api/graph/resolvers/customization/activation-code.model.js so that it only imports OnboardingWizardStepId. Specifically, on lines 21–25, remove OnboardingWizardBootMode and OnboardingWizardPoolMode from the named import list, leaving OnboardingWizardStepId as the sole imported symbol. No other code changes, methods, or additional imports are required.

Suggested changeset 1
api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts b/api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts
--- a/api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts
+++ b/api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts
@@ -19,8 +19,6 @@
 
 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 { Disk } from '@app/unraid-api/graph/resolvers/disks/disks.model.js';
EOF
@@ -19,8 +19,6 @@

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 { Disk } from '@app/unraid-api/graph/resolvers/disks/disks.model.js';
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (2)
api/src/unraid-api/config/onboarding-tracker.service.ts (2)

515-533: ⚠️ Potential issue | 🟠 Major

Serialize 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, or internalBootState updates. Please guard all writeTrackerState() 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 | 🟠 Major

Mutate the persisted base state, not the override-composed view.

saveDraft() still builds updatedState from getStateResult(), so override-only completed / forceOpen values can be written into onboarding-tracker.json. clearWizardState() has the opposite problem in the override branch: it only clears this.state in 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

📥 Commits

Reviewing files that changed from the base of the PR and between 56d028d and df546ab.

⛔ Files ignored due to path filters (1)
  • api/src/unraid-api/cli/generated/graphql.ts is excluded by !**/generated/**
📒 Files selected for processing (14)
  • api/generated-schema.graphql
  • api/src/unraid-api/config/onboarding-tracker.service.spec.ts
  • api/src/unraid-api/config/onboarding-tracker.service.ts
  • api/src/unraid-api/graph/resolvers/customization/activation-code.model.ts
  • api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts
  • api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts
  • api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts
  • web/__test__/components/Onboarding/OnboardingModal.test.ts
  • web/__test__/components/Onboarding/onboardingWizardState.test.ts
  • web/src/components/Onboarding/OnboardingModal.vue
  • web/src/components/Onboarding/graphql/onboardingBootstrap.query.ts
  • web/src/components/Onboarding/onboardingWizardState.ts
  • web/src/composables/gql/gql.ts
  • web/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

Comment on lines +154 to +163
const parsedSlotCount = Number(candidate.slotCount);
const parsedBootSize = Number(candidate.bootSizeMiB);

return {
poolName: normalizeString(candidate.poolName),
slotCount: Number.isFinite(parsedSlotCount)
? Math.max(1, Math.min(2, parsedSlotCount))
: undefined,
devices: normalizeBootDeviceArray(candidate.devices),
bootSizeMiB: Number.isFinite(parsedBootSize) ? Math.max(0, parsedBootSize) : undefined,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Cap devices when slotCount is clamped.

slotCount is normalized to at most 2 here, but devices still keeps every valid entry. That can persist an internally inconsistent selection like slotCount: 2 with 3+ devices.

Suggested fix
 const normalizeInternalBootSelection = (value: unknown): OnboardingInternalBootSelection | null => {
     if (value === null) {
         return null;
     }

     if (!value || typeof value !== 'object') {
         return null;
     }

     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 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.

Comment on lines 312 to 316
return {
...baseState,
completed: overrideState.onboarding.completed ?? false,
completedAtVersion: overrideState.onboarding.completedAtVersion ?? undefined,
forceOpen: overrideState.onboarding.forceOpen ?? false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve base completion fields when an override omits them.

A partial override that only sets forceOpen currently forces completed to false and clears completedAtVersion because the fallback here is hardcoded defaults instead of baseState.

Suggested 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 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.

@@ -0,0 +1,63 @@
import { describe, expect, it } from 'vitest';

import { normalizeOnboardingWizardDraft } from '~/components/Onboarding/onboardingWizardState';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 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 -20

Repository: 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 -i

Repository: 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 -30

Repository: 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 -5

Repository: unraid/api

Length of output: 125


🏁 Script executed:

# Look for tsconfig or build configuration
find . -name "tsconfig.json" -o -name "vite.config.*" | head -5

Repository: unraid/api

Length of output: 248


🏁 Script executed:

# Check if there are any other test files that import from onboardingWizardState
rg "from.*onboardingWizardState" -B0 -A0

Repository: unraid/api

Length of output: 1962


🏁 Script executed:

# Check the ESLint configuration for import rules
cat -n ./web/eslint.config.mjs | head -100

Repository: unraid/api

Length of output: 3306


🏁 Script executed:

# Check the Nuxt config to understand module resolution
cat -n ./web/nuxt.config.ts | head -100

Repository: unraid/api

Length of output: 106


🏁 Script executed:

# Find nuxt config with different extensions
find . -maxdepth 2 -name "nuxt.config.*" 2>/dev/null

Repository: 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=20

Repository: 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 -40

Repository: 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 -20

Repository: 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 -30

Repository: 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.ts

Repository: 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.

Comment on lines +78 to +79
const normalizeStringArray = (value: unknown): string[] =>
Array.isArray(value) ? value.filter((item): item is string => typeof item === 'string') : [];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve undefined vs explicit-empty draft state.

These helpers currently manufacture values that were never present: missing selectedIds/devices become [], and malformed internalBoot.selection becomes null. The draft types explicitly distinguish omitted state from explicit empty state, so a normalize/clone round-trip can turn “no saved value” into something that later gets persisted back through saveOnboardingDraft.

Also applies to: 108-113, 115-136, 159-165, 183-205

🤖 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 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.

Comment on lines +124 to +132
const candidate = value as Record<string, unknown>;
const parsedSlotCount = Number(candidate.slotCount);
const parsedBootSizeMiB = Number(candidate.bootSizeMiB);

return {
poolName: normalizeString(candidate.poolName),
slotCount: Number.isFinite(parsedSlotCount) ? parsedSlotCount : undefined,
devices: normalizeInternalBootDevices(candidate.devices),
bootSizeMiB: Number.isFinite(parsedBootSizeMiB) ? parsedBootSizeMiB : undefined,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Reject non-positive or fractional internal-boot numbers.

slotCount and bootSizeMiB currently accept any finite number, so malformed tracker data like -1 or 1.5 survives normalization and can be written back out. Restrict both to positive integers here.

Suggested fix
-    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 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).

- 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.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2026

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR1984/dynamix.unraid.net.plg

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

❌ Patch coverage is 80.45482% with 318 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.47%. Comparing base (6897f7e) to head (91336a3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
web/src/components/Onboarding/OnboardingModal.vue 71.57% 81 Missing ⚠️
...ts/Onboarding/steps/OnboardingInternalBootStep.vue 27.10% 78 Missing ⚠️
...rc/unraid-api/config/onboarding-tracker.service.ts 82.84% 47 Missing ⚠️
...g/standalone/OnboardingInternalBoot.standalone.vue 85.79% 26 Missing ⚠️
...raph/resolvers/customization/onboarding.service.ts 82.10% 17 Missing ⚠️
...ts/Onboarding/steps/OnboardingCoreSettingsStep.vue 70.90% 16 Missing ⚠️
...h/resolvers/customization/activation-code.model.ts 71.42% 10 Missing ⚠️
...nSummary/buildBootConfigurationSummaryViewModel.ts 92.80% 10 Missing ⚠️
...src/components/Onboarding/onboardingWizardState.ts 92.96% 9 Missing ⚠️
...ponents/Onboarding/steps/OnboardingPluginsStep.vue 81.08% 7 Missing ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1984      +/-   ##
==========================================
+ Coverage   52.21%   52.47%   +0.26%     
==========================================
  Files        1032     1035       +3     
  Lines       71646    72541     +895     
  Branches     8147     8376     +229     
==========================================
+ Hits        37410    38067     +657     
- Misses      34111    34351     +240     
+ Partials      125      123       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants