feat(cli): Android onboarding via Google OAuth (GCP + SA + Play invite automation)#2064
Merged
Conversation
…ay invite automation) Adds `capgo build init --platform android` — interactive TUI that handles the entire Android cloud-build setup for a developer. ## What it does Runs through six phases: 1. **Keystore**: generates a PKCS#12 (.p12) keystore via node-forge (3DES, RSA-2048, 27y validity), or accepts an existing .jks/.keystore/.p12 with auto-detected alias when possible. macOS file picker, JKS fall-through to manual alias entry. 2. **Sign in with Google**: PKCE OAuth flow on a loopback HTTP server (127.0.0.1:<random-port>). Scopes: openid, userinfo.email, androidpublisher, cloud-platform. OAuth client credentials are fetched at runtime from `/private/config/builder` (PR #2051) so they're not baked into the CLI binary. 3. **Play developer ID input**: user pastes their Play Console URL or the numeric developer ID — the Play API has no endpoint to enumerate developer accounts. Includes "Watch tutorial" + "Open Play Console" helpers. 4. **GCP project picker**: lists the user's existing projects (or "Create new"). Generates a unique project ID like `capgo-{slug}-{rand}`, max 30 chars. 5. **Android package picker**: parses `android/app/build.gradle` (Groovy + Kotlin DSL) for `applicationId` values — the Play Console package name often differs from the Capacitor JS appId because of the CapacitorUpdater plugin override. Falls back to manual input if no Gradle is found. 6. **Automated provisioning**: creates the GCP project (if new), enables `androidpublisher.googleapis.com`, creates a `capgo-native-build` service account + JSON key, invites it to the user's Play Console with release-only permissions on the chosen package. Saves the SA key as `PLAY_CONFIG_JSON` (matching the existing manual flow's credential format) so fastlane is untouched. ## Files - `cli/src/build/onboarding/android/` — new module: - `keystore.ts` — node-forge PKCS#12 generation + alias detection - `oauth-google.ts` — PKCE flow + loopback HTTP server + token exchange - `oauth-config.ts` — runtime fetch from /private/config/builder (no hardcoded secrets) - `gcp-api.ts` — Cloud Resource Manager + Service Usage + IAM wrappers, long-running operation polling, project ID generator - `play-api.ts` — Play Developer API v3 Users.create + URL parser - `gradle-parser.ts` — extracts applicationIds from build.gradle - `types.ts` + `progress.ts` — state machine + resume logic - `ui/app.tsx` — Ink TUI for all phases - `cli/src/build/onboarding/command.ts` — `--platform ios|android` routing - `cli/src/build/onboarding/file-picker.ts` — keystore + JSON file pickers - `cli/src/build/onboarding/ui/components.tsx` — `mask` prop on FilteredTextInput for password entry - `cli/src/index.ts` — `-p, --platform <platform>` flag ## Tests 29 new unit tests across 5 files (keystore, OAuth, GCP, Play, Gradle). All offline — no network round trips required. ## Depends on - #2051 — `/private/config/builder` endpoint that returns the OAuth client credentials. The CLI fetches from it at runtime; both `enabled: false` and missing credentials are surfaced as a clear error pointing the user at the manual setup docs. ## Backend env vars expected Already configured per #2051: - `GOOGLE_OAUTH_CLIENT_ID` — Capgo's OAuth Desktop-app client ID - `GOOGLE_OAUTH_CLIENT_SECRET` — same client's "secret" (treated as public per RFC 8252 §8.5; PKCE provides actual security) ## OAuth verification status Google has approved the four scopes for Capgo's OAuth client. Ready for end-to-end testing against a real Capacitor project that has its app already created in Play Console.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a complete Android onboarding flow to the CLI: keystore generation/inspection, Google OAuth 2.0 (PKCE + loopback), GCP project/service-account provisioning with polling, Play Console invite, Gradle package detection, progress persistence with resume, a React/Ink TUI, CLI platform resolution, and offline Node test suites. ChangesAndroid Onboarding Flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| catch (error) { | ||
| console.error(`❌ FAILED: ${name}`) | ||
| console.error(` Error: ${error.message}`) |
| assertEquals(GOOGLE_OAUTH_SCOPES_ANDROIDPUBLISHER.length, 3) | ||
| assert(GOOGLE_OAUTH_SCOPES_ANDROIDPUBLISHER.includes('openid')) | ||
| assert(GOOGLE_OAUTH_SCOPES_ANDROIDPUBLISHER.includes('https://www.googleapis.com/auth/userinfo.email')) | ||
| assert(GOOGLE_OAUTH_SCOPES_ANDROIDPUBLISHER.includes('https://www.googleapis.com/auth/androidpublisher')) |
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Six warnings from CodeQL js/unused-local-variable: - ui/app.tsx: drop unused state variable bindings on five useState calls (`keystoreMethod`, `googleSignIn`, `serviceAccountProvisioned`, `playInviteProvisioned`, `oauthClientSecret`). The five values were never read after assignment — readers go through `initialProgress?.completedSteps.X` (the persisted progress shape) or the corresponding `setX` setter only. Switch to bare-comma destructure `const [, setX]` to keep the setter while removing the dead binding. - ui/app.tsx: also drop the now-orphaned `setOauthClientSecret(cfg.clientSecret)` call in the OAuth start handler since neither the state value nor the setter is used elsewhere. - test/test-android-gradle.mjs: in the "picks up multiple flavors" test, use the destructured `extractApplicationIds` from the top of the test body instead of awaiting a second `imp()` call (which dynamic-imported the module again only to grab the same export). No behavior changes — all 29 android tests still pass.
…select The platform-select Select inside the iOS onboarding only listed iOS, with a "Android onboarding coming soon. Use capgo build credentials save" hint underneath. That hint was already stale (Android onboarding ships in this PR) and the listing was wrong (Android is selectable, just via a different command). Add Android as an explicit option. When picked, exit cleanly with a log line telling the user to re-run with `--platform android`. The Android flow has its own Ink app rendered by command.ts, so we can't transition to it inline from this iOS-scoped component — the cleanest UX is to give the user the exact command to copy. Drops the "coming soon" footer entirely.
…tually works The iOS Ink app's platform-select Select used to be the only place users got asked which platform to onboard. After we added an "Android" option to that Select, picking it could only ever exit-with-instructions — because the iOS-scoped React component can't host the Android Ink app without ripping itself out and rebuilding state. Move platform selection up into command.ts, before any Ink render: - @clack/prompts.select asks the user when --platform isn't passed - Smart default: only one platform dir present → use that one without prompting (keeps single-platform projects friction-free) - Cancellation (Ctrl+C in the prompt) exits cleanly - Then dispatch to AndroidOnboardingApp or OnboardingApp based on choice iOS app changes: - welcome step skips the legacy platform-select detour and runs the same checks platform-select used to gatekeep (no-platform, credentials-exist, or api-key-instructions) - adding-platform success path skips platform-select for the same reason - no-platform "I already fixed it, re-check" path also skips it - The platform-select step + render are kept as dead code for safety (nothing routes there now); follow-up could delete them entirely Net result: no flag → user gets a 🍎 iOS / 🤖 Android prompt before anything renders → picks one → that flow runs end-to-end.
TateLyman
reviewed
May 7, 2026
TateLyman
left a comment
TateLyman
left a comment
There was a problem hiding this comment.
One focused issue from the Android onboarding flow.
| const msg = err instanceof Error ? err.message : String(err) | ||
| // Treat "already exists" style failures as success — the SA is | ||
| // already a user on this developer account from a prior run. | ||
| if (!/already|exists|duplicate/i.test(msg)) |
There was a problem hiding this comment.
This retry path treats any already exists invite response as success, but it never verifies that the existing Play Console user has the app-level grants for the package selected in this run. A common recovery case is: first run invites the service account with the wrong/missing package grant, the user reruns onboarding with the correct package, users.create returns an already-exists conflict, and this branch continues to save credentials even though the service account still cannot upload/releases for that package. The existing-user path needs to fetch/update the Play user grants (or fail with a clear manual-fix message) before marking the invite confirmed.
TateLyman
reviewed
May 7, 2026
TateLyman
left a comment
TateLyman
left a comment
There was a problem hiding this comment.
One more generated-keystore edge case.
| filter="" | ||
| mask | ||
| onSubmit={(val) => { | ||
| const keyPw = val || keystoreStorePassword |
There was a problem hiding this comment.
For newly generated PKCS#12 keystores this lets the user choose a key password that differs from the store password, but generateKeystore() passes only options.storePassword to forge.pkcs12.toPkcs12Asn1(...) and never applies options.keyPassword to the private key. The credentials saved later therefore can contain KEYSTORE_KEY_PASSWORD != KEYSTORE_STORE_PASSWORD for a keystore that was actually written with only the store password, which will make the subsequent Android signing/build step fail for users who choose a custom key password. For generated .p12 files, either force keyPw = storePw/hide this prompt, or teach the generator to really encode a separate key password and cover the mismatch in tests.
adamsardo
reviewed
May 9, 2026
adamsardo
left a comment
adamsardo
left a comment
There was a problem hiding this comment.
One thing to tighten before merge: the Play invite fallback treats any already/exists/duplicate error from inviteServiceAccount() as success, but it never verifies that the existing Play Console user has the grant for the selected package.
The problematic path is in cli/src/build/onboarding/android/ui/app.tsx around the inviteServiceAccount call: if users.create returns an "already exists" style response, onboarding records playInviteProvisioned, continues to saving-credentials, and deletes the progress file. That is only safe if the existing service-account user already has the exact androidPackageChoice.packageName grant with the required app-level permissions.
A realistic failure case: a previous run invited capgo-native-build@... for package A, or with a narrower permission set. The user reruns onboarding for package B. Play returns that the user already exists, this code treats that as success, credentials are saved, and the later build/upload fails because the service account was never granted access to package B. The fix should either upsert/patch the user grants when the user already exists, or fetch the existing user/grants and require the selected package + permissions before marking the invite step complete.
mySebbe
reviewed
May 10, 2026
mySebbe
left a comment
mySebbe
left a comment
There was a problem hiding this comment.
One security isolation issue in the Android onboarding flow: the service-account identity is fixed per GCP project, not per app/package.
DEFAULT_SERVICE_ACCOUNT_ID is always capgo-native-build, and ensureServiceAccount() reuses an existing capgo-native-build@<project>.iam.gserviceaccount.com. The flow then creates a fresh JSON key for that same identity and saves it as this app's PLAY_CONFIG_JSON, while Play Console grants are added for the selected androidPackageChoice.packageName.
That means if the same GCP project is used to onboard multiple packages, the single service account accumulates grants for all of them. A JSON key saved/exported for app A can authenticate as the same service account and operate on package B after B is onboarded, so the per-app Capgo credential is not actually package-isolated.
This is separate from the existing "already exists" fallback issue: even when the invite/grant succeeds for each package, the saved keys still all belong to the same multi-package identity. Can we derive the service-account ID from the package/app, e.g. capgo-${hash(packageName)}, or otherwise create one service account per Play package and refuse/review reuse when the existing SA already has grants for other packages?
SpeedyArt
reviewed
May 10, 2026
SpeedyArt
left a comment
SpeedyArt
left a comment
There was a problem hiding this comment.
I think this still grants an account-wide Play Console permission even though the flow is described as package-scoped release setup.
inviteServiceAccount() always receives developerAccountPermissions: CAPGO_SA_DEVELOPER_PERMISSIONS, and that constant includes CAN_MANAGE_DRAFT_APPS_GLOBAL. Google treats developerAccountPermissions as permissions for the whole developer account, and this value allows draft apps to be created/edited/deleted at the account level. The same request already includes per-package grants for the selected androidPackageChoice.packageName, including app-level CAN_MANAGE_DRAFT_APPS, so the global grant does not look necessary for the normal package-scoped onboarding path.
As written, the JSON key saved for one app can manage draft apps outside the selected package, even if the app-level grant is correctly scoped. That undercuts the "release-only permissions on the chosen package" model and is separate from the fixed service-account reuse issue already mentioned above. I’d remove developerAccountPermissions for the package-scoped flow if Play accepts users with app-level grants only, or gate the account-wide permission behind an explicit warning/confirmation. A regression around the invite body should assert that normal onboarding does not include CAN_MANAGE_DRAFT_APPS_GLOBAL.
1412Atom
reviewed
May 10, 2026
1412Atom
left a comment
1412Atom
left a comment
There was a problem hiding this comment.
I found one Play Console permission issue that can make the happy path look successful while leaving the service account without release access.
| if (args.grants?.length) { | ||
| // Grant resource uses `appLevelPermissions`, NOT `permissions`. | ||
| // Ref: https://developers.google.com/android-publisher/api-ref/rest/v3/grants | ||
| body.grants = args.grants.map(g => ({ |
There was a problem hiding this comment.
The invite flow is sending the app grant inside users.create, but the Android Publisher User resource documents grants[] as output-only. That means onboarding can create/invite the service account while the package-level permissions are not actually created, so the later build/upload can still fail despite this step being marked confirmed.
The safer flow is to create/find the Play user first, then call the Grants API for developers/{developerId}/users/{email}/grants/{packageName} with appLevelPermissions. The already exists recovery path should also fetch/update that package grant before continuing, otherwise rerunning onboarding after a wrong/missing package grant will keep saving credentials that cannot upload releases.
Refs:
zinc-builds
approved these changes
May 10, 2026
zinc-builds
left a comment
zinc-builds
left a comment
There was a problem hiding this comment.
Security Review: #2064 — Android OAuth onboarding
Looks Good ✅
- PKCE S256 flow for OAuth token exchange — industry standard, prevents authorization code interception
- Loopback server on 127.0.0.1 only —
LOOPBACK_HOSThardcoded, no external network exposure - crypto.randomBytes for keystore/store/key passwords — cryptographically strong
- encodeURIComponent on all user-supplied values in GCP/Play API URLs — prevents path injection
- JSON response validation after every GCP/Play API call — catches unexpected responses before they propagate
- Service account key base64 never written to permanent credentials store (marked with underscore prefix convention)
- Progress file permissions 0o600 via
writeFileAtomic— not world-readable - ensureSecureDirectory with 0o700 for onboarding dir — prevents other users from reading progress files
- Gradle parser reads local files only — no remote fetching, no command injection
- sanitizeGcpProjectDisplayName strips disallowed characters before sending to GCP API
Warnings ⚠️
- Sensitive tokens in progress file:
_oauthRefreshToken,_serviceAccountKeyBase64,_keystoreBase64persist on disk at~/.capgo-credentials/onboarding/android-{appId}.json(0o600) until onboarding completes. If the process crashes or the user Ctrl-Cs mid-flow, these tokens remain on disk. ThedeleteAndroidProgresscall at the end of successful onboarding does clean up, but crash recovery does not guarantee it. - Math.random for project ID suffix (generateProjectId): Used for collision avoidance only (not security), and the comment acknowledges this. Acceptable for this use case, but consider
crypto.randomIntfor consistency with the rest of the codebase crypto hygiene. - Refresh token in memory:
refreshTokenStateheld in React component state for the full onboarding session. If a future code change logs component state for debugging, this token could leak.
Suggestions 💡
- Consider clearing sensitive underscore-prefixed fields from progress immediately after use (e.g., clear
_oauthRefreshTokenafter SA key is provisioned, clear_keystoreBase64after credentials are saved to permanent store) - Add a signal handler (SIGINT/SIGTERM) that cleans up the progress file before exit
- The
openpackage is imported but the fallback path inrunOAuthFlowalready handles browser-open failures gracefully — consider adding a timeout for the browser-open call to avoid hanging on headless systems
Summary
Well-structured, security-conscious code. The PKCE flow, loopback binding, and file permission practices are solid. The primary risk is token persistence in the progress file across crashes, which is partially mitigated by 0o600 permissions and the stated intent to wipe on completion. No auth bypass, no injection, no data exposure to external parties. Approved.
Contributor
Author
|
What the actual fuck.... this is a draft PR... |
…e OAuth tokens User feedback during Android OAuth E2E testing: the pre-consent screen explained WHAT the scopes do but never addressed the fears a thoughtful user has when granting cloud-platform + androidpublisher to a CLI they just downloaded. End result was risk of consent-screen drop-off at the most sensitive moment of the flow. ## Trust headline + "Learn more" pattern Default pre-consent screen is now ~8 lines instead of 12 and leads with the one fact most likely to convert a hesitant user: > "Sign in with Google so Capgo can set up Play Store publishing on > your account — your tokens never reach Capgo's servers." Adds a new Select option: ℹ️ Learn why the onboarding via Google is secure Picking it swaps the same step's render to an inline Q&A expander addressing the four fears that actually drive drop-off: - Can Capgo touch my other GCP projects? - Will Capgo upload to Play Store without me knowing? - Can Capgo employees access my Google account? - What if I change my mind later? Each answer is honest about what the scope permits vs what this CLI actually does, points at the GCP / Play Console UI the user can verify in afterwards, and links to the source. Dismissed with a "← Back to sign-in" action. Uses local React state (showOAuthLearnMore boolean) so the state machine + progress shape are unchanged. ## Behavioral change: auto-revoke after provisioning The strongest trust line that text alone can give is "we delete the progress file when we're done." That asks the user to trust our cleanup code. Stronger: after gcp-setup-running completes (project + SA + key + Play invite all succeeded), the CLI now calls revokeToken() against Google's revoke endpoint. From that point the refresh token is dead at Google's end — even if a copy lingered on the user's machine somehow, it can't be used. Implementation details: - Revoke fires once, in the success path of gcp-setup-running, before transitioning to saving-credentials - Failure is non-fatal: we surface a yellow warning and continue. The token expires within ~1 hour regardless; user value (saved credentials) takes priority - Resume-after-crash is unaffected: we only revoke on the happy path, so a partial run still has the token in the progress file for the next run to pick up ## Tests Three new tests in test-android-oauth.mjs cover: - revokeToken posts the right body to oauth2.googleapis.com/revoke - 400 (invalid_token / already revoked) is swallowed silently - 5xx throws so the caller can surface the failure All prior tests still pass: 12 keystore + 10 GCP + 6 Play + 7 Gradle + 11 OAuth = 46 green. ## Not changed - OAuth scopes requested (still openid, userinfo.email, androidpublisher, cloud-platform) - Credential file format (PLAY_CONFIG_JSON etc.) - State machine outside the OAuth phase - iOS flow - oauth-config.ts runtime fetch from /private/config/builder - Any of the API wrappers Google OAuth verification (approved 2026-05-02) covers the consent screen + scopes. Pre-consent CLI text and a new POST to Google's own revoke endpoint don't trigger re-review.
…als self-heals
When the user resumed Android onboarding after a partial run that had
lost keystoreStorePassword from the progress file (a separate race in
the void-persist calls in the keystore phase), getAndroidResumeStep
trusted completedSteps.keystoreReady as a boolean and routed all the way
through to saving-credentials. doSaveCredentials then threw "keystore
inputs missing" and the user was stuck — Google OAuth + GCP + Play
Console steps were already complete, but they had no way back to the
keystore-existing-store-password step to re-enter the missing field.
The bug is structural: each phase has a completedSteps marker (atomic
write to one field) AND a set of top-level ephemeral fields the marker
depends on (keystoreStorePassword, keystoreAlias, _keystoreBase64 for
keystore; _oauthRefreshToken for sign-in; _serviceAccountKeyBase64 for
SA provisioning). These writes can drift apart if persist() calls race,
and the resume logic must validate BOTH.
## getAndroidResumeStep rewrite
Each phase now checks both the marker AND its ephemeral dependencies:
- keystore: keystoreReady + alias + storePassword + _keystoreBase64
- google-sign-in: googleSignInComplete + _oauthRefreshToken
- service-account: serviceAccountProvisioned + _serviceAccountKeyBase64
If validation fails at any phase, the user is routed back to the input
step that collects the missing data — never forward to a phase that
would crash on the missing field. Extracted keystoreResumeStep() so
the routing rules (existing vs generate paths) are testable in
isolation.
## saving-credentials self-heal
As defense in depth, the saving-credentials handler now re-validates
the progress file before calling doSaveCredentials(). If
getAndroidResumeStep says the user should be somewhere earlier,
setStep there instead of throwing. The user sees:
ℹ Some required input was missing — sending you back to fill it in.
…and lands on the input step.
The user's case from the field: keystoreStorePassword was missing from
progress despite keystoreReady, googleSignInComplete, gcpProjectChosen,
androidPackageChosen, serviceAccountProvisioned, and playInviteProvisioned
all being set. With this fix:
1. Resume routes to keystore-existing-store-password.
2. After typing the password, persist sets it.
3. The remaining keystore steps fall through fast (alias/key already
in progress).
4. Next phases all skip — getAndroidResumeStep sees them complete and
routes straight to saving-credentials.
5. saving-credentials now passes validation → save succeeds.
No re-do of OAuth, no re-do of GCP/Play. Only the missing field gets
re-prompted.
## What this does NOT fix
The underlying race in the void persist(...) → setStep(...) pattern in
the keystore input handlers. That's a separate refactor (convert the
fire-and-forget persists to await persist within an async IIFE) and
deserves its own commit so we can review it carefully — it touches
~12 handlers. This fix shipping first means no future user gets stuck;
the race fix would only reduce the frequency of the race triggering.
All 46 tests pass.
Google's consent screen lets users uncheck individual scopes before
clicking Allow, granting whatever subset they approved. The CLI used to
proceed regardless, then crash several steps later with confusing 403s
when a downstream API call failed for lack of a required permission.
This change validates the granted scopes against what the CLI requested,
shows a clear error in BOTH the browser tab the user just came from AND
in the CLI itself, and routes back to the pre-consent screen so the user
can retry with all permissions checked.
## What's new
- `findMissingScopes(grantedScope, requestedScopes)` — public helper
that returns the requested scopes not present in the token response's
`scope` field. Handles extra (older-grant) scopes gracefully.
- `MissingScopesError` — typed exception thrown by `runOAuthFlow` when
any required scope is missing. Carries the `missing` list + the raw
`granted` string for downstream UX.
- `scopeMissingHtml(missing)` — HTML page shown in the browser when
scopes are missing. Lists which permissions weren't granted and tells
the user to head back to the terminal.
## Loopback server refactor
The server used to call `res.end(successHtml())` the moment Google
redirected back, then resolve the code promise. That meant the browser
tab said "you can close this tab" before the CLI had any chance to
validate scopes — too late to communicate "actually, please redo this".
The server now exposes a `finishResponse(html, statusCode)` callback
alongside the auth code. The CLI:
1. Receives `{ code, finishResponse }` from the server
2. Does the token exchange
3. Checks the granted scopes against the requested list
4. Calls finishResponse with success / missing-scopes / error HTML
5. The browser tab now reflects the post-exchange state, not a stale
generic success
`finishResponse` is idempotent — calling it twice is a no-op.
## Ink app
The `google-sign-in-running` handler catches MissingScopesError
specifically and routes back to `google-sign-in` (the pre-consent
prompt) with one log line per missing scope and a "please retry"
message. It does NOT count as a retry strike — this is a recoverable
user-input issue, not a transient failure.
## Tests
5 new unit tests on findMissingScopes + MissingScopesError:
- all-granted case
- one-deselected case
- empty-response case
- tolerates-extras case (older grants returned alongside ours)
- MissingScopesError shape contract
All 51 android tests pass (16 OAuth + 12 keystore + 10 GCP + 6 Play
+ 7 Gradle).
The progress file occasionally lost the just-typed field across a step transition. The user's symptom: typing the keystore store password, proceeding through the rest of the flow, then on resume being asked for the store password again (the field never made it to disk before being overwritten by the next persist). Cause: every input handler used the pattern void persist(updater) setStep(nextStep) `void persist(...)` issues the write fire-and-forget. setStep is synchronous and immediately transitions, so the next step's handler can fire its own persist that reads the on-disk state BEFORE the first persist has written — then computes next from stale state, writes, and clobbers the in-flight write from the previous handler. Classic read-modify-write race; the last write wins. Reproduces reliably when the user moves fast and consecutive steps issue persists in rapid succession (e.g. JKS keystore → store password persist races against alias persist after detecting-alias falls through fast). ## Fix Add a `persistAndStep(updater, nextStep)` helper that awaits the disk write before issuing setStep. Replace every `void persist(...) → setStep(...)` pattern (16 sites) with a single call to it. Side effect: each step transition now incurs one IO round-trip (~few ms) before the next render. Worth it — eliminates the lose-the-just-typed-field class of bug entirely. Touched handlers: - keystore-method-select (2 paths: existing / generate) - keystore-existing-path (manual input) - keystore-existing-store-password (the one the user hit) - keystore-existing-alias-select - keystore-existing-alias (manual fallback) - keystore-new-alias - keystore-new-password-method (random pw branch) - keystore-new-store-password - keystore-new-key-password - keystore-new-cn - play-developer-id-input - gcp-projects-select (existing) - gcp-project-create-name (new) - android-package-select (gradle-detected pick) - android-package-select (manual fallback) In a few cases the original pattern interleaved an addLog call between persist and setStep. addLog is sync state, doesn't depend on the persist completing, so it's moved above persistAndStep to preserve the user-visible log ordering (the log line still shows before the transition). All 51 android tests still pass.
User feedback: when resume routed them back into the keystore phase to
re-enter the lost store password, no log lines appeared for any field
they re-typed. The screen for "Key password" had no breadcrumbs above
it showing what they'd already confirmed in the current session, only
the pre-resume completedSteps markers.
Two fixes:
## 1. addLog before every persistAndStep in keystore handlers
Every input handler now logs a one-line confirmation immediately before
the persist+step transition. Visible chronological feedback as the user
re-confirms each field:
- keystore path (manual) → ✔ Keystore selected · /path
- existing store password → ✔ Store password set
- existing alias (manual) → ✔ Key alias · key0
- new alias → ✔ Key alias · release
- new password (random) → ✔ Store + key passwords generated
- new store password (manual) → ✔ Store password set
- new key password → ✔ Key password set
- new common name → ✔ Common name · com.example.app
(alias-select and the file picker already had these from before.)
## 2. Don't lie about keystore readiness when re-prompting
The initial resume log used to print "✔ Keystore ready — /path" any
time the keystoreReady marker was set, even if validation routed back
into the keystore phase to fix a missing input. That was misleading —
the user saw a green checkmark while being asked for the store
password again.
Now: when resume is landing in any keystore-phase step, the initial
log prints partial-input breadcrumbs ("Keystore selected" + "Key
alias") plus a yellow "↺ Re-confirming a missing keystore input" line.
The "✔ Keystore ready" line is only printed when the keystore phase
actually passed validation. Same fix applied to the google-sign-in
phase — if resume routes back to google-sign-in, we skip the
"✔ Signed in as ..." line.
51 android tests still pass.
…input breadcrumbs Two gaps in the checkmark coverage user just hit: 1. **Live: no "✔ Key password set" when the user submits.** The keystore-existing-key-password handler logged "✔ Keystore loaded" only after the async readFile + base64 + persist chain completed. For the ~50–100ms in between (and longer for big keystores), the user saw their input vanish into a silent void. Now it logs "✔ Key password set" synchronously the instant submit fires, then the existing "✔ Keystore loaded" line still appears after the async work completes. 2. **Resume: no checkmarks for the passwords that are already on disk.** The initial useEffect's keystore-phase breadcrumbs only covered path + alias. If the user had typed both passwords in a previous run (now safely persisted thanks to the race fix), they'd come back to a re-prompt screen and see no acknowledgment that those fields were already remembered. Now both keystoreStorePassword and keystoreKeyPassword get their own "✔ ... set" line. After this change, a typical resume into the keystore phase shows something like: ✔ Keystore selected · /Users/.../release.jks ✔ Key alias · release ✔ Store password set (no key-password line yet — that's what we're about to prompt for) ↺ Re-confirming a missing keystore input …and the user types the missing key password, sees "✔ Key password set" immediately, and the flow continues. 51 android tests still pass.
…re password
The key-password step asked the user every time, even when the answer
was already determinable. Two cases where it's not actually needed:
1. On resume, keystoreKeyPassword is already in the progress file
from a previous successful run. We had the value the whole time.
2. For PKCS#12 keystores (everything Capgo generates, plus the most
common .p12 / .pfx files in the wild) the store password and key
password are usually the same. We can verify this by trying to
decrypt the private-key bag with the store password — if it works,
they're the same and we don't need to ask.
Add `tryUnlockPrivateKey(bytes, password)` in keystore.ts: parses the
PKCS#12, verifies the MAC, then checks `pkcs8ShroudedKeyBag[0].key`
exists (node-forge populates this only when bag decryption succeeded
with the given password). Returns ok / wrong-password / no-private-key
/ unsupported-format / parse-error. JKS reports unsupported-format
(node-forge can't parse it), which gracefully falls back to asking.
UI: new ref-guarded useEffect on `keystore-existing-key-password`:
- If keystoreKeyPassword already in state (e.g. from initialProgress) →
auto-resolve, log "✔ Key password set", run the existing readFile +
persist + advance flow, smart-route via getAndroidResumeStep to skip
already-complete phases on resume.
- Else if PKCS#12 unlock probe with storePassword succeeds → auto-
resolve same way, plus log "ℹ Key password matches store password —
using the same value".
- Else (JKS, different key password, file read fails) → setKeyPassword
Probe('prompt') and render the existing FilteredTextInput.
While probing, render a spinner ("Checking if the key uses the same
password as the store...") so the user doesn't see a flash of empty.
Probe is ~tens of ms for a small keystore.
3 new tests on tryUnlockPrivateKey: ok-path, wrong-password rejection,
unsupported-format on non-PKCS12 bytes. All 54 android tests pass
(15 keystore + 16 OAuth + 10 GCP + 6 Play + 7 Gradle).
![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: 7
♻️ Duplicate comments (1)
cli/src/build/onboarding/android/ui/app.tsx (1)
829-850:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't treat Play invite conflicts as confirmed access.
This still accepts an
already existsresponse as success without checking whether the existing Play user actually hasCAPGO_SA_APP_PERMISSIONSforandroidPackageChoice.packageName. A rerun after an earlier invite with the wrong or missing app grant will continue, save credentials, and only fail later when releases cannot be uploaded. The conflict path needs to reconcile grants or stop with a manual-fix message.🤖 Prompt for 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. In `@cli/src/build/onboarding/android/ui/app.tsx` around lines 829 - 850, The catch block that swallows "already exists" errors in the invite flow must verify the existing service account's grants rather than assume success; in the catch for inviteServiceAccount (and related to androidPackageChoice, playAccountChoice, CAPGO_SA_APP_PERMISSIONS, sa.email), call the API to fetch the current grants/permissions for that service account on the developerId and packageName, and if the CAPGO_SA_APP_PERMISSIONS for androidPackageChoice.packageName are missing, either (a) attempt to reconcile by adding the missing grant via the same invite/permissions API or (b) abort with a clear addSetupStatus/manual-fix error instructing the operator to grant the required app permission; only treat "already exists" as success if the required grant is present after verification.
🧹 Nitpick comments (5)
cli/src/build/onboarding/ui/app.tsx (1)
810-816: 💤 Low valueMissing cancellation check in async IIFE.
The async IIFE triggered by the "recheck" option doesn't have access to a
cancelledflag, unlike similar patterns elsewhere in this file (e.g., lines 339-347). If the user navigates away or the component unmounts whileloadSavedCredentialsis in flight,setStepcould be called on an unmounted component.Consider wrapping this in the same cancellation pattern used elsewhere, or extracting this logic into the
useEffectblock where thecancelledflag is already managed.♻️ Suggested approach
Move the async credential check to a dedicated step (e.g.,
'rechecking-platform') handled in the mainuseEffect, which already manages thecancelledflag properly.🤖 Prompt for 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. In `@cli/src/build/onboarding/ui/app.tsx` around lines 810 - 816, The async IIFE that calls loadSavedCredentials and then setStep lacks the component unmount cancellation check; move this logic into the existing useEffect (which manages the cancelled flag) or implement the same cancelled guard used elsewhere: call loadSavedCredentials(appId) inside the effect, check if (cancelled) return before calling setStep('credentials-exist' or 'api-key-instructions'), or set an intermediate step like 'rechecking-platform' and handle it in the main useEffect so you reuse the cancelled boolean instead of calling setStep from an unprotected async IIFE.cli/src/build/onboarding/android/oauth-config.ts (1)
33-35: 💤 Low valueFix JSDoc formatting per static analysis.
The JSDoc comment should not have text on the first line after
/**. This was flagged by the CLI tests static analysis check.♻️ Proposed fix
-/** Scopes the backend tells the CLI to request. Always at least - * `https://www.googleapis.com/auth/androidpublisher`. */ +/** + * Scopes the backend tells the CLI to request. Always at least + * `https://www.googleapis.com/auth/androidpublisher`. + */🤖 Prompt for 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. In `@cli/src/build/onboarding/android/oauth-config.ts` around lines 33 - 35, The JSDoc for the scopes property has text on the same line as /** which fails static analysis; update the comment above the scopes property (scopes: string[]) so the opening /** is followed by a newline and the description lines start with " * " (e.g. /**\n * Scopes the backend tells the CLI to request. Always at least\n * `https://www.googleapis.com/auth/androidpublisher`.\n */) so no descriptive text appears on the first line after /**.cli/src/build/onboarding/android/progress.ts (1)
55-66: 💤 Low valueConsider logging non-ENOENT errors in
deleteAndroidProgress.The catch block silently ignores all errors, including permission errors (EACCES) or other unexpected failures. While ENOENT (file not found) is expected and fine to ignore, other errors might indicate real problems.
Optional: be selective about ignored errors
export async function deleteAndroidProgress( appId: string, baseDir?: string, ): Promise<void> { const filePath = getProgressPath(appId, baseDir) try { await unlink(filePath) } - catch { - // Not found — fine + catch (error: unknown) { + // ENOENT is expected when file doesn't exist + if (error instanceof Error && 'code' in error && (error as NodeJS.ErrnoException).code !== 'ENOENT') + console.warn(`Warning: could not delete progress file: ${(error as Error).message}`) } }🤖 Prompt for 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. In `@cli/src/build/onboarding/android/progress.ts` around lines 55 - 66, The catch in deleteAndroidProgress currently swallows all errors; update it to handle only ENOENT silently and surface or log other failures — after calling unlink(filePath) (use getProgressPath to locate the file), catch the error and if (err.code !== 'ENOENT') then log the error or rethrow it (e.g., console.error or throw) so permission or unexpected errors (EACCES, etc.) are not ignored.cli/src/build/onboarding/android/gcp-api.ts (1)
99-100: 💤 Low valueCasting empty response to
Tmay cause downstream issues.When the response body is empty, returning
undefined as unknown as Tcould cause type confusion if callers expect a valid object. Consider throwing an error for methods that require a response body, or make the return type explicit.🤖 Prompt for 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. In `@cli/src/build/onboarding/android/gcp-api.ts` around lines 99 - 100, The code currently returns "return undefined as unknown as T" when the response body is empty (the branch that checks "if (!text.trim())"), which can mislead callers expecting a valid T; either change the function signature to return T | undefined and propagate that to callers, or throw a descriptive error (e.g., "Empty response body") instead of casting; update the branch that checks text.trim() to implement one of these two approaches and adjust any callers of this function accordingly so they handle the new explicit undefined or catch the thrown error.cli/src/build/onboarding/android/ui/app.tsx (1)
325-339: ⚡ Quick winFormat onboarding errors before rendering them.
handleErrorcurrently shows raw.message/String(err)output from OAuth, GCP, and Play failures. Running that throughformatError(...)here would keep the TUI consistent with the rest of the CLI and avoid leaking noisy nested exception text. As per coding guidelines, "For user-visible error messages, format errors withformatError(...)instead of dumping raw exceptions when possible".🤖 Prompt for 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. In `@cli/src/build/onboarding/android/ui/app.tsx` around lines 325 - 339, The handleError callback is using raw error.message/String(err) which can leak noisy nested exceptions; wrap/replace that extraction with a call to formatError(err) and use its result wherever message is used (in setError, addLog and logs before exitOnboarding) so UI errors are consistently formatted; update the handleError function (reference: handleError, setError, setRetryStep, setRetryCount, addLog, exitOnboarding, AndroidOnboardingStep) to compute const message = formatError(err) and keep the existing retry/control flow.
🤖 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 `@cli/src/build/onboarding/android/gcp-api.ts`:
- Around line 340-349: ESLint flags complain about explicit A-Za-z ranges;
update the regexes in sanitizeGcpProjectDisplayName to use the case-insensitive
i flag and simplified character classes: replace each [A-Za-z0-9] with [A-Z0-9]
and append the i flag (e.g. /[^A-Z0-9 \-'!.]/gi, /^[^A-Z0-9]+/i, /[^A-Z0-9]+$/i,
and the final /[^A-Z0-9]+$/i), keeping existing global flags where needed so
validation behavior remains identical.
In `@cli/src/build/onboarding/android/oauth-google.ts`:
- Around line 450-456: startLoopbackServer currently handles an already-aborted
args.signal by calling onAbort() which rejects codePromise via codeReject but
never calls the outer resolveHandle/rejectHandle, leaving the returned promise
unresolved; update the early-abort branch so that when args.signal.aborted you
both invoke onAbort() and immediately call rejectHandle with an AbortError (or
the same error used by onAbort) so the outer promise is rejected, or
alternatively modify onAbort to also call rejectHandle; reference
startLoopbackServer, args.signal, onAbort, codePromise/codeReject, and
resolveHandle/rejectHandle.
In `@cli/src/build/onboarding/android/ui/app.tsx`:
- Around line 251-259: The helper persistAndStep currently uses a detached async
IIFE so failures from persist(...) become unhandled rejections; change
persistAndStep to be async (or return the promise) instead of an IIFE, await
persist(updater) then setStep(nextStep) inside a try/catch, and in the catch
call handleError(error) (or otherwise surface the error) so saveAndroidProgress
failures are routed through handleError instead of stalling silently; locate the
persistAndStep definition and update it to use async/await with try/catch
referencing persist, AndroidOnboardingProgress, AndroidOnboardingStep and
handleError.
- Around line 1265-1283: The manual keystore-password branch currently
unconditionally calls setStep('google-sign-in'); instead use the same
resume-routing logic as the auto-probe path: call getAndroidResumeStep(...) (the
same function used around Lines 590-594) to compute the next step based on the
updated persisted state/ completedSteps, then call setStep(resumeStep) instead
of hardcoding 'google-sign-in'; locate the async block that reads
keystoreExistingPath, remove the static setStep('google-sign-in') and replace it
with computing the resume step from getAndroidResumeStep and setting that value
so resumed runs continue at the correct phase.
- Around line 1499-1510: The onChange handler currently logs success
unconditionally after calling open(...); modify the two branches that call
open(PLAY_DEVELOPERS_URL) and open(PLAY_DEV_ID_TUTORIAL_URL) so the addLog
success message (e.g., "🌐 Opened Play Console..." and "🎬 Opened video
tutorial...") is executed inside the try block only when open() resolves, and in
the catch block call addLog with a helpful fallback/hint (e.g., instructing
headless/WSL users to open the URL manually or check their environment). Ensure
other actions like setPlayDevIdMode('input') remain where appropriate only after
successful open.
In `@cli/src/build/onboarding/file-picker.ts`:
- Around line 62-69: The exported function openServiceAccountJsonPicker is
currently unused and flagged by knip; either remove this dead export or wire it
into the onboarding flow that requires selecting a service-account JSON. If you
choose removal, delete the openServiceAccountJsonPicker function and its export
(and any tests/imports) to satisfy the analyzer; if you choose to wire it up,
call openServiceAccountJsonPicker from the Android onboarding handler where
service-account selection occurs and ensure openMacFilePicker is only used on
macOS (or gate the call with a platform check) so the function is referenced and
platform-safe.
In `@cli/src/index.ts`:
- Around line 762-764: Update the .option('-p, --platform <platform>') help text
to reflect the new resolver behavior: mention that resolvePlatform() will
auto-detect the single installed platform and will prompt the user when both or
neither are obvious, rather than claiming "ios (default)"; update the
description string in cli/src/index.ts where .option('-p, --platform
<platform>') is declared to something like "Platform to onboard: ios or android;
if omitted, resolvePlatform() will auto-detect or prompt when ambiguous" so the
CLI help matches the actual behavior.
---
Duplicate comments:
In `@cli/src/build/onboarding/android/ui/app.tsx`:
- Around line 829-850: The catch block that swallows "already exists" errors in
the invite flow must verify the existing service account's grants rather than
assume success; in the catch for inviteServiceAccount (and related to
androidPackageChoice, playAccountChoice, CAPGO_SA_APP_PERMISSIONS, sa.email),
call the API to fetch the current grants/permissions for that service account on
the developerId and packageName, and if the CAPGO_SA_APP_PERMISSIONS for
androidPackageChoice.packageName are missing, either (a) attempt to reconcile by
adding the missing grant via the same invite/permissions API or (b) abort with a
clear addSetupStatus/manual-fix error instructing the operator to grant the
required app permission; only treat "already exists" as success if the required
grant is present after verification.
---
Nitpick comments:
In `@cli/src/build/onboarding/android/gcp-api.ts`:
- Around line 99-100: The code currently returns "return undefined as unknown as
T" when the response body is empty (the branch that checks "if (!text.trim())"),
which can mislead callers expecting a valid T; either change the function
signature to return T | undefined and propagate that to callers, or throw a
descriptive error (e.g., "Empty response body") instead of casting; update the
branch that checks text.trim() to implement one of these two approaches and
adjust any callers of this function accordingly so they handle the new explicit
undefined or catch the thrown error.
In `@cli/src/build/onboarding/android/oauth-config.ts`:
- Around line 33-35: The JSDoc for the scopes property has text on the same line
as /** which fails static analysis; update the comment above the scopes property
(scopes: string[]) so the opening /** is followed by a newline and the
description lines start with " * " (e.g. /**\n * Scopes the backend tells the
CLI to request. Always at least\n *
`https://www.googleapis.com/auth/androidpublisher`.\n */) so no descriptive text
appears on the first line after /**.
In `@cli/src/build/onboarding/android/progress.ts`:
- Around line 55-66: The catch in deleteAndroidProgress currently swallows all
errors; update it to handle only ENOENT silently and surface or log other
failures — after calling unlink(filePath) (use getProgressPath to locate the
file), catch the error and if (err.code !== 'ENOENT') then log the error or
rethrow it (e.g., console.error or throw) so permission or unexpected errors
(EACCES, etc.) are not ignored.
In `@cli/src/build/onboarding/android/ui/app.tsx`:
- Around line 325-339: The handleError callback is using raw
error.message/String(err) which can leak noisy nested exceptions; wrap/replace
that extraction with a call to formatError(err) and use its result wherever
message is used (in setError, addLog and logs before exitOnboarding) so UI
errors are consistently formatted; update the handleError function (reference:
handleError, setError, setRetryStep, setRetryCount, addLog, exitOnboarding,
AndroidOnboardingStep) to compute const message = formatError(err) and keep the
existing retry/control flow.
In `@cli/src/build/onboarding/ui/app.tsx`:
- Around line 810-816: The async IIFE that calls loadSavedCredentials and then
setStep lacks the component unmount cancellation check; move this logic into the
existing useEffect (which manages the cancelled flag) or implement the same
cancelled guard used elsewhere: call loadSavedCredentials(appId) inside the
effect, check if (cancelled) return before calling setStep('credentials-exist'
or 'api-key-instructions'), or set an intermediate step like
'rechecking-platform' and handle it in the main useEffect so you reuse the
cancelled boolean instead of calling setStep from an unprotected async IIFE.
🪄 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: 35641335-a9e3-48b5-893f-e9bb2366a258
📒 Files selected for processing (19)
cli/src/build/onboarding/android/gcp-api.tscli/src/build/onboarding/android/gradle-parser.tscli/src/build/onboarding/android/keystore.tscli/src/build/onboarding/android/oauth-config.tscli/src/build/onboarding/android/oauth-google.tscli/src/build/onboarding/android/play-api.tscli/src/build/onboarding/android/progress.tscli/src/build/onboarding/android/types.tscli/src/build/onboarding/android/ui/app.tsxcli/src/build/onboarding/command.tscli/src/build/onboarding/file-picker.tscli/src/build/onboarding/ui/app.tsxcli/src/build/onboarding/ui/components.tsxcli/src/index.tscli/test/test-android-gcp.mjscli/test/test-android-gradle.mjscli/test/test-android-keystore.mjscli/test/test-android-oauth.mjscli/test/test-android-play.mjs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28d914e44d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Remove `verifyKeystore` from android/keystore.ts (only callers were its own tests; round-trip is already covered by `tryUnlockPrivateKey` and `listKeystoreAliases`). - Remove `openServiceAccountJsonPicker` from onboarding/file-picker.ts (no callers). - Drop the dedicated verifyKeystore tests; keep a basic-shape assertion on `generateKeystore` output. `knip --exports` now passes clean. 14/14 android keystore tests pass.
Real bugs:
- oauth-google: startLoopbackServer's early-abort branch only rejected
codePromise, leaving the outer handle Promise hanging forever. onAbort
now also calls rejectHandle; post-listen abort is a no-op on an
already-resolved Promise.
- ui/app.tsx persistAndStep: a detached async IIFE swallowed
saveAndroidProgress failures (disk full / EACCES / etc.) as unhandled
rejections, leaving the UI stalled. Now wraps the await in try/catch
and routes failures through handleError via a forward-declared ref
(avoids dep churn against retryCount).
- ui/app.tsx manual keystore key-password branch: hardcoded
setStep('google-sign-in') ignored already-completed phases on resume.
Now mirrors the auto-probe branch and uses getAndroidResumeStep().
- ui/app.tsx Play Console + tutorial onChange: logged success
unconditionally even when `open()` threw (headless / WSL / SSH). Now
logs success only inside try; catch emits a yellow fallback with the
URL.
- progress.ts deleteAndroidProgress: bare `catch {}` swallowed EACCES /
EPERM / EBUSY. Narrowed to ENOENT-only; everything else propagates.
Cosmetic / consistency:
- gcp-api.ts sanitizeGcpProjectDisplayName: switched [A-Za-z0-9] to
[A-Z0-9] + /i flag (ESLint preference; identical behavior).
- oauth-config.ts: moved JSDoc description off the `/**` line on the
CapgoOAuthClientConfig.scopes property.
- index.ts: --platform help text now describes auto-detect + prompt
behavior instead of the stale "ios (default)" wording.
Skipped (with reasons in the review thread):
- Play invite verify-grants: HIGH-risk architectural refactor, separate
PIOSEE-scoped change.
- gcp-api.ts `undefined as T` cast: same pattern in play-api.ts not
flagged; fixing one side creates inconsistency.
- formatError swap in handleError: no behavioral change for any error
this flow throws.
- ui/app.tsx (iOS) async IIFE cancelled flag: sibling onChange has the
identical pattern; addressing one in isolation is anti-minimal.
Validation: build + tsc --noEmit + eslint + knip all green;
53/53 android tests pass.
Contributor
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 `@cli/src/build/onboarding/android/gcp-api.ts`:
- Around line 70-107: The gcpFetch function can hang on network stalls; wrap the
fetch call with an AbortController and a configurable timeout (e.g., const
timeoutMs = 30_000) so the request is aborted after the interval: create an
AbortController, pass its signal into fetch({ ..., signal }), set a timer to
call controller.abort(), and clear the timer after fetch completes; catch the
abort error and rethrow a clear timeout-specific Error (e.g., "Google API
request to <url> timed out after <n>ms") while preserving existing JSON/error
parsing behavior in gcpFetch.
- Around line 272-290: The createServiceAccountKey function currently always
POSTs a new key via gcpFetch to IAM_ENDPOINT; change it to first list existing
keys for the given service account (call the IAM list keys endpoint) and, if a
usable user-managed key exists, decode and return that instead of creating a new
one; if no suitable key exists, then create a new key as now. Use the same args
(accessToken, projectId, serviceAccountEmail) and the gcpFetch helper to call
the list endpoint for keys, filter for user-managed keys (and optionally
KEY_ALG_RSA_2048/privateKeyType), reuse the existing key value if present,
otherwise proceed to POST to create and return the new key; alternatively
implement safe cleanup by deleting old user-managed keys via the IAM delete key
endpoint before creating a new one to avoid exceeding the 10-key quota.
In `@cli/src/build/onboarding/android/ui/app.tsx`:
- Around line 658-660: The current addLog call that prints the password-backup
hint is premature because the generated password isn't persisted until the
saving-credentials step; update the code so the hint is either moved from the
addLog invocation after keystore generation to the successful credentials-save
path (the saving-credentials completion handler), or change the message here to
state the password will be saved later (e.g., “will be stored in
~/.capgo-credentials/credentials.json when credentials are saved”) and only emit
the backup hint when saving actually succeeds; locate the addLog(...) line that
uses randomPasswordGenerated and adjust it and/or the success handler in the
saving-credentials flow accordingly.
🪄 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: a932bf41-004e-433d-a1fd-d952cdaad256
📒 Files selected for processing (9)
cli/src/build/onboarding/android/gcp-api.tscli/src/build/onboarding/android/keystore.tscli/src/build/onboarding/android/oauth-config.tscli/src/build/onboarding/android/oauth-google.tscli/src/build/onboarding/android/progress.tscli/src/build/onboarding/android/ui/app.tsxcli/src/build/onboarding/file-picker.tscli/src/index.tscli/test/test-android-keystore.mjs
🚧 Files skipped from review as they are similar to previous changes (4)
- cli/src/index.ts
- cli/src/build/onboarding/android/progress.ts
- cli/src/build/onboarding/android/oauth-config.ts
- cli/src/build/onboarding/android/oauth-google.ts
- Thread --apikey through the Android onboarding path. AppProps gains an optional `apikey`; command.ts passes options.apikey when rendering AndroidOnboardingApp; the requesting-build step now uses `apikey ?? findSavedKey(true)`, matching the iOS precedence at onboarding/ui/app.tsx:624. Fixes a regression where `build init --platform android --apikey FOO` silently ignored FOO and fell back to the saved key (or failed if none was logged in). - Add an AbortController + 30s timeout to gcpFetch. Previously a stalled Google API call (DNS hang, slow TCP handshake, server not responding) would block onboarding indefinitely. On abort, rethrow with a clear "Google API request to <url> timed out after <n>ms" so the caller can distinguish a timeout from any other network error. JSON/error parsing behavior is unchanged. Configurable per call via optional `timeoutMs`. - Move the random-password backup hint from after-keystore-generation to after-saving-credentials success. The original "stored in credentials.json" claim was emitted before that file was written; now it only fires once the save actually succeeds. Resume edge case (randomPasswordGenerated lost across CLI restarts → hint skipped) documented in a comment as an acceptable trade-off vs. persisting a one-off flag to progress.json. Validation: build + tsc --noEmit + eslint + knip all green; 53/53 android tests pass.
|
This was referenced May 18, 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
9 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.
Summary
Adds
capgo build init --platform android— interactive TUI that handles the entire Android cloud-build setup for a developer in one flow.Replaces the manual ~30-step process documented at /docs/cli/cloud-build/android (gcloud install → GCP project → enable API → service account → JSON key → Play Console invite → save credentials) with a single command. The output credential format (
PLAY_CONFIG_JSON+ keystore) is identical to the manual flow, so the existing fastlane-based build pipeline is untouched.What it does
.p12) via node-forge, or reuses an existing.jks/.keystore/.p12with auto-detected alias when possibleopenid,userinfo.email,androidpublisher,cloud-platform(Google has approved verification for Capgo's OAuth client)capgo-{slug}-{rand}oneandroid/app/build.gradle(Groovy + Kotlin DSL) forapplicationIds; falls back to manual input if Gradle is absent. Important becauseCapacitorUpdater.appIdoverrides often diverge from the Android applicationIdandroidpublisher.googleapis.com, createscapgo-native-buildSA + JSON key, invites SA to Play Console with release-only permissions on the chosen packageANDROID_KEYSTORE_FILE,KEYSTORE_*, andPLAY_CONFIG_JSONto~/.capgo-credentials/credentials.json(same format as the manual flow)Depends on
/private/config/builderendpoint (already merged). The CLI fetches OAuth client credentials from there at runtime — no secrets baked into the CLI binary. Bothenabled: falseand the Capgo backend missing env vars are surfaced as a clear error pointing the user at the manual setup docs.Backend env vars expected
Already documented per #2051:
Tests
29 new unit tests across 5 files in `cli/test/test-android-*.mjs` (keystore, OAuth, GCP, Play, Gradle parser). All offline — no network round trips.
Test plan
Notes
Summary by CodeRabbit
New Features
Tests