fix: docker build issues due to pnpm catalog packages #1463
Conversation
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
…url) instead of a string(url) Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
…ntroller Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
WalkthroughAdds pnpm workspace catalog and copies pnpm-workspace.yaml into Docker builds. Standardizes Swagger decorators, validation, and error handling. Introduces major ledger credential-definition features and related APIs. Refactors NATS client to use firstValueFrom and typed send surface. Adjusts several API signatures/return types and DTOs. Updates dependency resolution to catalog. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as Client
participant GW as API Gateway
participant LED as Ledger Cred-Def Service
participant AG as Agent Service
participant DB as Database
U->>GW: POST /credential-definitions (payload)
GW->>LED: NATS send(createCredentialDefinition, payload)
LED->>LED: Validate payload, resolve agent type
alt Dedicated agent
LED->>AG: NATS send(createCredDef, agentRedirectPayload)
AG-->>LED: { response: "credDefId" } or error
else Shared agent
LED->>AG: NATS send(createCredDefShared, agentRedirectPayload)
AG-->>LED: { response: "credDefId" } or error
end
LED->>DB: Persist credential_definition
LED-->>GW: credential_definition
GW-->>U: 201 Created + body
rect rgba(230,245,255,0.5)
note over GW,LED: Errors mapped to RpcException with reason/statusCode or fallback to error.response || error
end
sequenceDiagram
autonumber
actor U as Client
participant GW as API Gateway
participant LED as Ledger Cred-Def Service
participant AG as Agent Service
U->>GW: GET /credential-definitions/:id
GW->>LED: NATS send(getCredentialDefinitionById, { id, orgId })
LED->>AG: NATS send(routeToAgent, redirectionPayload)
AG-->>LED: { response: jsonString } or error
LED-->>GW: credential_definition
GW-->>U: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 33
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (21)
apps/ledger/src/schema/schema.service.ts (4)
89-90: Fix always-true null/undefined check
if (null !== schema || schema !== undefined)is always true. Use a proper nullish check.Apply this diff:
- if (null !== schema || schema !== undefined) { + if (schema != null) {
410-418: Ensure HttpException’s second argument is a numeric statusThis code path currently passes
error.erroras the status, which may be non-numeric, producing inconsistent statuses.If you keep the current Promise form, minimally adjust as:
- throw new HttpException( - { - status: error.statusCode, - error: error.error, - message: error.message - }, - error.error - ); + throw new HttpException( + { + status: error.statusCode ?? HttpStatus.INTERNAL_SERVER_ERROR, + error: error.error, + message: error.message + }, + error.statusCode ?? HttpStatus.INTERNAL_SERVER_ERROR + );Note: This is superseded if you adopt the firstValueFrom refactor above.
439-446: Same issue: numeric HttpException status requiredThe second ctor arg should be a valid HTTP status.
Minimal fix if not refactoring:
- throw new HttpException( - { - status: error.error.code, - error: error.message, - message: error.error.message.error.message - }, - error.error - ); + throw new HttpException( + { + status: error.error?.code ?? HttpStatus.INTERNAL_SERVER_ERROR, + error: error.message, + message: error.error?.message?.error?.message ?? error.message + }, + error.error?.code ?? HttpStatus.INTERNAL_SERVER_ERROR + );
563-569: Same issue here: pass numeric status to HttpExceptionAvoid passing arbitrary values as status.
Minimal fix if not refactoring:
- throw new HttpException( - { - status: error.statusCode, - error: error.message - }, - error.error - ); + throw new HttpException( + { + status: error.statusCode ?? HttpStatus.INTERNAL_SERVER_ERROR, + error: error.message + }, + error.statusCode ?? HttpStatus.INTERNAL_SERVER_ERROR + );apps/cloud-wallet/src/cloud-wallet.service.ts (5)
157-160: Bug: malformed URL (extra ‘}’).This will 404 or be rejected by the agent.
- const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}}`; + const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}`;
178-181: Bug: malformed URL and query assembly.Unnecessary slash before query and trailing ‘}’. Also encode threadId.
- const threadParam = threadId ? `?threadId=${threadId}` : ''; - const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${threadParam}}`; + const threadParam = threadId ? `?threadId=${encodeURIComponent(threadId)}` : ''; + const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}${threadParam}`;
257-262: Bug: unsafe null check can throw; condition is wrong.Accessing .id when response is falsy will crash; also logic should be OR.
- if (!createCloudWalletResponse && !createCloudWalletResponse.id) { + if (!createCloudWalletResponse || !createCloudWalletResponse.id) { throw new InternalServerErrorException(ResponseMessages.cloudWallet.error.createCloudWallet, { cause: new Error(), description: ResponseMessages.errorMessages.serverError }); }
272-285: Bug: missing await when encrypting agent token.agentApiKey is set to a Promise instead of string.
const walletKey = await this.commonService.dataEncryption(createCloudWalletResponse.config.walletConfig.token); if (!walletKey) { throw new BadRequestException(ResponseMessages.cloudWallet.error.encryptCloudWalletKey, { cause: new Error(), description: ResponseMessages.errorMessages.serverError }); } - const cloudWalletResponse: ICloudWalletDetails = { + const encryptedAgentApiKey = await this.commonService.dataEncryption(createCloudWalletResponse.token); + + const cloudWalletResponse: ICloudWalletDetails = { createdBy: userId, label, lastChangedBy: userId, tenantId: createCloudWalletResponse.id, type: CloudWalletType.SUB_WALLET, userId, - agentApiKey: this.commonService.dataEncryption(createCloudWalletResponse.token), + agentApiKey: encryptedAgentApiKey, agentEndpoint, email, key: walletKey, connectionImageUrl };
428-438: Use a DID DTO instead of IProofRequestRes[]
Define anIDidinterface matching the/didsresponse (e.g.{ did: string; verkey: string; endpoint?: string; … }), changegetDidList’s return type toPromise<IDid[]>, update the controller and API-gateway signatures accordingly, and keep the?? []default.apps/api-gateway/src/issuance/issuance.service.ts (1)
130-139: Align NATS command name with credentialRecordId
RPC currently sendsget-issued-credentials-by-credentialDefinitionIdbut handles acredentialRecordId; rename this command (and the matching @MessagePattern in issuance-service and agent-service) toget-issued-credentials-by-credentialRecordId.Dockerfiles/Dockerfile.verification (1)
2-2: Upgrade Node base image to 20+.Prevents Nest 11/node ABI mismatches.
-FROM node:18-alpine as build +FROM node:20-alpine as build @@ -FROM node:18-alpine +FROM node:20-alpineAlso applies to: 27-27
Dockerfiles/Dockerfile.agent-service (1)
2-2: Upgrade Node base to 20+.Align with NestJS 11 requirements.
-FROM node:18-alpine as build +FROM node:20-alpine as build @@ -FROM node:18-alpine +FROM node:20-alpineAlso applies to: 34-34
Dockerfiles/Dockerfile.agent-provisioning (1)
2-2: Upgrade Node base image to 20+ in Dockerfiles/Dockerfile.agent-provisioningIn Dockerfiles/Dockerfile.agent-provisioning (lines 2 & 41), change both
FROM node:18-alpinetonode:20-alpine:-FROM node:18-alpine as build +FROM node:20-alpine as build @@ -FROM node:18-alpine as prod +FROM node:20-alpine as prodNestJS v11.1.6 lockfile specifies
engines: { node: '>= 20' }.apps/api-gateway/src/app.module.ts (1)
83-101: Restore exclusion for the root/authzroute.Switching to
{ path: 'authz/(.*)', method: RequestMethod.ALL }drops the separate'authz'exclusion. As a result, thePOST /authzhandler (seeAuthzController.logininapps/api-gateway/src/authz/authz.controller.ts) now goes throughAuthzMiddleware, altering request handling for a previously exempt endpoint. Reintroduce a dedicated exclusion for the bare/authzpath alongside the wildcard entry..exclude( - // The below excludes authz with all its subpaths - { path: 'authz/(.*)', method: RequestMethod.ALL }, + // Exclude /authz root and all subpaths + { path: 'authz', method: RequestMethod.ALL }, + { path: 'authz/(.*)', method: RequestMethod.ALL }, 'admin/subscriptions', 'registry/organizations/',Dockerfiles/Dockerfile.seed (1)
1-1: Base image on Node 18 (EOL) — consider Node 20 LTS (NestJS 11 requires Node 20+)To align with NestJS 11 and security updates, bump to node:20-alpine.
-FROM node:18-alpine +FROM node:20-alpineDockerfiles/Dockerfile.notification (2)
15-18: pnpm i occurs before copying workspace packages — likely to break workspace resolutionRunning pnpm install with only root package.json + workspace file (no app/libs package.json present) can fail or install nothing. Either move install after copying sources or adopt fetch/offline pattern.
Two options:
- Simple (less cache-friendly):
-# Install dependencies -RUN pnpm i --ignore-scripts -# Copy the rest of the application code -COPY . . +# Copy the rest of the application code +COPY . . +# Install dependencies +RUN pnpm i --frozen-lockfile --ignore-scripts
- Cache-friendly:
COPY package.json pnpm-lock.yaml pnpm-workspace.yaml ./ RUN pnpm fetch COPY . . RUN pnpm install -r --offline --frozen-lockfile --ignore-scripts
2-2: Base image on Node 18 (EOL) — bump to Node 20 LTSAlign base images with Node 20.
-FROM node:18-alpine as build +FROM node:20-alpine as build ... -FROM node:18-alpine +FROM node:20-alpineDockerfiles/Dockerfile.organization (2)
16-19: Install occurs before copying workspaces — may break pnpm workspace installAs with notification Dockerfile, move install after COPY . . or use pnpm fetch/offline pattern.
-# Install dependencies while ignoring scripts (including Puppeteer's installation) -RUN pnpm i --ignore-scripts -# Copy the rest of the application code -COPY . . +# Copy the rest of the application code +COPY . . +# Install dependencies while ignoring scripts (including Puppeteer's installation) +RUN pnpm i --frozen-lockfile --ignore-scriptsOr the cache-friendly fetch/offline approach described earlier.
2-2: Base image on Node 18 (EOL) — upgrade to Node 20 LTSAlign with NestJS 11 requirements and security posture.
-FROM node:18-alpine as build +FROM node:20-alpine as build ... -FROM node:18-alpine +FROM node:20-alpineapps/api-gateway/src/connection/connection.service.ts (2)
115-128: Use NATSClient to include context headers (drop direct firstValueFrom)Direct ClientProxy.send omits the contextId headers set by NATSClient. Switch to natsClient.send to maintain tracing/correlation and consistency.
async _getWebhookUrl(tenantId?: string, orgId?: string): Promise<string> { const pattern = { cmd: 'get-webhookurl' }; const payload = { tenantId, orgId }; try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const message: string = await firstValueFrom(this.connectionServiceProxy.send(pattern, payload)); - return message; + const message: string = await this.natsClient.send<string>( + this.connectionServiceProxy, + pattern, + payload + ); + return message; } catch (error) { this.logger.error(`catch: ${JSON.stringify(error)}`); throw error; } }
130-141: Same issue: post webhook response should use NATSClientKeep context headers consistent.
async _postWebhookResponse(webhookUrl: string, data: object): Promise<string> { const pattern = { cmd: 'post-webhook-response-to-webhook-url' }; const payload = { webhookUrl, data }; try { - const message: string = await firstValueFrom(this.connectionServiceProxy.send(pattern, payload)); + const message: string = await this.natsClient.send<string>( + this.connectionServiceProxy, + pattern, + payload + ); return message; } catch (error) { this.logger.error(`catch: ${JSON.stringify(error)}`); throw error; } }
🧹 Nitpick comments (83)
libs/common/src/interfaces/schema.interface.ts (2)
1-1: Use type-only import to avoid accidental runtime dependencyThis import is used purely in type positions; make it explicit.
-import { ICredDefData } from './cred-def.interface'; +import type { ICredDefData } from './cred-def.interface';
76-76: Prefer a type alias overinterface extends Omit<…>; consider clearer namingThis is effectively an alias. A type alias is simpler and avoids confusion with the
Partial<T>utility. Consider a clearer name likePublicCredDefDataorRedactedCredDefData.-export interface PartialICredDefData extends Omit<ICredDefData, 'createdBy' | 'schemaId' | 'orgId'> {} +export type PartialICredDefData = Omit<ICredDefData, 'createdBy' | 'schemaId' | 'orgId'>;apps/ledger/src/schema/schema.service.ts (5)
519-527: Harden error normalization before throwing RpcExceptionThe current check assumes a very specific agent error shape (error.status.message.error). Make the mapping resilient to both HttpException and agent/NATS shapes, and set a single normalized payload.
Apply this diff:
- if (error?.status?.message?.error) { - throw new RpcException({ - message: error.status.message.error.reason || error.status.message.error, - statusCode: error.status?.code ?? HttpStatus.INTERNAL_SERVER_ERROR - }); - } - throw new RpcException(error.response || error); + const errStatus: any = (error as any)?.status ?? (error as any)?.response; + const statusCode = + (errStatus && (errStatus.code ?? errStatus.statusCode ?? errStatus.status)) ?? + (typeof (error as any)?.status === 'number' ? (error as any).status : HttpStatus.INTERNAL_SERVER_ERROR); + const message = + (errStatus?.message?.error?.reason) ?? + (errStatus?.message?.error) ?? + errStatus?.message ?? + errStatus?.error ?? + (typeof (error as any)?.message === 'string' ? (error as any).message : 'Internal server error'); + throw new RpcException({ statusCode, message });
402-421: Replace deprecated toPromise with firstValueFrom and fix HttpException status typetoPromise() is deprecated (RxJS 7) and removed in RxJS 8. Also, HttpException’s second arg must be a numeric status. Use firstValueFrom and ensure a numeric fallback.
Apply this diff:
- const schemaResponse = await from(this.natsClient.send<string>(this.schemaServiceProxy, pattern, payload)) - .pipe( - map((response) => ({ - response - })) - ) - .toPromise() - .catch((error) => { - this.logger.error(`Error in creating schema : ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error.statusCode, - error: error.error, - message: error.message - }, - error.error - ); - }); - return schemaResponse; + try { + const schemaResponse = await firstValueFrom( + this.natsClient + .send<string>(this.schemaServiceProxy, pattern, payload) + .pipe(map((response) => ({ response }))) + ); + return schemaResponse; + } catch (error: any) { + this.logger.error(`Error in creating schema : ${JSON.stringify(error)}`); + throw new HttpException( + { + status: error?.statusCode ?? error?.status ?? HttpStatus.INTERNAL_SERVER_ERROR, + error: error?.error, + message: error?.message + }, + error?.statusCode ?? error?.status ?? HttpStatus.INTERNAL_SERVER_ERROR + ); + }Additionally, update imports outside this hunk:
// replace import { from } from 'rxjs'; // with import { firstValueFrom } from 'rxjs';
429-446: Apply firstValueFrom pattern and numeric HttpException status here as wellAlign the W3C schema call with the same pattern; the current code passes a non-numeric second arg to HttpException.
Apply this diff:
- const W3CSchemaResponse = await from(this.natsClient.send<string>(this.schemaServiceProxy, natsPattern, payload)) - .pipe( - map((response) => ({ - response - })) - ) - .toPromise() - .catch((error) => { - this.logger.error(`Error in creating W3C schema : ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error.error.code, - error: error.message, - message: error.error.message.error.message - }, - error.error - ); - }); - return W3CSchemaResponse; + try { + const W3CSchemaResponse = await firstValueFrom( + this.natsClient + .send<string>(this.schemaServiceProxy, natsPattern, payload) + .pipe(map((response) => ({ response }))) + ); + return W3CSchemaResponse; + } catch (error: any) { + this.logger.error(`Error in creating W3C schema : ${JSON.stringify(error)}`); + throw new HttpException( + { + status: error?.error?.code ?? error?.statusCode ?? HttpStatus.INTERNAL_SERVER_ERROR, + error: error?.message, + message: error?.error?.message?.error?.message ?? error?.message + }, + error?.error?.code ?? error?.statusCode ?? HttpStatus.INTERNAL_SERVER_ERROR + ); + }
553-570: Unify NATS call: use firstValueFrom and ensure HttpException gets a numeric statusSame deprecation and status-type issue as above.
Apply this diff:
- const schemaResponse = await from(this.natsClient.send<string>(this.schemaServiceProxy, pattern, payload)) - .pipe( - map((response) => ({ - response - })) - ) - .toPromise() - .catch((error) => { - this.logger.error(`Catch : ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error.statusCode, - error: error.message - }, - error.error - ); - }); - return schemaResponse; + try { + const schemaResponse = await firstValueFrom( + this.natsClient + .send<string>(this.schemaServiceProxy, pattern, payload) + .pipe(map((response) => ({ response }))) + ); + return schemaResponse; + } catch (error: any) { + this.logger.error(`Catch : ${JSON.stringify(error)}`); + throw new HttpException( + { + status: error?.statusCode ?? error?.status ?? HttpStatus.INTERNAL_SERVER_ERROR, + error: error?.message + }, + error?.statusCode ?? error?.status ?? HttpStatus.INTERNAL_SERVER_ERROR + ); + }
348-349: Log the full error objectInterpolate with JSON.stringify for consistency and better diagnostics.
Apply this diff:
- this.logger.error(`[createSchema] - outer Error: ${JSON.stringify(error)}`); + this.logger.error(`[createW3CSchema] - outer Error: ${JSON.stringify(error)}`);apps/api-gateway/src/platform/platform.service.ts (2)
25-31: Rename param to avoid shadowing and improve clarity.
The parameter name matches the method name; prefer dto/params.Apply:
- async getAllPlatformCredDefs( - getAllPlatformCredDefs: GetAllPlatformCredDefsDto, + async getAllPlatformCredDefs( + params: GetAllPlatformCredDefsDto, user: IUserRequestInterface ): Promise<IPlatformCredDefsData> { - const credDefs = { ...getAllPlatformCredDefs, user }; + const credDefs = { ...params, user }; return this.natsClient.sendNatsMessage(this.platformServiceProxy, 'get-all-platform-cred-defs', credDefs); }
42-46: Tighten return type to match actual response shape- async getShorteningUrlById(referenceId: string): Promise<object> { + async getShorteningUrlById(referenceId: string): Promise<{ referenceId: string; invitationPayload: unknown }> {
- Define a named interface for
{ referenceId: string; invitationPayload: unknown }instead ofobject.- No change to the NATS payload – passing
referenceId: stringmatches the consumer’s signature.apps/api-gateway/src/fido/fido.service.ts (6)
20-20: Name is singular but command is plural — consider generateRegistrationOptions for clarityThe method name uses singular “Option” while the cmd is 'generate-registration-options'. Small mismatch; rename for consistency.
29-33: Unify error handling via a small helper to reduce duplication and ensure consistent RpcException messagesExtract a private helper and call it from all methods to keep catch logic uniform.
Example (outside this diff scope):
private async sendSafely<T>(cmd: string, payload: any): Promise<T> { try { // If you keep sendNats, this preserves the { response: ... } shape return await this.natsClient.sendNats(this.fidoServiceProxy, cmd, payload); // Or switch to typed send() if you drop the wrapper: // return await this.natsClient.send<T>(this.fidoServiceProxy, { cmd }, payload); } catch (error: any) { const message = error?.response ?? error?.message ?? 'Internal error'; throw new RpcException(message); } }Then replace:
return this.natsClient.sendNats(this.fidoServiceProxy, 'verify-registration', payload);with:
return this.sendSafely('verify-registration', payload);Repeat for the other methods.
34-38: Avoid usingobjectin return types; preferunknownor concrete types/generics
objectblocks useful type-narrowing. Either:
- Return
Promise<{ response: unknown }>and narrow at call sites, or- Use generics with the typed NATS client (recommended).
Example minimal tweak:
-async generateAuthenticationOption(generateAuthentication: GenerateAuthenticationDto): Promise<{ response: object }> { +async generateAuthenticationOption(generateAuthentication: GenerateAuthenticationDto): Promise<{ response: unknown }> {Longer-term: switch to the typed
send<T>surface in NATSClient and declare specific response DTOs.
48-54: Avoid leaking DTO class names into wire payload keys
{ updateFidoUserDetailsDto, credentialId }uses a DTO-typed property name. Prefer schema-oriented keys like{ updateFidoUserDetails: updateFidoUserDetailsDto, credentialId }to decouple payload from class names.
66-69: Align return types across methods for consistencySome methods return
{ response: object }, others{ response: string }. Standardize to a single pattern (e.g.,{ response: unknown }with specific DTOs where known) to reduce surprises for callers.
14-19: Use shared DI token and address unused BaseService logger
- Replace the literal
'NATS_CLIENT'in apps/api-gateway/src/fido/fido.service.ts with a shared constant/symbol (e.g.NATS_CLIENT_TOKEN) imported from one module.- Either emit minimal trace logs via
this.log.cmd(...)(inherited from BaseService) or remove theextends BaseServiceand its unused logger.apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts (3)
37-43: Prefer typed NATS helper for compile-time safety.Use NATSClient.send() instead of sendNatsMessage(any) to preserve return types.
Apply to this method as an example:
- configureBaseWallet(cloudBaseWalletConfigure: ICloudBaseWalletConfigure): Promise<IGetStoredWalletInfo> { - return this.natsClient.sendNatsMessage( - this.cloudWalletServiceProxy, - 'configure-cloud-base-wallet', - cloudBaseWalletConfigure - ); - } + configureBaseWallet(cloudBaseWalletConfigure: ICloudBaseWalletConfigure): Promise<IGetStoredWalletInfo> { + return this.natsClient.send<IGetStoredWalletInfo>( + this.cloudWalletServiceProxy, + { cmd: 'configure-cloud-base-wallet' }, + cloudBaseWalletConfigure + ); + }
81-87: Use camelCase for parameter names.Rename ReceiveInvitationDetails → receiveInvitationDetails.
- receiveInvitationByUrl(ReceiveInvitationDetails: IReceiveInvitation): Promise<Response> { + receiveInvitationByUrl(receiveInvitationDetails: IReceiveInvitation): Promise<Response> { return this.natsClient.sendNatsMessage( this.cloudWalletServiceProxy, 'receive-invitation-by-url', - ReceiveInvitationDetails + receiveInvitationDetails ); }
101-114: Method names: fix casing for readability.Consider getConnectionById and getAllConnectionsById (no behavior change; will require updating controller call sites).
apps/cloud-wallet/src/cloud-wallet.service.ts (3)
137-141: Variable shadowing: response overwrites parameter name.Rename local variable for clarity.
- const acceptProofRequest = await this.commonService.httpPost(url, proofAcceptRequestPayload, { + const acceptProofResponse = await this.commonService.httpPost(url, proofAcceptRequestPayload, { headers: { authorization: decryptedApiKey } }); - return acceptProofRequest; + return acceptProofResponse;
332-333: Incorrect log context.Message references createCloudWallet; this is receiveInvitationByUrl.
- this.logger.error(`[createCloudWallet] - error in receive invitation: ${error}`); + this.logger.error(`[receiveInvitationByUrl] - error in receive invitation: ${error}`);
375-376: Incorrect log context.Copy/paste artifact.
- this.logger.error(`[receiveInvitationByUrl] - error in accept offer: ${error}`); + this.logger.error(`[acceptOffer] - error in accept offer: ${error}`);apps/api-gateway/src/issuance/issuance.service.ts (2)
237-245: Replace deprecated .toPromise() with NATSClient or firstValueFrom.
Use the shared NATSClient helper for consistency and to avoid deprecated RxJS APIs.Apply:
async _getWebhookUrl(tenantId?: string, orgId?: string): Promise<string> { const pattern = { cmd: 'get-webhookurl' }; const payload = { tenantId, orgId }; - try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const message = await this.issuanceProxy.send<any>(pattern, payload).toPromise(); - return message; - } catch (error) { - this.logger.error(`catch: ${JSON.stringify(error)}`); - throw error; - } + try { + return await this.natsClient.send<string>(this.issuanceProxy, pattern, payload); + } catch (error) { + this.logger.error(`catch: ${JSON.stringify(error)}`); + throw error; + } }
251-260: Same here: remove .toPromise() for webhook POST.
Align with NATSClient for headers/context propagation and RxJS 7/8 compatibility.async _postWebhookResponse(webhookUrl: string, data: object): Promise<string> { const pattern = { cmd: 'post-webhook-response-to-webhook-url' }; const payload = { webhookUrl, data }; - try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const message = await this.issuanceProxy.send<any>(pattern, payload).toPromise(); - return message; - } catch (error) { - this.logger.error(`catch: ${JSON.stringify(error)}`); - - throw error; - } + try { + return await this.natsClient.send<string>(this.issuanceProxy, pattern, payload); + } catch (error) { + this.logger.error(`catch: ${JSON.stringify(error)}`); + throw error; + } }apps/api-gateway/src/main.ts (1)
80-84: Duplicate enableVersioning calls.
Define versioning once. Keep the later call with VERSION_NEUTRAL.- app.enableVersioning({ - type: VersioningType.URI, - defaultVersion: ['1'] - }); + // Versioning configured once below (includes VERSION_NEUTRAL)Also applies to: 98-101
libs/aws/package.json (1)
16-18: Move @nestjs/testing to devDependencies; optional: plan migration off aws-sdk v2
- @nestjs/testing should not be in dependencies; move it to devDependencies.
- Optional: consider migrating from aws-sdk v2 to modular @aws-sdk/* v3 for smaller images and faster cold starts.
Apply within this block:
- "@nestjs/testing": "catalog:",Then add it under devDependencies.
apps/api-gateway/src/issuance/issuance.controller.ts (1)
87-88: Class-level 401/403 decorators may duplicate method-level responsesYou add ApiUnauthorizedResponse/ApiForbiddenResponse at class level while many methods still declare them. This can duplicate Swagger responses. Consider relying on class-level defaults and removing method-level duplicates (or keep method-level only where descriptions differ).
apps/organization/src/organization.service.ts (2)
1763-1770: Prefer firstValueFrom over .toPromise; align with RxJS 7 and existing NATS usage.toPromise is deprecated in RxJS 7. Use firstValueFrom for consistency and future RxJS 8.
Apply this diff:
- return this.organizationServiceProxy - .send(pattern, userEmails) - .pipe( - map((response: string) => ({ - response - })) - ) - .toPromise() + return firstValueFrom( + this.organizationServiceProxy.send(pattern, userEmails).pipe( + map((response: string) => ({ response })) + ) + ) .catch((error) => { this.logger.error(`getUserKeycloakIdByEmail catch: ${JSON.stringify(error)}`); throw new HttpException( { status: error?.statusCode, error: error?.message }, error.error ); });Also add (outside this block) at the top-level imports:
import { firstValueFrom } from 'rxjs';
1792-1793: Replace .toPromise with firstValueFromKeeps style consistent and avoids deprecated APIs.
Apply this diff:
- const message = await this.organizationServiceProxy.send(pattern, payload).toPromise(); + const message = await firstValueFrom(this.organizationServiceProxy.send<string>(pattern, payload));Ensure you have:
import { firstValueFrom } from 'rxjs';apps/ledger/src/credential-definition/credential-definition.service.ts (4)
92-92: Remove JSON stringify/parse no-op.This deep-clone is unnecessary and allocates memory.
- const response = JSON.parse(JSON.stringify(credDefResponseFromAgentService.response)); + const response = credDefResponseFromAgentService.response as any;
327-341: Replace deprecated toPromise; reuse NATSClient for consistency.Align with other methods and remove RxJS deprecations.
- const getSchemaDetails = await this.credDefServiceProxy - .send(pattern, payload) - .toPromise() - .catch((error) => { - this.logger.error(`catch: ${JSON.stringify(error)}`); - throw new HttpException( - { - status: error.status, - error: error.message - }, - error.status - ); - }); + let getSchemaDetails: ISchemaDetail[]; + try { + getSchemaDetails = await this.natsClient.send<ISchemaDetail[]>(this.credDefServiceProxy, pattern, payload); + } catch (error) { + this.logger.error(`catch: ${JSON.stringify(error)}`); + throw new HttpException( + { status: error.status, error: error.message }, + error.status + ); + }
394-396: Use dot notation and tighten type.Bracket access is unnecessary.
- const activeCredDefSchemaList = credDefSchemaList.filter((item) => !item?.['isSchemaArchived']); + const activeCredDefSchemaList = credDefSchemaList.filter((item) => !item?.isSchemaArchived);
33-39: Remove unused cacheService injection.Dead dependency increases surface area.
- @Inject(CACHE_MANAGER) private readonly cacheService: Cache,Dockerfiles/Dockerfile.agent-provisioning (3)
13-15: Prefer Corepack over global pnpm and remove ephemeral PATH export.Corepack ships with Node 20 and is more reliable in CI.
- && npm install -g pnpm --ignore-scripts \ - && export PATH=$PATH:/usr/lib/node_modules/pnpm/bin \ + && corepack enable \ + && corepack prepare [email protected] --activate \
35-35: Remove debug ls layer.Unnecessary layer and noise.
-RUN ls -R /app/apps/agent-provisioning/AFJ/
63-67: Prune to production deps in runtime and avoid heavy tooling in prod image.Copying full node_modules bloats image; docker/docker-compose/aws-cli likely unnecessary at runtime.
Recommendations:
- In build stage: run “pnpm fetch && pnpm install --frozen-lockfile”.
- Before copying to prod: “pnpm prune --prod” or “pnpm deploy --filter agent-provisioning --prod”.
- Drop docker/docker-compose/aws-cli/openssh-client from the prod stage unless strictly needed.
Also applies to: 79-81
Dockerfiles/Dockerfile.verification (2)
24-24: Unify package runner; prefer pnpm in pnpm-based workspace.Avoid mixed tooling.
-RUN npm run build verification +RUN pnpm run build verification
42-43: Avoid generating Prisma client at runtime.You already generate during build; remove duplicate generation in CMD to speed startup and reduce dev-dep requirements.
-CMD ["sh", "-c", "cd libs/prisma-service && npx prisma migrate deploy && npx prisma generate && cd ../.. && node dist/apps/verification/main.js"] +CMD ["sh", "-c", "cd libs/prisma-service && npx prisma migrate deploy && cd ../.. && node dist/apps/verification/main.js"]Dockerfiles/Dockerfile.agent-service (2)
5-13: Remove duplicate global pnpm install and prefer Corepack.You install pnpm twice. Use Corepack once.
-RUN npm install -g pnpm --ignore-scripts \ +RUN corepack enable \ + && corepack prepare [email protected] --activate \ && apk update \ && apk add openssh-client \ && apk add aws-cli \ && apk add docker \ && apk add docker-compose - -RUN npm install -g pnpm
54-57: Avoid shipping dev deps; prune before copying to prod image.Copying full node_modules increases size and attack surface.
Recommendation:
- In build stage run “pnpm prune --prod” and copy the pruned node_modules to prod, or use “pnpm deploy --filter agent-service --prod”.
Dockerfiles/Dockerfile.seed (1)
9-19: Improve build determinism and cache efficiency for pnpmRecommend installing with lockfile before copying the whole repo to leverage caching and ensure pinning.
Apply this reordering (example):
WORKDIR /app -COPY . . -RUN chmod +x /app/libs/prisma-service/prisma/scripts/geo_location_data_import.sh -RUN chmod +x /app/libs/prisma-service/prisma/scripts/update_client_credential_data.sh - -ENV PUPPETEER_SKIP_DOWNLOAD=true -COPY pnpm-workspace.yaml ./ -RUN pnpm i --ignore-scripts +ENV PUPPETEER_SKIP_DOWNLOAD=true +COPY package.json pnpm-lock.yaml pnpm-workspace.yaml ./ +RUN pnpm i --frozen-lockfile --ignore-scripts +COPY . . +RUN chmod +x /app/libs/prisma-service/prisma/scripts/geo_location_data_import.sh +RUN chmod +x /app/libs/prisma-service/prisma/scripts/update_client_credential_data.shAlso consider corepack to pin pnpm:
-RUN npm install -g pnpm +RUN corepack enable && corepack prepare [email protected] --activateDockerfiles/Dockerfile.notification (2)
11-11: Copy lockfile alongside workspace manifestWithout pnpm-lock.yaml the install is not pinned and may drift.
COPY package.json ./ COPY pnpm-workspace.yaml ./ +# ensure deterministic installs +COPY pnpm-lock.yaml* ./
27-41: Final image includes full node_modules — consider pruning/deploying only runtime depsTo reduce size/attack surface, use pnpm deploy (or prune) for the app and copy only production deps.
Example:
# in build stage after install RUN pnpm -C apps/notification deploy --prod --filter ./apps/notification --no-optional # in final stage COPY --from=build /app/apps/notification/node_modules ./apps/notification/node_modulesAlternatively: pnpm prune --prod and copy the pruned node_modules.
Dockerfiles/Dockerfile.organization (1)
11-11: Copy lockfile with workspace file for deterministic installsInclude pnpm-lock.yaml before install.
COPY package.json ./ COPY pnpm-workspace.yaml ./ +COPY pnpm-lock.yaml* ./apps/api-gateway/src/schema/schema.service.ts (3)
46-49: Method name casing: getcredDefListBySchemaId → getCredDefListBySchemaIdFollow camelCase for readability and consistency. If this is consumed elsewhere, provide a deprecation alias to avoid breaking changes.
-getcredDefListBySchemaId( +getCredDefListBySchemaId(
2-2: Unify BaseService import pathElsewhere BaseService is imported via path alias (e.g., 'libs/service/base.service'). Consider using the alias here for consistency.
14-15: Avoid cross-app import from apps/ledger — move shared types to libs/commonImporting UpdateSchemaResponse from apps/ledger couples modules. Prefer relocating shared interfaces to a common lib (e.g., @credebl/common).
apps/api-gateway/src/webhook/webhook.controller.ts (1)
70-78: Use exceptionFactory return instead of throwPrefer returning an exception instance to avoid double-throw semantics.
- new ParseUUIDPipe({ - exceptionFactory: (): Error => { - throw new BadRequestException(ResponseMessages.organisation.error.invalidOrgId); - } - }) + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + })apps/api-gateway/src/agent/agent.service.ts (3)
28-32: Type the user parameterStrengthen types to reduce footguns and improve DX (e.g., a dedicated IUserRequest type) across service entry points.
48-59: Fix JSDoc mismatchComment lists role/alias/verkey/did, but method accepts id,user. Update docs to match actual signature.
28-32: Prefer typed NATS responsesWhere possible, call a typed helper (e.g., natsClient.send(...)) to return domain types instead of any/{response} wrappers.
Also applies to: 37-40, 42-46, 55-59, 61-64, 66-68, 70-74, 76-87
apps/api-gateway/src/schema/dtos/get-all-schema.dto.ts (2)
120-123: Validate sortByValue as enumCurrently it’s a free string; align with other DTOs.
- @ApiProperty({ required: false }) - @IsOptional() - sortByValue: string = SortValue.DESC; + @ApiProperty({ required: false, enum: [SortValue.DESC, SortValue.ASC] }) + @IsOptional() + @Transform(({ value }) => trim(value)) + @IsEnum(SortValue) + sortByValue: string = SortValue.DESC;
28-35: Optional: tighten property typesConsider typing sortField/sorting as their enums instead of string for stronger validation at compile-time.
Also applies to: 112-119
apps/api-gateway/src/connection/connection.controller.ts (11)
15-16: Remove unused NATS ClientProxy from controllerInjected ClientProxy isn’t used here; drop the import and constructor param to reduce surface and DI graph.
-import { - Inject, - ParseUUIDPipe, - BadRequestException, - Delete -} from '@nestjs/common'; +import { Inject, ParseUUIDPipe, BadRequestException, Delete } from '@nestjs/common'; @@ -import { ClientProxy } from '@nestjs/microservices'; +// import { ClientProxy } from '@nestjs/microservices'; @@ - constructor( - private readonly connectionService: ConnectionService, - @Inject('NATS_CLIENT') private readonly connectionServiceProxy: ClientProxy - ) {} + constructor(private readonly connectionService: ConnectionService) {}Also applies to: 56-56, 66-68
84-98: Validate orgId consistentlyAdd ParseUUIDPipe to orgId here (as in other endpoints) for uniform validation.
- @Param('orgId') orgId: string, + @Param( + 'orgId', + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + }) + ) + orgId: string,
174-179: Add orgId validationApply ParseUUIDPipe to orgId for agent-connections endpoint too.
- @Param('orgId') orgId: string, + @Param( + 'orgId', + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + }) + ) + orgId: string,
210-218: Validate orgId in question-answer record endpointAlign with other routes.
- async getQuestionAnswersRecord(@Param('orgId') orgId: string, @Res() res: Response): Promise<Response> { + async getQuestionAnswersRecord( + @Param( + 'orgId', + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + }) + ) + orgId: string, + @Res() res: Response + ): Promise<Response> {
236-241: Add orgId validation for createConnectionInvitationConsistent ParseUUIDPipe usage.
- @Param('orgId') orgId: string, + @Param( + 'orgId', + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + }) + ) + orgId: string,
279-286: Add orgId validation for sendQuestionConsistent with other endpoints.
- @Param('orgId') orgId: string, + @Param( + 'orgId', + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + }) + ) + orgId: string,
309-313: Add orgId validation for receiveInvitationUrlEnsure proper 400s on bad orgId.
- @Param('orgId') orgId: string, + @Param( + 'orgId', + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + }) + ) + orgId: string,
339-343: Add orgId validation for receiveInvitationSame as above.
- @Param('orgId') orgId: string, + @Param( + 'orgId', + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + }) + ) + orgId: string,
372-380: Consider reducing sensitive debug logsFull JSON of connectionDto might include PII. Prefer minimal fields or sanitize.
425-436: Same logging concern for question-answer webhookAvoid logging full payloads in production logs.
482-514: Add orgId validation for sendBasicMessageAlign with the rest and fail fast on invalid orgId.
- @Param('orgId') orgId: string, + @Param( + 'orgId', + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + }) + ) + orgId: string,apps/api-gateway/src/schema/schema.controller.ts (3)
75-83: Add orgId validation in getSchemaByIdOther endpoints validate orgId; add ParseUUIDPipe here for consistency and early 400s.
- @Param('orgId') orgId: string, + @Param( + 'orgId', + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + }) + ) + orgId: string,
219-231: Create schema: good validation; minor exceptionFactory nitConsider returning the BadRequestException instead of throwing from exceptionFactory for clarity.
- new ParseUUIDPipe({ - exceptionFactory: (): Error => { - throw new BadRequestException(ResponseMessages.organisation.error.invalidOrgId); - } - }) + new ParseUUIDPipe({ + exceptionFactory: (): Error => + new BadRequestException(ResponseMessages.organisation.error.invalidOrgId) + })
246-253: Roles without OrgRolesGuardThis endpoint is excluded from docs but still uses Roles(). If role enforcement is intended, add OrgRolesGuard; otherwise drop Roles() to avoid confusion.
libs/common/src/NATSClient.ts (1)
30-42: Migration to firstValueFrom with proper documentationGood migration from
.toPromise()tofirstValueFrom()which is the recommended approach in newer RxJS versions. The TODO comment correctly identifies a potential issue with nested response objects that should be addressed in a future refactoring.Consider tracking this TODO in a dedicated issue to ensure the response wrapping inconsistency is addressed systematically across all NATS calls.
libs/user-activity/repositories/index.ts (1)
64-72: Consider extracting response normalization logicThe response normalization logic (lines 66-72) could be extracted into a utility function since similar patterns might be needed elsewhere in the codebase when handling NATS responses.
+ private normalizeNatsResponse(resp: any): { message: string } { + if ('string' === typeof resp) { + return { message: resp }; + } + if (resp && 'string' === typeof resp.message) { + return { message: resp.message }; + } + return { message: 'OK' }; + } async _orgDeletedActivity( // ... parameters ): Promise<{ message: string }> { try { // ... existing code const resp = await lastValueFrom(this.userActivityServiceProxy.send(pattern, payload).pipe(timeout(10000))); - if ('string' === typeof resp) { - return { message: resp }; - } - if (resp && 'string' === typeof (resp as any).message) { - return { message: (resp as any).message }; - } - return { message: 'OK' }; + return this.normalizeNatsResponse(resp); } catch (error) { // ... error handling } }apps/api-gateway/src/notification/notification.service.ts (1)
22-30: Prefer typed NATS calls for stronger contractsUse NATSClient.send() to carry context headers and enforce return types instead of untyped sendNatsMessage. This improves typings to INotification and avoids any.
Apply:
- async registerOrgWebhookEndpoint( - registerOrgWebhhookEndpointDto: RegisterOrgWebhhookEndpointDto - ): Promise<INotification> { - return this.natsClient.sendNatsMessage( - this.serviceProxy, - 'register-org-webhook-endpoint-for-notification', - registerOrgWebhhookEndpointDto - ); - } + async registerOrgWebhookEndpoint( + registerOrgWebhhookEndpointDto: RegisterOrgWebhhookEndpointDto + ): Promise<INotification> { + return this.natsClient.send<INotification>( + this.serviceProxy, + { cmd: 'register-org-webhook-endpoint-for-notification' }, + registerOrgWebhhookEndpointDto + ); + } - async sendNotification(notificationRequestBody: SendNotificationDto): Promise<INotification> { - return this.natsClient.sendNatsMessage(this.serviceProxy, 'send-notification', notificationRequestBody); - } + async sendNotification(notificationRequestBody: SendNotificationDto): Promise<INotification> { + return this.natsClient.send<INotification>( + this.serviceProxy, + { cmd: 'send-notification' }, + notificationRequestBody + ); + }Also applies to: 37-39
apps/api-gateway/src/credential-definition/credential-definition.service.ts (2)
25-28: Return a specific type instead of object for getCredentialDefinitionByIdLeverage existing interfaces (e.g., ICredDef or ICredDefData) for accuracy and future safety. Also prefer typed NATS calls.
Example (outside changed lines):
getCredentialDefinitionById(credentialDefinitionId: string, orgId: string): Promise<ICredDefData> { const payload = { credentialDefinitionId, orgId }; return this.natsClient.send<ICredDefData>( this.credDefServiceProxy, { cmd: 'get-credential-definition-by-id' }, payload ); }
20-23: Use typed NATS send across methodsUnify on natsClient.send() for type safety and consistency.
createCredentialDefinition(credDef: CreateCredentialDefinitionDto, user: IUserRequestInterface): Promise<ICredDef> { const payload = { credDef, user }; - return this.natsClient.sendNatsMessage(this.credDefServiceProxy, 'create-credential-definition', payload); + return this.natsClient.send<ICredDef>( + this.credDefServiceProxy, + { cmd: 'create-credential-definition' }, + payload + ); } @@ getAllCredDefs( credDefSearchCriteria: GetAllCredDefsDto, user: IUserRequestInterface, orgId: string ): Promise<ICredDefData> { const payload = { credDefSearchCriteria, user, orgId }; - return this.natsClient.sendNatsMessage(this.credDefServiceProxy, 'get-all-credential-definitions', payload); + return this.natsClient.send<ICredDefData>( + this.credDefServiceProxy, + { cmd: 'get-all-credential-definitions' }, + payload + ); } @@ - return this.natsClient.sendNatsMessage( - this.credDefServiceProxy, - 'get-all-credential-definitions-by-schema-id', - payload - ); + return this.natsClient.send<ICredDefs>( + this.credDefServiceProxy, + { cmd: 'get-all-credential-definitions-by-schema-id' }, + payload + );Also applies to: 30-37, 41-45
apps/api-gateway/src/connection/connection.service.ts (1)
21-21: Remove now-unused import after switching to NATSClientIf you adopt the above change, firstValueFrom becomes unused.
-import { firstValueFrom } from 'rxjs';apps/api-gateway/src/credential-definition/credential-definition.controller.ts (1)
69-71: Add UUID validation to orgId for consistency with other endpointsOther routes use ParseUUIDPipe with a custom BadRequest exception.
Example (outside changed lines):
@Param( 'orgId', new ParseUUIDPipe({ exceptionFactory: (): Error => { throw new BadRequestException(ResponseMessages.organisation.error.invalidOrgId); } }) ) orgId: string,apps/api-gateway/src/agent/agent.controller.ts (2)
72-80: Parse boolean query param@query('_public') arrives as string. Use ParseBoolPipe (or a custom pipe) to ensure a boolean.
Example (outside changed lines):
getAllDid( @User() user: any, @Query('_public', new ParseBoolPipe({ optional: true })) _public?: boolean, @Query('verkey') verkey?: string, @Query('did') did?: string ): Promise<object> { ... }
215-221: Duplicate ApiQuery for 'status'Two ApiQuery decorators use the same name 'status'. Consolidate to one with the enum constraint.
- @ApiQuery({ name: 'status', required: false }) @ApiQuery({ name: 'sortValue', enum: sortValue, required: false }) - @ApiQuery({ name: 'status', enum: booleanStatus, required: false }) + @ApiQuery({ name: 'status', enum: booleanStatus, required: false })apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts (8)
109-129: Avoid injecting@Res(); let Nest handle response shaping.
@Res()bypasses Nest’s interceptors/filters/serialization. Return the response object and, optionally, use@HttpCode(...). Apply this pattern across the controller.Example refactor for this method:
- async createCloudWallet( - @Res() res: Response, - @Body() cloudWalletDetails: CreateCloudWalletDto, - @User() user: user - ): Promise<Response> { + @HttpCode(HttpStatus.CREATED) + async createCloudWallet( + @Body() cloudWalletDetails: CreateCloudWalletDto, + @User() user: user + ): Promise<IResponse> { const { email, id } = user; cloudWalletDetails.email = email; cloudWalletDetails.userId = id; const cloudWalletData = await this.cloudWalletService.createCloudWallet(cloudWalletDetails); const finalResponse: IResponse = { statusCode: HttpStatus.CREATED, message: ResponseMessages.cloudWallet.success.create, data: cloudWalletData }; - return res.status(HttpStatus.CREATED).json(finalResponse); + return finalResponse; }Add
HttpCodeto imports:import { Controller, Post, Logger, Body, HttpStatus, Res, UseFilters, UseGuards, Get, Param, Query, BadRequestException, HttpCode } from '@nestjs/common';
112-114: Remove duplicated@ApiBearerAuth()at method level.It’s already applied at class level; duplicating adds noise without value.
- @ApiBearerAuth() @UseGuards(AuthGuard('jwt'), UserRoleGuard)(Apply to methods at Lines 112-114, 255-257, 290-292, and 317-319.)
Also applies to: 255-257, 290-292, 317-319
328-333: Consolidate DID seed validation into the DTO; remove controller check.Current DTO uses
@MaxLength(32)while controller enforces exact length via constant — inconsistent. Move the exact-length rule to the DTO and drop the controller logic.Controller change:
- if (createDidDto.seed && CommonConstants.SEED_LENGTH !== createDidDto.seed.length) { - throw new BadRequestException(ResponseMessages.agent.error.seedChar, { - cause: new Error(), - description: ResponseMessages.errorMessages.badRequest - }); - }DTO example (apps/api-gateway/src/cloud-wallet/dtos/cloudWallet.dto.ts):
import { Length } from 'class-validator'; import { CommonConstants } from '@credebl/common/common.constant'; export class CreateCloudWalletDidDto { @ApiProperty({ example: '000000000000000000000000000Seed1' }) @IsString({ message: 'seed must be in string format.' }) @Matches(/^\S*$/, { message: 'Spaces are not allowed in seed' }) @Length(CommonConstants.SEED_LENGTH, CommonConstants.SEED_LENGTH, { message: `seed must be exactly ${CommonConstants.SEED_LENGTH} characters.` }) @IsOptional() seed?: string; // ... }
167-198: Document path parameters with@ApiParamfor Swagger.Add
@ApiParamforproofRecordId,connectionId, andcredentialRecordIdso they show up correctly in Swagger.Example for getProofById:
import { ApiParam } from '@nestjs/swagger'; @ApiParam({ name: 'proofRecordId', description: 'Proof record ID', required: true }) @Get('/proofs/:proofRecordId')Replicate for:
- connectionId (getconnectionById, getBasicMessageByConnectionId, sendBasicMessage)
- credentialRecordId (getCredentialByCredentialRecordId)
Also applies to: 416-447, 519-550, 558-589, 598-619
427-447: Rename method togetConnectionByIdfor consistency.Current name
getconnectionByIdviolates typical casing and reads oddly. Consider renaming here and in the service togetConnectionById.
462-479: Rename method to reflect behavior, e.g.,getAllConnections.
getAllconnectionByIdis confusing and mixes singular/plural. PrefergetAllConnections(and align the service).
487-491: Consider pluralizing the collection route to/credentials.Listing endpoint uses
/credential(singular) while other collections use plural/connections. Align for REST consistency, if not externally committed.If the route is already published/consumed, keep the path and consider an alias route instead.
Also applies to: 519-523
95-101: Deduplicate response-wrapping boilerplate.Every method builds
finalResponsethe same way. Consider a small helper (or an interceptor) to standardize success responses and reduce repetition.
| walletProvision(@Body() walletUserDetails: WalletDetailsDto, @User() user: object): Promise<object> { | ||
| this.logger.log(`**** Spin up the agent...${JSON.stringify(walletUserDetails)}`); | ||
|
|
||
| const regex = new RegExp('^[a-zA-Z0-9]+$'); | ||
| if (!regex.test(walletUserDetails.walletName)) { | ||
| this.logger.error(`Wallet name in wrong format.`); | ||
| throw new BadRequestException(`Please enter valid wallet name, It allows only alphanumeric values`); | ||
| } | ||
| const decryptedPassword = this.commonService.decryptPassword(walletUserDetails.walletPassword); | ||
| const decryptedPassword = this.commonService.decryptPassword(walletUserDetails.walletPassword); | ||
| walletUserDetails.walletPassword = decryptedPassword; |
There was a problem hiding this comment.
Avoid logging sensitive wallet details
Logging the entire DTO can leak credentials (even encrypted). Log non-sensitive fields only.
- this.logger.log(`**** Spin up the agent...${JSON.stringify(walletUserDetails)}`);
+ this.logger.log(`**** Spin up the agent... walletName=${walletUserDetails.walletName}, ledgerId=${walletUserDetails.ledgerId}`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| walletProvision(@Body() walletUserDetails: WalletDetailsDto, @User() user: object): Promise<object> { | |
| this.logger.log(`**** Spin up the agent...${JSON.stringify(walletUserDetails)}`); | |
| const regex = new RegExp('^[a-zA-Z0-9]+$'); | |
| if (!regex.test(walletUserDetails.walletName)) { | |
| this.logger.error(`Wallet name in wrong format.`); | |
| throw new BadRequestException(`Please enter valid wallet name, It allows only alphanumeric values`); | |
| } | |
| const decryptedPassword = this.commonService.decryptPassword(walletUserDetails.walletPassword); | |
| const decryptedPassword = this.commonService.decryptPassword(walletUserDetails.walletPassword); | |
| walletUserDetails.walletPassword = decryptedPassword; | |
| walletProvision(@Body() walletUserDetails: WalletDetailsDto, @User() user: object): Promise<object> { | |
| this.logger.log(`**** Spin up the agent... walletName=${walletUserDetails.walletName}, ledgerId=${walletUserDetails.ledgerId}`); | |
| const regex = new RegExp('^[a-zA-Z0-9]+$'); | |
| if (!regex.test(walletUserDetails.walletName)) { | |
| this.logger.error(`Wallet name in wrong format.`); | |
| throw new BadRequestException(`Please enter valid wallet name, It allows only alphanumeric values`); | |
| } | |
| const decryptedPassword = this.commonService.decryptPassword(walletUserDetails.walletPassword); | |
| walletUserDetails.walletPassword = decryptedPassword; |
🤖 Prompt for AI Agents
In apps/api-gateway/src/agent/agent.controller.ts around lines 117 to 126, the
current logger.log call prints the entire walletUserDetails DTO (which may
include sensitive fields like walletPassword); replace that with a safe log that
only includes non-sensitive fields (e.g., walletUserDetails.walletName and
optional user identifier) or a sanitized object that explicitly omits
walletPassword and any credentials, and do not log the decryptedPassword
anywhere; update the log statement accordingly (remove
JSON.stringify(walletUserDetails) and never log walletPassword or decrypted
values).
| @UseFilters(CustomExceptionFilter) | ||
| @Controller() | ||
| @ApiTags('cloud-wallet') | ||
| @ApiBearerAuth() | ||
| @ApiUnauthorizedResponse({ status: HttpStatus.UNAUTHORIZED, description: 'Unauthorized', type: UnauthorizedErrorDto }) | ||
| @ApiForbiddenResponse({ status: HttpStatus.FORBIDDEN, description: 'Forbidden', type: ForbiddenErrorDto }) | ||
| @ApiUnauthorizedResponse({ description: 'Unauthorized', type: UnauthorizedErrorDto }) | ||
| @ApiForbiddenResponse({ description: 'Forbidden', type: ForbiddenErrorDto }) | ||
| export class CloudWalletController { |
There was a problem hiding this comment.
❓ Verification inconclusive
Verify controller route prefix; @Controller() has no path.
If there isn’t a module/router-level prefix, these routes may be mounted at root and collide. Prefer @Controller('cloud-wallet') or ensure a global/router prefix exists.
Run to verify a prefix or router config exists:
🏁 Script executed:
#!/bin/bash
# Look for global prefix or router mappings and any controller with 'cloud-wallet'
rg -n -C3 'setGlobalPrefix\s*\('
rg -n -C3 'RouterModule\.register\s*\('
rg -n -C2 "@Controller\('cloud-wallet'\)"Length of output: 0
Add missing route prefix to CloudWalletController: Controller is declared with @Controller() and no global or module‐level prefix is defined, so its routes mount at /. Specify a path—e.g. change to @Controller('cloud-wallet')—or add a global prefix via app.setGlobalPrefix(...) in main.ts.
🤖 Prompt for AI Agents
In apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts around lines 60
to 66, the CloudWalletController uses @Controller() with no route prefix so its
endpoints mount at '/', causing routing collisions; update the controller
decorator to include an explicit path (e.g., change to
@Controller('cloud-wallet')) so routes are mounted under /cloud-wallet, and
verify ApiTags and any consumers are updated accordingly (alternatively ensure a
global prefix is set in main.ts if you prefer a global routing strategy).
| async acceptOffer(@Res() res: Response, @Body() acceptOffer: AcceptOfferDto, @User() user: user): Promise<Response> { | ||
| const { email, id } = user; | ||
| acceptOffer.email = email; | ||
| acceptOffer.userId = id; | ||
| const receiveInvitationData = await this.cloudWalletService.acceptOffer(acceptOffer); | ||
| const finalResponse: IResponse = { | ||
| statusCode: HttpStatus.CREATED, | ||
| message: ResponseMessages.cloudWallet.success.receive, | ||
| data: receiveInvitationData | ||
| }; | ||
| return res.status(HttpStatus.CREATED).json(finalResponse); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Ensure service errors are not swallowed in acceptOffer.
Service snippet shows a catch without throw, which would return undefined here and still respond 201.
Run to confirm the service behavior:
If it doesn’t rethrow, either:
- Have the service rethrow after
handleError(error), or - Check for falsy in controller and throw an appropriate HTTP exception.
🏁 Script executed:
#!/bin/bash
# Inspect acceptOffer implementation and whether it rethrows in catch
rg -n -C4 'async\s+acceptOffer\(' apps/cloud-wallet/src
rg -n -C2 'acceptOffer' apps/cloud-wallet/src | rg -n -C2 'catch|throw|handleError'Length of output: 2815
🏁 Script executed:
#!/bin/bash
sed -n '370,390p' apps/cloud-wallet/src/cloud-wallet.service.tsLength of output: 634
🏁 Script executed:
#!/bin/bash
rg -n 'handleError' -C3 --glob="*.service.ts"Length of output: 14850
Ensure errors aren't swallowed in acceptOffer — rethrow or handle falsy response
cloud-wallet.service.acceptOffer's catch only calls await this.commonService.handleError(error) and does not rethrow; if handleError doesn't throw, the controller will still return 201 with undefined.
- Fix: add
throw error;afterawait this.commonService.handleError(error);in apps/cloud-wallet/src/cloud-wallet.service.ts (acceptOffer) so errors propagate. - Alternative: have apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts (acceptOffer) validate the service result and throw an appropriate HTTP exception when the response is falsy.
🤖 Prompt for AI Agents
In apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts around lines
292-303, the controller can return 201 with undefined because
cloud-wallet.service.acceptOffer swallows errors; fix by updating the service to
rethrow after error handling: in apps/cloud-wallet/src/cloud-wallet.service.ts
acceptOffer, add "throw error" (or rethrow the processed error) immediately
after "await this.commonService.handleError(error);" so exceptions propagate to
controllers; alternatively (or additionally) validate receiveInvitationData in
this controller and if it's falsy throw an appropriate HTTP exception (e.g.,
InternalServerErrorException with a clear message) instead of returning a 201.
| const receiveInvitationData = await this.cloudWalletService.acceptOffer(acceptOffer); | ||
| const finalResponse: IResponse = { | ||
| statusCode: HttpStatus.CREATED, | ||
| message: ResponseMessages.cloudWallet.success.receive, | ||
| data: receiveInvitationData | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Fix response message and variable name in acceptOffer.
The response message uses “receive” and the variable is misnamed for offer acceptance.
- const receiveInvitationData = await this.cloudWalletService.acceptOffer(acceptOffer);
+ const acceptOfferData = await this.cloudWalletService.acceptOffer(acceptOffer);
const finalResponse: IResponse = {
statusCode: HttpStatus.CREATED,
- message: ResponseMessages.cloudWallet.success.receive,
- data: receiveInvitationData
+ message: ResponseMessages.cloudWallet.success.acceptOffer,
+ data: acceptOfferData
};Verify the exact key in ResponseMessages.cloudWallet.success (e.g., acceptOffer vs acceptedOffer) and adjust accordingly:
🏁 Script executed:
#!/bin/bash
# Find available success message keys for cloud-wallet accept offer
rg -n -C2 'cloudWallet\.success\.'
rg -n -C2 'acceptOffer' | catLength of output: 12424
🏁 Script executed:
#!/bin/bash
# Locate where ResponseMessages is imported in the controller
rg -n "ResponseMessages" -C2 apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts
# Find the file defining ResponseMessages
rg -l "export const ResponseMessages" -g "*.ts"
# Display the cloudWallet.success block from that file
resp_file=$(rg -l "export const ResponseMessages" -g "*.ts")
sed -n '/cloudWallet *:/,/},/p' "$resp_file"Length of output: 4746
Create missing success message and rename variables
In apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts rename receiveInvitationData → acceptOfferData and update the message to ResponseMessages.cloudWallet.success.acceptOffer. Then add this key under cloudWallet.success in libs/common/src/response-messages/index.ts:
acceptOffer: 'Credential offer accepted successfully'🤖 Prompt for AI Agents
In apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts around lines 296
to 301, rename the local variable receiveInvitationData to acceptOfferData and
change the response message key from
ResponseMessages.cloudWallet.success.receive to
ResponseMessages.cloudWallet.success.acceptOffer; then update the object
construction to use acceptOfferData as data. Also add a new key acceptOffer:
'Credential offer accepted successfully' under cloudWallet.success in
libs/common/src/response-messages/index.ts so the new message reference
resolves.
| getDidList(walletDetails: IWalletDetailsForDidList): Promise<IProofRequestRes[]> { | ||
| return this.natsClient.sendNatsMessage(this.cloudWalletServiceProxy, 'cloud-wallet-did-list', walletDetails); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Wrong return type for DID list (uses IProofRequestRes).
DID list is unrelated to proof presentations. Returning IProofRequestRes[] here is misleading and likely incorrect.
Run to locate the expected DID DTO and usages:
🏁 Script executed:
#!/bin/bash
# Find the DID list endpoint constant and usages
rg -n -C3 --type=ts '\bURL_AGENT_GET_DID\b'
# Look for any DID-related interfaces/types
rg -n -C2 --type=ts -P '\binterface\s+I.*Did\b|\btype\s+I.*Did\b'
# Verify message pattern wiring for DID list
rg -n -C2 --type=ts 'cloud-wallet-did-list|getDidList'Length of output: 5950
Update return type to proper DID DTO
The getDidList in apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts should return the DID interface (e.g. IDidList[] or a newly defined DID DTO), not IProofRequestRes[]. Adjust the method signature and import accordingly.
🤖 Prompt for AI Agents
In apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts around lines 97 to
99, the getDidList method currently returns Promise<IProofRequestRes[]> but
should return the DID DTO/interface (e.g. IDidList[] or the project’s canonical
DID DTO). Update the method signature to Promise<IDidList[]> (or the correct DTO
name), add or adjust the import for IDidList from its module, and ensure the
this.natsClient.sendNatsMessage generic matches the new return type so the
method and its callers receive the proper DID type.
| COPY package.json ./ | ||
| COPY pnpm-workspace.yaml ./ | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Copy pnpm-lock.yaml for deterministic builds.
COPY package.json ./
+COPY pnpm-lock.yaml ./
COPY pnpm-workspace.yaml ./🤖 Prompt for AI Agents
In Dockerfiles/Dockerfile.verification around lines 10 to 12, the Dockerfile
copies package.json and pnpm-workspace.yaml but omits pnpm-lock.yaml which means
non-deterministic installs; update the COPY instructions to also copy
pnpm-lock.yaml into the build context (e.g., add "COPY pnpm-lock.yaml ./")
before running install so pnpm can use the lockfile for reproducible builds.
| "@nestjs/axios": "catalog:", | ||
| "@nestjs/common": "catalog:", | ||
| "@nestjs/core": "catalog:", | ||
| "@nestjs/testing": "catalog:", | ||
| "@nestjs/microservices": "catalog:", | ||
| "@nestjs/swagger": "catalog:", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Catalog adoption looks good; move @nestjs/testing to devDeps and verify catalogs wiring
- Use of "catalog:" is fine. However, @nestjs/testing should be a devDependency to avoid bloating prod installs.
- Consider declaring peerDependencies for @nestjs/common/@nestjs/core/@nestjs/microservices to align consumers on a single Nest major.
Apply within this block:
- "@nestjs/testing": "catalog:",Then add it under devDependencies.
Run this script to verify catalog wiring, pnpm/Docker readiness (Node 20+ for Nest 11), and Dockerfiles copying pnpm-workspace.yaml:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "packageManager:"
jq -r '.packageManager // ""' package.json || true
python - <<'PY'
import json, re
try:
pm = json.load(open('package.json')).get('packageManager','')
m = re.match(r'pnpm@(\d+)\.(\d+)\.(\d+)', pm)
if m:
maj=int(m.group(1))
print(f'pnpm major: {maj} (catalogs need pnpm>=8)')
if maj<8: print('WARN: pnpm may be too old for catalogs')
else:
print('No pnpm packageManager found')
except Exception as e:
print(f'Error parsing package.json: {e}')
PY
echo
echo "Catalog entries in pnpm-workspace.yaml for required Nest packages:"
if [ -f pnpm-workspace.yaml ]; then
rg -nP '^\s+@nestjs/(axios|common|core|testing|microservices|swagger):' pnpm-workspace.yaml -C1 || true
else
echo "pnpm-workspace.yaml not found"
fi
echo
echo "Packages using catalog placeholders:"
rg -nP '"@nestjs/(axios|common|core|testing|microservices|swagger)":\s*"catalog:"' libs/common/package.json libs/aws/package.json || true
echo
echo "Dockerfiles copy pnpm-workspace.yaml:"
fd -a -HI 'Dockerfile*' | while read -r f; do
echo "File: $f"
rg -n 'COPY\s+pnpm-workspace.yaml' "$f" || echo " No COPY pnpm-workspace.yaml found"
done
echo
echo "Node base image majors in Dockerfiles (need >=20):"
fd -a -HI 'Dockerfile*' | while read -r f; do
rg -nP '^\s*FROM\s+node:(\d+)' "$f" | awk -v F="$f" -F: '{print "File: "F" -> " $0}'
doneLength of output: 9699
Move @nestjs/testing to devDependencies, add peerDependencies, ensure catalog entries & bump Docker Node
- In
libs/common/package.json, relocate"@nestjs/testing": "catalog:"from dependencies todevDependencies - Declare
peerDependenciesfor@nestjs/common,@nestjs/core, and@nestjs/microservices - Add catalog mappings for all
@nestjs/*packages (axios, common, core, testing, microservices, swagger) inpnpm-workspace.yaml - Update all Dockerfiles to use
node:20-alpineor newer for Nest 11 compatibility
🤖 Prompt for AI Agents
In libs/common/package.json around lines 16 to 21 the review asks to move
@nestjs/testing out of runtime dependencies, add peer dependency declarations,
add catalog mappings, and bump Docker base images: remove "@nestjs/testing":
"catalog:" from dependencies and add it under devDependencies; add
peerDependencies entries for "@nestjs/common", "@nestjs/core", and
"@nestjs/microservices" with appropriate version ranges matching the workspace
(e.g. same major as other Nest packages); update pnpm-workspace.yaml to include
catalog: mappings for all listed @nestjs/* packages (axios, common, core,
testing, microservices, swagger) so they resolve via the catalog; and update
every Dockerfile in the repo to use node:20-alpine or newer to support Nest 11
compatibility.
| export interface ICredDefWithPagination extends IPaginationDetails { | ||
| data: ICredDefData[]; | ||
| data: PartialICredDefData[]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Contract change: ensure runtime redaction and downstream updates
Switching to PartialICredDefData[] removes createdBy, schemaId, and orgId from the type. Verify:
- Producers actually strip these fields at runtime before serializing responses.
- Consumers no longer rely on these fields.
- API docs (Swagger/GraphQL) reflect the new shape.
If you need a mapper:
- const toPublic = ({ createdBy, orgId, schemaId, ...rest }: ICredDefData): PartialICredDefData => rest;
Also applies to: 82-85
🏁 Script executed:
#!/bin/bash
# Locate producers/consumers of the affected types and potential leaks of removed fields.
set -euo pipefail
echo "Usages of the updated interfaces:"
rg -nP -C3 --type=ts '\b(ICredDefWithPagination|ICredDefWithCount|PartialICredDefData)\b'
echo
echo "Check for places where cred-def payloads might include removed fields:"
rg -nP -C3 --type=ts '\b(createdBy|orgId|schemaId)\b' | rg -nP -C2 '(credDef|credentialDef|credentialDefinition)'
echo
echo "Optional: check DTOs/transformers for explicit exclusion:"
rg -nP -C2 --type=ts '@(Exclude|Expose)\('Length of output: 78678
Contract change: strip removed fields and update consumers & docs
- PartialICredDefData omits createdBy, schemaId, orgId—ensure all producers (e.g. schema.repository.ts, credential-definition.repository.ts) redact these before returning
data/credDefResult. - Remove or update any downstream code still accessing
createdBy,schemaId, ororgId. - Update Swagger/GraphQL schemas to reflect the new response shape.
Mapper example:
const toPublic = ({ createdBy, orgId, schemaId, ...rest }: ICredDefData): PartialICredDefData => rest;🤖 Prompt for AI Agents
In libs/common/src/interfaces/schema.interface.ts around lines 78-80, the
ICredDefWithPagination type now uses PartialICredDefData which omits createdBy,
schemaId, and orgId; update all producers (e.g., schema.repository.ts,
credential-definition.repository.ts) to strip those fields before returning
data/credDefResult (implement a mapper that returns only the remaining
properties), update any downstream code that still accesses
createdBy/schemaId/orgId to use available fields or fetch them separately, and
update Swagger/OpenAPI and GraphQL schemas/docs to reflect the new response
shape so clients see the removed fields are no longer returned.
| "@nestjs/axios": "catalog:", | ||
| "@nestjs/bull": "catalog:", | ||
| "@nestjs/cache-manager": "catalog:", | ||
| "@nestjs/common": "catalog:", | ||
| "@nestjs/config": "catalog:", | ||
| "@nestjs/core": "catalog:", | ||
| "@nestjs/jwt": "catalog:", | ||
| "@nestjs/microservices": "catalog:", | ||
| "@nestjs/passport": "catalog:", | ||
| "@nestjs/platform-express": "catalog:", | ||
| "@nestjs/platform-socket.io": "catalog:", | ||
| "@nestjs/schedule": "catalog:", | ||
| "@nestjs/swagger": "catalog:", | ||
| "@nestjs/typeorm": "catalog:", | ||
| "@nestjs/websockets": "catalog:", |
There was a problem hiding this comment.
Catalog specifier is incomplete and will break installs.
All of the updated dependencies now point to the version string catalog: (with nothing after the colon). pnpm expects an alias after the colon (e.g., catalog:nestjs11). Leaving it empty causes pnpm to throw an ERR_PNPM_BAD_PACKAGE_VERSION error, so pnpm install and every Docker build that runs it will fail.
Replace each catalog: with the actual catalog alias you defined in pnpm-workspace.yaml (for example, catalog:nestjs11 or whatever name you picked for the NestJS catalog). Until the alias is added, the build pipeline can’t resolve these packages.
Also applies to: 94-97, 128-131
🤖 Prompt for AI Agents
package.json lines 30-44 (also check lines 94-97 and 128-131): the dependency
entries use the incomplete specifier "catalog:" which lacks the catalog alias
and will break pnpm installs; replace each "catalog:" value with the proper
catalog alias you defined in pnpm-workspace.yaml (e.g., "catalog:nestjs11" or
your chosen alias) so pnpm can resolve the package, and verify all other catalog
entries in the file use the same valid alias format.
| catalog: | ||
| "@nestjs/common": ^11.1.6 | ||
| "@nestjs/core": ^11.1.6 | ||
| "@nestjs/platform-express": ^11.1.6 | ||
| "@nestjs/axios": ^4.0.1 | ||
| "@nestjs/bull": ^11.0.3 | ||
| "@nestjs/cache-manager": ^3.0.1 | ||
| "@nestjs/config": ^4.0.2 | ||
| "@nestjs/jwt": ^11.0.0 | ||
| "@nestjs/microservices": ^11.1.6 | ||
| "@nestjs/passport": ^11.0.5 | ||
| "@nestjs/platform-socket.io": ^11.1.6 | ||
| "@nestjs/schedule": ^6.0.1 | ||
| "@nestjs/swagger": ^11.2.0 | ||
| "@nestjs/typeorm": ^11.0.0 | ||
| "@nestjs/websockets": ^11.1.6 | ||
| "nestjs-cls": ^6.0.1 | ||
| "@nestjs/cli": ^11.0.10 | ||
| "@nestjs/schematics": ^11.0.7 | ||
| "@nestjs/testing": ^11.1.6 | ||
| "nestjs-rate-limiter": ^3.1.0 | ||
| "nestjs-supabase-auth": ^1.0.9 | ||
| "nestjs-typeorm-paginate": ^4.0.4 |
There was a problem hiding this comment.
🧩 Analysis chain
Ensure PNPM catalogs are honored in CI/Docker; consider pinning exact versions.
- Catalogs require pnpm v9+. Ensure Docker/CI uses pnpm>=9 and Node>=20 (Nest 11 requirement).
- Using caret ranges (^) in a catalog can reintroduce drift. Prefer exact pins to maximize reproducibility.
I recommend:
- Add "packageManager": "[email protected]" to the root package.json.
- Verify Dockerfiles copy pnpm-workspace.yaml and use pnpm 9.
Run this script to verify Docker and repo config:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking Dockerfiles for Node base image, pnpm usage, and copying pnpm-workspace.yaml..."
fd -a Dockerfile | while read -r f; do
echo "---- $f ----"
rg -n -N -C2 -e 'FROM\s+node' -e 'corepack' -e 'pnpm' -e 'pnpm-workspace.yaml' "$f" || true
done
echo
echo "Checking if packageManager is set to pnpm@9 in root package.json..."
if [ -f package.json ]; then
rg -n '"packageManager"\s*:\s*".*"' package.json -N || echo "packageManager not set in root package.json"
else
echo "No root package.json found at repo root"
fiLength of output: 43207
Switch Docker base images to Node 20 and pin pnpm
- Update all Dockerfiles’
FROM node:18-alpinetoFROM node:20-alpineto satisfy Nest 11’s Node ≥20 requirement. - Pin the global pnpm version in Dockerfiles (e.g.
npm install -g pnpm@9) or usecorepack prepare pnpm@9 --activateso builds always use v9. - (Optional) Replace the
^ranges in the pnpm-workspace.yaml catalog with exact version pins to prevent drift.
🤖 Prompt for AI Agents
In pnpm-workspace.yaml around lines 5 to 27, update project Dockerfiles and
package management to meet Nest 11 Node≥20 and stabilize pnpm: change all
Dockerfile bases from FROM node:18-alpine to FROM node:20-alpine; pin the pnpm
version used in Docker builds by adding either npm install -g pnpm@9 (or the
required v9.x) or using corepack prepare pnpm@9 --activate in the Dockerfile;
and if you want to prevent dependency drift, replace the caret (^) version
ranges in this catalog with exact version pins (remove ^ and set exact versions)
so builds are reproducible.
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores