Skip to content

fix(ack-id): require DID URIs in controllerClaimSchema - #210

Closed
kutluhaneth46 wants to merge 1 commit into
agentcommercekit:mainfrom
kutluhaneth46:cursor/fix-controller-claim-nonempty-88c1
Closed

kutluhaneth46 wants to merge 1 commit into
agentcommercekit:mainfrom
kutluhaneth46:cursor/fix-controller-claim-nonempty-88c1

Conversation

@kutluhaneth46

Copy link
Copy Markdown
Contributor

Summary

  • Require controllerClaimSchema id and controller to be DID URIs in both Valibot and Zod.
  • Rejects empty strings and non-DID values at the schema boundary, matching how createControllerCredential types these fields.
  • Add schema parity tests.

Testing

  • pnpm --filter @agentcommercekit/ack-id exec vitest run src/schemas/schemas.test.ts src/controller-claim-verifier.test.ts

AI usage disclosure

Assisted by Cursor. I reviewed the controller claim verifier and credential creator types, implemented the dual-schema change, and ran the focused tests myself.

Fixes #207

Validate controller credential subject id and controller as DID URIs in
both Valibot and Zod so empty strings and non-DID values fail at the
schema boundary.

Fixes agentcommercekit#207
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 1e5359c9-51a5-4aa9-9b57-3c12e6f2c356

📥 Commits

Reviewing files that changed from the base of the PR and between 7d23f83 and a95bab2.

📒 Files selected for processing (4)
  • .changeset/ack-id-controller-claim-did-uri.md
  • packages/ack-id/src/schemas/schemas.test.ts
  • packages/ack-id/src/schemas/valibot.ts
  • packages/ack-id/src/schemas/zod.ts

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

Comment @coderabbitai help to get the list of available commands.

@kutluhaneth46

Copy link
Copy Markdown
Contributor Author

@venables — controllerClaimSchema requires DID URIs. CI green + changeset. Ready for review.

@kutluhaneth46

Copy link
Copy Markdown
Contributor Author

Gentle bump — CI green / mergeable. Requires DID URIs in controllerClaimSchema. Ready for review when you have bandwidth.

@kutluhaneth46

Copy link
Copy Markdown
Contributor Author

Gentle bump — still open and green on my side. Happy to rebase or adjust if anything is blocking review/merge.

@kutluhaneth46

Copy link
Copy Markdown
Contributor Author

Friendly nudge — still open and mergeable. Happy to rebase or address review feedback anytime.

@kutluhaneth46

Copy link
Copy Markdown
Contributor Author

Thanks for the time and for keeping the repo open to external contributions — much appreciated.

Closing this PR to tidy up the queue: it's been open for a while with no maintainer review/feedback yet, and I don't want to leave stale noise around.

Happy to reopen or submit a refreshed PR if useful. Thanks again!

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.

bug(ack-id): controllerClaimSchema accepts empty id and controller strings

1 participant