fix: rm short-circuit in rc.unraid-api if plugin config dir is absent
#1515
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughInitialization lines defining a plugin directory and checking for its existence were removed from a startup script. A path constant and related test assertion for Changes
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
pujitm
commented
Jul 14, 2025
|
|
||
| flash="/boot/config/plugins/dynamix.my.servers" | ||
| [[ ! -d "${flash}" ]] && echo "Please reinstall the Unraid Connect plugin" && exit 1 | ||
| [[ ! -f "${flash}/env" ]] && echo 'env=production' >"${flash}/env" |
There was a problem hiding this comment.
@elibosley @ljm42 is it preferable to still write this env file?
There was a problem hiding this comment.
Is this for flash backup? I think we probably should nuke it if possible and just have flash backup work off production by default.
|
@CodeRabbit summary |
✅ Actions performedSummary regeneration triggered. |
elibosley
approved these changes
Jul 14, 2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
api/src/__test__/store/modules/__snapshots__/paths.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
api/src/__test__/store/modules/paths.test.ts(0 hunks)api/src/environment.ts(2 hunks)api/src/store/modules/paths.ts(0 hunks)api/src/unraid-api/cli/switch-env.command.ts(2 hunks)api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/.login.php.last-download-time(1 hunks)api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/DefaultPageLayout.php.last-download-time(1 hunks)api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/Notifications.page.last-download-time(1 hunks)api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/auth-request.php.last-download-time(1 hunks)api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/rc.nginx.last-download-time(1 hunks)
💤 Files with no reviewable changes (2)
- api/src/store/modules/paths.ts
- api/src/test/store/modules/paths.test.ts
✅ Files skipped from review due to trivial changes (6)
- api/src/unraid-api/unraid-file-modifier/modifications/test/fixtures/downloaded/.login.php.last-download-time
- api/src/unraid-api/unraid-file-modifier/modifications/test/fixtures/downloaded/auth-request.php.last-download-time
- api/src/unraid-api/unraid-file-modifier/modifications/test/fixtures/downloaded/Notifications.page.last-download-time
- api/src/unraid-api/unraid-file-modifier/modifications/test/fixtures/downloaded/DefaultPageLayout.php.last-download-time
- api/src/unraid-api/unraid-file-modifier/modifications/test/fixtures/downloaded/rc.nginx.last-download-time
- api/src/environment.ts
🧰 Additional context used
📓 Path-based instructions (2)
api/src/unraid-api/**/*
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
- .cursor/rules/api-rules.mdc
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: elibosley
PR: unraid/api#1151
File: plugin/builder/utils/consts.ts:6-6
Timestamp: 2025-03-04T14:55:00.903Z
Learning: The startingDir constant in plugin/builder/utils/consts.ts is defined using process.cwd(), which can cause issues if directory changes occur after importing this constant. Using __dirname.split('/builder')[0] would make it more reliable by making it relative to the file location rather than the current working directory.
Learnt from: elibosley
PR: unraid/api#1120
File: plugin/Dockerfile:1-3
Timestamp: 2025-02-05T14:25:37.316Z
Learning: The Dockerfile in the plugin directory is specifically for building artifacts during CI and is not used for production deployments. It creates a build environment with Node.js and necessary dependencies to generate plugin files.
Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:143-149
Timestamp: 2025-04-22T14:41:19.603Z
Learning: The `demo` property in the config.persistence.ts file's migrateLegacyConfig method is used for demonstration purposes, adding a current timestamp to keep demo environments looking fresh.
Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-generator/src/create-plugin.ts:91-112
Timestamp: 2025-04-21T18:27:36.482Z
Learning: For utility functions like file operations in api plugin generators, pujitm prefers minimal error handling rather than verbose try/catch blocks for each operation, as errors will propagate properly to higher level error handlers.
api/src/unraid-api/cli/switch-env.command.ts (6)
Learnt from: elibosley
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:0-0
Timestamp: 2025-04-21T18:44:39.643Z
Learning: When working with file operations in Node.js, prefer using the native Promise-based APIs from the fs/promises module instead of callback-based APIs from fs or manually wrapping callbacks in Promises.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Learnt from: elibosley
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:0-0
Timestamp: 2025-04-21T18:44:39.643Z
Learning: Use Promise-based file writing instead of callback-based approaches for better error handling and to allow proper propagation of success/failure states to callers.
Learnt from: pujitm
PR: unraid/api#1145
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:57-60
Timestamp: 2025-02-18T21:39:10.313Z
Learning: When using Node.js `rm` with `{ force: true }` option or Unix/Linux `rm -f`, existence checks are redundant as the command continues silently if the file doesn't exist.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-30T14:50:52.281Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the Nest repo located at api/src/unraid-api/ instead of the legacy code
Learnt from: elibosley
PR: unraid/api#1151
File: plugin/builder/utils/consts.ts:6-6
Timestamp: 2025-03-04T14:55:00.903Z
Learning: The startingDir constant in plugin/builder/utils/consts.ts is defined using process.cwd(), which can cause issues if directory changes occur after importing this constant. Using __dirname.split('/builder')[0] would make it more reliable by making it relative to the file location rather than the current working directory.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
api/src/unraid-api/cli/switch-env.command.ts (2)
1-11: LGTM! Clean imports with proper ESM extensions.The refactoring to use
RestartCommandinstead of separate stop/start commands simplifies the dependency graph. Usingfs/promisesaligns with best practices for async file operations.
51-57: Good validation of environment file existence.Using
fileExistsSyncbefore attempting the copy operation provides clear error messages and prevents cryptic failures.
Comment on lines
+44
to
+46
| // Determine target environment | ||
| const currentEnv = ENVIRONMENT; | ||
| const targetEnv = options.environment ?? 'production'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the default environment behavior.
The command now defaults to 'production' when no environment is specified. Ensure this aligns with user expectations, as the previous implementation may have toggled between environments.
🏁 Script executed:
#!/bin/bash
# Description: Check for documentation or tests that describe the expected default behavior
# Search for documentation about switch-env command behavior
fd -e md -e txt | xargs rg -A 5 -B 5 "switch-env"
# Check for tests that validate the default behavior
fd -e test.ts -e spec.ts | xargs rg -A 10 "switch-env.*default|switch-env.*no.*argument"Length of output: 10821
Ensure switch-env toggles by default when no environment is specified
The CLI docs and developer guide show that running
unraid-api switch-env without -e should toggle between production and staging, not always set to production. Update the default assignment in api/src/unraid-api/cli/switch-env.command.ts:
• File: api/src/unraid-api/cli/switch-env.command.ts
Lines: 44–46
Replace:
// Determine target environment
const currentEnv = ENVIRONMENT;
const targetEnv = options.environment ?? 'production';With:
// Determine target environment (toggle if none provided)
const currentEnv = ENVIRONMENT;
const targetEnv = options.environment
?? (currentEnv === 'production' ? 'staging' : 'production');This aligns the behavior with:
- api/docs/public/cli.md: “Switch between production and staging environments”
- api/docs/developer/development.md: step 2 runs
switch-envwithout args to switch environments.
🤖 Prompt for AI Agents
In api/src/unraid-api/cli/switch-env.command.ts around lines 44 to 46, the code
currently defaults the target environment to 'production' when no environment
option is provided, but it should toggle between 'production' and 'staging'
instead. Update the assignment of targetEnv to check if options.environment is
set; if not, set targetEnv to 'staging' when currentEnv is 'production',
otherwise set it to 'production'. This change will make the CLI toggle
environments by default as documented.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
elibosley
pushed a commit
that referenced
this pull request
Jul 15, 2025
🤖 I have created a release *beep* *boop* --- ## [4.10.0](v4.9.5...v4.10.0) (2025-07-15) ### Features * trial extension allowed within 5 days of expiration ([#1490](#1490)) ([f34a33b](f34a33b)) ### Bug Fixes * delay `nginx:reload` file mod effect by 10 seconds ([#1512](#1512)) ([af33e99](af33e99)) * **deps:** update all non-major dependencies ([#1489](#1489)) ([53b05eb](53b05eb)) * ensure no crash if emhttp state configs are missing ([#1514](#1514)) ([1a7d35d](1a7d35d)) * **my.servers:** improve DNS resolution robustness for backup server ([#1518](#1518)) ([eecd9b1](eecd9b1)) * over-eager cloud query from web components ([#1506](#1506)) ([074370c](074370c)) * replace myservers.cfg reads in UpdateFlashBackup.php ([#1517](#1517)) ([441e180](441e180)) * rm short-circuit in `rc.unraid-api` if plugin config dir is absent ([#1515](#1515)) ([29dcb7d](29dcb7d)) --- 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This short-circuit causes any/all
rc.unraid-apiinvocations to immediately fail on fresh 7.2 images (because/boot/config/dynamix.my.serversdoesn't exist).Summary by CodeRabbit