Skip to content

fix: check mdState to determine array stopped during onboarding#1969

Merged
elibosley merged 3 commits intomainfrom
feat/onboarding-use-history
Mar 27, 2026
Merged

fix: check mdState to determine array stopped during onboarding#1969
elibosley merged 3 commits intomainfrom
feat/onboarding-use-history

Conversation

@elibosley
Copy link
Copy Markdown
Member

@elibosley elibosley commented Mar 27, 2026

Summary

  • Fix onboarding wizard "array must be stopped" false positive during internal boot. The UpdateServerIdentity mutation was checking only fsState (filesystem state) to determine if the array was stopped. During internal boot, the boot pool keeps fsState as Started even when the disk array (mdState) is STOPPED. Now checks mdState === ArrayState.STOPPED as the primary indicator, falling back to fsState === 'Stopped' for backward compatibility.
  • Add regression test for the internal boot scenario: mdState=STOPPED + fsState=Started should allow server name changes.
  • Add mdState to existing test mock so the "array running" test explicitly asserts against mdState=STARTED.

Context

Bug report: "Onboarding Wizard: says array is running when it isn't" — tested on 7.3.0-beta.1.2 with Connect 2026.03.25.2146. After booting into internal boot, the onboarding wizard incorrectly blocked server name changes because the boot pool kept fsState as Started even though the array was stopped.

Root cause: The boot pool (ZFS pool used for internal boot) runs its own filesystem layer, keeping fsState as Started. The actual disk array state is tracked by mdState, which correctly shows STOPPED.

Test Coverage

CODE PATH COVERAGE
===========================
[+] server.service.ts — updateServerIdentity() array check
    ├── [★★★ TESTED] mdState=STARTED, name changed → throws
    ├── [★★★ TESTED] mdState=STOPPED, fsState=Started → allows (internal boot fix)
    ├── [★★★ TESTED] name unchanged → skips check
    └── [IMPLICIT]    fsState=Stopped fallback → covered by existing tests

COVERAGE: 4/4 paths (100%)

Pre-Landing Review

No issues found.

Test plan

  • All API tests pass (174 files, 1966 tests)
  • All Web tests pass (64 files, 618 tests)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved server name change validation to correctly identify when an array is stopped, allowing name updates in more scenarios including during filesystem initialization.

…istory

# Conflicts:
#	web/__test__/components/Onboarding/OnboardingInternalBootStandalone.test.ts
#	web/__test__/components/Onboarding/OnboardingSummaryStep.test.ts
#	web/src/components/Onboarding/OnboardingModal.vue
#	web/src/components/Onboarding/standalone/OnboardingInternalBoot.standalone.vue
The onboarding wizard was incorrectly reporting "array must be stopped"
during internal boot because it only checked fsState (filesystem state).
During internal boot, the boot pool keeps fsState as 'Started' even when
the array (mdState) is actually STOPPED. Now checks mdState === STOPPED
as the primary indicator, falling back to fsState === 'Stopped'.

Fixes: Onboarding Wizard says array is running when it isn't
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 27, 2026

Walkthrough

The server identity update validation logic now checks both mdState and fsState to determine if the array is stopped before permitting name changes. A combined condition accounts for internal boot scenarios where mdState is 'STOPPED' but fsState is not 'Stopped'.

Changes

Cohort / File(s) Summary
Server Identity State Validation
api/src/unraid-api/graph/resolvers/servers/server.service.ts, api/src/unraid-api/graph/resolvers/servers/server.service.spec.ts
Updated updateServerIdentity to check mdState in addition to fsState when validating stopped state for name changes. Introduced arrayStopped boolean combining both conditions. Added test case verifying name changes are permitted when mdState is 'STOPPED' regardless of fsState. Extended existing test mock with mdState: 'STARTED' to validate updated decision logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A hop and a bound, a bug I have found!
When mdState is STOPPED, no more running around,
The name can now change with a conditional cheer,
Both fsState and mdState in sync, crystal clear!

🚥 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 accurately summarizes the main change: checking mdState to determine if the array is stopped during onboarding, which fixes the false positive blocking server name changes.
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 feat/onboarding-use-history

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

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8236be82f7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const mdState = currentEmhttp.var?.mdState;
const fsState = currentEmhttp.var?.fsState;
if (fsState !== 'Stopped') {
const arrayStopped = mdState === ArrayState.STOPPED || fsState === 'Stopped';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Gate rename on mdState before fsState fallback

The new arrayStopped check uses mdState === STOPPED || fsState === 'Stopped', which means a stale fsState value can still allow renaming even when mdState indicates the array is running. Since this change makes mdState the primary signal, fsState should only be consulted when mdState is unavailable; otherwise the guard can be bypassed in mixed-state conditions and the mutation may permit name changes while the array is effectively started.

Useful? React with 👍 / 👎.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.05%. Comparing base (afe1ae6) to head (8236be8).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1969      +/-   ##
==========================================
+ Coverage   52.03%   52.05%   +0.02%     
==========================================
  Files        1030     1030              
  Lines       71349    71352       +3     
  Branches     8057     8062       +5     
==========================================
+ Hits        37125    37145      +20     
+ Misses      34100    34083      -17     
  Partials      124      124              

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

@github-actions
Copy link
Copy Markdown
Contributor

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/PR1969/dynamix.unraid.net.plg

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.

🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/servers/server.service.spec.ts (1)

114-114: Prefer a typed fixture over double casting in this mock return.

as unknown as ReturnType<...> weakens type safety in this test path. A typed fixture/helper would keep this strict without double casts.

As per coding guidelines, "Avoid using casting whenever possible, prefer proper typing from the start".

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

In `@api/src/unraid-api/graph/resolvers/servers/server.service.spec.ts` at line
114, Replace the double-cast "as unknown as ReturnType<typeof getters.emhttp>"
with a properly typed fixture: define a constant (e.g., emhttpFixture) typed as
ReturnType<typeof getters.emhttp> and populate its fields to match the mocked
shape, then return that fixture from the mock instead of casting; update the
test file's mock return where the cast is used and reference the getter function
name getters.emhttp to ensure the fixture type aligns exactly with the expected
return type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@api/src/unraid-api/graph/resolvers/servers/server.service.spec.ts`:
- Line 114: Replace the double-cast "as unknown as ReturnType<typeof
getters.emhttp>" with a properly typed fixture: define a constant (e.g.,
emhttpFixture) typed as ReturnType<typeof getters.emhttp> and populate its
fields to match the mocked shape, then return that fixture from the mock instead
of casting; update the test file's mock return where the cast is used and
reference the getter function name getters.emhttp to ensure the fixture type
aligns exactly with the expected return type.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0bb03a15-0d96-43b1-ba1e-05118f24db02

📥 Commits

Reviewing files that changed from the base of the PR and between afe1ae6 and 8236be8.

📒 Files selected for processing (2)
  • api/src/unraid-api/graph/resolvers/servers/server.service.spec.ts
  • api/src/unraid-api/graph/resolvers/servers/server.service.ts

@elibosley elibosley merged commit abe7283 into main Mar 27, 2026
13 checks passed
@elibosley elibosley deleted the feat/onboarding-use-history branch March 27, 2026 00:47
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.

1 participant