feat: websocket server proxying to provider websocket #1802
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughCertificate-based flows are removed across deployment and lease operations. JWT-based provider communication and WebSocket proxying are added. Schemas and services are updated to use walletId instead of certificates. New telemetry helpers, shutdown utility, WebSocket routes/services, JWT token service, and HTTP SDK methods are introduced. Tests and setup utilities are added/refactored. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as API (Deployment/Lease)
participant Reader as DeploymentReaderService
participant Writer as DeploymentWriterService
participant ProviderSvc as ProviderService
participant Jwt as JwtTokenService
participant HTTP as ProviderHttpService
Client->>Writer: update({ sdl })
Writer->>ProviderSvc: sendManifest({ provider, dseq, manifest, walletId })
ProviderSvc->>Jwt: generateJwtToken({ walletId, leases: scoped(send-manifest) })
Jwt-->>ProviderSvc: jwtToken
ProviderSvc->>HTTP: sendManifest({ hostUri, dseq, manifest, jwtToken })
HTTP-->>ProviderSvc: { success }
ProviderSvc-->>Writer: ok
Writer->>Reader: findByOwnerAndDseq(owner, dseq)
Reader->>UserWalletRepo: find wallet by owner
UserWalletRepo-->>Reader: wallet{id}
Reader->>ProviderSvc: getLeaseStatus(..., walletId)
ProviderSvc->>Jwt: generateJwtToken({ walletId, leases: scoped(status) })
Jwt-->>ProviderSvc: jwtToken
ProviderSvc->>HTTP: getLeaseStatus({ hostUri, dseq, gseq, oseq, jwtToken })
HTTP-->>ProviderSvc: leaseStatus
ProviderSvc-->>Reader: leaseStatus
Reader-->>Writer: deploymentWithStatuses
Writer-->>Client: response
sequenceDiagram
autonumber
participant Client as Browser WS
participant API as /ws (Hono)
participant Ctrl as WebsocketController
participant ProvWS as ProviderWebsocketService
participant Jwt as JwtTokenService
participant Provider as Provider WS
Client->>API: Connect /ws (Upgrade)
API->>Ctrl: onOpen
Client->>API: message {type:"ping"}
API->>Ctrl: handleMessage
Ctrl-->>Client: {type:"pong"}
Client->>API: message {type:"websocket", url, data}
API->>Ctrl: handleMessage
Ctrl->>ProvWS: proxyMessageToProvider(message, ws, stats)
ProvWS->>Jwt: generateJwtToken(scoped by url path)
Jwt-->>ProvWS: jwtToken
ProvWS->>Provider: Open WS with Bearer jwt
ProvWS->>Provider: forward data
Provider-->>ProvWS: messages/events
ProvWS-->>Client: proxy responses
Client-->>API: close
API->>Ctrl: handleClose
Ctrl->>ProvWS: closeProviderSocket
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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. 🪧 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 (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| "dependencies": { | ||
| "@akashnetwork/akash-api": "^1.3.0", | ||
| "@akashnetwork/akashjs": "^0.11.1", | ||
| "@akashnetwork/database": "*", |
There was a problem hiding this comment.
🔄 Carefully review the package-lock.json diff
Resolve the comment if everything is ok
* node_modules/@types/ws 8.5.13 -> 8.18.1
+ node_modules/@hono/node-ws 1.2.0
+ node_modules/wait-for-expect 4.0.0 
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🔭 Outside diff range comments (3)
apps/api/examples/lease-flow.ts (2)
101-113: Remove hardcoded provider; allow TARGET_PROVIDER override and fallback to first bidHardcoding a provider makes the example brittle. Prefer env var override and a safe fallback.
-// Select a bid from a specific provider -const targetProvider = "akash175llqyjvxfle9qwt740vm46772dzaznpzgm576"; -const selectedBid = bids.find(bid => bid.bid.bid_id.provider === targetProvider); +// Select a bid: use TARGET_PROVIDER if set, otherwise take the first bid +const targetProvider = process.env.TARGET_PROVIDER; +const selectedBid = targetProvider + ? bids.find(bid => bid.bid.bid_id.provider === targetProvider) + : bids[0]; if (!selectedBid) { - throw new Error(`No bid found from provider ${targetProvider}`); + throw new Error( + targetProvider + ? `No bid found from provider ${targetProvider}` + : "No bids available to select" + ); }Additionally, consider preferring cheapest/fastest bids if business logic permits.
101-105: Type the bids instead of using any to comply with TS guidelinesThe current code (waitForBids and downstream) uses any[]. Define a minimal Bid type to avoid any and tighten usage.
Below type additions are outside the changed range—add them near the imports:
// Minimal shape used by this script; extend if needed type BidId = { gseq: number; oseq: number; provider: string }; type BidItem = { bid: { bid_id: BidId } }; type BidsResponse = { data?: BidItem[] };Then update waitForBids signature and response handling:
-async function waitForBids(dseq: string, apiKey: string, maxAttempts = 10): Promise<any[]> { +async function waitForBids(dseq: string, apiKey: string, maxAttempts = 10): Promise<BidItem[]> { for (let i = 0; i < maxAttempts; i++) { try { - const response = await api.get(`/v1/bids?dseq=${dseq}`, { + const response = await api.get<BidsResponse>(`/v1/bids?dseq=${dseq}`, { headers: { "x-api-key": apiKey } }); - if (response.data?.data?.length > 0) { - return response.data.data; + if ((response.data?.data?.length ?? 0) > 0) { + return response.data.data!; } } catch (error) { console.error("Error waiting for bids:", error); }Optionally, add axios timeouts for resiliency (outside changed range):
// When creating axios instance: const api = axios.create({ baseURL: API_URL, timeout: 30_000, headers: { "Content-Type": "application/json" } });apps/api/test/functional/lease-flow.spec.ts (1)
256-256: Update deployment always includes certificate regardless of test parameterThe update deployment request on line 256 always includes the certificate (
{ certPem, keyPem: encryptedKey }), even when the test is running withincludeCertificate: false. This inconsistency could mask issues in the certificate-free flow.Apply this fix to maintain consistency with the test parameter:
- body: JSON.stringify({ data: { sdl: ymlUpdate, certificate: { certPem, keyPem: encryptedKey } } }) + body: JSON.stringify({ + data: { + sdl: ymlUpdate, + certificate: includeCertificate + ? { certPem, keyPem: encryptedKey } + : undefined + } + })
🧹 Nitpick comments (24)
apps/api/examples/lease-flow.ts (2)
5-11: Fix doc steps to match actual behavior and make provider selection configurableThe comments say “first received bid,” but code later selects a specific hard-coded provider. Clarify docs and make selection configurable via env, falling back to the first bid.
- * 4. Creates a lease using the first received bid + * 4. Creates a lease using a chosen provider (via TARGET_PROVIDER env var) or falls back to the first received bid
69-72: Documentation block below should reflect JWT/cert-less flow and error handlingMinor doc polish: the script creates a lease, deposits, updates, fetches details, then closes. Also note it’s certificate-less.
- * It creates a deployment, waits for bids, creates a lease, and then closes the deployment. + * It creates a deployment, waits for bids, creates a lease (JWT-based, no certificates), deposits, updates the deployment, fetches details, and then closes the deployment.apps/api/src/websocket/services/websocket-stats/websocket-stats.service.ts (1)
22-24: Do not expose internal array; return a shallow copy to avoid external mutationReturning the internal array reference allows external code to observe in-place mutations. Return a copy for safety.
- getItems(): ReadonlyArray<ClientWebSocketStats> { - return this.items; - } + getItems(): ReadonlyArray<ClientWebSocketStats> { + return [...this.items]; + }apps/api/src/billing/lib/wallet/wallet.ts (1)
50-52: Type annotate return and avoid unnecessary await in getInstanceReturn the underlying promise directly and add an explicit return type for clarity and to avoid the extra microtask hop.
Apply this diff:
- async getInstance() { - return await this.instanceAsPromised; - } + async getInstance(): Promise<DirectSecp256k1HdWallet> { + return this.instanceAsPromised; + }apps/api/src/lib/shutdown-server/shutdown-server.ts (1)
1-12: LGTM; minor: drop async when manually constructing a PromiseFunction already returns a Promise; removing async simplifies without behavioral change.
Apply this diff:
-export async function shutdownServer(server: ClosableServer | undefined): Promise<void> { +export function shutdownServer(server: ClosableServer | undefined): Promise<void> { if (!server || !server.listening) return; return new Promise<void>((resolve, reject) => { server.close(error => { if (error) { reject(error); } else { resolve(); } }); }); }apps/api/src/websocket/routes/websocket/websocket.router.ts (1)
21-24: Propagate tracing context on open for consistencyOther handlers are wrapped in propagateTracingContext. Do the same for onOpen to ensure span continuity.
Apply this diff:
- onOpen: async (_event: Event, wsContext: WSContext<unknown>) => { - trackingSpan?.setAttribute("ws.url", wsContext.url?.toString() ?? ""); - controller.handleOpen(stats); - }, + onOpen: async (_event: Event, wsContext: WSContext<unknown>) => { + trackingSpan?.setAttribute("ws.url", wsContext.url?.toString() ?? ""); + propagateTracingContext(() => { + controller.handleOpen(stats); + })(); + },apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (3)
5-5: JwtTokenService mock is not utilized in the testsThe
JwtTokenServiceis imported and mocked withgenerateJwtToken, but it doesn't appear to be invoked byWalletInitializerServicein these test scenarios. Consider removing this mock if it's not needed.If the service is not used, remove the unnecessary mock:
-import { JwtTokenService } from "@src/provider/services/jwt-token/jwt-token.service";- di.registerInstance( - JwtTokenService, - mock<JwtTokenService>({ - generateJwtToken: jest.fn().mockResolvedValue("mock-jwt-token") - }) - );Also applies to: 119-124
19-19:findWalletBymock is defined but never verified in testsThe
findWalletByfunction is mocked and passed to the setup, but none of the test cases verify thatfindOneByis actually called. This mock appears to be unused.Either add test assertions to verify this method is called, or remove the unused mock if it's not needed:
- const findWalletBy = jest.fn().mockImplementation(async () => newWallet); const findWalletByUserId = jest.fn().mockImplementation(async () => null); // ... other mocks const di = setup({ - findWalletBy, findWalletByUserId, // ... other setup params });Also update the SetupInput interface:
interface SetupInput { - findWalletBy?: UserWalletRepository["findOneBy"]; findWalletByUserId?: UserWalletRepository["findOneByUserId"]; // ... other fields }Also applies to: 26-26, 132-132
90-139: Move setup function to the bottom of the describe blockAccording to the coding guidelines, the
setupfunction should be at the bottom of the rootdescribeblock. Currently, it's placed after the test cases but should be moved to the end of the describe block.Move the
setupfunction andSetupInputinterface after line 88 (end of the last test) but before the closing brace of the describe block on line 139.apps/api/test/functional/lease-flow.spec.ts (1)
117-131: Consider using a more descriptive structure for test casesThe test parameterization using
forEachwith an array of objects is good, but could be more maintainable with a dedicated test cases structure.Consider extracting test cases to a more readable structure:
- [ - { - name: "should execute complete lease lifecycle without a certificate provided", - includeCertificate: false - }, - { - name: "should execute complete lease lifecycle with a certificate provided", - includeCertificate: true - } - ].forEach(({ name, includeCertificate }) => { - it(name, async () => { - await runLifecycle(includeCertificate); - }); - }); + const testCases = [ + { includeCertificate: false, description: "without a certificate provided" }, + { includeCertificate: true, description: "with a certificate provided" } + ]; + + testCases.forEach(({ includeCertificate, description }) => { + it(`should execute complete lease lifecycle ${description}`, async () => { + await runLifecycle(includeCertificate); + }); + });apps/api/src/deployment/services/lease/lease.service.ts (1)
33-33: Consider adding error handling for manifest sending failuresThe manifest sending could fail for individual providers, but the current implementation doesn't handle these failures. Consider whether partial failures should be logged or if the entire operation should be rolled back.
Consider collecting errors and handling them appropriately:
+ const manifestErrors: Array<{ provider: string; error: unknown }> = []; for (const lease of input.leases) { - await this.providerService.sendManifest({ provider: lease.provider, dseq: lease.dseq, manifest: input.manifest, walletId: wallet.id }); + try { + await this.providerService.sendManifest({ + provider: lease.provider, + dseq: lease.dseq, + manifest: input.manifest, + walletId: wallet.id + }); + } catch (error) { + manifestErrors.push({ provider: lease.provider, error }); + } } + + if (manifestErrors.length > 0) { + // Log errors or throw based on business requirements + console.error('Failed to send manifest to some providers:', manifestErrors); + }apps/api/test/functional/balances.spec.ts (2)
92-94: Move setup function to the bottom of the describe blockAccording to the coding guidelines, the
setupfunction should be at the bottom of the rootdescribeblock, after all test cases.Move the setup function after line 90 (the last test) but before the closing brace of the describe block on line 95:
it("should return 404 when user wallet is not found", async () => { // ... test code }); + async function setup(options?: SetupUserOptions) { + return await setupUser(options); + } }); - - async function setup(options?: SetupUserOptions) { - return await setupUser(options); - }
16-19: Consider consolidating cleanup logicHaving both
afterEachandafterAllwith identical cleanup code seems redundant. TheafterEachshould be sufficient for cleaning up after each test.Consider removing the redundant
afterAllblock:afterEach(() => { jest.restoreAllMocks(); nock.cleanAll(); }); - afterAll(async () => { - jest.restoreAllMocks(); - nock.cleanAll(); - });apps/api/src/websocket/http-schemas/websocket.schema.ts (1)
23-25: Consider improving the data field descriptionThe description for the
datafield mentions implementation details about char codes and array representation, which might change. Consider a more abstract description.data: z .string() .optional() - .describe( - "Currently it's used only for service shell communication and stores only buffered representation of string in char codes something like this: Array.from(Uint8Array).join(', ')" - ) + .describe("Optional data payload for the WebSocket message")packages/http-sdk/src/provider/provider-http.service.ts (1)
36-41: Headers helper is fine; optional: add Accept.The helper looks good. Optionally add
Accept: application/jsonfor completeness across providers.apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (1)
288-295: Consider making wallet lookup non-throwing for read paths.Optional: have this helper return null and let callers decide whether to degrade or error. Keeps public read endpoints from coupling to wallet presence.
Proposed alternative:
- private async getWalletByAddress(address: string) { - const wallet = await this.userWalletRepository.findOneBy({ address }); - if (!wallet) { - throw new Error(`Wallet not found for address: ${address}`); - } - return wallet; - } + private async getWalletByAddress(address: string) { + return await this.userWalletRepository.findOneBy({ address }) ?? null; + }Then gate usage at call sites as shown in prior comments.
apps/api/src/app.ts (1)
210-216: Minor: redundant string type check onport.
initApp’sportis typed as number with a numeric default. The runtimetypeof port === "string"branch is dead. Simplify.- const server = serve({ - fetch: appHono.fetch, - port: typeof port === "string" ? parseInt(port, 10) : port - }); + const server = serve({ + fetch: appHono.fetch, + port + });apps/api/test/seeders/x509-cert-pair.ts (1)
40-44: Optional: expose validity window helper for reuse.If other seeders need rolling validity, consider exporting
nextDayor allow avalidityDaysoption.apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (1)
126-131: Consider bounded parallelism for manifest delivery to multiple providers.Sequential sends can be slow with multiple leases; unbounded parallelism can overload the provider(s). Use limited concurrency.
Example with Promise.allSettled and a simple concurrency gate:
- for (const provider of leaseProviders) { - await this.providerService.sendManifest({ provider, dseq, manifest, walletId }); - } + const tasks = leaseProviders.map(p => async () => this.providerService.sendManifest({ provider: p, dseq, manifest, walletId })); + const concurrency = 5; + for (let i = 0; i < tasks.length; i += concurrency) { + const batch = tasks.slice(i, i + concurrency).map(fn => fn()); + await Promise.allSettled(batch); + }This balances throughput and resilience without failing the whole update on a single provider error.
apps/api/test/functional/websocket.spec.ts (1)
26-26: Remove console.log statement from testDebug console.log statement should be removed from the test code.
- console.log("messages", messages);apps/api/test/setup/provider-server.ts (1)
22-22: Use const instead of let for cleanupHandlersSince
cleanupHandlersis reassigned rather than mutated, usingconstwould be cleaner and prevent accidental mutations.- let cleanupHandlers = new Set<() => void>(); + const cleanupHandlers = new Set<() => void>();And update the clear operation:
server.on("close", () => { cleanupHandlers.forEach(handler => handler()); - cleanupHandlers = new Set(); + cleanupHandlers.clear(); });apps/api/src/websocket/controllers/websocket/websocket.controller.ts (1)
133-133: Remove unused parameter prefixThe
_eventparameter name uses underscore prefix to indicate it's unused, but the parameter itself should be typed properly for documentation purposes.- async handleClose(_event: any, stats: ClientWebSocketStats, span?: Span) { + async handleClose(_event: CloseEvent, stats: ClientWebSocketStats, span?: Span) {apps/api/test/setup/setup-user.ts (1)
73-78: Extract mock function to avoid duplicationThe
findOneByUserIdMockfunction is defined but could be more concise by directly returning the implementation.- const findOneByUserIdMock = jest.fn().mockImplementation(async (id: string) => { - if (id === userWithId.id) { - return options?.walletNotFound ? undefined : wallet; - } - return undefined; - }); - jest.spyOn(userWalletRepository, "accessibleBy").mockReturnValue(userWalletRepository); - jest.spyOn(userWalletRepository, "findOneByUserId").mockImplementation(findOneByUserIdMock); + jest.spyOn(userWalletRepository, "findOneByUserId").mockImplementation(async (id: string) => { + if (id === userWithId.id) { + return options?.walletNotFound ? undefined : wallet; + } + return undefined; + });apps/api/src/provider/services/provider/provider.service.ts (1)
31-32: Consider validating manifest structure before string replacementThe code performs a string replacement on the manifest JSON without validation. This could potentially cause issues if the manifest structure changes or contains unexpected patterns.
Consider parsing and validating the manifest before manipulation:
async sendManifest({ provider, dseq, manifest, walletId }: { provider: string; dseq: string; manifest: string; walletId: number }) { - const manifestWithSize = manifest.replace(/"quantity":{"val/g, '"size":{"val'); + // Parse, validate, and transform the manifest properly + let manifestObj; + try { + manifestObj = JSON.parse(manifest); + // Transform quantity to size in the parsed object + const transformedManifest = JSON.stringify(manifestObj).replace(/"quantity":{"val/g, '"size":{"val'); + const manifestWithSize = transformedManifest; + } catch (e) { + throw new Error('Invalid manifest JSON'); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
apps/api/examples/lease-flow.ts(9 hunks)apps/api/package.json(5 hunks)apps/api/src/app.ts(5 hunks)apps/api/src/billing/lib/wallet/wallet.ts(1 hunks)apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts(3 hunks)apps/api/src/core/lib/telemetry.ts(1 hunks)apps/api/src/deployment/http-schemas/deployment.schema.ts(1 hunks)apps/api/src/deployment/http-schemas/lease.schema.ts(0 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(5 hunks)apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts(3 hunks)apps/api/src/deployment/services/lease/lease.service.ts(1 hunks)apps/api/src/lib/shutdown-server/shutdown-server.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.spec.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.ts(3 hunks)apps/api/src/websocket/controllers/websocket/websocket.controller.ts(1 hunks)apps/api/src/websocket/http-schemas/websocket.schema.ts(1 hunks)apps/api/src/websocket/routes/websocket/websocket.router.ts(1 hunks)apps/api/src/websocket/services/provider-websocket/provider-websocket.service.ts(1 hunks)apps/api/src/websocket/services/websocket-stats/websocket-stats.service.ts(1 hunks)apps/api/test/functional/balances.spec.ts(2 hunks)apps/api/test/functional/deployments.spec.ts(7 hunks)apps/api/test/functional/lease-flow.spec.ts(3 hunks)apps/api/test/functional/websocket.spec.ts(1 hunks)apps/api/test/seeders/x509-cert-pair.ts(1 hunks)apps/api/test/setup/provider-server.ts(1 hunks)apps/api/test/setup/server.ts(1 hunks)apps/api/test/setup/setup-user.ts(1 hunks)packages/http-sdk/src/provider/provider-http.service.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api/src/deployment/http-schemas/lease.schema.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/test/setup/server.tsapps/api/src/websocket/http-schemas/websocket.schema.tsapps/api/src/core/lib/telemetry.tsapps/api/src/lib/shutdown-server/shutdown-server.tsapps/api/src/deployment/http-schemas/deployment.schema.tsapps/api/src/websocket/controllers/websocket/websocket.controller.tsapps/api/test/functional/deployments.spec.tspackages/http-sdk/src/provider/provider-http.service.tsapps/api/src/websocket/routes/websocket/websocket.router.tsapps/api/src/provider/services/provider/provider.service.spec.tsapps/api/test/setup/setup-user.tsapps/api/test/functional/websocket.spec.tsapps/api/src/app.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/src/billing/lib/wallet/wallet.tsapps/api/test/seeders/x509-cert-pair.tsapps/api/src/deployment/services/lease/lease.service.tsapps/api/src/websocket/services/websocket-stats/websocket-stats.service.tsapps/api/test/setup/provider-server.tsapps/api/examples/lease-flow.tsapps/api/test/functional/lease-flow.spec.tsapps/api/src/websocket/services/provider-websocket/provider-websocket.service.tsapps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/test/functional/balances.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/deployment/services/deployment-writer/deployment-writer.service.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/src/provider/services/provider/provider.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/test/setup/server.tsapps/api/src/websocket/http-schemas/websocket.schema.tsapps/api/src/core/lib/telemetry.tsapps/api/src/lib/shutdown-server/shutdown-server.tsapps/api/src/deployment/http-schemas/deployment.schema.tsapps/api/src/websocket/controllers/websocket/websocket.controller.tsapps/api/test/functional/deployments.spec.tspackages/http-sdk/src/provider/provider-http.service.tsapps/api/src/websocket/routes/websocket/websocket.router.tsapps/api/src/provider/services/provider/provider.service.spec.tsapps/api/test/setup/setup-user.tsapps/api/test/functional/websocket.spec.tsapps/api/src/app.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/src/billing/lib/wallet/wallet.tsapps/api/test/seeders/x509-cert-pair.tsapps/api/src/deployment/services/lease/lease.service.tsapps/api/src/websocket/services/websocket-stats/websocket-stats.service.tsapps/api/test/setup/provider-server.tsapps/api/examples/lease-flow.tsapps/api/test/functional/lease-flow.spec.tsapps/api/src/websocket/services/provider-websocket/provider-websocket.service.tsapps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/test/functional/balances.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/deployment/services/deployment-writer/deployment-writer.service.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/src/provider/services/provider/provider.service.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/test/functional/deployments.spec.tsapps/api/src/provider/services/provider/provider.service.spec.tsapps/api/test/functional/websocket.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/test/functional/lease-flow.spec.tsapps/api/test/functional/balances.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
🧠 Learnings (3)
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Applied to files:
apps/api/test/setup/setup-user.tsapps/api/test/functional/balances.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/api/test/functional/balances.spec.ts
📚 Learning: 2025-06-03T15:06:34.211Z
Learnt from: baktun14
PR: akash-network/console#1428
File: apps/api/src/deployment/controllers/deployment/deployment.controller.ts:0-0
Timestamp: 2025-06-03T15:06:34.211Z
Learning: The `getByOwnerAndDseq` method in `apps/api/src/deployment/controllers/deployment/deployment.controller.ts` is intentionally public without the `Protected` decorator because it serves public blockchain data from an indexer, following the pattern of public blockchain APIs.
Applied to files:
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
🧬 Code Graph Analysis (19)
apps/api/test/setup/server.ts (1)
apps/api/src/app.ts (2)
AppServer(255-258)initApp(205-231)
apps/api/src/websocket/http-schemas/websocket.schema.ts (3)
apps/provider-proxy/src/utils/schema.ts (1)
chainNetworkSchema(15-15)packages/net/src/index.ts (1)
netConfig(3-3)apps/provider-proxy/src/utils/isValidBech32.ts (1)
isValidBech32Address(3-10)
apps/api/src/core/lib/telemetry.ts (1)
apps/api/src/core/services/execution-context/execution-context.service.ts (1)
context(18-26)
apps/api/src/websocket/controllers/websocket/websocket.controller.ts (5)
apps/api/src/provider/services/jwt-token/jwt-token.service.ts (1)
singleton(24-67)apps/api/src/websocket/services/websocket-stats/websocket-stats.service.ts (3)
singleton(7-25)ClientWebSocketStats(27-94)WebSocketUsage(96-96)apps/api/src/websocket/services/provider-websocket/provider-websocket.service.ts (1)
singleton(22-224)apps/api/src/core/lib/telemetry.ts (1)
traceActiveSpan(5-7)apps/api/src/websocket/http-schemas/websocket.schema.ts (2)
WsMessage(29-29)MESSAGE_SCHEMA(13-27)
apps/api/test/functional/deployments.spec.ts (1)
apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)
UserWalletOutput(16-20)
apps/api/src/websocket/routes/websocket/websocket.router.ts (1)
apps/api/src/core/lib/telemetry.ts (1)
propagateTracingContext(9-12)
apps/api/src/provider/services/provider/provider.service.spec.ts (1)
packages/http-sdk/src/provider/provider-http.service.ts (1)
ProviderHttpService(6-42)
apps/api/test/setup/setup-user.ts (6)
apps/api/test/services/wallet-testing.service.ts (1)
WalletTestingService(13-150)apps/api/test/services/stub.ts (1)
stub(1-1)apps/api/src/utils/constants.ts (1)
apiNodeUrl(19-19)apps/api/test/seeders/fee-allowance-response.seeder.ts (1)
FeeAllowanceResponseSeeder(10-32)apps/api/test/seeders/deployment-grant-response.seeder.ts (1)
DeploymentGrantResponseSeeder(10-36)apps/api/test/seeders/deployment-list-response.seeder.ts (1)
DeploymentListResponseSeeder(6-19)
apps/api/test/functional/websocket.spec.ts (4)
apps/api/test/setup/provider-server.ts (2)
stopProviderServer(68-70)startProviderServer(14-66)apps/api/test/setup/setup-user.ts (1)
setupUser(26-140)apps/api/test/setup/server.ts (1)
startServer(6-10)apps/api/test/seeders/x509-cert-pair.ts (1)
createX509CertPair(4-26)
apps/api/src/app.ts (2)
apps/api/src/websocket/routes/websocket/websocket.router.ts (1)
initLeaseWebsocketRoute(11-50)apps/api/src/core/services/shutdown-server/shutdown-server.ts (1)
shutdownServer(7-33)
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts (2)
apps/api/src/billing/providers/config.provider.ts (1)
BillingConfig(9-9)apps/api/src/billing/lib/wallet/wallet.ts (1)
Wallet(7-53)
apps/api/src/deployment/services/lease/lease.service.ts (1)
apps/indexer/drizzle/schema.ts (1)
lease(110-178)
apps/api/test/setup/provider-server.ts (2)
apps/api/test/seeders/x509-cert-pair.ts (2)
createX509CertPair(4-26)CertPair(28-31)apps/api/src/lib/shutdown-server/shutdown-server.ts (1)
shutdownServer(1-12)
apps/api/src/websocket/services/provider-websocket/provider-websocket.service.ts (7)
apps/api/src/websocket/http-schemas/websocket.schema.ts (1)
WsMessage(29-29)apps/api/src/provider/services/jwt-token/jwt-token.service.ts (1)
singleton(24-67)apps/api/src/provider/services/provider/provider.service.ts (1)
singleton(19-225)apps/api/src/websocket/controllers/websocket/websocket.controller.ts (1)
singleton(14-148)apps/api/src/websocket/services/websocket-stats/websocket-stats.service.ts (2)
singleton(7-25)ClientWebSocketStats(27-94)apps/api/src/core/lib/telemetry.ts (1)
propagateTracingContext(9-12)packages/logging/src/servicies/logger/logger.service.ts (1)
error(107-109)
apps/api/src/provider/services/jwt-token/jwt-token.service.ts (3)
apps/api/src/billing/providers/config.provider.ts (2)
InjectBillingConfig(11-11)BillingConfig(9-9)apps/api/src/caching/helpers.ts (1)
Memoize(22-38)apps/api/src/billing/lib/wallet/wallet.ts (1)
Wallet(7-53)
apps/api/test/functional/balances.spec.ts (1)
apps/api/test/setup/setup-user.ts (2)
SetupUserOptions(19-21)setupUser(26-140)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (2)
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (2)
createWallet(59-69)createAndAuthorizeTrialSpending(44-57)apps/deploy-web/src/services/auth/auth.service.ts (1)
AuthService(5-21)
apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (2)
apps/indexer/drizzle/schema.ts (3)
deployment(180-203)lease(110-178)provider(205-237)apps/api/src/deployment/http-schemas/deployment.schema.ts (1)
GetDeploymentResponse(248-248)
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
apps/api/src/deployment/http-schemas/deployment.schema.ts (1)
GetDeploymentResponse(248-248)apps/indexer/drizzle/schema.ts (2)
lease(110-178)deployment(180-203)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: validate (apps/stats-web) / validate-unsafe
- GitHub Check: validate (apps/notifications) / validate-unsafe
- GitHub Check: validate (apps/deploy-web) / validate-unsafe
- GitHub Check: validate (apps/api) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (19)
apps/api/package.json (1)
45-45: Pin and audit WebSocket & JWT dependencies
- Replace
"*"for@akashnetwork/jwtwith an exact version (or useworkspace:*) to ensure reproducible builds and limit supply-chain risk. Verify how this package is being resolved (public registry vs. private registry or local workspace).- Pin a specific version of
@hono/node-ws, confirm your supported Node.js versions, and ensure it doesn’t inadvertently pull in multiple WS implementations.- If
node-forgeis only used for generating test certificates (per the PR summary), move it to devDependencies to minimize your production attack surface.- Audit both
node-forgeandwsfor known security advisories and apply any necessary patches or upgrades.Applies to apps/api/package.json at lines 45, 60, 94, 130, and 154.
apps/api/test/functional/deployments.spec.ts (7)
71-74: Confirm repository query shape for findOneBy spyThe spy assumes queries will always include address and returns a wallet by address. If production code calls findOneBy with other fields (e.g., userId), this will return undefined and tests may diverge from production behavior.
If findOneBy is used with multiple fields, extend the predicate to match all provided fields defensively.
117-118: LGTM: Consolidating wallets for findOneBy mockCollecting wallets in allWallets to back findOneBy simplifies the mocking story and reduces duplication.
694-711: LGTM: PUT update now aligns with wallet-based flow (no certs)Payload includes only sdl, matching the updated schema and flow. Assertions remain correct.
713-749: Validate schema compatibility when including certificate in payloadUpdate schema reportedly dropped certificate fields. Including certificate here may fail validation if unknown keys are rejected. If the API tolerates extraneous fields (strip/ignore), this is fine; otherwise, consider removing certificate from this test or updating the schema to allow it where intended.
Would you like me to scan the schema to confirm whether unknown keys are stripped or cause 400s and adjust the test accordingly?
764-764: LGTM: NotFound path updated to the new request shapeUsing only sdl in the payload keeps the test aligned with the simplified schema.
785-786: LGTM: 401 path updated to the new request shapeEnsures unauthenticated requests are validated against the simplified payload.
809-810: LGTM: Validation path targets invalid SDL onlyThe negative test aligns with the schema change by focusing solely on SDL invalidation.
apps/api/test/functional/lease-flow.spec.ts (1)
196-201: Certificate data might be used even whenincludeCertificateis falseWhen
includeCertificateis false, the certificate field is set toundefined, butcertPemandencryptedKeyare still extracted from the certificate response at line 156. This data exists but isn't being used, which could be confusing.The conditional certificate inclusion logic is correct and aligns with the PR's goal of supporting both certificate-based and wallet-based authentication paths.
apps/api/src/deployment/services/lease/lease.service.ts (1)
1-39: LGTM! Clean migration to wallet-based authenticationThe service has been successfully updated to use wallet-based authentication instead of certificates. The changes are well-structured and maintain clear separation of concerns.
apps/api/src/websocket/http-schemas/websocket.schema.ts (2)
1-30: LGTM! Well-structured WebSocket schema definitionsThe schema definitions are clean and properly typed with Zod. The discriminated union pattern for message types provides good type safety.
6-6: chainNetworkSchema is safe – netConfigData always contains at least one network
ThenetConfigDataobject is generated at build time and must include at least one network key, so
getSupportedNetworks()(which doesObject.keys(netConfigData)) can never return an empty array. No guard or schema change is required here.apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
40-60: Good per-lease error isolation in status lookup.
Your try/catch ensures a single provider failure doesn’t break the whole response for findByOwnerAndDseq. Keep this pattern.
97-105: Guard against undefined indexes when wiring leases to statuses.If any lease list failed or had fewer leases than expected, direct indexing may throw. Use nullish coalescing.
Apply:
- const deploymentsWithLeases = deployments.map((deployment, deploymentIndex) => ({ + const deploymentsWithLeases = deployments.map((deployment, deploymentIndex) => ({ deployment: deployment.deployment, - leases: - leaseResults[deploymentIndex]?.leases?.map(({ lease }, leaseIndex) => ({ + leases: + (leaseResultsOrdered[deploymentIndex]?.leases ?? []).map(({ lease }, leaseIndex) => ({ ...lease, - status: leaseStatuses[deploymentIndex][leaseIndex] + status: leaseStatuses[deploymentIndex]?.[leaseIndex] ?? null })) ?? [], escrow_account: deployment.escrow_account }));Please verify the surrounding identifiers (leaseResultsOrdered) if you adopt the previous suggestion.
apps/api/src/app.ts (2)
166-168: WS wiring looks correct.createNodeWebSocket + injectWebSocket(server) pattern is correctly applied, and the route initialization is clean.
221-226: AppServer shape is solid; graceful shutdown is clear.The
close()correctly reuses the centralized shutdown helper and disposes the DI container.apps/api/test/seeders/x509-cert-pair.ts (1)
4-26: Test certificate generator LGTM.Sane defaults, correct RSA keypair and self-signed X.509 creation. Suitable for TLS-enabled test servers.
apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (1)
95-107: Update flow refactor looks correct; manifest delivery uses walletId.Validation, version check, sendManifest with walletId, and final readback are coherent.
apps/api/src/provider/services/jwt-token/jwt-token.service.ts (1)
1-67: LGTM! Clean JWT token service implementationThe implementation correctly handles JWT token generation with proper memoization, TTL management, and lease payload construction. The service follows best practices and integrates well with the wallet infrastructure.
| export function traceActiveSpan<T extends (span: Span) => any>(name: string, callback: T): ReturnType<T> { | ||
| return trace.getTracer("default").startActiveSpan(name, callback); | ||
| } | ||
|
|
||
| export function propagateTracingContext<T extends (...args: any[]) => any>(callback: T): T { | ||
| const currentContext = context.active(); | ||
| return ((...args) => context.with(currentContext, () => callback(...args))) as T; | ||
| } |
There was a problem hiding this comment.
Avoid any in TS and auto-end spans to prevent leaks; preserve this binding when propagating context
- Coding guideline violation: use of any in generics.
- startActiveSpan variant requires you to end spans; failing to do so risks memory leaks and broken traces.
- Preserve callable this when propagating context.
-export function traceActiveSpan<T extends (span: Span) => any>(name: string, callback: T): ReturnType<T> {
- return trace.getTracer("default").startActiveSpan(name, callback);
-}
+export function traceActiveSpan<R>(name: string, callback: (span: Span) => R): R {
+ return trace.getTracer("default").startActiveSpan(name, span => {
+ try {
+ return callback(span);
+ } finally {
+ // Ensure spans are ended even if callback throws
+ span.end();
+ }
+ });
+}
-export function propagateTracingContext<T extends (...args: any[]) => any>(callback: T): T {
- const currentContext = context.active();
- return ((...args) => context.with(currentContext, () => callback(...args))) as T;
-}
+export function propagateTracingContext<T extends (...args: unknown[]) => unknown>(callback: T): T {
+ const currentContext = context.active();
+ return (function (this: ThisParameterType<T>, ...args: Parameters<T>): ReturnType<T> {
+ return context.with(currentContext, () => callback.apply(this, args)) as ReturnType<T>;
+ } as unknown) as T;
+}Note: If callers currently call span.end() manually, double end is a no-op in OTel SDKs; still, consider removing redundant ends where feasible.
🤖 Prompt for AI Agents
In apps/api/src/core/lib/telemetry.ts around lines 5 to 12, the functions use
"any" in generics, startActiveSpan is used without ensuring spans are ended
(risking leaks), and propagateTracingContext loses the caller's this binding;
change the generic signatures to avoid any (use proper callable types or unknown
for arguments and explicit return types), update traceActiveSpan to wrap the
provided callback so the span is ended automatically after the callback finishes
(ensuring span.end() is always called even on errors), and change
propagateTracingContext to preserve this by returning a function with an
explicit this parameter (e.g., function(this: unknown, ...args) { return
context.with(currentContext, () => callback.apply(this, args)); }) so the
original this is forwarded while keeping strong types.
| export const UpdateDeploymentRequestSchema = z.object({ | ||
| data: z.object({ | ||
| sdl: z.string(), | ||
| certificate: z.object({ | ||
| certPem: z.string(), | ||
| keyPem: z.string() | ||
| }) | ||
| sdl: z.string() | ||
| }) | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tighten SDL validation (min length, sensible size cap, OpenAPI metadata)
The update payload now only carries SDL, which is good. Add basic constraints and docs to prevent empty/oversized bodies and improve API docs.
export const UpdateDeploymentRequestSchema = z.object({
data: z.object({
- sdl: z.string()
+ sdl: z
+ .string()
+ .min(1, "SDL is required")
+ .max(1_000_000, "SDL too large (max 1MB)")
+ .openapi({
+ description: "Akash SDL manifest (YAML)",
+ example: "services:\n web:\n image: nginx\n expose:\n - port: 80"
+ })
})
});If 1MB is too restrictive/lenient for your providers, adjust accordingly.
📝 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.
| export const UpdateDeploymentRequestSchema = z.object({ | |
| data: z.object({ | |
| sdl: z.string(), | |
| certificate: z.object({ | |
| certPem: z.string(), | |
| keyPem: z.string() | |
| }) | |
| sdl: z.string() | |
| }) | |
| }); | |
| export const UpdateDeploymentRequestSchema = z.object({ | |
| data: z.object({ | |
| sdl: z | |
| .string() | |
| .min(1, "SDL is required") | |
| .max(1_000_000, "SDL too large (max 1MB)") | |
| .openapi({ | |
| description: "Akash SDL manifest (YAML)", | |
| example: "services:\n web:\n image: nginx\n expose:\n - port: 80" | |
| }) | |
| }) | |
| }); |
🤖 Prompt for AI Agents
In apps/api/src/deployment/http-schemas/deployment.schema.ts around lines
102–106, tighten the SDL field by enforcing a minimum length and a sensible
maximum (e.g. .min(1) after trimming and .max(1_048_576) for ~1MB), and add
OpenAPI metadata (description and example) so generated docs reflect the
constraints; implement trimming before length checks (or use a refine to reject
blank-only strings) and prefer numeric constant for the max size so it’s easy to
adjust.
| public async findByOwnerAndDseq(owner: string, dseq: string): Promise<GetDeploymentResponse["data"]> { | ||
| const wallet = await this.getWalletByAddress(owner); | ||
| const deploymentResponse = await this.deploymentHttpService.findByOwnerAndDseq(owner, dseq); | ||
|
|
There was a problem hiding this comment.
Don’t hard-require a wallet for public read endpoints; degrade gracefully.
Per prior design, these endpoints serve public indexer data. Throwing when a wallet isn’t found will break read-only requests for arbitrary owners. Fallback to “status: null” when no wallet exists.
Apply this diff to gracefully handle missing wallets:
- public async findByOwnerAndDseq(owner: string, dseq: string): Promise<GetDeploymentResponse["data"]> {
- const wallet = await this.getWalletByAddress(owner);
+ public async findByOwnerAndDseq(owner: string, dseq: string): Promise<GetDeploymentResponse["data"]> {
+ let walletId: number | null = null;
+ try {
+ const wallet = await this.getWalletByAddress(owner);
+ walletId = wallet.id;
+ } catch {
+ walletId = null;
+ }📝 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.
| public async findByOwnerAndDseq(owner: string, dseq: string): Promise<GetDeploymentResponse["data"]> { | |
| const wallet = await this.getWalletByAddress(owner); | |
| const deploymentResponse = await this.deploymentHttpService.findByOwnerAndDseq(owner, dseq); | |
| public async findByOwnerAndDseq(owner: string, dseq: string): Promise<GetDeploymentResponse["data"]> { | |
| let walletId: number | null = null; | |
| try { | |
| const wallet = await this.getWalletByAddress(owner); | |
| walletId = wallet.id; | |
| } catch { | |
| walletId = null; | |
| } | |
| const deploymentResponse = await this.deploymentHttpService.findByOwnerAndDseq(owner, dseq); |
| const wallet = await this.getWalletByAddress(owner); | ||
| const leaseStatuses = await Promise.all( | ||
| leaseResults.map(async ({ leases }) => { | ||
| return await Promise.all( | ||
| leases.map(async ({ lease }) => { | ||
| return await this.providerService.getLeaseStatus(lease.lease_id.provider, lease.lease_id.dseq, lease.lease_id.gseq, lease.lease_id.oseq, wallet.id); | ||
| }) | ||
| ); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Index misalignment and error handling can break listing; preserve order and isolate failures.
PromisePool.results may be shorter than deployments on failures, breaking index-based assembly. Also, provider status retrieval lacks error handling and will reject the whole request on a single failure.
Refactor to preserve order and isolate failures:
- const wallet = await this.getWalletByAddress(owner);
- const leaseStatuses = await Promise.all(
- leaseResults.map(async ({ leases }) => {
- return await Promise.all(
- leases.map(async ({ lease }) => {
- return await this.providerService.getLeaseStatus(lease.lease_id.provider, lease.lease_id.dseq, lease.lease_id.gseq, lease.lease_id.oseq, wallet.id);
- })
- );
- })
- );
+ // Wallet is optional: if not found, fall back to null statuses (public read)
+ let walletId: number | null = null;
+ try {
+ const wallet = await this.getWalletByAddress(owner);
+ walletId = wallet.id;
+ } catch {
+ walletId = null;
+ }
+
+ // Preserve index ordering even when some lease list calls fail
+ const leaseListResults = await Promise.allSettled(
+ deployments.map(d => this.leaseHttpService.list({ owner, dseq: d.deployment.deployment_id.dseq }))
+ );
+ const leaseResultsOrdered = leaseListResults.map(r =>
+ r.status === "fulfilled" ? r.value : { leases: [] as typeof r extends never ? never : any[] } // keep shape
+ );
+
+ // Compute statuses per lease with failure isolation and optional wallet
+ const leaseStatuses = await Promise.all(
+ leaseResultsOrdered.map(async ({ leases }) => {
+ return Promise.all(
+ leases.map(async ({ lease }) => {
+ if (walletId == null) return null;
+ try {
+ return await this.providerService.getLeaseStatus(
+ lease.lease_id.provider,
+ lease.lease_id.dseq,
+ lease.lease_id.gseq,
+ lease.lease_id.oseq,
+ walletId
+ );
+ } catch {
+ return null;
+ }
+ })
+ );
+ })
+ );Notes:
- allSettled keeps array length = deployments.length.
- Each provider call is wrapped with try/catch; failures become null.
- If wallet doesn’t exist, all statuses are null instead of throwing.
Committable suggestion skipped: line range outside the PR's diff.
| function setup(): { | ||
| jwtTokenService: JwtTokenService; | ||
| mockBillingConfig: MockProxy<BillingConfig>; | ||
| mockWalletId: number; | ||
| mockWallet: MockProxy<WalletModule.Wallet>; | ||
| leases: JwtTokenPayload["leases"]; | ||
| } { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove explicit return type from setup function
According to the coding guidelines, don't specify the return type of the setup function.
- function setup(): {
- jwtTokenService: JwtTokenService;
- mockBillingConfig: MockProxy<BillingConfig>;
- mockWalletId: number;
- mockWallet: MockProxy<WalletModule.Wallet>;
- leases: JwtTokenPayload["leases"];
- } {
+ function setup() {🤖 Prompt for AI Agents
In apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts around
lines 124 to 130, the setup function currently declares an explicit return type;
remove the ": { jwtTokenService: JwtTokenService; mockBillingConfig:
MockProxy<BillingConfig>; mockWalletId: number; mockWallet:
MockProxy<WalletModule.Wallet>; leases: JwtTokenPayload['leases']; }" annotation
so the function relies on TypeScript's type inference — simply declare "function
setup()" (keeping the same body) and let the compiler infer the return shape.
| type SetupOptions = { | ||
| onConnection?: (providerWs: WebSocket) => void; | ||
| }; | ||
|
|
||
| async function setup(options: SetupOptions) { | ||
| const { apiKey } = await setupUser(); | ||
|
|
||
| const proxyServerUrl = await startServer(); | ||
| const providerAddress = createAkashAddress(); | ||
| const certPair = createX509CertPair({ commonName: providerAddress }); | ||
| const { providerUrl } = await startProviderServer({ | ||
| certPair, | ||
| websocketServer: { | ||
| enable: true, | ||
| onConnection: options.onConnection | ||
| } | ||
| }); | ||
| const ws = new WebSocket(`${proxyServerUrl}/ws`, { | ||
| headers: { | ||
| "x-api-key": apiKey | ||
| } | ||
| }); | ||
|
|
||
| return { | ||
| providerAddress, | ||
| providerUrl, | ||
| ws | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Setup function should be at the bottom of the root describe block
According to the coding guidelines, the setup function must be at the bottom of the root describe block.
Move the setup function and helper functions to the bottom:
describe("Provider WebSocket", () => {
afterEach(() => {
stopProviderServer();
teardownUser();
});
it("proxies provider websocket messages", async () => {
// ... test
});
it("responds to ping messages", async () => {
// ... test
});
// ... other tests
+
+ function providerMessage<T>(message: T, extra?: Record<string, any>) {
+ return {
+ ...extra,
+ type: "websocket",
+ message
+ };
+ }
+
+ function ourMessage(message: string, url: string, extra?: Record<string, any>) {
+ return {
+ ...extra,
+ type: "websocket",
+ data: message
+ .split("")
+ .map(char => char.charCodeAt(0))
+ .join(","),
+ url: `${url}/test`,
+ providerAddress: extra?.providerAddress || createAkashAddress(),
+ chainNetwork: extra?.chainNetwork || "sandbox"
+ };
+ }
+
+ function waitForMessage<T>(ws: WebSocket) {
+ return new Promise<T>(resolve => {
+ ws.once("message", data => resolve(JSON.parse(data.toString())));
+ });
+ }
+
+ type SetupOptions = {
+ onConnection?: (providerWs: WebSocket) => void;
+ };
+
+ async function setup(options: SetupOptions) {
+ const { apiKey } = await setupUser();
+
+ const proxyServerUrl = await startServer();
+ const providerAddress = createAkashAddress();
+ const certPair = createX509CertPair({ commonName: providerAddress });
+ const { providerUrl } = await startProviderServer({
+ certPair,
+ websocketServer: {
+ enable: true,
+ onConnection: options.onConnection
+ }
+ });
+ const ws = new WebSocket(`${proxyServerUrl}/ws`, {
+ headers: {
+ "x-api-key": apiKey
+ }
+ });
+
+ return {
+ providerAddress,
+ providerUrl,
+ ws
+ };
+ }
-
- function providerMessage<T>(message: T, extra?: Record<string, any>) {
- // ... implementation
- }
-
- function ourMessage(message: string, url: string, extra?: Record<string, any>) {
- // ... implementation
- }
-
- function waitForMessage<T>(ws: WebSocket) {
- // ... implementation
- }
-
- type SetupOptions = {
- onConnection?: (providerWs: WebSocket) => void;
- };
-
- async function setup(options: SetupOptions) {
- // ... implementation
- }
});📝 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.
| type SetupOptions = { | |
| onConnection?: (providerWs: WebSocket) => void; | |
| }; | |
| async function setup(options: SetupOptions) { | |
| const { apiKey } = await setupUser(); | |
| const proxyServerUrl = await startServer(); | |
| const providerAddress = createAkashAddress(); | |
| const certPair = createX509CertPair({ commonName: providerAddress }); | |
| const { providerUrl } = await startProviderServer({ | |
| certPair, | |
| websocketServer: { | |
| enable: true, | |
| onConnection: options.onConnection | |
| } | |
| }); | |
| const ws = new WebSocket(`${proxyServerUrl}/ws`, { | |
| headers: { | |
| "x-api-key": apiKey | |
| } | |
| }); | |
| return { | |
| providerAddress, | |
| providerUrl, | |
| ws | |
| }; | |
| } | |
| describe("Provider WebSocket", () => { | |
| afterEach(() => { | |
| stopProviderServer(); | |
| teardownUser(); | |
| }); | |
| it("proxies provider websocket messages", async () => { | |
| // ... test | |
| }); | |
| it("responds to ping messages", async () => { | |
| // ... test | |
| }); | |
| // ... other tests | |
| function providerMessage<T>(message: T, extra?: Record<string, any>) { | |
| return { | |
| ...extra, | |
| type: "websocket", | |
| message | |
| }; | |
| } | |
| function ourMessage(message: string, url: string, extra?: Record<string, any>) { | |
| return { | |
| ...extra, | |
| type: "websocket", | |
| data: message | |
| .split("") | |
| .map(char => char.charCodeAt(0)) | |
| .join(","), | |
| url: `${url}/test`, | |
| providerAddress: extra?.providerAddress || createAkashAddress(), | |
| chainNetwork: extra?.chainNetwork || "sandbox" | |
| }; | |
| } | |
| function waitForMessage<T>(ws: WebSocket) { | |
| return new Promise<T>(resolve => { | |
| ws.once("message", data => resolve(JSON.parse(data.toString()))); | |
| }); | |
| } | |
| type SetupOptions = { | |
| onConnection?: (providerWs: WebSocket) => void; | |
| }; | |
| async function setup(options: SetupOptions) { | |
| const { apiKey } = await setupUser(); | |
| const proxyServerUrl = await startServer(); | |
| const providerAddress = createAkashAddress(); | |
| const certPair = createX509CertPair({ commonName: providerAddress }); | |
| const { providerUrl } = await startProviderServer({ | |
| certPair, | |
| websocketServer: { | |
| enable: true, | |
| onConnection: options.onConnection | |
| } | |
| }); | |
| const ws = new WebSocket(`${proxyServerUrl}/ws`, { | |
| headers: { | |
| "x-api-key": apiKey | |
| } | |
| }); | |
| return { | |
| providerAddress, | |
| providerUrl, | |
| ws | |
| }; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In apps/api/test/functional/websocket.spec.ts around lines 165 to 193, the setup
helper is defined mid-file but must be placed at the bottom of the root describe
block per guidelines; move the entire setup function (and any related helper
functions) so they appear after all tests and nested describes, i.e., append
them just before the file ends within the root describe block, update any
references/imports if necessary, and ensure no tests rely on hoisting behavior —
run the test suite to confirm nothing breaks.
| async function setup(options: SetupOptions) { | ||
| const { apiKey } = await setupUser(); | ||
|
|
||
| const proxyServerUrl = await startServer(); | ||
| const providerAddress = createAkashAddress(); | ||
| const certPair = createX509CertPair({ commonName: providerAddress }); | ||
| const { providerUrl } = await startProviderServer({ | ||
| certPair, | ||
| websocketServer: { | ||
| enable: true, | ||
| onConnection: options.onConnection | ||
| } | ||
| }); | ||
| const ws = new WebSocket(`${proxyServerUrl}/ws`, { | ||
| headers: { | ||
| "x-api-key": apiKey | ||
| } | ||
| }); | ||
|
|
||
| return { | ||
| providerAddress, | ||
| providerUrl, | ||
| ws | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Setup function should accept inline type definition directly
According to the coding guidelines, the setup function should accept a single parameter with inline type definition, not reference an external type.
- type SetupOptions = {
- onConnection?: (providerWs: WebSocket) => void;
- };
-
- async function setup(options: SetupOptions) {
+ async function setup(options: {
+ onConnection?: (providerWs: WebSocket) => void;
+ }) {📝 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.
| async function setup(options: SetupOptions) { | |
| const { apiKey } = await setupUser(); | |
| const proxyServerUrl = await startServer(); | |
| const providerAddress = createAkashAddress(); | |
| const certPair = createX509CertPair({ commonName: providerAddress }); | |
| const { providerUrl } = await startProviderServer({ | |
| certPair, | |
| websocketServer: { | |
| enable: true, | |
| onConnection: options.onConnection | |
| } | |
| }); | |
| const ws = new WebSocket(`${proxyServerUrl}/ws`, { | |
| headers: { | |
| "x-api-key": apiKey | |
| } | |
| }); | |
| return { | |
| providerAddress, | |
| providerUrl, | |
| ws | |
| }; | |
| } | |
| async function setup(options: { | |
| onConnection?: (providerWs: WebSocket) => void; | |
| }) { | |
| const { apiKey } = await setupUser(); | |
| const proxyServerUrl = await startServer(); | |
| const providerAddress = createAkashAddress(); | |
| const certPair = createX509CertPair({ commonName: providerAddress }); | |
| const { providerUrl } = await startProviderServer({ | |
| certPair, | |
| websocketServer: { | |
| enable: true, | |
| onConnection: options.onConnection | |
| } | |
| }); | |
| const ws = new WebSocket(`${proxyServerUrl}/ws`, { | |
| headers: { | |
| "x-api-key": apiKey | |
| } | |
| }); | |
| return { | |
| providerAddress, | |
| providerUrl, | |
| ws | |
| }; | |
| } |
🤖 Prompt for AI Agents
In apps/api/test/functional/websocket.spec.ts around lines 169-193, the setup
function currently types its parameter as SetupOptions; replace that external
type reference with an inline parameter type (a single object whose properties
include an optional onConnection callback that receives the new
websocket/connection), update the function signature to use that inline shape,
and remove any now-unused SetupOptions import/definition so the file compiles
and conforms to the coding guideline.
| import { createAkashAddress } from "@test/seeders"; | ||
| import { type CertPair, createX509CertPair } from "@test/seeders/x509-cert-pair"; | ||
|
|
||
| let runningServer: https.Server | undefined; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential memory leak with global server reference
The runningServer variable is set but may not be cleared after shutdown, potentially causing issues if multiple tests run in sequence.
Clear the reference after shutdown:
export function stopProviderServer(): Promise<void> {
- return shutdownServer(runningServer);
+ const serverToShutdown = runningServer;
+ runningServer = undefined;
+ return shutdownServer(serverToShutdown);
}📝 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.
| let runningServer: https.Server | undefined; | |
| export function stopProviderServer(): Promise<void> { | |
| const serverToShutdown = runningServer; | |
| runningServer = undefined; | |
| return shutdownServer(serverToShutdown); | |
| } |
🤖 Prompt for AI Agents
In apps/api/test/setup/provider-server.ts around line 12 the module-level
variable `runningServer` may hold a Server instance after shutdown causing a
potential memory leak; after calling server.close()/awaiting its close callback,
explicitly set `runningServer = undefined` (and similarly clear any other global
refs) so repeated test runs don’t retain the old server; update the
teardown/stop logic to await close and then null out the variable (and ensure
any error paths also clear it).
| export async function startServer(): Promise<string> { | ||
| server = await initApp(0); | ||
|
|
||
| return server!.host; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make server lifecycle idempotent and clear the handle after stop
- Prevent multiple running instances when startServer is called twice.
- Ensure the module-scoped handle is cleared to avoid leaking stale state between tests.
Apply this diff:
export async function startServer(): Promise<string> {
- server = await initApp(0);
-
- return server!.host;
+ if (server) {
+ await server.close();
+ server = undefined;
+ }
+ server = await initApp(0);
+ return server.host;
}
export async function stopServer(): Promise<void> {
- await server?.close();
+ if (server) {
+ await server.close();
+ server = undefined;
+ }
}Also applies to: 12-14
🤖 Prompt for AI Agents
In apps/api/test/setup/server.ts around lines 6-10 and 12-14, make startServer
idempotent and clear the module-scoped handle on stop: if a server handle
already exists, return its host instead of creating a new server; when
stopServer is called, await shutdown and set the module-scoped server variable
to null/undefined to avoid leaking stale state between tests. Ensure startServer
initializes only when server is falsy and stopServer always clears the handle
after successful stop (and is safe to call if server is already null).
| async sendManifest({ hostUri, dseq, manifest, jwtToken }: { hostUri: string; dseq: string; manifest: string; jwtToken: string }) { | ||
| return this.extractData( | ||
| await this.put(`/deployment/${dseq}/manifest`, { | ||
| baseURL: hostUri, | ||
| body: manifest, | ||
| headers: this.getJwtTokenHeaders(jwtToken), | ||
| timeout: 60000 | ||
| }) | ||
| ); |
There was a problem hiding this comment.
PUT payload not sent: use Axios data arg (or config.data) instead of body.
HttpService is Axios-based (constructor typed with AxiosRequestConfig). Axios ignores body in config and expects the payload as the second argument or in config.data. As written, the manifest won’t be sent.
Apply one of these fixes (preferred: pass manifest as second argument):
- async sendManifest({ hostUri, dseq, manifest, jwtToken }: { hostUri: string; dseq: string; manifest: string; jwtToken: string }) {
- return this.extractData(
- await this.put(`/deployment/${dseq}/manifest`, {
- baseURL: hostUri,
- body: manifest,
- headers: this.getJwtTokenHeaders(jwtToken),
- timeout: 60000
- })
- );
- }
+ async sendManifest({ hostUri, dseq, manifest, jwtToken }: { hostUri: string; dseq: string; manifest: string; jwtToken: string }) {
+ return this.extractData(
+ await this.put(`/deployment/${dseq}/manifest`,
+ manifest,
+ {
+ baseURL: hostUri,
+ headers: this.getJwtTokenHeaders(jwtToken),
+ timeout: 60000
+ }
+ )
+ );
+ }Alternatively, if HttpService.put only accepts a single config object, put the payload in data:
- body: manifest,
+ data: manifest,📝 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.
| async sendManifest({ hostUri, dseq, manifest, jwtToken }: { hostUri: string; dseq: string; manifest: string; jwtToken: string }) { | |
| return this.extractData( | |
| await this.put(`/deployment/${dseq}/manifest`, { | |
| baseURL: hostUri, | |
| body: manifest, | |
| headers: this.getJwtTokenHeaders(jwtToken), | |
| timeout: 60000 | |
| }) | |
| ); | |
| async sendManifest( | |
| { hostUri, dseq, manifest, jwtToken }: { hostUri: string; dseq: string; manifest: string; jwtToken: string } | |
| ) { | |
| return this.extractData( | |
| await this.put( | |
| `/deployment/${dseq}/manifest`, | |
| manifest, | |
| { | |
| baseURL: hostUri, | |
| headers: this.getJwtTokenHeaders(jwtToken), | |
| timeout: 60000 | |
| } | |
| ) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In packages/http-sdk/src/provider/provider-http.service.ts around lines 15 to
23, the PUT request currently places the manifest into a non‑existent `body`
config key so Axios will not send the payload; change the call to pass the
manifest as the request body (preferred: pass manifest as the second argument to
this.put and keep the config object for baseURL, headers and timeout) or, if
this.put only accepts a single config object, move the payload into `data` on
the config; ensure headers (getJwtTokenHeaders), baseURL and timeout remain in
the config and then return the result via extractData as before.
jzsfkzm
force-pushed
the
features/1024-stream-lease-logs
branch
from
9f8b4c9 to
9ef22b8
Compare
There was a problem hiding this comment.
suggestion: we could add an example on how to pull the logs or events
There was a problem hiding this comment.
I just added an example, not 100% sure about it though, can I run it locally?
jzsfkzm
force-pushed
the
features/1024-stream-lease-logs
branch
2 times, most recently
from
f38b451 to
fbbac53
Compare
jzsfkzm
force-pushed
the
features/1024-stream-lease-logs
branch
from
fbbac53 to
c873688
Compare
2 participants
refs #1024
made it a draft until #1766 is merged
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests