Fix the build after DIDDocument's optional relaxation - #59
Merged
Conversation
#58 made `verificationMethod` and `publicKeyMultibase` optional to match the canonical DID schema; #54 added RepoSigningKey, which unwraps both. Each PR was green on its own base and they merge cleanly as text, so main landed broken — `swift build` fails at RepoSigningKey.swift:53. Both absent cases now refuse with noAtprotoSigningKey, the same answer an empty method list already gave: a document that publishes no key material for this DID proves nothing about its repo, so the only sound response is to refuse. Adds a regression test for each, since these branches are reachable only from hand-written JSON and would otherwise be settled by whoever next touches the unwrap. The multibase case is mutation-verified.
🦋 Changeset detectedLatest commit: 8adbeca The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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
1 participant
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.
main is currently red
swift buildfails onmain(8a0248f):#58 made
verificationMethodandpublicKeyMultibaseoptional to match the canonical DID schema. #54 addedRepoSigningKey, which unwraps both. Each was green on its own base, and the two merge cleanly as text — nothing conflicts — so the breakage only appears once they're compiled together. #54's CI last ran against a base predating #58.The fix
Both absent cases refuse with
noAtprotoSigningKey— the same answer an emptyverificationMethodlist already gave. A document that publishes no key material for this DID proves nothing about its repo, so refusing is the only sound reading; treating absent as "skip the check" would let a document with no key silently pass verification.controllerstayed non-optional, so the existing empty-controller handling is untouched.Tests
Adds one regression test per branch. These are reachable only from hand-written JSON (the fixture builder always emits both fields), so without them the nil behaviour is decided by whoever next touches the unwrap rather than by a test.
The
publicKeyMultibasecase is mutation-verified — swapping the thrown error to.badMultibaseKeymakes the test fail withexpected error ".noAtprotoSigningKey" ... but ".badMultibaseKey" ... was thrown instead, confirming it exercises the new branch rather than passing incidentally.swift buildcleanswift test: 83 verify tests in 10 suites + 56 core in 14, all passing🤖 Generated with Claude Code