feat: swap to nodemon instead of PM2 #1798
Conversation
WalkthroughReplaces PM2 with nodemon across the API: removes PM2 configs, services, helpers and tests; adds nodemon config, NodemonService, PID/log paths, tests and integration; updates environment exports, CLI wiring, GraphQL types, build packaging, logging destination, pubsub channel usage, and removes PM2 readiness IPC. (≈38 words) Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Command
participant Nod as NodemonService
participant FS as Filesystem
participant Proc as nodemon child
rect rgb(235,245,255)
Note over CLI,Nod: Start flow (nodemon-based)
CLI->>Nod: ensureNodemonDependencies()
Nod->>FS: mkdir logs & pid dirs
CLI->>Nod: start({ env })
Nod->>Proc: execa nodemon (detached, stdio piped)
Proc->>FS: write logs, write nodemon.pid
end
rect rgb(233,255,235)
Note over CLI,Nod: Status / Stop / Restart
CLI->>Nod: status()
Nod->>FS: read nodemon.pid
Nod->>Nod: process.kill(pid, 0) => alive? -> boolean
CLI->>Nod: stop({ force? })
Nod->>Nod: send SIGTERM or SIGKILL, remove pid file
CLI->>Nod: restart()
Nod->>Nod: stop() then start()
end
rect rgb(255,243,230)
Note over CLI,Nod: Logs retrieval
CLI->>Nod: logs(lines)
Nod->>FS: tail logfile
FS-->>CLI: return log output
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
elibosley
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
api/scripts/build.ts (1)
10-10: Build script now correctly packagesnodemon.jsonIncluding
nodemon.jsonin the pack directory ensures nodemon-managed deployments get the config in the release artifacts, which matches the PR goal. ThegetDeploymentVersionimport via the@app/../scriptspath is acceptable given the existing aliasing, though long‑term you may want a dedicated path alias for scripts to avoid the../hop.Also applies to: 95-98
api/src/unraid-api/cli/__test__/report.command.test.ts (1)
29-34: Mocks point to the right module, but last test contains redundant scenario
- The
vi.mock('@app/core/utils/process/unraid-api-running.js', …)wiring and the reset ofmockIsUnraidApiRunninginbeforeEachlook correct and consistent with the new process-utils location and ESM style.- In
it('should pass correct apiRunning parameter to ApiReportService', …), the second block (Lines 153‑157) stubsmockIsUnraidApiRunningtotrueand assertsgenerateReport(true)again, which is effectively identical to the first part of the test and doesn’t add coverage. If you intended to cover a different status (e.g.,falseor some alternate path), consider adjusting the stub accordingly; otherwise, you can drop the redundant block to keep the test focused.Also applies to: 53-55, 153-157
api/legacy/generated-schema-legacy.graphql (1)
1244-1271: Legacy Versions schema pm2→nodemon rename is consistentReplacing the
pm2field withnodemononVersionsmatches the rest of the nodemon migration (models, resolvers, web schema). Just ensure any consumers of the legacy schema are updated to readnodemoninstead ofpm2.api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
6-6: Nodemon version wiring into PackageVersions is correctUsing
getPackageJson().dependencies?.nodemonto populate thenodemonfield aligns with the newPackageVersionsmodel and schema, and gracefully returnsnullif the dependency is missing.If this resolver is called frequently, consider reading and caching
getPackageJson()once at module init rather than re-readingpackage.jsonon every request.Also applies to: 34-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
api/ecosystem.config.json(0 hunks)api/generated-schema.graphql(1 hunks)api/legacy/generated-schema-legacy.graphql(1 hunks)api/nodemon.json(1 hunks)api/package.json(1 hunks)api/scripts/build.ts(2 hunks)api/src/__test__/core/utils/pm2/dummy-process.js(0 hunks)api/src/__test__/core/utils/pm2/unraid-api-running.integration.test.ts(0 hunks)api/src/__test__/core/utils/process/unraid-api-running.integration.test.ts(1 hunks)api/src/core/log.ts(1 hunks)api/src/core/utils/pm2/unraid-api-running.ts(0 hunks)api/src/core/utils/process/unraid-api-running.ts(1 hunks)api/src/environment.ts(1 hunks)api/src/unraid-api/cli/__test__/report.command.test.ts(3 hunks)api/src/unraid-api/cli/cli-services.module.ts(2 hunks)api/src/unraid-api/cli/cli.module.ts(2 hunks)api/src/unraid-api/cli/logs.command.ts(2 hunks)api/src/unraid-api/cli/nodemon.service.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.ts(1 hunks)api/src/unraid-api/cli/pm2.service.spec.ts(0 hunks)api/src/unraid-api/cli/pm2.service.ts(0 hunks)api/src/unraid-api/cli/report.command.ts(1 hunks)api/src/unraid-api/cli/restart.command.ts(2 hunks)api/src/unraid-api/cli/start.command.ts(1 hunks)api/src/unraid-api/cli/status.command.ts(1 hunks)api/src/unraid-api/cli/stop.command.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts(2 hunks)api/src/unraid-api/main.ts(0 hunks)web/composables/gql/graphql.ts(1 hunks)web/src/composables/gql/graphql.ts(5 hunks)web/src/composables/gql/index.ts(1 hunks)
💤 Files with no reviewable changes (7)
- api/src/unraid-api/main.ts
- api/src/unraid-api/cli/pm2.service.ts
- api/src/test/core/utils/pm2/unraid-api-running.integration.test.ts
- api/src/unraid-api/cli/pm2.service.spec.ts
- api/src/core/utils/pm2/unraid-api-running.ts
- api/src/test/core/utils/pm2/dummy-process.js
- api/ecosystem.config.json
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
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-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:
web/src/composables/gql/index.tsweb/src/composables/gql/graphql.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
Repo: unraid/api PR: 1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Applied to files:
web/src/composables/gql/index.ts
📚 Learning: 2024-12-11T17:20:09.604Z
Learnt from: pujitm
Repo: unraid/api PR: 978
File: web/helpers/apollo-cache/index.ts:133-157
Timestamp: 2024-12-11T17:20:09.604Z
Learning: In `web/helpers/apollo-cache/index.ts`, when updating the cache in the `cache.updateQuery` function, it's sufficient to check if `data` is present, as type checking inside `data` is handled at the GraphQL layer.
Applied to files:
web/src/composables/gql/index.ts
📚 Learning: 2025-09-14T14:07:28.265Z
Learnt from: elibosley
Repo: unraid/api PR: 1702
File: web/eslint.config.mjs:106-106
Timestamp: 2025-09-14T14:07:28.265Z
Learning: The unraid/api codebase specifically requires Node 20+ as a minimum version, so modern Node.js features like import.meta.dirname (available since Node v20.11.0) can be used without compatibility fallbacks.
Applied to files:
api/src/unraid-api/graph/resolvers/info/versions/versions.model.tsapi/src/core/utils/process/unraid-api-running.tsapi/src/unraid-api/cli/cli-services.module.tsapi/package.jsonapi/src/environment.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts
📚 Learning: 2025-02-04T13:48:45.521Z
Learnt from: pujitm
Repo: unraid/api PR: 1104
File: api/src/unraid-api/cli/stop.command.ts:14-18
Timestamp: 2025-02-04T13:48:45.521Z
Learning: The PM2Service's `run` method automatically handles logging when `raw: true` is not set in the context argument. Explicit logging is not needed in such cases.
Applied to files:
api/src/unraid-api/cli/logs.command.tsapi/src/core/log.ts
📚 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:
api/src/unraid-api/cli/report.command.ts
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
Repo: unraid/api PR: 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:
api/src/unraid-api/cli/cli-services.module.ts
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
Repo: unraid/api PR: 1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/__test__/core/utils/process/unraid-api-running.integration.test.tsapi/src/unraid-api/cli/__test__/report.command.test.tsapi/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
Repo: unraid/api PR: 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.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-03-26T15:18:27.452Z
Learnt from: pujitm
Repo: unraid/api PR: 1252
File: plugin/docker-compose.yml:15-15
Timestamp: 2025-03-26T15:18:27.452Z
Learning: The files in the `api/deploy` directory (including `packed-pnpm-store.txz`) are gitignored build artifacts that are generated when the API is built. Building the API is a prerequisite before using the plugin builder container, as it needs access to these artifacts.
Applied to files:
api/scripts/build.ts
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
Repo: unraid/api PR: 1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Applied to files:
api/scripts/build.ts
📚 Learning: 2025-02-04T16:42:21.782Z
Learnt from: elibosley
Repo: unraid/api PR: 1105
File: api/scripts/build.mjs:0-0
Timestamp: 2025-02-04T16:42:21.782Z
Learning: In Node.js build scripts, avoid manually copying node_modules as a cache layer. Instead, rely on npm's built-in caching mechanism and use direct `npm install` commands, as this ensures dependency consistency and proper version resolution while still maintaining good performance through npm's cache.
Applied to files:
api/scripts/build.ts
🧬 Code graph analysis (13)
api/src/unraid-api/cli/logs.command.ts (4)
api/src/unraid-api/cli/restart.command.ts (1)
Command(21-54)api/src/unraid-api/cli/start.command.ts (1)
Command(11-33)api/src/unraid-api/cli/status.command.ts (1)
Command(5-13)api/src/unraid-api/cli/stop.command.ts (1)
Command(8-28)
api/src/unraid-api/cli/status.command.ts (4)
api/src/unraid-api/cli/logs.command.ts (1)
Command(9-25)api/src/unraid-api/cli/restart.command.ts (1)
Command(21-54)api/src/unraid-api/cli/start.command.ts (1)
Command(11-33)api/src/unraid-api/cli/stop.command.ts (1)
Command(8-28)
api/src/core/utils/process/unraid-api-running.ts (2)
packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)api/src/environment.ts (1)
NODEMON_PID_PATH(114-114)
api/src/unraid-api/cli/report.command.ts (1)
api/src/core/utils/process/unraid-api-running.ts (1)
isUnraidApiRunning(6-23)
web/src/composables/gql/graphql.ts (1)
web/composables/gql/graphql.ts (2)
Node(1357-1359)Scalars(11-75)
web/composables/gql/graphql.ts (1)
web/src/composables/gql/graphql.ts (2)
Maybe(3-3)Scalars(11-75)
api/src/__test__/core/utils/process/unraid-api-running.integration.test.ts (1)
api/src/core/utils/process/unraid-api-running.ts (1)
isUnraidApiRunning(6-23)
api/src/unraid-api/cli/start.command.ts (4)
api/src/unraid-api/cli/logs.command.ts (1)
Command(9-25)api/src/unraid-api/cli/restart.command.ts (2)
Command(21-54)LogLevelOptions(9-11)api/src/unraid-api/cli/status.command.ts (1)
Command(5-13)api/src/unraid-api/cli/stop.command.ts (1)
Command(8-28)
api/src/unraid-api/cli/nodemon.service.ts (3)
api/src/environment.ts (6)
PATHS_LOGS_DIR(101-102)NODEMON_PID_PATH(114-114)PATHS_LOGS_FILE(103-103)NODEMON_PATH(105-112)NODEMON_CONFIG_PATH(113-113)UNRAID_API_CWD(115-115)api/src/unraid-api/cli/log.service.ts (1)
error(65-69)packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)
api/src/core/log.ts (1)
api/src/environment.ts (2)
SUPPRESS_LOGS(94-94)LOG_TYPE(88-88)
api/src/unraid-api/cli/stop.command.ts (4)
api/src/unraid-api/cli/logs.command.ts (1)
Command(9-25)api/src/unraid-api/cli/restart.command.ts (1)
Command(21-54)api/src/unraid-api/cli/start.command.ts (1)
Command(11-33)api/src/unraid-api/cli/status.command.ts (1)
Command(5-13)
api/src/unraid-api/cli/nodemon.service.spec.ts (1)
packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
api/src/environment.ts (1)
getPackageJson(49-53)
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/cli/nodemon.service.spec.ts
[error] 1-1: vitest error: No 'SUPPRESS_LOGS' export is defined on the '@app/environment.js' mock in nodemon.service.spec.ts. Ensure environment mock exports SUPPRESS_LOGS.
🔇 Additional comments (18)
web/src/composables/gql/index.ts (1)
1-2: ESM re-exports are consistent and correctThe barrel re-exports look good and align with the existing ESM/import style in this repo; no changes needed. Based on learnings.
api/src/core/log.ts (1)
20-29: Updated logging comments accurately describe behaviorThe revised comments now correctly describe that output goes directly to a log file and why colorization is disabled; they match the existing
colorize: false/destination: logDestinationconfiguration without changing behavior.api/package.json (1)
140-140: Nodemon 3.1.10 is secure and appropriate for runtime deploymentVerification confirms no npm security advisories affecting version 3.1.10. The latest stable release is 3.1.11, but Renovate will handle future updates automatically. The move to
dependenciesaligns with your new process manager strategy.api/nodemon.json (1)
1-18: Nodemon config looks sound and matches build layoutWatching
dist/main.jsandmyservers.cfg, ignoringnode_modules/src/.env.*, and executingnode ./dist/main.jswithNODE_ENV=productionall look consistent with the packaged API. No changes needed.api/src/unraid-api/cli/report.command.ts (1)
33-40: Dynamic import and log message now correctly reference nodemon-based checkImporting
@app/core/utils/process/unraid-api-running.jsand updating the debug message to "failed to check nodemon state" cleanly rewire the report command onto the new process-based liveness check without altering behavior.web/composables/gql/graphql.ts (1)
1511-1528: Frontend PackageVersions type correctly adds nodemonAdding the nullable
nodemon?: Maybe<Scalars['String']['output']>field with the corresponding docstring keeps the web GraphQL typings in sync with the server schema and versions resolver.api/generated-schema.graphql (1)
1666-1690: Generated schema PackageVersions now exposes nodemonThe
"""nodemon version"""docstring andnodemon: Stringfield correctly reflect the server-side change and stay consistent with the TS model and client typings. No issues here as long as this file continues to be generated from the updated schema sources.api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts (1)
17-42: PackageVersions model nodemon field matches schema & resolverAdding the nullable
nodemonfield with the proper description keeps the GraphQL model in sync with both the generated schema andVersionsResolver.packages(). Looks good.api/src/unraid-api/cli/cli.module.ts (1)
16-16: LGTM! Clean service replacement.The NodemonService import and provider registration are correctly implemented, aligning with the PM2-to-Nodemon migration strategy.
Also applies to: 67-67
api/src/unraid-api/cli/restart.command.ts (1)
5-7: LGTM! Restart command correctly migrated to Nodemon.The service replacement is consistent with the broader migration, and the log level uppercasing aligns with environment variable conventions.
Also applies to: 23-35
api/src/unraid-api/cli/cli-services.module.ts (1)
7-7: LGTM! Service wiring correctly updated.The NodemonService replacement in the CLI services module is consistent with the migration pattern.
Also applies to: 24-24
api/src/unraid-api/cli/nodemon.service.spec.ts (1)
49-98: Well-structured test suite.The test cases properly cover key NodemonService functionality with appropriate mocking and lifecycle management. Good use of Vitest patterns.
api/src/unraid-api/cli/status.command.ts (1)
3-3: LGTM! Status command correctly migrated.The service replacement follows the same clean pattern as other commands in this migration.
Also applies to: 7-7, 11-11
api/src/__test__/core/utils/process/unraid-api-running.integration.test.ts (1)
7-53: LGTM! Comprehensive integration test with proper lifecycle management.The test suite covers all critical scenarios (missing pid file, live pid, invalid pid) with appropriate setup/teardown and dynamic module mocking. Good use of Vitest patterns.
api/src/unraid-api/cli/logs.command.ts (1)
3-3: LGTM! Logs command successfully migrated.The NodemonService integration is clean and consistent with the rest of the migration.
Also applies to: 11-11, 23-23
api/src/unraid-api/cli/stop.command.ts (1)
1-28: LGTM!The migration from PM2Service to NodemonService is clean and well-structured. The semantic change from the
--deleteflag to--forceis appropriate—in the PM2 context, "delete" removed the process from PM2's process list, whereas in the Nodemon context, "force" appropriately indicates using SIGKILL instead of SIGTERM for process termination.web/src/composables/gql/graphql.ts (1)
562-571: Auto-generated GraphQL types reflect schema changes correctly.These changes are auto-generated by GraphQL CodeGen and correctly reflect the backend schema migration from PM2 to Nodemon. The
nodemonfield now replacespm2inPackageVersions, and the newCpuPackagestype with telemetry support has been added.Based on learnings.
Also applies to: 883-883, 900-901, 1548-1549, 2070-2070
api/src/unraid-api/cli/start.command.ts (1)
1-33: LGTM! Clean migration to Nodemon.The PM2-based startup flow with its lifecycle management has been cleanly replaced with a simple delegation to
NodemonService.start(). The log level configuration is properly preserved and passed through environment variables. This simplification removes complexity while maintaining the same functionality.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1798 +/- ##
==========================================
+ Coverage 52.04% 52.39% +0.35%
==========================================
Files 874 873 -1
Lines 50372 50522 +150
Branches 5017 5091 +74
==========================================
+ Hits 26214 26473 +259
+ Misses 24083 23974 -109
Partials 75 75 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/src/core/log.ts (1)
119-132: Arrays insanitizeParamscan leak sensitive values
sanitizeParamsrecurses only into plain objects and skips arrays, so something like{ users: [{ password: 'x' }] }will log the raw password. Given this is core logging, this is a non-trivial privacy/security risk.Consider extending masking to arrays of objects, e.g. by mapping over arrays and calling
sanitizeParamson object elements before logging.
🧹 Nitpick comments (9)
api/src/core/log.ts (2)
20-21: Clarify environment assumptions for disabling color and consider configurabilityThe comment and
colorize: false/colorizeObjects: falseenforce non-colored pretty output in all contexts, not only when logs are truly being redirected to a file. If you expect interactive/dev runs where stdout is a TTY, you might want a config flag (e.g.,LOG_COLORIZE) or a TTY check to re-enable color there, while keeping it off for file-piped production output.Also applies to: 25-32
18-19: Normalize SUPPRESS_LOGS handling to the imported constant
logDestinationchecksprocess.env.SUPPRESS_LOGS === 'true'directly whilestreamuses the importedSUPPRESS_LOGSflag. This duplication is easy to forget if env parsing changes and can also diverge ifprocess.envis mutated at runtime. Prefer routing both throughSUPPRESS_LOGSfor a single source of truth.Also applies to: 22-24
api/src/environment.ts (1)
105-116: Nodemon path/env constants look correct; ensure PID directory existsThe
NODEMON_PATH,NODEMON_CONFIG_PATH, andUNRAID_API_CWDcalculations fromimport.meta.dirnameand../../look consistent with both the monorepo layout and the packeddist + node_modulesrelease layout. With Node 20+ andimport.meta.dirnamethis should be robust.One thing to confirm:
NODEMON_PID_PATHdefaults to/var/run/unraid-api/nodemon.pid. Make sure whatever writes the PID file creates the/var/run/unraid-apidirectory (or its configured parent) before writing, otherwise you’ll getENOENTon first write on a clean system.Based on learnings.
api/scripts/build.ts (1)
10-10: Alias import for getDeploymentVersion is a bit subtle; consider simplifyingUsing
@app/../scripts/get-deployment-version.jsties this script to the@app/*path mapping and relies on the loader rewriting it to something like./src/../scripts/get-deployment-version.js. That’s a bit non-obvious for future readers and could break if the alias ever changes.Unless there’s a specific reason to route this through
@app, consider reverting to a plain relative import from the same directory, which is loader‑agnostic:-import { getDeploymentVersion } from '@app/../scripts/get-deployment-version.js'; +import { getDeploymentVersion } from './get-deployment-version.js';In any case, please run the build script (
pnpm --filter @unraid/api build:release) on a clean checkout to confirm this alias-based import resolves as expected.api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
38-38: Consider whether to return the dependency version range or the actual installed version.The current implementation returns
getPackageJson().dependencies?.nodemon, which may include a version range (e.g.,^3.0.0or~3.1.4) rather than the actual installed version. This is consistent with how dependency information is typically stored in package.json, but differs from other version fields that return actual running versions (e.g.,softwareVersions.nodereturns the runtime version like20.11.0).If you want to display the exact installed version, consider using
nodemon --versionor reading fromnode_modules/nodemon/package.json.api/src/unraid-api/cli/nodemon.service.ts (3)
45-50: Handle PID file races and read errors more defensively
getStoredPiddoes an existence check thenreadFile, which can still throw (e.g. if the file is removed between the two calls or unreadable). Right now any such error would bubble up intostop/status.Consider collapsing this into a single
readFilewith a catch and treating failures as “no PID”, e.g.:private async getStoredPid(): Promise<number | null> { try { const contents = (await readFile(NODEMON_PID_PATH, 'utf-8')).trim(); const pid = Number.parseInt(contents, 10); return Number.isNaN(pid) ? null : pid; } catch { return null; } }This makes
stop/statusresilient to transient file races or permissions issues.
87-105: Be cautious about removing PID file when kill fails
stopalways deletes the PID file in thefinallyblock, even ifprocess.killthrows for reasons other than “no such process” (e.g. permission issues). That can leave a still-running nodemon process without a PID file, making later status/stop operations think nothing is running.If you care about this edge case, consider:
- Only removing the PID file when
killsucceeds orisPidRunning(pid)returns false, or- Logging more explicitly when you remove the PID file after a failed kill so operators can track potential strays.
Not a blocker, but worth considering for robustness.
129-132: Logs command behavior is reasonable; consider friendlier error handlingUsing
execa('tail', ['-n',${lines}, PATHS_LOGS_FILE])is straightforward and leverages the system tail. If you expect this to run on systems wheretailor the log file might be missing, optionally wrap this in try/catch and translate failures into a clearer message like “No logs available yet” instead of surfacing a raw execa error.api/src/unraid-api/cli/start.command.ts (1)
20-32: Hardenrunerror handling and normalizeLOG_LEVELenvVerification confirms that
@OptiondefaultValuedoes not guarantee the property will be set in all invocation paths (e.g., programmatic calls bypassing commander parsing), so your concern aboutoptions.logLevelbeing undefined is valid.Two improvements are recommended:
- Wrap the call in a try/catch like
RestartCommand.runso CLI failures are logged cleanly and exit with a non‑zero code.- Normalize/fallback the env value so
LOG_LEVELis always a string:async run(_: string[], options: LogLevelOptions): Promise<void> { this.logger.info('Starting the Unraid API'); const logLevel = (options.logLevel ?? LOG_LEVEL).toUpperCase(); try { await this.nodemon.start({ env: { LOG_LEVEL: logLevel } }); } catch (error) { if (error instanceof Error) { this.logger.error(error.message); } else { this.logger.error('Unknown error occurred'); } process.exit(1); } }This keeps behavior consistent with
restartand avoids passing an undefined env value toexeca.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
api/ecosystem.config.json(0 hunks)api/generated-schema.graphql(1 hunks)api/legacy/generated-schema-legacy.graphql(1 hunks)api/nodemon.json(1 hunks)api/package.json(1 hunks)api/scripts/build.ts(2 hunks)api/src/__test__/core/utils/pm2/dummy-process.js(0 hunks)api/src/__test__/core/utils/pm2/unraid-api-running.integration.test.ts(0 hunks)api/src/__test__/core/utils/process/unraid-api-running.integration.test.ts(1 hunks)api/src/__test__/graphql/resolvers/rclone-api.service.test.ts(1 hunks)api/src/core/log.ts(1 hunks)api/src/core/utils/pm2/unraid-api-running.ts(0 hunks)api/src/core/utils/process/unraid-api-running.ts(1 hunks)api/src/environment.ts(1 hunks)api/src/unraid-api/cli/__test__/report.command.test.ts(3 hunks)api/src/unraid-api/cli/cli-services.module.ts(2 hunks)api/src/unraid-api/cli/cli.module.ts(2 hunks)api/src/unraid-api/cli/logs.command.ts(2 hunks)api/src/unraid-api/cli/nodemon.service.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.ts(1 hunks)api/src/unraid-api/cli/pm2.service.spec.ts(0 hunks)api/src/unraid-api/cli/pm2.service.ts(0 hunks)api/src/unraid-api/cli/report.command.ts(1 hunks)api/src/unraid-api/cli/restart.command.ts(2 hunks)api/src/unraid-api/cli/start.command.ts(1 hunks)api/src/unraid-api/cli/status.command.ts(1 hunks)api/src/unraid-api/cli/stop.command.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts(2 hunks)api/src/unraid-api/main.ts(0 hunks)web/composables/gql/graphql.ts(1 hunks)web/src/composables/gql/graphql.ts(5 hunks)web/src/composables/gql/index.ts(1 hunks)
💤 Files with no reviewable changes (7)
- api/src/unraid-api/main.ts
- api/src/unraid-api/cli/pm2.service.spec.ts
- api/src/core/utils/pm2/unraid-api-running.ts
- api/src/unraid-api/cli/pm2.service.ts
- api/src/test/core/utils/pm2/dummy-process.js
- api/src/test/core/utils/pm2/unraid-api-running.integration.test.ts
- api/ecosystem.config.json
🧰 Additional context used
🧠 Learnings (15)
📚 Learning: 2025-09-14T14:07:28.265Z
Learnt from: elibosley
Repo: unraid/api PR: 1702
File: web/eslint.config.mjs:106-106
Timestamp: 2025-09-14T14:07:28.265Z
Learning: The unraid/api codebase specifically requires Node 20+ as a minimum version, so modern Node.js features like import.meta.dirname (available since Node v20.11.0) can be used without compatibility fallbacks.
Applied to files:
api/src/core/utils/process/unraid-api-running.tsapi/src/unraid-api/cli/report.command.tsapi/src/environment.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.resolver.tsapi/src/unraid-api/cli/cli-services.module.tsapi/package.json
📚 Learning: 2025-01-27T14:57:46.617Z
Learnt from: elibosley
Repo: unraid/api PR: 1063
File: web/_data/serverState.ts:137-147
Timestamp: 2025-01-27T14:57:46.617Z
Learning: The values in `web/_data/serverState.ts` are used for testing purposes and should remain as hardcoded mock data to facilitate testing different scenarios.
Applied to files:
api/src/__test__/graphql/resolvers/rclone-api.service.test.ts
📚 Learning: 2025-02-03T18:57:28.442Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts:55-56
Timestamp: 2025-02-03T18:57:28.442Z
Learning: Using console.log is acceptable in test files, especially for debugging test execution. The logger instances in test files are often used as mocks/dependencies for the classes being tested, not for actual logging.
Applied to files:
api/src/__test__/graphql/resolvers/rclone-api.service.test.ts
📚 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:
api/src/unraid-api/cli/report.command.ts
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
Repo: unraid/api PR: 1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/__test__/core/utils/process/unraid-api-running.integration.test.tsapi/src/unraid-api/cli/nodemon.service.spec.tsapi/src/unraid-api/cli/__test__/report.command.test.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
Repo: unraid/api PR: 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.
Applied to files:
api/src/environment.ts
📚 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:
web/composables/gql/graphql.tsweb/src/composables/gql/index.ts
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
Repo: unraid/api PR: 1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Applied to files:
api/scripts/build.ts
📚 Learning: 2025-03-26T15:18:27.452Z
Learnt from: pujitm
Repo: unraid/api PR: 1252
File: plugin/docker-compose.yml:15-15
Timestamp: 2025-03-26T15:18:27.452Z
Learning: The files in the `api/deploy` directory (including `packed-pnpm-store.txz`) are gitignored build artifacts that are generated when the API is built. Building the API is a prerequisite before using the plugin builder container, as it needs access to these artifacts.
Applied to files:
api/scripts/build.ts
📚 Learning: 2025-02-04T16:42:21.782Z
Learnt from: elibosley
Repo: unraid/api PR: 1105
File: api/scripts/build.mjs:0-0
Timestamp: 2025-02-04T16:42:21.782Z
Learning: In Node.js build scripts, avoid manually copying node_modules as a cache layer. Instead, rely on npm's built-in caching mechanism and use direct `npm install` commands, as this ensures dependency consistency and proper version resolution while still maintaining good performance through npm's cache.
Applied to files:
api/scripts/build.ts
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
Repo: unraid/api PR: 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:
api/src/unraid-api/cli/cli-services.module.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
Repo: unraid/api PR: 1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Applied to files:
web/src/composables/gql/index.ts
📚 Learning: 2024-12-11T17:20:09.604Z
Learnt from: pujitm
Repo: unraid/api PR: 978
File: web/helpers/apollo-cache/index.ts:133-157
Timestamp: 2024-12-11T17:20:09.604Z
Learning: In `web/helpers/apollo-cache/index.ts`, when updating the cache in the `cache.updateQuery` function, it's sufficient to check if `data` is present, as type checking inside `data` is handled at the GraphQL layer.
Applied to files:
web/src/composables/gql/index.ts
📚 Learning: 2025-03-12T13:35:43.900Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
Applied to files:
web/src/composables/gql/index.ts
📚 Learning: 2025-02-04T13:48:45.521Z
Learnt from: pujitm
Repo: unraid/api PR: 1104
File: api/src/unraid-api/cli/stop.command.ts:14-18
Timestamp: 2025-02-04T13:48:45.521Z
Learning: The PM2Service's `run` method automatically handles logging when `raw: true` is not set in the context argument. Explicit logging is not needed in such cases.
Applied to files:
api/src/unraid-api/cli/logs.command.ts
🧬 Code graph analysis (13)
api/src/core/utils/process/unraid-api-running.ts (2)
packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)api/src/environment.ts (1)
NODEMON_PID_PATH(114-114)
api/src/unraid-api/cli/report.command.ts (1)
api/src/core/utils/process/unraid-api-running.ts (1)
isUnraidApiRunning(6-23)
api/src/__test__/core/utils/process/unraid-api-running.integration.test.ts (1)
api/src/core/utils/process/unraid-api-running.ts (1)
isUnraidApiRunning(6-23)
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
api/src/environment.ts (1)
getPackageJson(49-53)
api/src/unraid-api/cli/nodemon.service.spec.ts (1)
packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)
web/composables/gql/graphql.ts (1)
web/src/composables/gql/graphql.ts (2)
Maybe(3-3)Scalars(11-75)
api/src/unraid-api/cli/status.command.ts (4)
api/src/unraid-api/cli/logs.command.ts (1)
Command(9-25)api/src/unraid-api/cli/restart.command.ts (1)
Command(21-54)api/src/unraid-api/cli/start.command.ts (1)
Command(11-33)api/src/unraid-api/cli/stop.command.ts (1)
Command(8-28)
api/src/unraid-api/cli/stop.command.ts (4)
api/src/unraid-api/cli/logs.command.ts (1)
Command(9-25)api/src/unraid-api/cli/restart.command.ts (1)
Command(21-54)api/src/unraid-api/cli/start.command.ts (1)
Command(11-33)api/src/unraid-api/cli/status.command.ts (1)
Command(5-13)
api/src/unraid-api/cli/logs.command.ts (4)
api/src/unraid-api/cli/restart.command.ts (1)
Command(21-54)api/src/unraid-api/cli/start.command.ts (1)
Command(11-33)api/src/unraid-api/cli/status.command.ts (1)
Command(5-13)api/src/unraid-api/cli/stop.command.ts (1)
Command(8-28)
api/src/core/log.ts (1)
api/src/environment.ts (2)
SUPPRESS_LOGS(94-94)LOG_TYPE(88-88)
api/src/unraid-api/cli/nodemon.service.ts (3)
api/src/environment.ts (6)
PATHS_LOGS_DIR(101-102)NODEMON_PID_PATH(114-114)PATHS_LOGS_FILE(103-103)NODEMON_PATH(105-112)NODEMON_CONFIG_PATH(113-113)UNRAID_API_CWD(115-115)api/src/unraid-api/cli/log.service.ts (1)
error(65-69)packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)
api/src/unraid-api/cli/start.command.ts (4)
api/src/unraid-api/cli/logs.command.ts (1)
Command(9-25)api/src/unraid-api/cli/restart.command.ts (2)
Command(21-54)LogLevelOptions(9-11)api/src/unraid-api/cli/status.command.ts (1)
Command(5-13)api/src/unraid-api/cli/stop.command.ts (1)
Command(8-28)
web/src/composables/gql/graphql.ts (2)
api/src/unraid-api/cli/generated/graphql.ts (4)
CpuPackages(562-571)Node(1385-1387)Scalars(11-75)Maybe(3-3)web/composables/gql/graphql.ts (3)
Node(1357-1359)Scalars(11-75)Maybe(3-3)
🔇 Additional comments (26)
api/src/__test__/graphql/resolvers/rclone-api.service.test.ts (1)
54-55: LGTM! Environment mock standardization.The addition of
SUPPRESS_LOGSandLOG_LEVELto the environment mock aligns with the broader PM2-to-nodemon migration and standardizes the test configuration. Since the Logger is already mocked at lines 65-76, these additions don't affect test behavior but provide consistency with other test files.api/package.json (1)
140-140: Confirm nodemon is intended as a runtime dependencyAdding
nodemontodependenciesmakes sense if deployments rely on it as the process manager (mirroring how PM2 was treated). Just double‑check there are no usage contexts where@unraid/apiis consumed purely as a library and this heavier runtime dependency would be undesirable, and that release builds indeed neednodemonavailable at runtime.api/scripts/build.ts (1)
97-97: Including nodemon.json in the release pack matches nodemon-managed deploymentsUpdating the pack step to
cp -r dist README.md .env.* nodemon.json ./deploy/pack/ensures the nodemon configuration is shipped with the release, which aligns with switching process management from PM2 to nodemon.Just confirm that
api/nodemon.jsonis always present in the repo (and in any CI/build contexts), since a missing file here will now cause the build to fail at the copy step.web/src/composables/gql/index.ts (1)
1-2: Stylistic quote change only; exports remain the sameOnly the quote style changed here; the exported modules and paths are unchanged, so there’s no behavioral impact.
api/legacy/generated-schema-legacy.graphql (1)
1260-1260: Renaming Versions.pm2 → Versions.nodemon is a schema-breaking changeUpdating the
Versionstype to exposenodemon: Stringinstead ofpm2aligns with the runtime change, but it is a breaking change for any clients that still querypm2against the legacy schema.If all consumers (including anything relying on this “legacy” schema) are updated in lockstep with this PR, this is fine. Otherwise, consider temporarily exposing both fields, or at least coordinating a versioned rollout so older clients don’t start failing GraphQL queries unexpectedly.
api/src/unraid-api/cli/report.command.ts (1)
36-40: LGTM!The import path and error message have been correctly updated to reflect the migration from PM2 to nodemon-based process management.
api/src/core/utils/process/unraid-api-running.ts (1)
6-23: LGTM!The implementation correctly checks if the Unraid API process is running by:
- Verifying the PID file exists
- Reading and validating the PID
- Using
process.kill(pid, 0)to check process liveness without sending a signalThis is a standard and reliable approach for process health checking.
api/generated-schema.graphql (1)
1676-1677: LGTM!The GraphQL schema has been correctly updated to replace the
pm2field withnodemonin thePackageVersionstype. This is an auto-generated file that reflects the changes in the underlying model.api/src/unraid-api/cli/__test__/report.command.test.ts (1)
29-34: LGTM!The test mocks have been correctly updated to reference the new process-based utility instead of PM2-specific code.
api/src/unraid-api/cli/cli-services.module.ts (2)
7-7: LGTM!The import has been correctly updated to use
NodemonServiceinstead ofPM2Service, aligning with the broader migration to nodemon-based process management.
24-24: LGTM!The providers array has been correctly updated to register
NodemonServiceinstead ofPM2Service.api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts (1)
28-29: LGTM!The model has been correctly updated to replace the
pm2field withnodemon, consistent with the broader migration to nodemon-based process management.api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
6-6: LGTM!The import of
getPackageJsonis correctly added to support retrieving the nodemon version from package.json.api/src/__test__/core/utils/process/unraid-api-running.integration.test.ts (1)
7-54: Excellent test coverage for nodemon PID detection.The integration test is well-structured with proper setup/teardown lifecycle management and covers the key scenarios (missing file, valid PID, invalid PID). The use of dynamic imports with mocking ensures proper isolation between test cases.
api/src/unraid-api/cli/nodemon.service.spec.ts (1)
29-101: Comprehensive unit tests for NodemonService.The test suite provides excellent coverage of the service's key functionality: dependency setup, process starting, and status checking. The mocking strategy effectively isolates the service behavior, and the assertions verify all critical interactions with the filesystem and process management.
api/src/unraid-api/cli/status.command.ts (1)
3-11: Clean migration to NodemonService.The status command has been successfully migrated from PM2 to nodemon with a simplified implementation. The changes are consistent with the broader migration pattern across all CLI commands.
api/src/unraid-api/cli/logs.command.ts (1)
3-23: Successful migration to NodemonService.The logs command has been cleanly migrated to use NodemonService while maintaining the same functionality for viewing logs with configurable line counts.
web/composables/gql/graphql.ts (1)
1526-1527: GraphQL types updated to reflect nodemon migration.The generated types correctly reflect the schema change from
pm2tonodemonversion tracking. This aligns with the broader migration from PM2 to nodemon process management.api/src/unraid-api/cli/cli.module.ts (1)
16-67: Dependency injection updated for NodemonService.The CLI module has been correctly updated to provide NodemonService instead of PM2Service, ensuring all commands receive the new service through dependency injection.
api/src/unraid-api/cli/restart.command.ts (1)
5-35: Restart command successfully migrated to nodemon.The restart command has been cleanly migrated to use NodemonService with proper environment variable handling (uppercasing LOG_LEVEL). Error handling and logging remain consistent.
api/src/unraid-api/cli/stop.command.ts (1)
3-27: Excellent migration with improved naming.The stop command has been successfully migrated to NodemonService with a notable improvement: renaming the option from
--deleteto--forceprovides better semantic clarity for forcefully stopping the API process. The simplified implementation is cleaner and easier to maintain.web/src/composables/gql/graphql.ts (4)
562-571: CpuPackages type matches backend schema and Node interfaceThe new
CpuPackagesNode type and its fields (power/temp arrays plus totalPower) look consistent with the backend GraphQL snippet and existingNodeusage. No issues from the client typing perspective.
1548-1550: PackageVersions nodemon field aligns with pm2→nodemon migrationAdding
nodemon?: Maybe<Scalars['String']['output']>;toPackageVersions(and dropping pm2 on the API side) is consistent with the process manager swap. This keeps the web types in sync with the API’s version surface.
2069-2071: New CPU telemetry subscription field is consistent with CpuPackages
systemMetricsCpuTelemetry: CpuPackagescleanly reuses the new type for per-package telemetry and coexists withsystemMetricsCpu: CpuUtilization. Typing here looks correct; just ensure any consumers are updated when they start using this subscription.
883-901: Schema verification confirms code alignment – no action neededThe GraphQL schema in
api/generated-schema.graphql(lines 1474–1475) explicitly declares both fields as non-nullable:
topology: [[[Int!]!]!]!packages: CpuPackages!The
!suffix in GraphQL enforces non-nullability, meaning the server is required to return these fields. The TypeScript code correctly reflects this by not wrapping them inMaybe<...>. The code is properly aligned with the schema guarantees.Likely an incorrect or invalid review comment.
api/src/unraid-api/cli/nodemon.service.ts (1)
112-127: Status semantics and stale PID cleanup look good
statuscorrectly reports running/not-running and eagerly cleans up a stale PID file when the process is gone. Returning the boolean while still logging is useful for both CLI and programmatic callers; the implementation here looks solid.
elibosley
force-pushed
the
codex/switch-api-process-management-to-nodemon
branch
from
c6cb6d2 to
c1f8224
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
6-43: Nodemon version resolution is correct; consider caching for perfUsing
getPackageJson().dependencies?.nodemoninsidepackages()correctly reports the nodemon version and is safely wrapped in the existing try/catch. The only downside is a synchronouspackage.jsonread on each field resolution; ifinfo.versions.packagesis queried frequently, you might later introduce a simple in-memory cache forgetPackageJson()to avoid repeated disk I/O.api/src/unraid-api/cli/restart.command.ts (1)
23-36: Consider handlingLOG_LEVELfallback whenoptions.logLevelis absent
envis built as{ LOG_LEVEL: options.logLevel?.toUpperCase() }and then passed intothis.nodemon.restart({ env }). Ifrunis ever invoked programmatically without CLI option parsing (e.g.,restart.run()orrestart.run([], {})),options.logLevelwill beundefined, soLOG_LEVELwill also beundefinedin the env passed to NodemonService.Depending on how
NodemonService.restartusesenv, this could either:
- Silently override a default
LOG_LEVELwithundefined, or- Cause issues if child process spawning expects string env values.
To make this more robust, consider falling back to the configured default or omitting the key when it’s unset, e.g.:
- const env = { LOG_LEVEL: options.logLevel?.toUpperCase() }; - await this.nodemon.restart({ env }); + const logLevel = options.logLevel ?? LOG_LEVEL.toLowerCase(); + const env = { LOG_LEVEL: logLevel.toUpperCase() }; + await this.nodemon.restart({ env });(or filter out undefined keys inside
NodemonService).api/src/unraid-api/cli/start.command.ts (1)
8-8: AlignLOG_LEVELenv handling with a clear defaultSimilar to
RestartCommand,StartCommandbuildsenvas{ LOG_LEVEL: options.logLevel?.toUpperCase() }before callingthis.nodemon.start(...). While Nest Commander will typically supply a value due to the@Optiondefault, this method can still be called directly in code with an options object that omitslogLevel, which would propagateLOG_LEVEL: undefinedinto the NodemonService layer.To make behavior explicit and safer, consider normalizing the log level with a fallback to
LOG_LEVELand always passing a concrete string:- async run(_: string[], options: LogLevelOptions): Promise<void> { - this.logger.info('Starting the Unraid API'); - await this.nodemon.start({ env: { LOG_LEVEL: options.logLevel?.toUpperCase() } }); - } + async run(_: string[], options: LogLevelOptions): Promise<void> { + this.logger.info('Starting the Unraid API'); + const logLevel = options.logLevel ?? LOG_LEVEL.toLowerCase(); + await this.nodemon.start({ env: { LOG_LEVEL: logLevel.toUpperCase() } }); + }This mirrors the intended default and avoids leaking
undefinedinto the child process environment.Also applies to: 13-23
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
api/ecosystem.config.json(0 hunks)api/generated-schema.graphql(1 hunks)api/legacy/generated-schema-legacy.graphql(1 hunks)api/nodemon.json(1 hunks)api/package.json(1 hunks)api/scripts/build.ts(2 hunks)api/src/__test__/core/utils/pm2/dummy-process.js(0 hunks)api/src/__test__/core/utils/pm2/unraid-api-running.integration.test.ts(0 hunks)api/src/__test__/core/utils/process/unraid-api-running.integration.test.ts(1 hunks)api/src/__test__/graphql/resolvers/rclone-api.service.test.ts(1 hunks)api/src/core/log.ts(1 hunks)api/src/core/utils/pm2/unraid-api-running.ts(0 hunks)api/src/core/utils/process/unraid-api-running.ts(1 hunks)api/src/environment.ts(1 hunks)api/src/unraid-api/cli/__test__/report.command.test.ts(3 hunks)api/src/unraid-api/cli/cli-services.module.ts(2 hunks)api/src/unraid-api/cli/cli.module.ts(2 hunks)api/src/unraid-api/cli/logs.command.ts(2 hunks)api/src/unraid-api/cli/nodemon.service.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.ts(1 hunks)api/src/unraid-api/cli/pm2.service.spec.ts(0 hunks)api/src/unraid-api/cli/pm2.service.ts(0 hunks)api/src/unraid-api/cli/report.command.ts(1 hunks)api/src/unraid-api/cli/restart.command.ts(2 hunks)api/src/unraid-api/cli/start.command.ts(1 hunks)api/src/unraid-api/cli/status.command.ts(1 hunks)api/src/unraid-api/cli/stop.command.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts(2 hunks)api/src/unraid-api/main.ts(0 hunks)web/composables/gql/graphql.ts(1 hunks)web/src/composables/gql/graphql.ts(5 hunks)web/src/composables/gql/index.ts(1 hunks)
💤 Files with no reviewable changes (7)
- api/src/unraid-api/main.ts
- api/src/test/core/utils/pm2/unraid-api-running.integration.test.ts
- api/src/test/core/utils/pm2/dummy-process.js
- api/src/core/utils/pm2/unraid-api-running.ts
- api/ecosystem.config.json
- api/src/unraid-api/cli/pm2.service.ts
- api/src/unraid-api/cli/pm2.service.spec.ts
✅ Files skipped from review due to trivial changes (1)
- web/src/composables/gql/index.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- api/package.json
- api/src/unraid-api/cli/cli-services.module.ts
- api/src/unraid-api/cli/report.command.ts
- api/scripts/build.ts
- api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts
- api/src/unraid-api/cli/test/report.command.test.ts
- api/src/test/core/utils/process/unraid-api-running.integration.test.ts
- web/composables/gql/graphql.ts
- api/nodemon.json
- api/src/unraid-api/cli/status.command.ts
- api/src/unraid-api/cli/stop.command.ts
- api/src/unraid-api/cli/nodemon.service.spec.ts
- api/src/unraid-api/cli/nodemon.service.ts
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-02-03T18:57:28.442Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts:55-56
Timestamp: 2025-02-03T18:57:28.442Z
Learning: Using console.log is acceptable in test files, especially for debugging test execution. The logger instances in test files are often used as mocks/dependencies for the classes being tested, not for actual logging.
Applied to files:
api/src/__test__/graphql/resolvers/rclone-api.service.test.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
Repo: unraid/api PR: 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.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-02-05T14:43:48.568Z
Learnt from: elibosley
Repo: unraid/api PR: 1120
File: plugin/package.json:0-0
Timestamp: 2025-02-05T14:43:48.568Z
Learning: In Node.js projects, npm scripts should be organized with clear namespacing (e.g., build:*, docker:*, env:*) and include proper environment validation and error handling. Each script should follow the single responsibility principle.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-09-14T14:07:28.265Z
Learnt from: elibosley
Repo: unraid/api PR: 1702
File: web/eslint.config.mjs:106-106
Timestamp: 2025-09-14T14:07:28.265Z
Learning: The unraid/api codebase specifically requires Node 20+ as a minimum version, so modern Node.js features like import.meta.dirname (available since Node v20.11.0) can be used without compatibility fallbacks.
Applied to files:
api/src/environment.tsapi/src/core/utils/process/unraid-api-running.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts
📚 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:
api/src/core/utils/process/unraid-api-running.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
Repo: unraid/api PR: 942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.
Applied to files:
api/src/core/utils/process/unraid-api-running.ts
📚 Learning: 2025-01-23T21:26:17.766Z
Learnt from: elibosley
Repo: unraid/api PR: 1063
File: api/src/unraid-api/cli/sso/validate-token.command.ts:76-81
Timestamp: 2025-01-23T21:26:17.766Z
Learning: In the unraid/api codebase, store dispatches using Redux Toolkit's `unwrap()` method already handle errors appropriately and don't require additional try-catch blocks.
Applied to files:
api/src/core/utils/process/unraid-api-running.ts
📚 Learning: 2025-02-05T14:43:25.062Z
Learnt from: elibosley
Repo: unraid/api PR: 1120
File: plugin/package.json:1-8
Timestamp: 2025-02-05T14:43:25.062Z
Learning: The repository uses Renovate for automated dependency updates, making strict version pinning in package.json less critical as updates are handled automatically through PRs.
Applied to files:
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts
📚 Learning: 2025-02-04T13:48:45.521Z
Learnt from: pujitm
Repo: unraid/api PR: 1104
File: api/src/unraid-api/cli/stop.command.ts:14-18
Timestamp: 2025-02-04T13:48:45.521Z
Learning: The PM2Service's `run` method automatically handles logging when `raw: true` is not set in the context argument. Explicit logging is not needed in such cases.
Applied to files:
api/src/unraid-api/cli/logs.command.ts
🧬 Code graph analysis (6)
api/src/core/log.ts (1)
api/src/environment.ts (2)
SUPPRESS_LOGS(94-94)LOG_TYPE(88-88)
api/src/core/utils/process/unraid-api-running.ts (2)
packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)api/src/environment.ts (1)
NODEMON_PID_PATH(114-114)
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
api/src/environment.ts (1)
getPackageJson(49-53)
api/src/unraid-api/cli/logs.command.ts (4)
api/src/unraid-api/cli/restart.command.ts (1)
Command(21-54)api/src/unraid-api/cli/start.command.ts (1)
Command(11-33)api/src/unraid-api/cli/status.command.ts (1)
Command(5-13)api/src/unraid-api/cli/stop.command.ts (1)
Command(8-28)
web/src/composables/gql/graphql.ts (2)
api/src/unraid-api/cli/generated/graphql.ts (4)
CpuPackages(562-571)Node(1385-1387)Scalars(11-75)Maybe(3-3)web/composables/gql/graphql.ts (3)
Node(1357-1359)Scalars(11-75)Maybe(3-3)
api/src/unraid-api/cli/start.command.ts (4)
api/src/unraid-api/cli/logs.command.ts (1)
Command(9-25)api/src/unraid-api/cli/restart.command.ts (2)
Command(21-54)LogLevelOptions(9-11)api/src/unraid-api/cli/status.command.ts (1)
Command(5-13)api/src/unraid-api/cli/stop.command.ts (1)
Command(8-28)
🔇 Additional comments (14)
api/src/core/log.ts (1)
20-28: LGTM! Documentation updates align with nodemon migration.The comment updates correctly reflect the change from PM2-managed stdout to direct file piping. Disabling colorization (
colorize: false) remains appropriate to prevent ANSI escape codes from being written to the log file.api/src/__test__/graphql/resolvers/rclone-api.service.test.ts (1)
54-55: LGTM! Environment mock extended appropriately.The additions of
SUPPRESS_LOGSandLOG_LEVELto the environment mock are reasonable and align with standard testing practices. These logging configuration values are likely required by the code under test or its dependencies following the nodemon migration.api/generated-schema.graphql (1)
1676-1677: Schema nodemon version field looks consistentThe new
nodemon: Stringfield and its description align with the rest ofPackageVersionsand with the pm2→nodemon migration; no issues from the schema side.api/legacy/generated-schema-legacy.graphql (1)
1244-1271: Legacy Versions now exposes nodemon version correctlyAdding
nodemon: Stringto the legacyVersionstype keeps it in sync with the new schema and avoids divergence between legacy and current APIs.web/src/composables/gql/graphql.ts (4)
562-571: CpuPackages client type matches server schemaThe new
CpuPackagestype (Node +power,temp,totalPower) matches the schema and CLI-generated types; no adjustments needed.
883-901: InfoCpu packages/topology fields correctly modeledAdding
packages: CpuPackagesand the strongly-typedtopology: Array<Array<Array<Scalars['Int']['output']>>>matches the non-null schema definitions and should work as long as the backend resolvers always populate them.If not already covered, double-check the InfoCpu resolver/model always sets both fields to avoid
Cannot return null for non-nullable fielderrors under load.
1548-1549: PackageVersions nodemon field aligns with backendReplacing the pm2 field with
nodemon?: Maybe<Scalars['String']['output']>and its docstring keeps the frontend typings in sync with the updatedPackageVersionsschema.
2061-2071: Subscription wiring for systemMetricsCpuTelemetry looks correct
systemMetricsCpuTelemetry: CpuPackagesonSubscriptionmirrors the server-side addition and reuses the new CpuPackages type as expected.api/src/environment.ts (1)
105-115: Nodemon path and CWD constants now match repo layoutThe new
NODEMON_PATH,NODEMON_CONFIG_PATH,NODEMON_PID_PATH, andUNRAID_API_CWDvalues correctly resolve to root-levelnode_modules/nodemon,api/nodemon.json, the PID file, and the API working directory, fixing the earlier off-by-one issue while remaining overrideable via env vars.api/src/core/utils/process/unraid-api-running.ts (1)
1-23: Process-running check is robust and race-safe
isUnraidApiRunningcorrectly guards all I/O andprocess.killcalls in a try/catch, handles missing/invalid PID values, and safely returnsfalseon any error or race with the pidfile. This gives a clean boolean API for callers without leaking low-level errors.api/src/unraid-api/cli/logs.command.ts (1)
3-24: Logs command correctly migrated to NodemonServiceInjecting
NodemonServiceand delegatingrun()tothis.nodemon.logs(lines)aligns this command with the rest of the nodemon-based CLI lifecycle commands; option parsing semantics are unchanged.api/src/unraid-api/cli/cli.module.ts (1)
16-71: CLI module wiring for NodemonService is correctAdding
NodemonServiceto the imports andDEFAULT_PROVIDERSensures all lifecycle commands (start/stop/status/restart/logs/report) can inject it without additional module changes; the rest of the provider list is untouched.api/src/unraid-api/cli/restart.command.ts (1)
5-5: Consistent use ofLOG_LEVELfor CLI defaults looks goodImporting
LOG_LEVELfrom@app/environment.jsand reusing it as thedefaultValuefor the--log-leveloption keeps CLI defaults aligned with the runtime environment and avoids magic strings. Combined withparseLogLevelOption, this keeps validation centralized.Also applies to: 46-53
api/src/unraid-api/cli/start.command.ts (1)
6-6: Nice reuse of shared log-level parsing and defaultsUsing
LOG_LEVEL.toLowerCase()as thedefaultValueand delegating validation toparseLogLevelOptionreused fromrestart.command.tskeeps CLI behavior consistent acrossstartandrestartand avoids duplicating allowed-level logic.Also applies to: 25-32
elibosley
force-pushed
the
codex/switch-api-process-management-to-nodemon
branch
from
c1f8224 to
8f2c826
Compare
…demonService - Updated nodemon.json to remove unnecessary watch entry. - Adjusted NODEMON_CONFIG_PATH and UNRAID_API_CWD paths for better structure. - Enhanced error handling in isUnraidApiRunning and start methods of NodemonService to ensure proper logging and resource management. - Added tests for error scenarios in NodemonService to ensure robustness.
- Introduced UNRAID_API_ROOT to streamline path definitions for nodemon. - Replaced direct usage of import.meta.dirname with UNRAID_API_ROOT in NODEMON_PATH and UNRAID_API_CWD for improved clarity and maintainability. - Added dirname import to facilitate the new path structure.
elibosley
force-pushed
the
codex/switch-api-process-management-to-nodemon
branch
from
8f2c826 to
b35da13
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
api/src/core/log.ts (1)
20-28: LGTM! Comment updates align with the PM2-to-nodemon migration.The comment changes appropriately remove PM2-specific references and make the rationale more generic. The logic remains unchanged, and the comments accurately reflect the deployment context where stdout is redirected to log files.
Optional refinement for clarity:
Consider being more explicit that stdout is redirected rather than stating logs are written directly to file. For example:
-// Since process output is piped directly to the log file, we should not colorize stdout +// Since stdout is redirected to a log file in production, we should not colorize outputand
- colorize: false, // No colors since logs are written directly to file + colorize: false, // No colors since stdout is redirected to fileThis would more precisely reflect that
pino.destination()writes to stdout by default, with external redirection handling the file output.api/src/unraid-api/cli/nodemon.service.ts (2)
55-98: Consider handling execa’s promise to avoid unhandled rejections
start()intentionally treats nodemon as a background daemon and never awaits theexecapromise, but when nodemon fails very early (spawn error or immediate non‑zero exit), that promise can reject without a.catch, potentially surfacing as an unhandled‑rejection warning even though you also throw a synchronousErrorhere.You can keep the fire‑and‑forget behavior but explicitly observe the promise, for example:
- nodemonProcess = execa(NODEMON_PATH, ['--config', NODEMON_CONFIG_PATH, '--quiet'], { + nodemonProcess = execa(NODEMON_PATH, ['--config', NODEMON_CONFIG_PATH, '--quiet'], { cwd: UNRAID_API_CWD, env, detached: true, stdio: ['ignore', 'pipe', 'pipe'], }); + + // Prevent unhandled rejection noise for this long-running daemon. + nodemonProcess.catch(() => { + // Intentionally ignored; lifecycle is managed via PID + status(). + });This keeps the external behavior the same while hardening against noisy unhandled rejections.
100-118: Stop semantics are acceptable but always clear the PID
stop()is straightforward and robust against missing PID files; note that it removes the PID file even whenprocess.killthrows (e.g., ESRCH). That’s arguably desirable cleanup, but if you ever need to debug “couldn’t signal process X”, consider logging at warn/error as you already do and possibly guarding the PID removal by error type in a follow-up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
api/ecosystem.config.json(0 hunks)api/generated-schema.graphql(1 hunks)api/legacy/generated-schema-legacy.graphql(1 hunks)api/nodemon.json(1 hunks)api/package.json(1 hunks)api/scripts/build.ts(2 hunks)api/src/__test__/core/utils/pm2/dummy-process.js(0 hunks)api/src/__test__/core/utils/pm2/unraid-api-running.integration.test.ts(0 hunks)api/src/__test__/core/utils/process/unraid-api-running.integration.test.ts(1 hunks)api/src/__test__/graphql/resolvers/rclone-api.service.test.ts(1 hunks)api/src/core/log.ts(1 hunks)api/src/core/utils/pm2/unraid-api-running.ts(0 hunks)api/src/core/utils/process/unraid-api-running.ts(1 hunks)api/src/environment.ts(3 hunks)api/src/unraid-api/cli/__test__/report.command.test.ts(3 hunks)api/src/unraid-api/cli/cli-services.module.ts(2 hunks)api/src/unraid-api/cli/cli.module.ts(2 hunks)api/src/unraid-api/cli/logs.command.ts(2 hunks)api/src/unraid-api/cli/nodemon.service.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.ts(1 hunks)api/src/unraid-api/cli/pm2.service.spec.ts(0 hunks)api/src/unraid-api/cli/pm2.service.ts(0 hunks)api/src/unraid-api/cli/report.command.ts(1 hunks)api/src/unraid-api/cli/restart.command.ts(2 hunks)api/src/unraid-api/cli/start.command.ts(1 hunks)api/src/unraid-api/cli/status.command.ts(1 hunks)api/src/unraid-api/cli/stop.command.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts(2 hunks)api/src/unraid-api/main.ts(0 hunks)web/composables/gql/graphql.ts(1 hunks)web/src/composables/gql/graphql.ts(5 hunks)web/src/composables/gql/index.ts(1 hunks)
💤 Files with no reviewable changes (7)
- api/src/unraid-api/cli/pm2.service.spec.ts
- api/src/test/core/utils/pm2/unraid-api-running.integration.test.ts
- api/src/unraid-api/main.ts
- api/src/core/utils/pm2/unraid-api-running.ts
- api/src/test/core/utils/pm2/dummy-process.js
- api/src/unraid-api/cli/pm2.service.ts
- api/ecosystem.config.json
✅ Files skipped from review due to trivial changes (1)
- web/src/composables/gql/index.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- api/src/unraid-api/cli/cli-services.module.ts
- api/src/test/core/utils/process/unraid-api-running.integration.test.ts
- api/src/unraid-api/cli/nodemon.service.spec.ts
- api/src/unraid-api/cli/test/report.command.test.ts
- api/src/unraid-api/cli/report.command.ts
- api/src/unraid-api/cli/start.command.ts
- api/src/unraid-api/cli/cli.module.ts
- api/nodemon.json
- api/package.json
- api/generated-schema.graphql
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
📚 Learning: 2025-02-04T13:48:45.521Z
Learnt from: pujitm
Repo: unraid/api PR: 1104
File: api/src/unraid-api/cli/stop.command.ts:14-18
Timestamp: 2025-02-04T13:48:45.521Z
Learning: The PM2Service's `run` method automatically handles logging when `raw: true` is not set in the context argument. Explicit logging is not needed in such cases.
Applied to files:
api/src/unraid-api/cli/logs.command.ts
📚 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:
web/composables/gql/graphql.ts
📚 Learning: 2025-09-14T14:07:28.265Z
Learnt from: elibosley
Repo: unraid/api PR: 1702
File: web/eslint.config.mjs:106-106
Timestamp: 2025-09-14T14:07:28.265Z
Learning: The unraid/api codebase specifically requires Node 20+ as a minimum version, so modern Node.js features like import.meta.dirname (available since Node v20.11.0) can be used without compatibility fallbacks.
Applied to files:
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.model.tsapi/src/core/utils/process/unraid-api-running.tsapi/src/environment.ts
📚 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:
api/src/core/utils/process/unraid-api-running.tsapi/src/environment.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
Repo: unraid/api PR: 942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.
Applied to files:
api/src/core/utils/process/unraid-api-running.tsapi/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-01-23T21:26:17.766Z
Learnt from: elibosley
Repo: unraid/api PR: 1063
File: api/src/unraid-api/cli/sso/validate-token.command.ts:76-81
Timestamp: 2025-01-23T21:26:17.766Z
Learning: In the unraid/api codebase, store dispatches using Redux Toolkit's `unwrap()` method already handle errors appropriately and don't require additional try-catch blocks.
Applied to files:
api/src/core/utils/process/unraid-api-running.ts
📚 Learning: 2025-02-03T18:57:28.442Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts:55-56
Timestamp: 2025-02-03T18:57:28.442Z
Learning: Using console.log is acceptable in test files, especially for debugging test execution. The logger instances in test files are often used as mocks/dependencies for the classes being tested, not for actual logging.
Applied to files:
api/src/__test__/graphql/resolvers/rclone-api.service.test.ts
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
Repo: unraid/api PR: 1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Applied to files:
api/scripts/build.tsapi/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-03-26T15:18:27.452Z
Learnt from: pujitm
Repo: unraid/api PR: 1252
File: plugin/docker-compose.yml:15-15
Timestamp: 2025-03-26T15:18:27.452Z
Learning: The files in the `api/deploy` directory (including `packed-pnpm-store.txz`) are gitignored build artifacts that are generated when the API is built. Building the API is a prerequisite before using the plugin builder container, as it needs access to these artifacts.
Applied to files:
api/scripts/build.ts
📚 Learning: 2025-02-04T16:42:21.782Z
Learnt from: elibosley
Repo: unraid/api PR: 1105
File: api/scripts/build.mjs:0-0
Timestamp: 2025-02-04T16:42:21.782Z
Learning: In Node.js build scripts, avoid manually copying node_modules as a cache layer. Instead, rely on npm's built-in caching mechanism and use direct `npm install` commands, as this ensures dependency consistency and proper version resolution while still maintaining good performance through npm's cache.
Applied to files:
api/scripts/build.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
Repo: unraid/api PR: 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.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-02-05T14:43:48.568Z
Learnt from: elibosley
Repo: unraid/api PR: 1120
File: plugin/package.json:0-0
Timestamp: 2025-02-05T14:43:48.568Z
Learning: In Node.js projects, npm scripts should be organized with clear namespacing (e.g., build:*, docker:*, env:*) and include proper environment validation and error handling. Each script should follow the single responsibility principle.
Applied to files:
api/src/environment.tsapi/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-04-23T20:19:42.542Z
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
Repo: unraid/api PR: 1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-04-23T20:22:36.001Z
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: api/src/unraid-api/app/pubsub.module.ts:24-28
Timestamp: 2025-04-23T20:22:36.001Z
Learning: In the Unraid API project, string literals are intentionally used for dependency injection tokens instead of Symbols to enable "collision" behavior across modules that don't directly import from each other. This is a temporary approach during refactoring until a shared library for constants is implemented.
Applied to files:
api/src/environment.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/environment.ts
📚 Learning: 2025-04-21T18:25:16.491Z
Learnt from: pujitm
Repo: unraid/api PR: 1352
File: api/src/unraid-api/cli/dependency.service.ts:114-117
Timestamp: 2025-04-21T18:25:16.491Z
Learning: Error masking through try/catch blocks is not desirable for the `npmInstall` method in `DependencyService` as errors should propagate to the caller.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-04-21T18:25:16.491Z
Learnt from: pujitm
Repo: unraid/api PR: 1352
File: api/src/unraid-api/cli/dependency.service.ts:114-117
Timestamp: 2025-04-21T18:25:16.491Z
Learning: Methods in `DependencyService` like `npmInstall` and `rebuildVendorArchive` intentionally propagate errors from execa calls upward rather than handling them internally, allowing callers to implement appropriate error handling. This follows the pattern documented in the `rebuildVendorArchive` method's docstring.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-03-13T16:17:55.820Z
Learnt from: elibosley
Repo: unraid/api PR: 1211
File: api/src/unraid-api/graph/connect/connect-settings.service.ts:48-53
Timestamp: 2025-03-13T16:17:55.820Z
Learning: In NestJS services, dynamic imports should be wrapped in try/catch blocks with proper error logging and fallback handling to prevent unhandled rejections from crashing the application.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
🧬 Code graph analysis (9)
api/src/unraid-api/cli/logs.command.ts (4)
api/src/unraid-api/cli/restart.command.ts (1)
Command(21-54)api/src/unraid-api/cli/start.command.ts (1)
Command(11-33)api/src/unraid-api/cli/status.command.ts (1)
Command(5-13)api/src/unraid-api/cli/stop.command.ts (1)
Command(8-28)
web/composables/gql/graphql.ts (1)
web/src/composables/gql/graphql.ts (2)
Maybe(3-3)Scalars(11-75)
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
api/src/environment.ts (1)
getPackageJson(49-53)
api/src/unraid-api/cli/status.command.ts (4)
api/src/unraid-api/cli/logs.command.ts (1)
Command(9-25)api/src/unraid-api/cli/restart.command.ts (1)
Command(21-54)api/src/unraid-api/cli/start.command.ts (1)
Command(11-33)api/src/unraid-api/cli/stop.command.ts (1)
Command(8-28)
api/src/core/log.ts (1)
api/src/environment.ts (2)
SUPPRESS_LOGS(95-95)LOG_TYPE(89-89)
api/src/core/utils/process/unraid-api-running.ts (2)
packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)api/src/environment.ts (1)
NODEMON_PID_PATH(108-108)
web/src/composables/gql/graphql.ts (1)
api/src/unraid-api/cli/generated/graphql.ts (3)
CpuPackages(562-571)Node(1385-1387)Scalars(11-75)
api/src/unraid-api/cli/stop.command.ts (4)
api/src/unraid-api/cli/logs.command.ts (1)
Command(9-25)api/src/unraid-api/cli/restart.command.ts (1)
Command(21-54)api/src/unraid-api/cli/start.command.ts (1)
Command(11-33)api/src/unraid-api/cli/status.command.ts (1)
Command(5-13)
api/src/unraid-api/cli/nodemon.service.ts (3)
api/src/environment.ts (6)
PATHS_LOGS_DIR(102-103)NODEMON_PID_PATH(108-108)PATHS_LOGS_FILE(104-104)NODEMON_PATH(106-106)NODEMON_CONFIG_PATH(107-107)UNRAID_API_CWD(109-109)packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)api/src/unraid-api/cli/log.service.ts (1)
error(65-69)
🔇 Additional comments (20)
api/scripts/build.ts (1)
97-97: LGTM! Nodemon config copy aligns with PM2 migration.The change from copying
ecosystem.config.jsontonodemon.jsoncorrectly reflects the migration from PM2 to nodemon process management, ensuring the nodemon configuration is packaged in the release artifacts.api/legacy/generated-schema-legacy.graphql (1)
1260-1260: LGTM! GraphQL schema updated for nodemon migration.The field change from
pm2: Stringtonodemon: Stringin theVersionstype correctly reflects the transition from PM2 to nodemon process management in the GraphQL API surface.api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts (1)
28-29: LGTM! Model definition updated consistently with schema.The field change from
pm2?: stringtonodemon?: stringwith the updated description properly reflects the nodemon migration and maintains consistency with the GraphQL schema changes.web/composables/gql/graphql.ts (1)
1526-1527: LGTM! Generated types updated from GraphQL schema.The
PackageVersionstype change frompm2tonodemonis auto-generated by GraphQL CodeGen and correctly reflects the updated schema. As learnings indicate, the generated code structure is intentional for proper type system function.api/src/environment.ts (1)
5-5: LGTM! Clean path derivation using UNRAID_API_ROOT.The introduction of
UNRAID_API_ROOTas a base for nodemon-related paths is a solid improvement over the previous approach. Usingdirname(getPackageJsonPath())provides a reliable anchor point for:
NODEMON_CONFIG_PATH→api/nodemon.jsonNODEMON_PATH→api/node_modules/nodemon/bin/nodemon.js(valid in pnpm workspaces)UNRAID_API_CWD→api/(correct working directory)This approach is more maintainable and addresses the path construction issues noted in previous reviews.
Also applies to: 68-68, 106-109
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
6-6: Clarify nodemon version format - dependency declaration vs installed version.The code now reads the nodemon version from
package.jsondependencies usinggetPackageJson().dependencies?.nodemon. This will return the dependency declaration (e.g.,"^3.0.0") rather than the actual installed version.Is it acceptable to show the semver range specification, or should this display the resolved installed version instead? If the latter, consider using
await versions()from systeminformation (which is already imported) or reading fromnode_modules/nodemon/package.json.Current behavior: Shows
"^3.0.0"(declared dependency)
Alternative: Show"3.1.4"(installed version)Also applies to: 38-38
api/src/__test__/graphql/resolvers/rclone-api.service.test.ts (1)
54-55: LGTM! Test mock extended with logging controls.Adding
SUPPRESS_LOGSandLOG_LEVELto the environment mock provides proper test-level control over logging behavior, aligning with the logging controls used in the nodemon-based process management workflow.api/src/core/utils/process/unraid-api-running.ts (1)
1-23: LGTM! Robust nodemon process check with proper error handling.The implementation correctly verifies if the Unraid API is running under nodemon by:
- Checking PID file existence at
NODEMON_PID_PATH- Reading and parsing the PID
- Verifying the process is alive using
process.kill(pid, 0)The comprehensive try-catch wrapper properly handles all error cases (missing file, I/O errors, invalid PID, dead process) by returning
false, addressing the concerns raised in previous reviews. This provides a clean boolean interface without error leakage.web/src/composables/gql/graphql.ts (4)
562-571: CpuPackages type matches backend schemaThe
CpuPackagesnode shape (id/power/temp/totalPower) aligns with the CLI-generated GraphQL types and looks correct for client usage.
883-901: InfoCpu extensions are consistent and well-documentedAdding
packages: CpuPackagesand thetopologytriple-nested array with an example docstring makes the per-package CPU view clear and type-safe.
1548-1549: nodemon version field fits PackageVersions modelIntroducing the optional
nodemonversion alongside existing package fields cleanly reflects the runtime process manager swap.
2070-2070: CPU telemetry subscription wiring looks correct
systemMetricsCpuTelemetry: CpuPackagesmatches the newCpuPackagesnode and should integrate cleanly with telemetry consumers.api/src/unraid-api/cli/nodemon.service.ts (4)
34-44: PID file handling helper is reasonable
ensureNodemonDependencies+getStoredPid/parseIntgive a clear, small surface for PID management; letting mkdir errors propagate is appropriate so callers can abort early.
46-53: Process liveness check is simple and idiomaticUsing
process.kill(pid, 0)with a try/catch to probe liveness is a standard, low-overhead approach here.
125-140: Status correctly reconciles stale PID files
status()returning a boolean and cleaning up stale PID files when the process is no longer running provides a good single source of truth for higher-level commands.
142-160: Logs helper behavior is caller-friendly
logs()tails viaexeca('tail', ...), logs the content, and returns it, while gracefully handling file-missing/errors and emitting clear messages. Returning an empty string on failure is a reasonable, non-throwing contract for CLI callers.api/src/unraid-api/cli/logs.command.ts (1)
3-24: LogsCommand–NodemonService integration looks cleanConstructor injection of
NodemonServiceand delegatinglogs(lines)after simple, defensive parsing of the--linesoption is straightforward and keeps CLI logic thin.api/src/unraid-api/cli/status.command.ts (1)
3-12: StatusCommand correctly delegates to NodemonServiceSwapping to
NodemonServiceand simply awaitingnodemon.status()keeps the command minimal and aligns with the new process-manager abstraction.api/src/unraid-api/cli/restart.command.ts (1)
5-53: Restart flow via NodemonService is consistent and robustDelegating restarts to
nodemon.restart({ env })with normalizedLOG_LEVEL, plus structured try/catch that logs a clear message and exits with code 1 on failure, gives a clean and predictable restart UX in line with the start command.api/src/unraid-api/cli/stop.command.ts (1)
3-27: StopCommand force option and NodemonService wiring make intent clearInjecting
NodemonServiceand exposing a-f, --forceflag that maps directly to{ force }fornodemon.stopmakes the stop behavior explicit (SIGTERM vs SIGKILL) while keeping the command logic minimal.
| import { $, cd } from 'zx'; | ||
|
|
||
| import { getDeploymentVersion } from './get-deployment-version.js'; | ||
| import { getDeploymentVersion } from '@app/../scripts/get-deployment-version.js'; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the import path change for getDeploymentVersion.
The import path changed from './get-deployment-version.js' (resolving to api/scripts/get-deployment-version.js) to '@app/../scripts/get-deployment-version.js' (resolving to scripts/get-deployment-version.js at repo root). This suggests the file was moved from the api/scripts directory to a root-level scripts directory.
Run the following script to verify the file exists at the expected location:
🏁 Script executed:
#!/bin/bash
# Description: Verify get-deployment-version.js exists at the new location
echo "Checking for get-deployment-version.js..."
if [ -f "scripts/get-deployment-version.js" ]; then
echo "✓ Found at scripts/get-deployment-version.js"
echo "File contents preview:"
head -10 scripts/get-deployment-version.js
elif [ -f "api/scripts/get-deployment-version.js" ]; then
echo "⚠ File still exists at old location: api/scripts/get-deployment-version.js"
echo "The import path may be incorrect"
else
echo "✗ File not found at either location"
fiLength of output: 77
The file doesn't exist at either location. Let me search the entire repository to determine where this file actually is:
🏁 Script executed:
#!/bin/bash
# Search for any files with get-deployment-version in the name
echo "Searching for get-deployment-version files in repo..."
fd "get-deployment-version" --type f
echo -e "\n---"
echo "Checking if file exists anywhere with different extension..."
fd "get-deployment-version" --type f -e ts -e js -e mjs
echo -e "\n---"
echo "Searching for references to getDeploymentVersion..."
rg "getDeploymentVersion" -n --type ts --type js 2>/dev/null | head -20Length of output: 632
Let me verify the module alias configuration to understand how @app resolves:
🏁 Script executed:
#!/bin/bash
# Find tsconfig or path alias configuration
echo "Looking for tsconfig.json files..."
fd "tsconfig" --type f
echo -e "\n---"
echo "Checking for path alias configuration in tsconfig..."
rg '"@app"' --type json -A2 -B2
echo -e "\n---"
echo "Checking import in build.ts context..."
rg -n -B2 -A5 "import.*getDeploymentVersion" git/api/scripts/build.tsLength of output: 885
Let me check the specific tsconfig.json in the api directory where build.ts is located:
🏁 Script executed:
#!/bin/bash
# Check the api tsconfig.json for @app alias
echo "Contents of api/tsconfig.json:"
cat git/api/tsconfig.json
echo -e "\n---"
echo "Checking for package.json in api directory:"
cat git/api/package.json 2>/dev/null | head -40
echo -e "\n---"
echo "Double-checking - what is the correct relative import from build.ts to get-deployment-version.ts?"
echo "build.ts location: api/scripts/build.ts"
echo "get-deployment-version.ts location: api/scripts/get-deployment-version.ts"
echo "Correct import should be: './get-deployment-version.js' or './get-deployment-version.ts'"Length of output: 4417
Fix broken import path and extension in build.ts line 10.
The import path is incorrect on multiple levels:
@app/../scripts/get-deployment-version.jsresolves to./scripts/get-deployment-version.js(repo root) via the@appalias mapping ("@app/*": ["./src/*"]inapi/tsconfig.json)- The actual file is located at
api/scripts/get-deployment-version.ts(same directory asbuild.ts) - The file does not exist at the resolved path
Change line 10 to:
import { getDeploymentVersion } from './get-deployment-version.js';🤖 Prompt for AI Agents
In api/scripts/build.ts around line 10, the import path uses the @app alias and
points to @app/../scripts/get-deployment-version.js which resolves incorrectly
to the repo root and the target file doesn't exist there; update the import to
reference the local sibling module by changing it to import from
'./get-deployment-version.js' so it correctly loads
api/scripts/get-deployment-version.ts (compiled .js at runtime).
… handling - Introduced a new test file to validate nodemon path configurations, ensuring they anchor to the package root by default. - Enhanced the NodemonService to throw an error when nodemon exits immediately after starting, improving robustness. - Added tests to cover scenarios where nodemon fails to start, ensuring proper logging and resource cleanup.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/__test__/environment.nodemon-paths.test.ts (1)
1-28: Solid test for nodemon path derivation; consider adding a second case for explicit CWD.This test is well-structured: it safely snapshots/restores
UNRAID_API_CWD, resets modules before each run so@app/environment.jsre-evaluates with a clean env, and asserts the nodemon paths are anchored toUNRAID_API_ROOTas expected. No functional issues spotted.As a small enhancement, you might add a second test that sets
process.env.UNRAID_API_CWDbefore importing@app/environment.jsand asserts thatUNRAID_API_CWDreflects the override whileNODEMON_CONFIG_PATHandNODEMON_PATHstill anchor toUNRAID_API_ROOT. That would lock in both default and overridden behavior for the new env wiring.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/src/__test__/environment.nodemon-paths.test.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- api/src/unraid-api/cli/nodemon.service.spec.ts
- api/src/unraid-api/cli/nodemon.service.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
🧬 Code graph analysis (1)
api/src/__test__/environment.nodemon-paths.test.ts (1)
api/src/environment.ts (5)
environment(73-75)UNRAID_API_CWD(109-109)UNRAID_API_ROOT(68-68)NODEMON_CONFIG_PATH(107-107)NODEMON_PATH(106-106)
- Updated the return type of killSpy in nodemon.service.spec.ts to directly return a boolean instead of casting it, improving code clarity and maintainability.
- Implemented stopPm2IfRunning method to stop any running pm2-managed instances of unraid-api before starting nodemon. - Added findMatchingNodemonPids method to identify existing nodemon processes, improving resource management. - Updated start method to handle scenarios where a stored pid is running or dead, ensuring proper cleanup and logging. - Introduced new unit tests to validate the new functionality and ensure robustness in process handling.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
api/src/unraid-api/cli/nodemon.service.ts (3)
4-5: Remove unused imports flagged by static analysis
homedir(Line 4) andjoin(Line 5) are imported but never used. Please drop them to keep the module clean and satisfy the CodeQL notice.-import { homedir } from 'node:os'; -import { dirname, join } from 'node:path'; +import { dirname } from 'node:path';
35-38: Optionally add explicit return type to clarify intent
ensureNodemonDependenciescurrently relies on inference. For a public-ish service API, giving it an explicitPromise<void>return type makes the contract clearer and consistent with other async methods:- async ensureNodemonDependencies() { + async ensureNodemonDependencies(): Promise<void> { await mkdir(PATHS_LOGS_DIR, { recursive: true }); await mkdir(dirname(NODEMON_PID_PATH), { recursive: true }); }
140-182: Avoid double‑closinglogStreamand verify long‑running log behaviorTwo related points in
start:
- Double
logStream.close()on error paths
logStream.close()is called inside theif (!nodemonProcess.pid)andif (!stillRunning)branches (Lines 160 and 172) and then again in thecatchblock (Line 179). This is redundant and can lead to awkward double‑close behavior.You can rely on the
catchblock to close the stream and remove the inner closes:- if (!nodemonProcess.pid) { - logStream.close(); - throw new Error('Failed to start nodemon: process spawned but no PID was assigned'); - } + if (!nodemonProcess.pid) { + throw new Error('Failed to start nodemon: process spawned but no PID was assigned'); + } @@ - if (!stillRunning) { - const recentLogs = await this.logs(50); - await rm(NODEMON_PID_PATH, { force: true }); - logStream.close(); - const logMessage = recentLogs ? ` Recent logs:\n${recentLogs}` : ''; - throw new Error(`Nodemon exited immediately after start.${logMessage}`); - } + if (!stillRunning) { + const recentLogs = await this.logs(50); + await rm(NODEMON_PID_PATH, { force: true }); + const logMessage = recentLogs ? ` Recent logs:\n${recentLogs}` : ''; + throw new Error(`Nodemon exited immediately after start.${logMessage}`); + }
- Confirm that logs continue after the CLI process exits
Right now, nodemon’s
stdout/stderrare piped into acreateWriteStream(PATHS_LOGS_FILE)owned by this process. If the CLI process exits shortly afterstartreturns (which is typical for a detached daemon pattern), any further logging from nodemon will no longer go through this stream.If PATHS_LOGS_FILE is intended to capture long‑running API logs via this pipeline, consider instead wiring the log file directly via
stdioFDs in theexecaoptions so the child owns its log descriptors even after the CLI exits. For example (sketch only, adjust to your conventions):// Before spawning: const outFd = fs.openSync(PATHS_LOGS_FILE, 'a'); const errFd = fs.openSync(PATHS_LOGS_FILE, 'a'); const nodemonProcess = execa(NODEMON_PATH, ['--config', NODEMON_CONFIG_PATH, '--quiet'], { cwd: UNRAID_API_CWD, env, detached: true, stdio: ['ignore', outFd, errFd], });Then you can drop the
logStreamand explicit piping altogether.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api/src/unraid-api/cli/nodemon.service.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/cli/nodemon.service.spec.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
📚 Learning: 2025-04-21T18:25:16.491Z
Learnt from: pujitm
Repo: unraid/api PR: 1352
File: api/src/unraid-api/cli/dependency.service.ts:114-117
Timestamp: 2025-04-21T18:25:16.491Z
Learning: Error masking through try/catch blocks is not desirable for the `npmInstall` method in `DependencyService` as errors should propagate to the caller.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-04-21T18:25:16.491Z
Learnt from: pujitm
Repo: unraid/api PR: 1352
File: api/src/unraid-api/cli/dependency.service.ts:114-117
Timestamp: 2025-04-21T18:25:16.491Z
Learning: Methods in `DependencyService` like `npmInstall` and `rebuildVendorArchive` intentionally propagate errors from execa calls upward rather than handling them internally, allowing callers to implement appropriate error handling. This follows the pattern documented in the `rebuildVendorArchive` method's docstring.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-03-13T16:17:55.820Z
Learnt from: elibosley
Repo: unraid/api PR: 1211
File: api/src/unraid-api/graph/connect/connect-settings.service.ts:48-53
Timestamp: 2025-03-13T16:17:55.820Z
Learning: In NestJS services, dynamic imports should be wrapped in try/catch blocks with proper error logging and fallback handling to prevent unhandled rejections from crashing the application.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
Repo: unraid/api PR: 942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
Repo: unraid/api PR: 1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-02-05T14:43:48.568Z
Learnt from: elibosley
Repo: unraid/api PR: 1120
File: plugin/package.json:0-0
Timestamp: 2025-02-05T14:43:48.568Z
Learning: In Node.js projects, npm scripts should be organized with clear namespacing (e.g., build:*, docker:*, env:*) and include proper environment validation and error handling. Each script should follow the single responsibility principle.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
🧬 Code graph analysis (1)
api/src/unraid-api/cli/nodemon.service.ts (3)
api/src/environment.ts (6)
PATHS_LOGS_DIR(102-103)NODEMON_PID_PATH(108-108)NODEMON_CONFIG_PATH(107-107)PATHS_LOGS_FILE(104-104)NODEMON_PATH(106-106)UNRAID_API_CWD(109-109)packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)api/src/unraid-api/cli/log.service.ts (1)
error(65-69)
🪛 GitHub Check: CodeQL
api/src/unraid-api/cli/nodemon.service.ts
[notice] 4-4: Unused variable, import, function or class
Unused import homedir.
[notice] 5-5: Unused variable, import, function or class
Unused import join.
🔇 Additional comments (2)
api/src/unraid-api/cli/nodemon.service.ts (2)
70-84: PID file/liveness helpers look solid
getStoredPidandisPidRunningare straightforward and robust (NaN‑safe parsing andkill(pid, 0)check), which should work well with the rest of the lifecycle methods.
227-245: Nice, logs() now handles missing file/errors gracefullyWrapping the
tailcall in try/catch and distinguishing ENOENT from other failures yields much clearer diagnostics while returning a safe fallback string for callers.
- Implemented findDirectMainPids and terminatePids methods to identify and terminate existing unraid-api processes before starting nodemon. - Enhanced the start method to include checks for running processes, ensuring proper cleanup and logging. - Updated unit tests to validate the new process management functionality, improving overall robustness.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/unraid-api/cli/nodemon.service.spec.ts (1)
63-64: Simplify mock return values.The explicit type casting is unnecessary since
mockResolvedValue(undefined)correctly types toPromise<void>.Apply this diff to simplify:
- mockWriteFile.mockResolvedValue(undefined as unknown as void); - mockRm.mockResolvedValue(undefined as unknown as void); + mockWriteFile.mockResolvedValue(undefined); + mockRm.mockResolvedValue(undefined);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api/src/unraid-api/cli/nodemon.service.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/cli/nodemon.service.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
🧬 Code graph analysis (1)
api/src/unraid-api/cli/nodemon.service.spec.ts (1)
packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)
🔇 Additional comments (4)
api/src/unraid-api/cli/nodemon.service.spec.ts (4)
18-27: Environment mock is now complete.The addition of
SUPPRESS_LOGSresolves the previous pipeline failure. The mock now correctly mirrors all necessary exports fromenvironment.ts.
91-312: Comprehensive start method test coverage.The test suite thoroughly covers the
startmethod's behavior across multiple scenarios:
- ✅ Successful start with proper setup and PID file management
- ✅ Error handling with appropriate cleanup (closing streams, removing stale files)
- ✅ Edge cases (already running, stale PID files, immediate exit detection)
- ✅ Process discovery and adoption
- ✅ Termination of conflicting direct main.js processes
The tests properly verify that cleanup occurs on failures and that diagnostic information is collected when nodemon exits unexpectedly.
314-362: Well-tested status and logs methods.The tests for
statusandlogsmethods properly verify:
- Correct status reporting when PID file is absent
- Successful log retrieval via tail command
- Graceful error handling for missing log files (ENOENT)
- Proper error logging for tail command failures
1-363: Excellent test suite with thorough coverage.This test file demonstrates strong testing practices:
- ✅ Comprehensive coverage of all public methods
- ✅ Proper mock setup and isolation via
beforeEach- ✅ Both success and failure paths tested
- ✅ Error handling and cleanup verified
- ✅ Edge cases covered (stale PIDs, already running, immediate exit)
- ✅ Appropriate assertions for behavior and logging
The test suite provides confidence that NodemonService will handle various runtime scenarios correctly.
…n NodemonService - Added waitForNodemonExit method to ensure nodemon processes are fully terminated before restarting. - Updated restart method to call waitForNodemonExit, improving process management during restarts. - Introduced a new unit test to validate the behavior of the restart method, ensuring proper sequence of stop, wait, and start operations.
…tarts - Updated the start method to restart nodemon if a recorded pid is already running, ensuring proper cleanup and logging. - Modified the restart method to delegate to start, streamlining the process management logic. - Enhanced unit tests to validate the new behavior, including scenarios for cleaning up stray processes and ensuring fresh starts.
- Modified the log stream mock to use a file descriptor instead of pipe methods, aligning with the actual implementation in NodemonService. - Removed unnecessary stdout and stderr pipe mocks from unit tests, simplifying the test setup while maintaining functionality. - Ensured consistency between the test and implementation for improved clarity and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
api/src/unraid-api/cli/nodemon.service.spec.ts (3)
62-64: Consider simplifying void return type casts.The casts
undefined as unknown as voidare unnecessary sincemockResolvedValue(undefined)is sufficient for functions returningPromise<void>.- mockWriteFile.mockResolvedValue(undefined as unknown as void); - mockRm.mockResolvedValue(undefined as unknown as void); + mockWriteFile.mockResolvedValue(undefined); + mockRm.mockResolvedValue(undefined);
319-327: Consider adding a test for the "running" status case.The test suite only covers the scenario where the PID file is missing. Consider adding a test that verifies the behavior when the PID file exists and the process is running.
Example test to add:
it('returns running status when pid file exists and process is running', async () => { const service = new NodemonService(logger); vi.mocked(fileExists).mockResolvedValue(true); vi.spyOn( service as unknown as { getStoredPid: () => Promise<number | null> }, 'getStoredPid' ).mockResolvedValue(123); vi.spyOn( service as unknown as { isPidRunning: (pid: number) => Promise<boolean> }, 'isPidRunning' ).mockResolvedValue(true); const result = await service.status(); expect(result).toBe(true); expect(logger.info).toHaveBeenCalledWith(expect.stringContaining('running')); });
369-423: LGTM, but consider adding directstop()method tests.The restart tests are well-structured and correctly verify:
- Waiting for nodemon exit before restarting
- Clean start when nodemon is not running (no stop called)
However, there's no dedicated test for the
stop()method itself. Consider adding tests to verify:
- Stop behavior when PID file exists and process is running
- Stop behavior when PID file doesn't exist
- Stop with
{ quiet: true }optionWould you like me to help generate test cases for the
stop()method?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api/src/unraid-api/cli/nodemon.service.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/cli/nodemon.service.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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:
api/src/unraid-api/cli/nodemon.service.spec.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/cli/nodemon.service.spec.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:
api/src/unraid-api/cli/nodemon.service.spec.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/cli/nodemon.service.spec.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
api/src/unraid-api/cli/nodemon.service.spec.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Mock external services and API calls
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Prefer to not mock simple dependencies
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in Pinia store tests
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in Vue component tests
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: This is a Nuxt.js app but tests are run with vitest outside of the Nuxt environment
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.ts
🧬 Code graph analysis (1)
api/src/unraid-api/cli/nodemon.service.spec.ts (3)
api/src/core/log.ts (1)
logger(50-80)packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)api/src/unraid-api/cli/log.service.ts (1)
error(65-69)
🔇 Additional comments (5)
api/src/unraid-api/cli/nodemon.service.spec.ts (5)
1-27: LGTM!The imports use proper
.jsextensions for ESM compatibility, and the environment mock now correctly includes all required exports includingSUPPRESS_LOGSandLOG_LEVEL. The mock structure is well-organized.
73-89: LGTM!Good coverage for
ensureNodemonDependencies: tests both successful directory creation and error propagation. The assertions are specific and verify the expected behavior.
91-123: LGTM!Comprehensive test for the start flow. Correctly validates:
- PM2 cleanup is called first
- Execa invocation with proper arguments
- Log stream creation and detachment
- PID file writing
- Process is unreferenced for detachment
174-194: LGTM!Good test for the immediate exit scenario. Properly verifies:
- Log stream cleanup on failure
- PID file removal
- Recent logs are captured for debugging (logsSpy called with 50 lines)
329-367: LGTM!Good coverage for the
logsmethod with three distinct scenarios:
- Successful log retrieval with stdout capture
- ENOENT error handling with appropriate error message
- Generic tail failure handling
The error handling tests correctly verify both the logging and return value.
…ution - Introduced a new integration test file for NodemonService to validate the start and stop functionality of the real nodemon process. - Implemented setup and teardown logic to create temporary files and directories for testing. - Enhanced logging and error handling in the tests to ensure proper verification of nodemon's behavior during execution.
…onService - Introduced PATHS_NODEMON_LOG_FILE to configure the log file for nodemon, allowing for better log management. - Updated log stream handling in NodemonService to write to the specified nodemon log file. - Enhanced integration tests to validate logging behavior and ensure proper file creation for both application and nodemon logs.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
api/src/environment.ts (1)
5-5: Fix nodemon binary path resolution for pnpm monorepo layoutIn this pnpm monorepo, dependencies are installed at the monorepo root's
node_modules/, not underapi/node_modules/. The current code derivesNODEMON_PATHasjoin(UNRAID_API_ROOT, 'node_modules', 'nodemon', 'bin', 'nodemon.js'), which resolves to a non-existent path. Sinceapi/package.jsonexists,getPackageJsonPath()returns it first (line 30), makingUNRAID_API_ROOTresolve to theapi/directory. Thenodemonbinary actually lives at the monorepo root'snode_modules/.Either:
- Adjust
NODEMON_PATHto point to the monorepo root'snode_modules/while keepingNODEMON_CONFIG_PATHandUNRAID_API_CWDat the API root, or- Introduce a separate
MONOREPO_ROOTconstant for nodemon binary resolution
🧹 Nitpick comments (1)
api/src/unraid-api/cli/nodemon.service.ts (1)
35-40: Add process cleanup instart()catch block when spawn succeeds but subsequent steps failThe
start()method has a defensive gap: afterexeca()successfully spawns nodemon, if eitherawait writeFile()(line 251) orawait isPidRunning()(line 256) throws, the spawned process is left running while the catch block only closes the log stream and rethrows.Since
nodemonProcessis declared outside the try block (line 233), you can safely add cleanup:catch (error) { logStream?.close(); if (nodemonProcess?.pid) { await this.terminatePids([nodemonProcess.pid]); } const errorMessage = error instanceof Error ? error.message : String(error); throw new Error(`Failed to start nodemon: ${errorMessage}`); }This prevents an orphaned nodemon daemon if
unraid-api startfails after the spawn succeeds.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
api/src/core/log.ts(2 hunks)api/src/environment.ts(3 hunks)api/src/unraid-api/cli/nodemon.service.integration.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/core/log.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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:
api/src/unraid-api/cli/nodemon.service.integration.spec.tsapi/src/unraid-api/cli/nodemon.service.tsapi/src/unraid-api/cli/nodemon.service.spec.tsapi/src/environment.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/cli/nodemon.service.integration.spec.tsapi/src/unraid-api/cli/nodemon.service.tsapi/src/unraid-api/cli/nodemon.service.spec.tsapi/src/environment.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:
api/src/unraid-api/cli/nodemon.service.integration.spec.tsapi/src/unraid-api/cli/nodemon.service.tsapi/src/unraid-api/cli/nodemon.service.spec.tsapi/src/environment.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/cli/nodemon.service.integration.spec.tsapi/src/unraid-api/cli/nodemon.service.tsapi/src/unraid-api/cli/nodemon.service.spec.tsapi/src/environment.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
api/src/unraid-api/cli/nodemon.service.integration.spec.tsapi/src/unraid-api/cli/nodemon.service.tsapi/src/unraid-api/cli/nodemon.service.spec.tsapi/src/environment.ts
🧠 Learnings (23)
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Mock external services and API calls
Applied to files:
api/src/unraid-api/cli/nodemon.service.integration.spec.tsapi/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Use Vitest for the test suite, not Jest
Applied to files:
api/src/unraid-api/cli/nodemon.service.integration.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: This is a Nuxt.js app but tests are run with vitest outside of the Nuxt environment
Applied to files:
api/src/unraid-api/cli/nodemon.service.integration.spec.tsapi/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in Vue component tests
Applied to files:
api/src/unraid-api/cli/nodemon.service.integration.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Prefer adding new files to the Nest.js repository located at api/src/unraid-api/ instead of legacy code
Applied to files:
api/src/unraid-api/cli/nodemon.service.tsapi/src/environment.ts
📚 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/cli/nodemon.service.tsapi/src/environment.ts
📚 Learning: 2025-04-23T20:19:42.542Z
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
Applied to files:
api/src/unraid-api/cli/nodemon.service.tsapi/src/environment.ts
📚 Learning: 2025-04-21T18:25:16.491Z
Learnt from: pujitm
Repo: unraid/api PR: 1352
File: api/src/unraid-api/cli/dependency.service.ts:114-117
Timestamp: 2025-04-21T18:25:16.491Z
Learning: Error masking through try/catch blocks is not desirable for the `npmInstall` method in `DependencyService` as errors should propagate to the caller.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-04-21T18:25:16.491Z
Learnt from: pujitm
Repo: unraid/api PR: 1352
File: api/src/unraid-api/cli/dependency.service.ts:114-117
Timestamp: 2025-04-21T18:25:16.491Z
Learning: Methods in `DependencyService` like `npmInstall` and `rebuildVendorArchive` intentionally propagate errors from execa calls upward rather than handling them internally, allowing callers to implement appropriate error handling. This follows the pattern documented in the `rebuildVendorArchive` method's docstring.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-03-13T16:17:55.820Z
Learnt from: elibosley
Repo: unraid/api PR: 1211
File: api/src/unraid-api/graph/connect/connect-settings.service.ts:48-53
Timestamp: 2025-03-13T16:17:55.820Z
Learning: In NestJS services, dynamic imports should be wrapped in try/catch blocks with proper error logging and fallback handling to prevent unhandled rejections from crashing the application.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
Repo: unraid/api PR: 942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
Repo: unraid/api PR: 1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Applied to files:
api/src/unraid-api/cli/nodemon.service.ts
📚 Learning: 2025-02-05T14:43:48.568Z
Learnt from: elibosley
Repo: unraid/api PR: 1120
File: plugin/package.json:0-0
Timestamp: 2025-02-05T14:43:48.568Z
Learning: In Node.js projects, npm scripts should be organized with clear namespacing (e.g., build:*, docker:*, env:*) and include proper environment validation and error handling. Each script should follow the single responsibility principle.
Applied to files:
api/src/unraid-api/cli/nodemon.service.tsapi/src/environment.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Prefer to not mock simple dependencies
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in Pinia store tests
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-09-14T14:07:28.265Z
Learnt from: elibosley
Repo: unraid/api PR: 1702
File: web/eslint.config.mjs:106-106
Timestamp: 2025-09-14T14:07:28.265Z
Learning: The unraid/api codebase specifically requires Node 20+ as a minimum version, so modern Node.js features like import.meta.dirname (available since Node v20.11.0) can be used without compatibility fallbacks.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
Repo: unraid/api PR: 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.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
Repo: unraid/api PR: 1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Applied to files:
api/src/environment.ts
📚 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:
api/src/environment.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Always run scripts from api/package.json unless explicitly requested otherwise
Applied to files:
api/src/environment.ts
📚 Learning: 2025-04-23T20:22:36.001Z
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: api/src/unraid-api/app/pubsub.module.ts:24-28
Timestamp: 2025-04-23T20:22:36.001Z
Learning: In the Unraid API project, string literals are intentionally used for dependency injection tokens instead of Symbols to enable "collision" behavior across modules that don't directly import from each other. This is a temporary approach during refactoring until a shared library for constants is implemented.
Applied to files:
api/src/environment.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/environment.ts
🧬 Code graph analysis (2)
api/src/unraid-api/cli/nodemon.service.integration.spec.ts (1)
api/src/core/log.ts (1)
logger(52-82)
api/src/unraid-api/cli/nodemon.service.spec.ts (3)
api/src/core/log.ts (1)
logger(52-82)packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)api/src/unraid-api/cli/log.service.ts (1)
error(65-69)
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/cli/nodemon.service.integration.spec.ts
[error] 83-83: TypeScript error TS2345: Argument of type '{ readonly trace: Mock; readonly warn: Mock; readonly error: Mock; readonly log: Mock; readonly info: Mock<...>; readonly debug: Mock<...>; }' is not assignable to parameter of type 'LogService'.
🔇 Additional comments (3)
api/src/unraid-api/cli/nodemon.service.ts (2)
314-331: Log streaming andlogs()error handling are robust and consistentThe combination of
createLogStreamandlogs()gives you:
- A guaranteed-open log stream for nodemon’s own output (open/error handshake, cleanup on failure).
- Tail-based reading of the main app log with clear differentiation between “file missing” (ENOENT) and other failures, returning
''so callers can handle absence gracefully.This is a solid improvement over fire-and-forget logging and unhandled tail failures; no changes needed.
Also applies to: 334-359
42-84: PM2 and process-detection helpers verified as sound for Linux targetsAll code described in the review is confirmed to exist and matches the stated analysis:
- PM2 cleanup logic correctly guards on pid file existence and defensively falls back to daemon termination (lines 42–84)
ps -eo pid,argsparsing with regex and NODEMON_CONFIG_PATH / main.js filtering is properly implemented (lines 102–135)- Error handling returns empty arrays on ps failures (lines 116, 134)
waitForNodemonExitenforces a 5-second timeout and logs only at debug level (lines 150–163)- All process signaling uses SIGTERM with proper exception handling
No issues found. The service remains explicitly Linux/
ps-centric; cross-platform refactoring would isolate these helpers.api/src/unraid-api/cli/nodemon.service.integration.spec.ts (1)
16-60: Verify integration test runs successfully before merging—hardcoded nodemon path may break with current pnpm workspace layoutThe test assumes
process.cwd()/node_modules/nodemon/bin/nodemon.jsexists, but with a pnpm monorepo structure where packages live in subdirectories (e.g.,./api), nodemon may not be hoisted to the root. The binary was not found in the sandbox, and this test—which has no skip directives—will fail at runtime if dependencies aren't installed to the expected path or if pnpm doesn't hoist correctly.Before merging: Run
pnpm install && pnpm testlocally from both the monorepo root and from within theapipackage to confirm the test actually executes successfully and finds nodemon where it expects. If it passes, the current layout works; if not, consider:
- Adding a path resolution helper that uses
require.resolve('nodemon')instead of hardcoding- Or skip this integration test in CI if the workspace layout doesn't reliably support it
| const logger = { | ||
| trace: vi.fn(), | ||
| warn: vi.fn(), | ||
| error: vi.fn(), | ||
| log: vi.fn(), | ||
| info: vi.fn(), | ||
| debug: vi.fn(), | ||
| } as const; |
There was a problem hiding this comment.
🧩 Analysis chain
Fix logger mock typing to satisfy NodemonService/LogService (TS2345 pipeline error)
The CI error:
TS2345: Argument of type '{ readonly trace: Mock<...>; ... }' is not assignable to parameter of type 'LogService'.
comes from passing the logger object (typed via as const) into new NodemonService(logger). The mock shape and readonly properties aren’t structurally compatible with the LogService class type.
You already solve this in the unit spec by casting the mock to NodemonService['logger']. Doing the same here via a type-only import keeps runtime behavior unchanged and fixes the type error.
Suggested change:
@@
-import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
+import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
+import type { NodemonService } from './nodemon.service.js';
@@
-const logger = {
- trace: vi.fn(),
- warn: vi.fn(),
- error: vi.fn(),
- log: vi.fn(),
- info: vi.fn(),
- debug: vi.fn(),
-} as const;
+const logger = {
+ trace: vi.fn(),
+ warn: vi.fn(),
+ error: vi.fn(),
+ log: vi.fn(),
+ info: vi.fn(),
+ debug: vi.fn(),
+} as unknown as NodemonService['logger'];This keeps the mock simple while making it type-compatible with the service constructor, resolving the TS2345 failure.
Also applies to: 66-85
🏁 Script executed:
#!/bin/bash
# Re-run typecheck for this spec after applying the change
pnpm --filter @unraid/api test:types -- --runTestsByPath api/src/unraid-api/cli/nodemon.service.integration.spec.ts || \
pnpm --filter @unraid/api lint:types -- api/src/unraid-api/cli/nodemon.service.integration.spec.tsLength of output: 55
I need to identify the correct type-checking command for this monorepo. Let me find the available scripts and type-check configuration.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the correct package.json and available scripts for @unraid/api
find . -maxdepth 3 -name "package.json" | grep -E "(^./package\.json|api/package\.json)" | head -5
Length of output: 33
🏁 Script executed:
#!/bin/bash
# Check available scripts in the api directory
cat api/package.json | grep -A 20 '"scripts"'Length of output: 1123
🏁 Script executed:
#!/bin/bash
# Look for TypeScript/type-check configuration files
find api -maxdepth 2 -name "tsconfig*.json" -o -name "vitest.config.*" | head -10Length of output: 17
🏁 Script executed:
#!/bin/bash
# Check the current state of the integration spec file around the logger mock
cat api/src/unraid-api/cli/nodemon.service.integration.spec.ts | head -20Length of output: 549
🏁 Script executed:
#!/bin/bash
# Also check the unit spec to see how it handles the logger typing
cat api/src/unraid-api/cli/nodemon.service.spec.ts | head -30Length of output: 1067
🏁 Script executed:
#!/bin/bash
# Check NodemonService class definition to understand LogService type
cat api/src/unraid-api/cli/nodemon.service.ts | head -40Length of output: 1223
Let me continue investigating the LogService type and find the correct type-check command.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check LogService class definition
cat api/src/unraid-api/cli/log.service.ts 2>/dev/null || cat api/src/unraid-api/cli/log.service.js 2>/dev/null | head -50
Length of output: 2348
🏁 Script executed:
#!/bin/bash
# Search for logger mock definition in the unit spec
grep -n "logger" api/src/unraid-api/cli/nodemon.service.spec.ts | head -20Length of output: 1102
🏁 Script executed:
#!/bin/bash
# Find where the logger is used in constructor in unit spec
grep -A 5 "new NodemonService" api/src/unraid-api/cli/nodemon.service.spec.tsLength of output: 5001
🏁 Script executed:
#!/bin/bash
# Check root package.json for type-check scripts
cat package.json | grep -A 30 '"scripts"' | grep -E "(type|lint)"Length of output: 103
🏁 Script executed:
#!/bin/bash
# Check if vitest has a type-check command
pnpm --version && pnpm list vitest 2>/dev/null | head -5Length of output: 7
🏁 Script executed:
#!/bin/bash
# Try to run TypeScript type-check directly
cd api && npx tsc --noEmit api/src/unraid-api/cli/nodemon.service.integration.spec.ts 2>&1 | head -20Length of output: 626
Let me run the correct type-check command using the root script.
---
🏁 Script executed:
```shell
#!/bin/bash
# Run type-check for the api package using the correct root script
pnpm --filter @unraid/api type-check 2>&1 | grep -A 5 "nodemon.service.integration.spec"
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get full type-check output to see all errors
pnpm --filter @unraid/api type-check 2>&1 | tail -50Length of output: 298
🏁 Script executed:
#!/bin/bash
# View the exact logger definition lines in integration spec
sed -n '1,20p' api/src/unraid-api/cli/nodemon.service.integration.spec.tsLength of output: 549
🏁 Script executed:
#!/bin/bash
# View the logger definition in unit spec to confirm the pattern
sed -n '55,75p' api/src/unraid-api/cli/nodemon.service.spec.tsLength of output: 706
Let me check the "Also applies to: 66-85" reference mentioned in the review to ensure the fix scope is complete.
---
🏁 Script executed:
```shell
#!/bin/bash
# View lines 60-90 in integration spec to see the context mentioned in review
sed -n '60,90p' api/src/unraid-api/cli/nodemon.service.integration.spec.ts
Length of output: 1031
🏁 Script executed:
#!/bin/bash
# Search for all logger instantiations or usages in integration spec
grep -n "logger" api/src/unraid-api/cli/nodemon.service.integration.spec.tsLength of output: 73
🏁 Script executed:
#!/bin/bash
# Get full view of integration spec to understand structure
wc -l api/src/unraid-api/cli/nodemon.service.integration.spec.tsLength of output: 62
Fix logger mock typing to satisfy NodemonService/LogService (TS2345 pipeline error)
The CI error occurs because the logger mock is typed as as const, making all properties readonly Mock types. This is structurally incompatible with LogService methods, which expect signatures like error(...messages: unknown[]): void.
The unit spec (nodemon.service.spec.ts line 61-68) already demonstrates the correct pattern: as unknown as NodemonService['logger']. Your integration spec should use the same approach.
Apply the proposed fix to lines 7-14:
-import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
+import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
+import type { NodemonService } from './nodemon.service.js';
const logger = {
trace: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
log: vi.fn(),
info: vi.fn(),
debug: vi.fn(),
-} as const;
+} as unknown as NodemonService['logger'];This removes the as const restriction and uses the proper type cast, matching the established pattern in the unit spec and resolving the TS2345 error.
🤖 Prompt for AI Agents
In api/src/unraid-api/cli/nodemon.service.integration.spec.ts around lines 7 to
14, the logger mock is typed using "as const" which makes its properties
readonly and incompatible with LogService/NodemonService method signatures;
replace the "as const" cast with "as unknown as NodemonService['logger']"
(keeping the vi.fn() implementations) so the mock matches the expected mutable
function signatures used by NodemonService and resolves the TS2345 pipeline
error.
| describe('NodemonService', () => { | ||
| const logger = { | ||
| trace: vi.fn(), | ||
| warn: vi.fn(), | ||
| error: vi.fn(), | ||
| log: vi.fn(), | ||
| info: vi.fn(), | ||
| debug: vi.fn(), | ||
| } as unknown as NodemonService['logger']; | ||
|
|
||
| const mockMkdir = vi.mocked(fs.mkdir); | ||
| const mockWriteFile = vi.mocked(fs.writeFile); | ||
| const mockRm = vi.mocked(fs.rm); | ||
| const killSpy = vi.spyOn(process, 'kill'); | ||
| const stopPm2Spy = vi.spyOn( | ||
| NodemonService.prototype as unknown as { stopPm2IfRunning: () => Promise<void> }, | ||
| 'stopPm2IfRunning' | ||
| ); | ||
| const findMatchingSpy = vi.spyOn( | ||
| NodemonService.prototype as unknown as { findMatchingNodemonPids: () => Promise<number[]> }, | ||
| 'findMatchingNodemonPids' | ||
| ); | ||
| const findDirectMainSpy = vi.spyOn( | ||
| NodemonService.prototype as unknown as { findDirectMainPids: () => Promise<number[]> }, | ||
| 'findDirectMainPids' | ||
| ); | ||
| const terminateSpy = vi.spyOn( | ||
| NodemonService.prototype as unknown as { terminatePids: (pids: number[]) => Promise<void> }, | ||
| 'terminatePids' | ||
| ); | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| vi.mocked(createWriteStream).mockImplementation(() => createLogStreamMock()); | ||
| mockMkdir.mockResolvedValue(undefined); | ||
| mockWriteFile.mockResolvedValue(undefined as unknown as void); | ||
| mockRm.mockResolvedValue(undefined as unknown as void); | ||
| vi.mocked(fileExists).mockResolvedValue(false); | ||
| killSpy.mockReturnValue(true); | ||
| findMatchingSpy.mockResolvedValue([]); | ||
| findDirectMainSpy.mockResolvedValue([]); | ||
| terminateSpy.mockResolvedValue(); | ||
| stopPm2Spy.mockResolvedValue(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Restore process.kill spy after this suite to avoid cross-test leakage
The suite sets up a module-scope spy on process.kill:
const killSpy = vi.spyOn(process, 'kill');and reconfigures it in beforeEach, but never calls killSpy.mockRestore() (or equivalent). While vi.clearAllMocks() resets calls/implementations, the spy remains active for the rest of the test run and can affect other files that rely on the real process.kill.
Not urgent, but for test isolation it would be better to restore the original implementation, e.g.:
afterAll(() => {
killSpy.mockRestore();
});(or move the spy into a beforeEach/afterEach pair).
🏁 Script executed:
#!/bin/bash
# Search for other specs that might rely on process.kill to assess impact
rg -n "process\.kill" api/src --type tsLength of output: 525
---
🏁 Script executed:
```shell
#!/bin/bash
# Check integration spec to see if it relies on real process.kill
cat api/src/unraid-api/cli/nodemon.service.integration.spec.ts | head -150
Length of output: 4357
Restore process.kill spy after this suite to prevent integration test failure
The module-scope spy on process.kill is never restored and remains active across test files. This breaks the integration test, which relies on process.kill(pid, 0) throwing an error to detect process exit in waitForExit() (nodemon.service.integration.spec.ts:118–126).
Add to the suite:
afterAll(() => {
killSpy.mockRestore();
});🤖 Prompt for AI Agents
In api/src/unraid-api/cli/nodemon.service.spec.ts around lines 60 to 103 the
test suite creates a module-scoped spy on process.kill (killSpy) but never
restores it, which leaks into other tests and breaks the integration test that
expects process.kill to throw; add an afterAll block to call
killSpy.mockRestore() so the original process.kill is restored after this suite
completes.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
api/src/core/log.ts(2 hunks)api/src/environment.ts(3 hunks)api/src/unraid-api/cli/nodemon.service.integration.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.spec.ts(1 hunks)api/src/unraid-api/cli/nodemon.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- api/src/unraid-api/cli/nodemon.service.ts
- api/src/core/log.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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:
api/src/unraid-api/cli/nodemon.service.spec.tsapi/src/unraid-api/cli/nodemon.service.integration.spec.tsapi/src/environment.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/cli/nodemon.service.spec.tsapi/src/unraid-api/cli/nodemon.service.integration.spec.tsapi/src/environment.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:
api/src/unraid-api/cli/nodemon.service.spec.tsapi/src/unraid-api/cli/nodemon.service.integration.spec.tsapi/src/environment.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/cli/nodemon.service.spec.tsapi/src/unraid-api/cli/nodemon.service.integration.spec.tsapi/src/environment.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
api/src/unraid-api/cli/nodemon.service.spec.tsapi/src/unraid-api/cli/nodemon.service.integration.spec.tsapi/src/environment.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Mock external services and API calls
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.tsapi/src/unraid-api/cli/nodemon.service.integration.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Prefer to not mock simple dependencies
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in Pinia store tests
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: This is a Nuxt.js app but tests are run with vitest outside of the Nuxt environment
Applied to files:
api/src/unraid-api/cli/nodemon.service.spec.tsapi/src/unraid-api/cli/nodemon.service.integration.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Use Vitest for the test suite, not Jest
Applied to files:
api/src/unraid-api/cli/nodemon.service.integration.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in Vue component tests
Applied to files:
api/src/unraid-api/cli/nodemon.service.integration.spec.ts
📚 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/environment.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Prefer adding new files to the Nest.js repository located at api/src/unraid-api/ instead of legacy code
Applied to files:
api/src/environment.ts
📚 Learning: 2025-09-14T14:07:28.265Z
Learnt from: elibosley
Repo: unraid/api PR: 1702
File: web/eslint.config.mjs:106-106
Timestamp: 2025-09-14T14:07:28.265Z
Learning: The unraid/api codebase specifically requires Node 20+ as a minimum version, so modern Node.js features like import.meta.dirname (available since Node v20.11.0) can be used without compatibility fallbacks.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
Repo: unraid/api PR: 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.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-02-05T14:43:48.568Z
Learnt from: elibosley
Repo: unraid/api PR: 1120
File: plugin/package.json:0-0
Timestamp: 2025-02-05T14:43:48.568Z
Learning: In Node.js projects, npm scripts should be organized with clear namespacing (e.g., build:*, docker:*, env:*) and include proper environment validation and error handling. Each script should follow the single responsibility principle.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-04-23T20:19:42.542Z
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
Applied to files:
api/src/environment.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
Repo: unraid/api PR: 1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Applied to files:
api/src/environment.ts
📚 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:
api/src/environment.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Always run scripts from api/package.json unless explicitly requested otherwise
Applied to files:
api/src/environment.ts
📚 Learning: 2025-04-23T20:22:36.001Z
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: api/src/unraid-api/app/pubsub.module.ts:24-28
Timestamp: 2025-04-23T20:22:36.001Z
Learning: In the Unraid API project, string literals are intentionally used for dependency injection tokens instead of Symbols to enable "collision" behavior across modules that don't directly import from each other. This is a temporary approach during refactoring until a shared library for constants is implemented.
Applied to files:
api/src/environment.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/environment.ts
🧬 Code graph analysis (2)
api/src/unraid-api/cli/nodemon.service.spec.ts (3)
api/src/core/log.ts (1)
logger(52-82)packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)api/src/unraid-api/cli/log.service.ts (1)
error(65-69)
api/src/unraid-api/cli/nodemon.service.integration.spec.ts (1)
api/src/core/log.ts (1)
logger(52-82)
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/cli/nodemon.service.integration.spec.ts
[error] 83-83: Type error in nodemon.service.integration.spec.ts: Argument of type '{ readonly trace: Mock; readonly warn: Mock; readonly error: Mock; readonly log: Mock; readonly info: Mock<...>; readonly debug: Mock<...>; }' is not assignable to parameter of type 'LogService'. (TS2345)
🔇 Additional comments (10)
api/src/environment.ts (2)
68-68: Well-structured derivation of API root path.Using
dirname(getPackageJsonPath())to establishUNRAID_API_ROOTis a robust approach that ties all nodemon-related paths to a single source of truth. This avoids the off-by-one issues from the previous relative path approach.
105-111: Nodemon path constants look well-organized.The constants properly support environment overrides for
NODEMON_PID_PATHandUNRAID_API_CWD, with sensible defaults derived fromUNRAID_API_ROOTandPATHS_LOGS_DIR.api/src/unraid-api/cli/nodemon.service.integration.spec.ts (2)
102-134: Helper functions are well-implemented.Both
waitForLogEntryandwaitForExituse proper deadline-based polling with reasonable intervals. The use ofprocess.kill(pid, 0)to check process existence is the standard approach.
66-99: Integration test structure looks solid.The test properly exercises the full lifecycle: start → verify PID → verify logs → stop → verify cleanup. The 20-second timeout is appropriate for integration tests involving real process spawning.
api/src/unraid-api/cli/nodemon.service.spec.ts (6)
60-68: Logger mock correctly typed.The type assertion
as unknown as NodemonService['logger']properly satisfies the type checker while allowing the mock to be used flexibly in tests.
48-58: Environment mock is complete.The mock includes all required exports:
LOG_LEVEL,SUPPRESS_LOGS, and all nodemon-related paths. This aligns with the updatedenvironment.tsexports.
105-122: Directory creation tests cover both success and failure paths.Good coverage of
ensureNodemonDependenciesincluding the error propagation case.
124-157: Start flow test validates key behaviors.The test verifies: PM2 migration cleanup (
stopPm2Spy), correct execa arguments, detached mode, PID file writing, and stream handling. This covers the critical path for nodemon startup.
238-302: Good coverage of PID lifecycle edge cases.Tests for already-running processes, stale PID files, and stray nodemon cleanup ensure robust behavior in real-world scenarios where process state may be inconsistent.
403-455: Restart behavior tests are thorough.Testing both the "running process" and "not running" restart paths ensures the service handles both scenarios correctly.
- Updated API version in api.json from 4.25.3 to 4.27.2. - Refactored pubsub channel references across multiple files to use GRAPHQL_PUBSUB_CHANNEL instead of PUBSUB_CHANNEL, enhancing consistency and clarity in the codebase. - Adjusted related tests to ensure they align with the new pubsub channel structure.
There was a problem hiding this comment.
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/graph/resolvers/metrics/metrics.resolver.ts (1)
67-67: Remove duplicate debug log.Line 67 contains a redundant debug log with the label "payload2" that duplicates the logging already done on line 60. This appears to be leftover debug code.
Apply this diff:
- this.logger.debug(`CPU_TELEMETRY payload2: ${JSON.stringify(packages)}`);
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (1)
12-12: Consider using the shared pubsub instance for consistency.This resolver creates a local
PubSubinstance, whereas other resolvers (e.g.,display.resolver.ts) usecreateSubscription()from@app/core/pubsub.js. If events are published to the shared pubsub, this local instance won't receive them.-import { PubSub } from 'graphql-subscriptions'; +import { createSubscription } from '@app/core/pubsub.js'; -const pubSub = new PubSub(); - @Resolver(() => ParityCheck) export class ParityResolver { // ... @Subscription(() => ParityCheck) parityHistorySubscription() { - return pubSub.asyncIterableIterator(GRAPHQL_PUBSUB_CHANNEL.PARITY); + return createSubscription(GRAPHQL_PUBSUB_CHANNEL.PARITY); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
api/dev/configs/api.json(1 hunks)api/src/core/pubsub.ts(0 hunks)api/src/store/listeners/array-event-listener.ts(2 hunks)api/src/unraid-api/cli/nodemon.service.integration.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/array/parity.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/display/display.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts(2 hunks)api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts(4 hunks)api/src/unraid-api/graph/resolvers/docker/docker.service.ts(3 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts(12 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts(7 hunks)api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts(1 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts(3 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts(4 hunks)api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/servers/server.resolver.ts(2 hunks)api/src/unraid-api/graph/services/subscription-helper.service.spec.ts(13 hunks)api/src/unraid-api/graph/services/subscription-helper.service.ts(3 hunks)
💤 Files with no reviewable changes (1)
- api/src/core/pubsub.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/cli/nodemon.service.integration.spec.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{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:
api/src/unraid-api/graph/resolvers/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/services/subscription-helper.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/servers/server.resolver.tsapi/src/store/listeners/array-event-listener.tsapi/src/unraid-api/graph/services/subscription-helper.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/owner/owner.resolver.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/graph/resolvers/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/dev/configs/api.jsonapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/services/subscription-helper.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/servers/server.resolver.tsapi/src/store/listeners/array-event-listener.tsapi/src/unraid-api/graph/services/subscription-helper.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/owner/owner.resolver.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:
api/src/unraid-api/graph/resolvers/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/services/subscription-helper.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/servers/server.resolver.tsapi/src/store/listeners/array-event-listener.tsapi/src/unraid-api/graph/services/subscription-helper.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/owner/owner.resolver.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/graph/resolvers/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/services/subscription-helper.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/servers/server.resolver.tsapi/src/store/listeners/array-event-listener.tsapi/src/unraid-api/graph/services/subscription-helper.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/owner/owner.resolver.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
api/src/unraid-api/graph/resolvers/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/dev/configs/api.jsonapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/services/subscription-helper.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/servers/server.resolver.tsapi/src/store/listeners/array-event-listener.tsapi/src/unraid-api/graph/services/subscription-helper.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/owner/owner.resolver.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx,js,jsx}: Use VITEST for test suite, not jest
Use.rejects.toThrow()without arguments to test that functions throw errors, not exact error message strings
Files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts
api/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
api/**/*.test.{ts,tsx}: Use Vitest for the test suite, not Jest
Prefer to not mock simple dependencies
For error testing, use.rejects.toThrow()without arguments; do not test exact error message strings unless the message format is specifically what you're testing
Files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
**/*.test.ts: Use.rejects.toThrow()without arguments to test that functions throw errors. Don't test exact error message strings unless the message format is specifically what you're testing
Test what the code does, not implementation details like exact error message wording
Mock external services and API calls
Usevi.mock()for module-level mocks
Specify return values for component methods withvi.spyOn()
Reset mocks between tests withvi.clearAllMocks()
Always await async operations before making assertions
Files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts
**/store/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
Ensure Vue reactivity imports like
computed,ref, andwatchEffectare added to store files even with Nuxt auto-imports enabled
Files:
api/src/store/listeners/array-event-listener.ts
🧠 Learnings (35)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
Repo: unraid/api PR: 1526
File: api/src/unraid-api/graph/resolvers/ups/ups.resolver.ts:82-85
Timestamp: 2025-07-22T13:29:32.175Z
Learning: In GraphQL subscriptions using the graphql-subscriptions PubSub, the correct method is `asyncIterableIterator()`, not `asyncIterator()`. The `asyncIterableIterator` method returns an `AsyncIterableIterator` that conforms to both `AsyncIterable` and `AsyncIterator` interfaces, which is required for GraphQL subscription resolvers. Using `asyncIterator()` would cause TypeScript type errors as it only returns an `AsyncIterator` that doesn't satisfy the `AsyncIterable` interface requirement.
Applied to files:
api/src/unraid-api/graph/resolvers/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/services/subscription-helper.service.tsapi/src/unraid-api/graph/resolvers/servers/server.resolver.tsapi/src/unraid-api/graph/services/subscription-helper.service.spec.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.tsapi/src/unraid-api/graph/resolvers/owner/owner.resolver.ts
📚 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/graph/resolvers/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/servers/server.resolver.tsapi/src/unraid-api/graph/services/subscription-helper.service.spec.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/owner/owner.resolver.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
Repo: unraid/api PR: 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/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/servers/server.resolver.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/owner/owner.resolver.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/graph/resolvers/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/servers/server.resolver.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.tsapi/src/unraid-api/graph/resolvers/owner/owner.resolver.ts
📚 Learning: 2025-11-24T17:52:00.572Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-11-24T17:52:00.572Z
Learning: Applies to web/**/*.query.ts : GraphQL queries must be placed in `.query.ts` files
Applied to files:
api/src/unraid-api/graph/resolvers/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.ts
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
Repo: unraid/api PR: 1526
File: api/src/unraid-api/graph/resolvers/ups/ups.resolver.ts:82-85
Timestamp: 2025-07-22T13:29:32.175Z
Learning: In GraphQL subscriptions using the graphql-subscriptions PubSub, the correct method to create an async iterator is `asyncIterableIterator()`, not `asyncIterator()`. This is the modern/current method used in the graphql-subscriptions library.
Applied to files:
api/src/unraid-api/graph/resolvers/display/display.resolver.tsapi/src/unraid-api/graph/resolvers/array/array.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/services/subscription-helper.service.tsapi/src/unraid-api/graph/resolvers/servers/server.resolver.tsapi/src/unraid-api/graph/services/subscription-helper.service.spec.tsapi/src/unraid-api/graph/resolvers/array/parity.resolver.tsapi/src/unraid-api/graph/resolvers/owner/owner.resolver.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia store getter dependencies are properly mocked
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/services/subscription-helper.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in Pinia store tests
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Mock external services and API calls
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Prefer to not mock simple dependencies
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Do not mock the store being tested in the test file when using `createPinia()`
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia action side effects and state changes
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Use factory functions for module mocks in Pinia store tests to avoid hoisting issues
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Verify state changes after Pinia store actions
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in Vue component tests
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: This is a Nuxt.js app but tests are run with vitest outside of the Nuxt environment
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Use `vi.mock()` for module-level mocks
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Use Vitest for the test suite, not Jest
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
Repo: unraid/api PR: 1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 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 web/__test__/**/*.test.{ts,tsx} : Use `mount` from Vue Test Utils for component testing
Applied to files:
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Use `mount` from Vue Test Utils for component testing
Applied to files:
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Test component behavior and output, not implementation details
Applied to files:
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Reset mocks between tests with `vi.clearAllMocks()`
Applied to files:
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Specify return values for component methods with `vi.spyOn()`
Applied to files:
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
Repo: unraid/api PR: 942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/services/subscription-helper.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Use `createTestingPinia()` for mocking stores in Vue components
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Place all mock declarations at the top level in Pinia store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-01-30T19:56:42.055Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: web/components/Notifications/Sidebar.vue:52-57
Timestamp: 2025-01-30T19:56:42.055Z
Learning: Notification links in the Unraid API are validated at creation time on the server side, making additional client-side validation unnecessary when receiving notifications through GraphQL subscriptions.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-04-07T14:34:47.255Z
Learnt from: elibosley
Repo: unraid/api PR: 1334
File: api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts:63-66
Timestamp: 2025-04-07T14:34:47.255Z
Learning: In DockerEventService, the chokidar file watcher is configured with `ignoreInitial: false` to ensure that existing files (like the Docker socket) are detected and processed at application startup, not just when files change after the watcher is initialized.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts
📚 Learning: 2025-01-23T21:26:17.766Z
Learnt from: elibosley
Repo: unraid/api PR: 1063
File: api/src/unraid-api/cli/sso/validate-token.command.ts:76-81
Timestamp: 2025-01-23T21:26:17.766Z
Learning: In the unraid/api codebase, store dispatches using Redux Toolkit's `unwrap()` method already handle errors appropriately and don't require additional try-catch blocks.
Applied to files:
api/src/store/listeners/array-event-listener.ts
📚 Learning: 2025-08-29T19:48:45.517Z
Learnt from: elibosley
Repo: unraid/api PR: 1618
File: web/components/Logs/SingleLogViewer.vue:248-299
Timestamp: 2025-08-29T19:48:45.517Z
Learning: In Vue Apollo's composition API, subscribeToMore automatically handles subscription cleanup when the related query is stopped or the component unmounts, unlike React Apollo where manual cleanup is required. Manual unsubscribe is only needed when changing subscription variables, not for lifecycle management.
Applied to files:
api/src/unraid-api/graph/services/subscription-helper.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Verify Pinia actions are called with correct parameters
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
📚 Learning: 2025-11-24T17:52:00.572Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-11-24T17:52:00.572Z
Learning: Applies to web/**/*.{query.ts,mutation.ts} : All GraphQL under `web/` must follow the `.query.ts` and `.mutation.ts` naming convention
Applied to files:
api/src/unraid-api/graph/resolvers/array/parity.resolver.ts
📚 Learning: 2025-01-31T22:01:22.708Z
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.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
🧬 Code graph analysis (14)
api/src/unraid-api/graph/resolvers/display/display.resolver.ts (1)
api/src/core/pubsub.ts (1)
createSubscription(17-21)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
api/src/core/pubsub.ts (1)
pubsub(10-10)
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (1)
api/src/core/pubsub.ts (1)
createSubscription(17-21)
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts (1)
api/src/core/pubsub.ts (1)
createSubscription(17-21)
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts (1)
api/src/core/pubsub.ts (1)
pubsub(10-10)
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
api/src/core/pubsub.ts (1)
createSubscription(17-21)
api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts (1)
api/src/core/pubsub.ts (1)
pubsub(10-10)
api/src/unraid-api/graph/resolvers/servers/server.resolver.ts (1)
api/src/core/pubsub.ts (1)
createSubscription(17-21)
api/src/store/listeners/array-event-listener.ts (1)
api/src/core/pubsub.ts (1)
pubsub(10-10)
api/src/unraid-api/graph/services/subscription-helper.service.spec.ts (1)
api/src/core/pubsub.ts (1)
pubsub(10-10)
api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts (1)
api/src/core/pubsub.ts (1)
pubsub(10-10)
api/src/unraid-api/graph/resolvers/docker/docker.service.ts (1)
api/src/core/pubsub.ts (1)
pubsub(10-10)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
api/src/core/pubsub.ts (1)
pubsub(10-10)
api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts (1)
api/src/core/pubsub.ts (1)
createSubscription(17-21)
🔇 Additional comments (25)
api/dev/configs/api.json (1)
2-2: Version bump looks good.The update from "4.25.3" to "4.27.2" appropriately reflects the PM2→nodemon process manager migration. The minor version bump is semantically correct for this feature change.
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts (7)
5-8: GRAPHQL_PUBSUB_CHANNEL and pubsub imports look correctUsing
GRAPHQL_PUBSUB_CHANNELfrom@unraid/shared/pubsub/graphql.pubsub.jsand keepingpubsubfrom@app/core/pubsub.jswith.jsextensions matches the ESM import guidelines and aligns with the rest of the file’s usage.
111-118: CPU and memory polling tests correctly moved to GRAPHQL_PUBSUB_CHANNELThe subscriptions for CPU and memory polling now use
GRAPHQL_PUBSUB_CHANNEL.CPU_UTILIZATIONand.MEMORY_UTILIZATION, matching the new enum and keeping the concurrency/“execute at most once” semantics unchanged.Also applies to: 145-152
159-176: CPU pubsub publishing test aligned with GRAPHQL_PUBSUB_CHANNELThe CPU metrics publish test now subscribes, asserts, and unsubscribes using
GRAPHQL_PUBSUB_CHANNEL.CPU_UTILIZATION, which keeps the test consistent with the updated pubsub channel naming.
184-201: Memory pubsub publishing test aligned with GRAPHQL_PUBSUB_CHANNELThe memory metrics publish test has been fully switched to
GRAPHQL_PUBSUB_CHANNEL.MEMORY_UTILIZATIONfor subscription, expectation, and cleanup, matching the new channel API.
218-229: CPU polling error-handling test updated to new channel constantSubscribing and unsubscribing with
GRAPHQL_PUBSUB_CHANNEL.CPU_UTILIZATIONkeeps the CPU error-handling path test in sync with the new channel enum without changing behavior.
245-256: Memory polling error-handling test updated to new channel constantThe memory polling error test now uses
GRAPHQL_PUBSUB_CHANNEL.MEMORY_UTILIZATIONfor both subscription and cleanup, correctly reflecting the new pubsub channel naming.
267-291: Polling cleanup test consistently uses GRAPHQL_PUBSUB_CHANNELStartup, active-subscription checks, and post-destroy assertions all reference
GRAPHQL_PUBSUB_CHANNEL.CPU_UTILIZATIONand.MEMORY_UTILIZATION, so the cleanup behavior remains the same while matching the new enum.api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (1)
4-4: LGTM on the channel migration.The migration from
PUBSUB_CHANNEL.PARITYtoGRAPHQL_PUBSUB_CHANNEL.PARITYis consistent with the broader PR refactor.Also applies to: 36-36
api/src/store/listeners/array-event-listener.ts (1)
2-2: LGTM on the pubsub channel migration.The migration from
PUBSUB_CHANNELtoGRAPHQL_PUBSUB_CHANNELfor bothARRAYandINFOchannels is correctly implemented, and the shared pubsub instance is properly used.Also applies to: 24-24, 31-31
api/src/unraid-api/graph/services/subscription-helper.service.ts (1)
3-5: LGTM on the type and import updates.The migration from
PUBSUB_CHANNELtoGRAPHQL_PUBSUB_CHANNELis correctly applied in the import, method signature, and documentation example. The implementation logic remains intact.Also applies to: 26-27, 38-40
api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts (1)
5-5: LGTM on the test update.The test assertion correctly expects
GRAPHQL_PUBSUB_CHANNEL.INFOto align with the production code migration.Also applies to: 141-141
api/src/unraid-api/graph/resolvers/display/display.resolver.ts (1)
4-4: LGTM on the channel migration.The migration to
GRAPHQL_PUBSUB_CHANNEL.DISPLAYand the use of the sharedcreateSubscriptionhelper follows the correct pattern for subscription resolvers.Also applies to: 7-7, 30-30
api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts (1)
4-4: LGTM! Clean migration to GRAPHQL_PUBSUB_CHANNEL.The pubsub channel migration is correctly implemented with proper imports and usage.
Also applies to: 8-8, 136-136
api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts (1)
5-5: LGTM! Subscription channel correctly migrated.The change to GRAPHQL_PUBSUB_CHANNEL.OWNER is consistent with the broader migration.
Also applies to: 8-8, 43-43
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (1)
4-4: LGTM! Array subscription channel updated correctly.The migration to GRAPHQL_PUBSUB_CHANNEL.ARRAY is properly implemented.
Also applies to: 7-7, 30-30
api/src/unraid-api/graph/resolvers/docker/docker.service.ts (1)
5-5: LGTM! Docker service publish calls updated consistently.Both start() and stop() methods now correctly publish to GRAPHQL_PUBSUB_CHANNEL.INFO.
Also applies to: 9-9, 214-214, 244-244
api/src/unraid-api/graph/resolvers/servers/server.resolver.ts (1)
6-6: LGTM! Server subscription migrated correctly.The GRAPHQL_PUBSUB_CHANNEL.SERVERS usage is properly implemented.
Also applies to: 9-9, 46-46
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts (1)
4-4: LGTM! Test correctly validates the new channel constant.The test assertion properly verifies that GRAPHQL_PUBSUB_CHANNEL.DISPLAY is passed to createSubscription.
Also applies to: 81-81, 85-85
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts (1)
45-53: LGTM! Test mock updated to match the channel migration.The mock now exports GRAPHQL_PUBSUB_CHANNEL with the correct notification channel keys.
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
5-5: Migration to GRAPHQL_PUBSUB_CHANNEL is complete and consistent across the codebase.Verification confirms all PUBSUB_CHANNEL references have been migrated. No legacy constants remain, and the notifications resolver correctly uses GRAPHQL_PUBSUB_CHANNEL on all specified lines (5, 9, 156, 165).
api/src/unraid-api/graph/services/subscription-helper.service.spec.ts (1)
3-3: LGTM! Consistent migration to GRAPHQL_PUBSUB_CHANNEL.The test file has been properly updated to use
GRAPHQL_PUBSUB_CHANNELinstead ofPUBSUB_CHANNEL. All test cases consistently reference the new channel constants, and the test logic remains unchanged.Also applies to: 7-7, 32-334
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
6-6: LGTM! Notification pubsub channels properly migrated.The notification service has been correctly updated to use
GRAPHQL_PUBSUB_CHANNELfor bothNOTIFICATION_ADDEDandNOTIFICATION_OVERVIEWevents. The migration is consistent with the broader refactoring effort.Also applies to: 16-16, 122-144
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts (1)
5-5: LGTM! Docker service tests aligned with new channel constants.The test file has been properly updated to mock and assert against
GRAPHQL_PUBSUB_CHANNEL.INFO. Both the mock setup and test expectations are consistent with the migration.Also applies to: 10-10, 19-22, 278-283, 336-341
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
5-5: LGTM! Metrics resolver migrated to GRAPHQL_PUBSUB_CHANNEL.The resolver has been successfully updated to use
GRAPHQL_PUBSUB_CHANNELfor CPU utilization, CPU telemetry, and memory utilization subscriptions. The totalPower calculation logic (lines 46-52) is a solid addition that properly filters negative values and rounds to 2 decimals.Also applies to: 8-8, 32-83, 115-142
…adient based on CSS variable - Updated UserProfile component to load banner gradient from a CSS variable, allowing for dynamic styling. - Added tests to verify banner rendering behavior based on the presence of the CSS variable, ensuring correct functionality regardless of theme store settings. - Removed outdated test cases that relied solely on theme store flags for banner gradient rendering.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/src/components/UserProfile.standalone.vue (1)
38-48: CSS-variable banner gradient wiring looks solid; consider dynamic updates as a follow-upThe new
bannerGradientref,loadBannerGradientFromCsshelper, and conditional banner rendering are SSR-safe and align with the CSS-variable–driven design. One thing to be aware of: because the gradient is read once ononMountedand stored as a literalbackground-image: ...;string, runtime theme switches that change--banner-gradientwon’t update the banner without a reload. If theme changes are expected after mount, consider either re-runningloadBannerGradientFromCsswhen the active theme changes, or binding the style tovar(--banner-gradient)and only using the CSS read to decide presence/absence.Also applies to: 98-100, 113-116
web/__test__/components/UserProfile.test.ts (1)
361-404: Good behavior-focused test for CSS-driven banner gradient (minor optional cleanup)This test nicely validates that the banner appears and uses the CSS-variable gradient even when the theme store disables banner flags, which matches the new implementation’s contract. Using a separate
localPiniakeeps state isolated and avoids coupling to the sharedpiniainstance. As an optional refinement, you could wrap thelocalWrapper.unmount(), CSS var removal, andsetActivePinia(pinia)in atry/finallyto guarantee cleanup even if an assertion fails, but it’s not strictly necessary given the current setup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/__test__/components/UserProfile.test.ts(2 hunks)web/src/components/UserProfile.standalone.vue(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/__test__/components/UserProfile.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx,js,jsx}: Use VITEST for test suite, not jest
Use.rejects.toThrow()without arguments to test that functions throw errors, not exact error message strings
Files:
web/__test__/components/UserProfile.test.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/__test__/components/UserProfile.test.tsweb/src/components/UserProfile.standalone.vue
web/__test__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/__test__/**/*.test.{ts,tsx}: Test component behavior and output, not implementation details
Usemountfrom Vue Test Utils for component testing
Stub complex child components that aren't the focus of the test
Find elements with semantic queries likefind('button')rather than data-test IDs
UsecreatePinia()andsetActivePiniawhen testing Store files, only usecreateTestingPiniaif you specifically need its testing features
Files:
web/__test__/components/UserProfile.test.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/__test__/components/UserProfile.test.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
web/__test__/components/UserProfile.test.tsweb/src/components/UserProfile.standalone.vue
**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
**/*.test.ts: Use.rejects.toThrow()without arguments to test that functions throw errors. Don't test exact error message strings unless the message format is specifically what you're testing
Test what the code does, not implementation details like exact error message wording
Mock external services and API calls
Usevi.mock()for module-level mocks
Specify return values for component methods withvi.spyOn()
Reset mocks between tests withvi.clearAllMocks()
Always await async operations before making assertions
Files:
web/__test__/components/UserProfile.test.ts
**/__test__/components/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
**/__test__/components/**/*.ts: Usemountfrom Vue Test Utils for component testing
Stub complex child components that aren't the focus of the test
Mock external dependencies and services in Vue component tests
Test component behavior and output, not implementation details
Verify that the expected elements are rendered in Vue components
Test component interactions such as clicks and inputs
Check for expected prop handling and event emissions in Vue components
UsecreateTestingPinia()for mocking stores in Vue components
Use semantic queries likefind('button')orfind('[data-test="id"]')but prefer not to use data test ID's when finding elements
Find Vue components withfindComponent(ComponentName)
UsefindAllto check for multiple elements in Vue components
Assert on rendered text content withwrapper.text()
Assert on element attributes withelement.attributes()
Verify element existence withexpect(element.exists()).toBe(true)
Check component state through rendered output
Trigger events withawait element.trigger('click')
Set input values withawait input.setValue('value')
Test emitted events withwrapper.emitted()
Useawait nextTick()for DOM updates in Vue component tests
UseflushPromises()for complex promise chains in Vue component tests
Files:
web/__test__/components/UserProfile.test.ts
🧠 Learnings (20)
📓 Common learnings
Learnt from: pujitm
Repo: unraid/api PR: 1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Test component behavior and output, not implementation details
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia action side effects and state changes
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 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 web/__test__/**/*.test.{ts,tsx} : Use `createPinia()` and `setActivePinia` when testing Store files, only use `createTestingPinia` if you specifically need its testing features
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 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 web/__test__/**/*.test.{ts,tsx} : Test component behavior and output, not implementation details
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia store getter dependencies are properly mocked
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Verify state changes after Pinia store actions
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Use `createPinia()` instead of `createTestingPinia()` for most Pinia store tests
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in Pinia store tests
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Use `createPinia` and `setActivePinia` when testing Pinia store files
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Override specific Pinia action implementations when needed in tests
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-02-20T15:52:58.297Z
Learnt from: elibosley
Repo: unraid/api PR: 1155
File: web/store/theme.ts:161-172
Timestamp: 2025-02-20T15:52:58.297Z
Learning: The banner gradient implementation in web/store/theme.ts doesn't require explicit error handling for hexToRgba as CSS gracefully handles invalid values by ignoring them.
Applied to files:
web/__test__/components/UserProfile.test.tsweb/src/components/UserProfile.standalone.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Use `createTestingPinia()` for mocking stores in Vue components
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia store interactions with other stores
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Assert on rendered text content with `wrapper.text()`
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Verify element existence with `expect(element.exists()).toBe(true)`
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Verify that the expected elements are rendered in Vue components
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Test emitted events with `wrapper.emitted()`
Applied to files:
web/__test__/components/UserProfile.test.ts
📚 Learning: 2025-02-20T15:52:56.733Z
Learnt from: elibosley
Repo: unraid/api PR: 1155
File: web/store/theme.ts:49-50
Timestamp: 2025-02-20T15:52:56.733Z
Learning: CSS variable names in the theme store should be concise and follow established patterns. For example, prefer '--gradient-start' over '--color-customgradient-start' to maintain consistency with other variable names.
Applied to files:
web/src/components/UserProfile.standalone.vue
📚 Learning: 2024-12-17T14:59:32.458Z
Learnt from: elibosley
Repo: unraid/api PR: 972
File: web/store/theme.ts:46-49
Timestamp: 2024-12-17T14:59:32.458Z
Learning: In the `web/store/theme.ts` file of the Unraid web application, the header is intentionally designed to have a light background with dark text in dark mode, and a dark background with light text in light mode.
Applied to files:
web/src/components/UserProfile.standalone.vue
🔇 Additional comments (1)
web/__test__/components/UserProfile.test.ts (1)
406-418: Nice inverse test covering the “no CSS variable” caseThis test correctly asserts that without
--banner-gradientset, the banner element is not rendered even if theme flags are enabled, which complements the previous test and matches the component’s new behavior.
Summary
Testing
Codex Task
Summary by CodeRabbit
Refactor
New Features
Updates
Tests
✏️ Tip: You can customize this high-level summary in your review settings.