-
Notifications
You must be signed in to change notification settings - Fork 11
feat: add unraid api status manager #1708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds an Unraid API Status page and Vue component that can check and restart the Unraid API via a new backend “status” command. Updates PHP handler to support status and refine restart responses. Registers the component for lazy load, extends TypeScript typings and service payloads, tweaks page titles, and removes a CSS variable. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant B as Browser (Page)
participant C as ApiStatus.vue
participant S as WebguiUnraidApiCommand
participant P as PHP handler (unraid-api.php)
participant R as rc.unraid-api script
U->>B: Open "Unraid API Status" page
B->>C: Mount component
C->>S: command=status, csrf_token
S->>P: HTTP request (status)
P->>R: unraid-api status
R-->>P: status output
P-->>S: JSON { result, success }
S-->>C: Response
C-->>U: Show status (Running/Not Running)
alt User clicks Restart
U->>C: Click "Restart API" (confirm)
C->>S: command=restart, csrf_token
S->>P: HTTP request (restart)
P->>R: /etc/rc.d/rc.unraid-api restart
R-->>P: restart output + exit code
P-->>S: JSON { success, result, error? }
S-->>C: Response
C-->>C: Delay 3s, re-check status
C-->>U: Show restart message and updated status
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (3)
Comment |
|
Claude finished @elibosley's task —— View job Code Review - PR #1708 ChangesReviewing only the diff/changeset for critical issues: Todo List
Critical Issues Found🔴 SECURITY ISSUE - Line 72 in unraid-api.php: exec('/etc/rc.d/rc.unraid-api restart 2>&1', $output, $retval);The command path 🔴 POTENTIAL LOGIC ERROR - Lines 27-30 in ApiStatus.vue: isRunning.value =
response.result.includes('running') ||
response.result.includes('active') ||
response.result.includes('status : online');Simple string matching for status detection is fragile and could cause false positives. For example, a response like "not running" would still match "running" and incorrectly show as active. 🔴 DATA INTEGRITY ISSUE - Line 74 in unraid-api.php: response_complete(200, array('success' => ($retval === 0), 'result' => $output, 'error' => ($retval !== 0 ? $output : null)), $output);Always returns HTTP 200 even on failure. Clients cannot distinguish between success/failure using HTTP status codes, potentially masking critical restart failures. Minor Security Observations
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (8)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/include/unraid-api.php (1)
72-79: Return 500 on restart failure and consider absolute path for status.
- Minor: Use HTTP 500 when
$retval !== 0so clients can rely on status codes.- Optional: For consistency with restart, consider
/etc/rc.d/rc.unraid-api status 2>&1to avoid PATH issues.Apply this diff:
case 'restart': - exec('/etc/rc.d/rc.unraid-api restart 2>&1', $output, $retval); - $output = implode(PHP_EOL, $output); - response_complete(200, array('success' => ($retval === 0), 'result' => $output, 'error' => ($retval !== 0 ? $output : null)), $output); - break; + exec('/etc/rc.d/rc.unraid-api restart 2>&1', $output, $retval); + $output = implode(PHP_EOL, $output); + if ($retval === 0) { + response_complete(200, array('success' => true, 'result' => $output, 'error' => null), $output); + } else { + response_complete(500, array('success' => false, 'result' => $output, 'error' => $output), $output); + } + break; case 'status': - exec('unraid-api status 2>&1', $output, $retval); + exec('unraid-api status 2>&1', $output, $retval);web/src/components/ApiStatus/ApiStatus.vue (4)
22-26: Harden running-state detection to avoid false positives (e.g., "not running").Use a case-insensitive regex that excludes negatives.
Apply this diff:
- const data = await response.json(); - if (data.result) { - apiStatus.value = data.result; - isRunning.value = data.result.includes('running') || data.result.includes('active'); - } + type ApiResponse = { result?: string; success?: boolean; error?: string | null }; + const data: ApiResponse = await response.json(); + const text = String(data.result ?? '').toLowerCase(); + apiStatus.value = data.result ?? ''; + isRunning.value = + /\b(active|running)\b/.test(text) && + !/\b(inactive|not\s*running|stopped|dead)\b/.test(text);
55-63: Propagate backend error details to the UI toast.Prefer a specific message when provided.
Apply this diff:
- if (data.success) { + if (data.success) { toast.success('API service restart initiated. Please wait a few seconds.'); setTimeout(() => { checkStatus(); }, 3000); } else { - toast.error(data.error || 'Failed to restart API service'); + toast.error(data?.error ?? 'Failed to restart API service'); }
64-69: Type-safe error logging in catch blocks.Avoid assuming
errorhas.message.Apply this diff:
-} catch (error) { - console.error('Failed to restart API:', error); - toast.error('Failed to restart API service'); +} catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Failed to restart API:', msg); + toast.error('Failed to restart API service');
72-74: Auto-refresh on mount is good. Consider initial toast on failure (optional).api/src/unraid-api/graph/resolvers/api-management/api-management.resolver.ts (3)
25-31: Add timeout and harden status parsing.
- Prevent long hangs with a timeout.
- Avoid "not running" false positives.
Apply this diff:
- const { stdout } = await execa('unraid-api', ['status'], { shell: 'bash' }); - return { - status: stdout, - isRunning: stdout.includes('running') || stdout.includes('active'), - timestamp: new Date().toISOString(), - }; + const { stdout } = await execa('unraid-api', ['status'], { shell: 'bash', timeout: 10_000 }); + const text = stdout.toLowerCase(); + return { + status: stdout, + isRunning: /\b(active|running)\b/.test(text) && !/\b(inactive|not\s*running|stopped|dead)\b/.test(text), + timestamp: new Date().toISOString(), + };
31-38: Type-safe catch and consistent error message extraction.Avoid using
.messageon unknown.Apply this diff:
- } catch (error) { - this.logger.error('Failed to get API status:', error); + } catch (error: unknown) { + const err = error instanceof Error ? error : new Error(String(error)); + this.logger.error('Failed to get API status:', err); return { - status: `Error: ${error.message}`, + status: `Error: ${err.message}`, isRunning: false, timestamp: new Date().toISOString(), };
61-69: Same here: type-safe catch and error field alignment with model (string | null).Ensure
errorisstring | null.Apply this diff:
- } catch (error) { - this.logger.error('Failed to restart API:', error); + } catch (error: unknown) { + const err = error instanceof Error ? error : new Error(String(error)); + this.logger.error('Failed to restart API:', err); return { success: false, message: 'Failed to restart API service', - error: error.message, + error: err.message, timestamp: new Date().toISOString(), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
api/src/unraid-api/graph/resolvers/api-management/api-management.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-management/api-management.resolver.ts(1 hunks)plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page(1 hunks)plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/include/unraid-api.php(2 hunks)web/src/components/ApiStatus/ApiStatus.standalone.vue(1 hunks)web/src/components/ApiStatus/ApiStatus.vue(1 hunks)web/src/components/Wrapper/component-registry.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
api/src/unraid-api/**
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/graph/resolvers/api-management/api-management.model.tsapi/src/unraid-api/graph/resolvers/api-management/api-management.resolver.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
api/src/unraid-api/graph/resolvers/api-management/api-management.model.tsweb/src/components/Wrapper/component-registry.tsapi/src/unraid-api/graph/resolvers/api-management/api-management.resolver.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 TTL values must be in milliseconds (e.g., 600000 for 10 minutes)
Files:
api/src/unraid-api/graph/resolvers/api-management/api-management.model.tsapi/src/unraid-api/graph/resolvers/api-management/api-management.resolver.ts
**/components/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests
Files:
web/src/components/ApiStatus/ApiStatus.vueweb/src/components/ApiStatus/ApiStatus.standalone.vue
🧠 Learnings (5)
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
api/src/unraid-api/graph/resolvers/api-management/api-management.model.ts
📚 Learning: 2025-01-30T19:38:02.478Z
Learnt from: pujitm
PR: unraid/api#1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.
Applied to files:
web/src/components/Wrapper/component-registry.ts
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
web/src/components/Wrapper/component-registry.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
api/src/unraid-api/graph/resolvers/api-management/api-management.resolver.ts
📚 Learning: 2025-03-14T16:10:38.562Z
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Applied to files:
api/src/unraid-api/graph/resolvers/api-management/api-management.resolver.ts
🧬 Code graph analysis (2)
api/src/unraid-api/graph/resolvers/api-management/api-management.resolver.ts (1)
packages/unraid-shared/src/use-permissions.directive.ts (1)
UsePermissions(81-128)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/include/unraid-api.php (2)
api/src/unraid-api/cli/apikey/api-key.command.ts (1)
output(113-119)plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/include/UpdateFlashBackup.php (1)
response_complete(44-62)
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/graph/resolvers/api-management/api-management.model.ts
[error] 6-6: tsc --noEmit: TS2564: Property 'status' has no initializer and is not definitely assigned in the constructor.
web/src/components/ApiStatus/ApiStatus.vue
[error] 4-4: vue-tsc --noEmit failed: Cannot find module 'sonner' or its corresponding type declarations.
🔇 Additional comments (4)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/include/unraid-api.php (1)
42-45: Command added to allow status — looks good.plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page (1)
604-606: Mount point for API Status Manager — looks correct.Placed after connect settings; selector matches registry mapping.
web/src/components/ApiStatus/ApiStatus.standalone.vue (1)
1-5: Thin re-export wrapper is fine.No issues.
web/src/components/Wrapper/component-registry.ts (1)
144-148: Registry entry LGTM — appId unique and selector present.
Single match for appId 'api-status-manager' at web/src/components/Wrapper/component-registry.ts:147; found in plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page:605.
api/src/unraid-api/graph/resolvers/api-management/api-management.model.ts
Outdated
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/api-management/api-management.model.ts
Outdated
Show resolved
Hide resolved
This commit deletes the `api-management.model.ts` and `api-management.resolver.ts` files, which are no longer needed. The API status management functionality has been updated and is now handled directly in the frontend components. This change simplifies the codebase and improves maintainability.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1708 +/- ##
==========================================
- Coverage 52.60% 52.53% -0.07%
==========================================
Files 854 857 +3
Lines 47864 48044 +180
Branches 4749 4764 +15
==========================================
+ Hits 25177 25241 +64
- Misses 22618 22734 +116
Partials 69 69 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/auto-imports.d.ts(1 hunks)web/components.d.ts(2 hunks)web/src/components/ApiStatus/ApiStatus.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/components/ApiStatus/ApiStatus.vue
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
web/auto-imports.d.tsweb/components.d.ts
🧠 Learnings (6)
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Do not rely on Nuxt auto-imports in tests; import required Vue utilities explicitly in test files
Applied to files:
web/auto-imports.d.tsweb/components.d.ts
📚 Learning: 2024-12-09T15:47:29.325Z
Learnt from: pujitm
PR: unraid/api#975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Applied to files:
web/components.d.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/components/**/*.vue : Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests
Applied to files:
web/components.d.ts
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
web/components.d.ts
📚 Learning: 2024-12-17T13:55:42.068Z
Learnt from: elibosley
PR: unraid/api#972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Applied to files:
web/components.d.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/{store,stores}/**/*.ts : In store files, explicitly import Vue reactivity utilities (e.g., computed, ref, watchEffect); do not rely on Nuxt auto-imports
Applied to files:
web/components.d.ts
🔇 Additional comments (3)
web/components.d.ts (2)
19-20: LGTM — ApiStatus components registered in GlobalComponents.
web/src/components/ApiStatus/{ApiStatus.vue, ApiStatus.standalone.vue} exist; web/src/components/Wrapper/component-registry.ts registers ApiStatus.standalone via defineAsyncComponent (line 145).
108-118: Nuxt UI component types reference pnpm store paths — regeneration couldn't be verifiedjq on the repo root returned null for @nuxt/ui and unplugin-* (likely declared in workspace package.json); running pnpm install/build in the sandbox failed (node-gyp/Python errors for @unraid/libvirt and vite: not found), so I couldn't reproduce deterministic regeneration of web/components.d.ts.
- Action: Pin @nuxt/ui and unplugin-vue-components / unplugin-auto-import to exact versions (no ^/~) or change generation to use stable module specifiers.
- Add a CI job that regenerates and commits web/components.d.ts and web/auto-imports.d.ts (ensure CI has Python/node-gyp and vite).
- Dev checks: inspect web/package.json for those deps and run locally: pnpm -w install && pnpm -C web build && git diff -- web/components.d.ts web/auto-imports.d.ts.
web/auto-imports.d.ts (1)
9-37: Avoid committing pnpm store–hashed node_modules paths (web/auto-imports.d.ts)web/auto-imports.d.ts currently hard‑codes pnpm store paths — brittle. Use stable package specifiers (e.g. '@nuxt/ui/dist/runtime/...') via the generator, or remove this file from VCS and generate it on install/CI.
If keeping it committed, confirm a clean-clone install + typecheck succeed (run from repo root):
# locate hard-coded pnpm paths rg -nP 'node_modules/\.pnpm/.+@nuxt\+ui' web/auto-imports.d.ts || true # check where @nuxt/ui is declared (root or web package.json) and that the version is an exact pin (no ^ or ~) jq -r '.dependencies["@nuxt/ui"] // .devDependencies["@nuxt/ui"] // "not found"' package.json web/package.json # Expect: exact version (no ^/~) # from a clean clone: install and typecheck the web package cd web pnpm install --frozen-lockfile pnpm exec tsc --noEmitIf any of the above fail or the version is not an exact pin, fix the generator to emit stable specifiers or stop committing this generated file.
This commit introduces a new page for the Unraid API Status and updates the title of the Connect page to "Unraid API Settings" for clarity. The new page includes a component for displaying the API status, enhancing the user interface for management access.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.23.0](v4.22.2...v4.23.0) (2025-09-16) ### Features * add unraid api status manager ([#1708](#1708)) ([1d9ce0a](1d9ce0a)) ### Bug Fixes * **logging:** remove colorized logs ([#1705](#1705)) ([1d2c670](1d2c670)) * no sizeRootFs unless queried ([#1710](#1710)) ([9714b21](9714b21)) * use virtual-modal-container ([#1709](#1709)) ([44b4d77](44b4d77)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Style
Chores