feat: adds api background-jobs server setup#1833
Conversation
WalkthroughSplit the API into rest and background-jobs runtimes, introduce a startServer bootstrap with APP_INITIALIZER lifecycle, refactor job queue startup/health (setup/ping/dispose, per-job workers), add DB/JobQueue healthchecks and Sequelize startup hooks, update ports/configs, and adjust tests/imports. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor OS as Process/OS
participant S as server.ts
participant Child as Child Process
participant REST as rest-app.ts
participant BG as background-jobs-app.ts
OS->>S: start (PORT, INTERFACE)
alt INTERFACE = all
S->>Child: fork (INTERFACE=rest, PORT)
S->>Child: fork (INTERFACE=background-jobs, PORT+1)
Child-->>S: send "ready"
else INTERFACE = rest
S->>REST: import & bootstrap(PORT)
REST-->>S: process.send "ready"
else INTERFACE = background-jobs
S->>BG: import & bootstrap(PORT)
BG-->>S: process.send "ready"
end
OS->>S: SIGINT/SIGTERM
S->>Child: forward signal
sequenceDiagram
autonumber
participant App as Hono App
participant SS as startServer
participant DI as DI Container
participant Init as APP_INITIALIZER
participant SVR as HTTP Server
participant Proc as process events
SS->>App: beforeStart?()
SS->>DI: resolveAll(APP_INITIALIZER)
loop initializers
SS->>Init: ON_APP_START()
end
SS->>SVR: serve({fetch, port})
Proc->>SS: on SIGINT/SIGTERM -> shutdown()
SVR-->>SS: 'close' -> dispose container
sequenceDiagram
autonumber
participant Provider as jobs.provider
participant JQS as JobQueueService
participant H1 as Handler A
participant H2 as Handler B
Provider->>JQS: setup()
Provider->>JQS: registerHandlers([H1, H2])
par per-queue workers
JQS->>JQS: work(queue, batchSize=1, handler)
JQS-->>JQS: log JOB_STARTED(jobId)
alt success
JQS-->>JQS: log JOB_DONE(jobId)
else failure
JQS-->>JQS: log JOB_FAILED(jobId)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
stalniy
force-pushed
the
feat/background-jobs
branch
from
e55fcf7 to
812c2f0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
apps/api/test/functional/provider-deployments.spec.ts (1)
135-139: Replaceas anywith a typed response to satisfy repo TS rulesRepo guidelines prohibit
any. Define a response type and use it in both places.@@ -import { app, initDb } from "@src/rest-app"; +import { app, initDb } from "@src/rest-app"; + +type ProviderDeploymentsResponse = { + total: number; + deployments: Deployment[]; +}; @@ - const data = (await response.json()) as any; + const data = (await response.json()) as ProviderDeploymentsResponse; @@ - const data = (await response.json()) as any; + const data = (await response.json()) as ProviderDeploymentsResponse;Also applies to: 155-160
apps/api/test/functional/provider-earnings.spec.ts (1)
70-72: Avoidas anyin error-path assertionUse a minimal error shape instead of
anyto keep tests type-safe.@@ - const data = (await res.json()) as any; + type ErrorResponse = { message: string }; + const data = (await res.json()) as ErrorResponse;apps/api/test/functional/leases-duration.spec.ts (1)
134-140: Replaceanywith a proper typed response.Repo guidelines forbid
any. Define a response type and use it for JSON parsing.- const data = (await response.json()) as any; + const data = (await response.json()) as LeasesDurationResponse;Add this type near the imports to document the contract:
type LeasesDurationResponse = { leaseCount: number; totalDurationInSeconds: number; totalDurationInHours: number; leases: Array<{ dseq: string }>; };apps/api/test/functional/anonymous-user.spec.ts (2)
13-16: Avoidanywhen parsing the create-user response.Use the existing
AnonymousUserResponseOutputtype for JSON instead ofany.- const body = (await userResponse.json()) as any; + const body = (await userResponse.json()) as AnonymousUserResponseOutput;
55-56: Avoidanywhen parsing the "different user" response.Replace
anywith the response type.- const { token: differentUserToken } = (await differentUserResponse.json()) as any; + const { token: differentUserToken } = (await differentUserResponse.json()) as AnonymousUserResponseOutput;apps/api/test/functional/providers.spec.ts (1)
60-65: Await the instance updates to prevent race conditions.
Model.update(instance) returns a Promise; withoutawait, subsequent requests may read stale state.Apply this diff:
- providers[0].update({ + await providers[0].update({ lastSuccessfulSnapshotId: providerSnapshots[0].id }); - providers[1].update({ + await providers[1].update({ lastSuccessfulSnapshotId: providerSnapshots[2].id });apps/api/test/functional/provider-dashboard.spec.ts (1)
39-40: Removeas anycasts and type the response shape.
Per guidelines, avoidany. Define a local response type and cast explicitly.Apply this diff in-place to the two casts:
- const data = (await response.json()) as any; + const data = (await response.json()) as ProviderDashboardResponse; @@ - const data = (await response.json()) as any; + const data = (await response.json()) as ProviderDashboardResponse;Add this supporting type near the top of the file (outside the selected range):
type ProviderDashboardResponse = { current: { date: string; height: number }; previous: { date: string; height: number }; };Also applies to: 53-54
apps/api/src/healthz/controllers/healthz/healthz.controller.spec.ts (1)
51-56: Drop explicit return type fromsetup()to match test conventions.
Per repo guidelines, don’t specify the return type ofsetupin specs.- function setup(): { - controller: HealthzController; - service: MockProxy<HealthzService>; - } { + function setup() {apps/api/src/core/services/job-queue/job-queue.service.spec.ts (1)
116-120: Replaceanytypes inpgBoss.workmock to comply with “no any” guideline.Use precise types to avoid
anyin TS tests.- jest.spyOn(pgBoss, "work").mockImplementation(async (queueName: string, options: any, processFn: (jobs: any[]) => Promise<void>) => { - await processFn(jobs); + jest.spyOn(pgBoss, "work").mockImplementation(async (_queueName: string, _options: Parameters<PgBoss["work"]>[1], processFn: (jobs: EnqueuedJob[]) => Promise<void>) => { + await processFn(jobs as EnqueuedJob[]); return "work-id"; });Add once per file (near the bottom) to support the type:
type EnqueuedJob = { id: string; data: TestJob["data"] };
🧹 Nitpick comments (71)
apps/api/src/notifications/services/notification-handler/notification.handler.ts (3)
48-49: Type erasure viaas GenericNotificationTemplatehides template-specific var typesCasting each template to
GenericNotificationTemplatediscards the concretevarstype and lets mismatches slip through compile-time. Prefer localizing the cast behind a small invoker helper (overloaded) to limit the unsafe surface and document intent.Example helper (keeps casts contained; avoids
any):function invokeTemplate( t: (user: UserOutput) => CreateNotificationInput, user: UserOutput ): CreateNotificationInput; function invokeTemplate<V extends Record<string, unknown>>( t: (user: UserOutput, vars: V) => CreateNotificationInput, user: UserOutput, vars: V ): CreateNotificationInput; function invokeTemplate( t: (user: UserOutput, vars?: Record<string, unknown>) => CreateNotificationInput, user: UserOutput, vars?: Record<string, unknown> ): CreateNotificationInput { return t.length <= 1 ? t(user) : t(user, vars ?? {}); }Then in
handle:const t = notificationTemplates[payload.template]; await this.notificationService.createNotification( // TS will pick the correct overload; fallback `{}` remains enforced // @ts-expect-error allow union of function types via overload resolution invokeTemplate(t as any, user, (payload as any).vars) // if you keep the cast, keep it isolated here );If you’d prefer no
anyat all, keep the existing cast but still call throughinvokeTemplateso defaulting and arity handling are centralized.
68-72: Typo in event name: “MISTMATCH” → “MISMATCH”Minor, but it affects alerting/queries consistency.
- event: "USER_CONDITIONS_MISTMATCH", + event: "USER_CONDITIONS_MISMATCH",
37-47: Add tests for both arities (with/without vars) and conditions pathGiven the behavior change around
vars, add targeted tests to prevent regressions:
- Template with 2 params: when job omits
vars, handler should not throw and should call service with{}.- Template with 1 param: handler should ignore
varsand still create a notification.- Conditions mismatch: ensure early return with the corrected event name.
I can draft unit tests for
NotificationHandler.handleusing stub templates (1‑arg and 2‑arg) and a spy onNotificationService.createNotification. Want me to open a follow‑up PR or push to this branch?Also applies to: 57-66, 76-77
apps/api/tsconfig.json (1)
8-10: TS ≥5.3 Verified; No Breaking Imports Detected
- All workspace packages specify TypeScript
~5.8.2, satisfying the ≥ 5.3 requirement.- No
.ts-extension imports inapps/api—the newallowImportingTsExtensionsandrewriteRelativeImportExtensionsflags won’t affect existing code.apps/apiand itstest/tsconfig.jsonboth omitmoduleResolution, so editors and secondary tools default to CommonJS resolution despite"module": "NodeNext".Optional refactor for consistency across IDEs and tooling:
--- a/apps/api/tsconfig.json +++ b/apps/api/tsconfig.json @@ compilerOptions - "module": "NodeNext", + "module": "NodeNext", + "moduleResolution": "NodeNext", "allowImportingTsExtensions": true, "rewriteRelativeImportExtensions": truepackages/docker/docker-compose.prod.yml (1)
10-10: Background-jobs port mapping & healthcheck setup confirmed
- The background-jobs server binds to container port 3001 via the
backgroundJobsPort = port + 1logic inapps/api/src/server.ts(line 17), andstartServerinapps/api/src/background-jobs-app.tsdirectly uses that{ port }argument.- Health endpoints are already exposed by
healthzRouterin both the REST and background-jobs apps under/v1/healthz/readinessand/v1/healthz/liveness.Optional refactor—add a Docker Compose healthcheck so the
apiservice is marked healthy only when both the main API and background-jobs servers are ready:services: api: ports: - '3080:3000' - '3081:3001' # background jobs server + healthcheck: + test: ["CMD-SHELL", "curl -fsS http://localhost:3000/v1/healthz/readiness && curl -fsS http://localhost:3001/v1/healthz/readiness"] + interval: 10s + timeout: 3s + retries: 5apps/api/test/functional/provider-earnings.spec.ts (1)
27-33: Nit: stabilize timestamps in fixturesMultiple calls to
subDays(Date.now(), …)can drift by milliseconds. Prefer a single base time for determinism.- createAkashBlock({ - datetime: subDays(Date.now(), 1).toISOString(), + const now = new Date(); + createAkashBlock({ + datetime: subDays(now, 1).toISOString(), @@ - createAkashBlock({ - datetime: subDays(Date.now(), 2).toISOString(), + createAkashBlock({ + datetime: subDays(now, 2).toISOString(),apps/api/test/functional/blocks.spec.ts (1)
15-35: Refactor setup to avoid shared state and meet test guidelinesCurrent
setupdepends on outerisDbInitializedandtestData, violating the “no shared state in setup” rule; it’s also placed above the root describe body’s end, whereas guidelines require it at the bottom. Consider makingsetupself-contained, parameterized, and moving it to the bottom of the root describe.- const testData: { - blocks?: AkashBlock[]; - } = {}; - let isDbInitialized = false; - - const setup = async () => { - if (isDbInitialized) { - return testData; - } - - const validator = await createValidator(); - const baseTime = new Date(); - testData.blocks = await Promise.all( - Array.from({ length: 101 }, (_, i) => { - return createAkashBlock({ - height: i + 1, - proposer: validator.hexAddress, - datetime: subSeconds(baseTime, (101 - i) * blockFrequency) - }); - }) - ); - isDbInitialized = true; - return testData; - }; + // (move to bottom of root describe)And at the bottom of the root
describe("Blocks", …):+ // Setup must be at the bottom of the root describe and avoid shared state + const setup = async (_: { seed?: boolean } = { seed: true }) => { + const validator = await createValidator(); + const baseTime = new Date(); + const blocks = await Promise.all( + Array.from({ length: 101 }, (_, i) => + createAkashBlock({ + height: i + 1, + proposer: validator.hexAddress, + datetime: subSeconds(baseTime, (101 - i) * blockFrequency) + }) + ) + ); + return { blocks }; + };Note: If duplicate inserts are a concern due to unique constraints on height, switch to an idempotent seeding strategy (e.g., wrap createAkashBlock with an upsert-by-height).
apps/api/test/functional/certificate.spec.ts (1)
75-76: Fix suite description to match the actual endpoint.The suite title says "POST /v1/certificate/create" while the test hits "/v1/certificates". Align to reduce confusion in reports.
-describe("POST /v1/certificate/create", () => { +describe("POST /v1/certificates", () => {apps/api/test/functional/app.spec.ts (1)
20-21: Tighten version regex to avoid matching arbitrary characters.The dot (.) matches any character; escape it to match literal dots.
- version: expect.stringMatching(/^[0-9]+.[0-9]+.[0-9]+(-beta\.[0-9]+)?$/) + version: expect.stringMatching(/^[0-9]+\.[0-9]+\.[0-9]+(-beta\.[0-9]+)?$/)apps/api/test/functional/anonymous-user.spec.ts (1)
8-16: Consider moving setup to asetup()helper per repo test guidelines.Your
beforeEachthat creates a user can be converted into a localsetup()returning{ user, token }, called explicitly in each test. This improves isolation and follows the stated testing convention. Low priority for functional tests if you prefer the current style.apps/api/test/functional/wallets-refill.spec.ts (2)
22-71: Avoid mutating global BillingConfig without restoring it (test bleed risk).Mutating DI-provided config in-place can leak into other tests executed in the same Jest worker. Wrap the mutation in try/finally and restore the original value.
Apply this diff to scope and restore the change:
it("should refill wallets low on fee allowance", async () => { - config.FEE_ALLOWANCE_REFILL_THRESHOLD = 2; + const originalThreshold = config.FEE_ALLOWANCE_REFILL_THRESHOLD; + config.FEE_ALLOWANCE_REFILL_THRESHOLD = 2; + try { const NUMBER_OF_WALLETS = 5; const prepareRecords = Array.from({ length: NUMBER_OF_WALLETS }).map(async (_, index) => { const records = await walletService.createUserAndWallet(); const { user, token } = records; const { wallet } = records; let walletRecord = await userWalletRepository.findById(wallet.id); @@ return { user, token, wallet }; }); const records = await Promise.all(prepareRecords); await walletController.refillWallets(); const trialingWallet = records.pop()!; await Promise.all([ ...records.map(async ({ wallet }) => { const walletRecord = await userWalletRepository.findById(wallet.id); expect(walletRecord?.feeAllowance).toBe(config.FEE_ALLOWANCE_REFILL_AMOUNT); }), userWalletRepository.findById(trialingWallet.wallet.id).then(walletRecord => { expect(walletRecord?.feeAllowance).toBe(config.FEE_ALLOWANCE_REFILL_THRESHOLD); }) ]); - }); + } finally { + config.FEE_ALLOWANCE_REFILL_THRESHOLD = originalThreshold; + } +});
50-52: Assert against the updated DB record, not the stale in-memory wallet.
wallet.isTrialingdoesn’t reflect theupdateByIdmutation; assertwalletRecord.isTrialingagainst the intended boolean.Apply this diff:
- expect(walletRecord.feeAllowance).toBe(config.FEE_ALLOWANCE_REFILL_THRESHOLD); - expect(wallet.isTrialing).toBe(true); + expect(walletRecord.feeAllowance).toBe(config.FEE_ALLOWANCE_REFILL_THRESHOLD); + expect(walletRecord.isTrialing).toBe(index === NUMBER_OF_WALLETS - 1);apps/api/test/functional/sign-and-broadcast-tx.spec.ts (1)
47-48: Replaceas anywith a minimal response type.Comply with the guideline to avoid
anyin TS tests.Apply this diff:
- const { token } = (await differentUserResponse.json()) as any; + const { token } = (await differentUserResponse.json()) as { token: string };apps/api/test/functional/docs.spec.ts (1)
5-16: Ensure timers are restored even on failure.Wrap the assertions in try/finally so
useRealTimersalways executes.Apply this diff:
- it(`returns docs with all routes expected`, async () => { - jest.useFakeTimers(); - jest.setSystemTime(new Date("2025-07-03T12:00:00.000Z")); - - const response = await app.request(`/v1/doc`); - - expect(response.status).toBe(200); - const data = await response.json(); - expect(data).toMatchSnapshot(); - - jest.useRealTimers(); - }); + it(`returns docs with all routes expected`, async () => { + jest.useFakeTimers(); + jest.setSystemTime(new Date("2025-07-03T12:00:00.000Z")); + try { + const response = await app.request(`/v1/doc`); + expect(response.status).toBe(200); + const data = await response.json(); + expect(data).toMatchSnapshot(); + } finally { + jest.useRealTimers(); + } + });apps/api/test/functional/providers.spec.ts (3)
101-105: Avoidas anyin test parsing. Type the response.Use the domain type already imported.
Apply this diff:
- const data = (await response.json()) as any; + const data = (await response.json()) as Provider[];
110-114: Repeat: replaceas anywithProvider[].Same reasoning as above.
Apply this diff:
- const data = (await response.json()) as any; + const data = (await response.json()) as Provider[];
130-134: Replaceas anywith concrete type.Tighten types to comply with guidelines.
Apply this diff:
- const data = (await response.json()) as any; + const data = (await response.json()) as Provider;apps/api/test/functional/stripe-webhook.spec.ts (2)
34-47: Make STRIPE_WEBHOOK_SECRET deterministic in tests.The test fails if the env var isn’t set in CI. Set and restore it within this suite to avoid external coupling.
Apply this diff (inside the
describe("Stripe webhook", ...)block, near the top):describe("Stripe webhook", () => { + const originalStripeWebhookSecret = process.env.STRIPE_WEBHOOK_SECRET; + beforeAll(() => { + process.env.STRIPE_WEBHOOK_SECRET ??= "whsec_test"; + }); + afterAll(() => { + if (originalStripeWebhookSecret === undefined) { + delete process.env.STRIPE_WEBHOOK_SECRET; + } else { + process.env.STRIPE_WEBHOOK_SECRET = originalStripeWebhookSecret; + } + });This keeps signature generation aligned with the server’s verification logic.
66-82: Optionally assert nock expectations were consumed.Adds a stronger guarantee that the webhook handler performed the expected Stripe lookup.
Example (at the end of the test body):
expect(nock.isDone()).toBe(true);apps/api/test/functional/create-deployment.spec.ts (1)
94-98: Avoid live network IO in tests — use the existing api base or stub with nockThe test depends on an external endpoint, which can flake CI or fail offline. Prefer keeping all HTTP under
nockcontrol.Apply this diff to use the existing API base and keep calls mockable by
nock:- const response = await axios.get(`https://api.sandbox-01.aksh.pw/blocks/latest`); + // Keep this under nock's api base to avoid external IO in tests + const response = await axios.get(`${apiNodeUrl}/blocks/latest`);If
${apiNodeUrl}/blocks/latestisn’t the correct path for block height, either:
- add a dedicated height provider behind the same base, or
- stub axios:
nock(new URL(apiNodeUrl).origin).get('/blocks/latest').reply(200, { block: { header: { height: 123 }}})apps/api/src/app/index.ts (1)
1-2: Ensure DB init runs before job-queue setup (import order may matter)If APP_INITIALIZER execution order follows registration/import order, initialize DB before jobs to avoid transient startup failures.
Apply this diff to make the dependency explicit:
-import "./providers/jobs.provider"; -import "./providers/db-init.provider"; +import "./providers/db-init.provider"; +import "./providers/jobs.provider";If the DI container guarantees initializer ordering independently of import order, ignore the swap. Otherwise, add a small comment in jobs.provider asserting DB dependency or a runtime check in jobs setup.
apps/api/test/functional/templates.spec.ts (2)
14-15: Avoid leaking global env mutations across specsThe test overrides
env.GITHUB_PATbut never restores it. Capture and restore to prevent cross-test coupling.Apply this diff:
-env.GITHUB_PAT = "ghp_1234567890"; +const previousPat = env.GITHUB_PAT; +env.GITHUB_PAT = "ghp_1234567890"; @@ describe("Templates API", () => { - afterAll(() => { + afterAll(() => { jest.restoreAllMocks(); + // restore env to avoid side-effects on other specs + env.GITHUB_PAT = previousPat; });Also applies to: 39-42
17-37: Prefer DI-friendly mocking overjest.mockfor external clientsProject guidelines discourage
jest.mock()in specs. Long-term, consider injecting an Octokit client into the app/module and usingjest-mock-extendedto provide a mock in tests. For this functional spec, keeping the module mock is understandable; treat this as a backlog item.apps/api/test/functional/transactions.spec.ts (1)
25-25: Avoidanycasts; use concrete types already importedCasting responses to
anyviolates the typing guideline and loses safety. You already importTransaction; use it for the parsed payloads.Apply this diff:
- const transactionsFound = (await response.json()) as any; + const transactionsFound = (await response.json()) as Transaction[]; @@ - const transactionFound = (await response.json()) as any; + const transactionFound = (await response.json()) as Transaction;Also applies to: 49-49
apps/api/test/functional/balances.spec.ts (1)
47-48: Avoidas anyin tests; use typed response aliasesPer repo guidelines, don’t use
any. Define minimal response types for assertions and assert to those instead ofany.Example:
type BalancesResponse = { data: { balance: number; deployments: number; total: number }; }; // ... const result = (await response.json()) as BalancesResponse;Also applies to: 76-77
apps/api/test/functional/provider-attributes-schema.spec.ts (1)
8-9: Replaceas anywith a concrete schema typeKeep types explicit in spec files to follow the “no any” rule.
Example:
type ProviderAttribute = { key: string; type: string; required: boolean; description: string; values?: unknown[]; }; type ProviderAttributesSchemaResponse = Record<string, ProviderAttribute>; const data = (await response.json()) as ProviderAttributesSchemaResponse;apps/api/test/functional/bids.spec.ts (2)
31-60: Refactor frombeforeEachto a test-localsetup()functionGuidelines recommend a
setupfunction at the bottom of the rootdescribeinstead ofbeforeEach, with no shared state. MoveknownUsers/knownApiKeys/knownWalletsinsidesetup()and return helpers likemockUser/mockAdmin. Each test should callawait setup()and use the returned context.Sketch:
// At the bottom of the root describe function setup({ withAdmin = false }: { withAdmin?: boolean } = {}) { const knownUsers: Record<string, UserOutput> = {}; const knownApiKeys: Record<string, ApiKeyOutput> = {}; const knownWallets: Record<string, UserWalletOutput[]> = {}; // create typed mocks via jest-mock-extended if feasible and register via DI // provide mockUser, mockAdmin that close over the above maps return { knownUsers, knownApiKeys, knownWallets, mockUser, mockAdmin }; }If the route under test resolves dependencies from the DI container, consider creating typed mocks with
jest-mock-extendedand registering them viacontainer.registerInstance(...)for stricter typing and isolation.
129-136: Removeas anyfrom parsed JSONDefine a typed response for bids and assert to that type.
Example:
type BidsResponse = { data: Array<{ bid: string; escrow_account: string }> }; const result = (await response.json()) as BidsResponse; expect(result.data).toEqual([{ bid: "fake-bid", escrow_account: "fake-escrow-account" }]);Also applies to: 148-155, 168-175
apps/api/test/functional/provider-graph-data.spec.ts (1)
206-206: Replaceas anywith precise DTO types in testsCoding guidelines prohibit
anyin TS. Define lightweight DTO types and use them in assertions.Apply:
@@ -import { app, initDb } from "@src/rest-app"; +import { app, initDb } from "@src/rest-app"; + +type ProviderGraphNowCompare = { + count: number; + cpu: number; + gpu: number; + memory: number; + storage: number; +}; +type ProviderGraphData = { + currentValue: number; + compareValue: number; + snapshots: { date: string; value: number }[]; + now: ProviderGraphNowCompare; + compare: ProviderGraphNowCompare; +}; @@ - const data = (await response.json()) as any; + const data = (await response.json()) as ProviderGraphData; @@ - const data = (await response.json()) as any; + const data = (await response.json()) as ProviderGraphData;Also applies to: 286-286
apps/api/test/functional/market-data.spec.ts (1)
58-58: Introduce aMarketDataDTO inapps/api/test/functional/market-data.spec.ts
To satisfy the “noany” policy, define the exact response shape and replace eachas anycast with this type.Suggested changes:
@@ apps/api/test/functional/market-data.spec.ts -import { app } from "@src/rest-app"; +import { app } from "@src/rest-app"; + +type MarketData = { + price: number; + volume: number; + marketCap: number; + marketCapRank: number; + priceChange24h: number; + priceChangePercentage24: number; +}; @@ - const data = (await response.json()) as any; + const data = (await response.json()) as MarketData; @@ - const data = (await response.json()) as any; + const data = (await response.json()) as MarketData;• Both occurrences (lines 57 and 90) now use
MarketDatainstead ofany.
• A full-text scan (rg -nP '\bas\s+any\b' apps/api/test/functional) shows 30+ remainingas anycasts across other functional tests—consider defining minimal DTOs (or a generic parser helper) for each endpoint to eliminate those as well.apps/api/test/functional/api-key.spec.ts (4)
149-153: Replace “as any” with a precise response typeAvoid
anyin tests per guidelines. Narrow to the fields you assert.Apply:
- const result = (await response.json()) as any; + const result = (await response.json()) as { data: Array<{ keyFormat: string }> };
197-207: Type the GET-by-id response instead of casting to anyKeep types tight and local to what’s asserted.
- const result = (await response.json()) as any; + const result = (await response.json()) as { + data: { + id: string; + name: string; + expiresAt: string | null; + createdAt: string; + updatedAt: string; + apiKey?: string; + }; + };
244-251: Type the create response instead of using anyThis avoids implicit-
anyflows in expectations.- const result = (await response.json()) as any; + const result = (await response.json()) as { + data: { id: string; name: string; expiresAt: string; apiKey: string }; + };
362-369: Type the PATCH response instead of using anyKeep the type only to what you assert.
- const result = (await response.json()) as any; + const result = (await response.json()) as { data: { id: string; name: string } };apps/api/test/functional/provider-regions.spec.ts (1)
5-5: LGTM on switching to rest-app; mirror teardown like other suitesImport and usage look consistent. Consider closing DB connections here like in
dashboard-data.spec.tsto avoid pooled connection leaks when running tests in parallel.-import { app, initDb } from "@src/rest-app"; +import { app, initDb } from "@src/rest-app"; +import { closeConnections } from "@src/core"; @@ describe("ProviderRegions", () => { @@ beforeAll(async () => { await initDb(); @@ }); + + afterAll(async () => { + await closeConnections(); + });If a global test harness already disposes connections, feel free to skip this.
apps/api/src/app/providers/db-init.provider.ts (1)
11-13: Pass a DB-context logger or rely on the default to keep log context
connectUsingSequelizealready defaults toLoggerService.forContext("DB"). Passing a generic container-resolved logger may lose this DB context. Prefer relying on the default or passing the DB-context explicitly.- await connectUsingSequelize(c.resolve(LoggerService)); + // use default DB-context logger + await connectUsingSequelize(); + // or explicitly: + // await connectUsingSequelize(LoggerService.forContext("DB"));apps/api/test/functional/dashboard-data.spec.ts (1)
5-5: Import switch to rest-app is consistent; keep types tight in assertionsThe entrypoint change is good. One small clean-up: avoid
as anyon the parsed JSON.- const data = (await response.json()) as any; + type BlockStats = { + height: number; + date: string; + activeLeaseCount?: number | null; + totalLeaseCount?: number | null; + activeCPU?: number | null; + activeGPU?: number | null; + activeMemory?: number | null; + activeEphemeralStorage?: number | null; + activePersistentStorage?: number | null; + totalUAktSpent?: number | null; + totalUUsdSpent?: number | null; + totalUUsdcSpent?: number | null; + dailyLeaseCount?: number | null; + dailyUAktSpent?: number | null; + dailyUUsdSpent?: number | null; + dailyUUsdcSpent?: number | null; + }; + const data = (await response.json()) as { now: BlockStats; compare: BlockStats };apps/api/test/functional/provider-dashboard.spec.ts (2)
18-31: Use Date objects in seed data to match model typing and avoid runtime mismatches.
Factories/seeders often persist and returnDateinstances; passing ISO strings can causetoISOString()calls to fail at runtime if the seeder does not re-hydrate toDate. Prefer passingDatedirectly.- createAkashBlock({ - datetime: new Date().toISOString(), + createAkashBlock({ + datetime: new Date(), isProcessed: true, totalUUsdSpent: 1000, height: 300 }), - createAkashBlock({ - datetime: subDays(Date.now(), 1).toISOString(), + createAkashBlock({ + datetime: subDays(Date.now(), 1), height: 200 }), - createAkashBlock({ - datetime: subDays(Date.now(), 2).toISOString(), + createAkashBlock({ + datetime: subDays(Date.now(), 2), height: 100 })If
createAkashBlockexpects strings, keep current inputs but ensure it returnsBlockwithdatetime: Date. Otherwise, adopt the diff above.
9-33: Optional: adopt a localsetup()to align with the repo’s test pattern.
Current use ofbeforeAllis fine functionally, but tests in this repo generally favor asetup()at the bottom of the rootdescribe. Consider migrating to keep consistency.apps/api/test/functional/user-init.spec.ts (2)
15-19: Avoidjest.mock()module mocking in specs; prefer DI + jest-mock-extended.
Repo guideline: don’t usejest.mock()in test files. For functional tests, consider exposing an injectable token for user context so the test can register a stub rather than mocking the module.High-level direction:
- Introduce a DI token (e.g., USER_CONTEXT) wrapping
getCurrentUserId.- In production, register the real provider; in tests, register a mock via
jest-mock-extended.Example sketch (not a complete diff):
// production module (e.g., user-context.provider.ts) export interface UserContext { getCurrentUserId(): string | undefined } export const USER_CONTEXT = Symbol('USER_CONTEXT'); // wiring (app startup) container.register<UserContext>(USER_CONTEXT, { useValue: { getCurrentUserId } }); // usage (middlewares) const { getCurrentUserId } = container.resolve<UserContext>(USER_CONTEXT);// test import { mock } from 'jest-mock-extended'; const userCtx = mock<UserContext>(); userCtx.getCurrentUserId.mockReturnValue(auth0Payload.userId); container.register(USER_CONTEXT, { useValue: userCtx });I can help draft the provider and swap the middleware to consume it—want me to open a follow-up patch?
89-127: Conformsetup()to test conventions (param signature and placement).
- The setup function should accept a single parameter with an inline type and be placed at the bottom of the root
describe(it’s currently nested).- Also useful to allow payload overrides.
Minimal delta to the signature and body:
- function setup() { + function setup(options: { + auth0PayloadOverride?: Partial<{ + userId: string; + wantedUsername: string; + email: string; + emailVerified: boolean; + subscribedToNewsletter: boolean; + }>; + } = {}) { const auth0Payload = { userId: faker.string.alphanumeric(10), wantedUsername: faker.internet.userName(), email: faker.internet.email(), emailVerified: true, subscribedToNewsletter: false }; + Object.assign(auth0Payload, options.auth0PayloadOverride);And move this
setupto the bottom of the rootdescribe("User Init", ...)block.apps/api/src/core/providers/app-initializer.ts (1)
10-10: Good addition: APP_SHUTDOWN token for graceful teardown.
This enables clean disposal of shared resources (DB, queues, etc.). Consider introducing a named type for reuse across providers.Apply this small refinement:
+export interface AppShutdown { + dispose: () => void | Promise<void>; +} -export const APP_SHUTDOWN: InjectionToken<{ dispose: () => void | Promise<void> }> = Symbol("APP_SHUTDOWN"); +export const APP_SHUTDOWN: InjectionToken<AppShutdown> = Symbol("APP_SHUTDOWN");apps/api/src/app/providers/jobs.provider.ts (1)
9-21: Consider registering a shutdown hook to drain/close the job queue.
WithAPP_SHUTDOWNnow available, it’s a good place to ensure workers stop cleanly and transports are closed.Sketch (if
JobQueueServiceexposesdispose()orclose()):import { APP_SHUTDOWN } from "@src/core/providers/app-initializer"; container.register(APP_SHUTDOWN, { useValue: { async dispose() { const jobQueueManager = container.resolve(JobQueueService); await jobQueueManager.dispose?.(); // or close()/stop() } } });If
JobQueueServicedoesn’t yet have a shutdown method, I can add one.apps/api/src/healthz/controllers/healthz/healthz.controller.spec.ts (1)
12-14: Rename tests to reflect both checks (postgres + jobQueue).
Titles still say “postgres is ready” but now the payload includesjobQueue. Consider making the names precise, e.g., “should return ok if dependencies are healthy”.Also applies to: 33-35
apps/api/src/background-jobs-app.ts (1)
11-17: Consider initializing prerequisites inbeforeStartfor atomic boot or via an app initializer.If workers depend on DB migrations or other prerequisites, prefer moving the worker startup (or its prerequisites) into
beforeStart(or an APP_INITIALIZER) so failure aborts the boot atomically and shutdown handlers are registered bystartServer.Example:
- await startServer(app, LoggerService.forContext("BACKGROUND_JOBS"), process, { port }); - await container.resolve(JobQueueService).startWorkers(); + await startServer(app, LoggerService.forContext("BACKGROUND_JOBS"), process, { + port, + beforeStart: async () => { + // e.g. await migratePG(); // if required here as well + await container.resolve(JobQueueService).startWorkers(); + } + });apps/api/src/server.ts (3)
5-5: Add top-level error handling to avoid unhandled rejections on boot failure.If any boot task throws (e.g., worker startup moved to beforeStart), process will crash with an unhandled rejection. Make it explicit and exit non-zero.
-bootstrap(); +void bootstrap().catch(error => { + // Keep this minimal; logging provider may not be ready yet. + // eslint-disable-next-line no-console + console.error("FATAL_BOOT_ERROR", error); + process.exit(1); +});
9-9: Typo:boostrapList→bootstrapList.Minor readability fix.
- const boostrapList: Promise<void>[] = []; + const bootstrapList: Promise<void>[] = [];And update subsequent references:
- boostrapList.push(import("./rest-app.ts").then(m => m.bootstrap(port))); + bootstrapList.push(import("./rest-app.ts").then(m => m.bootstrap(port)));- boostrapList.push(import("./background-jobs-app.ts").then(m => m.bootstrap(backgroundJobsPort))); + bootstrapList.push(import("./background-jobs-app.ts").then(m => m.bootstrap(backgroundJobsPort)));- await Promise.all(boostrapList); + await Promise.all(bootstrapList);
16-19: Allow an explicit BACKGROUND_JOBS_PORT override (fallback toport + 1).Helpful in containerized envs where
port+1may already be allocated.- if (INTERFACE === "background-jobs" || INTERFACE === "all") { - const backgroundJobsPort = INTERFACE === "all" ? port + 1 : port; + if (INTERFACE === "background-jobs" || INTERFACE === "all") { + const { BACKGROUND_JOBS_PORT } = process.env; + const backgroundJobsPort = + BACKGROUND_JOBS_PORT ? parseInt(BACKGROUND_JOBS_PORT, 10) : (INTERFACE === "all" ? port + 1 : port); boostrapList.push(import("./background-jobs-app.ts").then(m => m.bootstrap(backgroundJobsPort))); }apps/api/test/functional/addresses.spec.ts (2)
214-216: Avoid reconnecting DB on every test run; connect once for speedCalling connectUsingSequelize() inside setup runs per test invocation and can add noticeable overhead. Prefer connecting once in beforeAll (and keeping your afterAll as-is) or memoize the connection in setup to ensure idempotency.
Example refactor outside this hunk:
describe("Addresses API", () => { beforeAll(async () => { await connectUsingSequelize(); }); // ... keep your tests & setup() without the connectUsingSequelize() call });
214-535: Minor guideline nit: setup() with a single paramGuidelines say setup should accept a single parameter with inline type. It’s fine functionally as-is, but for consistency you can accept a typed options param (even if unused) and return the created fixtures.
Possible tweak outside this hunk:
const setup = async (_opts: { seed?: boolean } = {}) => { // existing body return { address, validators, transactions }; };apps/api/src/core/providers/job-queue-healthcheck.ts (1)
9-17: Prefer a unique Symbol token and add explicit factory typeUsing a string token can collide. A Symbol guarantees uniqueness, and typing the factory helps keep the contract tight.
Apply this diff:
-export const JOB_QUEUE_HEALTHCHECK: InjectionToken<JobQueueHealthcheck> = "JOB_QUEUE_HEALTHCHECK"; -container.register(JOB_QUEUE_HEALTHCHECK, { - useFactory: c => { +export const JOB_QUEUE_HEALTHCHECK: InjectionToken<JobQueueHealthcheck> = Symbol("JOB_QUEUE_HEALTHCHECK"); +container.register<JobQueueHealthcheck>(JOB_QUEUE_HEALTHCHECK, { + useFactory: (c): JobQueueHealthcheck => { const jobQueueService = c.resolve(JobQueueService); return { ping: () => jobQueueService.ping() }; } });apps/api/src/console.ts (1)
23-23: Run APP_SHUTDOWN disposers on shutdown for symmetry with APP_INITIALIZERexecuteCliHandler triggers APP_INITIALIZER on start. For symmetry and future-proof cleanup (other providers may register APP_SHUTDOWN), run all APP_SHUTDOWN.dispose hooks during shutdown before disposing the container.
Apply this diff:
-import { APP_INITIALIZER, ON_APP_START } from "./core/providers/app-initializer"; +import { APP_INITIALIZER, ON_APP_START, APP_SHUTDOWN } from "./core/providers/app-initializer";const shutdown = once(async () => { // eslint-disable-next-line @typescript-eslint/no-var-requires const { closeConnections } = require("./core/providers/postgres.provider"); - await Promise.all([closeConnections(), chainDb.close(), container.dispose()]); + const shutdownHooks = container.resolveAll(APP_SHUTDOWN).map(h => h.dispose()); + await Promise.all([closeConnections(), chainDb.close(), ...shutdownHooks]); + await container.dispose(); });Also applies to: 133-138
apps/api/src/core/providers/postgres.provider.ts (1)
29-37: DbHealthcheck ping should conform to PromiseThe useValue currently returns the result of unsafe("SELECT 1"), which resolves to a result set. Make it explicit Promise for type correctness and to match the JobQueueHealthcheck shape.
Apply this diff:
container.register(DB_HEALTHCHECK, { useValue: { - ping: () => appClient.unsafe("SELECT 1") + ping: () => appClient.unsafe("SELECT 1").then(() => undefined) } });apps/api/src/core/services/job-queue/job-queue.service.ts (3)
28-43: Single-handler enforcement per queue is good; consider early throw before side-effectsThe duplicate-handler guard is solid. Minor improvement: detect duplicates before calling createQueue to avoid creating a queue and then failing on a later handler.
Potential refactor sketch:
const names = handlers.map(h => h.accepts[JOB_NAME]); const dup = names.find((n, i) => names.indexOf(n) !== i); if (dup) throw new Error(`JobQueue does not support multiple handlers for the same queue: ${dup}`); // then createQueue in a single pass
144-147: ping() implementation is fine; consider using the public API when availableUsing getDb().executeSql("SELECT 1") is pragmatic, but it relies on a non-typed API. If PgBoss exposes a public liveness method in future, prefer it. No change required now.
89-91: Doc: clarify that startWorkers blocksstartWorkers awaits the worker promises, so the returned promise won’t resolve until workers stop. Consider documenting that it’s intended for a long-lived background process and will block.
Suggested doc tweak:
/** Starts jobs processing (long-running; resolves when workers are stopped) */apps/api/src/db/dbConnection.ts (3)
66-73: Disambiguate success logs for chain vs user DB.Both success logs say “Connection has been established successfully.” Consider making them specific to avoid ambiguity in logs.
- logger.debug("Connection has been established successfully."); + logger.debug("Chain DB connection established."); @@ - logger.debug("Connection has been established successfully."); + logger.debug("User DB connection established.");Also applies to: 70-73
59-65: Docblock over-promises relative to current implementation.The comment mentions populate, per-version backups, and loading from backups; the function currently authenticates and syncs user schema only. Align doc with reality or implement the missing steps.
/** - * Initialize database schema - * Populate db - * Create backups per version - * Load from backup if exists for current version + * Initialize database connections and sync user schema. + * Note: population and backups are out of scope here and handled elsewhere. */
21-21: Risk: pg int8 parsing to JS number can lose precision.
pg.defaults.parseInt8 = truecoerces int8 to Number, which is unsafe for values > 2^53-1. If any int8 columns (e.g., heights, IDs) can exceed this, prefer parsing to BigInt or string.Example:
import pg from "pg"; // 20 = INT8 OID pg.types.setTypeParser(20, (val: string) => BigInt(val));Check whether any
int8columns inchainModels/userModelscan exceed JS safe integer range.apps/api/src/core/services/start-server/start-server.ts (2)
53-56: PreferbeforeExitoverexitfor async cleanup or drop theexithook.The
exitevent doesn’t allow async work; yourshutdown()awaits async tasks. UsebeforeExitor rely onSIGTERM/SIGINT+ serverclose.- processEvents.on("exit", shutdown); + processEvents.on("beforeExit", shutdown);
47-52: Add a “SERVER_STARTED” log with the actual bound port/host.When
port: 0,SERVER_STARTINGlogslocalhost:0. Log once the server is listening with the real address.server = serve({ fetch: app.fetch, port: options.port }); + // Log actual address once the server is listening + server.once("listening", () => { + const addr = server.address(); + const host = typeof addr === "string" ? addr : addr?.address ?? "0.0.0.0"; + const port = typeof addr === "string" ? options.port : addr?.port; + logger.info({ event: "SERVER_STARTED", url: `http://${host}:${port}` }); + });apps/api/src/core/services/start-server/start-server.spec.ts (2)
14-16: Await server close in afterEach to prevent race conditions between tests.Ensures the port is released and listeners are cleaned up before the next test runs.
- afterEach(() => { - startedServer?.close(); - }); + afterEach(async () => { + if (startedServer) { + await new Promise<void>(resolve => startedServer!.close(() => resolve())); + startedServer = undefined; + } + });
127-135: Optional: assertSERVER_START_ERRORis logged on failures.You already assert disposal on errors. Adding an assertion on
logger.error({ event: "SERVER_START_ERROR", ... })would fully validate the error path.Also applies to: 136-144
apps/api/src/healthz/services/healthz/healthz.service.spec.ts (1)
103-126: Stabilize TTL test by enabling fake timers before scheduling.Switching to fake timers mid-test can be flaky if the cache uses timers scheduled earlier. Enable fake timers at the start, then restore at the end.
- // First call - both should be called - await service.getLivenessStatus(); + jest.useFakeTimers(); + // First call - both should be called + await service.getLivenessStatus(); @@ - // Advance time beyond TTL - jest.useFakeTimers(); + // Advance time beyond TTL jest.advanceTimersByTime(millisecondsInMinute + 1); @@ - jest.useRealTimers(); + jest.useRealTimers();apps/api/src/rest-app.ts (3)
15-19: Ensure error handler retains class context when passed to HonoYou resolve
HonoErrorHandlerServiceand later pass.handletoappHono.onError. Ifhandleusesthis, it will lose context when passed unbound. Prefer binding the instance before registering, or wrap in a lambda.Example (outside this hunk):
const errorHandler = container.resolve(HonoErrorHandlerService); appHono.onError(errorHandler.handle.bind(errorHandler)); // or appHono.onError((err, c) => errorHandler.handle(err, c));
186-192: Make scheduler start resilient and observableStarting the scheduler during
APP_INITIALIZERwill fail the whole startup if it throws. If that’s intended — fine. Otherwise, guard it and add structured logs for operability.container.register(APP_INITIALIZER, { useValue: { async [ON_APP_START]() { - scheduler.start(); + try { + const logger = LoggerService.forContext("Scheduler"); + logger.info({ event: "SCHEDULER_STARTING" }); + scheduler.start(); + logger.info({ event: "SCHEDULER_STARTED" }); + } catch (error) { + // If the API should still boot without background jobs, demote to warning. + LoggerService.forContext("Scheduler").error({ event: "SCHEDULER_START_ERROR", error }); + // Optionally rethrow to keep current fail-fast behavior. + // throw error; + } } } satisfies AppInitializer });
196-201: Return the server frombootstrapfor testability and graceful shutdown
startServerreturnsServerType | undefinedbutbootstrapdiscards it. Returning it enables tests and CLIs to close the server and await shutdown.-export async function bootstrap(port: number) { - await startServer(appHono, LoggerService.forContext("APP"), process, { - port, - beforeStart: migratePG - }); -} +export async function bootstrap(port: number) { + return startServer(appHono, LoggerService.forContext("APP"), process, { + port, + beforeStart: migratePG + }); +}apps/api/src/healthz/services/healthz/healthz.service.ts (4)
30-40: Revisit liveness semantics: avoid coupling to external dependenciesLiveness typically answers “is the process healthy” and should be cheap and independent of downstreams; readiness should gate dependencies. Making liveness depend on DB and job queue can cause restart flaps during transient outages.
Options:
- Make
getLivenessStatus()always return ok (plus simple self-checks like event loop lag, heap thresholds).- Keep dependency checks only in readiness, or make them best-effort/soft-fail for liveness.
Would you like a patch moving DB/JobQueue checks to readiness only?
42-50: Don’t cache failures (or shorten TTL) for liveness pathsMemoizing liveness for 60s delays recovery reporting: after DB/queue recovers, pods may stay “error” for up to 60s. Either:
- Cache only success, or
- Use a much smaller TTL for liveness (e.g., 5–10s), or
- Drop caching for liveness entirely.
Minimal change:
-@Memoize({ ttlInSeconds: secondsInMinute }) +@Memoize({ ttlInSeconds: 10 }) private async isPostgresAlive() { return this.isPostgresHealthy(); } -@Memoize({ ttlInSeconds: secondsInMinute }) +@Memoize({ ttlInSeconds: 10 }) private async isJobQueueAlive() { return this.isJobQueueHealthy(); }If you prefer “cache only success,” I can draft a
MemoizeOnSuccessdecorator that bypasses caching on false.
52-63: DB health ping with structured error logging: LGTMClear error event and boolean collapse are appropriate. Consider including a short error class/name to ease alert rule matching, but current shape is fine.
65-76: Add explicit return type for consistency and clarity
isJobQueueHealthyreturns a boolean but lacks an explicit return type, unlike the DB counterpart.-private async isJobQueueHealthy() { +private async isJobQueueHealthy(): Promise<boolean> { try { await this.jobQueueHealthcheck.ping(); return true; } catch (error) { this.logger.error({ event: "JOB_QUEUE_HEALTHCHECK_ERROR", error }); return false; } }
apps/api/src/notifications/services/notification-handler/notification.handler.ts
Show resolved
Hide resolved
stalniy
force-pushed
the
feat/background-jobs
branch
from
812c2f0 to
e569b3c
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/core/services/job-queue/job-queue.service.ts (1)
76-87: Avoid logging entire job payload — potential PII leakage. Log only non-sensitive metadata.
jobmay include emails or other PII (see Notification jobs). Logging the full payload at INFO risks sensitive data exposure.Apply this diff to log minimal fields after send:
- async enqueue(job: Job, options?: EnqueueOptions): Promise<string | null> { - this.logger.info({ - event: "JOB_ENQUEUED", - job - }); - - return await this.pgBoss.send({ + async enqueue(job: Job, options?: EnqueueOptions): Promise<string | null> { + const jobId = await this.pgBoss.send({ name: job.name, data: { ...job.data, version: job.version }, options }); + this.logger.info({ + event: "JOB_ENQUEUED", + queue: job.name, + jobVersion: job.version, + jobId + }); + return jobId; }
♻️ Duplicate comments (5)
apps/api/src/core/services/job-queue/job-queue.service.ts (1)
99-121: Handler callback destructures as array while forcing batchSize=1 — will throw at runtime. Normalize payload shape.pg-boss calls the handler with a single job when batchSize=1, not an array. Destructuring
([job])risks “job is not iterable”. Normalize to both shapes.Apply this diff:
- const workersPromises = Array.from({ length: options.batchSize ?? 10 }).map(() => - this.pgBoss.work<Job["data"]>(queueName, workerOptions, async ([job]) => { + const workersPromises = Array.from({ length: options.batchSize ?? 10 }).map(() => + this.pgBoss.work<Job["data"]>(queueName, workerOptions, async (jobOrJobs) => { + const [job] = Array.isArray(jobOrJobs) ? jobOrJobs : [jobOrJobs]; this.logger.info({ event: "JOB_STARTED", jobId: job.id }); try { await handler.handle(job.data); this.logger.info({ event: "JOB_DONE", jobId: job.id }); } catch (error) { this.logger.error({ event: "JOB_FAILED", jobId: job.id, error }); throw error; } - }) + }) ); await Promise.all(workersPromises);apps/api/src/core/services/job-queue/job-queue.service.spec.ts (3)
109-136: Type-safe pgBoss.work mock and job payload; remove anys.Replace
anyusages in the mock with concrete types to comply with repo guidelines and improve safety.Apply this diff:
- const jobs = [{ id: "1", data: { message: "Job 1", userId: "user-1" } }]; - - jest.spyOn(pgBoss, "work").mockImplementation(async (queueName: string, options: any, processFn: (jobs: any[]) => Promise<void>) => { - await processFn(jobs); + const jobs: EnqueuedJob[] = [{ id: "1", data: { message: "Job 1", userId: "user-1" } }]; + + jest.spyOn(pgBoss, "work").mockImplementation(async (_queueName: string, _options: Parameters<PgBoss["work"]>[1], processFn: (jobs: EnqueuedJob[]) => Promise<void>) => { + await processFn(jobs as EnqueuedJob[]); return "work-id"; });Add this at the top-level of the file (outside tests) to define EnqueuedJob:
type EnqueuedJob = { id: string; data: TestJob["data"] };
142-166: Type-safe failure path mock; remove anys.Mirror the typing fix in the failure path.
Apply this diff:
- const jobs = [{ id: "1", data: { message: "Job 1", userId: "user-1" } }]; - - jest.spyOn(pgBoss, "work").mockImplementation(async (queueName: string, options: any, processFn: (jobs: any[]) => Promise<void>) => { - await processFn(jobs); + const jobs: EnqueuedJob[] = [{ id: "1", data: { message: "Job 1", userId: "user-1" } }]; + + jest.spyOn(pgBoss, "work").mockImplementation(async (_queueName: string, _options: Parameters<PgBoss["work"]>[1], processFn: (jobs: EnqueuedJob[]) => Promise<void>) => { + await processFn(jobs as EnqueuedJob[]); return "work-id"; });
168-183: Type-safe default-options mock; remove anys.Keep mocks consistently typed.
Apply this diff:
- jest.spyOn(pgBoss, "work").mockImplementation(async (queueName: string, options: any, processFn: (jobs: any[]) => Promise<void>) => { - await processFn([{ id: "1", data: { message: "Job 1", userId: "user-1" } }]); + jest.spyOn(pgBoss, "work").mockImplementation(async (_queueName: string, _options: Parameters<PgBoss["work"]>[1], processFn: (jobs: EnqueuedJob[]) => Promise<void>) => { + await processFn([{ id: "1", data: { message: "Job 1", userId: "user-1" } }] as EnqueuedJob[]); return "work-id"; });apps/api/src/rest-app.ts (1)
194-194: Alias exportinitDb→connectUsingSequelize: ensure callers use the public aliasGood transitional surface. As noted previously, please replace any lingering direct imports of
connectUsingSequelizewithinitDbunless a custom logger is required.#!/bin/bash # Find any direct imports/usages bypassing the alias rg -nP --type ts -C2 "\b(connectUsingSequelize)\b" apps/api
🧹 Nitpick comments (15)
apps/api/env/.env.unit.test (1)
28-28: dotenv-linter UnorderedKey warning — optional cleanupThe linter flags key ordering; consider alphabetizing keys in this file (or all env.test files) to silence the warning. Keep it as a separate housekeeping commit to avoid noisy diffs.
Minimal change (move the key near the top before POSTGRES_DB_URI):
-NOTIFICATIONS_API_BASE_URL=http://localhost:3090 +NOTIFICATIONS_API_BASE_URL=http://localhost:3090Alternatively, standardize on global alphabetical ordering across the file(s) in a follow-up.
apps/api/src/core/services/shutdown-server/shutdown-server.spec.ts (5)
7-101: Adopt the repository’s setup() testing pattern to reduce duplication and align with guidelines.Per the project’s spec guidelines, use a setup function at the bottom of the root describe to construct typed mocks and parameterize server behavior. This removes repeated mock wiring and improves readability.
Example refactor (sketch; keep existing imports):
describe(shutdownServer.name, () => { it("closes the server", async () => { const { server, appLogger, onShutdown } = setup(); await shutdownServer(server, appLogger, onShutdown); expect(server.close).toHaveBeenCalledTimes(1); expect(onShutdown).toHaveBeenCalledTimes(1); expect(appLogger.error).not.toHaveBeenCalled(); }); it("logs error if server close fails", async () => { const error = new Error("Failed to close server"); const { server, appLogger, onShutdown } = setup({ closeBehavior: "callbackError", error }); await shutdownServer(server, appLogger, onShutdown); expect(appLogger.error).toHaveBeenCalledWith({ event: "SERVER_CLOSE_ERROR", error }); expect(onShutdown).toHaveBeenCalled(); }); it("logs error if server close throws", async () => { const error = new Error("Failed to close server"); const { server, appLogger, onShutdown } = setup({ closeBehavior: "throw", error }); await shutdownServer(server, appLogger, onShutdown); expect(appLogger.error).toHaveBeenCalledWith({ event: "SERVER_CLOSE_ERROR", error }); expect(onShutdown).toHaveBeenCalled(); }); it("logs error if onShutdown callback fails", async () => { const error = new Error("Failed to dispose container"); const { server, appLogger } = setup(); const onShutdown = jest.fn().mockRejectedValue(error); await shutdownServer(server, appLogger, onShutdown); expect(appLogger.error).toHaveBeenCalledWith({ event: "ON_SHUTDOWN_ERROR", error }); }); it("calls shutdown directly if server is not listening", async () => { const { server, appLogger, onShutdown } = setup({ listening: false }); await shutdownServer(server, appLogger, onShutdown); expect(server.close).not.toHaveBeenCalled(); expect(onShutdown).toHaveBeenCalled(); }); it("calls server.close if server is listening", async () => { const { server, appLogger, onShutdown } = setup({ listening: true }); await shutdownServer(server, appLogger, onShutdown); expect(server.close).toHaveBeenCalled(); expect(onShutdown).toHaveBeenCalled(); }); function setup({ listening = true, closeBehavior = "success" as "success" | "callbackError" | "throw", error = new Error("close failed") }: { listening?: boolean; closeBehavior?: "success" | "callbackError" | "throw"; error?: Error; } = {}) { const appLogger = mock<Logger>(); const onShutdown = jest.fn(); const server = mock<ServerType>({ listening, close: closeBehavior === "throw" ? (jest.fn().mockImplementation(() => { throw error; }) as unknown as ServerType["close"]) : jest .fn() .mockImplementation((cb: (err?: Error) => void) => cb(closeBehavior === "callbackError" ? error : undefined) ) }); return { server, appLogger, onShutdown, error }; } });
32-35: De-duplicate event key literals (SERVER_CLOSE_ERROR).To avoid future drift when event keys change, reference a constant instead of repeating string literals in assertions.
Apply within this range:
- expect(appLogger.error).toHaveBeenCalledWith({ - event: "SERVER_CLOSE_ERROR", - error - }); + expect(appLogger.error).toHaveBeenCalledWith({ + event: EVENTS.SERVER_CLOSE_ERROR, + error + });Add once near the imports (outside this range):
const EVENTS = { SERVER_CLOSE_ERROR: "SERVER_CLOSE_ERROR", ON_SHUTDOWN_ERROR: "ON_SHUTDOWN_ERROR" } as const;
51-54: Keep using constants for event keys here as well.- expect(appLogger.error).toHaveBeenCalledWith({ - event: "SERVER_CLOSE_ERROR", - error - }); + expect(appLogger.error).toHaveBeenCalledWith({ + event: EVENTS.SERVER_CLOSE_ERROR, + error + });
68-71: Likewise, use a constant for ON_SHUTDOWN_ERROR.- expect(appLogger.error).toHaveBeenCalledWith({ - event: "ON_SHUTDOWN_ERROR", - error - }); + expect(appLogger.error).toHaveBeenCalledWith({ + event: EVENTS.ON_SHUTDOWN_ERROR, + error + });
42-45: Tighten typing of the “throws” mock to avoid broad jest.Mock cast.Prefer casting to the precise function type (ServerType["close"]) rather than generic jest.Mock.
- close: jest.fn().mockImplementation(() => { - throw error; - }) as jest.Mock + close: (jest.fn().mockImplementation(() => { + throw error; + }) as unknown) as ServerType["close"]apps/api/webpack.dev.js (1)
11-18: Refactor webpack entry configuration to a single orchestratorVerification confirms that neither rest-app.ts nor background-jobs-app.ts invoke any bootstrap or startServer calls at module load—only server.ts does (via bootstrap(process.env))—so the current multi-entry array won’t inadvertently spin up multiple servers. However, bundling all three modules as a single “multi-main” entry:
• duplicates imports (server.ts already imports both sub-apps)
• forces webpack to include and execute every top-level import in each file
• adds cognitive overhead and potential for subtle side-effects (e.g., duplicate DI provider registrations)To simplify and future-proof:
• Replace the array entry with a single orchestrator entry in apps/api/webpack.dev.js (lines 11–18):
- entry: ["./src/server.ts", "./src/rest-app.ts", "./src/background-jobs-app.ts"], + entry: { server: "./src/server.ts" }, … - filename: "server.js" + filename: "[name].js"• If you genuinely need separate bundles for each sub-app (but only run the “server” bundle in development), use named entries instead:
- entry: ["./src/server.ts", "./src/rest-app.ts", "./src/background-jobs-app.ts"], + entry: { + server: "./src/server.ts", + rest: "./src/rest-app.ts", + jobs: "./src/background-jobs-app.ts" + }, … - filename: "server.js" + filename: "[name].js"This refactor removes redundant entry points, clarifies execution order, and keeps your bundle aligned with the single-orchestrator pattern.
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (2)
121-136: Assertion will need updating if you adopt explicit concurrency.If you switch startWorkers to use
{ concurrency }, update this test to callstartWorkers({ concurrency: 5 })and keepworkerOptions.batchSizeat 1.I can provide the exact diff for all affected tests if you accept the concurrency change in the service.
230-274: Place setup() at the bottom of the root describe per test guidelines.Your setup function is above class helpers; move it to be the last item inside the root describe.
apps/api/test/functional/user-init.spec.ts (1)
11-19: Avoid jest.mock in spec files; prefer dependency injection or explicit test composition.Guidelines: don’t use
jest.mock()in spec files. Instead, pass mocks via DI or a test composition function. Since this is a functional test, two options:
- Expose an overridable user context provider (e.g., token via tsyringe) that your user middleware consumes; in tests, register a mock with
container.registerInstance(...).- Expose a
createRestApp({ userContextProvider })factory for tests to inject a stub without module-level mocking.I can sketch a minimal UserContextProvider token and wire it through the middleware so this test can replace the dependency without
jest.mock().apps/api/src/core/providers/postgres.provider.ts (1)
33-33: Prefer Symbol tokens to minimize collision risk across containers/modulesString tokens work but are easier to collide with. Consider switching to
Symbol("DB_HEALTHCHECK")(as is already done elsewhere for APP_INITIALIZER/ON_APP_START).-export const DB_HEALTHCHECK: InjectionToken<DbHealthcheck> = "DB_HEALTHCHECK"; +export const DB_HEALTHCHECK: InjectionToken<DbHealthcheck> = Symbol("DB_HEALTHCHECK");apps/api/src/server.ts (3)
33-37: Remove debug console.log or route through LoggerServiceLeaking
process.sendin logs is noisy and not actionable in production.- console.log("process.send", process.send, INTERFACE); + // If logging is needed here, prefer the project logger: + // LoggerService.forContext("BOOTSTRAP").debug({ hasIPC: Boolean(process.send), interface: INTERFACE });
15-16: Typo: boostrapList → bootstrapListMinor readability/polish.
- const boostrapList = SUPPORTED_INTERFACES.map((interfaceName, index) => bootstrapInChildProcess({ PORT: String(port + index), INTERFACE: interfaceName })); - await Promise.all(boostrapList); + const bootstrapList = SUPPORTED_INTERFACES.map((interfaceName, index) => + bootstrapInChildProcess({ PORT: String(port + index), INTERFACE: interfaceName }) + ); + await Promise.all(bootstrapList);
7-8: Narrow interface types for better checks and autocompleteLock SUPPORTED_INTERFACES to a literal tuple and derive a union for INTERFACE. This catches typos at compile time and improves DX.
-const SUPPORTED_INTERFACES = ["rest", "background-jobs"]; +const SUPPORTED_INTERFACES = ["rest", "background-jobs"] as const; +type InterfaceName = typeof SUPPORTED_INTERFACES[number]; + +type BootEnv = { + PORT?: string; + INTERFACE?: "all" | InterfaceName; +};And update bootstrap signature:
-async function bootstrap({ PORT = "3080", INTERFACE = "all" }: Record<string, string | undefined>): Promise<void> { +async function bootstrap({ PORT = "3080", INTERFACE = "all" }: BootEnv): Promise<void> {apps/api/src/db/dbConnection.ts (1)
71-83: Avoid variable shadowing and make logs unambiguousThe parameter
loggershadows the module-scopedlogger(PostgresLoggerService), which hampers readability. Rename the parameter to avoid confusion.-export async function connectUsingSequelize(logger: LoggerService = LoggerService.forContext("DB")): Promise<void> { - logger.debug(`Connecting to chain database (${chainDb.config.host}/${chainDb.config.database})...`); +export async function connectUsingSequelize(appLogger: LoggerService = LoggerService.forContext("DB")): Promise<void> { + appLogger.debug(`Connecting to chain database (${chainDb.config.host}/${chainDb.config.database})...`); await chainDb.authenticate(); - logger.debug("Connection has been established successfully."); + appLogger.debug("Chain DB connection established."); - logger.debug(`Connecting to user database (${userDb.config.host}/${userDb.config.database})...`); + appLogger.debug(`Connecting to user database (${userDb.config.host}/${userDb.config.database})...`); await userDb.authenticate(); - logger.debug("Connection has been established successfully."); + appLogger.debug("User DB connection established."); - logger.debug("Sync user schema..."); + appLogger.debug("Sync user schema..."); await syncUserSchema(); - logger.debug("User schema synced."); + appLogger.debug("User schema synced."); }If startup latency matters, consider authenticating both connections concurrently and adjusting logs accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (72)
apps/api/env/.env.functional.test(1 hunks)apps/api/env/.env.local.sample(1 hunks)apps/api/env/.env.unit.test(1 hunks)apps/api/src/app/index.ts(1 hunks)apps/api/src/app/providers/jobs.provider.ts(1 hunks)apps/api/src/background-jobs-app.ts(1 hunks)apps/api/src/console.ts(1 hunks)apps/api/src/core/index.ts(1 hunks)apps/api/src/core/providers/index.ts(1 hunks)apps/api/src/core/providers/job-queue-healthcheck.ts(1 hunks)apps/api/src/core/providers/postgres.provider.ts(4 hunks)apps/api/src/core/services/index.ts(1 hunks)apps/api/src/core/services/job-queue/job-queue.service.spec.ts(4 hunks)apps/api/src/core/services/job-queue/job-queue.service.ts(4 hunks)apps/api/src/core/services/shutdown-server/shutdown-server.spec.ts(1 hunks)apps/api/src/core/services/shutdown-server/shutdown-server.ts(1 hunks)apps/api/src/core/services/start-server/start-server.spec.ts(1 hunks)apps/api/src/core/services/start-server/start-server.ts(1 hunks)apps/api/src/db/dbConnection.ts(2 hunks)apps/api/src/healthz/controllers/healthz/healthz.controller.spec.ts(2 hunks)apps/api/src/healthz/routes/healthz.router.ts(1 hunks)apps/api/src/healthz/services/healthz/healthz.service.spec.ts(1 hunks)apps/api/src/healthz/services/healthz/healthz.service.ts(1 hunks)apps/api/src/index.ts(0 hunks)apps/api/src/notifications/services/notification-handler/notification.handler.ts(1 hunks)apps/api/src/rest-app.ts(4 hunks)apps/api/src/server.ts(1 hunks)apps/api/test/functional/addresses.spec.ts(2 hunks)apps/api/test/functional/anonymous-user.spec.ts(1 hunks)apps/api/test/functional/api-key.spec.ts(1 hunks)apps/api/test/functional/app.spec.ts(1 hunks)apps/api/test/functional/auditors.spec.ts(1 hunks)apps/api/test/functional/balances.spec.ts(1 hunks)apps/api/test/functional/bids.spec.ts(1 hunks)apps/api/test/functional/blocks.spec.ts(1 hunks)apps/api/test/functional/certificate.spec.ts(1 hunks)apps/api/test/functional/create-deployment.spec.ts(1 hunks)apps/api/test/functional/dashboard-data.spec.ts(1 hunks)apps/api/test/functional/deployment-setting.spec.ts(1 hunks)apps/api/test/functional/deployments.spec.ts(1 hunks)apps/api/test/functional/docs.spec.ts(1 hunks)apps/api/test/functional/gpu.spec.ts(1 hunks)apps/api/test/functional/graph-data.spec.ts(1 hunks)apps/api/test/functional/lease-flow.spec.ts(1 hunks)apps/api/test/functional/leases-duration.spec.ts(1 hunks)apps/api/test/functional/market-data.spec.ts(1 hunks)apps/api/test/functional/network-capacity.spec.ts(1 hunks)apps/api/test/functional/nodes-v1.spec.ts(1 hunks)apps/api/test/functional/pricing.spec.ts(1 hunks)apps/api/test/functional/proposals.spec.ts(1 hunks)apps/api/test/functional/provider-attributes-schema.spec.ts(1 hunks)apps/api/test/functional/provider-dashboard.spec.ts(1 hunks)apps/api/test/functional/provider-deployments.spec.ts(1 hunks)apps/api/test/functional/provider-earnings.spec.ts(1 hunks)apps/api/test/functional/provider-graph-data.spec.ts(1 hunks)apps/api/test/functional/provider-regions.spec.ts(1 hunks)apps/api/test/functional/provider-versions.spec.ts(1 hunks)apps/api/test/functional/providers.spec.ts(1 hunks)apps/api/test/functional/sign-and-broadcast-tx.spec.ts(1 hunks)apps/api/test/functional/stale-anonymous-users-cleanup.spec.ts(1 hunks)apps/api/test/functional/start-trial.spec.ts(1 hunks)apps/api/test/functional/stripe-webhook.spec.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)apps/api/test/functional/transactions.spec.ts(1 hunks)apps/api/test/functional/usage.spec.ts(1 hunks)apps/api/test/functional/user-init.spec.ts(1 hunks)apps/api/test/functional/validators.spec.ts(1 hunks)apps/api/test/functional/wallets-refill.spec.ts(1 hunks)apps/api/tsconfig.json(1 hunks)apps/api/webpack.dev.js(2 hunks)apps/api/webpack.prod.js(2 hunks)packages/docker/docker-compose.prod.yml(2 hunks)
💤 Files with no reviewable changes (1)
- apps/api/src/index.ts
✅ Files skipped from review due to trivial changes (5)
- apps/api/test/functional/blocks.spec.ts
- apps/api/env/.env.functional.test
- apps/api/test/functional/auditors.spec.ts
- apps/api/test/functional/create-deployment.spec.ts
- apps/api/test/functional/dashboard-data.spec.ts
🚧 Files skipped from review as they are similar to previous changes (54)
- apps/api/test/functional/gpu.spec.ts
- apps/api/test/functional/network-capacity.spec.ts
- apps/api/test/functional/sign-and-broadcast-tx.spec.ts
- apps/api/test/functional/graph-data.spec.ts
- apps/api/test/functional/provider-deployments.spec.ts
- apps/api/test/functional/certificate.spec.ts
- apps/api/test/functional/wallets-refill.spec.ts
- apps/api/tsconfig.json
- apps/api/test/functional/docs.spec.ts
- apps/api/test/functional/validators.spec.ts
- apps/api/test/functional/lease-flow.spec.ts
- apps/api/test/functional/start-trial.spec.ts
- apps/api/src/app/providers/jobs.provider.ts
- apps/api/webpack.prod.js
- apps/api/src/core/services/index.ts
- apps/api/test/functional/provider-graph-data.spec.ts
- apps/api/test/functional/pricing.spec.ts
- packages/docker/docker-compose.prod.yml
- apps/api/test/functional/transactions.spec.ts
- apps/api/src/background-jobs-app.ts
- apps/api/test/functional/provider-earnings.spec.ts
- apps/api/test/functional/providers.spec.ts
- apps/api/test/functional/stripe-webhook.spec.ts
- apps/api/test/functional/app.spec.ts
- apps/api/src/core/services/shutdown-server/shutdown-server.ts
- apps/api/src/healthz/routes/healthz.router.ts
- apps/api/env/.env.local.sample
- apps/api/test/functional/market-data.spec.ts
- apps/api/src/healthz/controllers/healthz/healthz.controller.spec.ts
- apps/api/test/functional/balances.spec.ts
- apps/api/test/functional/provider-attributes-schema.spec.ts
- apps/api/test/functional/anonymous-user.spec.ts
- apps/api/test/functional/deployments.spec.ts
- apps/api/test/functional/nodes-v1.spec.ts
- apps/api/test/functional/stale-anonymous-users-cleanup.spec.ts
- apps/api/src/healthz/services/healthz/healthz.service.spec.ts
- apps/api/src/core/services/start-server/start-server.spec.ts
- apps/api/test/functional/api-key.spec.ts
- apps/api/test/functional/deployment-setting.spec.ts
- apps/api/test/functional/bids.spec.ts
- apps/api/src/console.ts
- apps/api/test/functional/templates.spec.ts
- apps/api/test/functional/addresses.spec.ts
- apps/api/test/functional/usage.spec.ts
- apps/api/test/functional/leases-duration.spec.ts
- apps/api/src/core/providers/job-queue-healthcheck.ts
- apps/api/src/notifications/services/notification-handler/notification.handler.ts
- apps/api/test/functional/provider-versions.spec.ts
- apps/api/test/functional/provider-dashboard.spec.ts
- apps/api/src/core/services/start-server/start-server.ts
- apps/api/src/app/index.ts
- apps/api/test/functional/provider-regions.spec.ts
- apps/api/test/functional/proposals.spec.ts
- apps/api/src/healthz/services/healthz/healthz.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/src/core/services/shutdown-server/shutdown-server.spec.tsapps/api/test/functional/user-init.spec.tsapps/api/src/core/services/job-queue/job-queue.service.spec.ts
**/*.{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/src/core/services/shutdown-server/shutdown-server.spec.tsapps/api/src/core/providers/postgres.provider.tsapps/api/src/db/dbConnection.tsapps/api/test/functional/user-init.spec.tsapps/api/src/core/services/job-queue/job-queue.service.spec.tsapps/api/src/core/services/job-queue/job-queue.service.tsapps/api/src/core/index.tsapps/api/src/server.tsapps/api/src/rest-app.tsapps/api/src/core/providers/index.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/src/core/services/shutdown-server/shutdown-server.spec.tsapps/api/src/core/providers/postgres.provider.tsapps/api/src/db/dbConnection.tsapps/api/test/functional/user-init.spec.tsapps/api/src/core/services/job-queue/job-queue.service.spec.tsapps/api/src/core/services/job-queue/job-queue.service.tsapps/api/src/core/index.tsapps/api/webpack.dev.jsapps/api/src/server.tsapps/api/src/rest-app.tsapps/api/src/core/providers/index.ts
🧠 Learnings (2)
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/api/src/core/services/job-queue/job-queue.service.spec.ts
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
PR: akash-network/console#1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: In the Akash Network console project, services like JobQueueService implement dispose() methods for resource cleanup, and the tsyringe container has been extended with a dispose() method that iterates over all registered instances and calls their dispose() methods, enabling proper shutdown orchestration.
Applied to files:
apps/api/src/core/services/job-queue/job-queue.service.ts
🧬 Code graph analysis (6)
apps/api/src/core/providers/postgres.provider.ts (5)
apps/api/src/db/dbConnection.ts (2)
closeConnections(54-54)ON_APP_START(58-60)apps/api/src/core/providers/app-initializer.ts (3)
APP_INITIALIZER(3-3)ON_APP_START(4-4)AppInitializer(6-8)apps/api/src/app/providers/jobs.provider.ts (1)
ON_APP_START(11-19)apps/api/src/rest-app.ts (1)
ON_APP_START(188-190)apps/api/src/core/services/feature-flags/feature-flags.service.ts (1)
ON_APP_START(80-82)
apps/api/src/db/dbConnection.ts (4)
apps/api/src/core/providers/postgres.provider.ts (1)
closeConnections(54-54)apps/api/src/core/providers/app-initializer.ts (2)
APP_INITIALIZER(3-3)AppInitializer(6-8)apps/api/src/app/providers/jobs.provider.ts (1)
ON_APP_START(11-19)apps/api/src/rest-app.ts (2)
ON_APP_START(188-190)connectUsingSequelize(194-194)
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (2)
apps/api/src/core/services/job-queue/job-queue.service.ts (1)
setup(135-142)packages/logging/src/servicies/logger/logger.service.ts (1)
error(107-109)
apps/api/src/core/services/job-queue/job-queue.service.ts (5)
apps/api/src/notifications/services/notification-handler/notification.handler.ts (1)
singleton(37-78)apps/api/src/app/services/trial-started/trial-started.handler.ts (1)
singleton(13-94)apps/api/src/core/services/domain-events/domain-events.service.ts (1)
singleton(10-17)apps/api/src/notifications/services/notification/notification.service.ts (1)
singleton(13-67)packages/logging/src/servicies/logger/logger.service.ts (1)
error(107-109)
apps/api/src/server.ts (1)
apps/api/src/rest-app.ts (1)
bootstrap(196-201)
apps/api/src/rest-app.ts (5)
apps/api/src/core/providers/app-initializer.ts (2)
APP_INITIALIZER(3-3)AppInitializer(6-8)apps/api/src/app/providers/jobs.provider.ts (1)
ON_APP_START(11-19)apps/api/src/db/dbConnection.ts (1)
ON_APP_START(58-60)apps/api/src/core/services/start-server/start-server.ts (1)
startServer(19-64)apps/api/src/core/providers/postgres.provider.ts (1)
migratePG(26-26)
🪛 dotenv-linter (3.3.0)
apps/api/env/.env.unit.test
[warning] 28-28: [UnorderedKey] The NOTIFICATIONS_API_BASE_URL key should go before the POSTGRES_DB_URI key
(UnorderedKey)
⏰ 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). (11)
- 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: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (14)
apps/api/src/core/services/shutdown-server/shutdown-server.spec.ts (1)
68-71: Updated log event key is correct.Replacing CONTAINER_DISPOSE_ERROR with ON_SHUTDOWN_ERROR in the expectation aligns with the shutdown lifecycle changes. Test intent and coverage remain intact.
apps/api/webpack.dev.js (1)
21-23: TS-first resolution looks good.extensionAlias ensures bare .js imports resolve to .ts first in dev. Matches prod practice and avoids fragile path rewrites.
apps/api/src/core/services/job-queue/job-queue.service.ts (2)
144-147: pg-boss private API usage in ping(); keep a fallback or assert version compatibility.
getDb()is not part of the public types; relying on it can break across versions. Consider a guarded call or a light-weight public operation (e.g., a trivial send/cancel to a health queue), or document the version constraint.Proposed guarded approach:
// inside ping() const bossAny = this.pgBoss as unknown as { getDb?: () => { executeSql: (sql: string) => Promise<any> } }; if (!bossAny.getDb) { this.logger.warn({ event: "JOB_QUEUE_PING_UNSUPPORTED" }); return; } // @ts-expect-error - non-public method await bossAny.getDb().executeSql("SELECT 1");If you prefer to avoid private APIs entirely, I can provide an alternative ping using a dedicated transient queue.
28-43: Duplicate handler detection looks good.Set-based guard prevents multiple handlers for the same queue; fail-fast error message is clear.
apps/api/test/functional/user-init.spec.ts (1)
29-31: Functional test composition looks solid.Using WalletTestingService with app from rest-app maintains the new startup split and keeps the test clear.
apps/api/src/core/index.ts (1)
1-7: Side-effect provider import and explicit re-exports look consistent with the new bootstrap architecture.This ensures providers register with DI on core import and surfaces start-server and job-queue from a single barrel.
apps/api/src/core/providers/postgres.provider.ts (4)
54-55: LGTM: dispose returns void-compatible promiseReturning
undefinedpost-Promise.allis consistent withDisposableexpectations and makes disposal sequencing predictable.
56-62: APP_INITIALIZER + Disposable is a solid pattern for graceful shutdownRegistering a disposable initializer ensures connections are closed via
container.dispose()after initializers have been resolved by the server bootstrap. This fits the lifecycle used in startServer.
6-6: Typed import is appropriateImporting
Disposable/InjectionTokenas types keeps runtime clean.
15-17: Explicitly scoping initializer tokens keeps coupling lowImporting tokens/interfaces from app-initializer here is correct. No action needed.
apps/api/src/db/dbConnection.ts (2)
54-63: Good lifecycle wiring: init on start, dispose on shutdownRegistering an APP_INITIALIZER with an [ON_APP_START] hook and Disposable.dispose cleanly integrates Sequelize init and teardown with server lifecycle.
10-13: Centralizing LoggerService usage in DB init is consistent and clearImporting LoggerService here avoids leaking logging concerns to callers.
apps/api/src/rest-app.ts (2)
186-192: Scheduler startup via APP_INITIALIZER fits the new lifecycleStarting the Scheduler in an APP_INITIALIZER keeps side effects centralized and ordered with other initializers.
196-200: Bootstrap delegates correctly to startServer with migrations hookHooking
migratePGinto beforeStart ensures schema is up-to-date prior to serving. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (1)
116-119: Replaceanyin pgBoss.work mock with precise types; also type thejobsfixture.Repo guideline forbids
any. Use the actual parameter types and a small local job type for the test payload.Apply this diff:
- const jobs = [{ id: "1", data: { message: "Job 1", userId: "user-1" } }]; + const jobs: EnqueuedJob[] = [{ id: "1", data: { message: "Job 1", userId: "user-1" } }]; - jest.spyOn(pgBoss, "work").mockImplementation(async (queueName: string, options: any, processFn: (jobs: any[]) => Promise<void>) => { - await processFn(jobs); + jest.spyOn(pgBoss, "work").mockImplementation(async (_queueName: string, _options: Parameters<PgBoss["work"]>[1], processFn: (jobs: EnqueuedJob[]) => Promise<void>) => { + await processFn(jobs); return "work-id"; });Add this helper type near the top of the file (or just above this test):
type EnqueuedJob = { id: string; data: TestJob["data"] };Also applies to: 114-114
♻️ Duplicate comments (3)
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (2)
149-153: Avoidanyin failure-path work mock; mirror the typed signature used above.Same issue as the success-path mock; keep mocks consistently typed and prefix unused params.
- const jobs = [{ id: "1", data: { message: "Job 1", userId: "user-1" } }]; + const jobs: EnqueuedJob[] = [{ id: "1", data: { message: "Job 1", userId: "user-1" } }]; - jest.spyOn(pgBoss, "work").mockImplementation(async (queueName: string, options: any, processFn: (jobs: any[]) => Promise<void>) => { - await processFn(jobs); + jest.spyOn(pgBoss, "work").mockImplementation(async (_queueName: string, _options: Parameters<PgBoss["work"]>[1], processFn: (jobs: EnqueuedJob[]) => Promise<void>) => { + await processFn(jobs); return "work-id"; });
173-176: Avoidanyin default-options work mock; keep the signature typed.Mirror the same typed signature here; also mark unused params with underscores.
- jest.spyOn(pgBoss, "work").mockImplementation(async (queueName: string, options: any, processFn: (jobs: any[]) => Promise<void>) => { - await processFn([{ id: "1", data: { message: "Job 1", userId: "user-1" } }]); + jest.spyOn(pgBoss, "work").mockImplementation(async (_queueName: string, _options: Parameters<PgBoss["work"]>[1], processFn: (jobs: EnqueuedJob[]) => Promise<void>) => { + await processFn([{ id: "1", data: { message: "Job 1", userId: "user-1" } }] as EnqueuedJob[]); return "work-id"; });apps/api/src/core/providers/postgres.provider.ts (1)
30-41: DB_HEALTHCHECK.ping now correctly resolves to Promise — nice cleanupThe healthcheck implementation now awaits the
SELECT 1probe and returns no value, matching the declaredPromise<void>type. This prevents accidental reliance on driver return shapes and aligns with the token’s contract.
🧹 Nitpick comments (12)
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (2)
242-262: Movesetup()helper to the bottom of the root describe to follow our test convention.Our test guideline says the setup function must be at the bottom of the root describe; currently, class helpers come after it.
Apply this move (remove here, re-add after the helper classes and just before the closing
});):- function setup(input?: { pgBoss?: PgBoss; postgresDbUri?: string }) { - const mocks = { - logger: mock<LoggerService>(), - coreConfig: mock<CoreConfigService>({ - get: jest.fn().mockReturnValue(input?.postgresDbUri ?? "postgresql://localhost:5432/test") - }), - pgBoss: - input?.pgBoss ?? - mockDeep<PgBoss>({ - createQueue: jest.fn().mockResolvedValue(undefined), - send: jest.fn().mockResolvedValue("job-id"), - work: jest.fn().mockResolvedValue(undefined), - start: jest.fn().mockResolvedValue(undefined), - stop: jest.fn().mockResolvedValue(undefined) - }) - }; - - const service = new JobQueueService(mocks.logger, mocks.coreConfig, input && Object.hasOwn(input, "pgBoss") ? input?.pgBoss : mocks.pgBoss); - - return { service, ...mocks }; - }Then insert the same function after
class TestHandler { ... }:// Keep setup at the very bottom of the root describe (per repo guideline) function setup(input?: { pgBoss?: PgBoss; postgresDbUri?: string }) { const mocks = { logger: mock<LoggerService>(), coreConfig: mock<CoreConfigService>({ get: jest.fn().mockReturnValue(input?.postgresDbUri ?? "postgresql://localhost:5432/test") }), pgBoss: input?.pgBoss ?? mockDeep<PgBoss>({ createQueue: jest.fn().mockResolvedValue(undefined), send: jest.fn().mockResolvedValue("job-id"), work: jest.fn().mockResolvedValue(undefined), start: jest.fn().mockResolvedValue(undefined), stop: jest.fn().mockResolvedValue(undefined) }) }; const service = new JobQueueService(mocks.logger, mocks.coreConfig, input && Object.hasOwn(input, "pgBoss") ? input?.pgBoss : mocks.pgBoss); return { service, ...mocks }; }
109-140: Optional: DRY the repeatedpgBoss.workmock with a tiny helper.Three tests re-declare the same mock shape. Collapsing into a helper reduces duplication and keeps typing in one place.
Example helper you can place near the bottom:
function mockWorkToProcess(pgBoss: PgBoss, jobs: EnqueuedJob[] | (() => EnqueuedJob[])) { return jest.spyOn(pgBoss, "work").mockImplementation(async (_q: string, _o: Parameters<PgBoss["work"]>[1], processFn: (batch: EnqueuedJob[]) => Promise<void>) => { const batch = typeof jobs === "function" ? jobs() : jobs; await processFn(batch); return "work-id"; }); }Then in tests:
mockWorkToProcess(pgBoss, jobs); // or mockWorkToProcess(pgBoss, () => [{ id: "1", data: { message: "Job 1", userId: "user-1" } }]);Also applies to: 142-168, 168-184
apps/api/src/core/providers/postgres.provider.ts (3)
33-33: Prefer Symbol-based DI tokens for consistency and collision safetyElsewhere (e.g., APP_INITIALIZER/ON_APP_START) tokens are Symbols. Consider switching
DB_HEALTHCHECKto a Symbol for consistency and to avoid accidental string token collisions.Proposed change:
-export const DB_HEALTHCHECK: InjectionToken<DbHealthcheck> = "DB_HEALTHCHECK"; +export const DB_HEALTHCHECK: InjectionToken<DbHealthcheck> = Symbol("DB_HEALTHCHECK");Please confirm all imports use this constant (no raw string-based injections). If any third-party wiring expects the string token, this would be a breaking change.
34-41: Nit: tighten container.register typingWhile
satisfies DbHealthcheckis good, adding the generic improves readability and editor tooling.-container.register(DB_HEALTHCHECK, { +container.register<DbHealthcheck>(DB_HEALTHCHECK, { useValue: { async ping() { await appClient.unsafe("SELECT 1"); } - } satisfies DbHealthcheck + } satisfies DbHealthcheck });
56-64: Shutdown wiring: verify who calls dispose() and Disposable contractRegistering an
APP_INITIALIZERthat also exposesdispose: closeConnectionsis solid. Two checks:
- Ensure your bootstrap (e.g., startServer) resolves this initializer and calls
dispose()on shutdown signals.- Confirm the
Disposableinterface you import from tsyringe allowsdispose(): Promise<void>(some versions definevoid | Promise<void>, others justvoid).Minor simplification: you don’t need
.then(() => undefined)to coerce the return type.-export const closeConnections = async () => await Promise.all([migrationClient.end(), appClient.end()]).then(() => undefined); +export const closeConnections = async () => { + await Promise.all([migrationClient.end(), appClient.end()]); +}apps/api/src/healthz/services/healthz/healthz.service.spec.ts (4)
10-12: Consider removing cacheEngine cleanup or centralize timer reset in afterEachThe service under test no longer uses
cacheEngine; clearing it here may be unnecessary noise. If other specs rely on it, fine—otherwise consider dropping it. Also consider addingjest.useRealTimers()in afterEach to avoid timer leaks from tests that enable fake timers.afterEach(() => { - cacheEngine.clearAllKeyInCache(); + cacheEngine.clearAllKeyInCache(); // remove if no longer needed + jest.useRealTimers(); // ensures clean timer state between tests });
87-111: Strengthen assertion: ensure exactly two error logs (and in any order)You verify both structured error events; also assert they’re logged exactly twice to prevent duplicate/error spam.
expect(dbHealthcheck.ping).toHaveBeenCalled(); expect(jobQueueHealthcheck.ping).toHaveBeenCalled(); expect(logger.error).toHaveBeenCalledWith({ event: "POSTGRES_HEALTHCHECK_ERROR", error: dbError }); expect(logger.error).toHaveBeenCalledWith({ event: "JOBQUEUE_HEALTHCHECK_ERROR", error: jobQueueError }); +expect(logger.error).toHaveBeenCalledTimes(2);
153-198: Flaky-test guard (optional): pin the fake system timeIf CI ever flakes on the TTL boundary assertions, consider pinning time at test start with
jest.setSystemTime(new Date()). Not required now, but helps eliminate platform clock variance.
201-213: Align setup() with repo testing guidelines (optional)Guidelines suggest
setupaccepts a single parameter with an inline type. You can keep defaults but allow per-test overrides without shared state.-function setup() { +function setup({ dbOk = true, jobsOk = true }: { dbOk?: boolean; jobsOk?: boolean } = {}) { const logger = mock<LoggerService>(); - const dbHealthcheck = mock<DbHealthcheck>({ ping: jest.fn().mockResolvedValue(undefined) }); - const jobQueueHealthcheck = mock<JobQueueHealthcheck>({ ping: jest.fn().mockResolvedValue(undefined) }); + const dbHealthcheck = mock<DbHealthcheck>({ + ping: jest.fn()[dbOk ? "mockResolvedValue" : "mockRejectedValue"](dbOk ? undefined : new Error("Postgres is not ready")) + }); + const jobQueueHealthcheck = mock<JobQueueHealthcheck>({ + ping: jest.fn()[jobsOk ? "mockResolvedValue" : "mockRejectedValue"](jobsOk ? undefined : new Error("JobsQueue is not ready")) + }); const healthzService = new HealthzService(dbHealthcheck, jobQueueHealthcheck, logger); return { logger, service: healthzService, dbHealthcheck, jobQueueHealthcheck }; }apps/api/src/healthz/services/healthz/healthz.service.ts (3)
39-49: Avoid type assertion by deriving keys explicitly (optional)Relying on
as keyof HealthzResult["data"]works but can mask drift if a new healthcheck name is added. Construct the result by key to retain type-safety.- private buildResult(results: boolean[]): HealthzResult { - return { - status: results.every(Boolean) ? "ok" : "error", - data: results.reduce( - (acc, result, index) => { - acc[this.healthchecks[index].name as keyof HealthzResult["data"]] = result; - return acc; - }, - {} as HealthzResult["data"] - ) - }; - } + private buildResult(results: boolean[]): HealthzResult { + const data: HealthzResult["data"] = { + postgres: results[this.healthchecks.findIndex(h => h.name === "postgres")] ?? false, + jobQueue: results[this.healthchecks.findIndex(h => h.name === "jobQueue")] ?? false + }; + return { + status: data.postgres && data.jobQueue ? "ok" : "error", + data + }; + }This keeps the current public shape and avoids unsafe casting.
61-74: Simplify the healthchecker typeUsing a union inside
Pick<...,"ping">is equivalent to a structural{ ping(): Promise<void> }. Simplify for readability.- constructor( - public readonly name: string, - private readonly healthchecker: Pick<DbHealthcheck | JobQueueHealthcheck, "ping">, + constructor( + public readonly name: string, + private readonly healthchecker: { ping(): Promise<void> },
100-107: Nit: annotate return type of check for clarityExplicitly returning
Promise<void>clarifies intent and helps future refactors.- private check() { + private check(): Promise<void> { this.inflightPing ??= this.healthchecker.ping().finally(() => { this.checkedAt = new Date(); this.inflightPing = undefined; }); return this.inflightPing; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/api/src/core/providers/postgres.provider.ts(4 hunks)apps/api/src/core/services/job-queue/job-queue.service.spec.ts(5 hunks)apps/api/src/core/services/start-server/start-server.ts(1 hunks)apps/api/src/healthz/services/healthz/healthz.service.spec.ts(1 hunks)apps/api/src/healthz/services/healthz/healthz.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/src/core/services/start-server/start-server.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/src/healthz/services/healthz/healthz.service.tsapps/api/src/core/providers/postgres.provider.tsapps/api/src/healthz/services/healthz/healthz.service.spec.tsapps/api/src/core/services/job-queue/job-queue.service.spec.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/src/healthz/services/healthz/healthz.service.tsapps/api/src/core/providers/postgres.provider.tsapps/api/src/healthz/services/healthz/healthz.service.spec.tsapps/api/src/core/services/job-queue/job-queue.service.spec.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/src/healthz/services/healthz/healthz.service.spec.tsapps/api/src/core/services/job-queue/job-queue.service.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/api/src/core/services/job-queue/job-queue.service.spec.ts
🧬 Code graph analysis (4)
apps/api/src/healthz/services/healthz/healthz.service.ts (5)
apps/api/src/core/providers/logging.provider.ts (1)
injectable(7-8)apps/api/src/healthz/controllers/healthz/healthz.controller.ts (1)
injectable(6-17)apps/api/src/core/providers/postgres.provider.ts (2)
DB_HEALTHCHECK(33-33)DbHealthcheck(30-32)apps/api/src/core/providers/job-queue-healthcheck.ts (2)
JOB_QUEUE_HEALTHCHECK(9-9)JobQueueHealthcheck(6-8)packages/logging/src/servicies/logger/logger.service.ts (1)
error(107-109)
apps/api/src/core/providers/postgres.provider.ts (2)
apps/api/src/db/dbConnection.ts (2)
closeConnections(54-54)ON_APP_START(58-60)apps/api/src/core/providers/app-initializer.ts (3)
APP_INITIALIZER(3-3)ON_APP_START(4-4)AppInitializer(6-8)
apps/api/src/healthz/services/healthz/healthz.service.spec.ts (4)
apps/api/src/caching/helpers.ts (1)
cacheEngine(8-8)packages/logging/src/servicies/logger/logger.service.ts (1)
error(107-109)apps/api/src/core/providers/postgres.provider.ts (1)
DbHealthcheck(30-32)apps/api/src/core/providers/job-queue-healthcheck.ts (1)
JobQueueHealthcheck(6-8)
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (2)
apps/api/src/core/services/job-queue/job-queue.service.ts (1)
setup(135-142)packages/logging/src/servicies/logger/logger.service.ts (1)
error(107-109)
⏰ 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). (12)
- GitHub Check: validate (apps/provider-proxy) / validate-unsafe
- GitHub Check: validate (apps/api) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- 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
🔇 Additional comments (8)
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (5)
128-135: LGTM: updated expectations for multi-worker + batchSize=1 contract.The assertions align with the new per-job logging (JOB_STARTED/JOB_DONE), 5 workers, and single-job batches.
156-166: Good negative-path coverage and logging assertions.Capturing the rejection via Promise.allSettled and asserting JOB_FAILED payload (including jobId and error) is solid.
186-196: LGTM: setup() lifecycle test matches service implementation.Verifies JOB_QUEUE_STARTING/STARTED, error wiring, and start invocation; consistent with JobQueueService.setup().
210-217: LGTM: PgBoss error handler wiring is validated.The test spies on
on('error', ...)and asserts JOB_QUEUE_ERROR logging with the original error object.
230-240: LGTM: ping() test is pragmatic given pg-boss typings.Using
@ts-expect-errorwith a reference to the upstream typing gap is acceptable here and keeps the test intention clear.apps/api/src/healthz/services/healthz/healthz.service.spec.ts (1)
113-137: TTL and inflight assertions look solidGood use of fake timers and call-count checks to validate caching and deduplication. This should guard against regressions in the TTL logic.
apps/api/src/healthz/services/healthz/healthz.service.ts (2)
12-27: Constructor wiring and TTL setup look goodClear separation of concerns: injecting healthcheck providers, setting context, and registering TTL-configured probes. Using one-minute TTL is a reasonable default.
75-99: Healthy fallback logic and caching semantics are well thought-out
- First failure after a success is tolerated until TTL expiry.
- Failures before any success are surfaced immediately.
ignoreCacheforces a fresh probe and strict error reporting.This aligns with the liveness/readiness expectations encoded in tests.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/api/src/core/config/env.config.ts (1)
13-13: Validate schema name to a safe Postgres identifier; optionally normalize case.To avoid accidental quoted identifiers and runtime surprises, enforce a conservative identifier pattern. Optionally lowercase to match unquoted identifier semantics.
Option A (validation only):
- POSTGRES_BACKGROUND_JOBS_SCHEMA: z.string().optional().default("pgboss"), + POSTGRES_BACKGROUND_JOBS_SCHEMA: z + .string() + .optional() + .default("pgboss") + .refine(s => /^[a-z_][a-z0-9_]*$/.test(s), { + message: "POSTGRES_BACKGROUND_JOBS_SCHEMA must match ^[a-z_][a-z0-9_]*$", + }),Option B (also normalize to lowercase; use with care if someone intends a quoted/mixed-case schema):
- POSTGRES_BACKGROUND_JOBS_SCHEMA: z.string().optional().default("pgboss"), + POSTGRES_BACKGROUND_JOBS_SCHEMA: z + .string() + .optional() + .transform(v => (v ?? "").trim()) + .transform(s => (s.length ? s.toLowerCase() : "pgboss")) + .refine(s => /^[a-z_][a-z0-9_]*$/.test(s), { + message: "POSTGRES_BACKGROUND_JOBS_SCHEMA must match ^[a-z_][a-z0-9_]*$", + }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/src/core/config/env.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/src/core/config/env.config.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/src/core/config/env.config.ts
⏰ 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). (13)
- GitHub Check: validate (apps/notifications) / validate-unsafe
- GitHub Check: validate (apps/provider-proxy) / validate-unsafe
- GitHub Check: validate (apps/indexer) / validate-unsafe
- 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: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
| await jobQueueManager.start(); | ||
| await jobQueueManager.setup(); | ||
| await jobQueueManager.registerHandlers([ | ||
| // keep new lines |
There was a problem hiding this comment.
non-blocking. isn't there a way to configure prettier for this?
There was a problem hiding this comment.
No to my knowledge, prettier likes people to struggle. Their philosophy is either accept what it does or don't use it 😄 because formatting is not important, it just must be universal to eliminate stylistic holy wars. We could play with line width but it will break other things.
Refs:
- How can I prevent prettier from taking multi-line array to a single line? prettier/prettier-vscode#352
- RFC - Future of Prettier and (re)considerations about current mindset prettier/prettier#8507
- Support library-specific plugin prettier/prettier#4424
We just need to switch to https://eslint.style/ at some point. I've already used it in chain sdk project, as a replacement to prettier.
stalniy
force-pushed
the
feat/background-jobs
branch
from
8eb2d5e to
22d400f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/api/src/core/services/shutdown-server/shutdown-server.spec.ts (1)
7-101: Adopt the required setup() pattern in spec files (reduce duplication, align with repo testing guidelines)Per project guidelines for
**/*.spec.ts, tests should use asetupfunction (at the bottom of the rootdescribe) to build SUT/dependencies, avoid shared state, and return the object under test. This also eliminates duplicated mock wiring across tests and makes behavior toggles explicit (listening vs not, close callback variations, onShutdown rejection, etc.).Apply this refactor within the
describeblock:describe(shutdownServer.name, () => { - it("closes the server ", async () => { - const server = mock<ServerType>({ - close: jest.fn().mockImplementation(cb => cb()) - }); - const appLogger = mock<Logger>(); - const onShutdown = jest.fn(); - - await shutdownServer(server, appLogger, onShutdown); - - expect(server.close).toHaveBeenCalledTimes(1); - expect(onShutdown).toHaveBeenCalledTimes(1); - expect(appLogger.error).not.toHaveBeenCalled(); - }); + it("closes the server", async () => { + const { server, appLogger, onShutdown, run } = setup({ listening: true, closeBehavior: "cb-ok" }); + await run(); + expect(server.close).toHaveBeenCalledTimes(1); + expect(onShutdown).toHaveBeenCalledTimes(1); + expect(appLogger.error).not.toHaveBeenCalled(); + }); it("logs error if server close fails", async () => { const error = new Error("Failed to close server"); - const server = mock<ServerType>({ - close: jest.fn().mockImplementation(cb => cb(error)) - }); - const appLogger = mock<Logger>(); - const onShutdown = jest.fn(); - - await shutdownServer(server, appLogger, onShutdown); - - expect(appLogger.error).toHaveBeenCalledWith({ - event: "SERVER_CLOSE_ERROR", - error - }); + const { appLogger, onShutdown, run } = setup({ listening: true, closeBehavior: "cb-error", closeError: error }); + await run(); + expect(appLogger.error).toHaveBeenCalledWith(expect.objectContaining({ event: "SERVER_CLOSE_ERROR", error })); expect(onShutdown).toHaveBeenCalled(); }); it("logs error if server close throws", async () => { const error = new Error("Failed to close server"); - const server = mock<ServerType>({ - close: jest.fn().mockImplementation(() => { - throw error; - }) as jest.Mock - }); - const appLogger = mock<Logger>(); - const onShutdown = jest.fn(); - - await shutdownServer(server, appLogger, onShutdown); - - expect(appLogger.error).toHaveBeenCalledWith({ - event: "SERVER_CLOSE_ERROR", - error - }); + const { appLogger, onShutdown, run } = setup({ listening: true, closeBehavior: "throws", closeError: error }); + await run(); + expect(appLogger.error).toHaveBeenCalledWith(expect.objectContaining({ event: "SERVER_CLOSE_ERROR", error })); expect(onShutdown).toHaveBeenCalled(); }); it("logs error if onShutdown callback fails", async () => { const error = new Error("Failed to dispose container"); - const server = mock<ServerType>({ - close: jest.fn().mockImplementation(cb => cb()) - }); - const appLogger = mock<Logger>(); - const onShutdown = jest.fn().mockRejectedValue(error); - - await shutdownServer(server, appLogger, onShutdown); - - expect(appLogger.error).toHaveBeenCalledWith({ - event: "ON_SHUTDOWN_ERROR", - error - }); + const { appLogger, run } = setup({ listening: true, closeBehavior: "cb-ok", onShutdownRejected: true, onShutdownError: error }); + await run(); + expect(appLogger.error).toHaveBeenCalledWith(expect.objectContaining({ event: "ON_SHUTDOWN_ERROR", error })); }); it("calls shutdown directly if server is not listening", async () => { - const server = mock<ServerType>({ - listening: false, - close: jest.fn() - }); - const appLogger = mock<Logger>(); - const onShutdown = jest.fn(); - - await shutdownServer(server, appLogger, onShutdown); - - expect(server.close).not.toHaveBeenCalled(); - expect(onShutdown).toHaveBeenCalled(); + const { server, onShutdown, run } = setup({ listening: false }); + await run(); + expect(server.close).not.toHaveBeenCalled(); + expect(onShutdown).toHaveBeenCalled(); }); it("calls server.close if server is listening", async () => { - const server = mock<ServerType>({ - listening: true, - close: jest.fn().mockImplementation(cb => cb()) - }); - const appLogger = mock<Logger>(); - const onShutdown = jest.fn(); - - await shutdownServer(server, appLogger, onShutdown); - - expect(server.close).toHaveBeenCalled(); - expect(onShutdown).toHaveBeenCalled(); + const { server, onShutdown, run } = setup({ listening: true, closeBehavior: "cb-ok" }); + await run(); + expect(server.close).toHaveBeenCalled(); + expect(onShutdown).toHaveBeenCalled(); }); -}); + + // setup must be at the bottom of root describe (hoisted in JS, used above) + function setup({ + listening = true, + closeBehavior = "cb-ok", + closeError, + onShutdownRejected = false, + onShutdownError + }: { + listening?: boolean; + closeBehavior?: "cb-ok" | "cb-error" | "throws"; + closeError?: Error; + onShutdownRejected?: boolean; + onShutdownError?: Error; + }) { + const appLogger = mock<Logger>(); + const onShutdown = onShutdownRejected + ? jest.fn().mockRejectedValue(onShutdownError ?? new Error("onShutdown failed")) + : jest.fn().mockResolvedValue(undefined); + + const closeCbOk = jest.fn().mockImplementation((cb?: (err?: Error) => void) => setImmediate(() => cb?.())); + const closeCbErr = jest.fn().mockImplementation((cb?: (err?: Error) => void) => + setImmediate(() => cb?.(closeError ?? new Error("Failed to close server"))) + ); + const closeThrows = jest + .fn<(cb: (err?: Error) => void) => void>() + .mockImplementation(() => { + throw closeError ?? new Error("Failed to close server"); + }); + + const server = mock<ServerType>({ + listening, + close: closeBehavior === "cb-error" ? closeCbErr : closeBehavior === "throws" ? closeThrows : closeCbOk + }); + + const run = () => shutdownServer(server, appLogger, onShutdown); + return { server, appLogger, onShutdown, run }; + } +});apps/api/src/core/services/job-queue/job-queue.service.spec.ts (1)
116-119: Remove any from pgBoss.work mock; use library-derived types.Tests must not use any per guidelines. Type the options from the actual PgBoss overload and the job shape from the TestJob data.
Apply this diff:
-jest.spyOn(pgBoss, "work").mockImplementation(async (queueName: string, options: any, processFn: (jobs: any[]) => Promise<void>) => { - await processFn(jobs); - return "work-id"; -}); +jest + .spyOn(pgBoss, "work") + .mockImplementation( + async ( + _queueName: string, + _options: Parameters<PgBoss["work"]>[1], + processFn: (jobs: Array<{ id: string; data: TestJob["data"] }>) => Promise<void> + ) => { + await processFn(jobs); + return "work-id"; + } + );Optionally, DRY the job type:
type EnqueuedJob = { id: string; data: TestJob["data"] };and use EnqueuedJob[] in the mock signature.
apps/api/src/console.ts (3)
42-42: Fix concurrency option default — Zod default is never reached when the flag is omittedCommander only calls the parser function when the option is provided. Using Zod’s default inside the parser won’t apply when users omit -c, leaving concurrency as undefined downstream.
Set the default via Commander’s defaultValue argument and keep Zod for coercion/validation.
Apply these diffs:
- .option("-c, --concurrency <number>", "How many wallets are processed concurrently", value => z.number({ coerce: true }).optional().default(10).parse(value)) + .option( + "-c, --concurrency <number>", + "How many wallets are processed concurrently", + v => z.coerce.number().int().positive().parse(v), + 10 + )- .option("-c, --concurrency <number>", "How many wallets is processed concurrently", value => z.number({ coerce: true }).optional().default(10).parse(value)) + .option( + "-c, --concurrency <number>", + "How many wallets are processed concurrently", + v => z.coerce.number().int().positive().parse(v), + 10 + )- .option("-c, --concurrency <number>", "How many wallets are processed concurrently", value => z.number({ coerce: true }).optional().default(10).parse(value)) + .option( + "-c, --concurrency <number>", + "How many wallets are processed concurrently", + v => z.coerce.number().int().positive().parse(v), + 10 + )- .option("-c, --concurrency <number>", "How many wallets are processed concurrently", value => z.number({ coerce: true }).optional().default(10).parse(value)) + .option( + "-c, --concurrency <number>", + "How many wallets are processed concurrently", + v => z.coerce.number().int().positive().parse(v), + 10 + )- .option("-c, --concurrency <number>", "How many users are processed concurrently", value => z.number({ coerce: true }).optional().default(10).parse(value)) + .option( + "-c, --concurrency <number>", + "How many users are processed concurrently", + v => z.coerce.number().int().positive().parse(v), + 10 + )Also applies to: 53-53, 63-63, 73-73, 95-95
71-80: Make provider address required and validate non-empty inputThis command appears to require a provider address. As written, the Zod parser is only invoked when the option is present, so the command can run with provider = undefined.
Apply this diff:
- .option("-p, --provider <string>", "Provider address", value => z.string().parse(value)) + .requiredOption( + "-p, --provider <string>", + "Provider address", + v => z.string().min(1, "Provider address is required").parse(v) + )(Optional) If you want to validate Akash-style bech32 addresses, add a stricter regex/refinement later.
105-131: End the OpenTelemetry span; current code leaks spans
tracer.startSpan(name)is called but the span is never ended, which will skew traces and can leak memory/resources. Wrap with a finally and callspan.end()after the handler completes. While here, guard APP_INITIALIZER calls to avoid calling undefined.Apply this diff:
-async function executeCliHandler(name: string, handler: () => Promise<unknown>, options?: { type?: "action" | "daemon" }) { - await context.with(trace.setSpan(context.active(), tracer.startSpan(name)), async () => { - logger.info({ event: "COMMAND_START", name }); - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { migratePG } = require("./core/providers/postgres.provider"); - - try { - await Promise.all([migratePG(), chainDb.authenticate(), ...container.resolveAll(APP_INITIALIZER).map(initializer => initializer[ON_APP_START]())]); - - const result = await handler(); - - if (result && result instanceof Err) { - logger.error({ event: "COMMAND_ERROR", name, result: result.val }); - process.exitCode = 1; - } else { - logger.info({ event: "COMMAND_END", name }); - } - } catch (error) { - logger.error({ event: "COMMAND_ERROR", name, error }); - process.exitCode = 1; - } finally { - if (options?.type !== "daemon") { - await shutdown(); - } - } - }); -} +async function executeCliHandler( + name: string, + handler: () => Promise<unknown>, + options?: { type?: "action" | "daemon" }, +) { + const span = tracer.startSpan(name); + try { + await context.with(trace.setSpan(context.active(), span), async () => { + logger.info({ event: "COMMAND_START", name }); + // eslint-disable-next-line @typescript-eslint/no-var-requires + const { migratePG } = require("./core/providers/postgres.provider"); + + try { + await Promise.all([ + migratePG(), + chainDb.authenticate(), + ...container.resolveAll(APP_INITIALIZER).map(initializer => initializer[ON_APP_START]?.()), + ]); + + const result = await handler(); + + if (result && result instanceof Err) { + logger.error({ event: "COMMAND_ERROR", name, result: result.val }); + process.exitCode = 1; + } else { + logger.info({ event: "COMMAND_END", name }); + } + } catch (error) { + logger.error({ event: "COMMAND_ERROR", name, error }); + process.exitCode = 1; + } finally { + if (options?.type !== "daemon") { + await shutdown(); + } + } + }); + } finally { + span.end(); + } +}
♻️ Duplicate comments (7)
apps/api/env/.env.unit.test (2)
28-28: Align docker-compose host port to 3090 (duplicate of prior note) and re-verify for lingering 3081 referencesA previous review found a lingering host port mapping
'3081:3001'in packages/docker/docker-compose.prod.yml and advised updating dev compose too. Please ensure all compose files and local scripts map host 3090 for the Notifications container.Run the script below to surface stragglers and context around usages:
#!/bin/bash set -euo pipefail echo "== Compose mappings still using 3081:3001 ==" rg -nP -S --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' "(^|\s)3081:3001\b" -g '*compose*.yml' -g '*compose*.yaml' -n -C2 echo echo "== Any hardcoded 3081 references across repo ==" rg -nP -S --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' '\b3081\b' -C2 echo echo "== Usages of NOTIFICATIONS_API_BASE_URL (to confirm 3090 in tests/mocks) ==" rg -nP --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' '\bNOTIFICATIONS_API_BASE_URL\b' -C3
28-28: dotenv-linter UnorderedKey warning: optional reorderingdotenv-linter flags NOTIFICATIONS_API_BASE_URL as out of order relative to POSTGRES_DB_URI. If you keep lint clean, move NOTIFICATIONS_API_BASE_URL earlier (before POSTGRES_DB_URI) or adopt a consistent alphabetical/grouped ordering for keys. Otherwise, consider relaxing that rule for test env files.
apps/api/src/core/services/shutdown-server/shutdown-server.spec.ts (1)
1-101: Sanity-check the repo for leftover CONTAINER_DISPOSE_ERROR usagesTo ensure consistency across test and implementation code, run this quick audit. Similar check was suggested earlier; keeping it here for convenience.
#!/bin/bash set -euo pipefail echo "Occurrences of old event key (should be 0):" rg -nP 'CONTAINER_DISPOSE_ERROR' -C2 || true echo echo "ON_SHUTDOWN_ERROR call sites for context:" rg -nP --type=ts -C2 'ON_SHUTDOWN_ERROR' || trueapps/api/src/core/services/job-queue/job-queue.service.spec.ts (2)
150-153: Avoid any in pgBoss.work mock (failure path).Mirror the typed mock used above for consistency and to satisfy the no-any rule.
Apply this diff:
-jest.spyOn(pgBoss, "work").mockImplementation(async (queueName: string, options: any, processFn: (jobs: any[]) => Promise<void>) => { - await processFn(jobs); - return "work-id"; -}); +jest + .spyOn(pgBoss, "work") + .mockImplementation( + async ( + _queueName: string, + _options: Parameters<PgBoss["work"]>[1], + processFn: (jobs: Array<{ id: string; data: TestJob["data"] }>) => Promise<void> + ) => { + await processFn(jobs); + return "work-id"; + } + );
173-176: Avoid any in pgBoss.work mock (default-options path).Keep mocks consistently typed and compliant with the no-any guideline.
Apply this diff:
-jest.spyOn(pgBoss, "work").mockImplementation(async (queueName: string, options: any, processFn: (jobs: any[]) => Promise<void>) => { - await processFn([{ id: "1", data: { message: "Job 1", userId: "user-1" } }]); - return "work-id"; -}); +jest + .spyOn(pgBoss, "work") + .mockImplementation( + async ( + _queueName: string, + _options: Parameters<PgBoss["work"]>[1], + processFn: (jobs: Array<{ id: string; data: TestJob["data"] }>) => Promise<void> + ) => { + await processFn([{ id: "1", data: { message: "Job 1", userId: "user-1" } }]); + return "work-id"; + } + );apps/api/src/rest-app.ts (1)
194-194: Alias export initDb is helpful; ensure all call sites use it (no direct connectUsingSequelize imports).Keeps the public surface small and swappable. Please verify remaining call sites don’t bypass the alias.
Run:
#!/bin/bash # Find any lingering direct imports/usages of connectUsingSequelize outside rest-app rg -nP --type ts -C2 '\bconnectUsingSequelize\b' | rg -v 'src/rest-app\.ts'apps/api/src/core/providers/index.ts (1)
5-8: Barrel looks good; re-exports provide a clean public surface (plus job-queue-healthcheck)The exposed tokens/types look coherent for DI. Please ensure no modules still import "./postgres.provider" directly, which would bypass the barrel and risk initialization-order discrepancies.
Run to spot direct imports and potential cycles:
#!/bin/bash # 1) No direct postgres.provider imports outside the barrel rg -nP --type ts -C2 "(from|import)\\s+['\"][^'\"]*core/providers/postgres\\.provider['\"]" # 2) Prefer barrel usage everywhere rg -nP --type ts -C2 "(from|import)\\s+['\"][^'\"]*core/providers(?!/index)['\"]" # 3) Sanity-check that providers index is imported before any healthcheck resolution rg -nP --type ts -C2 "\\b(DB_HEALTHCHECK|JOB_QUEUE_HEALTHCHECK)\\b"
🧹 Nitpick comments (33)
apps/api/src/core/services/index.ts (2)
2-2: Prefer named re-exports overexport *to avoid leaking internalsStar re-exports can unintentionally widen the public API (including helper types/constants). Consider re-exporting only what you intend to support long-term.
Apply this diff to narrow the surface:
-export * from "@src/core/services/start-server/start-server"; +export { startServer } from "@src/core/services/start-server/start-server";If you need to expose types, add explicit type-only re-exports next to this line.
2-2: Verification: no direct circular deps; alias mapping present for build
- apps/api/src/core/services/start-server/start-server.ts contains no imports from
@src/coreor@src/core/services, so no immediate re-export cycle detected.- Duplicate re-export of
start-serverfound in two barrels:
- apps/api/src/core/index.ts:6
- apps/api/src/core/services/index.ts:2
Consider removing one to avoid redundancy (e.g. drop the root-level export inapps/api/src/core/index.ts).- The
@src/*alias is correctly configured in apps/api/tsconfig.build.json (baseUrl: ".",paths: ["./src/*"]), so your package build will resolve it.
- Note: apps/api/tsconfig.json (editor config) has no
pathsmapping—add the following if you need IDE support:{ "compilerOptions": { + "baseUrl": ".", + "paths": { + "@src/*": ["./src/*"] + } } }No blocking issues found; duplicate export cleanup and editor-side alias mapping are optional refinements.
apps/api/src/core/services/shutdown-server/shutdown-server.spec.ts (4)
41-45: Avoid unsafe type cast; type your jest.fn instead ofas jest.MockCasting to
jest.Mockhides type mismatches and can mask regressions. Prefer a typedjest.fnsignature that matchesServerType["close"].- const server = mock<ServerType>({ - close: jest.fn().mockImplementation(() => { - throw error; - }) as jest.Mock - }); + const server = mock<ServerType>({ + close: jest + .fn<(cb: (err?: Error) => void) => void>() + .mockImplementation(() => { + throw error; + }) + });
32-36: Make logger expectations resilient to extra log fieldsUse
expect.objectContainingso future additions to the log payload (e.g., requestId, tags) don’t break tests.- expect(appLogger.error).toHaveBeenCalledWith({ - event: "SERVER_CLOSE_ERROR", - error - }); + expect(appLogger.error).toHaveBeenCalledWith(expect.objectContaining({ event: "SERVER_CLOSE_ERROR", error }));- expect(appLogger.error).toHaveBeenCalledWith({ - event: "SERVER_CLOSE_ERROR", - error - }); + expect(appLogger.error).toHaveBeenCalledWith(expect.objectContaining({ event: "SERVER_CLOSE_ERROR", error }));- expect(appLogger.error).toHaveBeenCalledWith({ - event: "ON_SHUTDOWN_ERROR", - error - }); + expect(appLogger.error).toHaveBeenCalledWith(expect.objectContaining({ event: "ON_SHUTDOWN_ERROR", error }));Also applies to: 51-55, 68-71
8-8: Remove trailing whitespace in test nameMinor nit; keeps diffs clean and avoids noisy lint failures.
-it("closes the server ", async () => { +it("closes the server", async () => {
10-11: Simulate async server.close callback schedulingIn Node,
server.close(cb)invokes the callback on a future tick. Scheduling withsetImmediate(orsetTimeout(…, 0)) makes tests closer to real behavior and catches accidental sync-only code paths.If you don’t adopt the setup() refactor yet, tweak the inline fakes:
- close: jest.fn().mockImplementation(cb => cb()) + close: jest.fn().mockImplementation(cb => setImmediate(() => cb()))- close: jest.fn().mockImplementation(cb => cb()) + close: jest.fn().mockImplementation(cb => setImmediate(() => cb()))Also applies to: 91-92
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (3)
156-160: Simplify error assertion; avoid Promise.allSettled for a single promise.Use Jest’s rejects helper and assert the logged error directly. This keeps the test tighter and avoids type narrowing on PromiseSettledResult.
Apply this diff:
-const [result] = await Promise.allSettled([service.startWorkers({ batchSize: 1 })]); - -expect(result.status).toBe("rejected"); -expect((result as PromiseRejectedResult).reason).toBe(error); +await expect(service.startWorkers({ batchSize: 1 })).rejects.toBe(error); @@ -expect(logger.error).toHaveBeenCalledWith({ - event: "JOB_FAILED", - jobId: jobs[0].id, - error: (result as PromiseRejectedResult).reason -}); +expect(logger.error).toHaveBeenCalledWith({ + event: "JOB_FAILED", + jobId: jobs[0].id, + error +});Also applies to: 164-164
233-239: Remove ts-expect-error by typing the getDb mock once.Avoid repeating ts-expect-error; use a minimal local type for the DB handle and assert on the mock function directly.
Apply this diff:
-// @ts-expect-error - getDb is not typed, see https://github.com/timgit/pg-boss/issues/552#issuecomment-3213043039 -jest.spyOn(pgBoss, "getDb").mockReturnValue({ executeSql: jest.fn().mockResolvedValue(undefined) }); -await service.ping(); - -// @ts-expect-error - getDb is not typed, see https://github.com/timgit/pg-boss/issues/552#issuecomment-3213043039 -expect(pgBoss.getDb().executeSql).toHaveBeenCalledWith("SELECT 1"); +type PGBossDb = { executeSql: (sql: string) => Promise<unknown> }; +const executeSql = jest.fn().mockResolvedValue(undefined); +jest.spyOn(pgBoss, "getDb").mockReturnValue({ executeSql } as unknown as PGBossDb); +await service.ping(); +expect(executeSql).toHaveBeenCalledWith("SELECT 1");
259-261: Minor: simplify pgBoss injection check.Object.hasOwn is fine, but in guards like this the in operator is clearer.
Apply this diff:
-const service = new JobQueueService(mocks.logger, mocks.coreConfig, input && Object.hasOwn(input, "pgBoss") ? input?.pgBoss : mocks.pgBoss); +const service = new JobQueueService(mocks.logger, mocks.coreConfig, input && "pgBoss" in input ? input.pgBoss : mocks.pgBoss);apps/api/src/console.ts (6)
83-84: Polish command description (grammar/casing)Minor wording improvement.
- .description("Create deployments for every gpu models to get up to date pricing information") + .description("Create deployments for every GPU model to get up-to-date pricing information")
51-53: Polish description wordingMinor wording improvement.
- .description("Close deployments without leases created at least 10min ago") + .description("Close deployments without leases created at least 10 min ago")
91-96: Avoid resolving DI at module load for help text (defers side effects)Resolving UserConfigService at module scope may trigger DI construction before APP_INITIALIZER runs. If UserConfigService has any I/O, this can fail or slow
--help. Consider using a static description and logging the actual value at runtime, or compute the description lazily inside the action.If you want to avoid module-scope DI while keeping a dynamic help string, an acceptable compromise is a static description:
- .description(`Remove users that have been inactive for ${userConfig.get("STALE_ANONYMOUS_USERS_LIVE_IN_DAYS")} days`) + .description("Remove users that have been inactive for the configured number of days")…and log the resolved days at command start.
133-138: Verify tsyringe supports container.dispose() and make shutdown more robustTwo points:
- Some tsyringe versions don’t expose
container.dispose(). If unavailable, prefercontainer.reset()orcontainer.clearInstances().- Use
Promise.allSettledso one failure (e.g., DB close) doesn’t prevent other cleanups.Proposed improvement (after verifying available API):
- await Promise.all([closeConnections(), chainDb.close(), container.dispose()]); + await Promise.allSettled([closeConnections(), chainDb.close(), container.dispose?.() ?? Promise.resolve()]);If
disposeis not available, replace withcontainer.reset()(synchronous) orcontainer.clearInstances()as appropriate.
141-141: Prefer parseAsync() for async actionsYour actions are async and you do internal error handling, but using Commander’s async entry improves consistency and future-proofs error propagation.
-program.parse(); +void program.parseAsync();
53-53: Minor grammar nit“wallets are processed” reads better.
- .option("-c, --concurrency <number>", "How many wallets is processed concurrently", … + .option("-c, --concurrency <number>", "How many wallets are processed concurrently", …apps/api/src/core/providers/job-queue-healthcheck.ts (1)
9-17: Prefer a Symbol token and the project’s root DI container to avoid token collisions and container drift.
- Use Symbol("JOB_QUEUE_HEALTHCHECK") for the token (consistent with APP_INITIALIZER) to prevent accidental string token clashes.
- Register against the same root DI container used by startServer (rootContainer), not the global tsyringe container, to avoid surprises when an alternate container is supplied in tests/server bootstrap.
Apply:
- import type { InjectionToken } from "tsyringe"; - import { container } from "tsyringe"; + import type { InjectionToken } from "tsyringe"; + import { rootContainer as container } from "@src/core/di"; @@ -export const JOB_QUEUE_HEALTHCHECK: InjectionToken<JobQueueHealthcheck> = "JOB_QUEUE_HEALTHCHECK"; +export const JOB_QUEUE_HEALTHCHECK: InjectionToken<JobQueueHealthcheck> = Symbol("JOB_QUEUE_HEALTHCHECK"); container.register(JOB_QUEUE_HEALTHCHECK, { useFactory: c => { const jobQueueService = c.resolve(JobQueueService); return { ping: () => jobQueueService.ping() }; } });apps/api/src/db/dbConnection.ts (3)
55-63: Register DB shutdown under APP_SHUTDOWN as well to guarantee pool closure in all shutdown paths.Relying solely on the initializer instance’s Disposable.dispose being called by container.dispose can be fragile if the initializer wasn’t resolved or if shutdown logic changes. Also exposing closeConnections via APP_SHUTDOWN makes intent explicit and aligns with the broader shutdown orchestration.
Proposed change:
- import { APP_INITIALIZER, ON_APP_START } from "@src/core/providers/app-initializer"; + import { APP_INITIALIZER, APP_SHUTDOWN, ON_APP_START } from "@src/core/providers/app-initializer"; @@ container.register(APP_INITIALIZER, { useFactory: c => ({ async [ON_APP_START]() { await connectUsingSequelize(c.resolve(LoggerService)); }, dispose: closeConnections }) satisfies AppInitializer & Disposable }); + +// Ensure DB pools are closed even if initializers weren’t resolved or disposal strategy changes +container.register(APP_SHUTDOWN, { + useValue: { + dispose: closeConnections + } +});If you adopt this, please ensure start-server invokes APP_SHUTDOWN disposers before container.dispose(). If that enhancement already landed in this PR, we’re covered. Otherwise, I can provide a small patch for start-server.ts.
65-70: Docblock overpromises vs current implementation.The comment lists “Populate db”, “Create backups per version”, “Load from backup” which aren’t implemented here. Tighten the doc to reflect actual behavior to avoid misleading readers.
-/** - * Initialize database schema - * Populate db - * Create backups per version - * Load from backup if exists for current version - */ +/** + * Establish connections to chainDb and userDb, then sync user schema. + * NOTE: Seeding/backups are handled elsewhere and are not part of this function. + */
71-83: Optionally gate syncUserSchema behind config to avoid unintended DDL in prod.If Drizzle migrations (migratePG) are the primary schema mechanism in production, consider making Sequelize sync opt-in via config. Keeps tests/dev happy while preventing accidental DDL in prod.
Example:
export async function connectUsingSequelize(logger: LoggerService = LoggerService.forContext("DB")): Promise<void> { logger.debug(`Connecting to chain database (${chainDb.config.host}/${chainDb.config.database})...`); await chainDb.authenticate(); logger.debug("Connection has been established successfully."); logger.debug(`Connecting to user database (${userDb.config.host}/${userDb.config.database})...`); await userDb.authenticate(); logger.debug("Connection has been established successfully."); - logger.debug("Sync user schema..."); - await syncUserSchema(); - logger.debug("User schema synced."); + const shouldSync = coreConfig.get("SEQUELIZE_SYNC_ON_START") === "true"; + if (shouldSync) { + logger.debug("Sync user schema..."); + await syncUserSchema(); + logger.debug("User schema synced."); + } else { + logger.debug("Skipping user schema sync (SEQUELIZE_SYNC_ON_START !== 'true')."); + } }If some tests rely on sync always running, we can set SEQUELIZE_SYNC_ON_START="true" in the test env.
apps/api/src/rest-app.ts (2)
186-192: Add a disposer for Scheduler to avoid dangling timers on shutdown.startServer now coordinates graceful shutdown; add a dispose hook so timers/intervals are cleared even if the server fails to start or exits quickly.
- container.register(APP_INITIALIZER, { - useValue: { - async [ON_APP_START]() { - scheduler.start(); - } - } satisfies AppInitializer - }); +import type { Disposable } from "tsyringe"; +container.register(APP_INITIALIZER, { + useValue: { + async [ON_APP_START]() { + scheduler.start(); + }, + // Prefer an explicit stop if available + dispose: () => (scheduler.stop?.() ?? scheduler.dispose?.()) + } satisfies AppInitializer & Disposable +});If Scheduler lacks stop/dispose, I recommend adding one. Otherwise we risk leaked intervals in long-lived processes or tests.
196-201: bootstrap: consider returning the server instance (optional).Returning ServerType | undefined helps callers/tests to close or introspect the server without re-wiring shutdown hooks.
-export async function bootstrap(port: number) { - await startServer(appHono, LoggerService.forContext("APP"), process, { - port, - beforeStart: migratePG - }); -} +import type { ServerType } from "./core/types"; +export async function bootstrap(port: number): Promise<ServerType | undefined> { + return startServer(appHono, LoggerService.forContext("APP"), process, { + port, + beforeStart: migratePG + }); +}apps/api/test/functional/docs.spec.ts (1)
6-7: Optional: adopt the repo’s setup() pattern for tests.Guidelines recommend a setup function over Jest lifecycle calls. Here, timers are set inline; consider moving them into a local setup() near the bottom of the root describe for consistency.
Example:
// at bottom of the root describe function setup({ now = new Date("2025-07-03T12:00:00.000Z") }: { now?: Date } = {}) { jest.useFakeTimers(); jest.setSystemTime(now); return {}; }Then call setup() at the start of the test.
apps/api/src/core/providers/index.ts (1)
3-3: Remove redundant side‑effect import of postgres.providerSince you already re-export from "./postgres.provider", the module is evaluated once via that export; an extra side-effect import is unnecessary.
-import "./postgres.provider";apps/api/src/core/providers/postgres.provider.ts (3)
56-57: Optional: make closeConnections idempotent and resilientIf APP_INITIALIZER gets resolved/disposed more than once (tests or hot-reloads), double-closing could throw. Consider guarding and logging on failure so shutdown never hangs the process.
Example (outside the selected range, for illustration only):
let pgClosed = false; export const closeConnections = async () => { if (pgClosed) return; pgClosed = true; try { await Promise.all([migrationClient.end(), appClient.end()]); } catch (err) { logger.warn({ event: "POSTGRES_CLOSE_ERROR", err }); } };
6-6: Register APP_INITIALIZER as a singleton to avoid multiple disposablesBy default, tsyringe uses transient lifecycle; registering as a singleton prevents multiple instances (and multiple dispose calls) if resolved more than once.
-import type { Disposable, InjectionToken } from "tsyringe"; +import { Lifecycle, type Disposable, type InjectionToken } from "tsyringe";
58-64: Add singleton lifecycle to APP_INITIALIZER registrationPrevents duplicate initializers and duplicate dispose calls across multiple resolutions.
container.register(APP_INITIALIZER, { useFactory: () => ({ [ON_APP_START]: () => Promise.resolve(), dispose: closeConnections }) satisfies AppInitializer & Disposable -}); + , lifecycle: Lifecycle.Singleton +});apps/api/src/healthz/services/healthz/healthz.service.spec.ts (2)
73-111: Liveness covers happy path and dual failures; consider verifying cache miss/hit counters if availableCoverage is strong. If the health service exposes metrics/counters, asserting cache hit/miss would further harden expectations.
201-206: Setup helper is clear and colocated; optional: accept a param for overridesThe setup() meets internal guidelines (bottom of the root describe, no shared state). Optionally accept a single param for overrides (e.g., pre-set ping behaviors or custom TTL) to improve flexibility in future tests.
apps/api/src/healthz/services/healthz/healthz.service.ts (5)
100-107: Make check() return type explicit and guarantee a non-undefined Promise.As written, TypeScript can infer Promise | undefined because you return a possibly-undefined field. It works at runtime, but explicit typing avoids surprises under strict settings.
- private check() { - this.inflightPing ??= this.healthchecker.ping().finally(() => { - this.checkedAt = new Date(); - this.inflightPing = undefined; - }); - - return this.inflightPing; - } + private check(): Promise<void> { + const ping = + this.inflightPing ?? + (this.inflightPing = this.healthchecker.ping().finally(() => { + this.checkedAt = new Date(); + this.inflightPing = undefined; + })); + return ping; + }
68-69: Narrow the healthchecker type to a simple “Pingable” interface.Union-in-Pick is needlessly complex and can widen types. A tiny local interface makes intent clear and keeps compile-time checks tight.
+type Pingable = { ping: () => Promise<void> }; @@ - private readonly healthchecker: Pick<DbHealthcheck | JobQueueHealthcheck, "ping">, + private readonly healthchecker: Pingable,
12-26: Make TTL configurable; default to a constant.Hard-coding millisecondsInMinute is fine for now, but making it a named constant (and optionally overridable) eases tuning and testing.
+const DEFAULT_HEALTHCHECK_TTL_MS = millisecondsInMinute; @@ - new Healthcheck("postgres", dbHealthcheck, logger, { - cacheTTL: millisecondsInMinute - }) + new Healthcheck("postgres", dbHealthcheck, logger, { + cacheTTL: DEFAULT_HEALTHCHECK_TTL_MS + }) @@ - new Healthcheck("jobQueue", jobQueueHealthcheck, logger, { - cacheTTL: millisecondsInMinute - }) + new Healthcheck("jobQueue", jobQueueHealthcheck, logger, { + cacheTTL: DEFAULT_HEALTHCHECK_TTL_MS + })If you already have a config provider, consider injecting the TTL instead of a constant.
39-49: Avoid dynamic key casts when building the data object.The reduce + type-assertion works but obscures the data shape and relies on casts. Building the map in a type-safe way improves maintainability.
Option A (explicit keys; simplest and most type-safe for fixed checks):
- private buildResult(results: boolean[]): HealthzResult { - return { - status: results.every(Boolean) ? "ok" : "error", - data: results.reduce( - (acc, result, index) => { - acc[this.healthchecks[index].name as keyof HealthzResult["data"]] = result; - return acc; - }, - {} as HealthzResult["data"] - ) - }; - } + private buildResult(results: boolean[]): HealthzResult { + const data: HealthzResult["data"] = { + postgres: results[this.healthchecks.findIndex(h => h.name === "postgres")], + jobQueue: results[this.healthchecks.findIndex(h => h.name === "jobQueue")] + }; + return { + status: Object.values(data).every(Boolean) ? "ok" : "error", + data + }; + }Option B (keep dynamic but improve typing): define
type HealthcheckName = keyof HealthzResult["data"]and typename: HealthcheckNamein Healthcheck.
86-90: Consider logging normalized error details.Passing raw error is fine if your logger handles unknown objects. If you want uniformity, log message and stack explicitly.
- this.logger.error({ - event: `${this.name.toUpperCase()}_HEALTHCHECK_ERROR`, - error - }); + const err = error as unknown as { message?: string; stack?: string }; + this.logger.error({ + event: `${this.name.toUpperCase()}_HEALTHCHECK_ERROR`, + message: err?.message, + stack: err?.stack + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (73)
apps/api/env/.env.functional.test(1 hunks)apps/api/env/.env.local.sample(1 hunks)apps/api/env/.env.unit.test(1 hunks)apps/api/src/app/index.ts(1 hunks)apps/api/src/app/providers/jobs.provider.ts(1 hunks)apps/api/src/background-jobs-app.ts(1 hunks)apps/api/src/console.ts(1 hunks)apps/api/src/core/config/env.config.ts(1 hunks)apps/api/src/core/index.ts(1 hunks)apps/api/src/core/providers/index.ts(1 hunks)apps/api/src/core/providers/job-queue-healthcheck.ts(1 hunks)apps/api/src/core/providers/postgres.provider.ts(4 hunks)apps/api/src/core/services/index.ts(1 hunks)apps/api/src/core/services/job-queue/job-queue.service.spec.ts(5 hunks)apps/api/src/core/services/job-queue/job-queue.service.ts(4 hunks)apps/api/src/core/services/shutdown-server/shutdown-server.spec.ts(1 hunks)apps/api/src/core/services/shutdown-server/shutdown-server.ts(1 hunks)apps/api/src/core/services/start-server/start-server.spec.ts(1 hunks)apps/api/src/core/services/start-server/start-server.ts(1 hunks)apps/api/src/db/dbConnection.ts(2 hunks)apps/api/src/healthz/controllers/healthz/healthz.controller.spec.ts(2 hunks)apps/api/src/healthz/routes/healthz.router.ts(1 hunks)apps/api/src/healthz/services/healthz/healthz.service.spec.ts(1 hunks)apps/api/src/healthz/services/healthz/healthz.service.ts(1 hunks)apps/api/src/index.ts(0 hunks)apps/api/src/notifications/services/notification-handler/notification.handler.ts(1 hunks)apps/api/src/rest-app.ts(4 hunks)apps/api/src/server.ts(1 hunks)apps/api/test/functional/addresses.spec.ts(2 hunks)apps/api/test/functional/anonymous-user.spec.ts(1 hunks)apps/api/test/functional/api-key.spec.ts(1 hunks)apps/api/test/functional/app.spec.ts(1 hunks)apps/api/test/functional/auditors.spec.ts(1 hunks)apps/api/test/functional/balances.spec.ts(1 hunks)apps/api/test/functional/bids.spec.ts(1 hunks)apps/api/test/functional/blocks.spec.ts(1 hunks)apps/api/test/functional/certificate.spec.ts(1 hunks)apps/api/test/functional/create-deployment.spec.ts(1 hunks)apps/api/test/functional/dashboard-data.spec.ts(1 hunks)apps/api/test/functional/deployment-setting.spec.ts(1 hunks)apps/api/test/functional/deployments.spec.ts(1 hunks)apps/api/test/functional/docs.spec.ts(1 hunks)apps/api/test/functional/gpu.spec.ts(1 hunks)apps/api/test/functional/graph-data.spec.ts(1 hunks)apps/api/test/functional/lease-flow.spec.ts(1 hunks)apps/api/test/functional/leases-duration.spec.ts(1 hunks)apps/api/test/functional/market-data.spec.ts(1 hunks)apps/api/test/functional/network-capacity.spec.ts(1 hunks)apps/api/test/functional/nodes-v1.spec.ts(1 hunks)apps/api/test/functional/pricing.spec.ts(1 hunks)apps/api/test/functional/proposals.spec.ts(1 hunks)apps/api/test/functional/provider-attributes-schema.spec.ts(1 hunks)apps/api/test/functional/provider-dashboard.spec.ts(1 hunks)apps/api/test/functional/provider-deployments.spec.ts(1 hunks)apps/api/test/functional/provider-earnings.spec.ts(1 hunks)apps/api/test/functional/provider-graph-data.spec.ts(1 hunks)apps/api/test/functional/provider-regions.spec.ts(1 hunks)apps/api/test/functional/provider-versions.spec.ts(1 hunks)apps/api/test/functional/providers.spec.ts(1 hunks)apps/api/test/functional/sign-and-broadcast-tx.spec.ts(1 hunks)apps/api/test/functional/stale-anonymous-users-cleanup.spec.ts(1 hunks)apps/api/test/functional/start-trial.spec.ts(1 hunks)apps/api/test/functional/stripe-webhook.spec.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)apps/api/test/functional/transactions.spec.ts(1 hunks)apps/api/test/functional/usage.spec.ts(1 hunks)apps/api/test/functional/user-init.spec.ts(1 hunks)apps/api/test/functional/validators.spec.ts(1 hunks)apps/api/test/functional/wallets-refill.spec.ts(1 hunks)apps/api/tsconfig.json(1 hunks)apps/api/webpack.dev.js(2 hunks)apps/api/webpack.prod.js(2 hunks)packages/docker/docker-compose.prod.yml(2 hunks)
💤 Files with no reviewable changes (1)
- apps/api/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (56)
- apps/api/test/functional/stripe-webhook.spec.ts
- apps/api/src/core/services/shutdown-server/shutdown-server.ts
- apps/api/test/functional/app.spec.ts
- apps/api/env/.env.local.sample
- apps/api/test/functional/gpu.spec.ts
- apps/api/test/functional/network-capacity.spec.ts
- apps/api/test/functional/lease-flow.spec.ts
- apps/api/test/functional/provider-graph-data.spec.ts
- apps/api/test/functional/balances.spec.ts
- apps/api/test/functional/providers.spec.ts
- apps/api/test/functional/provider-versions.spec.ts
- apps/api/src/background-jobs-app.ts
- apps/api/test/functional/leases-duration.spec.ts
- apps/api/webpack.prod.js
- apps/api/test/functional/blocks.spec.ts
- apps/api/src/core/config/env.config.ts
- apps/api/src/app/index.ts
- apps/api/test/functional/proposals.spec.ts
- apps/api/test/functional/certificate.spec.ts
- apps/api/test/functional/start-trial.spec.ts
- apps/api/tsconfig.json
- apps/api/test/functional/pricing.spec.ts
- apps/api/src/app/providers/jobs.provider.ts
- apps/api/src/core/services/start-server/start-server.spec.ts
- apps/api/test/functional/provider-regions.spec.ts
- apps/api/test/functional/anonymous-user.spec.ts
- apps/api/test/functional/create-deployment.spec.ts
- apps/api/test/functional/provider-attributes-schema.spec.ts
- apps/api/env/.env.functional.test
- packages/docker/docker-compose.prod.yml
- apps/api/test/functional/graph-data.spec.ts
- apps/api/test/functional/provider-dashboard.spec.ts
- apps/api/test/functional/deployment-setting.spec.ts
- apps/api/test/functional/auditors.spec.ts
- apps/api/src/healthz/controllers/healthz/healthz.controller.spec.ts
- apps/api/test/functional/bids.spec.ts
- apps/api/test/functional/provider-earnings.spec.ts
- apps/api/test/functional/templates.spec.ts
- apps/api/webpack.dev.js
- apps/api/test/functional/deployments.spec.ts
- apps/api/test/functional/dashboard-data.spec.ts
- apps/api/test/functional/transactions.spec.ts
- apps/api/test/functional/usage.spec.ts
- apps/api/test/functional/provider-deployments.spec.ts
- apps/api/src/server.ts
- apps/api/src/healthz/routes/healthz.router.ts
- apps/api/test/functional/nodes-v1.spec.ts
- apps/api/test/functional/api-key.spec.ts
- apps/api/test/functional/market-data.spec.ts
- apps/api/src/core/index.ts
- apps/api/src/core/services/start-server/start-server.ts
- apps/api/test/functional/user-init.spec.ts
- apps/api/src/core/services/job-queue/job-queue.service.ts
- apps/api/src/notifications/services/notification-handler/notification.handler.ts
- apps/api/test/functional/wallets-refill.spec.ts
- apps/api/test/functional/addresses.spec.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/src/core/services/index.tsapps/api/src/core/services/shutdown-server/shutdown-server.spec.tsapps/api/src/core/providers/job-queue-healthcheck.tsapps/api/src/core/providers/postgres.provider.tsapps/api/src/healthz/services/healthz/healthz.service.tsapps/api/test/functional/validators.spec.tsapps/api/test/functional/sign-and-broadcast-tx.spec.tsapps/api/test/functional/docs.spec.tsapps/api/test/functional/stale-anonymous-users-cleanup.spec.tsapps/api/src/healthz/services/healthz/healthz.service.spec.tsapps/api/src/db/dbConnection.tsapps/api/src/core/providers/index.tsapps/api/src/console.tsapps/api/src/rest-app.tsapps/api/src/core/services/job-queue/job-queue.service.spec.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/src/core/services/index.tsapps/api/src/core/services/shutdown-server/shutdown-server.spec.tsapps/api/src/core/providers/job-queue-healthcheck.tsapps/api/src/core/providers/postgres.provider.tsapps/api/src/healthz/services/healthz/healthz.service.tsapps/api/test/functional/validators.spec.tsapps/api/test/functional/sign-and-broadcast-tx.spec.tsapps/api/test/functional/docs.spec.tsapps/api/test/functional/stale-anonymous-users-cleanup.spec.tsapps/api/src/healthz/services/healthz/healthz.service.spec.tsapps/api/src/db/dbConnection.tsapps/api/src/core/providers/index.tsapps/api/src/console.tsapps/api/src/rest-app.tsapps/api/src/core/services/job-queue/job-queue.service.spec.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/src/core/services/shutdown-server/shutdown-server.spec.tsapps/api/test/functional/validators.spec.tsapps/api/test/functional/sign-and-broadcast-tx.spec.tsapps/api/test/functional/docs.spec.tsapps/api/test/functional/stale-anonymous-users-cleanup.spec.tsapps/api/src/healthz/services/healthz/healthz.service.spec.tsapps/api/src/core/services/job-queue/job-queue.service.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/api/src/core/services/job-queue/job-queue.service.spec.ts
🧬 Code graph analysis (6)
apps/api/src/core/providers/postgres.provider.ts (5)
apps/api/src/db/dbConnection.ts (2)
closeConnections(54-54)ON_APP_START(58-60)apps/api/src/core/providers/app-initializer.ts (3)
APP_INITIALIZER(3-3)ON_APP_START(4-4)AppInitializer(6-8)apps/api/src/app/providers/jobs.provider.ts (1)
ON_APP_START(11-19)apps/api/src/rest-app.ts (1)
ON_APP_START(188-190)apps/api/src/core/services/feature-flags/feature-flags.service.ts (1)
ON_APP_START(80-82)
apps/api/src/healthz/services/healthz/healthz.service.ts (5)
apps/api/src/core/providers/logging.provider.ts (1)
injectable(7-8)apps/api/src/healthz/controllers/healthz/healthz.controller.ts (1)
injectable(6-17)apps/api/src/core/providers/postgres.provider.ts (2)
DB_HEALTHCHECK(33-33)DbHealthcheck(30-32)apps/api/src/core/providers/job-queue-healthcheck.ts (2)
JOB_QUEUE_HEALTHCHECK(9-9)JobQueueHealthcheck(6-8)packages/logging/src/servicies/logger/logger.service.ts (1)
error(107-109)
apps/api/src/healthz/services/healthz/healthz.service.spec.ts (4)
apps/api/src/caching/helpers.ts (1)
cacheEngine(8-8)packages/logging/src/servicies/logger/logger.service.ts (1)
error(107-109)apps/api/src/core/providers/postgres.provider.ts (1)
DbHealthcheck(30-32)apps/api/src/core/providers/job-queue-healthcheck.ts (1)
JobQueueHealthcheck(6-8)
apps/api/src/db/dbConnection.ts (4)
apps/api/src/core/providers/postgres.provider.ts (1)
closeConnections(56-56)apps/api/src/core/providers/app-initializer.ts (2)
APP_INITIALIZER(3-3)AppInitializer(6-8)apps/api/src/app/providers/jobs.provider.ts (1)
ON_APP_START(11-19)apps/api/src/rest-app.ts (2)
ON_APP_START(188-190)connectUsingSequelize(194-194)
apps/api/src/rest-app.ts (6)
apps/api/src/core/providers/app-initializer.ts (2)
APP_INITIALIZER(3-3)AppInitializer(6-8)apps/api/src/app/providers/jobs.provider.ts (1)
ON_APP_START(11-19)apps/api/src/db/dbConnection.ts (1)
ON_APP_START(58-60)apps/api/src/background-jobs-app.ts (1)
bootstrap(9-17)apps/api/src/core/services/start-server/start-server.ts (1)
startServer(18-63)apps/api/src/core/providers/postgres.provider.ts (1)
migratePG(26-26)
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (1)
apps/api/src/core/services/job-queue/job-queue.service.ts (1)
setup(135-142)
🪛 dotenv-linter (3.3.0)
apps/api/env/.env.unit.test
[warning] 28-28: [UnorderedKey] The NOTIFICATIONS_API_BASE_URL key should go before the POSTGRES_DB_URI key
(UnorderedKey)
⏰ 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). (11)
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
| export interface HealthzResult { | ||
| status: "ok" | "error"; | ||
| data: { | ||
| postgres: boolean; | ||
| jobQueue: boolean; | ||
| }; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Align controller types with the new HealthzResult.
Controller currently advertises HealthzResponse & { status: ... }. To avoid structural mismatches, update it to return HealthzResult and import the type from this service.
Proposed change in apps/api/src/healthz/controllers/healthz/healthz.controller.ts:
- async getReadinessStatus(): Promise<HealthzResponse & { status: "ok" | "error" }> {
+ async getReadinessStatus(): Promise<HealthzResult> {
return this.healthzService.getReadinessStatus();
}
- async getLivenessStatus(): Promise<HealthzResponse & { status: "ok" | "error" }> {
+ async getLivenessStatus(): Promise<HealthzResult> {
return this.healthzService.getLivenessStatus();
}To quickly verify repo-wide alignment and provider bindings, run:
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# 1) Where HealthzResponse is defined/used and whether jobQueue is included
rg -n --pretty '\bHealthzResponse\b' -C3
# 2) Ensure controller methods are typed as HealthzResult
rg -n --pretty 'class HealthzController' -C3 apps/api/src
rg -n --pretty '\bHealthzResult\b' -C2 apps/api/src/healthz
# 3) Check that DB_HEALTHCHECK and JOB_QUEUE_HEALTHCHECK are registered/bound
rg -nP --pretty 'register\((DB_HEALTHCHECK|JOB_QUEUE_HEALTHCHECK)\b' -C3 apps/api/src || true
# 4) Confirm LoggerService has setContext
rg -n --pretty '\bclass LoggerService\b' -C3
rg -n --pretty '\bsetContext\s*\(' -C2 || trueLength of output: 286919
Update HealthzController to return HealthzResult (service type).
Short check: healthz.service.ts defines/returns HealthzResult; controller currently types methods as HealthzResponse & { status: "ok" | "error" } (imported from the router). DB/JOB healthcheck providers and LoggerService usage were found—no blocking issues.
Files to change:
- apps/api/src/healthz/controllers/healthz/healthz.controller.ts — replace HealthzResponse-based signatures and import with HealthzResult from the service.
- (note) apps/api/src/healthz/services/healthz/healthz.service.ts defines HealthzResult and implements getReadinessStatus/getLivenessStatus.
Suggested diffs:
Import change:
- import type { HealthzResponse } from "@src/healthz/routes/healthz.router";
+ import type { HealthzResult } from "@src/healthz/services/healthz/healthz.service";Method signatures:
- async getReadinessStatus(): Promise<HealthzResponse & { status: "ok" | "error" }> {
+ async getReadinessStatus(): Promise<HealthzResult> {
return this.healthzService.getReadinessStatus();
}
- async getLivenessStatus(): Promise<HealthzResponse & { status: "ok" | "error" }> {
+ async getLivenessStatus(): Promise<HealthzResult> {
return this.healthzService.getLivenessStatus();
}📝 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 interface HealthzResult { | |
| status: "ok" | "error"; | |
| data: { | |
| postgres: boolean; | |
| jobQueue: boolean; | |
| }; | |
| } | |
| // apps/api/src/healthz/controllers/healthz/healthz.controller.ts | |
| import type { HealthzResult } from "@src/healthz/services/healthz/healthz.service"; | |
| export class HealthzController { | |
| constructor(private readonly healthzService: HealthzService) {} | |
| @Get("readiness") | |
| async getReadinessStatus(): Promise<HealthzResult> { | |
| return this.healthzService.getReadinessStatus(); | |
| } | |
| @Get("liveness") | |
| async getLivenessStatus(): Promise<HealthzResult> { | |
| return this.healthzService.getLivenessStatus(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/healthz/controllers/healthz/healthz.controller.ts (replace the
HealthzResponse-based signatures and import), import HealthzResult from
apps/api/src/healthz/services/healthz/healthz.service.ts instead of
HealthzResponse, then update the controller methods' return type annotations
from HealthzResponse & { status: "ok" | "error" } to HealthzResult; ensure any
uses of the previous HealthzResponse shape are adjusted to match HealthzResult's
data structure (postgres: boolean, jobQueue: boolean) and remove the now-unused
HealthzResponse import.
| const now = Date.now(); | ||
|
|
||
| return dbFailingFor < threshold; | ||
| } | ||
|
|
||
| private async isPostgresConnected(): Promise<boolean> { | ||
| try { | ||
| await this.pg.execute(sql`SELECT 1`); | ||
| this.dbFailedAt = undefined; | ||
| if (options?.ignoreCache || !this.checkedAt || now - this.checkedAt.getTime() > this.options.cacheTTL) { | ||
| await this.check(); | ||
| this.isFailed = false; | ||
| } | ||
|
|
||
| return true; | ||
| } catch (error) { | ||
| this.dbFailedAt = this.dbFailedAt || new Date(); | ||
| this.logger.error(error); | ||
| this.logger.error({ | ||
| event: `${this.name.toUpperCase()}_HEALTHCHECK_ERROR`, | ||
| error | ||
| }); | ||
|
|
||
| return false; | ||
| const prevIsFailed = this.isFailed; | ||
| if (this.isFailed === null || prevIsFailed || options?.ignoreCache) return false; | ||
|
|
||
| this.isFailed = true; | ||
| // tolerate failure for the 1st time and wait for the cache to expire until the next check | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix first-failure tolerance: liveness currently fails on initial ping failure (isFailed === null).
The catch-branch contradicts the comment “tolerate failure for the 1st time”. If the very first check fails (isFailed === null), the function returns false, which can make liveness flap on cold start. Readiness already uses ignoreCache: true, so it remains strict.
Apply this diff to align behavior: tolerate the first failure for liveness, fail on consecutive failures or when ignoreCache is true.
- const prevIsFailed = this.isFailed;
- if (this.isFailed === null || prevIsFailed || options?.ignoreCache) return false;
-
- this.isFailed = true;
- // tolerate failure for the 1st time and wait for the cache to expire until the next check
- return true;
+ const hadPreviousFailure = this.isFailed === true;
+ if (options?.ignoreCache || hadPreviousFailure) return false;
+
+ this.isFailed = true;
+ // tolerate the first failure for liveness; readiness still fails via ignoreCache
+ return true;📝 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 isHealthy(options?: { ignoreCache?: boolean }): Promise<boolean> { | |
| const now = Date.now(); | |
| return dbFailingFor < threshold; | |
| } | |
| private async isPostgresConnected(): Promise<boolean> { | |
| try { | |
| await this.pg.execute(sql`SELECT 1`); | |
| this.dbFailedAt = undefined; | |
| if (options?.ignoreCache || !this.checkedAt || now - this.checkedAt.getTime() > this.options.cacheTTL) { | |
| await this.check(); | |
| this.isFailed = false; | |
| } | |
| return true; | |
| } catch (error) { | |
| this.dbFailedAt = this.dbFailedAt || new Date(); | |
| this.logger.error(error); | |
| this.logger.error({ | |
| event: `${this.name.toUpperCase()}_HEALTHCHECK_ERROR`, | |
| error | |
| }); | |
| return false; | |
| const prevIsFailed = this.isFailed; | |
| if (this.isFailed === null || prevIsFailed || options?.ignoreCache) return false; | |
| this.isFailed = true; | |
| // tolerate failure for the 1st time and wait for the cache to expire until the next check | |
| return true; | |
| } | |
| async isHealthy(options?: { ignoreCache?: boolean }): Promise<boolean> { | |
| const now = Date.now(); | |
| try { | |
| if ( | |
| options?.ignoreCache || | |
| !this.checkedAt || | |
| now - this.checkedAt.getTime() > this.options.cacheTTL | |
| ) { | |
| await this.check(); | |
| this.isFailed = false; | |
| } | |
| return true; | |
| } catch (error) { | |
| this.logger.error({ | |
| event: `${this.name.toUpperCase()}_HEALTHCHECK_ERROR`, | |
| error | |
| }); | |
| const hadPreviousFailure = this.isFailed === true; | |
| if (options?.ignoreCache || hadPreviousFailure) return false; | |
| this.isFailed = true; | |
| // tolerate the first failure for liveness; readiness still fails via ignoreCache | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/healthz/services/healthz/healthz.service.ts around lines 75–97,
the catch branch currently treats isFailed === null as an immediate failure
which violates the "tolerate first failure" behavior; change the logic so that
if options?.ignoreCache is true we fail immediately, if prevIsFailed === true (a
consecutive failure) we fail, otherwise treat this as the first failure: set
this.isFailed = true and return true to tolerate it. Concretely, compute const
prevIsFailed = this.isFailed first, then if (options?.ignoreCache) return false;
if (prevIsFailed === true) return false; set this.isFailed = true and return
true.
3 participants
Why
ref #1735
Summary by CodeRabbit
New Features
Refactor
Chores