fead(did web): Use Credo's core DID document construction methods - #593
Open
rmlearney-digicatapult wants to merge 7 commits into
Open
rmlearney-digicatapult wants to merge 7 commits into
rmlearney-digicatapult wants to merge 7 commits into
Conversation
Signed-off-by: rmlearney-digicatapult <robert.learney@digicatapult.org.uk>
Signed-off-by: rmlearney-digicatapult <robert.learney@digicatapult.org.uk>
There was a problem hiding this comment.
🟡 Changes recommended
The new builder path omits document validation, allowing malformed service endpoints.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Replaces custom DID:web document construction with Credo-native parsing, key, service, and builder APIs.
Changes:
- Builds DID documents using Credo helpers.
- Adds DID method validation and regression coverage.
File summaries
| File | Description |
|---|---|
| tests/unit/didWebGenerator.test.ts | Tests rejection of non-web DIDs. |
| src/utils/didWebGenerator.ts | Refactors DID:web generation to Credo APIs. |
Review details
Suppressed comments (1)
tests/unit/didWebGenerator.test.ts:77
- The added test passes a syntactically valid
did:key, so it only covers the wrong-method branch. Add a malformed input case to protect the newparseDiderror wrapping and itsInvalid DID identifiermessage.
await didWebDocGenerator.generateDidWebDocument('did:key:z6Mkexample', 'http://localhost%3A5002')
- Files reviewed: 3/4 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: rmlearney-digicatapult <robert.learney@digicatapult.org.uk>
jonmattgray
reviewed
Sep 21, 2026
|
|
||
| try { | ||
| await didWebDocGenerator.generateDidWebDocument('did:key:z6Mkexample', 'http://localhost%3A5002') | ||
| } catch (error) { |
Contributor
There was a problem hiding this comment.
[MINOR] There's no test for the new parsedDid.did !== didId guard — e.g. a did:web identifier with an extra path or query segment. Worth adding alongside the two new tests here since it's the other half of the validation this PR introduces.
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
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.
Pull Request
Checklist
PR Type
Please delete options that are irrelevant.
Linked tickets
High level description
Replace hand-written document construction and validation with native Credo helpers
Detailed description
Credo exports multiple helper methods and validators for DID Document construction.
Swapped existing validation, key construction, document assembly using
JsonTransformerandTypedArrayEncoderetc. for these methods.Added simple
did:webinput parsing regression test.Describe alternatives you've considered
Operational impact
Additional context