[HYPER-502] add app.certified.actor.createdVia lexicon#228
Conversation
🦋 Changeset detectedLatest commit: bb4a721 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 |
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new Changesapp.certified.actor.createdVia lexicon
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (6 passed)
✨ 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 |
Design debate: should
|
Add a lightweight account-provenance record carrying the OAuth client_id URL of the application that created a Certified account. Lets consumers distinguish records originating from test/staging environments from real ones without requiring cryptographic platform attestations. - Required `clientId` (uri) and `createdAt` (datetime); key literal:self - Optional `signatures` array, consistent with other Certified records - Validation tests, ERD entry, regenerated SCHEMAS.md, changeset (minor) The property is named `clientId` (camelCase) to satisfy the repo style checker; its description and the changeset note it holds the OAuth `client_id` URL. Refs HYPER-502. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
74da239 to
52dfaae
Compare
i think we could leave client_id optional too. since this pattern is already allowed on app.bsky.actor.profile and app.certified.actor.profile (app.bsky.actor.profile does not even require the createdAAt) granted just because they do it isnt a good answer. but i think it would be better dx if both are optional since one of n is not possible |
I think making Also, why is the |
|
Lets make it optional! Talked with Sharfy as well. @aspiers |
Per PR #228 review consensus (holkexyz, Kzoeps, s-adamantine, aspiers), clientId becomes optional. A signature already establishes origin via the signing service DID, so a signature-only record is valid provenance. ATProto lexicons cannot express "at least one of clientId/signatures", so no cross-field constraint is added at the schema layer. createdAt remains required, consistent with all other Certified record lexicons (the lone exception being signature/proof). Update tests, changeset, and SCHEMAS.md to reflect optional clientId. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Made @s-adamantine on why
So the full URL is the actual OAuth |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ERD.puml`:
- Around line 60-64: `ERD.puml` is missing a new non-facet field from the
`createdVia` dataclass, so the diagram is out of sync with the schema. Update
the `createdVia` block in `ERD.puml` to include `signatures` alongside
`clientId` and `createdAt`, keeping the field list aligned with the new lexicon
and `SCHEMAS.md` while still excluding any facet fields.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 78cac150-11fe-46e6-a1c9-a1734e37b0c4
📒 Files selected for processing (5)
.changeset/add-actor-created-via.mdERD.pumlSCHEMAS.mdlexicons/app/certified/actor/createdVia.jsontests/validate-actor-created-via.test.ts
|
Changing back to draft in case we retire this in favour of using badges. |
Summary
Adds a new lexicon
app.certified.actor.createdVia— a lightweight account-provenance record identifying the application that created a Certified account via its OAuthclient_idURL.Refs HYPER-502.
Why
Lets consumers distinguish records originating from test/staging environments from real ones, without requiring full cryptographic platform attestations (a lighter-weight alternative to HYPER-181). The
signaturesarray still allows the provenance marker itself to be attested when a stronger guarantee is wanted.Schema
key:literal:self— onecreatedViarecord per repo, mirroringapp.certified.actor.profile/.organization.clientId(required,uri) — the OAuthclient_idURL (client metadata document URL) uniquely identifying the creating app. NamedclientId(camelCase) to satisfy the repo'sproperty-name-camelcasestyle rule; the description records that it holds the OAuthclient_idvalue.createdAt(required,datetime).signatures(optional) —app.certified.signature.defs#list, consistent with every other Certified record.Changeset
minor(non-breaking additive lexicon).Generated / regenerated
generated/— vianpm run gen-api(gitignored, not in diff).SCHEMAS.md— vianpm run gen-schemas-md.ERD.puml— added acreatedViadataclass.Open design question (draft)
Whether
clientIdshould be required or optional. A signature signed by the client's service DID (did:web) can itself identify the origin, makingclientIdpartly redundant; both-present enables a domain sanity check between theclient_idURL and thedid:webdomain. See the discussion comment below. Pending a decision on the minimum-validity constraint.Test plan
npm run check— validate + typecheck + build, 188 tests passnode scripts/check-lexicon-style.js— 0 errorstests/validate-actor-created-via.test.ts🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests