fix(init): save new app ID to capacitor config when original is taken#503
Conversation
When the app ID is already taken in Capgo and the user selects a new one from suggestions or enters a custom one, the new app ID is now saved to the capacitor config file (capacitor.config.ts/js/json). This ensures the local project stays in sync with the newly registered app in Capgo. Previously, the new app ID was only used for the API call but not persisted locally, causing a mismatch between the local config and the Capgo backend.
📝 WalkthroughWalkthroughThe PR adds functionality to persist the application ID to the Capacitor updater configuration during the onboarding flow. A new internal function reads the Capacitor config, updates the appId field, writes it back using the existing config writer, and includes error handling with user guidance for manual updates if persistence fails. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 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 |
|
![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: 2d6e80310c
ℹ️ 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".
| // Save the new app ID to capacitor config | ||
| await saveAppIdToCapacitorConfig(currentAppId) |
There was a problem hiding this comment.
This writes currentAppId to capacitor.config.* before addAppInternal confirms the app was actually created, so a later non-already exist failure (re-thrown at the bottom of the catch) leaves local config pointing to an unregistered app ID. That recreates the local/backend mismatch this change is trying to prevent in failure paths; persisting the ID should happen only after the add call succeeds.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/init.ts`:
- Around line 179-183: The current code silently skips saving when
extConfig.config is falsy; update the block around extConfig?.config to add an
else branch that calls pLog.warn (or pLog.info with warning) to inform the user
that the capacitor config was not found/usable and that the appId was not saved,
and include guidance on how to fix or retry; modify the logic that calls
writeConfig('CapacitorUpdater', extConfig, true) so it only runs when extConfig
&& extConfig.config, and emit the warning otherwise to mirror the existing catch
behavior (referencing extConfig, extConfig.config, writeConfig, and pLog).
- Around line 407-412: Currently saveAppIdToCapacitorConfig is invoked inside
the catch block immediately after the user picks currentAppId, which can leave
local config pointing to an ID that was never registered; instead, remove the
save from the catch and only call saveAppIdToCapacitorConfig(currentAppId) after
addAppInternal succeeds (the success path), and guard the save with a check that
the saved ID actually differs from the previous one so you don't overwrite
unnecessarily; update the flow around addAppInternal, currentAppId, maxRetries
and exit(1) to ensure the config is only written on confirmed registration.
- Line 19: Move the import of writeConfig so it appears before the import from
'./utils' to satisfy the perfectionist/sort-imports rule; specifically, adjust
the import ordering in src/init.ts so the statement importing writeConfig from
'./config' is placed above the import that references './utils' (preserving
existing named/default specifiers and spacing).
| import { createKeyInternal } from './key' | ||
| import { doLoginExists, loginInternal } from './login' | ||
| import { createSupabaseClient, findBuildCommandForProjectType, findMainFile, findMainFileForProjectType, findProjectType, findRoot, findSavedKey, getAllPackagesDependencies, getAppId, getBundleVersion, getConfig, getInstalledVersion, getLocalConfig, getOrganization, getPackageScripts, getPMAndCommand, PACKNAME, projectIsMonorepo, promptAndSyncCapacitor, updateConfigbyKey, updateConfigUpdater, validateIosUpdaterSync, verifyUser } from './utils' | ||
| import { writeConfig } from './config' |
There was a problem hiding this comment.
Fix import ordering to resolve ESLint error.
The static analysis tool reports: Expected "./config" to come before "./utils" (perfectionist/sort-imports). Move the writeConfig import above the ./utils import.
♻️ Proposed fix
+import { writeConfig } from './config'
import { checkAlerts } from './api/update'
import { addAppInternal } from './app/add'
...
import { checkAlerts, ... } from './utils'
-import { writeConfig } from './config'🧰 Tools
🪛 ESLint
[error] 19-19: Expected "./config" to come before "./utils".
(perfectionist/sort-imports)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/init.ts` at line 19, Move the import of writeConfig so it appears before
the import from './utils' to satisfy the perfectionist/sort-imports rule;
specifically, adjust the import ordering in src/init.ts so the statement
importing writeConfig from './config' is placed above the import that references
'./utils' (preserving existing named/default specifiers and spacing).
| if (extConfig?.config) { | ||
| extConfig.config.appId = appId | ||
| await writeConfig('CapacitorUpdater', extConfig, true) | ||
| pLog.info(`💾 Saved new app ID "${appId}" to capacitor config`) | ||
| } |
There was a problem hiding this comment.
Silent skip when extConfig.config is falsy — add a fallback warning.
If getConfig() somehow returns a value with a falsy .config (the ?. guard implies this is a considered possibility), the save is silently skipped and the user receives no feedback. This is inconsistent with the catch block, which does warn and guide the user.
♻️ Proposed fix
const extConfig = await getConfig()
if (extConfig?.config) {
extConfig.config.appId = appId
await writeConfig('CapacitorUpdater', extConfig, true)
pLog.info(`💾 Saved new app ID "${appId}" to capacitor config`)
}
+ else {
+ pLog.warn(`⚠️ Could not find capacitor config to update. Please set appId to "${appId}" manually.`)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (extConfig?.config) { | |
| extConfig.config.appId = appId | |
| await writeConfig('CapacitorUpdater', extConfig, true) | |
| pLog.info(`💾 Saved new app ID "${appId}" to capacitor config`) | |
| } | |
| if (extConfig?.config) { | |
| extConfig.config.appId = appId | |
| await writeConfig('CapacitorUpdater', extConfig, true) | |
| pLog.info(`💾 Saved new app ID "${appId}" to capacitor config`) | |
| } | |
| else { | |
| pLog.warn(`⚠️ Could not find capacitor config to update. Please set appId to "${appId}" manually.`) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/init.ts` around lines 179 - 183, The current code silently skips saving
when extConfig.config is falsy; update the block around extConfig?.config to add
an else branch that calls pLog.warn (or pLog.info with warning) to inform the
user that the capacitor config was not found/usable and that the appId was not
saved, and include guidance on how to fix or retry; modify the logic that calls
writeConfig('CapacitorUpdater', extConfig, true) so it only runs when extConfig
&& extConfig.config, and emit the warning otherwise to mirror the existing catch
behavior (referencing extConfig, extConfig.config, writeConfig, and pLog).
|
|
||
| // Save the new app ID to capacitor config | ||
| await saveAppIdToCapacitorConfig(currentAppId) | ||
|
|
||
| pLog.info(`🔄 Trying with new app ID: ${currentAppId}`) | ||
| continue |
There was a problem hiding this comment.
saveAppIdToCapacitorConfig is called before the new ID is confirmed to be registrable, leaving the local config in an inconsistent state.
The save is triggered inside the catch block, right after the user picks a new ID but before addAppInternal is called for it in the next iteration. If that attempt also fails (e.g. the suggested ID is also taken), the capacitor config now contains an unregistered app ID. With maxRetries = 5, all retries could fail and exit(1) fires, leaving the config pointing at an ID that was never successfully created on the Capgo backend.
The save belongs in the success path, guarded by a change check:
🐛 Proposed fix — move save to success path
try {
const s = pSpinner()
s.start(`Running: ${pm.runner} `@capgo/cli`@latest app add ${currentAppId}`)
const addRes = await addAppInternal(currentAppId, options, organization, true)
if (!addRes)
s.stop(`App already add ✅`)
else
s.stop(`App add Done ✅`)
+ // Persist the app ID only after it has been successfully registered,
+ // and only when it differs from the one already in the local config.
+ if (currentAppId !== appId) {
+ await saveAppIdToCapacitorConfig(currentAppId)
+ }
+
pLog.info(`This app is accessible to all members of your organization based on their permissions`)
await markStep(organization.gid, apikey, 'add-app', currentAppId)
return currentAppId
}
catch (error) {
...
if (choice === 'custom') { ... }
else { ... }
- // Save the new app ID to capacitor config
- await saveAppIdToCapacitorConfig(currentAppId)
-
pLog.info(`🔄 Trying with new app ID: ${currentAppId}`)
continue
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/init.ts` around lines 407 - 412, Currently saveAppIdToCapacitorConfig is
invoked inside the catch block immediately after the user picks currentAppId,
which can leave local config pointing to an ID that was never registered;
instead, remove the save from the catch and only call
saveAppIdToCapacitorConfig(currentAppId) after addAppInternal succeeds (the
success path), and guard the save with a check that the saved ID actually
differs from the previous one so you don't overwrite unnecessarily; update the
flow around addAppInternal, currentAppId, maxRetries and exit(1) to ensure the
config is only written on confirmed registration.
2 participants
Summary (AI generated)
When the app ID is already taken in Capgo and the user selects a new one during the
capgo initonboarding flow, the new app ID is now saved to the CapacitorUpdater plugin config. This ensures the local project stays in sync with the newly registered app in Capgo.Motivation (AI generated)
Previously, when a user ran
capgo initand the app ID was already taken:Users would end up with an app registered in Capgo but their local config still had the old (taken) app ID, leading to confusion and issues with subsequent CLI commands.
Business Impact (AI generated)
capgo initflow now handles the full setup automaticallyChanges (AI generated)
saveAppIdToCapacitorConfig()helper function that uses the existingupdateConfigUpdater()utilityconfig.plugins.CapacitorUpdater.appIdfollowing the existing patternTest Plan (AI generated)
capgo initwith an app ID that already exists in Capgocapacitor.config.tsunderplugins.CapacitorUpdater.appIdGenerated with AI