Add Apple OAuth setup from instant-cli#2566
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Apple as a supported provider to the CLI "auth client add" command: new Apple handler, private-key file reading/validation, interactive prompts and web-flow handling, CLI help updates, platform Apple constants, dashboard endpoint wiring, and new comprehensive Vitest tests. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(30,144,255,0.5)
participant CLI
end
rect rgba(34,139,34,0.5)
participant FS
end
rect rgba(255,165,0,0.5)
participant OAuthService
end
rect rgba(128,0,128,0.5)
participant Prompt
end
CLI->>Prompt: request missing flags / confirm web flow
Prompt-->>CLI: user responses (or --yes skip)
CLI->>FS: read private-key file (if web flow)
FS-->>CLI: file contents or error
CLI->>OAuthService: create OAuth client (name, servicesId, meta(team/key), clientSecret?, redirectTo?)
OAuthService-->>CLI: created client metadata
CLI-->>Prompt: display boxed summary & redirect/test instructions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
|
View Vercel preview at instant-www-js-drewh-apple-jsv.vercel.app. |
drew-harris
force-pushed
the
drewh/apple
branch
from
3cd7774 to
2d076a7
Compare
drew-harris
force-pushed
the
drewh/apple
branch
from
2d076a7 to
5bfd6a7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
client/packages/cli/src/commands/auth/client/add.ts (2)
556-566: Minor:metacarriesundefinedfields in the native-only flow.When
skipWebis true,teamIdandkeyIdare bothundefined, someta: { teamId: undefined, keyId: undefined }is sent.HttpBody.unsafeJsonwill drop these keys during serialization, so it's functionally harmless, but consider making it explicit so the wire payload and future readers don't have to reason about it:♻️ Proposed tweak
- meta: { teamId, keyId }, + meta: privateKey ? { teamId, keyId } : undefined,Note: this would require updating the native-success test at
authClientAddApple.test.ts:222-225fromtoEqual({ teamId: undefined, keyId: undefined })totoBeUndefined().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/cli/src/commands/auth/client/add.ts` around lines 556 - 566, The meta object sent to addOAuthClient currently includes teamId and keyId even when skipWeb=true (so both are undefined); change the call in the function that builds the addOAuthClient payload so meta is only included when at least one of teamId or keyId is defined (e.g., compute a local meta variable and pass meta: metaOnlyIfPresent), or set meta to undefined when both are undefined, and update the native-success test (authClientAddApple.test.ts) to expect undefined (toBeUndefined()) instead of { teamId: undefined, keyId: undefined } to match the new wire payload behavior.
446-468:--configure-webis a phantom flag.
optOrPromptBooleanis called withundefinedas the first argument rather thanopts['configure-web'], so the CLI flag cannot actually be supplied — it only appears in error/help wording viasimpleName. It's also not listed in the Apple help text inclient/packages/cli/src/index.ts(lines 112-117). Either:
- wire the value through (
opts['configure-web']) and document it, or- drop
simpleName: '--configure-web'to avoid surfacing a non-existent flag in any error output.A small precedence gotcha worth noting as well: if a user passes, say, only
--custom-redirect-uriwithout any ofteam-id/key-id/private-key-file,anyWebFlagProvidedisfalse, so the web flow is considered off and--custom-redirect-uriis then rejected viaskipMessage. This is already tested (native: web-only flags rejected when web flow not configured), but it means--custom-redirect-urialone isn't enough to opt into web flow — intentional, but worth surfacing in help text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/cli/src/commands/auth/client/add.ts` around lines 446 - 468, The flag handling incorrectly treats --configure-web as a phantom flag because optOrPromptBoolean is called with undefined instead of the CLI-parsed value; update the call site that builds configureWeb so the first argument to optOrPromptBoolean is opts['configure-web'] (or, if you choose not to expose the flag, remove simpleName: '--configure-web' from the optOrPromptBoolean call to avoid surfacing a non-existent flag), keeping the existing precedence logic around anyWebFlagProvided and yes; refer to symbols anyWebFlagProvided, configureWeb, optOrPromptBoolean, opts and the simpleName '--configure-web' when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/packages/platform/src/consts.ts`:
- Around line 12-13: The APPLE_DISCOVERY_ENDPOINT constant currently points to
'https://account.apple.com/.well-known/openid-configuration' which returns
endpoint hosts under account.apple.com; update the value of
APPLE_DISCOVERY_ENDPOINT to
'https://appleid.apple.com/.well-known/openid-configuration' so the discovered
authorization and token endpoints align with the rest of the code that uses
appleid.apple.com; modify the export of APPLE_DISCOVERY_ENDPOINT accordingly.
---
Nitpick comments:
In `@client/packages/cli/src/commands/auth/client/add.ts`:
- Around line 556-566: The meta object sent to addOAuthClient currently includes
teamId and keyId even when skipWeb=true (so both are undefined); change the call
in the function that builds the addOAuthClient payload so meta is only included
when at least one of teamId or keyId is defined (e.g., compute a local meta
variable and pass meta: metaOnlyIfPresent), or set meta to undefined when both
are undefined, and update the native-success test (authClientAddApple.test.ts)
to expect undefined (toBeUndefined()) instead of { teamId: undefined, keyId:
undefined } to match the new wire payload behavior.
- Around line 446-468: The flag handling incorrectly treats --configure-web as a
phantom flag because optOrPromptBoolean is called with undefined instead of the
CLI-parsed value; update the call site that builds configureWeb so the first
argument to optOrPromptBoolean is opts['configure-web'] (or, if you choose not
to expose the flag, remove simpleName: '--configure-web' from the
optOrPromptBoolean call to avoid surfacing a non-existent flag), keeping the
existing precedence logic around anyWebFlagProvided and yes; refer to symbols
anyWebFlagProvided, configureWeb, optOrPromptBoolean, opts and the simpleName
'--configure-web' when making the change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e2b1ea07-9bc2-419f-ad29-3c528f4a32a5
📒 Files selected for processing (8)
client/packages/cli/__tests__/authClientAddApple.test.tsclient/packages/cli/__tests__/authClientAddGithub.test.tsclient/packages/cli/__tests__/authClientAddGoogle.test.tsclient/packages/cli/src/commands/auth/client/add.tsclient/packages/cli/src/index.tsclient/packages/platform/src/consts.tsclient/packages/platform/src/index.tsclient/www/components/dash/auth/Apple.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
client/packages/cli/src/commands/auth/client/add.ts (1)
523-546: DRY: extract thecustom-redirect-uriprompt block.The prompt configuration here is nearly identical to
handleGoogleClient(lines 181–204) andhandleGithubClient(lines 315–337). With a third copy now, a small helper (e.g.,customRedirectUriOpt({ skipIf, skipMessage })) that returns theoptOrPromptconfig would eliminate drift between providers (each copy can silently diverge in wording/modifiers). Non-blocking; fine to defer to a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/cli/src/commands/auth/client/add.ts` around lines 523 - 546, The custom redirect URI prompt block duplicated in add.ts (the customRedirectUri optOrPrompt), handleGoogleClient, and handleGithubClient should be extracted into a single helper (e.g., customRedirectUriOpt({ skipIf, skipMessage })) that returns the optOrPrompt configuration object (including prompt, placeholder, modifyOutput using UI.modifiers.piped, stripFirstBlankLine, and chalk text) so callers simply call optOrPrompt(opts['custom-redirect-uri'], customRedirectUriOpt({ skipIf: skipWeb, skipMessage: `--custom-redirect-uri ${webSkipMessage}` })); update add.ts to use this helper (replace the current inline optOrPrompt), and update handleGoogleClient and handleGithubClient to import/consume the same helper to avoid copy/paste drift while preserving behavior and modifiers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/packages/cli/src/commands/auth/client/add.ts`:
- Around line 590-596: The native-only guidance message (the
summaryLines.push(...) branch that uses chalk.dim for the native-only flow) can
mislead because re-running the same command with the same --name will be
rejected by the client-name uniqueness check; update that message to instruct
users to either choose a different --name when re-running or delete the existing
client first (or provide both options), and ensure the text explicitly mentions
"re-run with a different --name or remove the existing client before re-running"
so users know how to enable web sign-in.
- Around line 556-566: The call to addOAuthClient currently always passes meta:
{ teamId, keyId } even when both are undefined; instead build meta conditionally
so the payload omits meta when values are not present. Update the caller in
add.ts (around the addOAuthClient invocation) to construct meta only when teamId
or keyId is defined (or when skipWeb is false), and pass meta only when it
contains at least one defined property so the native-only payload shape is
explicit; keep references to addOAuthClient, teamId, keyId, skipWeb and meta to
locate and change the code.
---
Nitpick comments:
In `@client/packages/cli/src/commands/auth/client/add.ts`:
- Around line 523-546: The custom redirect URI prompt block duplicated in add.ts
(the customRedirectUri optOrPrompt), handleGoogleClient, and handleGithubClient
should be extracted into a single helper (e.g., customRedirectUriOpt({ skipIf,
skipMessage })) that returns the optOrPrompt configuration object (including
prompt, placeholder, modifyOutput using UI.modifiers.piped, stripFirstBlankLine,
and chalk text) so callers simply call optOrPrompt(opts['custom-redirect-uri'],
customRedirectUriOpt({ skipIf: skipWeb, skipMessage: `--custom-redirect-uri
${webSkipMessage}` })); update add.ts to use this helper (replace the current
inline optOrPrompt), and update handleGoogleClient and handleGithubClient to
import/consume the same helper to avoid copy/paste drift while preserving
behavior and modifiers.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7f58bcf7-6942-459a-82fa-e3c5bb50a8ef
📒 Files selected for processing (1)
client/packages/cli/src/commands/auth/client/add.ts
stopachka
approved these changes
Apr 17, 2026
Contributor
stopachka
left a comment
stopachka
left a comment
There was a problem hiding this comment.
SGTM! Only bug I found was the filename w/ space
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/packages/cli/src/commands/auth/client/add.ts`:
- Around line 461-467: The promptText contains orphaned closing parentheses and
a typo; update the promptText string used in the add command so it reads clearly
and without unmatched parens (fix "reirect" -> "redirect" and remove the extra
')' characters), e.g. keep the chalk.dim(...) call only for the parenthetical
phrase and then append a properly punctuated second sentence ("Only necessary if
doing web redirect flow.") to the promptText property in this file (look for the
promptText definition in add.ts).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 213324b8-70a2-4c34-a0ba-b6fc8d93c7f1
📒 Files selected for processing (2)
client/packages/cli/src/commands/auth/client/add.tsclient/packages/platform/src/consts.ts
✅ Files skipped from review due to trivial changes (1)
- client/packages/platform/src/consts.ts
drew-harris
force-pushed
the
drewh/apple
branch
from
336fac9 to
5467982
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
client/packages/cli/src/commands/auth/client/add.ts (1)
554-560: Nit: branch on!skipWebrather thanprivateKeyfor consistency.
redirectUri(554) and the web-summary block at line 580 useprivateKeyas an implicit web-flow sentinel, while the rest of the function (teamId/keyId/customRedirectUri prompts andskipIf) keys offskipWeb. They happen to be equivalent today, but mixing the two predicates makes the web-vs-native split harder to follow and easier to break ifprivateKeyever becomes optional in web mode. Using!skipWebalso makesteamId/keyIddefinedness more obvious when readingmeta.♻️ Proposed tweak
- const redirectUri = privateKey - ? customRedirectUri || APPLE_DEFAULT_CALLBACK_URL - : undefined; + const redirectUri = skipWeb + ? undefined + : customRedirectUri || APPLE_DEFAULT_CALLBACK_URL; @@ - if (privateKey) { + if (!skipWeb) { summaryLines.push(`Team ID: ${teamId}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/cli/src/commands/auth/client/add.ts` around lines 554 - 560, Change the web-vs-native branching to use the existing skipWeb flag instead of privateKey: set redirectUri using !skipWeb (e.g. const redirectUri = !skipWeb ? customRedirectUri || APPLE_DEFAULT_CALLBACK_URL : undefined), update the web-summary block to check !skipWeb instead of privateKey, and ensure the meta population for teamId/keyId is only considered in the web branch (guard teamId/keyId handling behind !skipWeb so meta reflects web-mode fields consistently with the rest of the function).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/packages/cli/src/commands/auth/client/add.ts`:
- Around line 387-396: The normalization currently unconditionally runs
path.replace(/\\(.)/g, '$1') which corrupts Windows absolute paths (affects
normalizedPath used by fs.readFileString and the BadArgsError path in the error
message); change the logic so that the backslash-unescape is applied only on
POSIX environments (e.g., when process.platform !== 'win32') or only when the
input looks shell-escaped (not a Windows drive letter like /^[A-Za-z]:\\/), and
leave Windows paths untouched before calling fs.readFileString so valid Windows
paths like C:\Users\... are preserved.
---
Nitpick comments:
In `@client/packages/cli/src/commands/auth/client/add.ts`:
- Around line 554-560: Change the web-vs-native branching to use the existing
skipWeb flag instead of privateKey: set redirectUri using !skipWeb (e.g. const
redirectUri = !skipWeb ? customRedirectUri || APPLE_DEFAULT_CALLBACK_URL :
undefined), update the web-summary block to check !skipWeb instead of
privateKey, and ensure the meta population for teamId/keyId is only considered
in the web branch (guard teamId/keyId handling behind !skipWeb so meta reflects
web-mode fields consistently with the rest of the function).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2af5ad3e-fc45-4979-bf7b-572a0bb3e2d7
📒 Files selected for processing (8)
client/packages/cli/__tests__/authClientAddApple.test.tsclient/packages/cli/__tests__/authClientAddGithub.test.tsclient/packages/cli/__tests__/authClientAddGoogle.test.tsclient/packages/cli/src/commands/auth/client/add.tsclient/packages/cli/src/index.tsclient/packages/platform/src/consts.tsclient/packages/platform/src/index.tsclient/www/components/dash/auth/Apple.tsx
✅ Files skipped from review due to trivial changes (5)
- client/packages/cli/tests/authClientAddGoogle.test.ts
- client/www/components/dash/auth/Apple.tsx
- client/packages/platform/src/index.ts
- client/packages/platform/src/consts.ts
- client/packages/cli/tests/authClientAddApple.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- client/packages/cli/tests/authClientAddGithub.test.ts
- client/packages/cli/src/index.ts
This was referenced Apr 17, 2026
Merged
This was referenced Apr 30, 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 Apple OAuth setup to
instant-cli auth clients addOpted to make the private key inputted as a file because it felt more conventional with how other clis work, user is more likely to have it downloaded as a file already, we don't have a multi line input TUI input element yet.
Flags:
Also added tests.
cd sandbox/cli-nodejs pnpm dlx instant-cli auth client add --type apple --name asdf --services-id com.example.app --yesTest commands used