Add atproto-oauth skill#6
Conversation
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new ChangesAT Protocol OAuth Skill Addition
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
|
||
| ## Verifying against current docs | ||
|
|
||
| Endpoint signatures, scope grammar, and token formats can change. If an MCP client with AT Protocol documentation access is available, use it to check current details rather than relying solely on this skill. The AT Protocol docs MCP server is available at `https://atproto.mcp.kapa.ai` if one isn't already connected. |
There was a problem hiding this comment.
This can be dropped if it's too large a shift from the skills approach, in my harness I include a trailing section to always verify knowledge with an mcp.
|
just a quick thought i think it might overlap quite a bit with the epds login skill as well: https://github.com/hypercerts-org/ePDS/tree/main/.agents/skills/epds-login. But i guess this is also more generic to normal pds oauth so i guess it fine? Do let me know what you think once you look into epds-login and if you think it makes sense to have this here as well then we can merge it |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.agents/skills/atproto-oauth/SKILL.md:
- Around line 56-68: The OAuth skill examples still use transition:generic as if
it were the default scope, which conflicts with the later migration-only
guidance. Update the normative example in SKILL.md and the later inline example
to use the preferred modern scope instead, and keep transition:generic only in a
clearly labeled legacy migration example. Refer to the example JSON snippet and
the later inline example so both occurrences stay consistent.
- Around line 24-46: The fenced blocks in SKILL.md are missing a language tag,
triggering markdownlint MD040 and reducing readability. Update the three
affected fences in the atproto-oauth skill content to use a plain text language
tag, keeping the existing prose and structure unchanged. Locate the code fences
in the numbered workflow section and apply the same fix consistently to all
three instances.
In `@README.md`:
- Around line 31-37: The README’s hosted-skills section is currently empty, so
the newly hosted skill is not discoverable. Update the “Included skills (hosted
here)” section to add a bullet for the hosted skill using its actual name, such
as atproto-oauth, or remove the subsection entirely until it contains real
entries. Keep the change localized to the README section that lists hosted
skills so the skill is visible to readers.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c86a738-9f5c-47ca-82e6-9e823af5caea
📒 Files selected for processing (4)
.agents/skills/atproto-oauth/SKILL.mdCONTRIBUTING.mdREADME.mdskills/hypercerts/references/skill-map.md
- Formatting - Make skill discoverable in readme explicitly
|
|
||
| In the BFF (confidential) pattern, the browser only ever talks to your backend; only the backend talks to the AS and PDS (PAR, token exchange, and all XRPC calls). That separation is what keeps DPoP private keys and access/refresh tokens off the browser entirely — the browser holds nothing but an `HttpOnly` session cookie. | ||
|
|
||
| ## Client Metadata Document |
There was a problem hiding this comment.
Given that this is a Hypercerts skills repo not just a generic atproto one, I think it would be helpful for this section to mention that the ePDS extends this metadata schema, and that for agents building apps which utilise the ePDS, the full reference for that can be found at in the epds-login skill under references/client-metadata.md#all-supported-fields.
There was a problem hiding this comment.
BTW my thinking here is based on the observation that it's not unusual for an agent to fail to load all the correct skills, so having hints and cross-references between skills can help defend against this. E.g. if the agent loads this skill but incorrectly misses epds-login, then mentioning it here ensures that it won't miss knowledge on ePDS-only client metadata if that's relevant to the context.
| ### Discourage transitional scopes | ||
|
|
||
| - `transition:generic`, `transition:chat.bsky`, `transition:email` exist **only** as a migration path off legacy App Passwords — each grants broad, coarse-grained access (`transition:generic` alone allows writing any record type, uploading any blob, and most XRPC calls except account management and `chat.bsky.*`). Requesting one is an implicit admission "I haven't scoped this app's permissions." | ||
| - **Do not default new implementations to `transition:generic`.** Treat it as a deprecated escape hatch, not a starting template — reach for it only when porting an app-password-era client that hasn't been re-scoped yet, and flag that as follow-up work. |
There was a problem hiding this comment.
Could maybe go into a bit more detail here in terms of offering concrete examples of alternatives. For example account:email instead of transition:email.
| - Build the client metadata's `scope` from the granular resources actually needed (`repo:`, `rpc:`, `blob:`, `account:`, `include:`) instead. It's more upfront design work, but it shrinks the blast radius of a stolen token and is what AS consent screens are increasingly optimized to explain to users — a client asking for "post, like, and read your profile" reads very differently from one asking for "everything". | ||
| - If a permission-set lexicon (`include:`) covering the app's use case already exists, prefer it over hand-assembling granular scopes **and** over falling back to `transition:*`. | ||
|
|
||
| ### XRPC audience (`aud`) — why it matters more than the method name |
There was a problem hiding this comment.
As with my comment above about including links to ePDS-flavoured docs, here we can link to docs which explain the relevance of aud when building on top of the CGS:
|
@Kzoeps commented on Jul 1, 2026, 15:23 GMT+1:
Once we have this, we can drop generic PDS info from the ePDS skill and replace it with a link to this. Then the ePDS skill can focus on ePDS stuff. |
Add a more generic atproto-oauth skill that can be used anywhere. Using this skill will prevent the use of outdated knowledge such as relying on transition scopes but also build in institutional knowledge about XRPC/RPC methods (particularly when using app.bsky RPC).
Summary by CodeRabbit