feat: adds support for jwt token in offline mode#2036
Conversation
WalkthroughReplaces certificate-only provider auth with a unified ProviderCredentials model (mtls | jwt); adds hooks (useProviderCredentials, useProviderJwt, useAsyncCallback); introduces CreateCredentialsButton and removes CreateCertificateButton; updates provider-proxy payloads, WebSocket/auth wiring, many UI/components/tests, and bumps @akashnetwork/chain-sdk deps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Deployment UI
participant PC as useProviderCredentials
participant CJ as useProviderJwt
participant Cert as CertificateProvider
participant Settings as Settings
User->>UI: open deployment view
UI->>PC: resolve credentials.details
PC->>Settings: isBlockchainDown?
alt blockchain down
PC->>CJ: read/generate token
CJ-->>PC: { type: "jwt", value: token, usable }
else blockchain up
PC->>Cert: read/create local cert
Cert-->>PC: { type: "mtls", value: { cert, key }, usable }
end
UI-->>User: render based on credentials.details.usable
sequenceDiagram
autonumber
participant UI as Manifest sender
participant PC as useProviderCredentials
participant PP as provider-proxy.service
participant Provider as Provider API
UI->>PC: credentials.details
UI->>PP: sendManifest({ dseq, chainNetwork, credentials })
PP->>PP: providerCredentialsToApiCredentials(credentials)
PP->>Provider: POST /manifest { auth, dseq, ... }
Provider-->>PP: 200 / accepted
PP-->>UI: result
sequenceDiagram
autonumber
participant UI as Logs/Shell
participant PC as useProviderCredentials
participant WS as WebSocket
UI->>PC: credentials.details
UI->>WS: sendJsonMessage({ auth: providerCredentialsToApiCredentials(details), provider info })
WS-->>UI: stream messages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
stalniy
force-pushed
the
feat/jwt-offline
branch
from
0a72060 to
7cc163a
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -40,7 +40,7 @@ | |||
| "dependencies": { | |||
There was a problem hiding this comment.
🔄 Carefully review the package-lock.json diff
Resolve the comment if everything is ok
* node_modules/@akashnetwork/chain-sdk 1.0.0-alpha.3 -> 1.0.0-alpha.7
- node_modules/@akashnetwork/jwt link:packages/jwt
- node_modules/@multiformats/base-x 4.0.1
- node_modules/@types/bn.js 5.1.6
- node_modules/@types/elliptic 6.4.18
- node_modules/base64url 3.0.1
- node_modules/canonicalize 2.1.0
- node_modules/did-jwt/node_modules/@noble/ciphers 1.3.0
- node_modules/did-jwt 8.0.14
- node_modules/did-resolver 4.1.0
- node_modules/multibase 4.0.6
- packages/jwt/node_modules/@cosmjs/crypto 0.32.4
- packages/jwt/node_modules/@cosmjs/encoding 0.32.4
- packages/jwt/node_modules/@cosmjs/math 0.32.4
- packages/jwt/node_modules/@faker-js/faker 9.7.0
- packages/jwt/node_modules/ajv-formats 3.0.1
- packages/jwt/node_modules/ajv 8.17.1
- packages/jwt/node_modules/json-schema-traverse 1.0.0 
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/deploy-web/src/components/shared/PrerequisiteList.tsx (1)
92-93: Handle undefined minimum deposit values in UI text.The UI displays
{minDeposit.akt}and{minDeposit.usdc}directly, which will render "undefined" if one of these values isn't configured. This creates a confusing user experience.Apply this diff to conditionally display only configured minimums:
- <p className="text-sm text-muted-foreground"> - The balance of the wallet needs to be of at least {minDeposit.akt} AKT or {minDeposit.usdc} USDC. If you do not have {minDeposit.akt} AKT or{" "} - {minDeposit.usdc} USDC, you will need to specify an authorized depositor. - </p> + <p className="text-sm text-muted-foreground"> + The balance of the wallet needs to be of at least{" "} + {minDeposit.akt && `${minDeposit.akt} AKT`} + {minDeposit.akt && minDeposit.usdc && " or "} + {minDeposit.usdc && `${minDeposit.usdc} USDC`} + . If you do not have sufficient balance, you will need to specify an authorized depositor. + </p>apps/deploy-web/src/components/deployments/DeploymentLogs.tsx (3)
40-41: Eliminate any and guard JSON parsing (avoid crashes).
- Avoid
anyper guidelines; and the firstJSON.parse(event.data)must be inside try/catch (currently outside; can crash handler).As per coding guidelines
Apply these diffs:
- const logs = useRef<any[]>([]); + const logs = useRef<DisplayLog[]>([]);- const message = JSON.parse(event.data).message; - setIsLoadingLogs(true); - if (logs.current.length === 0) { - setStickToBottom(true); - } - // TODO Type - let parsedLog: any = null; - try { - parsedLog = JSON.parse(message); - if (selectedLogsMode === "logs") { - parsedLog.service = parsedLog?.name ? parsedLog?.name.split("-")[0] : ""; - parsedLog.message = `[${parsedLog.service}]: ${parsedLog.message}`; - } else { - parsedLog.service = parsedLog.object?.name ? parsedLog.object?.name.split("-")[0] : ""; - parsedLog.message = `[${parsedLog.service}]: [${parsedLog.type}] [${parsedLog.reason}] [${parsedLog.object?.kind}] ${parsedLog.note}`; - } - logs.current = logs.current.concat([parsedLog]); - updateLogText(); - } catch (error) { - console.log(error); - } + try { + const outer = JSON.parse(event.data) as { message?: string }; + if (!outer?.message) return; + setIsLoadingLogs(true); + if (logs.current.length === 0) setStickToBottom(true); + + const payload = JSON.parse(outer.message) as unknown; + let displayLog: DisplayLog; + if (selectedLogsMode === "logs") { + const name = (payload as { name?: string })?.name; + const svc = name ? name.split("-")[0] : ""; + const msg = (payload as { message?: string })?.message ?? ""; + displayLog = { service: svc, message: `[${svc}]: ${msg}` }; + } else { + const obj = (payload as { object?: { name?: string; kind?: string } })?.object; + const svc = obj?.name ? obj.name.split("-")[0] : ""; + const type = (payload as { type?: string })?.type ?? ""; + const reason = (payload as { reason?: string })?.reason ?? ""; + const kind = obj?.kind ?? ""; + const note = (payload as { note?: string })?.note ?? ""; + displayLog = { service: svc, message: `[${svc}]: [${type}] [${reason}] [${kind}] ${note}` }; + } + logs.current = logs.current.concat(displayLog); + updateLogText(); + } catch (error) { + console.log(error); + }Add this type near the top of the file:
type DisplayLog = { service: string; message: string };Also applies to: 165-176, 156-186
75-88: Clean up Monaco scroll listener; avoid private _oldScrollTop.
- Current effect doesn’t dispose the listener and uses a private property via
as any.- Track previous scrollTop in a ref and dispose the subscription.
As per coding guidelines
- useEffect(() => { - if (monacoEditorRef.current) { - const editor = monacoEditorRef.current; - editor.onDidScrollChange(event => { - // TODO Verify - if (event.scrollTop < (event as any)._oldScrollTop) { - setStickToBottom(false); - } - }); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [monacoEditorRef.current]); + const prevScrollTopRef = useRef(0); + useEffect(() => { + if (!monacoEditorRef.current) return; + const editor = monacoEditorRef.current; + const disposable = editor.onDidScrollChange(e => { + if (e.scrollTop < prevScrollTopRef.current) setStickToBottom(false); + prevScrollTopRef.current = e.scrollTop; + }); + return () => disposable.dispose(); + }, []);
122-154: Effect dependency causes duplicate WS requests; depend on usable flag.Depending on the
providerCredentialsobject causes re-runs every render; may send multiple websocket open messages beforeisConnectionEstablishedflips.- }, [ - providerCredentials, + }, [ + providerCredentials.details.usable, selectedLogsMode, selectedLease, selectedServices, services?.length, - updateLogText, canSetConnection, isConnectionEstablished, providerInfo ]);Also applies to: 145-146
🧹 Nitpick comments (8)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.ts (1)
66-70: Consider testing edge cases for credential values.The auth object assertion correctly uses optional chaining to handle null/undefined values. However, consider adding test cases for edge cases:
- Credentials with
nullvalue- Credentials with
undefinedvalue- No credentials provided (if supported)
These tests would ensure the service handles missing or invalid credentials gracefully.
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
34-54: Avoid capturingisTokenExpiredvalue; read getter instead (prevents stale values)Destructuring the getter into a boolean snapshots it. Read
jwt.isTokenExpiredwhen computing details.- ? ({ - type: "jwt", - value: accessToken, - isExpired: isTokenExpired, - usable: !!accessToken && !isTokenExpired - } as const) + ? ({ + type: "jwt", + value: accessToken, + isExpired: jwt.isTokenExpired, + usable: !!accessToken && !jwt.isTokenExpired + } as const) : ({ type: "mtls", @@ - isExpired: isLocalCertExpired, - usable: !!localCert?.certPem && !!localCert?.keyPem && !isLocalCertExpired && isLocalCertMatching + isExpired: isLocalCertExpired, + usable: !!localCert?.certPem && !!localCert?.keyPem && !isLocalCertExpired && isLocalCertMatching } as const); - }, [settings.isBlockchainDown, localCert, accessToken, isLocalCertExpired, isLocalCertMatching, isTokenExpired]); + }, [settings.isBlockchainDown, localCert, accessToken, isLocalCertExpired, isLocalCertMatching, jwt]);Note: UI still won’t auto-refresh exactly at expiry without a timer. If needed, schedule a re-render at
expor exposeexpiresAtfromuseProviderJwtto drive a timeout.
Based on learningsapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (1)
39-41: Wire upafterCreatecallback on success
afterCreateis declared but never used.- const [createCredentials, createCredentialsState] = useAsyncCallback(credentials.generate, []); + const [createCredentials, createCredentialsState] = useAsyncCallback(credentials.generate, []); @@ - onClick={createCredentials} + onClick={async () => { + await createCredentials(); + afterCreate?.(); + }} >Also applies to: 60-63
apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (1)
76-85: Optional: auto-refresh token expiry state
isTokenExpiredwon’t trigger a re-render exactly at expiry. Consider exposingexpiresAtand scheduling asetTimeoutto force update, or include a small interval to re-check near expiry.apps/deploy-web/src/queries/useLeaseQuery.spec.tsx (1)
265-273: Align test name with new credentials modelThe test still says “local cert”; it now tests missing/unusable credentials. Rename for clarity.
-it("returns null when local cert is not provided", async () => { +it("returns null when credentials are missing or unusable", async () => {Also consider adding a JWT-credentials case to ensure the status fetch works in offline/JWT mode.
apps/deploy-web/src/components/deployments/ManifestUpdate.tsx (1)
244-247: Optional: passafterCreateto refresh parent state/UINot required, but you can pass
afterCreatetoCreateCredentialsButtonto run any post-generation actions (e.g., telemetry or immediate refresh).apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (2)
17-27: Auth mapping in payload looks correct; minor nit on destructure use.Passing
auth: providerCredentialsToApiCredentials(credentials)aligns with the new union. Consider using the destructuredchainNetworkvar for consistency and skip the ternary (the helper already returnsundefined).- const { chainNetwork, providerIdentity, timeout, credentials, ...params } = options; + const { chainNetwork, providerIdentity, timeout, credentials, ...params } = options; return this.axios.post( "/", { ...params, method: options.method || "GET", url: providerIdentity.hostUri + url, providerAddress: providerIdentity.owner, - network: options.chainNetwork, - auth: credentials ? providerCredentialsToApiCredentials(credentials) : undefined + network: chainNetwork, + auth: providerCredentialsToApiCredentials(credentials) }, { timeout } );Confirm provider-proxy backend expects the
authenvelope with{ type, token|certPem,keyPem }(no longercertPem/keyPemat top level).
81-86: Types and converter are clear; optional API polish.The discriminated unions are good. Optional:
- Mark the unions as
readonlyto prevent accidental mutation.- Use
switch (credentials.type)for exhaustiveness.-export type ProviderCredentials = +export type ProviderCredentials = | { - type: "mtls"; - value: + readonly type: "mtls"; + readonly value: | { - cert: string; - key: string; + readonly cert: string; + readonly key: string; } | null | undefined; } | { - type: "jwt"; - value: string | undefined | null; + readonly type: "jwt"; + readonly value: string | undefined | null; };-export function providerCredentialsToApiCredentials(credentials: ProviderCredentials | null | undefined): ProviderApiCredentials | undefined { - if (!credentials?.value) return; - if (credentials.type === "mtls") - return { - type: credentials.type, - certPem: credentials.value.cert, - keyPem: credentials.value.key - }; - return { - type: credentials.type, - token: credentials.value - }; -} +export function providerCredentialsToApiCredentials( + credentials: ProviderCredentials | null | undefined +): ProviderApiCredentials | undefined { + if (!credentials?.value) return; + switch (credentials.type) { + case "mtls": + return { type: "mtls", certPem: credentials.value.cert, keyPem: credentials.value.key }; + case "jwt": + return { type: "jwt", token: credentials.value }; + } +}Also applies to: 93-137, 125-137
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
apps/api/package.json(1 hunks)apps/api/src/provider/http-schemas/jwt-token.schema.ts(1 hunks)apps/deploy-web/package.json(1 hunks)apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx(0 hunks)apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx(0 hunks)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx(1 hunks)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx(1 hunks)apps/deploy-web/src/components/deployments/DeploymentDetail.tsx(4 hunks)apps/deploy-web/src/components/deployments/DeploymentLeaseShell.tsx(8 hunks)apps/deploy-web/src/components/deployments/DeploymentListRow.tsx(2 hunks)apps/deploy-web/src/components/deployments/DeploymentLogs.tsx(8 hunks)apps/deploy-web/src/components/deployments/LeaseRow.tsx(5 hunks)apps/deploy-web/src/components/deployments/ManifestUpdate.tsx(5 hunks)apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx(3 hunks)apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx(2 hunks)apps/deploy-web/src/components/shared/PrerequisiteList.tsx(1 hunks)apps/deploy-web/src/context/BackgroundTaskProvider/BackgroundTaskProvider.tsx(3 hunks)apps/deploy-web/src/context/CustomChainProvider/CustomChainProvider.tsx(2 hunks)apps/deploy-web/src/context/CustomChainProvider/index.ts(1 hunks)apps/deploy-web/src/context/WalletProvider/index.ts(1 hunks)apps/deploy-web/src/hooks/useAsyncCallback/useAsyncCallback.ts(1 hunks)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts(1 hunks)apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.spec.tsx(1 hunks)apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts(1 hunks)apps/deploy-web/src/hooks/useProviderWebsocket.ts(3 hunks)apps/deploy-web/src/queries/useLeaseQuery.spec.tsx(10 hunks)apps/deploy-web/src/queries/useLeaseQuery.ts(3 hunks)apps/deploy-web/src/services/app-di-container/browser-di-container.ts(2 hunks)apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.ts(3 hunks)apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts(4 hunks)apps/deploy-web/src/utils/deploymentUtils.ts(0 hunks)apps/deploy-web/src/utils/walletUtils.ts(1 hunks)apps/provider-proxy/package.json(1 hunks)
💤 Files with no reviewable changes (3)
- apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx
- apps/deploy-web/src/utils/deploymentUtils.ts
- apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/deploy-web/src/context/WalletProvider/index.tsapps/deploy-web/src/queries/useLeaseQuery.tsapps/deploy-web/src/components/deployments/DeploymentDetail.tsxapps/deploy-web/src/hooks/useAsyncCallback/useAsyncCallback.tsapps/deploy-web/src/components/shared/PrerequisiteList.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/context/CustomChainProvider/index.tsapps/api/src/provider/http-schemas/jwt-token.schema.tsapps/deploy-web/src/context/BackgroundTaskProvider/BackgroundTaskProvider.tsxapps/deploy-web/src/services/provider-proxy/provider-proxy.service.tsapps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.tsapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsxapps/deploy-web/src/hooks/useProviderWebsocket.tsapps/deploy-web/src/components/deployments/DeploymentLogs.tsxapps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsxapps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.spec.tsxapps/deploy-web/src/utils/walletUtils.tsapps/deploy-web/src/components/deployments/LeaseRow.tsxapps/deploy-web/src/context/CustomChainProvider/CustomChainProvider.tsxapps/deploy-web/src/services/app-di-container/browser-di-container.tsapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsxapps/deploy-web/src/components/deployments/DeploymentLeaseShell.tsxapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.tsapps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.tsapps/deploy-web/src/queries/useLeaseQuery.spec.tsxapps/deploy-web/src/components/deployments/ManifestUpdate.tsxapps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx
**/*.{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/deploy-web/src/context/WalletProvider/index.tsapps/deploy-web/src/queries/useLeaseQuery.tsapps/deploy-web/src/components/deployments/DeploymentDetail.tsxapps/deploy-web/src/hooks/useAsyncCallback/useAsyncCallback.tsapps/deploy-web/src/components/shared/PrerequisiteList.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/context/CustomChainProvider/index.tsapps/api/src/provider/http-schemas/jwt-token.schema.tsapps/deploy-web/src/context/BackgroundTaskProvider/BackgroundTaskProvider.tsxapps/deploy-web/src/services/provider-proxy/provider-proxy.service.tsapps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.tsapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsxapps/deploy-web/src/hooks/useProviderWebsocket.tsapps/deploy-web/src/components/deployments/DeploymentLogs.tsxapps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsxapps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.spec.tsxapps/deploy-web/src/utils/walletUtils.tsapps/deploy-web/src/components/deployments/LeaseRow.tsxapps/deploy-web/src/context/CustomChainProvider/CustomChainProvider.tsxapps/deploy-web/src/services/app-di-container/browser-di-container.tsapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsxapps/deploy-web/src/components/deployments/DeploymentLeaseShell.tsxapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.tsapps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.tsapps/deploy-web/src/queries/useLeaseQuery.spec.tsxapps/deploy-web/src/components/deployments/ManifestUpdate.tsxapps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx
**/*.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/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.tsapps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsxapps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.spec.tsxapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsxapps/deploy-web/src/queries/useLeaseQuery.spec.tsx
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsxapps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.spec.tsxapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsxapps/deploy-web/src/queries/useLeaseQuery.spec.tsx
🧬 Code graph analysis (19)
apps/deploy-web/src/queries/useLeaseQuery.ts (1)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
fetchProviderUrl(16-30)
apps/deploy-web/src/components/deployments/DeploymentDetail.tsx (2)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
useProviderCredentials(26-60)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (1)
CreateCredentialsButton(39-66)
apps/deploy-web/src/components/deployments/DeploymentListRow.tsx (2)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
useProviderCredentials(26-60)apps/deploy-web/src/queries/useLeaseQuery.ts (1)
useLeaseStatus(71-96)
apps/deploy-web/src/context/BackgroundTaskProvider/BackgroundTaskProvider.tsx (1)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
useProviderCredentials(26-60)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.ts (1)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
ProviderCredentials(99-113)
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (4)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
DEPENDENCIES(8-12)packages/ui/components/button.tsx (1)
ButtonProps(36-38)apps/deploy-web/src/hooks/useAsyncCallback/useAsyncCallback.ts (1)
useAsyncCallback(3-24)apps/provider-console/src/utils/styleUtils.ts (1)
cn(4-6)
apps/deploy-web/src/hooks/useProviderWebsocket.ts (2)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
useProviderCredentials(26-60)apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
providerCredentialsToApiCredentials(125-137)
apps/deploy-web/src/components/deployments/DeploymentLogs.tsx (3)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(29-31)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
useProviderCredentials(26-60)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (1)
CreateCredentialsButton(39-66)
apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx (1)
apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (1)
localCert(365-367)
apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.spec.tsx (3)
apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (2)
useProviderJwt(16-86)DEPENDENCIES(11-14)apps/deploy-web/src/context/CustomChainProvider/CustomChainProvider.tsx (1)
useSelectedChain(66-69)packages/network-store/src/network.store.ts (1)
NetworkStore(20-124)
apps/deploy-web/src/components/deployments/LeaseRow.tsx (3)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
useProviderCredentials(26-60)apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
sendManifest(32-76)apps/deploy-web/src/utils/deploymentData/index.ts (1)
deploymentData(14-14)
apps/deploy-web/src/services/app-di-container/browser-di-container.ts (2)
apps/deploy-web/src/services/auth/auth/auth.service.ts (1)
AuthService(7-36)apps/deploy-web/src/services/app-di-container/server-di-container.service.ts (1)
services(23-34)
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx (2)
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (1)
CreateCredentialsButton(39-66)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
UseProviderCredentialsResult(14-20)
apps/deploy-web/src/components/deployments/DeploymentLeaseShell.tsx (2)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
useProviderCredentials(26-60)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (1)
CreateCredentialsButton(39-66)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (3)
apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (2)
DEPENDENCIES(11-14)useProviderJwt(16-86)apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
ProviderCredentials(99-113)apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (2)
localCert(365-367)isLocalCertExpired(369-371)
apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (2)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
DEPENDENCIES(8-12)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(29-31)
apps/deploy-web/src/queries/useLeaseQuery.spec.tsx (4)
apps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.ts (1)
FallbackableHttpClient(6-6)apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
ProviderProxyService(8-77)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
UseProviderCredentialsResult(14-20)apps/deploy-web/src/queries/useLeaseQuery.ts (1)
USE_LEASE_STATUS_DEPENDENCIES(97-100)
apps/deploy-web/src/components/deployments/ManifestUpdate.tsx (2)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
useProviderCredentials(26-60)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (1)
CreateCredentialsButton(39-66)
apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx (1)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
SendManifestToProviderOptions(93-97)
⏰ 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). (7)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (31)
apps/provider-proxy/package.json (1)
25-25: Dependency bump looks good.Thanks for keeping
@akashnetwork/chain-sdkaligned with the new credentials workflow; no issues spotted here.apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.ts (2)
5-5: LGTM!The import of
ProviderCredentialstype is correctly added and used to type the credentials object in the test.
54-55: Add test coverage for JWT credentials.The mtls credentials test is correctly implemented. However, this PR introduces JWT support, and there's no test coverage for the JWT credential type.
Consider adding a test case for JWT credentials to ensure the new feature works correctly. For example:
it("sends manifest with JWT credentials", async () => { jest.useFakeTimers(); const { service, httpClient } = setup(); const provider = buildProvider(); const response = {}; const dseq = "1"; httpClient.post.mockResolvedValue(response); const manifest = [/* ... same manifest structure ... */]; const credentials: ProviderCredentials = { type: "jwt", value: "jwt-token-here" }; const promise = service.sendManifest(provider, manifest, { dseq, chainNetwork: "mainnet", credentials }); const [result] = await Promise.all([promise, jest.runAllTimersAsync()]); expect(httpClient.post).toHaveBeenCalledWith( "/", { method: "PUT", url: `${provider.hostUri}/deployment/${dseq}/manifest`, providerAddress: provider.owner, network: "mainnet", auth: { type: "jwt", token: credentials.value }, body: expect.any(String) }, { timeout: expect.any(Number) } ); expect(result).toBe(response); jest.useRealTimers(); });Note: Verify the exact structure of the
authobject for JWT credentials in the implementation.apps/deploy-web/src/context/BackgroundTaskProvider/BackgroundTaskProvider.tsx (1)
34-34: LGTM! Unified credentials approach improves maintainability.The use of
useProviderCredentialsreplaces certificate-specific logic with a unified credential model that supports both mTLS and JWT authentication flows. This aligns well with the PR's objective to support JWT tokens in offline mode.apps/api/src/provider/http-schemas/jwt-token.schema.ts (3)
38-41: LGTM!The new
ScopedAccessSchemais well-structured and follows the established pattern. The requiredscopefield appropriately distinguishes scoped access from full access.
48-50: LGTM!The updated discriminated union correctly includes all three access types. The
satisfiesconstraint ensures compile-time type safety with the chain-sdk'sJwtTokenPayload["leases"]type, catching any mismatches early.
34-36: Breaking change is localized — no external callers found.rg shows
FullAccessSchemaonly in apps/api/src/provider/http-schemas/jwt-token.schema.ts (its definition and the local LeasesSchema union); no external usages or callers accessing ascopefield were found (the.scopematches are in unrelated minified test fixtures). The removal of the optionalscopefield appears safe and can be resolved as-is.apps/deploy-web/src/components/shared/PrerequisiteList.tsx (1)
31-34: Remove unnecessary undefined guards
minDeposit.aktandminDeposit.usdcare both required numbers (per MinDeposit) andwalletBalanceis null-checked before access, so neither utility call can receiveundefined. No additional guards needed.Likely an incorrect or invalid review comment.
apps/deploy-web/src/context/CustomChainProvider/index.ts (1)
1-1: LGTM!The addition of
type ChainContextto the public exports improves type safety for consumers of this module.apps/deploy-web/src/context/CustomChainProvider/CustomChainProvider.tsx (1)
7-7: LGTM!The type-level changes improve the public API surface and type safety without altering runtime behavior. The explicit return type annotation on
useSelectedChainfollows TypeScript best practices.Also applies to: 12-12, 65-66
apps/deploy-web/src/context/WalletProvider/index.ts (1)
1-1: LGTM!The addition of
type ContextTypeto exports follows the pattern established in other context providers and improves type safety.apps/deploy-web/src/services/app-di-container/browser-di-container.ts (1)
7-7: LGTM!The addition of
storedWalletsServicefollows the established DI container pattern and properly exposes wallet utilities as a service. The service definitions are correct and consistent with the existing container structure.Also applies to: 55-56
apps/deploy-web/src/utils/walletUtils.ts (1)
11-11: LGTM!The addition of the optional
tokenfield toBaseLocalWalletis backward-compatible and aligns with the PR's goal of supporting JWT tokens in offline mode.apps/deploy-web/src/components/deployments/DeploymentListRow.tsx (1)
27-27: LGTM!The refactoring from certificate-based to credentials-based gating is implemented correctly. The use of
providerCredentials.details.usableproperly gates lease status queries based on credential availability (either mTLS or JWT), which aligns with the unified provider credentials model introduced in this PR.Also applies to: 87-88
apps/deploy-web/src/components/deployments/DeploymentDetail.tsx (1)
20-20: LGTM!The refactoring consistently applies the unified credentials model:
- Import changes align with the new credentials-based approach
- The
providerCredentials.details.usablecheck properly gates theCreateCredentialsButtondisplay- The component now handles both mTLS and JWT credentials based on blockchain availability
This change is consistent with similar refactoring in DeploymentListRow.tsx and properly implements the credentials flow.
Also applies to: 32-32, 95-95, 286-286
apps/deploy-web/src/hooks/useAsyncCallback/useAsyncCallback.ts (2)
20-20: RemoveisPendingfrom the dependency array.Including
isPendingin theuseCallbackdependency array causes the callback to be recreated every time the pending state changes (i.e., on every invocation). This defeats the purpose of memoization and can cause unnecessary re-renders in components using this hook.Apply this diff:
- [fn, isPending, ...deps] + [fn, ...deps]The
isPendingcheck inside the callback will still work correctly by capturing the current value via closure, but the callback reference will remain stable across pending state changes.Likely an incorrect or invalid review comment.
17-18: Review error propagation in useAsyncCallback
Catching errors with.catch(setError)swallows rejections, so callers can’t handle failures via try/catch or.catch(). If consumers need to catch errors, rethrow after setting state (e.g..catch(err => { setError(err); throw err; })); otherwise confirm all callers rely solely on the hook’s
errorstate.apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx (1)
210-217: LGTM! Test correctly reflects the new credentials payload structure.The test now expects the manifest send payload to use
credentials: { type: "mtls", value: { cert, key } }instead of the previous format. This aligns with the new provider credentials flow introduced in the PR.apps/deploy-web/src/hooks/useProviderWebsocket.ts (1)
7-7: LGTM! WebSocket authentication correctly updated to use provider credentials.The hook now uses
useProviderCredentialsand converts credentials to API format usingproviderCredentialsToApiCredentials. This properly replaces the previous certificate-based authentication flow.Also applies to: 10-10, 20-20, 60-60
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx (1)
1-177: LGTM! Comprehensive test coverage for both credential types.The test suite thoroughly covers:
- Rendering create/regenerate buttons for missing/expired credentials
- Hiding UI when credentials are usable
- Spinner display during generation
- Both mtls and jwt credential flows
The tests follow coding guidelines: no
jest.mock(), usessetupfunction at bottom, and properly mocks dependencies withjest-mock-extended.apps/deploy-web/src/components/deployments/LeaseRow.tsx (2)
21-21: LGTM! Provider credentials hook properly integrated.The component now uses
useProviderCredentialsto manage credentials instead of directly using the certificate context.Also applies to: 57-57
70-70: LGTM! Credentials usage correctly propagated throughout the component.The changes consistently:
- Gate lease status loading on
providerCredentials.details.usable- Include
providerCredentials.detailsin dependency arrays- Pass
credentials: providerCredentials.detailsto sendManifestThis properly replaces the previous certificate-based flow.
Also applies to: 94-98, 117-117, 125-125
apps/deploy-web/src/components/deployments/DeploymentLeaseShell.tsx (2)
9-9: LGTM! Imports updated for provider credentials flow.The component now imports and uses
useProviderCredentialsandCreateCredentialsButton, replacing the previous certificate-specific flow.Also applies to: 18-18, 39-39
84-84: LGTM! Shell functionality properly gated on credentials availability.The changes correctly:
- Gate WebSocket connection setup on
providerCredentials.details.usable- Include
providerCredentials.detailsin effect dependencies- Show
CreateCredentialsButtonwhen credentials are not usable- Display shell UI and download file button only when credentials are usable
Also applies to: 98-98, 240-290
apps/deploy-web/src/queries/useLeaseQuery.ts (1)
6-6: LGTM! Lease status query correctly migrated to provider credentials.The hook now:
- Uses
useProviderCredentialsinstead ofuseCertificate- Gates query execution on
providerCredentials.details.usable- Passes
credentials: providerCredentials.detailsto the provider API- Updates
USE_LEASE_STATUS_DEPENDENCIESaccordinglyThis maintains the same query behavior while properly supporting both mtls and jwt credential types.
Also applies to: 79-79, 85-85, 89-89, 99-99
apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx (2)
46-46: LGTM! Import path updated for type relocation.The
SendManifestToProviderOptionstype is now imported from the provider-proxy service instead of deploymentUtils, reflecting the centralized credentials handling.
209-219: LGTM! Manifest options correctly updated to use credentials structure.The manifest send payload now uses the new
credentialsobject withtype: "mtls"andvalue: { cert, key }, replacing the previous format. ThedseqandchainNetworkfields remain unchanged.apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.spec.tsx (1)
1-230: LGTM! Comprehensive test coverage for JWT token management.The test suite thoroughly validates:
- Initial state when no token exists
- Token retrieval from storage on wallet address change
- Token generation for managed wallets via API with correct payload (TTL, leases, scopes)
- Token generation for non-managed wallets via direct signing with signature verification
- Guard against generation when wallet is disconnected
- Token expiry detection for both expired and valid tokens
The tests follow coding guidelines:
- No
jest.mock()for dependencies (usesjest-mock-extended)- Uses
setupfunction at the bottom- Properly structured with clear test descriptions
apps/deploy-web/src/components/deployments/ManifestUpdate.tsx (1)
124-131: Auth payload uses new credentials shape — looks correct
sendManifestnow passescredentials: providerCredentials.detailsto provider-proxy. Matches the new API.apps/deploy-web/src/components/deployments/DeploymentLogs.tsx (1)
237-238: Credentials gating LGTM.Switch to
providerCredentials.details.usableandCreateCredentialsButtonrenders is clear and consistent with the new flow.Please confirm
useProviderWebsocketpicks up the active credentials implicitly (no explicit credential param here).Also applies to: 327-328
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
49-61: sendManifest forwards credentials properly.Forwarding
options.credentialstofetchProviderUrlkeeps the API clean. No issues found.Also applies to: 55-56
...ploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx
Show resolved
Hide resolved
apps/deploy-web/src/context/BackgroundTaskProvider/BackgroundTaskProvider.tsx
Show resolved
Hide resolved
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/provider/services/provider/provider.service.ts (1)
56-67: HandleErrfrom JWT generation before unwrapping.
generateJwtTokennow returnsResult<string, string[]>. When validation fails it returnsErr(errors). Callingunwrap()on that branch throwsError("Tried to unwrap Err value"), collapsing the original validation details and bubbling up as a 500. Please gate the unwrap and turn those validation errors into an explicit 4xx so callers receive actionable feedback. Suggestion:const result = await this.jwtTokenService.generateJwtToken({ walletId: auth.walletId, leases: this.jwtTokenService.getGranularLeases({ provider: auth.provider, scope: ["send-manifest"] }) }); + if (result.isErr()) { + assert(false, 400, result.error.join(", ")); + } + return { type: "jwt", - token: result.unwrap() + token: result.unwrap() };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/api/src/provider/controllers/jwt-token/jwt-token.controller.spec.ts(3 hunks)apps/api/src/provider/controllers/jwt-token/jwt-token.controller.ts(2 hunks)apps/api/src/provider/http-schemas/jwt-token.schema.ts(1 hunks)apps/api/src/provider/routes/jwt-token/jwt-token.router.ts(1 hunks)apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts(2 hunks)apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.ts(2 hunks)apps/api/src/provider/services/provider/provider.service.spec.ts(7 hunks)apps/api/src/provider/services/provider/provider.service.ts(2 hunks)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx(1 hunks)apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx(1 hunks)apps/deploy-web/src/context/SettingsProvider/index.ts(1 hunks)apps/deploy-web/src/hooks/useAsyncCallback/useAsyncCallback.ts(1 hunks)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts(1 hunks)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts(1 hunks)apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/src/hooks/useAsyncCallback/useAsyncCallback.ts
- apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts
- apps/api/src/provider/http-schemas/jwt-token.schema.ts
- apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsxapps/deploy-web/src/context/SettingsProvider/index.tsapps/api/src/provider/services/provider/provider.service.spec.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.tsapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.tsapps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.tsapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsxapps/api/src/provider/routes/jwt-token/jwt-token.router.tsapps/api/src/provider/controllers/jwt-token/jwt-token.controller.spec.tsapps/api/src/provider/controllers/jwt-token/jwt-token.controller.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/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsxapps/deploy-web/src/context/SettingsProvider/index.tsapps/api/src/provider/services/provider/provider.service.spec.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.tsapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.tsapps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.tsapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsxapps/api/src/provider/routes/jwt-token/jwt-token.router.tsapps/api/src/provider/controllers/jwt-token/jwt-token.controller.spec.tsapps/api/src/provider/controllers/jwt-token/jwt-token.controller.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/provider/services/provider/provider.service.spec.tsapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.tsapps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.tsapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsxapps/api/src/provider/controllers/jwt-token/jwt-token.controller.spec.ts
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx
🧬 Code graph analysis (4)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (4)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (2)
useProviderCredentials(26-60)DEPENDENCIES(8-12)apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (2)
localCert(365-367)isLocalCertExpired(369-371)apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx (1)
SettingsContextType(41-41)apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (2)
DEPENDENCIES(11-15)UseProviderJwtResult(89-93)
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx (2)
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (1)
CreateCredentialsButton(39-66)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
UseProviderCredentialsResult(14-20)
apps/api/src/provider/controllers/jwt-token/jwt-token.controller.spec.ts (1)
apps/api/test/seeders/user.seeder.ts (1)
UserSeeder(5-45)
apps/api/src/provider/controllers/jwt-token/jwt-token.controller.ts (1)
apps/api/src/provider/http-schemas/jwt-token.schema.ts (2)
CreateJwtTokenRequest(8-8)CreateJwtTokenResponse(13-13)
⏰ 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). (9)
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (11)
apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx (1)
333-335: LGTM! Explicit return type improves type safety.Adding the explicit return type annotation to
useSettingsfollows TypeScript best practices and improves code maintainability.apps/deploy-web/src/context/SettingsProvider/index.ts (1)
1-1: LGTM! Type export enhances the public API.Exporting
SettingsContextTypemakes it available to consumers and complements the explicit return type annotation in the companion file. The use of thetypekeyword for type-only exports is correct.apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (1)
1-205: Excellent test coverage and adherence to guidelines!The test suite is well-structured and comprehensive:
- All 10 test cases cover the critical paths for both JWT and mTLS credential types
- Proper use of
jest-mock-extendedfor dependency mocking- Setup function correctly positioned at the bottom with inline type definition
- Focused assertions that verify behavior without over-asserting
- Clean separation between happy paths and edge cases
The tests effectively validate:
- Credential type selection based on blockchain status
- Usability flags under various conditions (expiration, missing values, cert matching)
- The
generate()function delegation to the appropriate underlying methodapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx (3)
1-5: LGTM!The imports are well-structured and properly typed.
152-177: LGTM!The
setupfunction structure fully complies with coding guidelines:
- Positioned at the bottom of the root describe block
- Accepts a single parameter with inline type definition
- No return type specified
- Returns the render result
- No shared state
The use of
jest.fn()for creating simple mock functions is appropriate, as it's passed through the dependencies prop for dependency injection.
7-177: Excellent test coverage and organization.The test suite provides comprehensive coverage for both mtls and jwt credential types:
- Missing credentials scenarios
- Expired credentials scenarios
- Usable credentials scenarios (no rendering)
- Loading states during credential generation
The symmetric structure between mtls and jwt test suites makes the tests easy to understand and maintain.
apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts (2)
16-34: LGTM! Result unwrapping correctly implemented.The test properly unwraps the Result type using
.unwrap()to access the token value, and all mock expectations align with the new service interface that includes payload validation before token generation.
86-89: LGTM! Mock structure matches the updated service interface.The mock correctly includes both
validatePayload(returning{ errors: undefined }to simulate successful validation) andgenerateToken(resolving to the token value), which aligns with the service's updated control flow.apps/api/src/provider/controllers/jwt-token/jwt-token.controller.ts (3)
19-23: LGTM! Result-based error handling properly implemented.The method signature and early validation correctly use the Result pattern with appropriate HTTP error types. The flow ensures authentication and wallet existence before proceeding.
25-31: LGTM! Result transformation correctly implemented.The service call and Result transformation properly handle both success (mapping to response object) and error cases (joining error messages). The error formatting with
".\n"appropriately creates multi-line error messages.Based on learnings (ts-results v3.3.0 supports proper Result.map/mapErr patterns).
27-27: Verifyleasestype alignment
Ensure thatCreateJwtTokenRequestSchema.leases(currentlyRecord<string, any>) structurally matchesJwtTokenPayload["leases"]from@akashnetwork/chain-sdk. Either update the schema to use the exact payload type, add a runtime conversion/validation layer, or document why this cast is safe.
...ploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx
Show resolved
Hide resolved
...ploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx
Show resolved
Hide resolved
stalniy
force-pushed
the
feat/jwt-offline
branch
from
6543a05 to
7a945a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (1)
45-75: FixgenerateTokendependencies: addstoredWalletsService; remove unusedcustodialWallet
- Missing
storedWalletsServicerisks updating a stale service instance.custodialWalletisn’t used in the callback body and can be dropped.- }, [isWalletConnected, isManaged, custodialWallet, jwtTokenManager, address, consoleApiHttpClient]); + }, [isWalletConnected, isManaged, jwtTokenManager, address, consoleApiHttpClient, storedWalletsService]);
🧹 Nitpick comments (3)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (2)
55-134: Consider asserting full credential state in unusable tests.Tests for unusable credentials (expired/missing JWT or mTLS) only verify the
usableflag. Adding assertions fortype,value, andisExpiredwould provide stronger coverage and catch regressions in error-state properties.Example for the expired JWT test:
it("returns unusable JWT credentials when token is expired", () => { const accessToken = "jwt-token-123"; const isTokenExpired = true; const { result } = setup({ settings: { isBlockchainDown: true }, providerJwt: { accessToken, isTokenExpired } }); + expect(result.current.details).toEqual({ + type: "jwt", + value: accessToken, + isExpired: true, + usable: false + }); - expect(result.current.details.usable).toBe(false); });
93-100: Consider combining duplicate test scenarios.Lines 93–100 and 136–144 both test the missing-cert scenario (
localCert: null) but assert different properties. Merging them into one test that verifiesusable,type, andvaluewould reduce duplication.Example merged test:
- it("returns unusable mTLS credentials when cert is missing", () => { - const { result } = setup({ - settings: { isBlockchainDown: false }, - certificate: { localCert: null, isLocalCertExpired: false, isLocalCertMatching: true } - }); - - expect(result.current.details.usable).toBe(false); - }); - - it("returns null mTLS value when cert is missing", () => { + it("returns unusable mTLS credentials with null value when cert is missing", () => { const { result } = setup({ settings: { isBlockchainDown: false }, certificate: { localCert: null, isLocalCertExpired: false, isLocalCertMatching: true } }); expect(result.current.details.type).toBe("mtls"); expect(result.current.details.value).toBeNull(); + expect(result.current.details.usable).toBe(false); });Also applies to: 136-144
apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (1)
40-43: Guard token decoding to avoid render-time crashes on bad/legacy tokens
decodeTokenmay throw; returnnullinstead of breaking render.- const parsedToken = useMemo(() => { - if (!accessToken) return null; - return jwtTokenManager.decodeToken(accessToken); - }, [accessToken, jwtTokenManager]); + const parsedToken = useMemo(() => { + if (!accessToken) return null; + try { + return jwtTokenManager.decodeToken(accessToken); + } catch { + return null; + } + }, [accessToken, jwtTokenManager]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
apps/api/src/provider/controllers/jwt-token/jwt-token.controller.spec.ts(3 hunks)apps/api/src/provider/controllers/jwt-token/jwt-token.controller.ts(2 hunks)apps/api/src/provider/http-schemas/jwt-token.schema.ts(1 hunks)apps/api/src/provider/routes/jwt-token/jwt-token.router.ts(1 hunks)apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts(2 hunks)apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.ts(2 hunks)apps/api/src/provider/services/provider/provider.service.spec.ts(7 hunks)apps/api/src/provider/services/provider/provider.service.ts(2 hunks)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx(1 hunks)apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx(1 hunks)apps/deploy-web/src/context/SettingsProvider/index.ts(1 hunks)apps/deploy-web/src/hooks/useAsyncCallback/useAsyncCallback.ts(1 hunks)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts(1 hunks)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts(1 hunks)apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/deploy-web/src/hooks/useAsyncCallback/useAsyncCallback.ts
- apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts
- apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts
- apps/deploy-web/src/context/SettingsProvider/index.ts
- apps/api/src/provider/controllers/jwt-token/jwt-token.controller.spec.ts
- apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.spec.tsx
- apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx
- apps/api/src/provider/services/provider/provider.service.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/provider/services/provider-jwt-token/provider-jwt-token.service.tsapps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.tsapps/api/src/provider/http-schemas/jwt-token.schema.tsapps/api/src/provider/services/provider/provider.service.spec.tsapps/api/src/provider/routes/jwt-token/jwt-token.router.tsapps/api/src/provider/controllers/jwt-token/jwt-token.controller.tsapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.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/provider/services/provider-jwt-token/provider-jwt-token.service.tsapps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.tsapps/api/src/provider/http-schemas/jwt-token.schema.tsapps/api/src/provider/services/provider/provider.service.spec.tsapps/api/src/provider/routes/jwt-token/jwt-token.router.tsapps/api/src/provider/controllers/jwt-token/jwt-token.controller.tsapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.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/provider/services/provider/provider.service.spec.tsapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts
🧬 Code graph analysis (3)
apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (1)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
DEPENDENCIES(8-12)
apps/api/src/provider/controllers/jwt-token/jwt-token.controller.ts (1)
apps/api/src/provider/http-schemas/jwt-token.schema.ts (2)
CreateJwtTokenRequest(8-8)CreateJwtTokenResponse(13-13)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (4)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (2)
useProviderCredentials(26-60)DEPENDENCIES(8-12)apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (2)
localCert(365-367)isLocalCertExpired(369-371)apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx (1)
SettingsContextType(41-41)apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (2)
DEPENDENCIES(11-15)UseProviderJwtResult(89-93)
⏰ 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). (10)
- GitHub Check: validate / validate-app
- 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: test-build
- GitHub Check: test-build
🔇 Additional comments (7)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (4)
1-8: LGTM!Imports are clean and follow guidelines—correctly using
jest-mock-extendedfor mocking dependencies.
11-53: LGTM!Both happy-path tests comprehensively verify all credential properties (type, value, expiry, usability, generate function) for JWT and mTLS modes.
146-170: LGTM!Both tests correctly verify that
generate()delegates to the appropriate credential-generation method (generateTokenorcreateCertificate) based on the blockchain state.
172-204: LGTM!The
setupfunction correctly follows all guidelines:
- Positioned at the bottom of the describe block.
- Accepts a single parameter with an inline type definition.
- No explicit return type.
- No shared state.
- Uses
jest-mock-extendedto create mocks with sensible defaults.apps/api/src/provider/services/provider/provider.service.spec.ts (1)
7-7: LGTM! Test mocks properly align with Result-based flow.The test updates correctly wrap mock return values with
Ok()to match the service's new Result-based API. The mock initialization in thesetupfunction also properly returnsOk("mock-jwt-token").Also applies to: 38-38, 78-78, 126-126, 154-154, 192-192, 244-246
apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.ts (1)
36-53: LGTM! Proper Result-based error handling pattern.The refactor to return
Result<string, string[]>provides explicit error handling without exceptions. The validation step before token generation is a good defensive practice, and returningErr(validationResult.errors)when validation fails allows callers to handle errors appropriately.apps/api/src/provider/controllers/jwt-token/jwt-token.controller.ts (1)
19-32: LGTM! Result-based error handling improves control flow.The refactor to return
Result<CreateJwtTokenResponse, HttpError>provides explicit error handling without exceptions. Early returns for missing user and wallet make the happy path clearer, and the.mapErr()transformation properly converts validation errors into HTTP errors.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/deploy-web/src/components/deployments/DeploymentList.tsx (1)
232-240: Same aria-disabled issue in no-deployments section.This Link has the same issue as the one on Line 188:
aria-disableddoesn't prevent navigation when the blockchain is down.Apply the same fix as suggested for lines 188-196 to maintain consistency.
♻️ Duplicate comments (1)
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (1)
53-55: Avoid rendering an empty Alert when there is no message.
warningTextcan be undefined (e.g., when credentials exist but are not usable for other reasons), causing a blank Alert to be rendered.- <d.Alert variant="warning" className={cn({ "py-2 text-sm": buttonProps?.size === "sm" }, "truncate")}> - {warningText} - </d.Alert> + {warningText && ( + <d.Alert variant="warning" className={cn({ "py-2 text-sm": buttonProps?.size === "sm" }, "truncate")}> + {warningText} + </d.Alert> + )}
🧹 Nitpick comments (5)
apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsx (1)
10-10: Consider usingqueryBymethods per coding guidelines.The coding guidelines specify that
.spec.tsxfiles should usequeryBymethods instead ofgetBymethods in test expectations. Lines 12 and 18 usegetByTextwithinexpect()calls.As per coding guidelines, apply this diff:
it("renders button enabled when blockchain is up", () => { - const { getByText } = setup({ isBlockchainDown: false }); + const { queryByText } = setup({ isBlockchainDown: false }); - expect(getByText("Start Trial").parentElement).not.toHaveAttribute("disabled"); + expect(queryByText("Start Trial")?.parentElement).not.toHaveAttribute("disabled"); }); it("renders button disabled when blockchain is unavailable", () => { - const { getByText } = setup({ isBlockchainDown: true }); + const { queryByText } = setup({ isBlockchainDown: true }); - expect(getByText("Start Trial").parentElement).toHaveAttribute("disabled"); + expect(queryByText("Start Trial")?.parentElement).toHaveAttribute("disabled"); });Also applies to: 12-12, 16-16, 18-18
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (1)
93-100: Consider combining overlapping test cases.Lines 93-100 and 136-144 test the same scenario (missing cert) but assert different aspects:
- Lines 93-100: Checks
usableis false- Lines 136-144: Checks
typeis "mtls" andvalueis nullWhile documenting different behavioral aspects is valid, combining these into a single test would reduce duplication and make the relationship between these assertions clearer:
- it("returns unusable mTLS credentials when cert is missing", () => { - const { result } = setup({ - settings: { isBlockchainDown: false }, - certificate: { localCert: null, isLocalCertExpired: false, isLocalCertMatching: true } - }); - - expect(result.current.details.usable).toBe(false); - }); - - it("returns null mTLS value when cert is missing", () => { + it("returns unusable mTLS credentials with null value when cert is missing", () => { const { result } = setup({ settings: { isBlockchainDown: false }, certificate: { localCert: null, isLocalCertExpired: false, isLocalCertMatching: true } }); expect(result.current.details.type).toBe("mtls"); expect(result.current.details.value).toBeNull(); + expect(result.current.details.usable).toBe(false); });Also applies to: 136-144
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (3)
23-37: Remove unused unableToCreate message.The
unableToCreatemessage for MTLS (Line 29) is defined but never referenced in the component code. Either remove it or implement the logic to display it when appropriate.If the message is not needed:
const MESSAGES = { mtls: { expired: "Your certificate has expired. Please create a new one.", missing: "You need to create a certificate to view deployment details.", regenerateButton: "Regenerate Certificate", - createButton: "Create Certificate", - unableToCreate: "You cannot view deployment lease details because the blockchain is unavailable and you don't have a local certificate." + createButton: "Create Certificate" },
56-58: Simplify conditional margin logic.If the Alert is conditionally rendered (per the previous comment), the margin logic should be updated accordingly to avoid applying margin when no Alert is shown.
After applying the conditional Alert rendering fix:
<d.Button - className={warningText ? "mt-4" : ""} + className={cn({ "mt-4": warningText })} {...buttonProps}
39-39: Consider avoiding single-letter aliases for readability.The shorthand alias
dfordependenciesreduces code readability and is non-standard. Consider using a more descriptive name or destructuring specific dependencies.Option 1: Destructure specific dependencies:
-export const CreateCredentialsButton: FC<Props> = ({ afterCreate, containerClassName, dependencies: d = DEPENDENCIES, ...buttonProps }) => { - const credentials = d.useProviderCredentials(); +export const CreateCredentialsButton: FC<Props> = ({ + afterCreate, + containerClassName, + dependencies = DEPENDENCIES, + ...buttonProps +}) => { + const { Alert, Button, Spinner, useProviderCredentials } = dependencies; + const credentials = useProviderCredentials();Then update JSX:
- <d.Alert variant="warning" className={cn({ "py-2 text-sm": buttonProps?.size === "sm" }, "truncate")}> + <Alert variant="warning" className={cn({ "py-2 text-sm": buttonProps?.size === "sm" }, "truncate")}> {warningText} - </d.Alert> - <d.Button + </Alert> + <Button className={warningText ? "mt-4" : ""} {...buttonProps} disabled={buttonProps?.disabled || createCredentialsState.isPending} onClick={createCredentials} > - {createCredentialsState.isPending ? <d.Spinner size="small" /> : buttonText} - </d.Button> + {createCredentialsState.isPending ? <Spinner size="small" /> : buttonText} + </Button>Option 2: Use a more descriptive alias like
deps:-export const CreateCredentialsButton: FC<Props> = ({ afterCreate, containerClassName, dependencies: d = DEPENDENCIES, ...buttonProps }) => { +export const CreateCredentialsButton: FC<Props> = ({ afterCreate, containerClassName, dependencies: deps = DEPENDENCIES, ...buttonProps }) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/deploy-web/src/components/authorizations/Authorizations.tsx(1 hunks)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx(1 hunks)apps/deploy-web/src/components/deployments/DeploymentList.tsx(1 hunks)apps/deploy-web/src/components/deployments/ManifestUpdate.tsx(5 hunks)apps/deploy-web/src/components/layout/TopBanner.tsx(1 hunks)apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx(5 hunks)apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx(3 hunks)apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsx(1 hunks)apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx(1 hunks)apps/deploy-web/src/components/wallet/ManagedWalletPopup.tsx(1 hunks)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx(3 hunks)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts(1 hunks)apps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.ts(3 hunks)apps/deploy-web/src/services/createFetchAdapter/createFetchAdapter.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx
- apps/deploy-web/src/components/layout/TopBanner.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx
- apps/deploy-web/src/components/deployments/ManifestUpdate.tsx
- apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/deploy-web/src/components/wallet/ManagedWalletPopup.tsxapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsxapps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsxapps/deploy-web/src/components/authorizations/Authorizations.tsxapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.tsapps/deploy-web/src/services/createFetchAdapter/createFetchAdapter.tsapps/deploy-web/src/components/deployments/DeploymentList.tsxapps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.tsapps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx
**/*.{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/deploy-web/src/components/wallet/ManagedWalletPopup.tsxapps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsxapps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsxapps/deploy-web/src/components/authorizations/Authorizations.tsxapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.tsapps/deploy-web/src/services/createFetchAdapter/createFetchAdapter.tsapps/deploy-web/src/components/deployments/DeploymentList.tsxapps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.tsapps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx
**/*.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/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsxapps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-29T03:55:35.893Z
Learnt from: stalniy
PR: akash-network/console#1971
File: apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.tsx:47-48
Timestamp: 2025-09-29T03:55:35.893Z
Learning: In the Akash Network Console project, when blockchain is down (settings.isBlockchainDown), disabling buttons like "Start Trial" is acceptable instead of completely hiding them.
Applied to files:
apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsxapps/deploy-web/src/components/authorizations/Authorizations.tsx
🧬 Code graph analysis (4)
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (4)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
DEPENDENCIES(8-12)packages/ui/components/button.tsx (1)
ButtonProps(36-38)apps/deploy-web/src/hooks/useAsyncCallback/useAsyncCallback.ts (1)
useAsyncCallback(3-25)apps/provider-console/src/utils/styleUtils.ts (1)
cn(4-6)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (3)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (2)
useProviderCredentials(26-60)DEPENDENCIES(8-12)apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (2)
localCert(365-367)isLocalCertExpired(369-371)apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (2)
DEPENDENCIES(11-15)UseProviderJwtResult(89-93)
apps/deploy-web/src/components/deployments/DeploymentList.tsx (3)
apps/deploy-web/src/utils/urlUtils.ts (1)
UrlService(16-81)apps/provider-console/src/utils/styleUtils.ts (1)
cn(4-6)packages/ui/components/button.tsx (1)
buttonVariants(46-46)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (3)
apps/deploy-web/src/services/app-di-container/app-di-container.ts (1)
withInterceptors(151-155)apps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.ts (1)
createFallbackableHttpClient(7-45)packages/http-sdk/src/utils/isHttpError.ts (1)
isHttpError(3-5)
⏰ 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). (10)
- 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 / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (10)
apps/deploy-web/src/components/authorizations/Authorizations.tsx (1)
215-215: LGTM! Improved user-facing text.The wording change from "down" to "unavailable" improves clarity and maintains a more professional tone.
apps/deploy-web/src/services/createFetchAdapter/createFetchAdapter.ts (1)
14-14: LGTM! Improved parameter naming.The parameter rename from
configtorequestConfigmakes the callback signature more descriptive and self-documenting.apps/deploy-web/src/components/wallet/ManagedWalletPopup.tsx (1)
60-60: LGTM! Improved error messaging.The updated text is clearer and more professional than "down". This improves the user experience when the blockchain is unavailable.
apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsx (1)
15-15: Terminology update aligns with broader UI changes.The test description now uses "blockchain is unavailable" instead of "blockchain is down", which aligns with the broader terminology shift across the UI mentioned in the PR summary.
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (2)
1-205: Excellent test coverage and coding guidelines adherence!The test suite comprehensively covers all credential scenarios (JWT/mTLS, usable/unusable conditions) and correctly follows the coding guidelines:
- ✓ Uses
jest-mock-extendedfor mocking dependencies- ✓
setupfunction at the bottom with inline type definition- ✓ No shared state or return type annotation
- ✓ Proper TypeScript typing without
any
147-147: Verifyjest.fn()usage aligns with project conventions.Lines 147 and 160 use
jest.fn()directly to create function mocks. While the coding guideline prohibitsjest.mock(), it doesn't explicitly addressjest.fn()for individual function mocks within the dependency injection pattern.If stricter adherence is preferred, these could be replaced with fully mocked objects from
jest-mock-extended:// Current approach (lines 147-151) const generateToken = jest.fn().mockResolvedValue(undefined); const { result } = setup({ settings: { isBlockchainDown: true }, providerJwt: { generateToken } }); // Alternative with jest-mock-extended const providerJwt = mock<UseProviderJwtResult>({ generateToken: jest.fn().mockResolvedValue(undefined) }); const { result } = setup({ settings: { isBlockchainDown: true }, providerJwt });However, the current approach is pragmatic and works well with the setup function's partial merging pattern.
Based on coding guidelines
Also applies to: 160-160
apps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.ts (2)
1-1: LGTM: Modern TypeScript import syntax.The inline type import syntax is correct and follows modern TypeScript conventions.
17-17: Verify the circuit breaker timing reduction is intentional.The
maxDelayfor the exponential backoff has been reduced from 10 minutes to 5 minutes. This means the circuit breaker will attempt to transition to half-open state more aggressively. Ensure this aligns with the expected recovery characteristics of the backend services—if services need more time to recover from failures, this could result in premature retry attempts.apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (2)
1-15: LGTM!The dependency injection pattern via the
DEPENDENCIESconstant is well-designed for testability and follows React best practices.
17-21: LGTM!The Props interface is well-structured, properly extends ButtonProps while omitting
onClick, and includes the necessary props for dependency injection and post-creation callbacks.
apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx
Show resolved
Hide resolved
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.ts
Show resolved
Hide resolved
stalniy
force-pushed
the
feat/jwt-offline
branch
from
fb9686d to
17c1a0a
Compare
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/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx (1)
141-141: Replaceas anycast with proper typing.The
null as anycast violates the coding guideline that prohibits usinganytypes. Define the proper return type for thegetBlockfunction.Apply this diff:
- getBlock: async () => null as any + getBlock: async () => null as unknown as BlockDetailOr better yet, update the
getBlockparameter type in thesetupfunction to acceptnull:getBlock?: () => Promise<BlockDetail | null>;
♻️ Duplicate comments (2)
apps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.ts (1)
27-30: Address the unresolved issues from previous review.The async error handler still has the two issues identified in the previous review:
- Type mismatch:
onUnavailableErroris typed as() => void(line 49) but is now called asynchronously- Missing error handling: If the handler throws or rejects, the fallback request won't execute
Please see the detailed previous review comment with suggested fixes for type signature and error handling.
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
48-70: Ping still bypasses the intended outage detection.The critical issue flagged in the previous review remains unresolved. Line 50 issues the health probe through
chainApiHttpClient, which is the same fallbackable client being guarded. When the primary RPC is down but the fallback stays up, the ping succeeds via the fallback path, thecatchbranch at line 56 never setsisBlockchainDown = true, and the original error is re-thrown at line 54—effectively preventing the offline fallback from activating.As suggested in the previous review, create a separate non-fallbackable HTTP client for the ping:
+ const pingHttpClient = withInterceptors( + rootContainer.createAxios({ baseURL: settingsState.settings?.apiEndpoint }), + { request: [config => (config.baseURL ? config : neverResolvedPromise)] } + ); + inflightPingRequest ??= chainApiHttpClient - .get(BLOCKCHAIN_PING_URL, { adapter: "fetch", timeout: 5000 }) + inflightPingRequest ??= pingHttpClient + .get(BLOCKCHAIN_PING_URL, { adapter: "fetch", timeout: 5000 })
🧹 Nitpick comments (4)
apps/deploy-web/src/components/wallet/ManagedWalletPopup.tsx (1)
60-60: LGTM! Text improvement enhances clarity.The updated message "blockchain is unavailable" is more appropriate and professional than "blockchain is down".
Optional: Consider styling consistency with the parent container.
The fallback message uses
text-white, while the parent container (line 31) usestext-primary dark:text-foregroundfor better theme integration. Additionally,space-x-2is ineffective here since there's only one child element.Apply this diff for improved consistency:
-)) || <div className="space-x-2 text-xs text-white">Wallet Balance is unknown because the blockchain is unavailable</div>} +)) || <div className="text-xs text-primary dark:text-foreground">Wallet Balance is unknown because the blockchain is unavailable</div>}apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsx (1)
10-18: Consider usingqueryBymethods per coding guidelines.The coding guidelines specify using
queryBymethods instead ofgetBymethods in test expectations for.spec.tsxfiles. While this is a pre-existing pattern, consider refactoring both test cases to usequeryByTextfor consistency with project standards.Apply this diff:
it("renders button enabled when blockchain is up", () => { - const { getByText } = setup({ isBlockchainDown: false }); + const { queryByText } = setup({ isBlockchainDown: false }); - expect(getByText("Start Trial").parentElement).not.toHaveAttribute("disabled"); + expect(queryByText("Start Trial")?.parentElement).not.toHaveAttribute("disabled"); }); it("renders button disabled when blockchain is unavailable", () => { - const { getByText } = setup({ isBlockchainDown: true }); + const { queryByText } = setup({ isBlockchainDown: true }); - expect(getByText("Start Trial").parentElement).toHaveAttribute("disabled"); + expect(queryByText("Start Trial")?.parentElement).toHaveAttribute("disabled"); });As per coding guidelines.
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (1)
11-91: Consider extracting magic strings and grouping tests.The test logic is correct and covers important scenarios. For improved maintainability, consider:
Extract magic strings to constants at the top of the file:
const TEST_JWT_TOKEN = "jwt-token-123"; const TEST_CERT_PEM = "cert-content"; const TEST_KEY_PEM = "key-content"; const TEST_ADDRESS = "akash123";Group related tests with nested
describeblocks:describe("when blockchain is down", () => { it("returns JWT credentials", () => { ... }); it("returns unusable JWT credentials when token is expired", () => { ... }); it("returns unusable JWT credentials when token is missing", () => { ... }); }); describe("when blockchain is up", () => { it("returns mTLS credentials", () => { ... }); it("returns unusable mTLS credentials when cert is expired", () => { ... }); // ... other mTLS tests });apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx (1)
179-179: Consider usingqueryByTextper coding guidelines.The coding guidelines specify using
queryBymethods instead ofgetBymethods in test expectations for.spec.tsxfiles. WhilegetByTextworks correctly insidewaitFor(it throws and triggers retries), usingqueryByTextmaintains consistency with the guidelines.As per coding guidelines.
Apply this diff if you want to align with guidelines:
- expect(screen.getByText(/Blockchain is unavailable/i)).toBeInTheDocument(); + expect(screen.queryByText(/Blockchain is unavailable/i)).toBeInTheDocument();Note: The existing pattern with
getByTextinsidewaitForis common and functional, so this change is optional for consistency only.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/deploy-web/src/components/authorizations/Authorizations.tsx(1 hunks)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx(1 hunks)apps/deploy-web/src/components/deployments/DeploymentList.tsx(1 hunks)apps/deploy-web/src/components/deployments/ManifestUpdate.tsx(5 hunks)apps/deploy-web/src/components/layout/TopBanner.tsx(1 hunks)apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx(5 hunks)apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx(3 hunks)apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsx(1 hunks)apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx(1 hunks)apps/deploy-web/src/components/wallet/ManagedWalletPopup.tsx(1 hunks)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx(3 hunks)apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts(1 hunks)apps/deploy-web/src/queries/useBalancesQuery.ts(1 hunks)apps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.ts(3 hunks)apps/deploy-web/src/services/createFetchAdapter/createFetchAdapter.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/src/components/authorizations/Authorizations.tsx
- apps/deploy-web/src/components/deployments/DeploymentList.tsx
- apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx
- apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.tsapps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsxapps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx
**/*.{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/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.tsapps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsxapps/deploy-web/src/components/deployments/ManifestUpdate.tsxapps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsxapps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.tsapps/deploy-web/src/services/createFetchAdapter/createFetchAdapter.tsapps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsxapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/src/queries/useBalancesQuery.tsapps/deploy-web/src/components/layout/TopBanner.tsxapps/deploy-web/src/components/wallet/ManagedWalletPopup.tsx
**/*.{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/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.tsapps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsxapps/deploy-web/src/components/deployments/ManifestUpdate.tsxapps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsxapps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.tsapps/deploy-web/src/services/createFetchAdapter/createFetchAdapter.tsapps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsxapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/src/queries/useBalancesQuery.tsapps/deploy-web/src/components/layout/TopBanner.tsxapps/deploy-web/src/components/wallet/ManagedWalletPopup.tsx
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsxapps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-29T03:55:35.893Z
Learnt from: stalniy
PR: akash-network/console#1971
File: apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.tsx:47-48
Timestamp: 2025-09-29T03:55:35.893Z
Learning: In the Akash Network Console project, when blockchain is down (settings.isBlockchainDown), disabling buttons like "Start Trial" is acceptable instead of completely hiding them.
Applied to files:
apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsx
🧬 Code graph analysis (5)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (4)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (2)
useProviderCredentials(26-60)DEPENDENCIES(8-12)apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (2)
localCert(365-367)isLocalCertExpired(369-371)apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx (1)
SettingsContextType(41-41)apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.ts (2)
DEPENDENCIES(11-15)UseProviderJwtResult(89-93)
apps/deploy-web/src/components/deployments/ManifestUpdate.tsx (3)
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.ts (1)
useProviderCredentials(26-60)apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx (1)
useSettings(333-335)apps/deploy-web/src/components/deployments/CreateCredentialsButton/CreateCredentialsButton.tsx (1)
CreateCredentialsButton(39-66)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (3)
apps/deploy-web/src/services/app-di-container/app-di-container.ts (1)
withInterceptors(151-155)apps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.ts (1)
createFallbackableHttpClient(7-45)packages/http-sdk/src/utils/isHttpError.ts (1)
isHttpError(3-5)
apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx (1)
apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (1)
localCert(365-367)
apps/deploy-web/src/queries/useBalancesQuery.ts (2)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(29-31)apps/deploy-web/src/queries/queryKeys.ts (1)
QueryKeys(1-122)
⏰ 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). (9)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (20)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx (1)
65-65: LGTM! Improved user-facing message.The text change from "blockchain is down" to "blockchain is unavailable" is more professional and aligns with the broader messaging consistency effort across the app.
apps/deploy-web/src/services/createFetchAdapter/createFetchAdapter.ts (1)
14-14: LGTM! Parameter name improves clarity.Renaming the second parameter from
configtorequestConfigin theonFailurecallback signature makes the interface more self-documenting. The implementation at Line 57 correctly passes theconfigargument, and since TypeScript interface parameter names are documentation-only, this change has no runtime impact.apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsx (1)
15-15: LGTM! Terminology alignment.The test description update from "down" to "unavailable" aligns with the broader terminology standardization across the codebase for blockchain availability status.
apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts (3)
1-9: LGTM!The imports are clean and follow the coding guidelines by using
jest-mock-extendedfor mocking instead ofjest.mock().
93-170: Excellent edge case coverage!The tests comprehensively cover all failure scenarios:
- Missing cert, missing key, expired cert, non-matching cert for mTLS
- Missing and expired token for JWT
- Both
generate()code pathsThe use of
jest.fn()for mockinggenerateTokenandcreateCertificateis appropriate and doesn't violate the coding guideline (which prohibitsjest.mock()for module mocking, notjest.fn()for function mocking).
172-204: LGTM! Setup function follows all coding guidelines.The setup function is correctly implemented:
- ✓ Located at the bottom of the describe block
- ✓ Accepts a single parameter with inline type definition
- ✓ No shared state across tests
- ✓ No return type specified
- ✓ Uses
jest-mock-extended'smock()function- ✓ Proper dependency injection pattern with sensible defaults
apps/deploy-web/src/queries/useBalancesQuery.ts (3)
9-9: LGTM! Clean refactoring.Destructuring services directly from
useServices()is clearer than accessing through adiwrapper.
12-15: LGTM! Explicit guard improves clarity.The explicit null check before calling
getBalancesis good practice and makes the logic more transparent.
17-17: Consistent fallback behavior confirmed
All query hooks disable themselves whenchainApiHttpClient.isFallbackEnabledistrue; no changes required if this is the intended UX.apps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.ts (1)
17-17: Verify the maxDelay reduction is intentional.The maximum exponential backoff delay has been reduced from 10 minutes to 5 minutes. While this improves responsiveness, it also means the circuit breaker will attempt to recover more frequently, potentially increasing load on a failing service.
Confirm this change is intentional and aligns with operational requirements for circuit breaker recovery timing.
apps/deploy-web/src/components/layout/TopBanner.tsx (1)
25-25: LGTM! Minor text improvement.The wording change from "down" to "unavailable" improves clarity for users.
apps/deploy-web/src/components/deployments/ManifestUpdate.tsx (4)
13-13: LGTM! Import updates align with the credential refactor.The new imports for
useSettingsanduseProviderCredentials, plus the switch toCreateCredentialsButton, correctly support the unified JWT/MTLS credential model.Also applies to: 15-15, 26-26
52-52: LGTM! Hook usage is correct.Both
useProviderCredentialsanduseSettingsare integrated properly to support the new credential flow.Also applies to: 54-54
130-130: LGTM! Credential payload integration is correct.The
providerCredentials.detailsobject properly replaces the previous certificate-based approach and supports both JWT and MTLS credential types.
247-260: Clarify offline manifest-only update behavior
Confirm whether all updates should be blocked whensettings.isBlockchainDownis true, or only version-changing updates should be disabled while manifest-only (JWT-based) updates proceed.apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
39-47: LGTM: Wrapper structure and caching mechanism.The approach of wrapping the fallbackable client with interceptors, caching the ping request to deduplicate concurrent checks, and clearing the cache after 3 seconds are all sound design choices. The
shouldFallbackcallback correctly coerces the blockchain-down flag to boolean.Also applies to: 71-81
apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.spec.tsx (4)
152-152: Text update improves clarity.The test description change from "Blockchain is down" to "Blockchain is unavailable" is more precise and user-friendly.
267-274: Credentials structure correctly applied in certificate creation test.The credentials payload structure is consistently applied in the test case for new certificate creation.
330-337: Credentials structure correctly applied in re-send manifest test.The credentials payload structure is consistently applied in the test case for re-sending the manifest.
210-217: Credentials structure and usage verified:ProviderCredentialsis defined as a proper discriminated union, thesendManifestsignature accepts it viaSendManifestToProviderOptions, and all UI call sites consistently pass the newcredentialsshape.
stalniy
force-pushed
the
feat/jwt-offline
branch
from
17c1a0a to
1207c74
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
59-62: Cache timeout may serve stale blockchain status.The 10-second cache window (line 61) means if the blockchain status changes mid-window, subsequent requests will use the stale cached result. For example, if the blockchain goes down during the cache period, requests will still think it's up and re-throw errors instead of falling back. Consider shortening the timeout or removing the cache if real-time status accuracy is critical.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx(3 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/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx
**/*.{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/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx
🧬 Code graph analysis (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (2)
apps/deploy-web/src/services/createFallbackableHttpClient/createFallbackableHttpClient.ts (2)
FallbackableHttpClient(6-6)createFallbackableHttpClient(7-45)apps/deploy-web/src/services/app-di-container/app-di-container.ts (1)
withInterceptors(151-155)
⏰ 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). (10)
- 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: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
50-69: Ping still uses the fallbackable client, masking real RPC outages.
The health check at lines 50–51 useschainApiHttpClient(which falls back internally), so a primary RPC outage is hidden,isBlockchainDownstays false, and the original request never falls back. Confirm whether this ping should use a non-fallback HTTP client (e.g. fromrootContainer.createAxios) to reliably detect primary RPC failures.
| }); | ||
| return inflightPingRequest.then(result => { | ||
| if (!result.isBlockchainDown) { | ||
| // if blockchain is available, then we have an issue wit some endpoint |
There was a problem hiding this comment.
Fix typo in comment.
"wit" should be "with".
- // if blockchain is available, then we have an issue wit some endpoint
+ // if blockchain is available, then we have an issue with some endpoint📝 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.
| // if blockchain is available, then we have an issue wit some endpoint | |
| // if blockchain is available, then we have an issue with some endpoint |
🤖 Prompt for AI Agents
In apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx around line
65, there's a typo in the inline comment where "wit" should be "with"; update
the comment text to read "if blockchain is available, then we have an issue with
some endpoint" (replace "wit" with "with") and keep formatting/spacing
consistent.
2 participants
Why
#1946
Summary by CodeRabbit
New Features
Improvements
Tests
Chores