fix: stop app-add success message from flashing during onboarding#566
fix: stop app-add success message from flashing during onboarding#566WcaleNieWolny wants to merge 3 commits into
Conversation
- Merge two redundant confirm prompts ("Is this the correct app ID?" +
"Add in Capgo?") into a single select with emoji labels
- Fix duplicate app ID error not being caught (Supabase returns
"duplicate key" / code 23505, not "already exist")
- Skip checkAlerts() in addAppInternal when called silently from init
to avoid flashing clack output during Ink session
The "App add Done ✅" log was pushed then instantly cleared by the next step's renderCurrentStep → clearInitLogs cycle, making it flash for ~100ms. - Stop spinner silently after successful app add (progress bar already shows completion) - Move the "accessible to all members" info to the start of step 2 (addChannelStep) where it persists on screen
📝 WalkthroughWalkthroughApp onboarding flow and spinner initialization were adjusted: app ID confirmation is replaced by a single selection prompt with re-prompting on change; addAppInternal only runs alerts when not silent; duplicate-app detection expanded; onboarding frame now initializes an init spinner. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InitCommand as Init Command
participant Prompt as Prompt System
participant AddModule as Add App Module
participant DB as Database/API
participant Logger as Logger
User->>Prompt: start onboarding / provide appId
Prompt->>User: present select: keep appId or change
User->>Prompt: choose (keep/change)
alt change
Prompt->>User: request new appId
User->>Prompt: supply new appId
Prompt->>InitCommand: return new appId
InitCommand->>InitCommand: saveAppIdToCapacitorConfig(...)
end
Prompt->>InitCommand: return chosen appId
InitCommand->>AddModule: addAppInternal(appId, silent=false)
AddModule->>AddModule: if (!silent) checkAlerts()
AddModule->>DB: attempt to create/add app
alt success
DB-->>AddModule: app created
AddModule->>Logger: log success
Logger-->>InitCommand: notify success
else duplicate/error
DB-->>AddModule: error (exists / duplicate key / 23505)
AddModule->>InitCommand: surface duplicate error
InitCommand->>Prompt: suggest retry with different appId
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
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: a1a05cfd90
ℹ️ 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".
| options: [ | ||
| { value: 'yes', label: '✅ Yes, add it' }, | ||
| { value: 'change', label: '❌ No, use a different app ID' }, | ||
| ], |
There was a problem hiding this comment.
This new selection prompt removed the previous doAdd === false path, so onboarding now always tries to create an app instead of letting users continue with an already-existing one. In addAppStep, users who are onboarding a project whose app already exists in Capgo will hit the duplicate-key branch and be forced to choose a different app ID, whereas the prior flow allowed marking add-app complete and proceeding with the current ID.
Useful? React with 👍 / 👎.
- Remove s.stop() from addAppStep success path — spinner persists through the async markStep call instead of leaving an empty frame - Add setInitSpinner() to renderInitOnboardingFrame so any running spinner is cleared atomically with the step transition
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/command.ts`:
- Around line 1078-1079: The success message shown after calling addAppInternal
(the block with await addAppInternal(currentAppId, options, organization, true)
followed by s.stop()) was changed and now omits the original qualifier "based on
their permissions"; restore that exact wording in the moved step-2 success copy
so it matches the canonical message used by src/app/add.ts (ensure the same
phrase appears in both places, including the duplicate spot noted around the
other block at lines 1171-1174), keeping the spinner stop behavior intact
(s.stop) but updating the success string emitted after addAppInternal to include
"based on their permissions".
- Around line 1068-1070: When the user picks the "change" branch, the code
should use the edited currentAppId as the base for any duplicate-ID suggestion
generation and for the value saved; update the logic so that after
askForAppId(...) the subsequent suggestion-builder and recovery logic use
currentAppId (not the original appId) and ensure
saveAppIdToCapacitorConfig(currentAppId) persists that chosen value; apply the
same replacement of appId -> currentAppId in the duplicate-ID recovery code
paths referenced around the other spots (the blocks that build alternatives and
present suggestions).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 03e02a18-1b22-4d45-9aab-40ecaa47a449
📒 Files selected for processing (2)
src/app/add.tssrc/init/command.ts
| if (addChoice === 'change') { | ||
| currentAppId = await askForAppId('Enter the correct app ID (e.g., com.example.app):') | ||
| await saveAppIdToCapacitorConfig(currentAppId) |
There was a problem hiding this comment.
Use the edited app ID as the suggestion base.
Once the user takes the new “use a different app ID” branch, currentAppId can diverge from the original appId. The duplicate-ID recovery still builds alternatives from appId, so the suggestions — and the config value saved right after selection — can jump back to the stale base instead of the ID that actually collided.
♻️ Proposed fix
- const rawSuggestions = [
- `${appId}-${Math.random().toString(36).substring(2, 6)}`,
- `${appId}.dev`,
- `${appId}.app`,
- `${appId}-${Date.now().toString().slice(-4)}`,
- `${appId}2`,
- `${appId}3`,
+ const suggestionBase = currentAppId
+ const rawSuggestions = [
+ `${suggestionBase}-${Math.random().toString(36).substring(2, 6)}`,
+ `${suggestionBase}.dev`,
+ `${suggestionBase}.app`,
+ `${suggestionBase}-${Date.now().toString().slice(-4)}`,
+ `${suggestionBase}2`,
+ `${suggestionBase}3`,
]Also applies to: 1093-1104, 1158-1159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/init/command.ts` around lines 1068 - 1070, When the user picks the
"change" branch, the code should use the edited currentAppId as the base for any
duplicate-ID suggestion generation and for the value saved; update the logic so
that after askForAppId(...) the subsequent suggestion-builder and recovery logic
use currentAppId (not the original appId) and ensure
saveAppIdToCapacitorConfig(currentAppId) persists that chosen value; apply the
same replacement of appId -> currentAppId in the duplicate-ID recovery code
paths referenced around the other spots (the blocks that build alternatives and
present suggestions).
| await addAppInternal(currentAppId, options, organization, true) | ||
| s.stop(`App add Done ✅`) | ||
| s.stop() |
There was a problem hiding this comment.
Good flash fix, but keep the original access-scope wording.
Silently stopping the spinner is the right way to avoid the transient success flash. The moved step-2 success copy, though, drops the existing “based on their permissions” qualifier that src/app/add.ts still uses, so onboarding now communicates broader access semantics than the regular app add flow.
✏️ Proposed fix
- pLog.success(`✅ App ${appId} added — accessible to all members of your organization`)
+ pLog.success(`✅ App ${appId} added — accessible to all members of your organization based on their permissions`)Also applies to: 1171-1174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/init/command.ts` around lines 1078 - 1079, The success message shown
after calling addAppInternal (the block with await addAppInternal(currentAppId,
options, organization, true) followed by s.stop()) was changed and now omits the
original qualifier "based on their permissions"; restore that exact wording in
the moved step-2 success copy so it matches the canonical message used by
src/app/add.ts (ensure the same phrase appears in both places, including the
duplicate spot noted around the other block at lines 1171-1174), keeping the
spinner stop behavior intact (s.stop) but updating the success string emitted
after addAppInternal to include "based on their permissions".
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/init/command.ts (2)
1092-1103:⚠️ Potential issue | 🟡 MinorUse
currentAppIdas suggestion base in duplicate recovery.After Line 1069,
currentAppIdmay differ from the originalappId; Lines 1097-1102 still generate alternatives from staleappId, which can propose irrelevant IDs.♻️ Suggested fix
- const rawSuggestions = [ - `${appId}-${Math.random().toString(36).substring(2, 6)}`, - `${appId}.dev`, - `${appId}.app`, - `${appId}-${Date.now().toString().slice(-4)}`, - `${appId}2`, - `${appId}3`, + const suggestionBase = currentAppId + const rawSuggestions = [ + `${suggestionBase}-${Math.random().toString(36).substring(2, 6)}`, + `${suggestionBase}.dev`, + `${suggestionBase}.app`, + `${suggestionBase}-${Date.now().toString().slice(-4)}`, + `${suggestionBase}2`, + `${suggestionBase}3`, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/init/command.ts` around lines 1092 - 1103, The duplicate-app handling block uses stale appId when generating alternatives; change the suggestion base to currentAppId so suggestions reflect the actual (possibly modified) ID. In the error branch that logs pLog.error(`❌ App ID "${currentAppId}" is already taken`) update the rawSuggestions array (and any subsequent validation) to interpolate currentAppId instead of appId (e.g., `${currentAppId}-${...}`, `${currentAppId}.dev`, `${currentAppId}2`, etc.) so generated proposals are relevant to the current attempted ID.
1172-1172:⚠️ Potential issue | 🟡 MinorRestore the permission qualifier in the moved success copy.
Line 1172 currently broadens access semantics; align it with the canonical app-add message by keeping “based on their permissions”.
✏️ Suggested fix
- pLog.success(`✅ App ${appId} added — accessible to all members of your organization`) + pLog.success(`✅ App ${appId} added — accessible to all members of your organization based on their permissions`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/init/command.ts` at line 1172, The success message logged via pLog.success currently reads "✅ App ${appId} added — accessible to all members of your organization" which broadens access semantics; restore the canonical wording by updating the pLog.success call to include the permission qualifier ("based on their permissions") so it reads something like "✅ App ${appId} added — accessible to all members of your organization based on their permissions" (update the string inside the pLog.success invocation where it constructs the success copy).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/init/command.ts`:
- Around line 1092-1103: The duplicate-app handling block uses stale appId when
generating alternatives; change the suggestion base to currentAppId so
suggestions reflect the actual (possibly modified) ID. In the error branch that
logs pLog.error(`❌ App ID "${currentAppId}" is already taken`) update the
rawSuggestions array (and any subsequent validation) to interpolate currentAppId
instead of appId (e.g., `${currentAppId}-${...}`, `${currentAppId}.dev`,
`${currentAppId}2`, etc.) so generated proposals are relevant to the current
attempted ID.
- Line 1172: The success message logged via pLog.success currently reads "✅ App
${appId} added — accessible to all members of your organization" which broadens
access semantics; restore the canonical wording by updating the pLog.success
call to include the permission qualifier ("based on their permissions") so it
reads something like "✅ App ${appId} added — accessible to all members of your
organization based on their permissions" (update the string inside the
pLog.success invocation where it constructs the success copy).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5b6b5af-6040-458c-9815-84f12e0a3067
📒 Files selected for processing (2)
src/init/command.tssrc/init/ui.ts
1 participant
Summary
clearInitLogs(), making it flash for ~100msTest plan
node dist/index.js ithrough app add — no flash after app is addedSummary by CodeRabbit
Improvements
Bug Fixes