fix: credential controller#359
Merged
GHkrishna merged 7 commits intofeat/oidc-main-syncfrom Apr 3, 2026
Merged
Conversation
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
GHkrishna
added
the
enhancement
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR activates the DIDComm credential exchange controller with full REST endpoint routing, updates Hyperledger library dependency versions with caret ranges, modifies DID key handling from seed-based to private-key-based imports, and patches TypeScript declarations and JSON-LD canonization settings to support ES module exports and safe-mode configuration changes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
src/controllers/did/DidController.ts (1)
535-536: Fix formatting: remove semicolons per project style.ESLint/Prettier flags these lines. The project appears to use a no-semicolon style.
🧹 Proposed formatting fix
- let key; - let publicJwk; + let key + let publicJwk🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/did/DidController.ts` around lines 535 - 536, Remove trailing semicolons from the standalone variable declarations in DidController.ts: replace the lines declaring let key; and let publicJwk; with the project style (no-semicolon) equivalents (i.e., declare the variables without a trailing semicolon) so they conform to ESLint/Prettier rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cliAgent.ts`:
- Line 304: The Prettier errors are due to missing trailing commas on multiline
parameter/argument lists—fix by adding trailing commas where a multi-line
parameter or argument list is broken across lines: add a trailing comma after
the "endpoints: string[]" parameter in the function/type declaration containing
that symbol, and similarly add trailing commas to the multiline
call/array/object arguments at the locations around the other failing symbols
(the call sites near lines 452 and 467). After adding the commas, run your
formatter/linter (e.g., yarn lint or prettier --write) to ensure all multiline
params/args conform to Prettier rules.
In `@src/controllers/did/DidController.ts`:
- Around line 561-572: The code in DidController uses
transformPrivateKeyToPrivateJwk with didOptions.seed (treating seed as a raw
private key), creating a semantic mismatch with the existing DidCreate API; fix
by either (A) changing the call to transformSeedToPrivateJwk and pass
didOptions.seed so seed semantics remain consistent, or (B) rename the
API/variable from seed → privateKey and keep transformPrivateKeyToPrivateJwk;
update usages of didOptions.seed and any related tests or validations
accordingly (see transformPrivateKeyToPrivateJwk, transformSeedToPrivateJwk,
didOptions.seed, and any callers constructing DidCreate).
In `@src/controllers/didcomm/credentials/CredentialController.ts`:
- Around line 230-240: The response currently clears a provided invitation DID
(invitationDid) by returning an empty string (in the object returned from
CredentialController) which loses the actual DID; update the return so
invitationDid is returned when present (i.e., invitationDid:
outOfBandOption?.invitationDid ? invitationDid : invitationDid) or simply
invitationDid: invitationDid, and if you need to mark whether it was supplied vs
generated add a separate boolean property (e.g., invitationDidSupplied:
Boolean(outOfBandOption?.invitationDid)) alongside the existing fields
(references: CredentialController, outOfBandRecord, outOfBandOption,
invitationDid, offerOob).
- Around line 191-199: The code uses routing.endpoints[0] without checking that
routing.endpoints is present and non-empty (in the flow that calls
request.agent.modules.didcomm.mediationRecipient.getRouting and then
createPeerDidDocumentFromServices), which can produce an invalid peer DID
service or a broken invitationUrl; update the logic around the getRouting result
to validate that routing.endpoints is an array with at least one entry and if
not, fail fast (throw or return a clear error) before calling
createPeerDidDocumentFromServices or building the invitationUrl; apply the same
guard to the other occurrence that also indexes endpoints[0] (the second block
around lines 230–233) so both code paths require a configured public DIDComm
endpoint.
- Around line 214-215: Update the DTO type for
CreateOfferOobOptions.protocolVersion from string to the literal union type used
by the service (CredentialProtocolVersionType, e.g. 'v1' | 'v2') so the request
boundary enforces allowed values; then remove the cast in CredentialController
where you call request.agent.modules.didcomm.credentials.createOffer (the
outOfBandOption.protocolVersion cast to CredentialProtocolVersionType) so you
pass the typed DTO value directly and rely on TSOA/OpenAPI validation to reject
unsupported protocolVersion values.
In `@src/routes/routes.ts`:
- Around line 1835-1849: The generated schema allows an empty credentialFormats
object; update the upstream type used to generate
"DidCommCredentialFormatPayload_CredentialFormatType-Array.createProposal_" so
that at least one of the nested props (indy, jsonld, anoncreds) is required (or
add a validator that enforces "at least one of indy|jsonld|anoncreds present")
and then re-run tsoa to regenerate the file; ensure the change is applied to the
ProposeCredentialOptions credentialFormats definition and the equivalent models
noted (the other occurrences around the referenced ranges).
- Around line 1796-1807: The JsonCredential schema currently restricts the
"@context" property to either string[] or JsonObject; update the "@context"
definition in the JsonCredential schema so arrays may contain mixed strings and
objects (i.e., make the array element type a union of string and JsonObject,
matching other W3C credential schemas), then regenerate the DTOs; locate the
"JsonCredential" schema and replace the "@context" entry to allow an array whose
items are "string | JsonObject" (or a union subSchema) instead of only string
array or JsonObject.
- Around line 1929-1945: Change CreateOfferOobOptions.protocolVersion from plain
string to the ProtocolVersion enum so the OOB flow uses the same validated type
as ProposeCredentialOptions and CreateOfferOptions; update the type in the
CreateOfferOobOptions definition (where protocolVersion is declared) to
ProtocolVersion and remove the downstream type cast (the as
CredentialProtocolVersionType<[]>) in the controller so callers cannot pass
arbitrary values and compile-time validation is restored.
---
Nitpick comments:
In `@src/controllers/did/DidController.ts`:
- Around line 535-536: Remove trailing semicolons from the standalone variable
declarations in DidController.ts: replace the lines declaring let key; and let
publicJwk; with the project style (no-semicolon) equivalents (i.e., declare the
variables without a trailing semicolon) so they conform to ESLint/Prettier
rules.
🪄 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: a9effcc9-47da-4dfe-8dd9-f284d28437f2
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
package.jsonpatches/@credo-ts+core+0.6.2+001+fix: change version string type-import esm-export interface.patchpatches/@digitalcredentials+jsonld-signatures+12.0.1.patchsrc/cliAgent.tssrc/controllers/did/DidController.tssrc/controllers/didcomm/credentials/CredentialController.tssrc/controllers/types.tssrc/routes/routes.tssrc/routes/swagger.json
Comment on lines
+1835
to
+1849
| "DidCommCredentialFormatPayload_CredentialFormatType-Array.createProposal_": { | ||
| "dataType": "refAlias", | ||
| "type": {"dataType":"nestedObjectLiteral","nestedProperties":{"indy":{"ref":"LegacyIndyDidCommProposeCredentialFormat"},"jsonld":{"ref":"DidCommJsonLdCredentialDetailFormat"},"anoncreds":{"ref":"AnonCredsDidCommProposeCredentialFormat"}},"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 | ||
| "DidCommAutoAcceptCredential": { | ||
| "dataType": "refEnum", | ||
| "enums": ["always","contentApproved","never"], | ||
| }, | ||
| // 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 | ||
| "ProposeCredentialOptions": { | ||
| "dataType": "refObject", | ||
| "properties": { | ||
| "protocolVersion": {"ref":"ProtocolVersion","required":true}, | ||
| "credentialFormats": {"ref":"DidCommCredentialFormatPayload_CredentialFormatType-Array.createProposal_","required":true}, |
There was a problem hiding this comment.
Require at least one credential format.
These payload models make every format branch optional, so { "credentialFormats": {} } satisfies the generated schema for proposeCredential, createOffer, and the OOB offer path. That request has no credential payload and will only fail deeper in the stack. Please enforce at least one of indy, jsonld, or anoncreds in the upstream type and regenerate.
Also applies to: 1905-1916, 1924-1927
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/routes.ts` around lines 1835 - 1849, The generated schema allows
an empty credentialFormats object; update the upstream type used to generate
"DidCommCredentialFormatPayload_CredentialFormatType-Array.createProposal_" so
that at least one of the nested props (indy, jsonld, anoncreds) is required (or
add a validator that enforces "at least one of indy|jsonld|anoncreds present")
and then re-run tsoa to regenerate the file; ensure the change is applied to the
ProposeCredentialOptions credentialFormats definition and the equivalent models
noted (the other occurrences around the referenced ranges).
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
|
Merged
Merged
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.
What:
Breaking changes
/accept-proposalbody key change fromcredentialRecordIdtocredentialExchangeRecordId/accept-credentialbody key change fromcredentialRecordIdtocredentialExchangeRecordId/accept-offerbody key change fromcredentialRecordIdtocredentialExchangeRecordId/accept-requestbody key change fromcredentialRecordIdtocredentialExchangeRecordIdSummary by CodeRabbit
New Features
Bug Fixes
Chores