feat(onboarding): reuse pending apps during init#548
Merged
Conversation
📝 WalkthroughWalkthroughAdds pending onboarding app support: new types and API to list/complete pending apps, schema extensions for onboarding metadata, and integration of pending-app selection and Capacitor readiness into the init flow. Changes
Sequence DiagramsequenceDiagram
participant User
participant Init as Init Flow
participant Supabase as Supabase (apps table)
participant CapConfig as Capacitor Config
User->>Init: Start initialization
Init->>Supabase: listPendingOnboardingApps(owner_org, need_onboarding=true)
Supabase-->>Init: Return pending apps or [] on missing-schema
alt Pending apps found
Init->>User: Prompt/select pending app (or use requested appId)
User-->>Init: Confirm selection
Init->>Supabase: completePendingOnboardingApp(owner_org, appId)
Supabase-->>Init: Update success
alt selected existing_app == false
Init->>CapConfig: Check for Capacitor config
alt No Capacitor config
Init->>User: Prompt to run `cap init` with appId/name
User-->>Init: Run `cap init`
Init->>CapConfig: saveAppIdToCapacitorConfig(appId)
end
else existing app
Init->>CapConfig: Optionally persist appId
end
end
Init->>Init: ensureCapacitorProjectReady()
Init-->>User: Continue onboarding flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/api/app.ts (1)
61-76: Consider verifying the update affected a row.The update operation succeeds silently if no rows match the criteria (e.g., app already completed or doesn't exist). While this could be intentional for idempotency, it may mask issues where the caller expects confirmation that an app was actually found and updated.
♻️ Optional: Add row count verification
export async function completePendingOnboardingApp( supabase: SupabaseClient<Database>, orgId: string, appId: string, ): Promise<void> { - const { error } = await supabase + const { error, count } = await supabase .from('apps') .update({ need_onboarding: false }) .eq('owner_org', orgId) .eq('app_id', appId) .eq('need_onboarding', true) + .select('app_id', { count: 'exact', head: true }) if (error) { throw new Error(`Could not complete onboarding for app ${appId}: ${error.message}`) } + + if (count === 0) { + // App was already completed or doesn't exist - this is fine for idempotency + // but log it for debugging if needed + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/app.ts` around lines 61 - 76, The update in completePendingOnboardingApp can succeed without touching any rows; modify the function to verify the update actually affected a row by inspecting the supabase response (e.g., check that the returned data exists and data.length > 0 or use count if enabled) after the .update() call, and throw a descriptive error (include appId and orgId) when no rows were updated so callers know the app was not found or already completed.
🤖 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 308-321: The current block in shouldInitCapacitor builds a shell
string with appName/appId and calls execSync, which risks command injection;
change the call to a non-shell variant that accepts an argv array (e.g.,
child_process.spawnSync or execFileSync) so pm.runner is executed with
['cap','init', appName, appId] (or equivalent) as arguments and execOption
preserved; keep the spinner logic (pSpinner, spinner.start/stop) and still call
saveAppIdToCapacitorConfig(appId) on success, but replace execSync usage with
spawnSync/execFileSync to pass appName and appId as separate, safely-escaped
arguments.
---
Nitpick comments:
In `@src/api/app.ts`:
- Around line 61-76: The update in completePendingOnboardingApp can succeed
without touching any rows; modify the function to verify the update actually
affected a row by inspecting the supabase response (e.g., check that the
returned data exists and data.length > 0 or use count if enabled) after the
.update() call, and throw a descriptive error (include appId and orgId) when no
rows were updated so callers know the app was not found or already completed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12ddaacb-4b04-40fc-888d-17be5248f1d8
📒 Files selected for processing (3)
src/api/app.tssrc/init.tssrc/types/supabase.types.ts
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/api/app.ts (1)
18-37: Consider documenting the error suppression rationale.The token matching is intentionally broad to handle schema migration scenarios. Adding a brief comment explaining why these specific tokens are checked (e.g.,
42703= PostgreSQL undefined_column,pgrst204= PostgREST schema cache) would help future maintainers understand the design intent.📝 Suggested documentation
+/** + * Detects errors caused by missing onboarding-related schema elements. + * Used for forward/backward compatibility during schema migrations. + * Tokens: column names (need_onboarding, existing_app, etc.), + * PostgreSQL 42703 (undefined_column), pgrst204 (PostgREST schema cache miss). + */ function isMissingOnboardingSchemaError(error: { message?: string, details?: string, hint?: string, code?: string | null } | null) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/app.ts` around lines 18 - 37, Add a concise comment above the isMissingOnboardingSchemaError function explaining the rationale for broad token-matching: state that we intentionally suppress onboarding/schema-migration errors by matching tokens like '42703' (Postgres undefined_column), 'pgrst204' (PostgREST schema cache), 'need_onboarding', 'existing_app', 'ios_store_url', 'android_store_url', and generic terms like 'column'/'schema cache' so future maintainers understand these map to schema migration/onboarding failures and why they are treated as non-fatal; include the mapping of each notable token to its meaning and a one-line note that this is deliberate to handle transient schema state during migrations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/api/app.ts`:
- Around line 18-37: Add a concise comment above the
isMissingOnboardingSchemaError function explaining the rationale for broad
token-matching: state that we intentionally suppress onboarding/schema-migration
errors by matching tokens like '42703' (Postgres undefined_column), 'pgrst204'
(PostgREST schema cache), 'need_onboarding', 'existing_app', 'ios_store_url',
'android_store_url', and generic terms like 'column'/'schema cache' so future
maintainers understand these map to schema migration/onboarding failures and why
they are treated as non-fatal; include the mapping of each notable token to its
meaning and a one-line note that this is deliberate to handle transient schema
state during migrations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1090dd01-0f83-4c06-a6ce-964ea1319418
📒 Files selected for processing (2)
src/api/app.tssrc/init.ts
This was referenced Mar 20, 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
1 participant
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 (AI generated)
initinstead of always creating a new appcap initwhen the pending app was created as a brand-new app from the web onboarding flowMotivation (AI generated)
The unified Capgo onboarding now starts in the web console, creates a pending app, and may seed demo data before the user switches to the CLI. The CLI needs to detect that pending app and continue onboarding from it, otherwise users get asked to create a second app and the flow becomes inconsistent.
Business Impact (AI generated)
This removes a major source of onboarding confusion and reduces drop-off between the web console and the CLI. Users can move from demo mode to real onboarding without duplicating apps, which makes first-time activation clearer and should improve conversion from exploration to real app setup.
Test Plan (AI generated)
bun run typecheckbun run lintcapgo initcap initis offered when the pending app was created as a new appGenerated with AI
Summary by CodeRabbit