Manage OAuth Redirects from the CLI (#2603) [bump to v1.0.20]#2603
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds CLI subcommands to manage authorized redirect origins (list, add, delete), HTTP client helpers for adding/removing origins, comprehensive CLI tests, and updates server-side authorization for creating origins to require :apps/write scope. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (auth origin add)
participant HTTP as CLI HTTP helper
participant Server as Server API
participant DB as Database
CLI->>HTTP: addAuthorizedOrigin({service, params})
HTTP->>Server: POST /apps/:appId/authorized-redirect-origins {service, params} (auth token)
Server->>Server: authorize via req->app-accepting-superadmin-or-ref-token! (:apps/write)
Server->>DB: persist authorized redirect origin
DB-->>Server: persisted origin with id
Server-->>HTTP: 200 { origin }
HTTP-->>CLI: decoded AuthorizedOriginResponse
CLI-->>CLI: log confirmation (display, type, id)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
|
View Vercel preview at instant-www-js-drewh-oauth-redirect-manage-jsv.vercel.app. |
drew-harris
force-pushed
the
drewh/cli-links
branch
from
a205053 to
9836a1d
Compare
drew-harris
force-pushed
the
drewh/oauth-redirect-manage
branch
from
1fe5f55 to
bf0db74
Compare
drew-harris
force-pushed
the
drewh/oauth-redirect-manage
branch
from
bf0db74 to
4e7707a
Compare
drew-harris
force-pushed
the
drewh/oauth-redirect-manage
branch
from
a3a51d3 to
88f853e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
client/packages/cli/src/commands/auth/origin/delete.ts (1)
22-49: Optional: collapse the twoifblocks intoif/elseand consider mirroringadd.ts'sBadArgsErrorhandling.The two
if (!opts.id)andif (opts.id)branches are mutually exclusive but currently duplicate theremoveAuthorizedOrigincall and rely on fall-through; anif/elsemakes the intent obvious and is friendlier to future edits.Separately,
add.tswraps its body withEffect.catchTag('BadArgsError', ...)to log the error message plus a--helphint. Here, theMust specify --iderror escapes as a runPromise rejection (the test relies on it viarejects.toThrow). If you want consistent UX betweenauth origin addandauth origin deletefailures, consider applying the samecatchTag(and updating the test to assert on logged output instead of a thrown rejection).♻️ Proposed if/else refactor (drop-in)
- if (!opts.id) { - const { yes } = yield* GlobalOpts; - if (yes) { - return yield* BadArgsError.make({ - message: 'Must specify --id', - }); - } - - const picked = yield* runUIEffect( - new UI.Select({ - options: origins.map((origin) => ({ - label: - `${originSource(origin)} — ${originDisplay(origin)} ` + - chalk.dim(`(${origin.id})`), - value: origin, - })), - promptText: 'Select an origin to delete:', - }), - ); - - yield* removeAuthorizedOrigin(picked.id); - } - - if (opts.id) { - yield* removeAuthorizedOrigin(opts.id); - } + let targetId: string; + if (opts.id) { + targetId = opts.id; + } else { + const { yes } = yield* GlobalOpts; + if (yes) { + return yield* BadArgsError.make({ message: 'Must specify --id' }); + } + const picked = yield* runUIEffect( + new UI.Select({ + options: origins.map((origin) => ({ + label: + `${originSource(origin)} — ${originDisplay(origin)} ` + + chalk.dim(`(${origin.id})`), + value: origin, + })), + promptText: 'Select an origin to delete:', + }), + ); + targetId = picked.id; + } + + yield* removeAuthorizedOrigin(targetId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/cli/src/commands/auth/origin/delete.ts` around lines 22 - 49, Collapse the two mutually exclusive branches into a single if/else: if (!opts.id) run the GlobalOpts/interactive picker (runUIEffect with UI.Select) and call removeAuthorizedOrigin(picked.id) in the if branch, else call removeAuthorizedOrigin(opts.id) in the else branch; keep the existing BadArgsError.make usage when yes is set. Then wrap the command's Effect pipeline with Effect.catchTag('BadArgsError', ...) mirroring auth origin add: catch the BadArgsError and log its message plus a short hint to run --help (so failures are logged instead of bubbling as a rejected promise). Ensure you reference the same symbols: removeAuthorizedOrigin, GlobalOpts, runUIEffect/UI.Select, BadArgsError, and Effect.catchTag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@client/packages/cli/src/commands/auth/origin/delete.ts`:
- Around line 22-49: Collapse the two mutually exclusive branches into a single
if/else: if (!opts.id) run the GlobalOpts/interactive picker (runUIEffect with
UI.Select) and call removeAuthorizedOrigin(picked.id) in the if branch, else
call removeAuthorizedOrigin(opts.id) in the else branch; keep the existing
BadArgsError.make usage when yes is set. Then wrap the command's Effect pipeline
with Effect.catchTag('BadArgsError', ...) mirroring auth origin add: catch the
BadArgsError and log its message plus a short hint to run --help (so failures
are logged instead of bubbling as a rejected promise). Ensure you reference the
same symbols: removeAuthorizedOrigin, GlobalOpts, runUIEffect/UI.Select,
BadArgsError, and Effect.catchTag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4714d53e-9dc6-4cb9-8922-5f8fb480d949
📒 Files selected for processing (7)
client/packages/cli/__tests__/authOriginManagement.test.tsclient/packages/cli/src/commands/auth/origin/add.tsclient/packages/cli/src/commands/auth/origin/delete.tsclient/packages/cli/src/commands/auth/origin/list.tsclient/packages/cli/src/index.tsclient/packages/cli/src/lib/oauth.tsserver/src/instant/dash/routes.clj
drew-harris
changed the title
This was referenced May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds commands for managing Oauth redirects in the CLI.
Uses the same "add" "list" "delete" structure as how to manage clients.
Tests were written by AI to match the same pattern as the auth client tests