Skip to content

[codex] Complete onboarding after first upload#2291

Merged
riderx merged 2 commits into
mainfrom
codex/complete-onboarding-after-upload
May 18, 2026
Merged

[codex] Complete onboarding after first upload#2291
riderx merged 2 commits into
mainfrom
codex/complete-onboarding-after-upload

Conversation

@riderx
Copy link
Copy Markdown
Member

@riderx riderx commented May 18, 2026

Summary (AI generated)

  • Added a database trigger that completes app onboarding once the first real bundle upload is ready.
  • Preserved the uploaded bundle while cleaning stale onboarding/demo rows.
  • Removed CLI init-time onboarding completion calls so backend owns the state transition, while keeping the helper available for compatibility.
  • Added regression coverage for R2 and external upload completion paths plus execute-grant hardening coverage.

Motivation (AI generated)

Apps created from the dashboard could stay stuck in onboarding when users skipped CLI init and uploaded from CI/CD. The onboarding flag was only cleared by CLI init, which made the app state depend on one specific setup path.

Business Impact (AI generated)

Users who create apps in the dashboard and upload from CI/CD now get moved into the normal app state automatically after their first upload. This reduces onboarding friction and prevents support issues caused by apps staying in setup mode after a successful upload.

Test Plan (AI generated)

  • bun lint
  • bun run lint:backend
  • bun run cli:lint
  • bun run cli:typecheck
  • bun run cli:build
  • bun run lint:deadcode
  • bun run supabase:db:reset
  • bun run supabase:with-env -- bunx vitest run tests/onboarding-first-upload.test.ts tests/security-definer-execute-hardening.test.ts

Screenshots (AI generated)

Not applicable; backend and CLI state-management change only.

Checklist (AI generated)

  • My code follows the code style of this project and passes relevant lint checks.
  • My change does not require customer-facing documentation changes.
  • My change has focused database regression coverage.
  • I tested the migration through a local Supabase reset.

Generated with AI

Summary by CodeRabbit

  • New Features

    • Onboarding now automatically completes after the first app version upload and cleans up onboarding data while preserving the uploaded version.
  • Bug Fixes

    • Initialization flows no longer always force-complete pending onboarding; completion is now conditional.
  • Tests

    • Added tests verifying onboarding completion behavior for different upload types.
  • Chores

    • Strengthened security checks for restricted database procedures.

Review Change Stack

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 18, 2026

Warning

Rate limit exceeded

@riderx has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minute and 56 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7e230f49-33c5-42c6-9640-b0630fbe3885

📥 Commits

Reviewing files that changed from the base of the PR and between b5024df and 7a99a47.

📒 Files selected for processing (1)
  • supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql
📝 Walkthrough

Walkthrough

Adds DB trigger functions that mark apps as onboarded on the first real app_version insert/update and clear onboarding data; updates the CLI to conditionally perform legacy-backend completion only when CAPGO_CLI_COMPLETE_ONBOARDING === 'true'; adds integration and security tests for the new procedures.

Changes

Onboarding automation and CLI gating

Layer / File(s) Summary
Database onboarding trigger system
supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql
Adds clear_onboarding_app_data(uuid, bigint) SECURITY DEFINER, overload without preservation, cleanup_onboarding_app_data_on_complete() trigger to call the clearer when need_onboarding flips, and complete_onboarding_after_first_upload() trigger to mark apps onboarded on first valid app_version upload; recreates the app_versions trigger with WHEN guards.
CLI: gate legacy completion
cli/src/init/command.ts
Replaces unconditional CLI onboarding completion calls with maybeCompletePendingOnboardingAppForLegacyBackend(...) and adds shouldCompleteOnboardingInCliForLegacyBackend() to only complete when CAPGO_CLI_COMPLETE_ONBOARDING === 'true'; updates reuse and conflict-resolution paths to use the new helper.
Integration tests: first-upload onboarding
tests/onboarding-first-upload.test.ts
Adds Vitest tests (transactional) verifying r2-direct and external/bundle upload flows complete onboarding, preserve external_url, and update last_version and version rows as expected.
Security allowlist update
tests/security-definer-execute-hardening.test.ts
Adds the two new public.clear_onboarding_app_data(...) signatures to the SERVICE_ONLY_PROCS allowlist so existing tests assert they are service-only.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Cap-go/capgo#1966: Related security-definer grants and tests for onboarding-clear procedures.

