feat: support W3C JSON-LD credential issuance and presentation#389
Open
sagarkhole4 wants to merge 2 commits into
Open
feat: support W3C JSON-LD credential issuance and presentation#389sagarkhole4 wants to merge 2 commits into
sagarkhole4 wants to merge 2 commits into
Conversation
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
📝 WalkthroughWalkthroughThis PR implements comprehensive W3C Verifiable Credentials support in OpenID4VC flows. New holder endpoints retrieve and manage W3C credentials; issuer offer generation gains version-aware payload transformation for v1.1 and v2.0 contexts; JWT VC JSON formats are mapped into W3C credentials; verifier proof requests now support Presentation Exchange. Types, routes, and API docs are updated throughout. ChangesW3C VC Support Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/controllers/openid4vc/holder/credentialBindingResolver.ts (1)
100-107:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate fallback messaging to match newly supported JWT VC formats.
The branch now supports
JwtVcJsonLdandJwtVcJson, but the comment/error text still implies only sd-jwt/mdoc support.✏️ Suggested text alignment
- // Otherwise we also support plain jwk for sd-jwt only + // Otherwise we also support plain jwk for sd-jwt, jwt-vc-json(-ld), and mdoc @@ - `No supported binding method could be found. Supported methods are did:key and did:jwk, or plain jwk for sd-jwt/mdoc. Issuer supports ${ + `No supported binding method could be found. Supported methods are did:key and did:jwk, or plain jwk for sd-jwt/jwt-vc-json/jwt-vc-json-ld/mdoc. Issuer supports ${ supportsJwk ? 'jwk, ' : '' }${supportedDidMethods?.join(', ') ?? 'Unknown'}`,Also applies to: 115-118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/controllers/openid4vc/holder/credentialBindingResolver.ts` around lines 100 - 107, Update the comment/error text in credentialBindingResolver.ts to reflect the expanded fallback support: where the code checks supportsJwk and credentialFormat against OpenId4VciCredentialFormatProfile (including SdJwtVc, SdJwtDc, JwtVcJsonLd, JwtVcJson, MsoMdoc), change any messaging that currently mentions only "sd-jwt/mdoc" to list or generically state "sd-jwt, JWT VC (JSON/JSON-LD) and mdoc" so logs and comments align with the actual supported formats checked by the supportsJwk/credentialFormat branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/controllers/openid4vc/holder/holder.service.ts`:
- Line 157: The if-condition formatting around credentialRecord and
W3cCredentialRecord is violating Prettier; update the three occurrences (the
conditional at the current if and the ones around lines referenced) to a
Prettier-compliant style by reformatting the conditional expression that checks
instance and type (use a consistent cast/parentheses and spacing for
"(credentialRecord as any).type" or prefer a safe type guard) so the file
holder.service.ts compiles with linting; locate references to credentialRecord
and W3cCredentialRecord in the methods around the mentioned checks and apply the
consistent formatting change.
In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts`:
- Around line 36-43: transformPayloadForVersion currently returns the original
payload so effectiveVersion is ignored and downstream mapping still treats
offers as v1; update transformPayloadForVersion (and its calls in
issuance-sessions.service.ts where effectiveVersion is computed) to actually
rewrite the credential JSON-LD for 'v2.0' (e.g., set `@context`, validFrom,
structure and vct fields to v2 names) or, alternatively, persist the requested
version on the session/offer object (e.g., add a requestedVersion property) so
oid4vc-agent's mapper (which decides between W3cCredential and W3cV2Credential)
can read it; locate transformPayloadForVersion and any code referencing
effectiveVersion/statusBlock/cred.payload and implement the real transformation
or persist the explicit version flag accordingly.
- Around line 71-72: The code always forces version to 'v1' when building the
offer (see the assignment to version in the issuance session creation), so
options.version is ignored; change the logic in the offer creation (the
variable/assignment that currently reads version: options.version ? 'v1' : 'v1')
to actually use options.version when provided (e.g., set version =
options.version ?? 'v1' or pass options.version directly) so the emitted offer
reflects the requested version; update any callers or validation in the issuance
session creation method (e.g., methods in IssuanceSessionsService that construct
the offer) to accept and propagate the provided options.version.
- Around line 88-90: The current validation always requires
cred.payload.credentialSubject and thereby rejects non-W3C credential formats;
update the validator in createCredentialOffer to only enforce credentialSubject
for W3C VC formats — i.e., first detect W3C VCs by checking the credential's
format metadata (e.g., cred.format or cred.format.type / formatId used in your
code) and only then throw BadRequestError using the existing message referencing
cred.credentialSupportedId when cred.payload?.credentialSubject is missing;
leave SdJwtDc and MsoMdoc flows (flat claims or namespaces) untouched so their
mapper logic can run.
In `@src/controllers/openid4vc/types/issuer.types.ts`:
- Line 7: Replace the value import of SignerMethod with a type-only import since
SignerMethod is used only in type positions; update the import statement that
currently reads "import { SignerMethod } ..." to use a type import (e.g.,
"import type { SignerMethod } ...") so it satisfies
`@typescript-eslint/consistent-type-imports` and unblocks linting for the types
referenced in issuer.types.ts.
In
`@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts`:
- Line 66: Remove the trailing comma after "version: dto.version" in the object
literal and reformat the indented block around lines 180-184 to match project
Prettier settings; specifically, in the VerificationSessionsService (where the
verification session object is constructed) remove the extra comma from the
"version: dto.version" property and align the nested block (the subsequent
object/array entries) to the same indentation level as other object properties
so Prettier linting passes.
- Around line 74-82: The code must enforce mutual exclusivity between
dto.presentationExchange and dto.dcql before building options for
verifier.createAuthorizationRequest: validate that not both are provided and
throw/return a clear error when they are; if only presentationExchange is
present set options.presentationExchange = dto.presentationExchange and do not
set options.dcql, otherwise set options.dcql = dto.dcql; keep the existing
parsedCertificate/requestSigner handling (parsedCertificate.publicJwk.keyId and
options.requestSigner.x5c) unchanged but ensure options.dcql is not forwarded
when presentationExchange is used and vice versa.
In `@src/routes/routes.ts`:
- Around line 1444-1473: The schemas W3cIssuer, W3cCredentialStatus, and
W3cCredentialSubject are too strict (additionalProperties: false) and reject
valid VCs; fix by updating the original TypeScript model definitions that
produce these refs so tsoa will allow extra top-level fields — add an index
signature (e.g. [key: string]: any) or otherwise enable additionalProperties on
the interfaces/types for W3cIssuer, W3cCredentialStatus, and
W3cCredentialSubject (and ensure W3cCredentialSubject still includes the
existing id and claims properties but permits other top-level terms); then
re-run tsoa to regenerate routes.ts so the generated refs no longer set
additionalProperties: false.
In `@src/routes/swagger.json`:
- Around line 1783-1804: The schema for validFrom and validUntil currently uses
anyOf with {type: "string"} plus {type: "string", format: "date-time"}, which
effectively allows any string and bypasses date-time validation; replace the
anyOf with a single schema that enforces date-time (e.g., { "type": "string",
"format": "date-time" }) for the properties named validFrom and validUntil in
the swagger.json so the format constraint is enforced (also make the identical
change for the other occurrence around the 2057-2078 region).
In `@src/utils/oid4vc-agent.ts`:
- Around line 206-247: The code flattens array credentialSubject values by doing
"{ ...(payload.credentialSubject || {}) }" and later assumes a single object
when assigning ids; change the logic to detect whether payload.credentialSubject
is an Array or an Object (preserve arrays as-is), build credentialSubjectRaw
accordingly, and when applying subject ids set the id on each object element
(e.g., loop through credentialSubjectRaw array and assign subjectId per element)
before creating credInstance via JsonTransformer.fromJSON (used with
W3cV2Credential / W3cCredential), and likewise, when mutating
credInstance.credentialSubject assign id per-element if it's an array or assign
directly if it's an object.
- Around line 260-262: The catch block that currently does
fs.appendFileSync('mapper-error.log', e.stack + '\n') (the catch (e: any) {...}
that ends with throw e;) must not perform a synchronous file write that can
throw and block; either log the stack via the existing logger (e.g.,
processLogger or logger) instead of appendFileSync, or if you still want a file
fallback use fs.promises.appendFile(...) and wrap that call in its own try/catch
so any error from the write is swallowed or logged but does not override the
original exception; in all cases ensure you always rethrow the original error
variable e with throw e.
- Around line 193-253: The mapping for JwtVcJson/JwtVcJsonLd currently always
uses issuerDidVerificationMethod and thus drops SignerMethod.X5c signer data;
fix by detecting when credential.signerOptions.method === SignerMethod.X5c (or
binding.method indicates x5c) and either (A) reject/unable-to-map with a clear
error/throw when X5c is configured for JwtVcJson/JwtVcJsonLd, or (B) implement
the x5c path: populate verificationMethod with an object containing the x5c
certificate chain (e.g., include x5c array and appropriate type/id) instead of
issuerDidVerificationMethod, and ensure the returned credential entry (inside
the credentials.map) carries that verificationMethod. Reference symbols:
JwtVcJson/JwtVcJsonLd branch, credential.signerOptions.method, SignerMethod.X5c,
issuerDidVerificationMethod, and the credentials.map return object.
---
Outside diff comments:
In `@src/controllers/openid4vc/holder/credentialBindingResolver.ts`:
- Around line 100-107: Update the comment/error text in
credentialBindingResolver.ts to reflect the expanded fallback support: where the
code checks supportsJwk and credentialFormat against
OpenId4VciCredentialFormatProfile (including SdJwtVc, SdJwtDc, JwtVcJsonLd,
JwtVcJson, MsoMdoc), change any messaging that currently mentions only
"sd-jwt/mdoc" to list or generically state "sd-jwt, JWT VC (JSON/JSON-LD) and
mdoc" so logs and comments align with the actual supported formats checked by
the supportsJwk/credentialFormat branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 75954c14-55f0-4b8c-bd82-a66658c621a6
📒 Files selected for processing (12)
src/controllers/openid4vc/holder/credentialBindingResolver.tssrc/controllers/openid4vc/holder/holder.Controller.tssrc/controllers/openid4vc/holder/holder.service.tssrc/controllers/openid4vc/issuance-sessions/issuance-sessions.service.tssrc/controllers/openid4vc/types/holder.types.tssrc/controllers/openid4vc/types/issuer.types.tssrc/controllers/openid4vc/types/verifier.types.tssrc/controllers/openid4vc/verifier-sessions/verification-sessions.service.tssrc/enums/enum.tssrc/routes/routes.tssrc/routes/swagger.jsonsrc/utils/oid4vc-agent.ts
Comment on lines
+1444
to
+1473
| "W3cIssuer": { | ||
| "dataType": "refObject", | ||
| "properties": { | ||
| "id": {"dataType":"string","required":true}, | ||
| }, | ||
| "additionalProperties": false, | ||
| }, | ||
| // WARNING: This file was auto-generated with tsoa. Please do not modify it. Re-run tsoa to re-generate this file: https://github.com/lukeautry/tsoa | ||
| "W3cCredentialSchema": { | ||
| "dataType": "refObject", | ||
| "properties": { | ||
| "id": {"dataType":"string","required":true}, | ||
| "type": {"dataType":"string","required":true}, | ||
| }, | ||
| "additionalProperties": false, | ||
| }, | ||
| // WARNING: This file was auto-generated with tsoa. Please do not modify it. Re-run tsoa to re-generate this file: https://github.com/lukeautry/tsoa | ||
| "SingleOrArray_W3cCredentialSchema_": { | ||
| "dataType": "refAlias", | ||
| "type": {"dataType":"union","subSchemas":[{"ref":"W3cCredentialSchema"},{"dataType":"array","array":{"dataType":"refObject","ref":"W3cCredentialSchema"}}],"validators":{}}, | ||
| }, | ||
| // WARNING: This file was auto-generated with tsoa. Please do not modify it. Re-run tsoa to re-generate this file: https://github.com/lukeautry/tsoa | ||
| "W3cCredentialStatus": { | ||
| "dataType": "refObject", | ||
| "properties": { | ||
| "id": {"dataType":"string","required":true}, | ||
| "type": {"dataType":"string","required":true}, | ||
| }, | ||
| "additionalProperties": false, | ||
| }, |
There was a problem hiding this comment.
W3C VC verification schema is overly strict for standard extension fields.
W3cIssuer, W3cCredentialStatus, and W3cCredentialSubject are generated with additionalProperties: false, and W3cCredentialSubject only allows id + claims. This can reject valid W3C credentials where issuer/status/subject contain additional top-level terms (common in real VC payloads), causing false validation failures on verify flows.
Also applies to: 1592-1616
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/routes.ts` around lines 1444 - 1473, The schemas W3cIssuer,
W3cCredentialStatus, and W3cCredentialSubject are too strict
(additionalProperties: false) and reject valid VCs; fix by updating the original
TypeScript model definitions that produce these refs so tsoa will allow extra
top-level fields — add an index signature (e.g. [key: string]: any) or otherwise
enable additionalProperties on the interfaces/types for W3cIssuer,
W3cCredentialStatus, and W3cCredentialSubject (and ensure W3cCredentialSubject
still includes the existing id and claims properties but permits other top-level
terms); then re-run tsoa to regenerate routes.ts so the generated refs no longer
set additionalProperties: false.
Comment on lines
+1783
to
+1804
| "validUntil": { | ||
| "anyOf": [ | ||
| { | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "type": "string", | ||
| "format": "date-time" | ||
| } | ||
| ] | ||
| }, | ||
| "validFrom": { | ||
| "anyOf": [ | ||
| { | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "type": "string", | ||
| "format": "date-time" | ||
| } | ||
| ] | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Finds "anyOf: [string, string+date-time]" patterns for temporal fields
rg -n -C3 '"valid(From|Until)"\s*:\s*\{|"format"\s*:\s*"date-time"|\"anyOf\"\s*:\s*\[' src/routes/swagger.jsonRepository: credebl/agent-controller
Length of output: 265
🏁 Script executed:
#!/bin/bash
# Read the file to check line counts and structure
wc -l src/routes/swagger.jsonRepository: credebl/agent-controller
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Check the first occurrence around lines 1783-1804
sed -n '1780,1810p' src/routes/swagger.json | cat -nRepository: credebl/agent-controller
Length of output: 862
🏁 Script executed:
#!/bin/bash
# Check the second occurrence around lines 2057-2078
sed -n '2054,2084p' src/routes/swagger.json | cat -nRepository: credebl/agent-controller
Length of output: 802
🏁 Script executed:
#!/bin/bash
# Use a simpler pattern to find validFrom and validUntil
rg -n 'validFrom|validUntil' src/routes/swagger.json | head -20Repository: credebl/agent-controller
Length of output: 229
🏁 Script executed:
#!/bin/bash
# Get context around each occurrence to understand which schemas they belong to
rg -B10 '"validUntil"|"validFrom"' src/routes/swagger.json | grep -E '(^[0-9]+-|"(validUntil|validFrom)"|"title"|schema)'Repository: credebl/agent-controller
Length of output: 199
🏁 Script executed:
#!/bin/bash
# Check context for the first occurrence to understand parent schema
sed -n '1770,1810p' src/routes/swagger.json | cat -nRepository: credebl/agent-controller
Length of output: 1088
🏁 Script executed:
#!/bin/bash
# Look for all temporal field definitions in the file
rg -B5 -A8 '"validUntil"' src/routes/swagger.json | head -80Repository: credebl/agent-controller
Length of output: 832
🏁 Script executed:
#!/bin/bash
# Get all anyOf occurrences with their exact line numbers and context
rg -n 'anyOf' src/routes/swagger.json | grep -B2 -A2 'validFrom\|validUntil'Repository: credebl/agent-controller
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Find all places where validUntil or validFrom have anyOf structure
rg -B1 -A10 '"(validUntil|validFrom)"\s*:\s*\{\s*"anyOf"' src/routes/swagger.json | grep -E '^[0-9]+' | head -30Repository: credebl/agent-controller
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Count how many validFrom/validUntil fields use anyOf pattern
rg '"(validUntil|validFrom)"\s*:\s*\{\s*"anyOf"' src/routes/swagger.json | wc -lRepository: credebl/agent-controller
Length of output: 70
🏁 Script executed:
#!/bin/bash
# Count total validFrom and validUntil definitions
rg '"(validUntil|validFrom)"\s*:' src/routes/swagger.json | wc -lRepository: credebl/agent-controller
Length of output: 70
🏁 Script executed:
#!/bin/bash
# Extract the exact structure for each validFrom/validUntil
for line in 1783 1794 1854 1858 2057 2068; do
echo "=== Line $line ==="
sed -n "${line},$((line+12))p" src/routes/swagger.json | head -15
doneRepository: credebl/agent-controller
Length of output: 1672
validFrom / validUntil currently bypass date-time validation due to anyOf shape.
These fields are declared as anyOf of { type: "string" } and { type: "string", format: "date-time" }, which effectively accepts any string and makes the date-time constraint non-operative. This can let invalid temporal values through contract validation.
🔧 Suggested schema fix
- "validUntil": {
- "anyOf": [
- { "type": "string" },
- { "type": "string", "format": "date-time" }
- ]
- },
+ "validUntil": {
+ "type": "string",
+ "format": "date-time"
+ },
- "validFrom": {
- "anyOf": [
- { "type": "string" },
- { "type": "string", "format": "date-time" }
- ]
- },
+ "validFrom": {
+ "type": "string",
+ "format": "date-time"
+ },Also applies to: 2057-2078
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/swagger.json` around lines 1783 - 1804, The schema for validFrom
and validUntil currently uses anyOf with {type: "string"} plus {type: "string",
format: "date-time"}, which effectively allows any string and bypasses date-time
validation; replace the anyOf with a single schema that enforces date-time
(e.g., { "type": "string", "format": "date-time" }) for the properties named
validFrom and validUntil in the swagger.json so the format constraint is
enforced (also make the identical change for the other occurrence around the
2057-2078 region).
sairanjit
requested changes
May 21, 2026
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts (1)
73-73:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass
options.versionto the offer instead of hardcoding'v1'.The code computes
effectiveVersion(line 31) and transforms the payload accordingly, but line 73 always passesversion: 'v1'to the offer creation. This means wallets negotiating v2.0 credentials will receive v1 offer metadata despite having v2.0-transformed payloads, making the versioning system ineffective.🔧 Proposed fix
- version: 'v1', + version: options.version ?? 'v1',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts` at line 73, The offer is being built with a hardcoded version: 'v1' which ignores the computed effectiveVersion and breaks v2 negotiations; update the object passed to the offer creation in issuance-sessions.service.ts to use the computed effectiveVersion (or options.version if that's what you prefer) instead of the literal 'v1' so the version field matches the transformed payload (refer to effectiveVersion and the offer creation call near the existing version: 'v1' line).src/controllers/openid4vc/holder/holder.service.ts (1)
47-58:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMajor: complete the W3C VC v2.0 holder lifecycle (store/list/delete)
getW3cCredentialsonly returnsagentReq.agent.w3cCredentials.getAll();w3cV2Credentials.getAll()support is commented out.requestAndStoreCredentialsonly storesW3cCredentialRecord; theW3cV2CredentialRecordstore branch is commented out, so v2 records hitUnsupported credential record type.deleteCredentialonly usesW3cCredentialService; theW3cV2CredentialService.removeCredentialRecordfallback is commented out and v2 deletion never happens.This breaks the advertised v2.0 flow (
version?: 'v1.1' | 'v2.0', plus v2 mapping inoid4vc-agent.ts->W3cV2Credential).Suggested direction
import { Mdoc, SdJwtVcRecord, MdocRecord, W3cCredentialRecord, W3cCredentialService, - // W3cV2CredentialRecord, - // W3cV2CredentialService, + W3cV2CredentialRecord, + W3cV2CredentialService, } from '`@credo-ts/core`' @@ public async getW3cCredentials(agentReq: Req) { - return await agentReq.agent.w3cCredentials.getAll() + const [v1Records, v2Records] = await Promise.all([ + agentReq.agent.w3cCredentials.getAll(), + agentReq.agent.w3cV2Credentials.getAll(), + ]) + + return [...v1Records, ...v2Records] } @@ if ( credentialRecord instanceof W3cCredentialRecord || (credentialRecord as { type?: string }).type === 'W3cCredentialRecord' ) { return await agentReq.agent.w3cCredentials.store({ record: credentialRecord as W3cCredentialRecord, }) } + + if ( + credentialRecord instanceof W3cV2CredentialRecord || + (credentialRecord as { type?: string }).type === 'W3cV2CredentialRecord' + ) { + return await agentReq.agent.w3cV2Credentials.store({ + record: credentialRecord as W3cV2CredentialRecord, + }) + } @@ if (credentialType === CredentialType.W3C_VC) { const w3cCredentialService = await agentReq.agent.dependencyManager.resolve(W3cCredentialService) + const w3cV2CredentialService = await agentReq.agent.dependencyManager.resolve(W3cV2CredentialService) @@ try { return await w3cCredentialService.removeCredentialRecord(agentReq.agent.context, credentialId) } catch (error) { - throw error + return await w3cV2CredentialService.removeCredentialRecord(agentReq.agent.context, credentialId) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/controllers/openid4vc/holder/holder.service.ts` around lines 47 - 58, getW3cCredentials currently returns only agentReq.agent.w3cCredentials.getAll() and the w3cV2 path is commented out—re-enable and combine both by fetching agentReq.agent.w3cCredentials.getAll() and agentReq.agent.w3cV2Credentials.getAll() (e.g., Promise.all) and returning the concatenated results; in requestAndStoreCredentials un-comment and implement the W3cV2 branch so that when the credential is a W3cV2CredentialRecord you store it using the W3cV2CredentialRecord handling code (mirror the W3cCredentialRecord branch but use the v2 record/service); in deleteCredential try removing via W3cCredentialService first and if not found/fails, call W3cV2CredentialService.removeCredentialRecord as a fallback so v2 records are deletable (ensure you reference the existing functions/classes: getW3cCredentials, requestAndStoreCredentials, deleteCredential, W3cCredentialRecord, W3cV2CredentialRecord, agentReq.agent.w3cV2Credentials.getAll(), and W3cV2CredentialService.removeCredentialRecord).
🧹 Nitpick comments (1)
src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts (1)
174-197: ⚡ Quick winRemove unreachable else block.
The early return at line 129 ensures that lines 133-199 only execute when
version === 'v2.0'. This makes the conditional at line 174 always true and the else block (lines 186-197) unreachable dead code.♻️ Proposed cleanup
- if (version === 'v2.0') { - const currentCtx = Array.isArray(transformed['`@context`']) - ? transformed['`@context`'] - : typeof transformed['`@context`'] === 'string' - ? [transformed['`@context`']] - : [] - - const ctxSet = new Set(currentCtx) - ctxSet.delete(v1Context) - ctxSet.delete(v2Context) - // W3C V2.0 requires the V2 context to be the very first element. - transformed['`@context`'] = [v2Context, v1Context, ...Array.from(ctxSet)] - } else { - // W3C V1.1 / Default behavior - if (!transformed['`@context`']) { - transformed['`@context`'] = [v1Context] - } else if (Array.isArray(transformed['`@context`'])) { - const ctxSet = new Set(transformed['`@context`']) - ctxSet.delete(v1Context) - transformed['`@context`'] = [v1Context, ...Array.from(ctxSet)] - } else if (typeof transformed['`@context`'] === 'string') { - transformed['`@context`'] = [v1Context, transformed['`@context`']] - } - } + const currentCtx = Array.isArray(transformed['`@context`']) + ? transformed['`@context`'] + : typeof transformed['`@context`'] === 'string' + ? [transformed['`@context`']] + : [] + + const ctxSet = new Set(currentCtx) + ctxSet.delete(v1Context) + ctxSet.delete(v2Context) + // W3C V2.0 requires the V2 context to be the very first element. + transformed['`@context`'] = [v2Context, v1Context, ...Array.from(ctxSet)]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts` around lines 174 - 197, The else branch that handles non-v2 contexts is dead code because the surrounding function returns early unless version === 'v2.0'; remove the unreachable else block and simplify the code to only perform the v2.0 handling for transformed['`@context`'] (using v2Context, v1Context and ctxSet), keeping the existing logic that ensures v2Context is first and v1Context second; also remove any related array/string checks inside the removed branch and ensure no unused variables remain (version, v1Context, v2Context, transformed['`@context`']).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts`:
- Around line 5-7: Reorder and group the related `@credo-ts` imports to satisfy
import-order rules: move the CREDENTIALS_CONTEXT_V1_URL and
CREDENTIALS_CONTEXT_V2_URL import (symbols CREDENTIALS_CONTEXT_V1_URL,
CREDENTIALS_CONTEXT_V2_URL) so it appears before the
OpenId4VcIssuanceSessionRepository import (symbol
OpenId4VcIssuanceSessionRepository) and remove the blank line between them so
both `@credo-ts` imports are adjacent and correctly ordered.
---
Duplicate comments:
In `@src/controllers/openid4vc/holder/holder.service.ts`:
- Around line 47-58: getW3cCredentials currently returns only
agentReq.agent.w3cCredentials.getAll() and the w3cV2 path is commented
out—re-enable and combine both by fetching
agentReq.agent.w3cCredentials.getAll() and
agentReq.agent.w3cV2Credentials.getAll() (e.g., Promise.all) and returning the
concatenated results; in requestAndStoreCredentials un-comment and implement the
W3cV2 branch so that when the credential is a W3cV2CredentialRecord you store it
using the W3cV2CredentialRecord handling code (mirror the W3cCredentialRecord
branch but use the v2 record/service); in deleteCredential try removing via
W3cCredentialService first and if not found/fails, call
W3cV2CredentialService.removeCredentialRecord as a fallback so v2 records are
deletable (ensure you reference the existing functions/classes:
getW3cCredentials, requestAndStoreCredentials, deleteCredential,
W3cCredentialRecord, W3cV2CredentialRecord,
agentReq.agent.w3cV2Credentials.getAll(), and
W3cV2CredentialService.removeCredentialRecord).
In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts`:
- Line 73: The offer is being built with a hardcoded version: 'v1' which ignores
the computed effectiveVersion and breaks v2 negotiations; update the object
passed to the offer creation in issuance-sessions.service.ts to use the computed
effectiveVersion (or options.version if that's what you prefer) instead of the
literal 'v1' so the version field matches the transformed payload (refer to
effectiveVersion and the offer creation call near the existing version: 'v1'
line).
---
Nitpick comments:
In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts`:
- Around line 174-197: The else branch that handles non-v2 contexts is dead code
because the surrounding function returns early unless version === 'v2.0'; remove
the unreachable else block and simplify the code to only perform the v2.0
handling for transformed['`@context`'] (using v2Context, v1Context and ctxSet),
keeping the existing logic that ensures v2Context is first and v1Context second;
also remove any related array/string checks inside the removed branch and ensure
no unused variables remain (version, v1Context, v2Context,
transformed['`@context`']).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f2c9616-ddad-472a-a729-4f264f42ce21
📒 Files selected for processing (4)
src/controllers/openid4vc/holder/holder.service.tssrc/controllers/openid4vc/issuance-sessions/issuance-sessions.service.tssrc/controllers/openid4vc/types/issuer.types.tssrc/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts
Comment on lines
5
to
+7
| import { OpenId4VcIssuanceSessionRepository } from '@credo-ts/openid4vc' | ||
|
|
||
| import { CREDENTIALS_CONTEXT_V1_URL, CREDENTIALS_CONTEXT_V2_URL } from '@credo-ts/core' |
There was a problem hiding this comment.
Fix import order violations.
The pipeline and ESLint are flagging import order issues: the @credo-ts/core import should appear before @credo-ts/openid4vc, and the empty line between related @credo-ts imports should be removed.
📦 Proposed fix
import type { OpenId4VcIssuanceSessionState } from '`@credo-ts/openid4vc`'
+import { CREDENTIALS_CONTEXT_V1_URL, CREDENTIALS_CONTEXT_V2_URL } from '`@credo-ts/core`'
import { OpenId4VcIssuanceSessionRepository } from '`@credo-ts/openid4vc`'
-
-import { CREDENTIALS_CONTEXT_V1_URL, CREDENTIALS_CONTEXT_V2_URL } from '`@credo-ts/core`'
-
import { CredentialFormat, SignerMethod } from '../../../enums/enum'🧰 Tools
🪛 ESLint
[error] 5-5: There should be no empty line within import group
(import/order)
[error] 7-7: @credo-ts/core import should occur before import of @credo-ts/openid4vc
(import/order)
🪛 GitHub Actions: Continuous Integration / Validate
[error] 5-5: ESLint import/order: There should be no empty line within import group
🪛 GitHub Check: Validate
[failure] 7-7:
@credo-ts/core import should occur before import of @credo-ts/openid4vc
[failure] 5-5:
There should be no empty line within import group
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts`
around lines 5 - 7, Reorder and group the related `@credo-ts` imports to satisfy
import-order rules: move the CREDENTIALS_CONTEXT_V1_URL and
CREDENTIALS_CONTEXT_V2_URL import (symbols CREDENTIALS_CONTEXT_V1_URL,
CREDENTIALS_CONTEXT_V2_URL) so it appears before the
OpenId4VcIssuanceSessionRepository import (symbol
OpenId4VcIssuanceSessionRepository) and remove the blank line between them so
both `@credo-ts` imports are adjacent and correctly ordered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit