-
Notifications
You must be signed in to change notification settings - Fork 16
chore: replace graphql server #1875
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce SSO authentication integration and Unraid Core service management. Updates include new NGINX proxy endpoints for SSO and GraphQL APIs, a new Unraid Core service script with lifecycle management, installation/verification logic for core package handling, and frontend OAuth callback routing with CSRF token centralization. Changes
Sequence DiagramssequenceDiagram
actor User
participant WebApp as Web App
participant NGINX as NGINX Proxy
participant CoreSvc as Unraid Core<br/>(rc.unraid)
participant OIDC as OIDC Callback<br/>Endpoint
User->>WebApp: Click SSO Login
WebApp->>WebApp: Set loading state
WebApp->>WebApp: Generate state token
WebApp->>NGINX: POST /auth/sso (with state)
NGINX->>CoreSvc: Proxy to unraid-core.sock
CoreSvc-->>NGINX: SSO redirect to provider
NGINX-->>WebApp: Redirect to OAuth provider
WebApp->>WebApp: User authenticates at provider
WebApp->>NGINX: Callback with code & state
NGINX-->>WebApp: Redirect to /login
WebApp->>OIDC: POST internal OIDC callback
OIDC-->>WebApp: Session established
WebApp->>WebApp: Route to authenticated page
sequenceDiagram
participant Installer as Plugin<br/>Installer
participant FileSystem as File System
participant APISvc as Unraid API<br/>(rc.unraid)
participant CoreSvc as Unraid Core<br/>(rc.unraid)
Installer->>Installer: Start installation
Installer->>FileSystem: Stop unraid-core service
Installer->>FileSystem: Remove existing core packages
Installer->>FileSystem: Download/install core package
Installer->>FileSystem: chmod +x rc.unraid
Installer->>APISvc: Start API service
APISvc-->>Installer: API started
Installer->>CoreSvc: Start core service (rc.unraid)
CoreSvc-->>Installer: Core started
Installer->>FileSystem: chmod +x K30unraid-core
Installer->>Installer: Verify critical files/dirs
Installer-->>Installer: Installation complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1875 +/- ##
==========================================
+ Coverage 46.40% 46.43% +0.02%
==========================================
Files 954 954
Lines 59791 59813 +22
Branches 5538 5541 +3
==========================================
+ Hits 27749 27772 +23
+ Misses 31923 31922 -1
Partials 119 119 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
230e0cd to
861c49d
Compare
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
🤖 Fix all issues with AI agents
In `@plugin/plugins/dynamix.unraid.net.plg`:
- Around line 333-346: Replace the broad glob "unraid-*" used when setting
core_pkg_installed with a version-specific pattern or filter to avoid matching
unrelated packages: update the ls/grep invocation that assigns
core_pkg_installed to only match names like "unraid-" followed by a digit (e.g.,
use "unraid-[0-9]*" or pipe through grep -E '^unraid-[0-9]') so
core_pkg_installed and core_pkg_basename resolve to the actual core package name
and removepkg --terse is invoked only for that package.
In `@plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid`:
- Around line 67-79: The rollback() function currently does rm -rf
/usr/local/unraid then mv /usr/local/unraid.prev /usr/local/unraid which is not
atomic and can leave the system without an installation if mv fails; change to a
safer three-step swap: rename the current dir to a temp backup (e.g., mv
/usr/local/unraid /usr/local/unraid.tmp), then mv /usr/local/unraid.prev to
/usr/local/unraid, and only after verifying that succeeds remove the temp
backup; ensure each mv is checked for failure and on any error attempt to
restore from the temp backup and return a non-zero exit code so rollback is
atomic and recoverable, updating the rollback() function and its error handling
accordingly.
🧹 Nitpick comments (5)
web/src/components/sso/useSsoAuth.ts (1)
91-93: Guard placement has no effect - consider moving to function start.This guard at the end of the try block is a no-op: if a token or error exists, the function already returned at lines 76 or 88. If neither exists, there's no code after this guard anyway.
If the intent is to restrict OAuth callback processing to the login page only, move this guard to the beginning of
handleOAuthCallback:♻️ Suggested refactor
const handleOAuthCallback = async () => { + if (window.location.pathname !== '/login') { + return; + } + try { // First check hash parameters (for token and error - keeps them out of server logs) const hashParams = new URLSearchParams(window.location.hash.slice(1)); ... - if (window.location.pathname !== '/login') { - return; - } } catch (err) {plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid (2)
25-26: Minor: Simplify command substitution.Line 26 uses an unnecessary
catpipe. Consider simplifying:export SECRET_KEY_BASE=$(cat "$CONFIG_DIR/secret_key_base") -export RELEASE_COOKIE=$(cat "$CONFIG_DIR/secret_key_base" | head -c 20) +export RELEASE_COOKIE=$(head -c 20 "$CONFIG_DIR/secret_key_base")
81-88: Unknown commands should exit with non-zero status.When an invalid command is passed, the script shows usage but exits with status 0 (implicit). Consider exiting with non-zero for unknown commands:
case "${1:-}" in start) start ;; stop) stop ;; restart) restart ;; status) status ;; rollback) rollback ;; - *) echo "Usage: $0 {start|stop|restart|status|rollback}" ;; + *) echo "Usage: $0 {start|stop|restart|status|rollback}"; exit 1 ;; esacplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh (1)
219-219: Trailing whitespace.Line 219 has trailing whitespace after
fi. Consider removing it for consistency.-fi +fiapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts (1)
98-105: Clarify why OS version check is disabled.The change to pass
{ checkOsVersion: false }removes version gating for this modification. Consider adding a brief comment explaining why this modification should apply to all versions, given the class docstring mentions "< Unraid 7.2.0".async shouldApply(): Promise<ShouldApplyWithReason> { + // Apply to all versions as core socket routing is needed regardless of OS version const { shouldApply, reason } = await super.shouldApply({ checkOsVersion: false });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/rc.nginx.modified.snapshotapi/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patchapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.tsplugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/etc/rc.d/rc.unraidplugin/source/dynamix.unraid.net/etc/rc.d/rc6.d/K30unraid-coreplugin/source/dynamix.unraid.net/install/doinst.shplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.shweb/__test__/components/SsoButton.test.tsweb/src/components/sso/useSsoAuth.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- web/test/components/SsoButton.test.ts
- plugin/source/dynamix.unraid.net/install/doinst.sh
- plugin/source/dynamix.unraid.net/etc/rc.d/rc6.d/K30unraid-core
- api/src/unraid-api/unraid-file-modifier/modifications/test/snapshots/rc.nginx.modified.snapshot
🧰 Additional context used
📓 Path-based instructions (7)
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraidweb/src/components/sso/useSsoAuth.tsplugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.shapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
web/src/components/sso/useSsoAuth.tsapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/components/sso/useSsoAuth.ts
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/components/sso/useSsoAuth.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
web/src/components/sso/useSsoAuth.tsapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
api/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer adding new files to the NestJS repo located at
api/src/unraid-api/instead of the legacy code
Files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 expects TTL values in milliseconds, not seconds (e.g., 600000 for 10 minutes, not 600)
Files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 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.
Learnt from: mdatelle
Repo: unraid/api PR: 1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
📚 Learning: 2025-06-11T14:14:30.348Z
Learnt from: pujitm
Repo: unraid/api PR: 1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
Applied to files:
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraidplugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
📚 Learning: 2025-01-27T14:31:42.305Z
Learnt from: elibosley
Repo: unraid/api PR: 1063
File: web/components/SsoButton.ce.vue:5-8
Timestamp: 2025-01-27T14:31:42.305Z
Learning: In the Unraid API web components, SSO-related props are intentionally provided in both camelCase (`ssoEnabled`) and lowercase (`ssoenabled`) variants to support interchangeable usage across different contexts (e.g., HTML attributes vs Vue props).
Applied to files:
web/src/components/sso/useSsoAuth.ts
📚 Learning: 2025-09-04T18:42:53.531Z
Learnt from: pujitm
Repo: unraid/api PR: 1658
File: plugin/plugins/dynamix.unraid.net.plg:73-79
Timestamp: 2025-09-04T18:42:53.531Z
Learning: In the dynamix.unraid.net plugin, versions 6.12.1-6.12.14 and 6.12.15 prereleases are intentionally allowed to install with warnings (rather than immediate cleanup) to provide users with a grace period and notice before functionality is completely removed. This is a deliberate UX decision to avoid immediately breaking existing setups while encouraging upgrades.
Applied to files:
plugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
📚 Learning: 2025-01-29T00:59:26.633Z
Learnt from: zackspear
Repo: unraid/api PR: 1079
File: web/scripts/deploy-dev.sh:51-54
Timestamp: 2025-01-29T00:59:26.633Z
Learning: For the Unraid web components deployment process, JS file validation isn't required in auth-request.php updates since the files come from the controlled build pipeline where we are the source.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-09-04T15:26:34.416Z
Learnt from: elibosley
Repo: unraid/api PR: 1657
File: web/scripts/deploy-dev.sh:37-41
Timestamp: 2025-09-04T15:26:34.416Z
Learning: In web/scripts/deploy-dev.sh, the command `rm -rf /usr/local/emhttp/plugins/dynamix.my.servers/unraid-components/*` intentionally removes all contents of the unraid-components directory before deploying standalone components. This broader cleanup is desired behavior according to the maintainer elibosley.
Applied to files:
plugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
📚 Learning: 2025-05-07T16:07:47.236Z
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/setup_api.sh:107-113
Timestamp: 2025-05-07T16:07:47.236Z
Learning: The Unraid API is designed to handle missing configuration files gracefully with smart internal fallbacks rather than requiring installation scripts to create default configurations.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-05-08T19:28:54.365Z
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:19-24
Timestamp: 2025-05-08T19:28:54.365Z
Learning: The directory `/usr/local/emhttp/plugins/dynamix.my.servers` is a valid directory that exists as part of the Unraid API plugin installation and should be included in verification checks.
Applied to files:
plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
📚 Learning: 2025-03-27T13:34:53.438Z
Learnt from: pujitm
Repo: unraid/api PR: 1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Applied to files:
plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
📚 Learning: 2025-01-29T16:35:43.699Z
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: In the Unraid API, FileModification implementations (apply/rollback methods) don't need to implement their own error handling as it's handled by the UnraidFileModifierService caller.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
📚 Learning: 2025-01-29T16:35:43.699Z
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations, including detailed error logging with stack traces and modification IDs. Individual FileModification implementations should focus on their core functionality without duplicating error handling.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
📚 Learning: 2025-01-29T16:36:04.777Z
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: In the Unraid API, FileModification implementations (like LogRotateModification) don't need to handle errors internally as error handling is managed at the UnraidFileModifierService level.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
📚 Learning: 2025-01-29T16:36:04.777Z
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations. It includes try/catch blocks, detailed error logging, and safe rollback mechanisms. Individual FileModification implementations (like LogRotateModification) should allow errors to propagate to this service layer rather than handling them internally.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
📚 Learning: 2025-02-03T18:57:53.577Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts:80-80
Timestamp: 2025-02-03T18:57:53.577Z
Learning: In the unraid/api codebase, patch files generated for file modifications always use the .patch extension, regardless of the source file type being modified.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to api/**/* : Prefer adding new files to the NestJS repo located at `api/src/unraid-api/` instead of the legacy code
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
📚 Learning: 2025-02-03T17:21:26.738Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/DefaultPageLayout.php.modified.snapshot.php:203-207
Timestamp: 2025-02-03T17:21:26.738Z
Learning: The project uses patches to override existing Unraid pages rather than modifying them directly.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test API
🔇 Additional comments (11)
web/src/components/sso/useSsoAuth.ts (1)
52-56: LGTM!Clean simplification with proper URL encoding of the provider ID to prevent injection issues.
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid (1)
1-10: LGTM!Clear header, appropriate variable definitions with sensible defaults for paths. Using
${UNRAID_LOG_PATH:-...}for configurable logging is good practice.plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh (2)
43-50: LGTM!The additions of
/etc/rc.d/rc.unraidto critical files and/usr/local/unraidto critical directories are consistent with the new Unraid Core service being introduced in this PR.
164-170: LGTM!The new shutdown script check for
K30unraid-corefollows the established pattern used forK10flash-backupandK20unraid-api. The ordering (K30) ensures the core service stops after the API service (K20).api/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch (2)
25-40: LGTM - Well-structured remote access check.The
check_remote_access()function properly validates multiple conditions before enabling remote access:
- Plugin enablement via
api_utils.sh- Config file and
jqavailability- Both
wanaccess=trueAND valid usernameThe defensive checks (file existence, command availability, null handling) are appropriate.
73-86: Good fix for array iteration.Quoting the array expansion (
"${!NET_FQDN[@]}"instead of${!NET_FQDN[@]}) correctly prevents word splitting on interface names that might contain spaces or special characters.api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts (1)
71-74: LGTM!The proxy_pass target change aligns with routing
/graphqlrequests through the new Unraid Core service socket.plugin/plugins/dynamix.unraid.net.plg (4)
12-15: LGTM!Core package entities are properly defined with SHA256 checksum for integrity verification.
59-63: LGTM!Core package download block follows the same pattern as the main API package with proper URL and SHA256 verification.
555-584: Core package installation follows good patterns.The installation logic properly:
- Cleans up old package versions
- Stops the service before upgrade
- Uses
upgradepkg --install-new --reinstall- Sets executable permission on rc.unraid
- Exits with error if package file is missing
One note: Line 559's cleanup pattern
unraid-*.txzcould also be made more specific similar to the uninstall pattern concern.
661-667: LGTM!Core service startup after API startup is appropriate. The warning message when
rc.unraidis not found provides good visibility for debugging.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
api/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch
Outdated
Show resolved
Hide resolved
| # Stop and remove Unraid Core package | ||
| if [ -x "/etc/rc.d/rc.unraid" ]; then | ||
| echo "Stopping Unraid Core..." | ||
| /etc/rc.d/rc.unraid stop || echo "Warning: Failed to stop Unraid Core" | ||
| fi | ||
| core_pkg_installed=$(ls -1 /var/log/packages/unraid-* 2>/dev/null | head -1) | ||
| if [ -n "$core_pkg_installed" ]; then | ||
| core_pkg_basename=$(basename "$core_pkg_installed") | ||
| echo "Removing core package: $core_pkg_basename" | ||
| removepkg --terse "$core_pkg_basename" | ||
| else | ||
| echo "No Unraid Core package found" | ||
| fi |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n plugin/plugins/dynamix.unraid.net.plg | sed -n '333,346p'Repository: unraid/api
Length of output: 722
🏁 Script executed:
# Check for package naming patterns in the codebase
rg "unraid-" plugin/plugins/dynamix.unraid.net.plg | head -20Repository: unraid/api
Length of output: 1437
🏁 Script executed:
# Look for references to package removals or installations to understand naming conventions
rg "removepkg|installpkg|txz" plugin/plugins/dynamix.unraid.net.plg | head -30Repository: unraid/api
Length of output: 1699
Use a more specific pattern for core package matching.
The glob pattern unraid-* on line 339 could match unrelated packages like unraid-api if they're installed in /var/log/packages/. Since the core package follows a version-based naming convention (e.g., unraid-0.1.0-2026.01.14.1), use:
Suggested fix
- core_pkg_installed=$(ls -1 /var/log/packages/unraid-* 2>/dev/null | head -1)
+ core_pkg_installed=$(ls -1 /var/log/packages/unraid-[0-9]* 2>/dev/null | head -1)📝 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.
| # Stop and remove Unraid Core package | |
| if [ -x "/etc/rc.d/rc.unraid" ]; then | |
| echo "Stopping Unraid Core..." | |
| /etc/rc.d/rc.unraid stop || echo "Warning: Failed to stop Unraid Core" | |
| fi | |
| core_pkg_installed=$(ls -1 /var/log/packages/unraid-* 2>/dev/null | head -1) | |
| if [ -n "$core_pkg_installed" ]; then | |
| core_pkg_basename=$(basename "$core_pkg_installed") | |
| echo "Removing core package: $core_pkg_basename" | |
| removepkg --terse "$core_pkg_basename" | |
| else | |
| echo "No Unraid Core package found" | |
| fi | |
| # Stop and remove Unraid Core package | |
| if [ -x "/etc/rc.d/rc.unraid" ]; then | |
| echo "Stopping Unraid Core..." | |
| /etc/rc.d/rc.unraid stop || echo "Warning: Failed to stop Unraid Core" | |
| fi | |
| core_pkg_installed=$(ls -1 /var/log/packages/unraid-[0-9]* 2>/dev/null | head -1) | |
| if [ -n "$core_pkg_installed" ]; then | |
| core_pkg_basename=$(basename "$core_pkg_installed") | |
| echo "Removing core package: $core_pkg_basename" | |
| removepkg --terse "$core_pkg_basename" | |
| else | |
| echo "No Unraid Core package found" | |
| fi |
🤖 Prompt for AI Agents
In `@plugin/plugins/dynamix.unraid.net.plg` around lines 333 - 346, Replace the
broad glob "unraid-*" used when setting core_pkg_installed with a
version-specific pattern or filter to avoid matching unrelated packages: update
the ls/grep invocation that assigns core_pkg_installed to only match names like
"unraid-" followed by a digit (e.g., use "unraid-[0-9]*" or pipe through grep -E
'^unraid-[0-9]') so core_pkg_installed and core_pkg_basename resolve to the
actual core package name and removepkg --terse is invoked only for that package.
| rollback() { | ||
| if [ -d "/usr/local/unraid.prev" ]; then | ||
| echo "Rolling back to previous version..." | ||
| stop | ||
| rm -rf /usr/local/unraid | ||
| mv /usr/local/unraid.prev /usr/local/unraid | ||
| start | ||
| echo "Rollback complete" | ||
| else | ||
| echo "No previous version available" | ||
| exit 1 | ||
| fi | ||
| } |
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.
Rollback operation is not atomic and risks data loss.
If the mv command on line 72 fails after rm -rf on line 71, the system will be left without any installation. Consider a safer swap approach:
🔧 Suggested safer rollback
rollback() {
if [ -d "/usr/local/unraid.prev" ]; then
echo "Rolling back to previous version..."
stop
- rm -rf /usr/local/unraid
- mv /usr/local/unraid.prev /usr/local/unraid
+ # Atomic swap: rename current to .old, then prev to current
+ rm -rf /usr/local/unraid.old 2>/dev/null
+ mv /usr/local/unraid /usr/local/unraid.old && \
+ mv /usr/local/unraid.prev /usr/local/unraid && \
+ rm -rf /usr/local/unraid.old
start
echo "Rollback complete"
else📝 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.
| rollback() { | |
| if [ -d "/usr/local/unraid.prev" ]; then | |
| echo "Rolling back to previous version..." | |
| stop | |
| rm -rf /usr/local/unraid | |
| mv /usr/local/unraid.prev /usr/local/unraid | |
| start | |
| echo "Rollback complete" | |
| else | |
| echo "No previous version available" | |
| exit 1 | |
| fi | |
| } | |
| rollback() { | |
| if [ -d "/usr/local/unraid.prev" ]; then | |
| echo "Rolling back to previous version..." | |
| stop | |
| rm -rf /usr/local/unraid.old 2>/dev/null | |
| mv /usr/local/unraid /usr/local/unraid.old && \ | |
| mv /usr/local/unraid.prev /usr/local/unraid && \ | |
| rm -rf /usr/local/unraid.old | |
| start | |
| echo "Rollback complete" | |
| else | |
| echo "No previous version available" | |
| exit 1 | |
| fi | |
| } |
🤖 Prompt for AI Agents
In `@plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid` around lines 67 - 79,
The rollback() function currently does rm -rf /usr/local/unraid then mv
/usr/local/unraid.prev /usr/local/unraid which is not atomic and can leave the
system without an installation if mv fails; change to a safer three-step swap:
rename the current dir to a temp backup (e.g., mv /usr/local/unraid
/usr/local/unraid.tmp), then mv /usr/local/unraid.prev to /usr/local/unraid, and
only after verifying that succeeds remove the temp backup; ensure each mv is
checked for failure and on any error attempt to restore from the temp backup and
return a non-zero exit code so rollback is atomic and recoverable, updating the
rollback() function and its error handling accordingly.
861c49d to
2a31900
Compare
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/components/sso/useSsoAuth.ts (2)
11-23: Return type doesn't reflect nullable fields.The function's return type declares all fields as non-null, but the implementation can return null values when DOM queries fail. This creates a type safety gap where callers rely on the return type but receive null values at runtime.
Suggested fix
- const getInputFields = (): { - form: HTMLFormElement; - passwordField: HTMLInputElement; - usernameField: HTMLInputElement; - } => { + const getInputFields = (): { + form: HTMLFormElement | null; + passwordField: HTMLInputElement | null; + usernameField: HTMLInputElement | null; + } => {
58-93: Move the pathname check to the start of the function.The login path check on line 91 only executes when no token or error is present. If a user navigates to a non-login page with a token in the URL (e.g.,
/dashboard#token=xyz), the code will still attempt to process the callback—changing state, attempting form manipulation, and cleaning up the URL—before the form lookup fails silently.Moving the check to the start avoids unnecessary processing and potential side effects on non-login pages.
Suggested fix
const handleOAuthCallback = async () => { try { + if (window.location.pathname !== '/login') { + return; + } + // First check hash parameters (for token and error - keeps them out of server logs) const hashParams = new URLSearchParams(window.location.hash.slice(1)); const hashToken = hashParams.get('token'); const hashError = hashParams.get('error'); // Then check query parameters (for error/token fallback) const search = new URLSearchParams(window.location.search); // Check for error in hash (preferred) or query params (fallback) const errorParam = hashError || search.get('error') || ''; if (errorParam) { currentState.value = 'error'; error.value = errorParam; // Clean up the URL (both hash and query params) window.history.replaceState({}, document.title, window.location.pathname); return; } // Handle OAuth callback if we have a token in hash (from OIDC redirect) const token = hashToken || search.get('token'); // Check hash first, query as fallback if (token) { currentState.value = 'loading'; disableFormOnSubmit(); enterCallbackTokenIntoField(token); // Clean up the URL (both hash and query params) window.history.replaceState({}, document.title, window.location.pathname); return; } - - if (window.location.pathname !== '/login') { - return; - } } catch (err) {
♻️ Duplicate comments (1)
plugin/plugins/dynamix.unraid.net.plg (1)
339-346: Use a more specific pattern for core package matching.The glob pattern
unraid-*on line 339 could match unrelated packages likeunraid-api-*in/var/log/packages/. Since the core package follows a version-based naming convention (e.g.,unraid-0.1.0-2026.01.14.1), use a more specific pattern.Suggested fix
- core_pkg_installed=$(ls -1 /var/log/packages/unraid-* 2>/dev/null | head -1) + core_pkg_installed=$(ls -1 /var/log/packages/unraid-[0-9]* 2>/dev/null | head -1)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/rc.nginx.modified.snapshotapi/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patchapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.tsplugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/etc/rc.d/rc.unraidplugin/source/dynamix.unraid.net/etc/rc.d/rc6.d/K30unraid-coreplugin/source/dynamix.unraid.net/install/doinst.shplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.shweb/__test__/components/SsoButton.test.tsweb/src/components/sso/useSsoAuth.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- web/test/components/SsoButton.test.ts
- plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid
- api/src/unraid-api/unraid-file-modifier/modifications/test/snapshots/rc.nginx.modified.snapshot
- plugin/source/dynamix.unraid.net/install/doinst.sh
- api/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch
- api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
plugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/etc/rc.d/rc6.d/K30unraid-coreweb/src/components/sso/useSsoAuth.tsplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
web/src/components/sso/useSsoAuth.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/components/sso/useSsoAuth.ts
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/components/sso/useSsoAuth.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
web/src/components/sso/useSsoAuth.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 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.
Learnt from: mdatelle
Repo: unraid/api PR: 1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: pujitm
Repo: unraid/api PR: 1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
📚 Learning: 2025-09-04T18:42:53.531Z
Learnt from: pujitm
Repo: unraid/api PR: 1658
File: plugin/plugins/dynamix.unraid.net.plg:73-79
Timestamp: 2025-09-04T18:42:53.531Z
Learning: In the dynamix.unraid.net plugin, versions 6.12.1-6.12.14 and 6.12.15 prereleases are intentionally allowed to install with warnings (rather than immediate cleanup) to provide users with a grace period and notice before functionality is completely removed. This is a deliberate UX decision to avoid immediately breaking existing setups while encouraging upgrades.
Applied to files:
plugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/etc/rc.d/rc6.d/K30unraid-coreplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
📚 Learning: 2025-09-04T15:26:34.416Z
Learnt from: elibosley
Repo: unraid/api PR: 1657
File: web/scripts/deploy-dev.sh:37-41
Timestamp: 2025-09-04T15:26:34.416Z
Learning: In web/scripts/deploy-dev.sh, the command `rm -rf /usr/local/emhttp/plugins/dynamix.my.servers/unraid-components/*` intentionally removes all contents of the unraid-components directory before deploying standalone components. This broader cleanup is desired behavior according to the maintainer elibosley.
Applied to files:
plugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/etc/rc.d/rc6.d/K30unraid-coreplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
📚 Learning: 2025-02-04T17:21:39.710Z
Learnt from: mdatelle
Repo: unraid/api PR: 1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-05-08T19:28:54.365Z
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:19-24
Timestamp: 2025-05-08T19:28:54.365Z
Learning: The directory `/usr/local/emhttp/plugins/dynamix.my.servers` is a valid directory that exists as part of the Unraid API plugin installation and should be included in verification checks.
Applied to files:
plugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
📚 Learning: 2025-06-11T14:14:30.348Z
Learnt from: pujitm
Repo: unraid/api PR: 1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
Applied to files:
plugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/etc/rc.d/rc6.d/K30unraid-coreplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
📚 Learning: 2025-05-08T19:31:52.417Z
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:11-16
Timestamp: 2025-05-08T19:31:52.417Z
Learning: The `dynamix.my.servers` namespace is still valid and should not be changed to `dynamix.unraid.net` in file paths, as both namespaces coexist in the codebase.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-03-27T13:34:53.438Z
Learnt from: pujitm
Repo: unraid/api PR: 1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Applied to files:
plugin/plugins/dynamix.unraid.net.plgplugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
📚 Learning: 2025-02-05T21:10:48.136Z
Learnt from: elibosley
Repo: unraid/api PR: 1120
File: plugin/plugins/dynamix.unraid.net.plg:35-38
Timestamp: 2025-02-05T21:10:48.136Z
Learning: When providing error handling guidance for Unraid plugins, direct users to use the web GUI (Plugins > Installed Plugins) rather than suggesting command-line actions.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-01-29T00:59:26.633Z
Learnt from: zackspear
Repo: unraid/api PR: 1079
File: web/scripts/deploy-dev.sh:51-54
Timestamp: 2025-01-29T00:59:26.633Z
Learning: For the Unraid web components deployment process, JS file validation isn't required in auth-request.php updates since the files come from the controlled build pipeline where we are the source.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-05-07T16:07:47.236Z
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/setup_api.sh:107-113
Timestamp: 2025-05-07T16:07:47.236Z
Learning: The Unraid API is designed to handle missing configuration files gracefully with smart internal fallbacks rather than requiring installation scripts to create default configurations.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-01-27T14:31:42.305Z
Learnt from: elibosley
Repo: unraid/api PR: 1063
File: web/components/SsoButton.ce.vue:5-8
Timestamp: 2025-01-27T14:31:42.305Z
Learning: In the Unraid API web components, SSO-related props are intentionally provided in both camelCase (`ssoEnabled`) and lowercase (`ssoenabled`) variants to support interchangeable usage across different contexts (e.g., HTML attributes vs Vue props).
Applied to files:
web/src/components/sso/useSsoAuth.ts
🔇 Additional comments (12)
web/src/components/sso/useSsoAuth.ts (4)
1-9: LGTM!Clean imports and well-typed state initialization.
25-50: LGTM!Form manipulation logic is straightforward. The null guards properly protect against missing DOM elements.
52-56: LGTM!Clean simplification. Using
encodeURIComponenton theproviderIdis good practice for preventing path injection.
102-111: LGTM!Clean lifecycle hook usage and well-scoped public API.
plugin/source/dynamix.unraid.net/etc/rc.d/rc6.d/K30unraid-core (1)
1-7: LGTM!The shutdown script follows the established pattern and correctly orders the core service shutdown (K30) after the API (K20). The executable check provides a safe guard when
rc.unraidisn't present.plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh (2)
45-50: LGTM!The additions to
CRITICAL_FILESandCRITICAL_DIRSare consistent with the new core service infrastructure.
164-170: LGTM!The shutdown script verification for
K30unraid-corefollows the established pattern and integrates correctly with the error tracking.plugin/plugins/dynamix.unraid.net.plg (5)
12-15: LGTM!The core package entity definitions follow the established pattern for the API package and include proper SHA256 integrity verification.
59-63: LGTM!The download block correctly references the core package entities and maintains SHA256 integrity verification.
432-434: LGTM!The core package variables follow the established pattern for the API package.
661-667: LGTM!The startup sequence correctly starts the core service after the API, with appropriate error handling and user feedback.
555-584: Core installation block looks correct.The installation flow properly:
- Cleans up old core package files using glob
unraid-*.txz- Stops the service before upgrade to avoid races
- Installs the package
- Fails fast if package is missing (consistent with project patterns)
The glob is safe since core packages use
unraid-*.txznaming while API packages usedynamix.unraid.net-*.txznaming, so cleanup won't accidentally remove API packages.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
9182cff to
f73919a
Compare
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
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: 0
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/unraid-file-modifier/modifications/rc-nginx.modification.ts (1)
32-40: Add idempotency check for MYSERVERS replacement to match pattern used elsewhere in file.The MYSERVERS replacement (lines 36-40) lacks the idempotency protection used by other major patches in this file (lines 42, 76, 84, 102). If the exact string doesn't match—due to whitespace variations or file differences—the replace silently succeeds without actually replacing, leaving
API_UTILSandCONNECT_CONFIGundefined while subsequent patches likecheck_remote_access()still execute.Either:
- Wrap lines 36-40 with
if (!newContent.includes('CONNECT_CONFIG='))check- Add back the MYSERVERS validation (lines 32-34) to fail fast on unexpected content
- Check for pre-existence of the replacement before attempting it
♻️ Duplicate comments (1)
plugin/plugins/dynamix.unraid.net.plg (1)
339-346: Use a more specific pattern for core package matching.The glob pattern
unraid-*on line 339 could potentially match unrelated packages. Since the core package follows the naming conventionunraid-X.Y.Z-YYYY.MM.DD.N(starting with a digit afterunraid-), a more specific pattern would be safer.Suggested fix
- core_pkg_installed=$(ls -1 /var/log/packages/unraid-* 2>/dev/null | head -1) + core_pkg_installed=$(ls -1 /var/log/packages/unraid-[0-9]* 2>/dev/null | head -1)
🧹 Nitpick comments (2)
plugin/plugins/dynamix.unraid.net.plg (1)
559-564: Use a more specific pattern for cleanup loop.Similar to the removal logic, the glob pattern
unraid-*.txzcould match unrelated packages. Use a version-specific pattern for consistency and safety.Suggested fix
- for txz_file in /boot/config/plugins/dynamix.my.servers/unraid-*.txz; do + for txz_file in /boot/config/plugins/dynamix.my.servers/unraid-[0-9]*.txz; doweb/src/helpers/create-apollo-client.ts (1)
46-51: Good centralization of CSRF token handling.The extraction of
csrfTokenwith a fallback value ensures consistent CSRF token usage across both HTTP headers and WebSocket query parameters. This aligns with the sandbox behavior where CSRF validation should fail silently with a fallback.Consider adding a type declaration for
globalThis.csrf_tokento improve type safety:declare global { var csrf_token: string | undefined; }This could be added to the existing
Windowinterface declaration block or in a separate global types file.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/rc.nginx.modified.snapshotapi/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patchapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.tsplugin/plugins/dynamix.unraid.net.plgweb/src/components/sso/useSsoAuth.tsweb/src/helpers/create-apollo-client.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/components/sso/useSsoAuth.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
web/src/helpers/create-apollo-client.tsapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/helpers/create-apollo-client.ts
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/helpers/create-apollo-client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
web/src/helpers/create-apollo-client.tsapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
web/src/helpers/create-apollo-client.tsplugin/plugins/dynamix.unraid.net.plgapi/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patchapi/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/rc.nginx.modified.snapshotapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
api/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer adding new files to the NestJS repo located at
api/src/unraid-api/instead of the legacy code
Files:
api/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patchapi/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/rc.nginx.modified.snapshotapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 expects TTL values in milliseconds, not seconds (e.g., 600000 for 10 minutes, not 600)
Files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
🧠 Learnings (20)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 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.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Learnt from: elibosley
Repo: unraid/api PR: 1063
File: web/components/SsoButton.ce.vue:5-8
Timestamp: 2025-01-27T14:31:42.305Z
Learning: In the Unraid API web components, SSO-related props are intentionally provided in both camelCase (`ssoEnabled`) and lowercase (`ssoenabled`) variants to support interchangeable usage across different contexts (e.g., HTML attributes vs Vue props).
Learnt from: mdatelle
Repo: unraid/api PR: 1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: mdatelle
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:32-55
Timestamp: 2025-03-07T17:35:50.406Z
Learning: Helmet security configuration in the Unraid API is intentionally relaxed (with disabled CSP, CORS policies, and HSTS) to maintain compatibility with existing Unraid plugins. Stricter security settings might break current plugin functionality.
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations. It includes try/catch blocks, detailed error logging, and safe rollback mechanisms. Individual FileModification implementations (like LogRotateModification) should allow errors to propagate to this service layer rather than handling them internally.
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/setup_api.sh:107-113
Timestamp: 2025-05-07T16:07:47.236Z
Learning: The Unraid API is designed to handle missing configuration files gracefully with smart internal fallbacks rather than requiring installation scripts to create default configurations.
Learnt from: elibosley
Repo: unraid/api PR: 1211
File: unraid-ui/src/components/form/number/NumberFieldInput.vue:1-21
Timestamp: 2025-03-13T16:17:21.897Z
Learning: The unraid-ui is a UI library being published externally, where wrapping third-party components (like those from reka-ui) is an intentional design choice to maintain a consistent interface, standardize styling, and control the exposed API.
Learnt from: pujitm
Repo: unraid/api PR: 1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations, including detailed error logging with stack traces and modification IDs. Individual FileModification implementations should focus on their core functionality without duplicating error handling.
Learnt from: mdatelle
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:57-63
Timestamp: 2025-03-07T17:36:52.790Z
Learning: The CORS configuration in the unraid API is intentionally set to allow all origins (`origin: true`). This was a deliberate architectural decision when removing the previous custom CORS implementation and implementing helmet security headers instead.
📚 Learning: 2025-01-15T21:34:00.006Z
Learnt from: pujitm
Repo: unraid/api PR: 1047
File: api/src/unraid-api/graph/sandbox-plugin.ts:57-57
Timestamp: 2025-01-15T21:34:00.006Z
Learning: In the GraphQL sandbox (api/src/unraid-api/graph/sandbox-plugin.ts), CSRF token validation should fail silently with a fallback value to maintain sandbox accessibility, as it's a development tool where strict security measures aren't required.
Applied to files:
web/src/helpers/create-apollo-client.ts
📚 Learning: 2025-03-10T17:24:06.914Z
Learnt from: mdatelle
Repo: unraid/api PR: 1219
File: api/src/unraid-api/auth/cookie.strategy.ts:19-20
Timestamp: 2025-03-10T17:24:06.914Z
Learning: In the auth system, CSRF token validation and cookie validation have been unified in the `validateCookiesCasbin()` method in the AuthService class, which takes the entire FastifyRequest object and performs both validations sequentially.
Applied to files:
web/src/helpers/create-apollo-client.ts
📚 Learning: 2025-09-04T18:42:53.531Z
Learnt from: pujitm
Repo: unraid/api PR: 1658
File: plugin/plugins/dynamix.unraid.net.plg:73-79
Timestamp: 2025-09-04T18:42:53.531Z
Learning: In the dynamix.unraid.net plugin, versions 6.12.1-6.12.14 and 6.12.15 prereleases are intentionally allowed to install with warnings (rather than immediate cleanup) to provide users with a grace period and notice before functionality is completely removed. This is a deliberate UX decision to avoid immediately breaking existing setups while encouraging upgrades.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-05-08T19:28:54.365Z
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:19-24
Timestamp: 2025-05-08T19:28:54.365Z
Learning: The directory `/usr/local/emhttp/plugins/dynamix.my.servers` is a valid directory that exists as part of the Unraid API plugin installation and should be included in verification checks.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-02-04T17:21:39.710Z
Learnt from: mdatelle
Repo: unraid/api PR: 1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-09-04T15:26:34.416Z
Learnt from: elibosley
Repo: unraid/api PR: 1657
File: web/scripts/deploy-dev.sh:37-41
Timestamp: 2025-09-04T15:26:34.416Z
Learning: In web/scripts/deploy-dev.sh, the command `rm -rf /usr/local/emhttp/plugins/dynamix.my.servers/unraid-components/*` intentionally removes all contents of the unraid-components directory before deploying standalone components. This broader cleanup is desired behavior according to the maintainer elibosley.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-06-11T14:14:30.348Z
Learnt from: pujitm
Repo: unraid/api PR: 1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
Applied to files:
plugin/plugins/dynamix.unraid.net.plgapi/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch
📚 Learning: 2025-05-08T19:31:52.417Z
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:11-16
Timestamp: 2025-05-08T19:31:52.417Z
Learning: The `dynamix.my.servers` namespace is still valid and should not be changed to `dynamix.unraid.net` in file paths, as both namespaces coexist in the codebase.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-02-05T21:10:48.136Z
Learnt from: elibosley
Repo: unraid/api PR: 1120
File: plugin/plugins/dynamix.unraid.net.plg:35-38
Timestamp: 2025-02-05T21:10:48.136Z
Learning: When providing error handling guidance for Unraid plugins, direct users to use the web GUI (Plugins > Installed Plugins) rather than suggesting command-line actions.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-03-27T13:34:53.438Z
Learnt from: pujitm
Repo: unraid/api PR: 1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-01-29T00:59:26.633Z
Learnt from: zackspear
Repo: unraid/api PR: 1079
File: web/scripts/deploy-dev.sh:51-54
Timestamp: 2025-01-29T00:59:26.633Z
Learning: For the Unraid web components deployment process, JS file validation isn't required in auth-request.php updates since the files come from the controlled build pipeline where we are the source.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-05-07T16:07:47.236Z
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/setup_api.sh:107-113
Timestamp: 2025-05-07T16:07:47.236Z
Learning: The Unraid API is designed to handle missing configuration files gracefully with smart internal fallbacks rather than requiring installation scripts to create default configurations.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-02-03T18:57:53.577Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts:80-80
Timestamp: 2025-02-03T18:57:53.577Z
Learning: In the unraid/api codebase, patch files generated for file modifications always use the .patch extension, regardless of the source file type being modified.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch
📚 Learning: 2025-02-03T17:21:26.738Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/DefaultPageLayout.php.modified.snapshot.php:203-207
Timestamp: 2025-02-03T17:21:26.738Z
Learning: The project uses patches to override existing Unraid pages rather than modifying them directly.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch
📚 Learning: 2025-03-07T17:35:50.406Z
Learnt from: mdatelle
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:32-55
Timestamp: 2025-03-07T17:35:50.406Z
Learning: Helmet security configuration in the Unraid API is intentionally relaxed (with disabled CSP, CORS policies, and HSTS) to maintain compatibility with existing Unraid plugins. Stricter security settings might break current plugin functionality.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
Repo: unraid/api PR: 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/unraid-file-modifier/modifications/__test__/snapshots/rc.nginx.modified.snapshotapi/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
📚 Learning: 2025-09-02T19:35:14.427Z
Learnt from: elibosley
Repo: unraid/api PR: 1639
File: web/components/standalone-mount.ts:38-54
Timestamp: 2025-09-02T19:35:14.427Z
Learning: In the unraid/api project, exposing Apollo client and GraphQL utilities (apolloClient, gql, graphqlParse) on the global window object is intentional to allow developers to interact with GraphQL from the browser console without needing to be inside Vue components. Developer experience is prioritized over minimizing global exposure in this context.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/rc.nginx.modified.snapshot
📚 Learning: 2025-01-29T16:35:43.699Z
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations, including detailed error logging with stack traces and modification IDs. Individual FileModification implementations should focus on their core functionality without duplicating error handling.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
📚 Learning: 2025-01-29T16:36:04.777Z
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations. It includes try/catch blocks, detailed error logging, and safe rollback mechanisms. Individual FileModification implementations (like LogRotateModification) should allow errors to propagate to this service layer rather than handling them internally.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts
🔇 Additional comments (11)
plugin/plugins/dynamix.unraid.net.plg (5)
12-15: LGTM!The entity definitions for the Unraid Core package follow a consistent pattern with the existing API package definitions, including proper SHA256 verification and versioned naming.
59-63: LGTM!The download definition for the Unraid Core package mirrors the existing API package download pattern with proper SHA256 integrity verification.
432-434: LGTM!Variable definitions follow the established naming convention and mirror the API package variables above.
573-584: Consider cleanup on core installation failure.If the core package installation fails (lines 573-576) or the core package file is missing (lines 582-583), the script exits with an error. However, at this point the API package has already been installed successfully. This could leave the system in a partially installed state.
Consider whether the installation should be atomic (rolling back the API package on core failure) or if this partial state is acceptable for troubleshooting purposes.
661-667: LGTM!The conditional startup of the Unraid Core service with a warning fallback is appropriate. The service startup order (API first, then Core) aligns with the dependency structure.
api/src/unraid-api/unraid-file-modifier/modifications/patches/rc-nginx.patch (3)
19-40: Well-structured remote access check function.The
check_remote_access()function properly:
- Verifies the API utils script exists before sourcing
- Checks plugin enablement via the utility script
- Parses JSON config with jq, handling missing commands gracefully
- Validates both
wanaccessandusernamefields correctly (including the"null"string check for jq -r output)
69-96: GraphQL routing split correctly handles the migration.The routing changes appropriately:
/graphql/api→unraid-api.sockpreserves access to the legacy API/graphql→unraid-core.sockroutes to the new core service- WebSocket upgrade detection with rewrite to
/graphql/socketfollows the expected patternThe
if ($http_upgrade = "websocket")usage inside a location block is acceptable here since it only performs a simple rewrite withbreak, avoiding the typical pitfalls of nginx'sifdirective.
110-125: Correct fix for bash array iteration.Quoting the array expansion
"${!NET_FQDN[@]}"prevents word splitting on keys containing special characters.api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/rc.nginx.modified.snapshot (1)
386-454: Snapshot correctly captures the new proxy configurations.The test snapshot properly reflects:
- SSO endpoint at
/auth/sso(lines 386-395)- GraphQL API endpoint at
/graphql/api(lines 431-439)- Updated
/graphqllocation with websocket upgrade handling (lines 440-454)This provides good regression coverage for the nginx configuration modifications.
api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts (2)
76-90: Good use of idempotent injection pattern.The guards (
!newContent.includes('location /auth/sso')and!newContent.includes('location /graphql/api')) ensure the blocks are only injected once, making the modification safe to re-run.
114-121: Clarify OS version handling.The class docstring states this patch is for "< Unraid 7.2.0", but
checkOsVersion: falsemeans it will apply on all versions. If Unraid 7.2.0+ already includes these changes natively (via the backported webgui PR), the idempotent checks should prevent double-injection, but the patch will still run unnecessarily.Is
checkOsVersion: falseintentional to ensure the patch always applies for safety, or should this be restored to check the OS version and skip on 7.2.0+?
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.