[INJIVER-1464] reverted using /vp-results?response_code=xxx endpoint#1249
Conversation
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
WalkthroughPersist request/transaction IDs in sessionStorage (replacing in-memory refs), consolidate VP result handling (onVPProcessed vs onVPReceived), update vpResult API to accept an optional response_code and target /v2/vp-results/{transactionId}, and bump package version. Changes
Sequence DiagramsequenceDiagram
participant Component as OpenID4VPVerification
participant SessionStore as sessionStorage
participant PageVis as Page Visibility
participant API as vpResult API
participant Callback as Callback Handler
rect rgba(200,150,100,0.5)
PageVis->>Component: init / visibility change
Component->>SessionStore: read OVP_SESSION_REQUEST_ID_KEY / OVP_SESSION_TRANSACTION_ID_KEY
Component->>API: fetchVPStatus(transactionId, responseCode?)
API->>API: GET /v2/vp-results/{transactionId}?response_code=...
API-->>Component: VP status / credentialResults
alt onVPProcessed present
Component->>Callback: onVPProcessed(vpResult)
else fallback
Component->>Callback: onVPReceived(transactionId)
end
Component->>SessionStore: clearSessionData() on reset
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx (3)
145-189: Consider addingresetStateto dependencies.
resetStateis called withinfetchVPResultbut is not listed in the dependency array. While this may work correctly ifresetStatedoesn't change, it's inconsistent with React's exhaustive-deps rule.♻️ Proposed fix
}, - [verifyServiceUrl, onVPProcessed, onVPReceived, onError, vpVerificationV2Request] + [verifyServiceUrl, onVPProcessed, onVPReceived, onError, vpVerificationV2Request, resetState] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx` around lines 145 - 189, fetchVPResult uses resetState inside but resetState is missing from the useCallback dependency array; update the useCallback dependencies for fetchVPResult to include resetState so React's hooks consistency is preserved (i.e., add resetState alongside verifyServiceUrl, onVPProcessed, onVPReceived, onError, vpVerificationV2Request) and ensure resetState is stable or memoized if necessary to avoid unnecessary re-creations of fetchVPResult.
191-219: MissingresetStateandonErrorin dependency array.
fetchVPStatuscalls bothresetState()andonError()but neither is included in the dependency array.♻️ Proposed fix
[ verifyServiceUrl, onQrCodeExpired, fetchVPResult, + resetState, + onError, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx` around lines 191 - 219, fetchVPStatus uses resetState and onError but they are missing from the useCallback dependency array, which can cause stale closures; update the dependency list for the fetchVPStatus hook to include resetState and onError (alongside existing verifyServiceUrl, onQrCodeExpired, fetchVPResult) so the callback always captures the latest implementations used when calling resetState(), onError(), and interacting with isActiveRef and vpRequestStatus.
77-91: Missing dependency inresetStatecallback.
clearSessionDatais used insideresetStatebut is not included in the dependency array. SinceclearSessionDatais auseCallback, omitting it can lead to stale closure issues ifclearSessionDatawere to change (though currently it has no dependencies itself).♻️ Proposed fix
const resetState = useCallback(() => { if (redirectTimeoutRef.current) { clearTimeout(redirectTimeoutRef.current); redirectTimeoutRef.current = null; } setQrCodeData(null); setLoading(false); isActiveRef.current = false; clearSessionData(); - }, []); + }, [clearSessionData]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx` around lines 77 - 91, The resetState useCallback closes over clearSessionData but doesn't list it in its dependency array, which can cause stale closures; update the resetState declaration to include clearSessionData in its dependencies so the hook re-creates when clearSessionData changes (i.e., change useCallback(() => { ... }, [clearSessionData]) for the resetState function that references redirectTimeoutRef, setQrCodeData, setLoading, isActiveRef and clearSessionData).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inji-verify-sdk/package.json`:
- Line 3: The in-repo consumer verify-ui is still pinning
`@injistack/react-inji-verify-sdk` to "0.18.0-beta.14", so update the dependency
entry in verify-ui/package.json (the `@injistack/react-inji-verify-sdk` line) to
the new version "0.18.0-beta.16" (or a compatible range like "^0.18.0-beta.16"),
then reinstall and update the lockfile (yarn.lock or package-lock.json) so the
app picks up the bumped SDK and the new OpenID4VP behavior.
In `@inji-verify-sdk/src/utils/api.ts`:
- Around line 160-163: The code sets response_code on the URL fragment via
baseUrl.hash which is never sent to the server; change this to add response_code
as a query parameter instead (e.g., use baseUrl.searchParams.append or
equivalent) so the backend receives it; update the logic around baseUrl and
responseCode in the function that constructs the URL (look for baseUrl and
responseCode in api.ts) to append response_code to baseUrl.searchParams rather
than setting baseUrl.hash.
---
Nitpick comments:
In
`@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx`:
- Around line 145-189: fetchVPResult uses resetState inside but resetState is
missing from the useCallback dependency array; update the useCallback
dependencies for fetchVPResult to include resetState so React's hooks
consistency is preserved (i.e., add resetState alongside verifyServiceUrl,
onVPProcessed, onVPReceived, onError, vpVerificationV2Request) and ensure
resetState is stable or memoized if necessary to avoid unnecessary re-creations
of fetchVPResult.
- Around line 191-219: fetchVPStatus uses resetState and onError but they are
missing from the useCallback dependency array, which can cause stale closures;
update the dependency list for the fetchVPStatus hook to include resetState and
onError (alongside existing verifyServiceUrl, onQrCodeExpired, fetchVPResult) so
the callback always captures the latest implementations used when calling
resetState(), onError(), and interacting with isActiveRef and vpRequestStatus.
- Around line 77-91: The resetState useCallback closes over clearSessionData but
doesn't list it in its dependency array, which can cause stale closures; update
the resetState declaration to include clearSessionData in its dependencies so
the hook re-creates when clearSessionData changes (i.e., change useCallback(()
=> { ... }, [clearSessionData]) for the resetState function that references
redirectTimeoutRef, setQrCodeData, setLoading, isActiveRef and
clearSessionData).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d127d775-3bbd-4049-a097-23238f595695
⛔ Files ignored due to path filters (1)
inji-verify-sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
inji-verify-sdk/package.jsoninji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsxinji-verify-sdk/src/components/qrcode-verification/QRCodeVerification.tsxinji-verify-sdk/src/utils/api.tsinji-verify-sdk/src/utils/constants.ts
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx (1)
249-257:⚠️ Potential issue | 🟡 MinorMissing dependencies in
createVPRequestcallback.Several values used within the callback are missing from the dependency array:
acceptVPWithoutHolderProof(line 232)isSameDeviceFlowEnabled(lines 236, 241)fetchVPStatus(line 242)resetState(line 247)This can lead to stale closure bugs where the callback captures outdated values of these dependencies.
🔧 Proposed fix
}, [ verifyServiceUrl, transactionId, presentationDefinitionId, presentationDefinition, getPresentationDefinitionParams, onError, - clientId + clientId, + acceptVPWithoutHolderProof, + isSameDeviceFlowEnabled, + fetchVPStatus, + resetState ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx` around lines 249 - 257, The createVPRequest callback is missing several dependencies which can cause stale closures; update the dependency array used where createVPRequest is declared (the array currently listing verifyServiceUrl, transactionId, presentationDefinitionId, presentationDefinition, getPresentationDefinitionParams, onError, clientId) to also include acceptVPWithoutHolderProof, isSameDeviceFlowEnabled, fetchVPStatus, and resetState so the callback always captures the latest values of those symbols (ensure any functions are stable or memoized if necessary).inji-verify-sdk/src/utils/api.ts (1)
149-163:⚠️ Potential issue | 🟠 MajorPreserve the old 3-argument
vpResult(url, transactionId, config?)call shape.Line 149 turns an exported SDK API into a new positional signature. Existing JS/untyped consumers that still call
vpResult(url, txnId, { includeClaims: true })will now hit Lines 161-163 withresponse_code=[object Object], and the config flags on Lines 153-157 are silently dropped.💡 Backward-compatible fix
-export const vpResult = async (url: string, transactionId: string, responseCode?: string | null, config?: VPVerificationV2Request) => { +export const vpResult = async ( + url: string, + transactionId: string, + responseCodeOrConfig?: string | null | VPVerificationV2Request, + config?: VPVerificationV2Request +) => { + const responseCode = + typeof responseCodeOrConfig === "string" || responseCodeOrConfig == null + ? responseCodeOrConfig + : undefined; + const effectiveConfig = + typeof responseCodeOrConfig === "object" && responseCodeOrConfig !== null + ? responseCodeOrConfig + : config; + if (!transactionId) { throw new Error("Transaction ID is required for VP verification"); } const requestBody = { - skipStatusChecks: config?.skipStatusChecks ?? false, - statusCheckFilters: config?.statusCheckFilters ?? [], - includeClaims: config?.includeClaims ?? false, + skipStatusChecks: effectiveConfig?.skipStatusChecks ?? false, + statusCheckFilters: effectiveConfig?.statusCheckFilters ?? [], + includeClaims: effectiveConfig?.includeClaims ?? false, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inji-verify-sdk/src/utils/api.ts` around lines 149 - 163, The vpResult export changed the positional signature and now treats a passed config object as responseCode; to preserve backward compatibility, update vpResult to detect when the third positional argument (currently named responseCode) is actually an options object (typeof responseCode === "object" and config is undefined/null) and in that case reassign config = responseCode and set responseCode = undefined before building the request; ensure the URL search param for response_code is only appended when responseCode is a string and keep using config?.skipStatusChecks, config?.statusCheckFilters, and config?.includeClaims as before (refer to the vpResult function and its local variables requestBody, responseCode, and config).
🧹 Nitpick comments (4)
inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx (4)
188-188: MissingresetStatein dependency array.The
fetchVPResultcallback usesresetState(lines 171, 177, 182) but doesn't include it in the dependency array. This could lead to stale closure issues.🔧 Proposed fix
- [verifyServiceUrl, onVPProcessed, onVPReceived, onError, vpVerificationV2Request] + [verifyServiceUrl, onVPProcessed, onVPReceived, onError, vpVerificationV2Request, resetState]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx` at line 188, The fetchVPResult callback references resetState but the dependency array for the useCallback (currently listing [verifyServiceUrl, onVPProcessed, onVPReceived, onError, vpVerificationV2Request]) omits resetState, risking stale closures; update the dependency array for fetchVPResult to include resetState so React recreates the callback when resetState changes, ensuring the function always uses the latest resetState implementation (locate fetchVPResult and the useCallback that declares its dependencies to make this change).
82-91: MissingclearSessionDatain dependency array.The
resetStatecallback now usesclearSessionDatabut doesn't include it in the dependency array. While this works in practice (sinceclearSessionDatahas stable references), it violates React's exhaustive-deps rule and could cause issues ifclearSessionData's dependencies change in the future.🔧 Proposed fix
const resetState = useCallback(() => { if (redirectTimeoutRef.current) { clearTimeout(redirectTimeoutRef.current); redirectTimeoutRef.current = null; } setQrCodeData(null); setLoading(false); isActiveRef.current = false; clearSessionData(); - }, []); + }, [clearSessionData]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx` around lines 82 - 91, The resetState callback (defined with useCallback) closes over clearSessionData but currently omits it from the dependency array; update the useCallback dependency array for resetState to include clearSessionData so React's exhaustive-deps rule is satisfied and the callback will update if clearSessionData changes (locate the resetState definition and add clearSessionData to its dependencies).
5-5: Unused import:SessionState
SessionStateis imported but not used anywhere in this file. The code stores and retrievesrequestIdandtransactionIdindividually from sessionStorage rather than as a serializedSessionStateobject.🧹 Proposed fix
import { AppError, - SessionState, OpenID4VPVerificationProps, VerificationResults, CredentialResult } from "./OpenID4VPVerification.types";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx` at line 5, Remove the unused import SessionState from OpenID4VPVerification.tsx and update any references if present to avoid relying on it; instead continue using the individual sessionStorage keys requestId and transactionId (ensure functions or hooks like the ones reading/writing sessionStorage use those keys directly), and run a quick lint/type check to confirm no remaining references to SessionState remain in the file.
129-143: Dead code:processVPResultResponseis defined but never called.This function is not invoked anywhere in this file. The same mapping logic is duplicated inline in
fetchVPResult(lines 160-168). Consider either removing this unused function or refactoringfetchVPResultto use it to avoid code duplication.Option 1: Remove the dead code
- const processVPResultResponse = useCallback( - (response: { credentialResults?: CredentialResult[]; transactionId?: string }, txnId?: string) => { - const VPResult: VerificationResults = - (response.credentialResults ?? []).map((cred: CredentialResult) => ({ - vc: normalizeVp(cred.verifiableCredential), - verificationResponse: cred, - })); - if (onVPProcessed) { - onVPProcessed(VPResult); - } else if (onVPReceived && (txnId || response.transactionId)) { - onVPReceived(txnId ?? response.transactionId ?? ""); - } - }, - [onVPProcessed, onVPReceived] - ); -Option 2: Reuse the helper in fetchVPResult
If
processVPResultResponseis intended for future use, refactorfetchVPResultto call it instead of duplicating the mapping logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx` around lines 129 - 143, The defined but unused helper processVPResultResponse should be invoked from fetchVPResult to remove duplicated mapping logic; update fetchVPResult to call processVPResultResponse(response, txnId) (or just response) instead of performing the (response.credentialResults ?? []).map(...) logic inline, ensuring you still pass/derive the transaction id and keep usage of normalizeVp, onVPProcessed and onVPReceived intact; alternatively, if you truly don't need it, delete processVPResultResponse to remove dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx`:
- Around line 249-257: The createVPRequest callback is missing several
dependencies which can cause stale closures; update the dependency array used
where createVPRequest is declared (the array currently listing verifyServiceUrl,
transactionId, presentationDefinitionId, presentationDefinition,
getPresentationDefinitionParams, onError, clientId) to also include
acceptVPWithoutHolderProof, isSameDeviceFlowEnabled, fetchVPStatus, and
resetState so the callback always captures the latest values of those symbols
(ensure any functions are stable or memoized if necessary).
In `@inji-verify-sdk/src/utils/api.ts`:
- Around line 149-163: The vpResult export changed the positional signature and
now treats a passed config object as responseCode; to preserve backward
compatibility, update vpResult to detect when the third positional argument
(currently named responseCode) is actually an options object (typeof
responseCode === "object" and config is undefined/null) and in that case
reassign config = responseCode and set responseCode = undefined before building
the request; ensure the URL search param for response_code is only appended when
responseCode is a string and keep using config?.skipStatusChecks,
config?.statusCheckFilters, and config?.includeClaims as before (refer to the
vpResult function and its local variables requestBody, responseCode, and
config).
---
Nitpick comments:
In
`@inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsx`:
- Line 188: The fetchVPResult callback references resetState but the dependency
array for the useCallback (currently listing [verifyServiceUrl, onVPProcessed,
onVPReceived, onError, vpVerificationV2Request]) omits resetState, risking stale
closures; update the dependency array for fetchVPResult to include resetState so
React recreates the callback when resetState changes, ensuring the function
always uses the latest resetState implementation (locate fetchVPResult and the
useCallback that declares its dependencies to make this change).
- Around line 82-91: The resetState callback (defined with useCallback) closes
over clearSessionData but currently omits it from the dependency array; update
the useCallback dependency array for resetState to include clearSessionData so
React's exhaustive-deps rule is satisfied and the callback will update if
clearSessionData changes (locate the resetState definition and add
clearSessionData to its dependencies).
- Line 5: Remove the unused import SessionState from OpenID4VPVerification.tsx
and update any references if present to avoid relying on it; instead continue
using the individual sessionStorage keys requestId and transactionId (ensure
functions or hooks like the ones reading/writing sessionStorage use those keys
directly), and run a quick lint/type check to confirm no remaining references to
SessionState remain in the file.
- Around line 129-143: The defined but unused helper processVPResultResponse
should be invoked from fetchVPResult to remove duplicated mapping logic; update
fetchVPResult to call processVPResultResponse(response, txnId) (or just
response) instead of performing the (response.credentialResults ?? []).map(...)
logic inline, ensuring you still pass/derive the transaction id and keep usage
of normalizeVp, onVPProcessed and onVPReceived intact; alternatively, if you
truly don't need it, delete processVPResultResponse to remove dead code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e202953-584b-4093-a47a-4abb956130a7
📒 Files selected for processing (2)
inji-verify-sdk/src/components/openid4vp-verification/OpenID4VPVerification.tsxinji-verify-sdk/src/utils/api.ts
Summary by CodeRabbit
Bug Fixes
Improvements
Chores