fix: refactor API client to support Unix socket connections #1575
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughAdds an InternalGraphQLClientFactory and SocketConfigService, introduces INTERNAL_CLIENT_SERVICE_TOKEN, refactors CLI and Connect internal clients to use the injected factory with caching/concurrency/clear semantics, adds undici and Vitest migration, many unit/integration tests, and new UI barrel exports. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CliInternalClientService
participant Admin as AdminKeyService
participant Factory as InternalGraphQLClientFactory
participant SocketCfg as SocketConfigService
participant Apollo as ApolloClient
CLI->>Admin: getOrCreateLocalAdminKey()
Admin-->>CLI: adminApiKey
CLI->>Factory: createClient({ getApiKey, enableSubscriptions: false })
Factory->>SocketCfg: getApiAddress()/getWebSocketUri()
Factory-->>CLI: Apollo client
CLI->>Apollo: cache client / use client
CLI->>Apollo: clearClient() -> stop()
sequenceDiagram
participant Connect as Connect InternalClientService
participant KeySvc as ConnectApiKeyService
participant Factory as InternalGraphQLClientFactory
participant Apollo as ApolloClient
Connect->>Connect: check cached client
alt no client
Connect->>KeySvc: getOrCreateLocalApiKey()
KeySvc-->>Connect: apiKey
Connect->>Factory: createClient({ getApiKey, enableSubscriptions: true })
Factory-->>Connect: Apollo client
Connect->>Connect: cache client
end
Connect-->>Apollo: return client
Connect->>Apollo: clearClient() -> stop()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
api/src/unraid-api/cli/internal-client.service.ts (1)
37-58: Cache the PORT lookup once & handle NaN
isRunningOnSocket,getSocketPath, andgetNumericPorteach readPORTseparately andgetNumericPortmay returnNaNif the var is an empty string.-const port = this.configService.get<string>('PORT', '/var/run/unraid-api.sock'); +const rawPort = this.configService.get<string>('PORT', '/var/run/unraid-api.sock') ?? '';Consider reading the value once in the constructor or a private readonly field to avoid triple I/O and guard against
NaN.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
api/package.json(1 hunks)api/src/unraid-api/cli/internal-client.service.ts(4 hunks)packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*: Use pnpm as the only package manager
Always run scripts from api/package.json unless specifically requested otherwise
Files:
api/package.jsonapi/src/unraid-api/cli/internal-client.service.ts
api/src/unraid-api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
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/internal-client.service.ts
🧠 Learnings (16)
📚 Learning: 2025-03-14T19:18:23.324Z
Learnt from: pujitm
PR: unraid/api#1211
File: unraid-ui/package.json:43-43
Timestamp: 2025-03-14T19:18:23.324Z
Learning: The internationalized/number package is a dependency of reka-ui, which is used for number formatting in the Unraid UI components.
Applied to files:
api/package.json
📚 Learning: 2025-02-04T17:21:39.710Z
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Applied to files:
api/package.json
📚 Learning: 2025-02-05T14:43:25.062Z
Learnt from: elibosley
PR: unraid/api#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/package.json
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
Applied to files:
api/package.jsonapi/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Applied to files:
api/package.jsonpackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2025-03-14T16:10:38.562Z
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2024-12-13T17:28:15.384Z
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In `web/helpers/apollo-cache/index.ts`, it's acceptable to use `structuredClone` since the project targets Node.js v20 and browsers where `structuredClone` is supported.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-11-15T16:22:03.485Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:176-188
Timestamp: 2024-11-15T16:22:03.485Z
Learning: Atomic writes are not required for the `saveApiKey` method in `ApiKeyService` (`api/src/unraid-api/auth/api-key.service.ts`) unless specifically needed.
Applied to files:
api/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-11-04T20:44:46.432Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Applied to files:
api/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-11-04T22:00:41.946Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:42-105
Timestamp: 2024-11-04T22:00:41.946Z
Learning: In the 'AuthService' within `api/src/unraid-api/auth/auth.service.ts`, rate limiting is not required because the API is not public facing.
Applied to files:
api/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-11-15T16:22:12.460Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.module.ts:94-96
Timestamp: 2024-11-15T16:22:12.460Z
Learning: In `CookieService.defaultOpts()` in `api/src/unraid-api/auth/cookie.service.ts`, use `sameSite: 'lax'` for session cookie configuration.
Applied to files:
api/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-12-11T20:39:02.511Z
Learnt from: pujitm
PR: unraid/api#978
File: web/helpers/apollo-cache/index.ts:134-139
Timestamp: 2024-12-11T20:39:02.511Z
Learning: In `web/helpers/apollo-cache/index.ts`, the `cache.readQuery` method returns `null` when it fails to read from the cache, rather than throwing an exception.
Applied to files:
api/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-12-11T17:20:09.604Z
Learnt from: pujitm
PR: unraid/api#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:
api/src/unraid-api/cli/internal-client.service.ts
🔇 Additional comments (2)
api/package.json (1)
142-142: Verify necessity & compatibility of the newundiciruntime dependency
undiciis now bundled as a prod-dependency whilegotis already present.
Please double-check:
- The minimal Node version deployed in production is ≥ 18 ‑ if so, bundling
undicimerely for its fetch wrapper/Agent may be redundant.- There are no conflicting global-fetch polyfills at runtime (
globalThis.fetchfrom Node vsundicipolyfill).- Whether
gotusage can be consolidated or removed to avoid two HTTP stacks.If all three points check out, no action needed.
api/src/unraid-api/cli/internal-client.service.ts (1)
116-120: Non-standardOriginheader value
Origin: '/var/run/unraid-cli.sock'is not a valid origin per RFC 6454 (must bescheme://host[:port]).
Servers tolerant today may break under stricter CORS/proxy layers later. Suggest using a conventional origin such ashttp://localhostunless there is a compelling reason.
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
Outdated
Show resolved
Hide resolved
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
Outdated
Show resolved
Hide resolved
|
Claude finished @elibosley's task —— View job Code Review CompleteReview Tasks
Overall AssessmentThis PR successfully refactors the internal API clients to support Unix socket connections. The implementation is well-structured and follows good practices. ✅ StrengthsCode Quality & Architecture:
Socket Implementation:
Performance:
|
|
Claude finished @elibosley's task —— View job Code Review CompleteReview Tasks
Overall AssessmentThis PR successfully refactors the internal API clients to support Unix socket connections. The implementation is well-structured and follows good practices. ✅ StrengthsCode Quality & Architecture:
Socket Implementation:
Performance:
|
|
Claude finished @elibosley's task —— View job Code Review CompleteReview Tasks
Overall AssessmentThis PR successfully refactors the internal API clients to support Unix socket connections. The implementation is well-structured and follows good practices. ✅ StrengthsCode Quality & Architecture:
Socket Implementation:
Performance:
|
|
Claude finished @elibosley's task —— View job Code Review CompleteReview Tasks
Overall AssessmentThis PR successfully refactors the internal API clients to support Unix socket connections. The implementation is well-structured and follows good practices. ✅ StrengthsCode Quality & Architecture:
Socket Implementation:
Performance:
|
|
Claude finished @elibosley's task —— View job Code Review CompleteReview Tasks
Overall AssessmentThis PR successfully refactors the internal API clients to support Unix socket connections. The implementation is well-structured and follows good practices. ✅ StrengthsCode Quality & Architecture:
Socket Implementation:
Performance:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
api/src/unraid-api/shared/internal-graphql-client.factory.ts (1)
98-101: WebSocket URI may break on pure Unix-socket deploymentsWhen running solely on the Unix socket,
'ws://localhost/graphql'will fail because no TCP listener exists.Guard against this (e.g., disable subscriptions or expose a bridge) before attempting WS connections.
🧹 Nitpick comments (4)
packages/unraid-api-plugin-connect/package.json (1)
65-66: Avoid duplicatingundiciacross dependency blocks
undiciis now listed in bothdevDependenciesandpeerDependencies.
Keeping it in one place (peer or runtime deps, depending on actual runtime use) avoids version-resolution surprises and audit noise.Also applies to: 101-102
api/src/unraid-api/shared/internal-graphql-client.factory.ts (1)
102-118: Consider reusing a sharedAgent
new Agent({ connect: { socketPath }})is instantiated on everycreateClientcall.
If many modules call the factory, this can leak file descriptors. Holding a singleton agent or usingkeepAlive: truewould be lighter.packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts (1)
24-49: Concurrency race is minor but worth notingTwo concurrent
getClient()calls before the first finishes will build two Apollo clients, with the second discarded.
Caching the pending promise or guarding with a mutex removes this micro-race.packages/unraid-shared/src/services/base-internal-client.service.ts (1)
41-44: Avoid mutating the caller-suppliedoptionsobject
this.options.origin = …mutates the options instance that was passed in, which may be shared by other consumers. Create a shallow clone instead to prevent unintended side-effects.- this.options.origin = this.options.origin ?? '/var/run/unraid-cli.sock'; + this.options = { + origin: '/var/run/unraid-cli.sock', + ...this.options, + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
api/dev/configs/api.json(1 hunks)api/src/unraid-api/cli/cli-services.module.ts(2 hunks)api/src/unraid-api/cli/internal-client.service.ts(2 hunks)api/src/unraid-api/plugin/global-deps.module.ts(4 hunks)api/src/unraid-api/shared/internal-graphql-client.factory.ts(1 hunks)packages/unraid-api-plugin-connect/package.json(2 hunks)packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts(1 hunks)packages/unraid-api-plugin-connect/tsconfig.json(1 hunks)packages/unraid-api-plugin-generator/tsconfig.build.json(1 hunks)packages/unraid-api-plugin-generator/tsconfig.json(1 hunks)packages/unraid-api-plugin-health/tsconfig.json(1 hunks)packages/unraid-shared/package.json(2 hunks)packages/unraid-shared/src/index.ts(1 hunks)packages/unraid-shared/src/services/base-internal-client.service.ts(1 hunks)packages/unraid-shared/src/tokens.ts(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- packages/unraid-api-plugin-connect/tsconfig.json
- api/dev/configs/api.json
- packages/unraid-api-plugin-generator/tsconfig.build.json
- packages/unraid-shared/src/tokens.ts
- packages/unraid-api-plugin-generator/tsconfig.json
- packages/unraid-api-plugin-health/tsconfig.json
- packages/unraid-shared/src/index.ts
- packages/unraid-shared/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/cli/internal-client.service.ts
🧰 Additional context used
📓 Path-based instructions (2)
api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*: Use pnpm as the only package manager
Always run scripts from api/package.json unless specifically requested otherwise
Files:
api/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.tsapi/src/unraid-api/shared/internal-graphql-client.factory.ts
api/src/unraid-api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
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/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.tsapi/src/unraid-api/shared/internal-graphql-client.factory.ts
🧠 Learnings (39)
📓 Common learnings
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Learnt from: elibosley
PR: unraid/api#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`.
Learnt from: pujitm
PR: unraid/api#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-06-11T14:14:30.348Z
Learnt from: pujitm
PR: unraid/api#1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
Applied to files:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2025-02-04T17:21:39.710Z
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Applied to files:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2025-03-14T19:18:23.324Z
Learnt from: pujitm
PR: unraid/api#1211
File: unraid-ui/package.json:43-43
Timestamp: 2025-03-14T19:18:23.324Z
Learning: The internationalized/number package is a dependency of reka-ui, which is used for number formatting in the Unraid UI components.
Applied to files:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2025-02-05T14:43:25.062Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2024-11-27T15:30:02.252Z
Learnt from: elibosley
PR: unraid/api#969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the `setup` commands in `web/justfile` and `api/justfile` are intentional and correct behavior.
Applied to files:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Use Vitest as the test suite; do not use Jest
Applied to files:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-api-plugin-connect/package.jsonapi/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/**/*.{test,spec}.{ts,js} : Test suite is VITEST, do not use jest in the API package
Applied to files:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Avoid relying on Nuxt's auto-imports in test environment
Applied to files:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Mock external dependencies and services in tests
Applied to files:
packages/unraid-api-plugin-connect/package.jsonapi/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Mock external dependencies appropriately in store tests
Applied to files:
packages/unraid-api-plugin-connect/package.jsonapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Don't rely on Nuxt auto-imports in tests
Applied to files:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Remember that `vi.mock()` calls are hoisted
Applied to files:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use `vi.mock()` for module-level mocks
Applied to files:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
PR: unraid/api#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:
packages/unraid-api-plugin-connect/package.json
📚 Learning: 2024-12-13T17:28:15.384Z
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In `web/helpers/apollo-cache/index.ts`, it's acceptable to use `structuredClone` since the project targets Node.js v20 and browsers where `structuredClone` is supported.
Applied to files:
packages/unraid-api-plugin-connect/package.jsonapi/src/unraid-api/shared/internal-graphql-client.factory.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js} : Mock external dependencies and services in web/__test__
Applied to files:
packages/unraid-api-plugin-connect/package.jsonapi/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
PR: unraid/api#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/cli-services.module.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/plugin/global-deps.module.tsapi/src/unraid-api/shared/internal-graphql-client.factory.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
api/src/unraid-api/cli/cli-services.module.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/plugin/global-deps.module.tsapi/src/unraid-api/shared/internal-graphql-client.factory.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
api/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use factory functions for module mocks to avoid hoisting issues
Applied to files:
api/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/plugin/global-deps.module.tsapi/src/unraid-api/shared/internal-graphql-client.factory.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-15T16:22:03.485Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:176-188
Timestamp: 2024-11-15T16:22:03.485Z
Learning: Atomic writes are not required for the `saveApiKey` method in `ApiKeyService` (`api/src/unraid-api/auth/api-key.service.ts`) unless specifically needed.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/shared/internal-graphql-client.factory.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-14T16:10:38.562Z
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2024-11-15T16:22:12.460Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.module.ts:94-96
Timestamp: 2024-11-15T16:22:12.460Z
Learning: In `CookieService.defaultOpts()` in `api/src/unraid-api/auth/cookie.service.ts`, use `sameSite: 'lax'` for session cookie configuration.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-01-15T21:34:00.006Z
Learnt from: pujitm
PR: unraid/api#1047
File: api/src/unraid-api/graph/sandbox-plugin.ts:57-57
Timestamp: 2025-01-15T21:34:00.006Z
Learning: In the GraphQL sandbox (api/src/unraid-api/graph/sandbox-plugin.ts), CSRF token validation should fail silently with a fallback value to maintain sandbox accessibility, as it's a development tool where strict security measures aren't required.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/plugin/global-deps.module.tsapi/src/unraid-api/shared/internal-graphql-client.factory.ts
📚 Learning: 2025-03-10T18:43:13.016Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/types/fastify.ts:25-30
Timestamp: 2025-03-10T18:43:13.016Z
Learning: In the Unraid API, cookies should be marked as optional (with `?`) in the FastifyRequest type definition since the cookies may not always be present, which is consistent with the authentication flow throughout the codebase.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under `web/` must follow this naming convention
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-01-30T19:56:42.055Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations)
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-01-30T19:38:02.478Z
Learnt from: pujitm
PR: unraid/api#1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-04-23T20:22:36.001Z
Learnt from: pujitm
PR: unraid/api#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/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-03-12T13:35:43.900Z
Learnt from: pujitm
PR: unraid/api#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:
api/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-03-03T16:04:20.705Z
Learnt from: pujitm
PR: unraid/api#1214
File: web/.gitignore:1-2
Timestamp: 2025-03-03T16:04:20.705Z
Learning: In the Unraid web application, environment variables with the VITE_ prefix (like VITE_CALLBACK_KEY) are intentionally exposed client-side and are not considered sensitive secrets, so it's acceptable to track them in Git.
Applied to files:
api/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2024-11-04T20:44:46.432Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-04-23T20:19:42.542Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
🔇 Additional comments (2)
api/src/unraid-api/cli/cli-services.module.ts (1)
15-16: Provider registration LGTMImporting and providing
InternalGraphQLClientFactoryhere is correct and keeps the CLI module self-contained.Also applies to: 36-37
packages/unraid-shared/src/services/base-internal-client.service.ts (1)
112-135:Originheader value is not a valid originAn HTTP
Originheader must be a scheme + host (e.g.http://localhost).
Using the default socket path (/var/run/unraid-cli.sock) violates the spec and may be stripped or rejected by intermediaries.Consider mapping socket mode to a conventional origin, e.g.:
- Origin: this.options.origin!, + Origin: this.isRunningOnSocket() ? 'http://localhost' : this.options.origin!,
packages/unraid-shared/src/services/base-internal-client.service.ts
Outdated
Show resolved
Hide resolved
packages/unraid-shared/src/services/base-internal-client.service.ts
Outdated
Show resolved
Hide resolved
packages/unraid-shared/src/services/base-internal-client.service.ts
Outdated
Show resolved
Hide resolved
|
Claude finished @elibosley's task —— View job Code Review CompleteReview Tasks
Overall AssessmentThis PR successfully refactors the internal API clients to support Unix socket connections. The implementation is well-structured and follows good practices. ✅ StrengthsCode Quality & Architecture:
Socket Implementation:
Performance:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
api/src/unraid-api/plugin/global-deps.module.ts (1)
55-64: Export the class provider to prevent per-module re-instantiationExport InternalGraphQLClientFactory so modules (e.g., CLI) can inject by class without re-providing it.
exports: [ UPNP_CLIENT_TOKEN, GRAPHQL_PUBSUB_TOKEN, API_KEY_SERVICE_TOKEN, NGINX_SERVICE_TOKEN, INTERNAL_CLIENT_SERVICE_TOKEN, + InternalGraphQLClientFactory, PrefixedID, LIFECYCLE_SERVICE_TOKEN, LifecycleService, ],
♻️ Duplicate comments (3)
api/src/unraid-api/plugin/global-deps.module.ts (1)
27-27: Deduplicate factory provider with useExistingRegistering the class and again with useClass under INTERNAL_CLIENT_SERVICE_TOKEN creates two instances. Alias the token to the existing class provider.
providers: [ - InternalGraphQLClientFactory, + InternalGraphQLClientFactory, { provide: UPNP_CLIENT_TOKEN, useValue: upnpClient, }, { provide: GRAPHQL_PUBSUB_TOKEN, useValue: pubsub, }, { provide: API_KEY_SERVICE_TOKEN, useClass: ApiKeyService, }, { provide: NGINX_SERVICE_TOKEN, useClass: NginxService, }, { - provide: INTERNAL_CLIENT_SERVICE_TOKEN, - useClass: InternalGraphQLClientFactory, + provide: INTERNAL_CLIENT_SERVICE_TOKEN, + useExisting: InternalGraphQLClientFactory, }, PrefixedID, LifecycleService,Also applies to: 44-47
api/src/unraid-api/cli/internal-client.service.ts (1)
59-62: ApolloClient.stop() is not part of Apollo 3.x; guard or use clearStore/resetStoreCalling
this.client?.stop()will throw at runtime on Apollo 3.x. Guard the call and clear/reset the store instead.Apply:
- // Stop the Apollo client to terminate any active processes - this.client?.stop(); - this.client = null; + // Best-effort shutdown to avoid dangling timers/pollers + if (this.client) { + // Some custom client wrappers expose stop(); Apollo 3.x doesn't. + (this.client as any).stop?.(); + // Avoid awaiting in a void method; swallow errors. + void this.client.clearStore().catch(() => undefined); + } + this.client = null;packages/unraid-shared/src/services/base-internal-client.service.ts (1)
226-235: Guard client shutdown; Apollo 3.x has nostop()Same concern as elsewhere: avoid calling a non-existent
stop(). Dispose WS client and clear the store.Apply:
- // Stop the Apollo client to terminate any active processes - this.client?.stop(); - // Clean up WebSocket client if it exists + // Best-effort shutdown + if (this.client) { + (this.client as any).stop?.(); // optional if a custom client adds it + void this.client.clearStore().catch(() => undefined); + } + // Clean up WebSocket client if it exists if (this.wsClient) { this.wsClient.dispose(); this.wsClient = null; } this.client = null;
🧹 Nitpick comments (11)
packages/unraid-shared/package.json (2)
47-47: Peer dep pin may be too strict for Apollo ClientExact pinning in peerDependencies can cause version conflicts in consumers. Consider widening the range.
Example:
- "@apollo/client": "3.13.8", + "@apollo/client": "^3.13.0"If you intentionally pin for mono-repo alignment, ignore. Otherwise, please confirm this won’t conflict with web/api packages that may float on minor versions.
60-61: Are rxjs and undici intended as peers?
- rxjs as peer implies consumers must provide the same major. If only types are used, devDependency suffices; if emitted code imports rxjs, keeping as peer is fine.
- undici as peer means consumers must install it. Since Node 18+ bundles undici APIs, consider making undici a regular dependency where needed (e.g., api package) and not a peer here, unless shared code directly imports undici at runtime.
If you keep them as peers, consider marking them optional to ease downstream installs:
"peerDependencies": { "rxjs": "7.8.2", "undici": "7.13.0" }, + "peerDependenciesMeta": { + "rxjs": { "optional": true }, + "undici": { "optional": true } + }api/src/unraid-api/shared/internal-graphql-client.factory.ts (3)
97-98: Logger placeholders are not formatted — use template strings / paramsNest Logger doesn’t apply %s/%o formatting. Use template strings or pass objects as additional params.
- this.logger.debug('Creating GraphQL client using Unix socket: %s', socketPath); + this.logger.debug(`Creating GraphQL client using Unix socket: ${socketPath}`); ... - this.logger.debug('WebSocket subscriptions over Unix socket: %s', wsUri); + this.logger.debug(`WebSocket subscriptions over Unix socket: ${wsUri}`); ... - this.logger.debug('Creating GraphQL client using HTTP: %s', httpUri); + this.logger.debug(`Creating GraphQL client using HTTP: ${httpUri}`); ... - this.logger.warn('[GRAPHQL-CLIENT] NETWORK ERROR ENCOUNTERED %o', networkError); + this.logger.warn('[GRAPHQL-CLIENT] NETWORK ERROR ENCOUNTERED', networkError as any);Also applies to: 102-103, 130-130, 148-148
152-160: Explicit WebSocket implementation for NodeIn Node, pass a WebSocket implementation to graphql-ws or ensure ws is installed. Otherwise, subscriptions may fail at runtime.
If ws is available:
- const wsLink = new GraphQLWsLink( - createClient({ - url: wsUri, - connectionParams: () => ({ 'x-api-key': apiKey }), - }) - ); + const wsLink = new GraphQLWsLink( + createClient({ + url: wsUri, + // @ts-expect-error Provide WebSocket impl in Node + webSocketImpl: (globalThis as any).WebSocket ?? (await import('ws')).default, + connectionParams: () => ({ 'x-api-key': apiKey }), + }) + );If you prefer not to add dynamic import, add ws as a dependency in the API package and import it statically. I can open a follow-up PR.
105-121: Close undici Agent on client teardownWhen creating a new Agent per client, consider exposing a cleanup to close the dispatcher when the Apollo client is stopped to avoid FD leaks on short-lived clients.
Approach: wrap the link to capture the Agent and register a disposer, or make the Agent a singleton per-socketPath within this factory.
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts (1)
41-45: Confirm Origin header for ConnectFactory defaults Origin to '/var/run/unraid-cli.sock'. If Connect requires a distinct origin, pass it explicitly.
- const client = await this.clientFactory.createClient({ - apiKey: localApiKey, - enableSubscriptions: true - }); + const client = await this.clientFactory.createClient({ + apiKey: localApiKey, + enableSubscriptions: true, + origin: '/var/run/unraid-connect.sock', // if required for Connect + });Please confirm the expected Origin value used for policy checks upstream.
packages/unraid-shared/src/services/base-internal-client.service.ts (5)
123-136: Headers duplication: consider a small helper to avoid driftThe HTTP headers are duplicated in both socket and TCP branches. Factor to a single object to reduce maintenance risk.
Apply:
- if (this.isRunningOnSocket()) { + const commonHeaders = { + Origin: this.options.origin!, + 'x-api-key': apiKey, + 'Content-Type': 'application/json', + }; + + if (this.isRunningOnSocket()) { // ... httpLink = new HttpLink({ uri: 'http://localhost/graphql', fetch: ((uri: any, options: any) => { return undiciFetch(uri as string, { ...options, dispatcher: agent, } as any); }) as unknown as typeof fetch, - headers: { - Origin: this.options.origin!, - 'x-api-key': apiKey, - 'Content-Type': 'application/json', - }, + headers: commonHeaders, }); } else { const httpUri = this.getApiAddress('http'); this.logger.debug('Internal GraphQL URL: %s', httpUri); httpLink = new HttpLink({ uri: httpUri, fetch, - headers: { - Origin: this.options.origin!, - 'x-api-key': apiKey, - 'Content-Type': 'application/json', - }, + headers: commonHeaders, }); }Also applies to: 141-149
152-156: Also log GraphQL errors for better diagnosticsCurrently only
networkErroris logged. IncludegraphQLErrorssummaries to aid debugging.Apply:
- const errorLink = onError(({ networkError }) => { - if (networkError) { - this.logger.warn('[GRAPHQL-CLIENT] NETWORK ERROR ENCOUNTERED %o', networkError); - } - }); + const errorLink = onError(({ graphQLErrors, networkError }) => { + if (graphQLErrors?.length) { + for (const err of graphQLErrors) { + this.logger.warn(`[GRAPHQL-CLIENT] GQL ERROR: ${err.message}`, (err.extensions as any)?.code); + } + } + if (networkError) { + this.logger.warn('[GRAPHQL-CLIENT] NETWORK ERROR %o', networkError); + } + });
192-204: Default options inconsistent between WS and HTTP-only branchesWS branch sets
mutate.fetchPolicy: 'no-cache', HTTP-only branch doesn't. Align behavior to avoid cache surprises.Apply:
return new ApolloClient({ defaultOptions: { query: { fetchPolicy: 'no-cache', }, + mutate: { + fetchPolicy: 'no-cache', + }, }, cache: new InMemoryCache(), link: errorLink.concat(httpLink), });Also applies to: 206-215
98-106: Error type: consider including the underlying message/stackWrapping and rethrowing as a generic
Errorloses context. Includecauseor appenderror?.messagefor easier diagnosis.Apply:
- throw new Error( - 'Unable to get API key for internal client. Ensure the API server is running.' - ); + throw new Error( + `Unable to get API key for internal client. Ensure the API server is running. ${(error as Error)?.message ?? ''}`, + { cause: error as Error } + );
38-46: Avoid mutating theoptionsargumentMutating
this.options.originin the constructor can surprise callers who reuse the same object. Prefer an immutable merge.Apply:
- ) { - this.logger = new Logger(this.constructor.name); - this.options.origin = this.options.origin ?? '/var/run/unraid-cli.sock'; - } + ) { + this.logger = new Logger(this.constructor.name); + this.options = { origin: 'http://127.0.0.1', ...options }; + }Note: if you adopt this, update the constructor signature to
options: InternalClientOptions = {}withoutreadonly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
api/dev/configs/api.json(1 hunks)api/src/unraid-api/cli/cli-services.module.ts(2 hunks)api/src/unraid-api/cli/internal-client.service.ts(2 hunks)api/src/unraid-api/plugin/global-deps.module.ts(4 hunks)api/src/unraid-api/shared/internal-graphql-client.factory.ts(1 hunks)packages/unraid-api-plugin-connect/package.json(2 hunks)packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts(1 hunks)packages/unraid-api-plugin-connect/tsconfig.json(1 hunks)packages/unraid-api-plugin-generator/tsconfig.build.json(1 hunks)packages/unraid-api-plugin-generator/tsconfig.json(1 hunks)packages/unraid-api-plugin-health/tsconfig.json(1 hunks)packages/unraid-shared/package.json(2 hunks)packages/unraid-shared/src/index.ts(1 hunks)packages/unraid-shared/src/services/base-internal-client.service.ts(1 hunks)packages/unraid-shared/src/tokens.ts(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- packages/unraid-api-plugin-generator/tsconfig.json
- packages/unraid-api-plugin-connect/tsconfig.json
- api/dev/configs/api.json
- packages/unraid-api-plugin-connect/package.json
- packages/unraid-api-plugin-health/tsconfig.json
- packages/unraid-api-plugin-generator/tsconfig.build.json
- packages/unraid-shared/src/index.ts
- packages/unraid-shared/src/tokens.ts
🧰 Additional context used
📓 Path-based instructions (2)
api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*: Use pnpm as the only package manager
Always run scripts from api/package.json unless specifically requested otherwise
Files:
api/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/shared/internal-graphql-client.factory.tsapi/src/unraid-api/plugin/global-deps.module.tsapi/src/unraid-api/cli/internal-client.service.ts
api/src/unraid-api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
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/cli-services.module.tsapi/src/unraid-api/shared/internal-graphql-client.factory.tsapi/src/unraid-api/plugin/global-deps.module.tsapi/src/unraid-api/cli/internal-client.service.ts
🧠 Learnings (48)
📓 Common learnings
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Learnt from: pujitm
PR: unraid/api#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: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
PR: unraid/api#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/cli-services.module.tsapi/src/unraid-api/shared/internal-graphql-client.factory.tsapi/src/unraid-api/plugin/global-deps.module.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
api/src/unraid-api/cli/cli-services.module.tspackages/unraid-shared/package.jsonapi/src/unraid-api/shared/internal-graphql-client.factory.tsapi/src/unraid-api/plugin/global-deps.module.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
api/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/shared/internal-graphql-client.factory.tsapi/src/unraid-api/plugin/global-deps.module.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
api/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
PR: unraid/api#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/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use factory functions for module mocks to avoid hoisting issues
Applied to files:
api/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Mock external dependencies and services in tests
Applied to files:
api/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js} : Mock external dependencies and services in web/__test__
Applied to files:
api/src/unraid-api/cli/cli-services.module.tsapi/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-02-04T17:21:39.710Z
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Applied to files:
packages/unraid-shared/package.json
📚 Learning: 2025-02-05T14:43:25.062Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/package.json
📚 Learning: 2025-03-14T19:18:23.324Z
Learnt from: pujitm
PR: unraid/api#1211
File: unraid-ui/package.json:43-43
Timestamp: 2025-03-14T19:18:23.324Z
Learning: The internationalized/number package is a dependency of reka-ui, which is used for number formatting in the Unraid UI components.
Applied to files:
packages/unraid-shared/package.json
📚 Learning: 2024-12-13T17:28:15.384Z
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In `web/helpers/apollo-cache/index.ts`, it's acceptable to use `structuredClone` since the project targets Node.js v20 and browsers where `structuredClone` is supported.
Applied to files:
packages/unraid-shared/package.jsonapi/src/unraid-api/shared/internal-graphql-client.factory.tsapi/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under `web/` must follow this naming convention
Applied to files:
packages/unraid-shared/package.jsonpackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Always run pnpm codegen for GraphQL code generation in the web directory
Applied to files:
packages/unraid-shared/package.json
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Always run `pnpm codegen` for GraphQL code generation in the web directory
Applied to files:
packages/unraid-shared/package.json
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations)
Applied to files:
packages/unraid-shared/package.jsonpackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.query.ts : GraphQL queries must be placed in .query.ts files under web/
Applied to files:
packages/unraid-shared/package.json
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.mutation.ts : GraphQL mutations must be placed in .mutation.ts files under web/
Applied to files:
packages/unraid-shared/package.json
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.query.ts : GraphQL queries must be placed in `.query.ts` files
Applied to files:
packages/unraid-shared/package.json
📚 Learning: 2025-04-21T18:44:24.383Z
Learnt from: elibosley
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.entity.ts:37-39
Timestamp: 2025-04-21T18:44:24.383Z
Learning: The project prefers using class-validator decorators directly on entity classes rather than separate Zod schemas to maintain type consistency between GraphQL entities and validation rules.
Applied to files:
packages/unraid-shared/package.json
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.mutation.ts : GraphQL mutations must be placed in `.mutation.ts` files
Applied to files:
packages/unraid-shared/package.json
📚 Learning: 2024-11-15T16:22:03.485Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:176-188
Timestamp: 2024-11-15T16:22:03.485Z
Learning: Atomic writes are not required for the `saveApiKey` method in `ApiKeyService` (`api/src/unraid-api/auth/api-key.service.ts`) unless specifically needed.
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tsapi/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-15T21:34:00.006Z
Learnt from: pujitm
PR: unraid/api#1047
File: api/src/unraid-api/graph/sandbox-plugin.ts:57-57
Timestamp: 2025-01-15T21:34:00.006Z
Learning: In the GraphQL sandbox (api/src/unraid-api/graph/sandbox-plugin.ts), CSRF token validation should fail silently with a fallback value to maintain sandbox accessibility, as it's a development tool where strict security measures aren't required.
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.tsapi/src/unraid-api/plugin/global-deps.module.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-04T20:44:46.432Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.tsapi/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-04-23T20:22:36.001Z
Learnt from: pujitm
PR: unraid/api#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/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Mock external dependencies appropriately in store tests
Applied to files:
api/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-03-12T13:35:43.900Z
Learnt from: pujitm
PR: unraid/api#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:
api/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
Applied to files:
api/src/unraid-api/plugin/global-deps.module.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Applied to files:
api/src/unraid-api/plugin/global-deps.module.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
PR: unraid/api#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/plugin/global-deps.module.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*store*.{test,spec}.{ts,js} : Use factory functions for module mocks to avoid hoisting issues in store tests
Applied to files:
api/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-03-03T16:04:20.705Z
Learnt from: pujitm
PR: unraid/api#1214
File: web/.gitignore:1-2
Timestamp: 2025-03-03T16:04:20.705Z
Learning: In the Unraid web application, environment variables with the VITE_ prefix (like VITE_CALLBACK_KEY) are intentionally exposed client-side and are not considered sensitive secrets, so it's acceptable to track them in Git.
Applied to files:
api/src/unraid-api/plugin/global-deps.module.ts
📚 Learning: 2025-03-14T16:10:38.562Z
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Applied to files:
api/src/unraid-api/plugin/global-deps.module.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2024-11-15T16:22:12.460Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.module.ts:94-96
Timestamp: 2024-11-15T16:22:12.460Z
Learning: In `CookieService.defaultOpts()` in `api/src/unraid-api/auth/cookie.service.ts`, use `sameSite: 'lax'` for session cookie configuration.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-10T18:43:13.016Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/types/fastify.ts:25-30
Timestamp: 2025-03-10T18:43:13.016Z
Learning: In the Unraid API, cookies should be marked as optional (with `?`) in the FastifyRequest type definition since the cookies may not always be present, which is consistent with the authentication flow throughout the codebase.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-30T19:56:42.055Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2025-01-30T19:38:02.478Z
Learnt from: pujitm
PR: unraid/api#1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
📚 Learning: 2024-11-04T22:00:41.946Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:42-105
Timestamp: 2024-11-04T22:00:41.946Z
Learning: In the 'AuthService' within `api/src/unraid-api/auth/auth.service.ts`, rate limiting is not required because the API is not public facing.
Applied to files:
api/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-11-04T20:41:22.303Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:41:22.303Z
Learning: In `api/src/unraid-api/auth/auth.service.ts`, the `addRoleToApiKey` function operates on API keys stored as JSON files in a directory, not a database, so concurrency is not a concern for modifying `apiKey.roles`.
Applied to files:
api/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2025-04-23T20:19:42.542Z
Learnt from: pujitm
PR: unraid/api#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/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-27T13:34:53.438Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-04-07T14:34:47.255Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-28T16:39:50.278Z
Learnt from: elibosley
PR: unraid/api#1078
File: api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts:35-36
Timestamp: 2025-01-28T16:39:50.278Z
Learning: When reviewing TypeScript files containing PHP code snippets intended for file modifications (like in `SSOFileModification`), PHP functions that appear undefined should not be flagged as issues since they will be available in the target PHP environment where the code is injected.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-27T14:57:46.617Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-11T20:39:02.511Z
Learnt from: pujitm
PR: unraid/api#978
File: web/helpers/apollo-cache/index.ts:134-139
Timestamp: 2024-12-11T20:39:02.511Z
Learning: In `web/helpers/apollo-cache/index.ts`, the `cache.readQuery` method returns `null` when it fails to read from the cache, rather than throwing an exception.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-11T17:20:09.604Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
🔇 Additional comments (3)
api/src/unraid-api/cli/internal-client.service.ts (1)
48-53: Good DI refactor; factory usage is cleanShifting client construction to
InternalGraphQLClientFactoryand disabling subscriptions for CLI is the right call. Keeps concerns separated and consistent with the new architecture.packages/unraid-shared/src/services/base-internal-client.service.ts (2)
69-75: Fix NaN handling for PORT and falsy check in getApiAddress
Number(port)can yieldNaN, andif (numericPort)treatsNaNlike "no port" (and will also skip on port0). Returnundefinedfor invalid input and explicitly branch onnumbertype.
[ suggest_essential_refactor ]
[ duplicate_comment ]
Apply:- private getNumericPort() { - const port = this.configService.get<string>('PORT', '/var/run/unraid-api.sock'); - if (port.includes('.sock')) { - return undefined; - } - return Number(port); - } + private getNumericPort() { + const port = this.configService.get<string>('PORT', '/var/run/unraid-api.sock'); + if (port.includes('.sock')) return undefined; + const parsed = Number(port); + return Number.isFinite(parsed) && parsed > 0 ? parsed : undefined; + }- const numericPort = this.getNumericPort(); - if (numericPort) { + const numericPort = this.getNumericPort(); + if (typeof numericPort === 'number') { return `${protocol}://127.0.0.1:${numericPort}/graphql`; }Also applies to: 83-92
44-45: I don’t see any CSRF enforcement or origin validation on the server—CORS is wide-open (origin: true) and only applies to browser clients, and Node-to-Node calls aren’t subject to it. Defaultingoptions.originto a filesystem path does not break functionality. As this header is only used for internal service calls (never in a browser) and the API server does not validate it, no change is needed here.Likely an incorrect or invalid review comment.
packages/unraid-shared/src/services/base-internal-client.service.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
api/src/unraid-api/cli/cli.module.spec.ts (1)
29-31: Nit: Add generics tomodule.getfor stronger typing (align with other spec)Use explicit generics as in the other spec to improve type safety and consistency.
- const service = module.get(CliInternalClientService); + const service = module.get<CliInternalClientService>(CliInternalClientService); ... - const service = module.get(AdminKeyService); + const service = module.get<AdminKeyService>(AdminKeyService); ... - const factory = module.get(InternalGraphQLClientFactory); + const factory = module.get<InternalGraphQLClientFactory>(InternalGraphQLClientFactory);Also applies to: 35-38, 41-44
api/src/unraid-api/cli/internal-client.service.spec.ts (3)
48-50: Ensure test isolation: clear/reset mocks inafterEach
mockApolloClientis created once at describe scope and its spies persist across tests. Clear/reset between tests to avoid cross-test leakage.- afterEach(async () => { - await module?.close(); - }); + afterEach(async () => { + // Ensure no cross-test leakage of calls/state + vi.clearAllMocks(); + // Explicitly reset shared client spies + mockApolloClient.query.mockReset(); + mockApolloClient.mutate.mockReset(); + mockApolloClient.stop.mockReset(); + await module?.close(); + });
23-41: Optional: Re-initialize the shared Apollo client mocks per testAn alternative to resetting in
afterEachis to re-create fresh spies inbeforeEach, ensuring every test starts clean.beforeEach(async () => { + // Fresh spies each test to ensure isolation + mockApolloClient.query = vi.fn(); + mockApolloClient.mutate = vi.fn(); + mockApolloClient.stop = vi.fn(); module = await Test.createTestingModule({ imports: [ConfigModule.forRoot()], providers: [ CliInternalClientService, { provide: InternalGraphQLClientFactory, useValue: { - createClient: vi.fn().mockResolvedValue(mockApolloClient), + createClient: vi.fn().mockResolvedValue(mockApolloClient), }, }, { provide: AdminKeyService, useValue: { getOrCreateLocalAdminKey: vi.fn().mockResolvedValue('test-admin-key'), }, }, ], }).compile();
96-96: Add coverage for factory creation failureCurrently only the admin key failure is tested. Also verify behavior when
createClientrejects.describe('getClient', () => { ... it('should handle errors when getting admin key', async () => { const error = new Error('Failed to get admin key'); vi.mocked(adminKeyService.getOrCreateLocalAdminKey).mockRejectedValueOnce(error); await expect(service.getClient()).rejects.toThrow( 'Unable to get admin API key for internal client' ); }); + + it('should surface errors when client factory creation fails', async () => { + const error = new Error('factory boom'); + vi.mocked(clientFactory.createClient).mockRejectedValueOnce(error); + await expect(service.getClient()).rejects.toThrow(); // assert rejection without overfitting message + }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/src/unraid-api/cli/cli.module.spec.ts(1 hunks)api/src/unraid-api/cli/cli.module.ts(2 hunks)api/src/unraid-api/cli/internal-client.service.spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- api/src/unraid-api/cli/cli.module.ts
🧰 Additional context used
📓 Path-based instructions (4)
api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*: Use pnpm as the only package manager
Always run scripts from api/package.json unless specifically requested otherwise
Files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
api/src/unraid-api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
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/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
api/**/*.{test,spec}.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,ts,tsx}: Use Vitest as the test suite; do not use Jest
Prefer not to mock simple dependencies in tests
Files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
api/**/*.{test,spec}.{ts,js}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Test suite is VITEST, do not use jest in the API package
Files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Learnt from: pujitm
PR: unraid/api#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-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Use Vitest as the test suite; do not use Jest
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/**/*.{test,spec}.{ts,js} : Test suite is VITEST, do not use jest in the API package
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js} : Mock external dependencies and services in web/__test__
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Mock external dependencies and services in tests
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify state changes by updating the store in store tests
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Mock external dependencies appropriately in store tests
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*store*.{test,spec}.{ts,js} : Use factory functions for module mocks to avoid hoisting issues in store tests
Applied to files:
api/src/unraid-api/cli/cli.module.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use `vi.mock()` for module-level mocks
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use factory functions for module mocks to avoid hoisting issues
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify state changes after actions in store tests
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Prefer not to mock simple dependencies in tests
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Avoid relying on Nuxt's auto-imports in test environment
Applied to files:
api/src/unraid-api/cli/cli.module.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test getter dependencies are properly mocked in store tests
Applied to files:
api/src/unraid-api/cli/cli.module.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
PR: unraid/api#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/internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Clear mocks between tests to ensure isolation
Applied to files:
api/src/unraid-api/cli/internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Reset mocks between tests with `vi.clearAllMocks()`
Applied to files:
api/src/unraid-api/cli/internal-client.service.spec.ts
🔇 Additional comments (1)
api/src/unraid-api/cli/cli.module.spec.ts (1)
4-4: Good: Using Vitest in api testsCorrect test runner and imports for api/* per project rules. No Jest usage.
|
Claude finished @elibosley's task —— View job I'll analyze this and get back to you. |
|
Claude finished @elibosley's task —— View job I'll analyze this and get back to you. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/unraid-shared/src/services/base-internal-client.service.ts (2)
120-126: Still missing explicitwebSocketImplfor Node /ws+unixURLsPrior feedback noted that Node’s global WebSocket (undici) doesn’t handle
ws+unix://schemes.
createClient()should be given a compatible implementation (e.g. thewspackage) or subscriptions disabled in socket mode.
173-176:ApolloClient.stop()is not part of the public APIApollo Client 3.x exposes
clearStore()/resetStore()but nostop()method.
Calling it risks a runtime crash.Reuse the earlier suggestion:
- this.client?.stop(); + if (this.client) { + (this.client as any).stop?.(); // no-op if absent + await this.client.clearStore().catch(() => undefined); + }
🧹 Nitpick comments (8)
packages/unraid-shared/src/services/socket-config.service.ts (1)
52-61: Truthiness check hides valid port0
if (numericPort)will treat0as false and fall through to the nginx-port branch even though0is a valid numeric value. Check fornumericPort !== undefinedinstead.api/src/unraid-api/cli/internal-client.service.ts (2)
21-24: Consider exposing a dedicated DI token for the factory
INTERNAL_CLIENT_SERVICE_TOKENresolves to anInternalGraphQLClientFactoryinstance, but the token name suggests a service rather than a factory. Renaming the constant (e.g.INTERNAL_GRAPHQL_CLIENT_FACTORY_TOKEN) or injecting the class directly would make intent clearer and avoid future confusion in module registration.
60-64:stop()is asynchronous – await or ignore errors explicitly
ApolloClient.stop()returns a promise.
If the CLI process exits immediately afterclearClient(), outstanding asynchronous work could be abandoned. Either:-// Stop the Apollo client to terminate any active processes -this.client?.stop(); +// Ensure the promise resolves before clearing reference +await this.client?.stop().catch(err => this.logger.debug('Client stop failed', err));or document why fire-and-forget is acceptable here.
packages/unraid-shared/src/services/base-internal-client.service.spec.ts (2)
55-59: Unused stub can be removed
mockApolloClientis created but never injected or asserted, leaving dead code in the suite.
156-168:stopspy may break on future Apollo upgradesThe test spies on a private method (
stop) that is not part of Apollo’s public API and may disappear.
A safer pattern is to spy onservice.clearClient()side-effects (e.g.,this.client = null) or inject a custom client with a controlledstopstub.packages/unraid-shared/src/services/socket-config.service.spec.ts (1)
10-13: Prefer a lightweight stub over realConfigServiceInstantiating the real
ConfigServicepulls in its internal validation logic that the unit under test doesn’t need. A plain object with agetmock keeps the test faster and eliminates implicit behaviour changes across NestJS versions.packages/unraid-shared/src/services/base-internal-client.service.ts (2)
73-87: Unnecessary wrapper URI may break absolute-path routingWhen using a Unix socket, the request still targets
http://localhost/graphql, meaning the HTTPHostheader is set tolocalhost.
If the backend relies on virtual-host routing (or absolute redirects) this diverges from production behaviour.Either:
- Use
http://unix/graphql(commonly used dummy host for socket mode), or- Add an explicit
Hostheader that mirrors external traffic expectations.
139-150:mutatedefaultOptions omitted in HTTP-only branchThe subscription branch sets both
queryandmutatefetch policies to'no-cache', but the HTTP-only path below omitsmutate, causing inconsistent caching behaviour.Add the same
mutateblock for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
api/src/unraid-api/cli/cli.module.spec.ts(1 hunks)api/src/unraid-api/cli/internal-client.service.spec.ts(1 hunks)api/src/unraid-api/cli/internal-client.service.ts(2 hunks)api/src/unraid-api/plugin/global-deps.module.ts(3 hunks)api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts(1 hunks)api/src/unraid-api/shared/internal-graphql-client.factory.ts(1 hunks)packages/unraid-shared/package.json(4 hunks)packages/unraid-shared/src/index.ts(1 hunks)packages/unraid-shared/src/services/base-internal-client.service.spec.ts(1 hunks)packages/unraid-shared/src/services/base-internal-client.service.ts(1 hunks)packages/unraid-shared/src/services/socket-config.service.spec.ts(1 hunks)packages/unraid-shared/src/services/socket-config.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/unraid-shared/package.json
- api/src/unraid-api/plugin/global-deps.module.ts
- api/src/unraid-api/cli/internal-client.service.spec.ts
- packages/unraid-shared/src/index.ts
- api/src/unraid-api/cli/cli.module.spec.ts
- api/src/unraid-api/shared/internal-graphql-client.factory.ts
🧰 Additional context used
📓 Path-based instructions (4)
api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*: Use pnpm as the only package manager
Always run scripts from api/package.json unless specifically requested otherwise
Files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tsapi/src/unraid-api/cli/internal-client.service.ts
api/src/unraid-api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tsapi/src/unraid-api/cli/internal-client.service.ts
api/**/*.{test,spec}.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,ts,tsx}: Use Vitest as the test suite; do not use Jest
Prefer not to mock simple dependencies in tests
Files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts
api/**/*.{test,spec}.{ts,js}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Test suite is VITEST, do not use jest in the API package
Files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts
🧠 Learnings (45)
📓 Common learnings
Learnt from: pujitm
PR: unraid/api#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.
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:140-145
Timestamp: 2024-12-13T17:26:55.220Z
Learning: In Apollo Client, the `cache.updateQuery` method can be used on the `InMemoryCache` instance to update queries, even when not using an `ApolloClient` instance directly.
Learnt from: pujitm
PR: unraid/api#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.
Learnt from: pujitm
PR: unraid/api#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 ConfigService implementation that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use factory functions for module mocks to avoid hoisting issues
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Mock external dependencies appropriately in store tests
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*store*.{test,spec}.{ts,js} : Use factory functions for module mocks to avoid hoisting issues in store tests
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test getter dependencies are properly mocked in store tests
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify state changes by updating the store in store tests
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Mock external dependencies and services in tests
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify state changes after actions in store tests
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
PR: unraid/api#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/shared/internal-graphql-client.factory.spec.tsapi/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Clear mocks between tests to ensure isolation
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/mocks/**/*.ts : Frequently used mocks are stored under `web/test/mocks`
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js} : Mock external dependencies and services in web/__test__
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use `vi.mock()` for module-level mocks
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Remember that `vi.mock()` calls are hoisted
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-04-23T20:19:42.542Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/socket-config.service.tsapi/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2025-04-23T20:19:42.542Z
Learnt from: pujitm
PR: unraid/api#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 ConfigService implementation that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
Applied to files:
packages/unraid-shared/src/services/socket-config.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js} : Test component behavior and output, not implementation details in web/__test__
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify proper error handling in store tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test async operations completely in store tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/**/*.{test,spec}.{ts,js} : Test suite is VITEST, do not use jest in the API package
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Specify return values for component methods with `vi.spyOn()`
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Use Vitest as the test suite; do not use Jest
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test action side effects and state changes in store tests
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Reset mocks between tests with `vi.clearAllMocks()`
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.spec.ts
📚 Learning: 2024-11-15T16:22:03.485Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:176-188
Timestamp: 2024-11-15T16:22:03.485Z
Learning: Atomic writes are not required for the `saveApiKey` method in `ApiKeyService` (`api/src/unraid-api/auth/api-key.service.ts`) unless specifically needed.
Applied to files:
api/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
api/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
api/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-04T20:44:46.432Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Applied to files:
api/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-04T22:00:41.946Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:42-105
Timestamp: 2024-11-04T22:00:41.946Z
Learning: In the 'AuthService' within `api/src/unraid-api/auth/auth.service.ts`, rate limiting is not required because the API is not public facing.
Applied to files:
api/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-11-15T16:22:12.460Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.module.ts:94-96
Timestamp: 2024-11-15T16:22:12.460Z
Learning: In `CookieService.defaultOpts()` in `api/src/unraid-api/auth/cookie.service.ts`, use `sameSite: 'lax'` for session cookie configuration.
Applied to files:
api/src/unraid-api/cli/internal-client.service.ts
📚 Learning: 2024-12-13T17:28:15.384Z
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In `web/helpers/apollo-cache/index.ts`, it's acceptable to use `structuredClone` since the project targets Node.js v20 and browsers where `structuredClone` is supported.
Applied to files:
api/src/unraid-api/cli/internal-client.service.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-27T13:34:53.438Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-10T18:43:13.016Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/types/fastify.ts:25-30
Timestamp: 2025-03-10T18:43:13.016Z
Learning: In the Unraid API, cookies should be marked as optional (with `?`) in the FastifyRequest type definition since the cookies may not always be present, which is consistent with the authentication flow throughout the codebase.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-15T21:34:00.006Z
Learnt from: pujitm
PR: unraid/api#1047
File: api/src/unraid-api/graph/sandbox-plugin.ts:57-57
Timestamp: 2025-01-15T21:34:00.006Z
Learning: In the GraphQL sandbox (api/src/unraid-api/graph/sandbox-plugin.ts), CSRF token validation should fail silently with a fallback value to maintain sandbox accessibility, as it's a development tool where strict security measures aren't required.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-04-07T14:34:47.255Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-30T19:56:42.055Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-27T15:30:02.252Z
Learnt from: elibosley
PR: unraid/api#969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the `setup` commands in `web/justfile` and `api/justfile` are intentional and correct behavior.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-28T16:39:50.278Z
Learnt from: elibosley
PR: unraid/api#1078
File: api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts:35-36
Timestamp: 2025-01-28T16:39:50.278Z
Learning: When reviewing TypeScript files containing PHP code snippets intended for file modifications (like in `SSOFileModification`), PHP functions that appear undefined should not be flagged as issues since they will be available in the target PHP environment where the code is injected.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under `web/` must follow this naming convention
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-11T20:39:02.511Z
Learnt from: pujitm
PR: unraid/api#978
File: web/helpers/apollo-cache/index.ts:134-139
Timestamp: 2024-12-11T20:39:02.511Z
Learning: In `web/helpers/apollo-cache/index.ts`, the `cache.readQuery` method returns `null` when it fails to read from the cache, rather than throwing an exception.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-11T17:20:09.604Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations)
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
🧬 Code Graph Analysis (1)
api/src/unraid-api/cli/internal-client.service.ts (3)
api/src/unraid-api/shared/internal-graphql-client.factory.ts (1)
Injectable(23-152)packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts (1)
Injectable(22-57)packages/unraid-shared/src/tokens.ts (1)
INTERNAL_CLIENT_SERVICE_TOKEN(5-5)
🪛 GitHub Actions: CI - Main (API)
packages/unraid-shared/src/services/base-internal-client.service.spec.ts
[error] 111-111: TestInternalClientService failed to get API key due to 'API key error'.
packages/unraid-shared/src/services/base-internal-client.service.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/unraid-shared/src/services/base-internal-client.service.ts (2)
45-48: Defaultoriginis not a valid HTTP origin
Using a filesystem path (/var/run/unraid-cli.sock) as the Origin header is semantically wrong and may be rejected by strict CORS checks. Default to a proper scheme/host string (e.g.http://localhost) or omit the header for internal calls.
175-178:ApolloClient.stop()is not part of the public API
Apollo 3 exposesclearStore()/resetStore(), but nostop(). Calling it will throw at runtime.- this.client?.stop(); + if (this.client) { + // End polling links if present + (this.client as any).stop?.(); + await this.client.clearStore().catch(() => undefined); + }
🧹 Nitpick comments (4)
packages/unraid-shared/src/services/base-internal-client.service.ts (1)
109-113: Consider logging GraphQL errors as well
OnlynetworkErroris handled;graphQLErrorsare silently ignored, making debugging harder.packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts (3)
3-3: Import extension may break TS-time resolution
./internal.client.jsrefers to a file that actually lives asinternal.client.tsin sources.
Vitest’s ts-node resolver usually maps this automatically, but the extra “.js” can bite in IDE tooling and when the compiled JS ends up in a different directory structure. Safer to drop the explicit extension:-import { InternalClientService } from './internal.client.js'; +import { InternalClientService } from './internal.client';
6-9: Avoidanyin test doublesUsing
anyforclientFactoryandapiKeyServiceloses type-safety and can hide breaking refactors.
Define minimal interfaces or usevi.mocked<Interface>()to keep the compiler on your side.interface MockClientFactory { createClient: ReturnType<typeof vi.fn>; }
31-33: Prefervi.resetAllMocks()for full isolation
vi.clearAllMocks()resets call counts but preserves stubbed implementations modified in a test.
If a test mutatesmockImplementation, the next test could inherit it. UsingafterEach(() => vi.resetAllMocks());restores originals and avoids inter-test leakage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
api/src/unraid-api/shared/internal-graphql-client.factory.ts(1 hunks)packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts(1 hunks)packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts(1 hunks)packages/unraid-shared/src/services/base-internal-client.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
- api/src/unraid-api/shared/internal-graphql-client.factory.ts
🧰 Additional context used
🧠 Learnings (42)
📓 Common learnings
Learnt from: pujitm
PR: unraid/api#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.
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:140-145
Timestamp: 2024-12-13T17:26:55.220Z
Learning: In Apollo Client, the `cache.updateQuery` method can be used on the `InMemoryCache` instance to update queries, even when not using an `ApolloClient` instance directly.
Learnt from: pujitm
PR: unraid/api#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.
Learnt from: pujitm
PR: unraid/api#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 ConfigService implementation that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/**/*.{test,spec}.{ts,js} : Test suite is VITEST, do not use jest in the API package
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Use Vitest as the test suite; do not use Jest
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Mock external dependencies appropriately in store tests
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Clear mocks between tests to ensure isolation
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify state changes by updating the store in store tests
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Mock external dependencies and services in tests
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test async operations completely in store tests
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify proper error handling in store tests
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify state changes after actions in store tests
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js} : Mock external dependencies and services in web/__test__
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Reset mocks between tests with `vi.clearAllMocks()`
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Remember that `vi.mock()` calls are hoisted
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use `vi.mock()` for module-level mocks
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Specify return values for component methods with `vi.spyOn()`
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-15T16:22:03.485Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:176-188
Timestamp: 2024-11-15T16:22:03.485Z
Learning: Atomic writes are not required for the `saveApiKey` method in `ApiKeyService` (`api/src/unraid-api/auth/api-key.service.ts`) unless specifically needed.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-04T20:44:46.432Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-27T13:34:53.438Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-10T18:43:13.016Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/types/fastify.ts:25-30
Timestamp: 2025-03-10T18:43:13.016Z
Learning: In the Unraid API, cookies should be marked as optional (with `?`) in the FastifyRequest type definition since the cookies may not always be present, which is consistent with the authentication flow throughout the codebase.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-15T21:34:00.006Z
Learnt from: pujitm
PR: unraid/api#1047
File: api/src/unraid-api/graph/sandbox-plugin.ts:57-57
Timestamp: 2025-01-15T21:34:00.006Z
Learning: In the GraphQL sandbox (api/src/unraid-api/graph/sandbox-plugin.ts), CSRF token validation should fail silently with a fallback value to maintain sandbox accessibility, as it's a development tool where strict security measures aren't required.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-04-07T14:34:47.255Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-30T19:56:42.055Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-27T15:30:02.252Z
Learnt from: elibosley
PR: unraid/api#969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the `setup` commands in `web/justfile` and `api/justfile` are intentional and correct behavior.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-28T16:39:50.278Z
Learnt from: elibosley
PR: unraid/api#1078
File: api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts:35-36
Timestamp: 2025-01-28T16:39:50.278Z
Learning: When reviewing TypeScript files containing PHP code snippets intended for file modifications (like in `SSOFileModification`), PHP functions that appear undefined should not be flagged as issues since they will be available in the target PHP environment where the code is injected.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under `web/` must follow this naming convention
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-13T17:28:15.384Z
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In `web/helpers/apollo-cache/index.ts`, it's acceptable to use `structuredClone` since the project targets Node.js v20 and browsers where `structuredClone` is supported.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-11T20:39:02.511Z
Learnt from: pujitm
PR: unraid/api#978
File: web/helpers/apollo-cache/index.ts:134-139
Timestamp: 2024-12-11T20:39:02.511Z
Learning: In `web/helpers/apollo-cache/index.ts`, the `cache.readQuery` method returns `null` when it fails to read from the cache, rather than throwing an exception.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-11T17:20:09.604Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations)
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-07T17:36:52.790Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:57-63
Timestamp: 2025-03-07T17:36:52.790Z
Learning: The CORS configuration in the unraid API is intentionally set to allow all origins (`origin: true`). This was a deliberate architectural decision when removing the previous custom CORS implementation and implementing helmet security headers instead.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-15T16:22:12.460Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.module.ts:94-96
Timestamp: 2024-11-15T16:22:12.460Z
Learning: In `CookieService.defaultOpts()` in `api/src/unraid-api/auth/cookie.service.ts`, use `sameSite: 'lax'` for session cookie configuration.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-05-08T19:28:37.034Z
Learnt from: elibosley
PR: unraid/api#1381
File: plugin/builder/cli/common-environment.ts:18-22
Timestamp: 2025-05-08T19:28:37.034Z
Learning: In plugin/builder/cli/common-environment.ts, the getDefaultBaseUrl() function intentionally returns an invalid URL when running in CI environments to force explicit specification of the base URL parameter rather than relying on defaults.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
PR: unraid/api#1151
File: plugin/builder/utils/consts.ts:6-6
Timestamp: 2025-03-04T14:55:00.903Z
Learning: The startingDir constant in plugin/builder/utils/consts.ts is defined using process.cwd(), which can cause issues if directory changes occur after importing this constant. Using __dirname.split('/builder')[0] would make it more reliable by making it relative to the file location rather than the current working directory.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-07T17:35:50.406Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:32-55
Timestamp: 2025-03-07T17:35:50.406Z
Learning: Helmet security configuration in the Unraid API is intentionally relaxed (with disabled CSP, CORS policies, and HSTS) to maintain compatibility with existing Unraid plugins. Stricter security settings might break current plugin functionality.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
🪛 GitHub Actions: CI - Main (API)
packages/unraid-shared/src/services/base-internal-client.service.ts
[error] 12-12: TypeScript error TS2307: Cannot find module 'ws' or its corresponding type declarations.
packages/unraid-shared/src/services/base-internal-client.service.ts
Outdated
Show resolved
Hide resolved
3 tasks
3 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/unraid-shared/src/services/base-internal-client.service.ts (1)
181-186:ApolloClient.stop()is still not a public API – will throw at runtimeApollo 3.x exposes
clearStore()/resetStore(), but nostop()method.
Ifstopisundefinedthis line throws, breaking every consumer attempting to clear the client.- // Stop the Apollo client to terminate any active processes - this.client?.stop(); + // Best-effort teardown – clear polling / terminate links if available + if (this.client) { + (this.client as any).stop?.(); // optional: community patches + await this.client.clearStore().catch(() => undefined); + }
🧹 Nitpick comments (2)
packages/unraid-shared/src/services/base-internal-client.service.ts (2)
45-47: Prefer DI over manual instantiation forSocketConfigServiceConstructing the service with
newbypasses Nest’s lifecycle hooks and makes testing harder.
Inject it instead:constructor( private readonly socketConfig: SocketConfigService, protected readonly configService: ConfigService, … ) {}
183-186:this.wsClient.dispose()returns a Promise – consider awaiting
dispose()is async; ignoring the promise risks unresolved handles during shutdown (notably in Jest).- if (this.wsClient) { - this.wsClient.dispose(); + if (this.wsClient) { + await this.wsClient.dispose(); this.wsClient = null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts(1 hunks)api/src/unraid-api/shared/internal-graphql-client.factory.ts(1 hunks)packages/unraid-shared/package.json(3 hunks)packages/unraid-shared/src/jsonforms/__tests__/settings.test.ts(1 hunks)packages/unraid-shared/src/services/__tests__/config-file.test.ts(1 hunks)packages/unraid-shared/src/services/base-internal-client.service.spec.ts(1 hunks)packages/unraid-shared/src/services/base-internal-client.service.ts(1 hunks)packages/unraid-shared/src/services/socket-config.service.spec.ts(1 hunks)packages/unraid-shared/src/services/socket-config.service.ts(1 hunks)packages/unraid-shared/src/util/__tests__/config-definition.test.ts(1 hunks)packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts(1 hunks)packages/unraid-shared/src/util/__tests__/key-order.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- packages/unraid-shared/src/util/tests/key-order.test.ts
- packages/unraid-shared/src/services/tests/config-file.test.ts
- packages/unraid-shared/src/util/tests/config-file-handler.test.ts
- packages/unraid-shared/src/jsonforms/tests/settings.test.ts
- packages/unraid-shared/src/util/tests/config-definition.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/unraid-shared/package.json
- api/src/unraid-api/shared/internal-graphql-client.factory.ts
- packages/unraid-shared/src/services/base-internal-client.service.spec.ts
- packages/unraid-shared/src/services/socket-config.service.ts
- packages/unraid-shared/src/services/socket-config.service.spec.ts
- api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts
🧰 Additional context used
🧠 Learnings (29)
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-15T16:22:03.485Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:176-188
Timestamp: 2024-11-15T16:22:03.485Z
Learning: Atomic writes are not required for the `saveApiKey` method in `ApiKeyService` (`api/src/unraid-api/auth/api-key.service.ts`) unless specifically needed.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-04T20:44:46.432Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-27T13:34:53.438Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-10T18:43:13.016Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/types/fastify.ts:25-30
Timestamp: 2025-03-10T18:43:13.016Z
Learning: In the Unraid API, cookies should be marked as optional (with `?`) in the FastifyRequest type definition since the cookies may not always be present, which is consistent with the authentication flow throughout the codebase.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-15T21:34:00.006Z
Learnt from: pujitm
PR: unraid/api#1047
File: api/src/unraid-api/graph/sandbox-plugin.ts:57-57
Timestamp: 2025-01-15T21:34:00.006Z
Learning: In the GraphQL sandbox (api/src/unraid-api/graph/sandbox-plugin.ts), CSRF token validation should fail silently with a fallback value to maintain sandbox accessibility, as it's a development tool where strict security measures aren't required.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-04-07T14:34:47.255Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-30T19:56:42.055Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-27T15:30:02.252Z
Learnt from: elibosley
PR: unraid/api#969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the `setup` commands in `web/justfile` and `api/justfile` are intentional and correct behavior.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under `web/` must follow this naming convention
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-28T16:39:50.278Z
Learnt from: elibosley
PR: unraid/api#1078
File: api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts:35-36
Timestamp: 2025-01-28T16:39:50.278Z
Learning: When reviewing TypeScript files containing PHP code snippets intended for file modifications (like in `SSOFileModification`), PHP functions that appear undefined should not be flagged as issues since they will be available in the target PHP environment where the code is injected.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-13T17:28:15.384Z
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In `web/helpers/apollo-cache/index.ts`, it's acceptable to use `structuredClone` since the project targets Node.js v20 and browsers where `structuredClone` is supported.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-11T20:39:02.511Z
Learnt from: pujitm
PR: unraid/api#978
File: web/helpers/apollo-cache/index.ts:134-139
Timestamp: 2024-12-11T20:39:02.511Z
Learning: In `web/helpers/apollo-cache/index.ts`, the `cache.readQuery` method returns `null` when it fails to read from the cache, rather than throwing an exception.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-11T17:20:09.604Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations)
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Mock external dependencies appropriately in store tests
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-07T17:36:52.790Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:57-63
Timestamp: 2025-03-07T17:36:52.790Z
Learning: The CORS configuration in the unraid API is intentionally set to allow all origins (`origin: true`). This was a deliberate architectural decision when removing the previous custom CORS implementation and implementing helmet security headers instead.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-15T16:22:12.460Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.module.ts:94-96
Timestamp: 2024-11-15T16:22:12.460Z
Learning: In `CookieService.defaultOpts()` in `api/src/unraid-api/auth/cookie.service.ts`, use `sameSite: 'lax'` for session cookie configuration.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-05-08T19:28:37.034Z
Learnt from: elibosley
PR: unraid/api#1381
File: plugin/builder/cli/common-environment.ts:18-22
Timestamp: 2025-05-08T19:28:37.034Z
Learning: In plugin/builder/cli/common-environment.ts, the getDefaultBaseUrl() function intentionally returns an invalid URL when running in CI environments to force explicit specification of the base URL parameter rather than relying on defaults.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
PR: unraid/api#1151
File: plugin/builder/utils/consts.ts:6-6
Timestamp: 2025-03-04T14:55:00.903Z
Learning: The startingDir constant in plugin/builder/utils/consts.ts is defined using process.cwd(), which can cause issues if directory changes occur after importing this constant. Using __dirname.split('/builder')[0] would make it more reliable by making it relative to the file location rather than the current working directory.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-07T17:35:50.406Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:32-55
Timestamp: 2025-03-07T17:35:50.406Z
Learning: Helmet security configuration in the Unraid API is intentionally relaxed (with disabled CSP, CORS policies, and HSTS) to maintain compatibility with existing Unraid plugins. Stricter security settings might break current plugin functionality.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
🔇 Additional comments (1)
packages/unraid-shared/src/services/base-internal-client.service.ts (1)
11-13: ws dependency is already declared inpackages/unraid-shared
packages/unraid-shared/package.jsonincludes:
- ws: ^8.18.3
- @types/ws: ^8.5.13
No other package compiles this file, so CI will not fail for missing
ws.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/unraid-shared/src/services/base-internal-client.service.ts (1)
181-181:ApolloClient.stop()is undefined in v3This call still risks a runtime crash; earlier review already covered it.
🧹 Nitpick comments (5)
packages/unraid-shared/src/services/socket-config.service.spec.ts (3)
1-1: Restore spies between tests to avoid leakageAdd afterEach to restore mocks; this prevents any accidental spy bleed between tests and aligns with Vitest best practices.
-import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; beforeEach(() => { configService = new ConfigService(); service = new SocketConfigService(configService); }); + +afterEach(() => { + vi.restoreAllMocks(); +});Also applies to: 10-13
71-75: Remove duplicate test for string numeric port“should handle string ports correctly” duplicates “should return numeric port when configured.” Consider deleting the duplicate or consolidating via table-driven tests.
- it('should handle string ports correctly', () => { - vi.spyOn(configService, 'get').mockReturnValue('8080'); - - expect(service.getNumericPort()).toBe(8080); - });Optionally, consolidate with it.each for multiple valid inputs.
Also applies to: 83-88
161-169: Parity check: omit port for ws when nginx port is 80?You test omission for http@80; consider verifying the same behavior for ws if that’s the intended contract.
If desired, add:
+ it('should omit port for WS when nginx port is default (80)', () => { + vi.spyOn(configService, 'get').mockImplementation((key, defaultValue) => { + if (key === 'PORT') return '/var/run/unraid-api.sock'; + if (key === 'store.emhttp.nginx.httpPort') return '80'; + return defaultValue; + }); + // Not on socket, no numeric PORT → fallback to nginx port 80 + vi.spyOn(configService, 'get').mockImplementation((key, defaultValue) => { + if (key === 'PORT') return 'invalid-port'; + if (key === 'store.emhttp.nginx.httpPort') return '80'; + return defaultValue; + }); + expect(service.getApiAddress('ws')).toBe('ws://127.0.0.1/graphql'); + });Confirm whether ws should mirror http’s default-port omission.
packages/unraid-shared/src/services/base-internal-client.service.ts (2)
86-92: Replace broadanycasts with a typed overload or// @ts-expect-errorCasting to
unknown as typeof fetch(and usinganyparams) hides type issues and conflicts with the project’s rule favouringts-expect-errorfor deliberate escapes (see learning in PR #1219).
160-167: Align defaultOptions for mutations in HTTP-only clientThe WebSocket-enabled branch sets
mutate.fetchPolicy = 'no-cache', but the HTTP-only branch omits it, leading to inconsistent behaviour.- defaultOptions: { - query: { fetchPolicy: 'no-cache' }, - }, + defaultOptions: { + query: { fetchPolicy: 'no-cache' }, + mutate: { fetchPolicy: 'no-cache' }, + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts(1 hunks)api/src/unraid-api/shared/internal-graphql-client.factory.ts(1 hunks)packages/unraid-shared/package.json(3 hunks)packages/unraid-shared/src/jsonforms/__tests__/settings.test.ts(1 hunks)packages/unraid-shared/src/services/__tests__/config-file.test.ts(1 hunks)packages/unraid-shared/src/services/base-internal-client.service.spec.ts(1 hunks)packages/unraid-shared/src/services/base-internal-client.service.ts(1 hunks)packages/unraid-shared/src/services/socket-config.service.spec.ts(1 hunks)packages/unraid-shared/src/services/socket-config.service.ts(1 hunks)packages/unraid-shared/src/util/__tests__/config-definition.test.ts(1 hunks)packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts(1 hunks)packages/unraid-shared/src/util/__tests__/key-order.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- packages/unraid-shared/src/services/tests/config-file.test.ts
- packages/unraid-shared/src/util/tests/config-definition.test.ts
- packages/unraid-shared/src/jsonforms/tests/settings.test.ts
- packages/unraid-shared/src/util/tests/key-order.test.ts
- packages/unraid-shared/src/util/tests/config-file-handler.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/unraid-shared/package.json
- api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts
- packages/unraid-shared/src/services/socket-config.service.ts
- api/src/unraid-api/shared/internal-graphql-client.factory.ts
- packages/unraid-shared/src/services/base-internal-client.service.spec.ts
🧰 Additional context used
🧠 Learnings (45)
📓 Common learnings
Learnt from: pujitm
PR: unraid/api#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.
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:140-145
Timestamp: 2024-12-13T17:26:55.220Z
Learning: In Apollo Client, the `cache.updateQuery` method can be used on the `InMemoryCache` instance to update queries, even when not using an `ApolloClient` instance directly.
Learnt from: pujitm
PR: unraid/api#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.
Learnt from: pujitm
PR: unraid/api#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 ConfigService implementation that includes a `set()` method for runtime configuration mutation, unlike the standard nestjs/config package which only provides getter methods.
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js} : Mock external dependencies and services in web/__test__
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Mock external dependencies appropriately in store tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Mock external dependencies and services in tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify state changes by updating the store in store tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test async operations completely in store tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify proper error handling in store tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test getter dependencies are properly mocked in store tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js} : Test component behavior and output, not implementation details in web/__test__
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify state changes after actions in store tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*store*.{test,spec}.{ts,js} : Use factory functions for module mocks to avoid hoisting issues in store tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/**/*.{test,spec}.{ts,js} : Test suite is VITEST, do not use jest in the API package
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Specify return values for component methods with `vi.spyOn()`
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Remember that `vi.mock()` calls are hoisted
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/mocks/**/*.ts : Frequently used mocks are stored under `web/test/mocks`
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Use Vitest as the test suite; do not use Jest
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-07-21T14:01:04.139Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use `vi.mock()` for module-level mocks
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-15T16:22:03.485Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:176-188
Timestamp: 2024-11-15T16:22:03.485Z
Learning: Atomic writes are not required for the `saveApiKey` method in `ApiKeyService` (`api/src/unraid-api/auth/api-key.service.ts`) unless specifically needed.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-04T20:44:46.432Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-27T13:34:53.438Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-10T18:43:13.016Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/types/fastify.ts:25-30
Timestamp: 2025-03-10T18:43:13.016Z
Learning: In the Unraid API, cookies should be marked as optional (with `?`) in the FastifyRequest type definition since the cookies may not always be present, which is consistent with the authentication flow throughout the codebase.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-15T21:34:00.006Z
Learnt from: pujitm
PR: unraid/api#1047
File: api/src/unraid-api/graph/sandbox-plugin.ts:57-57
Timestamp: 2025-01-15T21:34:00.006Z
Learning: In the GraphQL sandbox (api/src/unraid-api/graph/sandbox-plugin.ts), CSRF token validation should fail silently with a fallback value to maintain sandbox accessibility, as it's a development tool where strict security measures aren't required.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-04-07T14:34:47.255Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-30T19:56:42.055Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-27T15:30:02.252Z
Learnt from: elibosley
PR: unraid/api#969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the `setup` commands in `web/justfile` and `api/justfile` are intentional and correct behavior.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-01-28T16:39:50.278Z
Learnt from: elibosley
PR: unraid/api#1078
File: api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts:35-36
Timestamp: 2025-01-28T16:39:50.278Z
Learning: When reviewing TypeScript files containing PHP code snippets intended for file modifications (like in `SSOFileModification`), PHP functions that appear undefined should not be flagged as issues since they will be available in the target PHP environment where the code is injected.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under `web/` must follow this naming convention
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-13T17:28:15.384Z
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In `web/helpers/apollo-cache/index.ts`, it's acceptable to use `structuredClone` since the project targets Node.js v20 and browsers where `structuredClone` is supported.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-11T20:39:02.511Z
Learnt from: pujitm
PR: unraid/api#978
File: web/helpers/apollo-cache/index.ts:134-139
Timestamp: 2024-12-11T20:39:02.511Z
Learning: In `web/helpers/apollo-cache/index.ts`, the `cache.readQuery` method returns `null` when it fails to read from the cache, rather than throwing an exception.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-12-11T17:20:09.604Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations)
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-07T17:36:52.790Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:57-63
Timestamp: 2025-03-07T17:36:52.790Z
Learning: The CORS configuration in the unraid API is intentionally set to allow all origins (`origin: true`). This was a deliberate architectural decision when removing the previous custom CORS implementation and implementing helmet security headers instead.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2024-11-15T16:22:12.460Z
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.module.ts:94-96
Timestamp: 2024-11-15T16:22:12.460Z
Learning: In `CookieService.defaultOpts()` in `api/src/unraid-api/auth/cookie.service.ts`, use `sameSite: 'lax'` for session cookie configuration.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-28T15:34:24.715Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-07-21T14:00:06.258Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-05-08T19:28:37.034Z
Learnt from: elibosley
PR: unraid/api#1381
File: plugin/builder/cli/common-environment.ts:18-22
Timestamp: 2025-05-08T19:28:37.034Z
Learning: In plugin/builder/cli/common-environment.ts, the getDefaultBaseUrl() function intentionally returns an invalid URL when running in CI environments to force explicit specification of the base URL parameter rather than relying on defaults.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
PR: unraid/api#1151
File: plugin/builder/utils/consts.ts:6-6
Timestamp: 2025-03-04T14:55:00.903Z
Learning: The startingDir constant in plugin/builder/utils/consts.ts is defined using process.cwd(), which can cause issues if directory changes occur after importing this constant. Using __dirname.split('/builder')[0] would make it more reliable by making it relative to the file location rather than the current working directory.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-03-07T17:35:50.406Z
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:32-55
Timestamp: 2025-03-07T17:35:50.406Z
Learning: Helmet security configuration in the Unraid API is intentionally relaxed (with disabled CSP, CORS policies, and HSTS) to maintain compatibility with existing Unraid plugins. Stricter security settings might break current plugin functionality.
Applied to files:
packages/unraid-shared/src/services/base-internal-client.service.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/base-internal-client.service.ts
🔇 Additional comments (1)
packages/unraid-shared/src/services/socket-config.service.spec.ts (1)
1-280: Overall: Solid coverage and clear scenariosGood use of Vitest, thorough edge cases, and integration-like scenarios. After addressing the minor fixes above, this suite will be very robust.
packages/unraid-shared/src/services/base-internal-client.service.ts
Outdated
Show resolved
Hide resolved
packages/unraid-shared/src/services/base-internal-client.service.ts
Outdated
Show resolved
Hide resolved
packages/unraid-shared/src/services/socket-config.service.spec.ts
Outdated
Show resolved
Hide resolved
elibosley
force-pushed
the
fix/local-fetching-using-sockets
branch
from
9a277b9 to
44e5b92
Compare
- Updated BaseInternalClientService to accept SocketConfigService as a constructor parameter, enhancing socket configuration management. - Modified unit tests to create and inject SocketConfigService instances, ensuring proper initialization and functionality. - Adjusted test cases to validate the use of injected SocketConfigService, improving test coverage and reliability.
- Introduced a new test suite for WebSocket connections over Unix sockets, validating connection establishment, message exchange, and error handling. - Implemented tests for multiple concurrent connections and ensured compatibility with the expected URI formats. - Enhanced cleanup procedures to manage socket files and connections effectively after tests.
- Added afterEach hook to restore all mocks and spies after each test in SocketConfigService test suite. - Improved test reliability by ensuring a clean state for each test execution.
- Enhanced the afterAll and individual test cleanup processes to ensure all WebSocket clients are closed gracefully. - Added error handling for socket file cleanup to prevent potential issues during test execution. - Updated test cases to reflect the new cleanup logic, improving test reliability and resource management.
- Improved cleanup logic in the afterAll hook to ensure all WebSocket clients are closed properly using promises. - Updated connection tests to use `once` for event listeners, preventing potential memory leaks. - Enhanced error handling in tests for non-existent Unix sockets, ensuring proper rejection of invalid connections. - Streamlined test assertions for better clarity and reliability.
…pm-lock.yaml - Updated the version of @apollo/client from 3.13.8 to 3.13.9 in both package.json and pnpm-lock.yaml to ensure compatibility with the latest features and fixes. - Removed outdated entries related to the previous version from pnpm-lock.yaml for cleaner dependency management.
- Included `graphql-ws` version 6.0.6 in `package.json` to support WebSocket-based GraphQL subscriptions and enhance real-time capabilities.
…GraphQL client structure - Deleted the BaseInternalClientService and its associated test file to streamline the codebase. - Updated the SocketConfigService documentation to clarify its usage across different services. - Refactored the GraphQL client structure, introducing a new ClientConsumerService that utilizes a factory pattern for better client management and testing. - Added comprehensive tests for the new ClientConsumerService, ensuring proper API key handling, client lifecycle management, and configuration scenarios.
- Refactored the CliInternalClientService to create clients using a lazy-loaded getApiKey function instead of an immediate API key. - Updated the InternalGraphQLClientFactory to require a getApiKey function for client creation, enhancing flexibility and security. - Adjusted related tests to verify the new client creation logic and error handling for API key retrieval. - Improved SocketConfigService to validate the nginx port configuration, ensuring robustness in service setup.
- Modified the exports in the form component index file to include explicit file extensions for better clarity and compatibility. - Ensured all component exports now point to their respective index.js files.
…y retrieval - Enhanced the test for InternalClientService to confirm that the API key is fetched lazily through the new getApiKey function. - Added assertions to ensure the getApiKey function works correctly and that the API key service is called as expected. - Improved clarity and reliability of the test suite by validating the client creation process with the updated API key handling.
…aphQLClientFactory interface - Updated import statements across multiple files to simplify the import paths for the INTERNAL_CLIENT_SERVICE_TOKEN and InternalGraphQLClientFactory. - Introduced a new InternalGraphQLClientFactory interface in the shared package to standardize client creation across services. - Removed redundant interface definitions from internal client files, enhancing code clarity and maintainability.
pujitm
force-pushed
the
fix/local-fetching-using-sockets
branch
from
5c83151 to
ee4394e
Compare
Contributor
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/unraid-shared/src/services/socket-config.service.spec.ts (1)
52-65: “PORT not set” scenario now correctly exercises the defaultThis fixes the earlier issue by returning the provided default only for PORT. Thanks for addressing it.
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts (1)
162-167: Avoid asserting exact error messages; use rejects.toThrow() without argumentsPer the testing guidelines, don’t assert exact error strings to keep tests resilient.
Apply this diff:
- await expect(service.getClient()).rejects.toThrow('API key error'); + await expect(service.getClient()).rejects.toThrow();
🧹 Nitpick comments (11)
packages/unraid-shared/src/services/socket-config.service.spec.ts (5)
15-18: Remove obvious comment; keep the cleanupThe comment explains an obvious practice; trim it per guidelines to reduce noise.
afterEach(() => { - // Clean up all spies and mocks after each test vi.restoreAllMocks(); });
21-28: Tighten the spy to the expected key for robustnessCurrently the spy returns '8080' for any key, which can mask accidental lookups of the wrong key. Restrict it to the nginx key and fall back otherwise.
- vi.spyOn(configService, 'get').mockReturnValue('8080'); + vi.spyOn(configService, 'get').mockImplementation((key, defaultValue) => + key === 'store.emhttp.nginx.httpPort' ? '8080' : defaultValue + );
83-143: Reduce repetition in getNumericPort tests with parameterized casesMultiple cases differ only by the input value. Parameterize to make intent clearer and cut duplication.
it('should handle string ports correctly', () => { - vi.spyOn(configService, 'get').mockReturnValue('8080'); - - expect(service.getNumericPort()).toBe(8080); - }); - - it('should return undefined for non-numeric port values', () => { - vi.spyOn(configService, 'get').mockReturnValue('invalid-port'); - - expect(service.getNumericPort()).toBeUndefined(); - }); - - it('should return undefined for empty string port', () => { - vi.spyOn(configService, 'get').mockReturnValue(''); - - expect(service.getNumericPort()).toBeUndefined(); - }); - - it('should return undefined for port with mixed characters', () => { - vi.spyOn(configService, 'get').mockReturnValue('3000abc'); - - expect(service.getNumericPort()).toBeUndefined(); - }); - - it('should return undefined for port 0', () => { - vi.spyOn(configService, 'get').mockReturnValue('0'); - - expect(service.getNumericPort()).toBeUndefined(); - }); - - it('should return undefined for negative port', () => { - vi.spyOn(configService, 'get').mockReturnValue('-1'); - - expect(service.getNumericPort()).toBeUndefined(); - }); - - it('should return undefined for port above 65535', () => { - vi.spyOn(configService, 'get').mockReturnValue('70000'); - - expect(service.getNumericPort()).toBeUndefined(); - }); - - it('should return valid port 65535', () => { - vi.spyOn(configService, 'get').mockReturnValue('65535'); - - expect(service.getNumericPort()).toBe(65535); - }); + vi.spyOn(configService, 'get').mockReturnValue('8080'); + expect(service.getNumericPort()).toBe(8080); + }); + + it.each([ + 'invalid-port', + '', + '3000abc', + '0', + '-1', + '70000', + ])('should return undefined for invalid port "%s"', (value) => { + vi.spyOn(configService, 'get').mockReturnValue(value); + expect(service.getNumericPort()).toBeUndefined(); + }); + + it.each([ + ['8080', 8080], + ['65535', 65535], + ])('should parse valid string ports: %s -> %d', (value, expected) => { + vi.spyOn(configService, 'get').mockReturnValue(value); + expect(service.getNumericPort()).toBe(expected); + });
145-173: Add WS + nginx fallback case for getApiAddressYou cover HTTP + nginx fallback but not WS + nginx fallback. Adding it ensures parity across protocols.
it('should use nginx port when no numeric port configured', () => { vi.spyOn(configService, 'get').mockImplementation((key, defaultValue) => { if (key === 'PORT') return '/var/run/unraid-api.sock'; if (key === 'store.emhttp.nginx.httpPort') return '8080'; return defaultValue; }); expect(service.getApiAddress('http')).toBe('http://127.0.0.1:8080/graphql'); }); + + it('should return WS address via nginx port when no numeric port configured', () => { + vi.spyOn(configService, 'get').mockImplementation((key, defaultValue) => { + if (key === 'PORT') return '/var/run/unraid-api.sock'; + if (key === 'store.emhttp.nginx.httpPort') return '8080'; + return defaultValue; + }); + expect(service.getApiAddress('ws')).toBe('ws://127.0.0.1:8080/graphql'); + });
228-261: Remove duplicated WebSocket TCP test“should return ws:// URI when running on TCP port” and “should use TCP port for WebSocket when running on TCP port” assert the same observable behavior. Keep one to avoid redundancy.
- it('should use TCP port for WebSocket when running on TCP port', () => { - // Configure to use TCP port instead of Unix socket - // This naturally causes isRunningOnSocket() to return false - vi.spyOn(configService, 'get').mockImplementation((key, defaultValue) => { - if (key === 'PORT') return '3001'; // TCP port, not a socket - if (key === 'store.emhttp.nginx.httpPort') return '8080'; - return defaultValue; - }); - - const uri = service.getWebSocketUri(true); - - // When PORT is numeric, it uses that port directly for WebSocket - expect(uri).toBe('ws://127.0.0.1:3001/graphql'); - });packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts (2)
43-47: Avoid leaking a client when clearClient() is called mid-creationIf clearClient() is invoked while creation is in-flight, this branch won’t set the cached client (good), but the created client is still returned to the original caller and the service drops its reference. Consider proactively stopping the now-orphaned client to avoid resource leaks.
Apply this diff to safely dispose the orphaned client:
- if (this.clientCreationPromise === creationPromise) { - this.client = client; - } + if (this.clientCreationPromise === creationPromise) { + this.client = client; + } else { + // clearClient() occurred during creation; prevent leaking the orphaned client + client.stop?.(); + }
68-73: Clear in-flight creation on clearClient() is good; consider logging for observabilityClearing both the cached client and the in-flight promise is correct. Consider logging when an in-flight creation is cleared to aid debugging of races.
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts (3)
68-72: Don’t mutate constructor args; set defaults immutablyMutating the passed options object can surprise callers. Prefer creating a new options object with defaults.
Apply this diff:
- // Use default origin if not provided - if (!this.options.origin) { - this.options.origin = 'http://localhost'; - } + // Use default origin if not provided + this.options = { origin: 'http://localhost', ...this.options };
16-22: Remove unused undici mock to reduce noiseThis spec’s mock factory doesn’t use undici; the mock adds test noise without benefit.
Apply this diff:
-// Mock undici -vi.mock('undici', () => ({ - Agent: vi.fn(() => ({ - connect: { socketPath: '/test/socket.sock' }, - })), - fetch: vi.fn(() => Promise.resolve({ ok: true })), -}));
41-53: Prefer Partial/satisfies over any for mock client typingUse Partial<ApolloClient> with satisfies to avoid any.
Apply this diff:
- const mockClient = { + const mockClient = { query: vi.fn(), mutate: vi.fn(), stop: vi.fn(), subscribe: vi.fn(), watchQuery: vi.fn(), readQuery: vi.fn(), writeQuery: vi.fn(), cache: { reset: vi.fn(), }, - } as any; + } satisfies Partial<ApolloClient<any>> as unknown as ApolloClient<any>;api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts (1)
158-170: Test title vs setup mismatch; ensure wsUri is provided when asserting “even when wsUri is provided”The test name claims wsUri is provided but the mock returns undefined. Either rename the test or provide a wsUri while keeping enableSubscriptions: false.
Apply this diff to actually provide a wsUri:
- vi.mocked(socketConfig.getWebSocketUri).mockReturnValue(undefined); // Subscriptions disabled + vi.mocked(socketConfig.getWebSocketUri).mockReturnValue('ws://127.0.0.1:3001/graphql');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
api/dev/configs/api.json(1 hunks)api/package.json(1 hunks)api/src/unraid-api/cli/cli.module.spec.ts(1 hunks)api/src/unraid-api/cli/internal-client.service.spec.ts(1 hunks)api/src/unraid-api/cli/internal-client.service.ts(2 hunks)api/src/unraid-api/plugin/global-deps.module.ts(3 hunks)api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts(1 hunks)api/src/unraid-api/shared/internal-graphql-client.factory.ts(1 hunks)packages/unraid-api-plugin-connect/package.json(2 hunks)packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts(1 hunks)packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts(1 hunks)packages/unraid-api-plugin-connect/tsconfig.json(1 hunks)packages/unraid-api-plugin-generator/tsconfig.build.json(1 hunks)packages/unraid-api-plugin-generator/tsconfig.json(1 hunks)packages/unraid-api-plugin-health/tsconfig.json(1 hunks)packages/unraid-shared/package.json(3 hunks)packages/unraid-shared/src/index.ts(1 hunks)packages/unraid-shared/src/jsonforms/__tests__/settings.test.ts(1 hunks)packages/unraid-shared/src/services/__tests__/config-file.test.ts(1 hunks)packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts(1 hunks)packages/unraid-shared/src/services/socket-config.service.spec.ts(1 hunks)packages/unraid-shared/src/services/socket-config.service.ts(1 hunks)packages/unraid-shared/src/services/ws-unix-socket-test.spec.ts(1 hunks)packages/unraid-shared/src/tokens.ts(1 hunks)packages/unraid-shared/src/types/internal-graphql-client.factory.ts(1 hunks)packages/unraid-shared/src/util/__tests__/config-definition.test.ts(1 hunks)packages/unraid-shared/src/util/__tests__/config-file-handler.test.ts(1 hunks)packages/unraid-shared/src/util/__tests__/key-order.test.ts(1 hunks)unraid-ui/src/components/common/index.ts(1 hunks)unraid-ui/src/components/form/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/unraid-shared/src/util/tests/config-file-handler.test.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- packages/unraid-shared/src/tokens.ts
- packages/unraid-shared/src/util/tests/config-definition.test.ts
- api/package.json
- packages/unraid-shared/src/index.ts
- unraid-ui/src/components/common/index.ts
- packages/unraid-api-plugin-generator/tsconfig.json
- packages/unraid-api-plugin-generator/tsconfig.build.json
- packages/unraid-shared/src/jsonforms/tests/settings.test.ts
- api/dev/configs/api.json
- packages/unraid-shared/src/types/internal-graphql-client.factory.ts
- packages/unraid-api-plugin-health/tsconfig.json
- api/src/unraid-api/cli/cli.module.spec.ts
- packages/unraid-api-plugin-connect/package.json
- packages/unraid-shared/src/util/tests/key-order.test.ts
- packages/unraid-shared/src/services/socket-config.service.ts
- packages/unraid-shared/src/services/ws-unix-socket-test.spec.ts
- api/src/unraid-api/shared/internal-graphql-client.factory.ts
- packages/unraid-shared/src/services/tests/config-file.test.ts
- packages/unraid-api-plugin-connect/tsconfig.json
- packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
- packages/unraid-shared/package.json
- api/src/unraid-api/cli/internal-client.service.ts
- unraid-ui/src/components/form/index.ts
- api/src/unraid-api/plugin/global-deps.module.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
TypeScript source files must use import specifiers with .js extensions for ESM compatibility
Files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/internal-graphql-client-usage.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
api/src/unraid-api/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place new API source files in api/src/unraid-api/ rather than legacy locations
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
api/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Prefer not to mock simple dependencies in API tests
Files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: General testing: use .rejects.toThrow() without arguments when asserting thrown errors
General testing: focus on behavior, not exact error message wording
General testing: avoid brittle tests tied to non-essential details (exact messages, log formats)
General testing: use mocks as nouns (objects), not verbs (behavioral overuse)
Files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/internal-graphql-client-usage.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Avoid unnecessary or obvious comments; only add comments when needed for clarity
Files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/internal-graphql-client-usage.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.ts
🧠 Learnings (23)
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/internal-graphql-client-usage.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in component tests (e.g., vi.mock for helper modules)
Applied to files:
api/src/unraid-api/shared/internal-graphql-client.factory.spec.tspackages/unraid-shared/src/services/internal-graphql-client-usage.spec.tsapi/src/unraid-api/cli/internal-client.service.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:07:39.222Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-08-11T15:07:39.222Z
Learning: Applies to api/**/*.{test,spec}.{js,jsx,ts,tsx} : For error testing, use `.rejects.toThrow()` without arguments; avoid asserting exact error messages unless the message format is the subject under test
Applied to files:
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use .rejects.toThrow() without arguments when asserting that async functions throw; avoid checking exact error message strings unless the message format is explicitly under test
Applied to files:
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to **/*.{test,spec}.{js,ts,jsx,tsx} : General testing: use .rejects.toThrow() without arguments when asserting thrown errors
Applied to files:
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to **/*.{test,spec}.{js,ts,jsx,tsx} : General testing: focus on behavior, not exact error message wording
Applied to files:
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to **/*.{test,spec}.{js,ts,jsx,tsx} : General testing: avoid brittle tests tied to non-essential details (exact messages, log formats)
Applied to files:
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js,tsx,jsx} : Web (Vue) component tests should mock external dependencies and services
Applied to files:
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/store/**/*.ts : In store tests, mock external dependencies used by the store and verify interactions with those mocks
Applied to files:
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:07:39.222Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-08-11T15:07:39.222Z
Learning: Applies to api/**/*.{test,spec}.{js,jsx,ts,tsx} : Use Vitest for tests in the api; do not use Jest
Applied to files:
api/src/unraid-api/cli/internal-client.service.spec.tspackages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to api/**/*.{test,spec}.{ts,tsx,js,jsx} : Prefer not to mock simple dependencies in API tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use vi.spyOn() to specify return values or behavior of methods under test
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: This Nuxt app is tested with vitest outside the Nuxt runtime; structure tests accordingly
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to web/**/*.{test,spec}.{ts,js,tsx,jsx} : Store testing with Pinia: place all mock declarations at top level; use factory functions for module mocks; clear mocks between tests
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/mocks/**/*.ts : Frequently used mocks can also be placed under __test__/mocks
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to api/**/jest*.{js,ts,cjs,mjs,json} : In the API package, use Vitest for tests and do not add Jest configuration
Applied to files:
packages/unraid-shared/src/services/socket-config.service.spec.ts
📚 Learning: 2025-08-09T01:06:39.314Z
Learnt from: elibosley
PR: unraid/api#1575
File: packages/unraid-shared/src/services/base-internal-client.service.ts:126-132
Timestamp: 2025-08-09T01:06:39.314Z
Learning: The `ws` npm package (version 8.x) natively supports `ws+unix://` URIs for Unix socket connections. The library's URL parser explicitly recognizes the `ws+unix:` protocol alongside `ws:`, `wss:`, `http:`, and `https:` protocols, allowing direct connection to Unix domain sockets using URIs like `ws+unix:///path/to/socket:/endpoint`.
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts
🧬 Code Graph Analysis (5)
api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts (2)
packages/unraid-shared/src/index.ts (2)
InternalGraphQLClientFactory(6-6)SocketConfigService(2-2)web/helpers/create-apollo-client.ts (1)
client(93-96)
packages/unraid-shared/src/services/internal-graphql-client-usage.spec.ts (2)
packages/unraid-shared/src/index.ts (1)
SocketConfigService(2-2)web/helpers/create-apollo-client.ts (1)
client(93-96)
api/src/unraid-api/cli/internal-client.service.spec.ts (5)
packages/unraid-shared/src/index.ts (1)
InternalGraphQLClientFactory(6-6)packages/unraid-shared/src/types/internal-graphql-client.factory.ts (1)
InternalGraphQLClientFactory(7-13)packages/unraid-shared/src/tokens.ts (1)
INTERNAL_CLIENT_SERVICE_TOKEN(5-5)web/helpers/create-apollo-client.ts (1)
client(93-96)api/src/unraid-api/cli/log.service.ts (1)
error(65-69)
packages/unraid-shared/src/services/socket-config.service.spec.ts (1)
packages/unraid-shared/src/index.ts (1)
SocketConfigService(2-2)
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts (4)
packages/unraid-shared/src/tokens.ts (1)
INTERNAL_CLIENT_SERVICE_TOKEN(5-5)packages/unraid-shared/src/index.ts (1)
InternalGraphQLClientFactory(6-6)packages/unraid-shared/src/types/internal-graphql-client.factory.ts (1)
InternalGraphQLClientFactory(7-13)web/helpers/create-apollo-client.ts (1)
client(93-96)
🔇 Additional comments (4)
packages/unraid-shared/src/services/socket-config.service.spec.ts (2)
1-5: ESM import specifier and Vitest hooks are correctGood use of .js extension for ESM compatibility and proper import of afterEach.
219-246: No adapter needed—ws+unix:// URIs are natively supportedThe internal‐client factory correctly passes the
ws+unix://…:/graphqlURI straight through tocreateClient({ url })using the Nodewsimplementation. Per thews8.x URL parser (and confirmed by thews-unix-socket-test.spec.tsintegration tests), the library recognizesws+unix://socketPath:/pathas a Unix‐domain‐socket endpoint. No additional parsing orsocketPathextraction is required.• The factory’s subscription block:
const wsLink = new GraphQLWsLink( createClient({ url: wsUri, webSocketImpl: WebSocket, … }) );• Under the hood,
new WebSocket(ws+unix://…)(fromwsv8.x) handles the Unix socket and path.You can safely ignore the suggestion to add a custom adapter—everything already works as intended.
Likely an incorrect or invalid review comment.
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts (1)
25-55: Solid DI + concurrency guard; caching logic looks correctUsing an in-flight promise to serialize client creation is correct and prevents duplicate clients under concurrent calls. Caching and finally-block cleanup are handled well.
api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts (1)
4-4: No mismatched ApolloClient imports detectedBoth the production factory and its spec file import from the identical path (
@apollo/client/core/index.js), so there’s no dual-module issue to address.
• api/src/unraid-api/shared/internal-graphql-client.factory.ts:5
• api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts:4
api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts
Outdated
Show resolved
Hide resolved
Contributor
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
Contributor
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
api/src/unraid-api/cli/internal-client.service.ts (6)
4-4: Use a type-only import for Apollo types to avoid a runtime dependencyThis file only uses ApolloClient and NormalizedCacheObject as types; import them as type-only to avoid emitting a runtime ESM import.
-import { ApolloClient, NormalizedCacheObject } from '@apollo/client/core/index.js'; +import type { ApolloClient, NormalizedCacheObject } from '@apollo/client/core/index.js';
58-61: Simplify creationPromise declaration; drop eslint-disableYou can use const for creationPromise and remove the eslint disable. The closure still captures the correct reference.
- // Start creating the client with race condition protection - let creationPromise!: Promise<ApolloClient<NormalizedCacheObject>>; - // eslint-disable-next-line prefer-const - creationPromise = (async () => { + // Start creating the client with race condition protection + const creationPromise: Promise<ApolloClient<NormalizedCacheObject>> = (async () => {
67-73: Remove over-explanatory comments per repo guidelinesThese comments are unnecessary and slightly confusing. The code is already guarded via the creatingClient equality checks.
- // awaiting *before* checking this.creatingClient is important! - // by yielding to the event loop, it ensures - // `this.creatingClient = creationPromise;` is executed before the next check. - - // This prevents race conditions where the client is assigned to the wrong instance. - // Only assign client if this creation is still current + // Only assign client if this creation is still current
54-55: Avoid unnecessary await when returning a promiseReturning the promise directly preserves the stack and avoids an extra microtask.
- return await this.creatingClient; + return this.creatingClient;
88-88: Avoid unnecessary await when returning a promiseReturn the promise directly.
- return await creationPromise; + return creationPromise;
35-35: Improve error logging: pass stack/message instead of objectNest Logger.error expects message and optional stack/context. Logging the raw error object can result in “[object Object]”. Include stack or message.
- this.logger.error('Failed to get admin API key:', error); + this.logger.error( + 'Failed to get admin API key', + error instanceof Error ? error.stack : String(error) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
api/src/unraid-api/cli/internal-client.service.spec.ts(1 hunks)api/src/unraid-api/cli/internal-client.service.ts(2 hunks)api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts(1 hunks)packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts(1 hunks)packages/unraid-shared/src/types/internal-graphql-client.factory.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- api/src/unraid-api/cli/internal-client.service.spec.ts
- api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
TypeScript source files must use import specifiers with .js extensions for ESM compatibility
Files:
api/src/unraid-api/cli/internal-client.service.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.tspackages/unraid-shared/src/types/internal-graphql-client.factory.ts
api/src/unraid-api/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place new API source files in api/src/unraid-api/ rather than legacy locations
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/cli/internal-client.service.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Avoid unnecessary or obvious comments; only add comments when needed for clarity
Files:
api/src/unraid-api/cli/internal-client.service.tspackages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.tspackages/unraid-shared/src/types/internal-graphql-client.factory.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: General testing: use .rejects.toThrow() without arguments when asserting thrown errors
General testing: focus on behavior, not exact error message wording
General testing: avoid brittle tests tied to non-essential details (exact messages, log formats)
General testing: use mocks as nouns (objects), not verbs (behavioral overuse)
Files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
🧠 Learnings (10)
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use .rejects.toThrow() without arguments when asserting that async functions throw; avoid checking exact error message strings unless the message format is explicitly under test
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-08-11T15:07:39.222Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-08-11T15:07:39.222Z
Learning: Applies to api/**/*.{test,spec}.{js,jsx,ts,tsx} : For error testing, use `.rejects.toThrow()` without arguments; avoid asserting exact error messages unless the message format is the subject under test
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to **/*.{test,spec}.{js,ts,jsx,tsx} : General testing: use .rejects.toThrow() without arguments when asserting thrown errors
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to **/*.{test,spec}.{js,ts,jsx,tsx} : General testing: focus on behavior, not exact error message wording
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to **/*.{test,spec}.{js,ts,jsx,tsx} : General testing: avoid brittle tests tied to non-essential details (exact messages, log formats)
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in component tests (e.g., vi.mock for helper modules)
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Applied to files:
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts
📚 Learning: 2024-12-13T17:28:15.384Z
Learnt from: pujitm
PR: unraid/api#982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In `web/helpers/apollo-cache/index.ts`, it's acceptable to use `structuredClone` since the project targets Node.js v20 and browsers where `structuredClone` is supported.
Applied to files:
packages/unraid-shared/src/types/internal-graphql-client.factory.ts
📚 Learning: 2024-12-11T17:20:09.604Z
Learnt from: pujitm
PR: unraid/api#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:
packages/unraid-shared/src/types/internal-graphql-client.factory.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
PR: unraid/api#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:
packages/unraid-shared/src/types/internal-graphql-client.factory.ts
🧬 Code Graph Analysis (3)
api/src/unraid-api/cli/internal-client.service.ts (4)
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts (1)
Injectable(13-74)api/src/unraid-api/shared/internal-graphql-client.factory.ts (1)
Injectable(26-168)packages/unraid-shared/src/tokens.ts (1)
INTERNAL_CLIENT_SERVICE_TOKEN(5-5)packages/unraid-shared/src/types/internal-graphql-client.factory.ts (1)
InternalGraphQLClientFactory(7-13)
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts (2)
web/helpers/create-apollo-client.ts (1)
client(93-96)api/src/unraid-api/cli/log.service.ts (1)
error(65-69)
packages/unraid-shared/src/types/internal-graphql-client.factory.ts (1)
packages/unraid-shared/src/index.ts (1)
InternalGraphQLClientFactory(6-6)
🔇 Additional comments (2)
packages/unraid-shared/src/types/internal-graphql-client.factory.ts (1)
1-13: LGTM: type-only import and ESM-compatible specifier are correctInterface shape and JSDoc are clear; aligns with the factory implementation and shared token usage. No issues found.
packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts (1)
1-274: Tests look solid and follow our guidelines
- Uses .js extension in imports for ESM.
- Uses .rejects.toThrow() without message assertions.
- Concurrency and clearClient behaviors are thoroughly exercised.
pujitm
approved these changes
Aug 13, 2025
Member
pujitm
left a comment
pujitm
left a comment
There was a problem hiding this comment.
significant performance improvement in the connect dashboard!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactor
Tests
Chores