fix(registry-cli): granular OAuth scopes, error reporting, exit hang#929
fix(registry-cli): granular OAuth scopes, error reporting, exit hang#929
Conversation
…n/logout exit hang - Switch login from `transition:generic` to granular scopes derived from `@emdash-cms/registry-lexicons`: `repo:` for every record-shaped lexicon and `rpc:?aud=*` for every aggregator query. Auto-fall-back to `transition:generic` if the AS returns `invalid_scope`. - Surface OAuth response failures with HTTP status, endpoint, error code/description, and a body snippet when the response wasn't OAuth-shaped JSON, instead of bubbling up a bare `unknown_error` and stack trace. - Resolve the post-login handle from the DID document via `LocalActorResolver` instead of `com.atproto.server.getSession`, which avoids PDS-side DPoP/Bearer compatibility quirks and lets us drop the corresponding `rpc:` scope. - Force-exit on success at the top level. After login/logout, run() returns but a ref'd handle in the OAuth path keeps the event loop alive indefinitely; this is a workaround pending root-cause. - Expose `RECORD_NSIDS` / `QUERY_NSIDS` from `@emdash-cms/registry-lexicons` so consumers can derive scope/permission lists from the lexicon set instead of hand-rolling them.
🦋 Changeset detectedLatest commit: e934ee4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-perf-coordinator | e934ee4 | May 07 2026, 09:55 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-i18n | e934ee4 | May 07 2026, 09:55 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | e934ee4 | May 07 2026, 09:56 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | e934ee4 | May 07 2026, 09:56 AM |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | e934ee4 | May 07 2026, 09:56 AM |
There was a problem hiding this comment.
Pull request overview
This PR improves the @emdash-cms/registry-cli OAuth login/logout flow and scope handling, and adds lexicon-derived NSID lists in @emdash-cms/registry-lexicons so scope/permission lists can be generated from the authoritative lexicon set.
Changes:
- Add
RECORD_NSIDS/QUERY_NSIDSexports to@emdash-cms/registry-lexiconsfor consumers derivingrepo:/rpc:scopes from lexicons. - Update registry CLI OAuth to request granular scopes by default, retrying once with
transition:genericoninvalid_scope, and improve OAuth error reporting inlogin. - Move handle resolution to DID-document-based resolution (via
LocalActorResolver) and force-exit the CLI after successful completion to avoid post-login/logout hangs.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/registry-lexicons/src/index.ts | Exports RECORD_NSIDS and QUERY_NSIDS lists derived from the lexicon NSIDs. |
| packages/registry-cli/src/profile.ts | Switches handle resolution to DID document resolution via LocalActorResolver. |
| packages/registry-cli/src/oauth.ts | Builds granular default OAuth scopes from lexicon NSID lists; adds invalid_scope fallback flow and factors out an actor resolver factory. |
| packages/registry-cli/src/index.ts | Forces process.exit(0) after runMain resolves to work around an event-loop hang. |
| packages/registry-cli/src/commands/login.ts | Adds structured OAuth response error reporting and messaging for legacy-scope fallback. |
| .changeset/registry-lexicons-nsid-lists.md | Changeset for new lexicon NSID list exports. |
| .changeset/registry-cli-login-error.md | Changeset for improved OAuth error reporting. |
| .changeset/registry-cli-granular-scopes.md | Changeset for granular scopes + handle resolution change. |
| .changeset/registry-cli-exit-hang.md | Changeset for the forced-exit hang workaround. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * the atproto OAuth permission spec, scoped to the two record collections | ||
| * the CLI publishes plus an `rpc:com.atproto.server.getSession` for handle | ||
| * resolution. |
| * NSIDs of record-shaped lexicons in this package (one row per NSID in the | ||
| * publisher's repo). Embedded objects (`releaseExtension`) and shared defs | ||
| * (`aggregator.defs`) are excluded — they don't address their own collection. | ||
| * | ||
| * Useful for consumers building OAuth `repo:` scopes or enumerating writable | ||
| * collections without hand-rolling a list that drifts from the lexicons. | ||
| */ |
|
/review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
| * the atproto OAuth permission spec, scoped to the two record collections | ||
| * the CLI publishes plus an `rpc:com.atproto.server.getSession` for handle | ||
| * resolution. | ||
| */ |
There was a problem hiding this comment.
Low: stale JSDoc. This comment now describes neither the actual default scope nor the current handle-resolution path:
- "the two record collections the CLI publishes" —
DEFAULT_CLI_SCOPEderives fromRECORD_NSIDS, which has four entries (package profile, package release, publisher profile, publisher verification). - "rpc:com.atproto.server.getSession for handle resolution" —
getSessionwas removed in this PR (the whole point of switching toLocalActorResolver). The actualrpc:scopes in the default cover the five aggregator queries fromQUERY_NSIDS.
Suggest something like: "Defaults to DEFAULT_CLI_SCOPE — atproto plus repo:<nsid> for every record-shaped lexicon in @emdash-cms/registry-lexicons and rpc:<nsid>?aud=* for every aggregator query." Same comment was raised by Copilot.
| await reportOAuthFailure(error); | ||
| process.exit(1); | ||
| } | ||
| throw error; |
There was a problem hiding this comment.
Low: OAuthCallbackError falls through to a stack trace. The whole motivation for the new error reporting is that users were seeing bare unknown_error + stack traces and couldn't tell a transient PDS issue from a config issue. This catch handles OAuthResponseError (the PAR/token-endpoint path) but @atcute/oauth-node-client throws OAuthCallbackError for the other half of the surface — user denied consent (error=access_denied), state mismatch, expired code, issuer mismatch, etc. Those still surface as a citty stack trace, which is the same UX bug just in a different code path.
Not blocking — the cirrus symptom that prompted this PR was a PAR failure, so this catch fixes the immediate report. Worth a follow-up to extend reportOAuthFailure (or a sibling) to render OAuthCallbackError cleanly too: at minimum render error.error and error.errorDescription without the stack.
|
Review posted successfully with two line-anchored comments at #929 (review). Findings summary: The PR is well-scoped and well-justified — four cleanly separated fixes with matching changesets and clear forensic notes for the unidentified handle leak. Verified all the substantive claims:
Two low-severity comments posted:
|
Fix stale JSDoc on scope option, remove releaseExtension from RegistryRecords (embedded object, not a standalone collection), and catch OAuthCallbackError for clean error reporting.
What does this PR do?
Four related improvements to the
@emdash-cms/registry-clilogin flow, surfaced while logging in tomk.gg:Granular OAuth scopes.
transition:genericis replaced with the narrowest scope set derived from@emdash-cms/registry-lexicons:atprotorepo:<nsid>for every record-shaped lexicon (package profile, package release, publisher profile, publisher verification)rpc:<nsid>?aud=*for every aggregator query (the publish CLI doesn't call them yet, but granting them at login means future tooling that resumes the stored session can call the aggregator without forcing a re-login).If the AS rejects the granular request with
invalid_scope, login automatically retries once withtransition:genericand prints a notice — keeps publishers on un-upgraded PDSes unblocked.OAuth error reporting.
OAuthResponseErroris caught in the login command and rendered with HTTP status, endpoint, OAuth error code/description, and a body snippet when the AS response wasn't OAuth-shaped JSON. The original symptom was a bareunknown_errorplus a stack trace whenever the PDS hiccuped at PAR — users had no way to tell "transient PDS issue" from "config problem."Display name / handle resolution. Post-login handle resolution moved off
com.atproto.server.getSession(which 401s on cirrus PDSes that only acceptAuthorization: Bearer ..., since atcute sendsAuthorization: DPoP ...for OAuth sessions) ontoLocalActorResolver.resolve(did). The handle is read from the DID document'salsoKnownAsand round-tripped through DNS/well-known to verify it points back. No PDS XRPC, no DPoP/Bearer compatibility, norpc:com.atproto.*scope needed.Login/logout exit hang. After a successful
loginorlogout,run()was returning correctly but the CLI hung indefinitely. Root cause is a ref'd handle somewhere in the OAuth path (atcute, undici, or the loopback server) that prevents Node's event loop from draining. Workaround: force-exit at the top level oncerunMainresolves. The underlying handle leak is unidentified — flagged in the changeset and the source comment.Bonus:
@emdash-cms/registry-lexiconsnow exportsRECORD_NSIDSandQUERY_NSIDSso other consumers can derive permission/scope lists from the lexicon set instead of hand-rolling one that drifts.Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been run@emdash-cms/registry-cli, one minor on@emdash-cms/registry-lexiconsfor the new exports)AI-generated code disclosure
Screenshots / test output
node packages/registry-cli/dist/index.mjs login mk.ggbefore this PR (the actual symptom that prompted the work):After this PR: handle resolves to
mk.gg(via DID doc, no PDS XRPC), and the prompt returns immediately afterPDS:prints.