Poem

🐰 I hopped through SQL and CLI,

Triggers now finish onboarding sly,
Tests ensured the steps behave,
And cleardown procs the mess will save,
Hooray — a tidy app to try!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main feature: completing app onboarding after the first upload, which is the primary purpose of the changeset.
Description check ✅ Passed The description includes all required template sections (Summary, Test plan, Screenshots, Checklist) with comprehensive details about changes, testing performed, and migration validation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/complete-onboarding-after-upload

Comment @coderabbitai help to get the list of available commands and usage tips.

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 18, 2026

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing codex/complete-onboarding-after-upload (7a99a47) with main (22ea833)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@riderx riderx force-pushed the codex/complete-onboarding-after-upload branch from 557bc22 to b5024df Compare May 18, 2026 15:17
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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
`@supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql`:
- Around line 157-179: The function cleanup_onboarding_app_data_on_complete() is
declared but never attached to the apps table; add a trigger named
cleanup_onboarding_app_data_on_complete on public.apps so the function runs when
need_onboarding flips from TRUE to FALSE. Specifically, drop any existing
trigger with that name, then create an AFTER UPDATE OF need_onboarding trigger
ON public.apps FOR EACH ROW with a WHEN clause (OLD.need_onboarding IS TRUE AND
NEW.need_onboarding IS FALSE) that EXECUTE FUNCTION
public.cleanup_onboarding_app_data_on_complete(); this ensures the cleanup runs
on the intended transition.
🪄 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: eb3fc5b9-70b2-4559-ad0e-37fff6094401

📥 Commits

Reviewing files that changed from the base of the PR and between 70f5798 and 557bc22.

📒 Files selected for processing (4)
  • cli/src/init/command.ts
  • supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql
  • tests/onboarding-first-upload.test.ts
  • tests/security-definer-execute-hardening.test.ts

@sonarqubecloud
Copy link
Copy Markdown

@riderx riderx merged commit d824bb8 into main May 18, 2026
44 checks passed
@riderx riderx deleted the codex/complete-onboarding-after-upload branch May 18, 2026 16:05
WcaleNieWolny added a commit that referenced this pull request May 19, 2026
…ion apps

The earlier commit on this branch closed the auto-fire path (the
upload-driven trigger from PR #2291) but left a user-triggered path
open: `capgo init` against an existing app with `need_onboarding = TRUE`
still calls `completePendingOnboardingApp`, which flips the flag and
fires `cleanup_onboarding_app_data_on_complete`, which calls
`clear_onboarding_app_data` -- still wiping the app.

Because the exact cohort described in #2295 is "apps provisioned via
dashboard or CI without ever running `capgo init`," and the natural
follow-up for those users is to run `capgo init` and pick "use existing
app," that path needs a real safety check, not a circular "the only
caller is `capgo init` so it's fine" argument.

Add a production-safety guard at the top of
`clear_onboarding_app_data(uuid, bigint)`. It refuses (RAISE WARNING
plus early RETURN) when any of four independent indicators of real
production usage exists:

  * any row in `public.devices`
  * any row in `public.channel_devices`
  * any row in `public.deploy_history` beyond the preserved version
  * any channel whose `version` is set and != the preserved version

This is defense-in-depth at the function level: every present and
future caller of either overload (`uuid` or `uuid, bigint`) is now
protected, regardless of how the flag transition got triggered. Genuine
onboarding placeholder apps -- the original intent -- pass the guard
and are cleaned up as before.

Refs: #2295
WcaleNieWolny added a commit that referenced this pull request May 19, 2026
fix(onboarding): revert #2291 trigger that wipes production data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant