Skip to content

sdk: cache DID resolution and accept an app resolver - #19

Open
Kzoeps wants to merge 5 commits into
mainfrom
sdk/did-resolver-cache
Open

Kzoeps wants to merge 5 commits into
mainfrom
sdk/did-resolver-cache

Conversation

@Kzoeps

@Kzoeps Kzoeps commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Problem

The SDK looked up a DID document on every call with a fresh, uncached resolver, so each operation hit plc.directory again. Apps that already have a resolver (for example oauthClient.identityResolver, which caches) had no way to hand it to the SDK.

Fix

  • createSDKContext({ client, didResolver? }): new optional didResolver, typed as DidDocumentResolver. Accepts an @atproto-labs/did-resolver resolver or oauthClient.identityResolver.
  • Without it, each context builds one cached resolver (in-memory cache, concurrent lookups share a request, failures not cached). It reads global fetch at request time, so tests that stub fetch still work.
  • Error messages are unchanged. The per-call PDS client sharing from sdk/collection: add createCollection #13 is kept as is.
  • Skill docs show passing oauthClient.identityResolver; minor changeset.

createSDKContext takes an optional didResolver (a DidResolver or oauthClient.identityResolver). Without one, each context gets its own cached resolver instead of a new uncached one per lookup. Adds @atproto-labs/identity-resolver as a dev dependency to type-test the oauth shape.
@coderabbitai

coderabbitai Bot commented Nov 11, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 39 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2d8815a9-27fc-4907-a7a2-05589b45dba5
Commits

Reviewing files that changed from the base of the PR and between 3ca68ab and 369aa14.

Files ignored due to path filters: 1

  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
Files selected for processing: 11

  • .agents/skills/hypercerts-org-sdk/SKILL.md
  • .agents/skills/hypercerts-org-sdk/references/collection.md
  • .changeset/did-resolver-cache.md
  • docs/e2e-testing.md
  • packages/sdk/package.json
  • packages/sdk/src/context.test.ts
  • packages/sdk/src/context.ts
  • packages/sdk/src/index.ts
  • packages/sdk/src/internal/resolve-pds.ts
  • packages/sdk/src/internal/resolve-record-ref.test.ts
  • packages/sdk/src/internal/resolve-record-ref.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@changeset-bot

changeset-bot Bot commented Nov 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 369aa14

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@hypercerts-org/sdk Minor

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

@sonarqubecloud

Copy link
Copy Markdown

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

@socket-security

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​atproto-labs/​identity-resolver@​0.4.131001006598100

View full report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant