Skip to content

Add atproto-oauth skill#6

Open
Ashex wants to merge 3 commits into
mainfrom
task/atproto-oauth-skill
Open

Add atproto-oauth skill#6
Ashex wants to merge 3 commits into
mainfrom
task/atproto-oauth-skill

Conversation

@Ashex

@Ashex Ashex commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

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

  • Documentation
    • Added a comprehensive guide for AT Protocol OAuth 2.1 flows, covering required security behaviors, state/session handling, DPoP details, scope rules, and troubleshooting.
    • Updated README to clarify which skills are reusable here versus app-focused pointer skills elsewhere, and adjusted design expectations.
    • Updated contributing instructions for adding new skills and maintaining catalog pointers.
  • New Features
    • Documented a new AT Protocol OAuth-focused skill in the skill map, including the install command and a fallback reference.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Ashex, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ab73c39a-eb0a-4af9-af53-72efb184b31d

📥 Commits

Reviewing files that changed from the base of the PR and between a249586 and 6a4a7bb.

📒 Files selected for processing (3)
  • .agents/skills/atproto-oauth/SKILL.md
  • README.md
  • skills/hypercerts/references/skill-map.md
📝 Walkthrough

Walkthrough

Adds a new atproto-oauth skill guide for AT Protocol OAuth 2.1 and updates repository docs and the skill map to distinguish hosted skills from pointer-based entries.

Changes

AT Protocol OAuth Skill Addition

Layer / File(s) Summary
atproto-oauth SKILL.md content
.agents/skills/atproto-oauth/SKILL.md
New skill document covering OAuth 2.1 requirements, flow, client metadata, DPoP, scopes, identity verification, session storage, security, troubleshooting, library guidance, and implementation notes.
Catalog documentation and skill-map wiring
CONTRIBUTING.md, README.md, skills/hypercerts/references/skill-map.md
CONTRIBUTING.md and README.md now distinguish hosted skills from external pointers, and skill-map.md adds the new atproto-oauth catalog entry.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • hypercerts-org/skills#1: Updates the same skills/hypercerts/references/skill-map.md catalog path for skill routing.
  • hypercerts-org/skills#5: Touches the same AT Protocol skill-selection area by refining how focused skills are surfaced.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding the atproto-oauth skill.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task/atproto-oauth-skill

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.


## 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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Kzoeps

Kzoeps commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 147cbb2 and 3511e1f.

📒 Files selected for processing (4)
  • .agents/skills/atproto-oauth/SKILL.md
  • CONTRIBUTING.md
  • README.md
  • skills/hypercerts/references/skill-map.md

Comment thread .agents/skills/atproto-oauth/SKILL.md
Comment thread .agents/skills/atproto-oauth/SKILL.md
Comment thread README.md
Ashex added 2 commits July 7, 2026 16:31
- 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

@aspiers

aspiers commented Jul 11, 2026

Copy link
Copy Markdown

@Kzoeps commented on Jul 1, 2026, 15:23 GMT+1:

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

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.

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.

3 participants