fix(onboarding): seed demo data when skipping CLI setup - #2854
Conversation
Open Dashboard / Install later now call the same demo seeding path as the explore-demo choice, so skipping the CLI step still populates the pending app before landing on the dashboard. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughOnboarding navigation now seeds demo data before redirecting. Buttons show loading states and disable during seeding. Bun downloads retry failed attempts. Test concurrency groups include workflow event types. Supabase startup reserves ports and retries failed starts. ChangesOnboarding dashboard navigation
Bun download resilience
Test concurrency isolation
Supabase worktree startup
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant OnboardingFlow
participant seedDemoData
participant Dashboard
OnboardingFlow->>seedDemoData: Seed demo data
seedDemoData-->>OnboardingFlow: Return result
OnboardingFlow->>Dashboard: Navigate after seeding
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_5f0d972b-2118-4a57-97c2-8c547ee28d04) |
Previous Run tests jobs were cancelled by concurrency groups, not by test failures. Empty commit to get a clean CI pass. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_99e56942-7117-4966-ab53-5b60dafb9d49) |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/dashboard/AppOnboardingFlow.vue`:
- Around line 1476-1481: Update both install-later buttons in the onboarding
flow to retain an accessible “Install later” name while isSeedingDemo is true,
using a visually hidden translated label or aria-label. Mark each IconLoader as
aria-hidden because it is decorative, while preserving the existing loading and
non-loading visual behavior.
- Line 859: Update the openDashboard flow around seedDemoData() so the
router.push() navigation is awaited and its NavigationFailure result is
explicitly handled. Ensure isSeedingDemo remains active until navigation
settles, and preserve the existing behavior for successful navigation.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 14801dca-420b-417a-bd1e-22721facff22
📒 Files selected for processing (1)
src/components/dashboard/AppOnboardingFlow.vue
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
| async function openDashboard() { | ||
| // Skipping the CLI step should still populate the app with demo data | ||
| // so the dashboard is usable to explore Capgo before a real upload. | ||
| await seedDemoData() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'router\.push\(|beforeEach\(|beforeEnter|onBeforeRoute' \
--glob '*.{ts,tsx,vue,js}' || trueRepository: Cap-go/capgo.app
Length of output: 154
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- target file ---'
sed -n '760,875p' src/components/dashboard/AppOnboardingFlow.vue
printf '%s\n' '--- router navigation and guards ---'
rg -n -C 6 \
'router\.(push|replace)\(|beforeEach\(|beforeEnter|onBeforeRoute|isSeedingDemo|seedDemoData|openDashboard' \
src --glob '*.{ts,tsx,vue,js}' || trueRepository: Cap-go/capgo.app
Length of output: 50373
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('src/components/dashboard/AppOnboardingFlow.vue')
text = p.read_text()
for needle in ('async function seedDemoData', 'const seedDemoData', 'function seedDemoData', 'async function openDashboard', 'const openDashboard', 'function openDashboard'):
start = text.find(needle)
if start >= 0:
line = text.count('\n', 0, start) + 1
print(f'--- {needle} at line {line} ---')
print('\n'.join(f'{i}: {line_text}' for i, line_text in enumerate(text.splitlines()[max(0,line-12):line+55], max(1,line-11))))
PYRepository: Cap-go/capgo.app
Length of output: 7646
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- target route definition ---'
rg -n -C 12 \
"path: '/app|path: '/dashboard|meta:.*middleware|middleware:" \
src/main.ts src --glob '*.ts' --glob '*.vue' | head -n 240
printf '%s\n' '--- router guards ---'
sed -n '225,285p' src/main.ts
sed -n '420,475p' src/modules/auth.ts
sed -n '45,100p' src/modules/sso-enforcement.tsRepository: Cap-go/capgo.app
Length of output: 18672
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- all local uses of the target navigation result ---'
rg -n -C 4 \
'isNavigationFailure|NavigationFailure|await router\.(push|replace)|router\.(push|replace)\(.*\.catch|\.push\(.*\.then' \
src --glob '*.{ts,tsx,vue,js}' || true
printf '%s\n' '--- package versions ---'
rg -n '"vue-router"|"vue"' package.json bun.lockb bun.lock package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || trueRepository: Cap-go/capgo.app
Length of output: 35246
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for name in ('src/main.ts', 'src/modules/auth.ts', 'src/modules/sso-enforcement.ts'):
p = Path(name)
lines = p.read_text().splitlines()
print(f'--- {name}: relevant route/guard lines ---')
for i, line in enumerate(lines, 1):
if ('/app/' in line or 'middleware' in line or 'next(' in line or 'return false' in line
or 'return next' in line or 'router.beforeEach' in line):
lo, hi = max(1, i - 3), min(len(lines), i + 5)
print(f'[{i}]')
for j in range(lo, hi + 1):
print(f'{j}: {lines[j-1]}')
PYRepository: Cap-go/capgo.app
Length of output: 25386
Await and handle dashboard navigation.
seedDemoData() does not await router.push(). The auth guard can return next(false), so openDashboard() clears isSeedingDemo while navigation is still pending and silently ignores the navigation failure. Await the push and handle its NavigationFailure result.
🤖 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 `@src/components/dashboard/AppOnboardingFlow.vue` at line 859, Update the
openDashboard flow around seedDemoData() so the router.push() navigation is
awaited and its NavigationFailure result is explicitly handled. Ensure
isSeedingDemo remains active until navigation settles, and preserve the existing
behavior for successful navigation.
| <button class="d-btn min-h-11" :class="whiteCardPrimaryButtonClass()" :disabled="isSeedingDemo" @click="openDashboard"> | ||
| <IconLoader v-if="isSeedingDemo" class="h-4 w-4 animate-spin" /> | ||
| <template v-else> | ||
| {{ t('app-onboarding-install-later') }} | ||
| <IconArrowRight class="h-4 w-4" /> | ||
| </template> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep an accessible name while Install later is loading.
When isSeedingDemo is true, the v-else block removes the translated label and arrow. The button then contains only IconLoader, and the opening tag has no aria-label. Keep the original label in a visually hidden element or add an aria-label. Mark the decorative loader as aria-hidden.
Proposed fix
- <IconLoader v-if="isSeedingDemo" class="h-4 w-4 animate-spin" />
+ <IconLoader v-if="isSeedingDemo" class="h-4 w-4 animate-spin" aria-hidden="true" />
+ <span v-if="isSeedingDemo" class="sr-only">{{ t('app-onboarding-install-later') }}</span>
<template v-else>Apply the same change to both install-later buttons.
Also applies to: 1619-1624
🤖 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 `@src/components/dashboard/AppOnboardingFlow.vue` around lines 1476 - 1481,
Update both install-later buttons in the onboarding flow to retain an accessible
“Install later” name while isSeedingDemo is true, using a visually hidden
translated label or aria-label. Mark each IconLoader as aria-hidden because it
is decorative, while preserving the existing loading and non-loading visual
behavior.
setup-bun.sh failed CI when GitHub Releases returned 504. Add curl retries plus an outer download loop so SQL catalog and other jobs survive transient release CDN errors. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_201980c4-ff23-403f-aed4-788c0412035d) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Confidence score: 3/5
- In
src/components/dashboard/AppOnboardingFlow.vue, moving both CTA paths to rely fully onseedDemoData()risks blocking users from reaching/app/:app_idwhen demo seeding does not complete, creating a visible onboarding dead end—restore an unconditional dashboard navigation fallback (or handle non-demo paths explicitly). - In
src/components/dashboard/AppOnboardingFlow.vue(openDashboard()/seedDemoData()), navigation triggered insideseedDemoData()appears not to be awaited, so router failures (like guards cancelling) can be swallowed and leave users with no feedback—return and await therouter.push()promise and surface/handle rejected navigation outcomes.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/dashboard/AppOnboardingFlow.vue">
<violation number="1" location="src/components/dashboard/AppOnboardingFlow.vue:856">
P2: openDashboard() awaits seedDemoData() but if seedDemoData() internally calls router.push() without awaiting/handling the result, navigation failures (e.g. an auth guard returning next(false)) will be silently ignored and isSeedingDemo may be cleared before navigation completes. Ensure the router.push() call is awaited and its NavigationFailure result is checked.</violation>
<violation number="2" location="src/components/dashboard/AppOnboardingFlow.vue:859">
P2: The "Open Dashboard" and "Install later" buttons previously always navigated to `/app/:app_id`, so the user reach the dashboard regardless of backend state. Now they delegate entirely to `seedDemoData()`, which only calls `router.push("...?refresh=true")` on the success path. If the `app/demo` invocation fails (network/backend error) or short-circuits (e.g. `currentOrg?.gid` missing), the function just logs, fires a toast, and resets the spinner — the user stays stuck on the CLI onboarding step with no way to get to their dashboard. This is a UX regression in the failure path for a button whose primary purpose is to open the dashboard. Consider falling back to direct navigation when seeding cannot complete, or having `seedDemoData` return a success flag and navigating here regardless.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| async function openDashboard() { | ||
| // Skipping the CLI step should still populate the app with demo data | ||
| // so the dashboard is usable to explore Capgo before a real upload. | ||
| await seedDemoData() |
There was a problem hiding this comment.
P2: The "Open Dashboard" and "Install later" buttons previously always navigated to /app/:app_id, so the user reach the dashboard regardless of backend state. Now they delegate entirely to seedDemoData(), which only calls router.push("...?refresh=true") on the success path. If the app/demo invocation fails (network/backend error) or short-circuits (e.g. currentOrg?.gid missing), the function just logs, fires a toast, and resets the spinner — the user stays stuck on the CLI onboarding step with no way to get to their dashboard. This is a UX regression in the failure path for a button whose primary purpose is to open the dashboard. Consider falling back to direct navigation when seeding cannot complete, or having seedDemoData return a success flag and navigating here regardless.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/dashboard/AppOnboardingFlow.vue, line 859:
<comment>The "Open Dashboard" and "Install later" buttons previously always navigated to `/app/:app_id`, so the user reach the dashboard regardless of backend state. Now they delegate entirely to `seedDemoData()`, which only calls `router.push("...?refresh=true")` on the success path. If the `app/demo` invocation fails (network/backend error) or short-circuits (e.g. `currentOrg?.gid` missing), the function just logs, fires a toast, and resets the spinner — the user stays stuck on the CLI onboarding step with no way to get to their dashboard. This is a UX regression in the failure path for a button whose primary purpose is to open the dashboard. Consider falling back to direct navigation when seeding cannot complete, or having `seedDemoData` return a success flag and navigating here regardless.</comment>
<file context>
@@ -853,11 +853,10 @@ function goToInstallStep() {
+async function openDashboard() {
+ // Skipping the CLI step should still populate the app with demo data
+ // so the dashboard is usable to explore Capgo before a real upload.
+ await seedDemoData()
}
</file context>
| return | ||
|
|
||
| router.push(`/app/${encodeURIComponent(createdApp.value.app_id)}`) | ||
| async function openDashboard() { |
There was a problem hiding this comment.
P2: openDashboard() awaits seedDemoData() but if seedDemoData() internally calls router.push() without awaiting/handling the result, navigation failures (e.g. an auth guard returning next(false)) will be silently ignored and isSeedingDemo may be cleared before navigation completes. Ensure the router.push() call is awaited and its NavigationFailure result is checked.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/dashboard/AppOnboardingFlow.vue, line 856:
<comment>openDashboard() awaits seedDemoData() but if seedDemoData() internally calls router.push() without awaiting/handling the result, navigation failures (e.g. an auth guard returning next(false)) will be silently ignored and isSeedingDemo may be cleared before navigation completes. Ensure the router.push() call is awaited and its NavigationFailure result is checked.</comment>
<file context>
@@ -853,11 +853,10 @@ function goToInstallStep() {
- return
-
- router.push(`/app/${encodeURIComponent(createdApp.value.app_id)}`)
+async function openDashboard() {
+ // Skipping the CLI step should still populate the app with demo data
+ // so the dashboard is usable to explore Capgo before a real upload.
</file context>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/setup-bun.sh`:
- Around line 43-45: Update the retry handling around the download attempt loop
so the “retrying...” message and backoff sleep execute only when attempt is less
than download_attempts. Preserve the final failure reporting path without
delaying after the last failed attempt.
- Line 40: Update the curl invocation in the setup script to include bounded
connection, total operation, and cumulative retry durations via
--connect-timeout, --max-time, and --retry-max-time. Keep the existing retry
behavior and archive download arguments unchanged, using appropriate finite
values so stalled connections and retries cannot run indefinitely.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 40731ee7-b6ea-4138-b0f4-55a0b31f541a
📒 Files selected for processing (1)
scripts/setup-bun.sh
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
| download_attempts=5 | ||
| download_ok=0 | ||
| for attempt in $(seq 1 "$download_attempts"); do | ||
| if curl --retry 3 --retry-delay 2 --retry-all-errors -fsSL "$asset_url" -o "$archive_path"; then |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- scripts/setup-bun.sh ---'
cat -n scripts/setup-bun.sh
printf '%s\n' '--- related curl and invocation context ---'
rg -n -C 4 'setup-bun|curl|retry|max-time|connect-timeout|archive_path|asset_url' scripts package.json .github 2>/dev/null || trueRepository: Cap-go/capgo.app
Length of output: 43523
🏁 Script executed (no clone):
#!/bin/bash
set -eu
printf '%s\n' '--- curl version and timeout option descriptions ---'
curl --version | head -n 2
curl --help all | rg -n -A 2 -- '--connect-timeout|--max-time|--retry-max-time|--retry-all-errors|--retry-delay'
printf '%s\n' '--- static verifier for the setup-bun download command ---'
python3 - <<'PY'
from pathlib import Path
import re
path = Path("/workspace/scripts/setup-bun.sh")
if not path.exists():
path = Path("scripts/setup-bun.sh")
text = path.read_text()
match = re.search(r'^\s*if (curl .+); then$', text, re.MULTILINE)
if not match:
raise SystemExit("download curl command not found")
command = match.group(1)
options = set(re.findall(r'(--[a-z-]+)(?:=\S+|\s+\S+)?', command))
print(command)
for option in ("--connect-timeout", "--max-time", "--retry-max-time"):
print(f"{option}: {'present' if option in options else 'absent'}")
print(f"outer attempts: {re.search(r'download_attempts=(\\d+)', text).group(1)}")
print(f"curl retry count: {re.search(r'--retry\\s+(\\d+)', command).group(1)}")
PY
Length of output: 1539
🌐 Web query:
curl man page --connect-timeout --max-time --retry-max-time transfer stalls no timeout
💡 Result:
To manage curl behavior regarding connection, transfer duration, and stalls, it is important to understand the specific scope of each option. --connect-timeout This option strictly limits the duration of the connection phase, which includes DNS resolution, TCP connection, and TLS/QUIC handshakes [1][2][3]. If the connection is not established within this time, curl aborts with a timeout error (exit code 28) [2][4]. It does not impact the time taken for the subsequent data transfer [3]. --max-time This option sets the maximum allowed duration for the entire transfer operation [2][5]. Once this time is reached, curl will terminate the operation regardless of whether it is still transferring data [2][6]. If used with --retry, the --max-time limit is reset for each retry attempt [7][5]. --retry-max-time When using the --retry option, this sets a total time limit for all retry attempts combined [7]. The timer includes the time spent transferring data and the wait times (sleep) between retries [8][9]. Retries continue until either the maximum number of retries is reached or this total time limit is exceeded [7]. Handling Stalled Transfers If a transfer "stalls" (i.e., the connection is successful but data transfer is extremely slow or stops entirely), neither --connect-timeout nor --max-time may be sufficient if you need to detect inactivity without killing long-running successful transfers [3][10][11]. To handle stalls, consider these options: 1. --speed-limit <bytes/second> and --speed-time : These options abort the transfer if the speed remains below a specified rate (speed-limit) for a specific duration (speed-time) [10]. The default speed-time is 30 seconds [10]. 2. --keepalive-time : If you suspect the connection is being dropped due to inactivity, this option helps by sending keep-alive probes on the connection [3]. In cases where libcurl hangs indefinitely, it is often recommended to ensure both --max-time (or similar transfer timeouts) and appropriate low-speed limits are configured to catch silent failures [12][11][13].
Citations:
- 1: https://man7.org/linux/man-pages/man1/curl.1.html
- 2: https://everything.curl.dev/usingcurl/timeouts.html
- 3: https://daniel.haxx.se/blog/2020/06/23/
- 4: https://man.archlinux.org/man/curl.1
- 5: https://github.com/curl/curl/blob/c37405cb/docs/cmdline-opts/max-time.md
- 6: https://github.com/curl/everything-curl/blob/master/usingcurl/timeouts.md
- 7: https://everything.curl.dev/usingcurl/downloads/retry.html
- 8: docs: clarify retry-max-time timing check and sleep behavior curl/curl#21411
- 9: docs: clarify retry-max-time timing check and sleep behavior curl/curl#21411
- 10: https://stackoverflow.com/questions/47434251/how-to-timeout-established-curl-connection-apart-from-max-time
- 11: https://curl.se/mail/archive-2016-02/0024.html
- 12: libcurl can hang indefinitely instead of failing or retrying. curl/curl#11873
- 13: https://curl.se/mail/lib-2010-08/0350.html
🏁 Script executed (no clone):
#!/bin/bash
set -eu
printf '%s\n' '--- curl version and timeout option descriptions ---'
curl --version | head -n 2
curl --help all | rg -n -A 2 -- '--connect-timeout|--max-time|--retry-max-time|--retry-all-errors|--retry-delay'
printf '%s\n' '--- static verifier for the setup-bun download command ---'
python3 - <<'PY'
from pathlib import Path
import re
path = Path("scripts/setup-bun.sh")
text = path.read_text()
match = re.search(r'^\s*if (curl .+); then$', text, re.MULTILINE)
if not match:
raise SystemExit("download curl command not found")
command = match.group(1)
options = set(re.findall(r'(--[a-z-]+)(?:=\S+|\s+\S+)?', command))
attempts = re.search(r'download_attempts=(\d+)', text)
retries = re.search(r'--retry\s+(\d+)', command)
print(command)
for option in ("--connect-timeout", "--max-time", "--retry-max-time"):
print(f"{option}: {'present' if option in options else 'absent'}")
print(f"outer attempts: {attempts.group(1)}")
print(f"curl retry count: {retries.group(1)}")
PY
Length of output: 1807
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
text = Path("scripts/setup-bun.sh").read_text()
match = re.search(r'^\s*if (curl .+); then$', text, re.MULTILINE)
if not match:
raise SystemExit("download curl command not found")
command = match.group(1)
options = set(re.findall(r'(--[a-z-]+)(?:=\S+|\s+\S+)?', command))
attempts = re.search(r'download_attempts=(\d+)', text)
retries = re.search(r'--retry\s+(\d+)', command)
print(command)
for option in ("--connect-timeout", "--max-time", "--retry-max-time"):
print(f"{option}: {'present' if option in options else 'absent'}")
print("outer attempts:", attempts.group(1))
print("curl retry count:", retries.group(1))
PYRepository: Cap-go/capgo.app
Length of output: 350
Bound the curl operation. Without transfer and retry limits, a connected server can stop sending data and keep curl running indefinitely. Add --connect-timeout, --max-time, and --retry-max-time so the retry loop can reach its final error.
🤖 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 `@scripts/setup-bun.sh` at line 40, Update the curl invocation in the setup
script to include bounded connection, total operation, and cumulative retry
durations via --connect-timeout, --max-time, and --retry-max-time. Keep the
existing retry behavior and archive download arguments unchanged, using
appropriate finite values so stalled connections and retries cannot run
indefinitely.
| fi | ||
| echo "Bun download failed (attempt ${attempt}/${download_attempts}), retrying..." >&2 | ||
| sleep $((attempt * 2)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Skip the delay after the final failed attempt.
When attempt equals download_attempts, Lines 44-45 still print retrying... and sleep for 10 seconds. Line 48 then reports the final failure. Only log and sleep when attempt is less than download_attempts.
Proposed fix
fi
- echo "Bun download failed (attempt ${attempt}/${download_attempts}), retrying..." >&2
- sleep $((attempt * 2))
+ if [ "$attempt" -lt "$download_attempts" ]; then
+ echo "Bun download failed (attempt ${attempt}/${download_attempts}), retrying..." >&2
+ sleep $((attempt * 2))
+ fi
done📝 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.
| fi | |
| echo "Bun download failed (attempt ${attempt}/${download_attempts}), retrying..." >&2 | |
| sleep $((attempt * 2)) | |
| fi | |
| if [ "$attempt" -lt "$download_attempts" ]; then | |
| echo "Bun download failed (attempt ${attempt}/${download_attempts}), retrying..." >&2 | |
| sleep $((attempt * 2)) | |
| fi |
🤖 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 `@scripts/setup-bun.sh` around lines 43 - 45, Update the retry handling around
the download attempt loop so the “retrying...” message and backoff sleep execute
only when attempt is less than download_attempts. Preserve the final failure
reporting path without delaying after the last failed attempt.
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Confidence score: 3/5
- In
scripts/setup-bun.sh, the unboundedcurlcall can hang indefinitely if the server stalls mid-transfer, which can block CI jobs and bypass the intended retry behavior — add--connect-timeoutand--max-time(and keep retries bounded). - In
scripts/setup-bun.sh, the retry loop still logs "retrying..." and sleeps after the final attempt, which adds avoidable CI delay and misleading output when failure is already final — gate the retry message/backoff so they only run when another attempt remains.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/setup-bun.sh">
<violation number="1" location="scripts/setup-bun.sh:40">
P2: This curl call has no --connect-timeout/--max-time bounds, so a server that stops sending data mid-transfer can hang indefinitely, bypassing the retry loop entirely. Consider adding --connect-timeout, --max-time, and --retry-max-time alongside the existing --retry flags.</violation>
<violation number="2" location="scripts/setup-bun.sh:44">
P3: The final failed attempt still prints "retrying..." and sleeps for 10 seconds even though the loop is exhausted, unnecessarily delaying CI failure and misleading the logs. Restrict the message and backoff to attempts before `download_attempts`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| download_attempts=5 | ||
| download_ok=0 | ||
| for attempt in $(seq 1 "$download_attempts"); do | ||
| if curl --retry 3 --retry-delay 2 --retry-all-errors -fsSL "$asset_url" -o "$archive_path"; then |
There was a problem hiding this comment.
P2: This curl call has no --connect-timeout/--max-time bounds, so a server that stops sending data mid-transfer can hang indefinitely, bypassing the retry loop entirely. Consider adding --connect-timeout, --max-time, and --retry-max-time alongside the existing --retry flags.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/setup-bun.sh, line 40:
<comment>This curl call has no --connect-timeout/--max-time bounds, so a server that stops sending data mid-transfer can hang indefinitely, bypassing the retry loop entirely. Consider adding --connect-timeout, --max-time, and --retry-max-time alongside the existing --retry flags.</comment>
<file context>
@@ -34,7 +34,21 @@ archive_path="$tmp_dir/$asset_name"
+download_attempts=5
+download_ok=0
+for attempt in $(seq 1 "$download_attempts"); do
+ if curl --retry 3 --retry-delay 2 --retry-all-errors -fsSL "$asset_url" -o "$archive_path"; then
+ download_ok=1
+ break
</file context>
| if curl --retry 3 --retry-delay 2 --retry-all-errors -fsSL "$asset_url" -o "$archive_path"; then | |
| if curl --connect-timeout 10 --max-time 60 --retry 3 --retry-delay 2 --retry-all-errors --retry-max-time 60 -fsSL "$asset_url" -o "$archive_path"; then |
| echo "Bun download failed (attempt ${attempt}/${download_attempts}), retrying..." >&2 | ||
| sleep $((attempt * 2)) |
There was a problem hiding this comment.
P3: The final failed attempt still prints "retrying..." and sleeps for 10 seconds even though the loop is exhausted, unnecessarily delaying CI failure and misleading the logs. Restrict the message and backoff to attempts before download_attempts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/setup-bun.sh, line 44:
<comment>The final failed attempt still prints "retrying..." and sleeps for 10 seconds even though the loop is exhausted, unnecessarily delaying CI failure and misleading the logs. Restrict the message and backoff to attempts before `download_attempts`.</comment>
<file context>
@@ -34,7 +34,21 @@ archive_path="$tmp_dir/$asset_name"
+ download_ok=1
+ break
+ fi
+ echo "Bun download failed (attempt ${attempt}/${download_attempts}), retrying..." >&2
+ sleep $((attempt * 2))
+done
</file context>
| echo "Bun download failed (attempt ${attempt}/${download_attempts}), retrying..." >&2 | |
| sleep $((attempt * 2)) | |
| if [ "$attempt" -lt "$download_attempts" ]; then | |
| echo "Bun download failed (attempt ${attempt}/${download_attempts}), retrying..." >&2 | |
| sleep $((attempt * 2)) | |
| fi |
Job-level local-services concurrency groups were shared across push and pull_request. With cancel-in-progress false, a newer pending request still cancels an older pending job in the same group, which repeatedly cancelled Playwright/backend shards on this PR. Mirror the workflow-level event_name isolation so push and PR no longer fight for the same slots. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6f690db0-647d-4dba-83e1-1838b68b38c0) |
Cloudflare shard 5/8 hit a transient Docker host-port bind on a fresh ubuntu-latest runner. Playwright and SQL catalog already passed. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6d022982-d1af-4e72-8cf8-52b1e2c6b71b) |
CI worktree offsets land in Linux's default local port range, so outbound sockets can steal a DB/API port and fail supabase start with address already in use. Reserve the worktree ports, free them on retry, and bump start attempts to 5. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_3ed3c30e-58b0-4d23-b767-252383acccc8) |
Keep explore-dashboard CTA copy from main and seed demo data when skipping the CLI step. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_d2632600-82a5-459c-ba55-e4bcf48c8e6b) |
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Confidence score: 2/5
- In
scripts/supabase-worktree.ts, theip_local_reserved_portsupdate appears to overwrite the full reserved-port list, which can silently remove existing reservations and disrupt unrelated services on the host/runner — read the current value and merge in only the worktree ports before writing it back. - In
scripts/supabase-worktree.ts, retry cleanup uses unconditionalfuser -kper port, so unrelated processes bound to those ports may be terminated and cause flaky or broken jobs outside this workflow — restrict kills to known Supabase/Docker PIDs (or avoid host-wide kills when not in CI).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/supabase-worktree.ts">
<violation number="1" location="scripts/supabase-worktree.ts:344">
P1: Port reservation can unintentionally break other services on the runner/host because this call overwrites the entire `ip_local_reserved_ports` list instead of appending the worktree ports. Consider reading the current sysctl value and writing a merged deduplicated list.</violation>
<violation number="2" location="scripts/supabase-worktree.ts:364">
P2: Retry cleanup can kill unrelated processes because `fuser -k` is unconditional for each port. Limiting termination to known Supabase/Docker-owned PIDs (or skipping host-wide kills outside CI) would avoid collateral process shutdowns.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return | ||
|
|
||
| const reserved = ports.join(',') | ||
| const result = spawnSync('sudo', ['sysctl', '-w', `net.ipv4.ip_local_reserved_ports=${reserved}`], { |
There was a problem hiding this comment.
P1: Port reservation can unintentionally break other services on the runner/host because this call overwrites the entire ip_local_reserved_ports list instead of appending the worktree ports. Consider reading the current sysctl value and writing a merged deduplicated list.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/supabase-worktree.ts, line 344:
<comment>Port reservation can unintentionally break other services on the runner/host because this call overwrites the entire `ip_local_reserved_ports` list instead of appending the worktree ports. Consider reading the current sysctl value and writing a merged deduplicated list.</comment>
<file context>
@@ -322,13 +322,60 @@ function isTransientDockerPortBindFailure(output: string): boolean {
+ return
+
+ const reserved = ports.join(',')
+ const result = spawnSync('sudo', ['sysctl', '-w', `net.ipv4.ip_local_reserved_ports=${reserved}`], {
+ encoding: 'utf8',
+ })
</file context>
| return | ||
|
|
||
| for (const port of ports) { | ||
| spawnSync('fuser', ['-k', `${port}/tcp`], { stdio: 'ignore' }) |
There was a problem hiding this comment.
P2: Retry cleanup can kill unrelated processes because fuser -k is unconditional for each port. Limiting termination to known Supabase/Docker-owned PIDs (or skipping host-wide kills outside CI) would avoid collateral process shutdowns.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/supabase-worktree.ts, line 364:
<comment>Retry cleanup can kill unrelated processes because `fuser -k` is unconditional for each port. Limiting termination to known Supabase/Docker-owned PIDs (or skipping host-wide kills outside CI) would avoid collateral process shutdowns.</comment>
<file context>
@@ -322,13 +322,60 @@ function isTransientDockerPortBindFailure(output: string): boolean {
+ return
+
+ for (const port of ports) {
+ spawnSync('fuser', ['-k', `${port}/tcp`], { stdio: 'ignore' })
+ }
+}
</file context>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/supabase-worktree.ts (1)
383-388: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClean up after the final failed attempt.
When the fifth attempt fails with a transient bind error,
canRetryis false becauseattempt < maxAttemptsis false. Line [385] returns before Lines [387-388] stop the stack and free its ports.A partial stack can keep ports occupied and break the next invocation. Move failed-start cleanup into a shared failure path that also runs before the final non-zero return.
🤖 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 `@scripts/supabase-worktree.ts` around lines 383 - 388, Update the retry failure handling around canRetry in the Supabase start flow so transient Docker port-bind failures always run runSupabase(['stop', '--no-backup'], repoRoot) and freeHostPorts(ports), including when attempt reaches maxAttempts. Keep retry logging and retry behavior for attempts that can continue, while ensuring the final failed attempt performs cleanup before returning its non-zero status.
🤖 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 `@scripts/supabase-worktree.ts`:
- Around line 343-356: Update the port reservation flow around the existing
sudo/sysctl calls to read the current net.ipv4.ip_local_reserved_ports value,
merge it with ports, and write the combined unique reservation list instead of
replacing existing entries. Serialize the read/modify/write sequence across
concurrent starts, while preserving the existing sudo fallback and failure
handling.
- Around line 359-365: Update freeHostPorts to avoid blindly using fuser -k on
shared ports: track the PIDs created by the current Supabase start or inspect
each port’s processes and verify they belong to the failed Supabase stack before
sending termination signals, while preserving the existing Windows and
empty-port early returns.
---
Outside diff comments:
In `@scripts/supabase-worktree.ts`:
- Around line 383-388: Update the retry failure handling around canRetry in the
Supabase start flow so transient Docker port-bind failures always run
runSupabase(['stop', '--no-backup'], repoRoot) and freeHostPorts(ports),
including when attempt reaches maxAttempts. Keep retry logging and retry
behavior for attempts that can continue, while ensuring the final failed attempt
performs cleanup before returning its non-zero status.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6346200d-7727-48ea-93fc-a3425c457b73
📒 Files selected for processing (1)
scripts/supabase-worktree.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
| const reserved = ports.join(',') | ||
| const result = spawnSync('sudo', ['sysctl', '-w', `net.ipv4.ip_local_reserved_ports=${reserved}`], { | ||
| encoding: 'utf8', | ||
| }) | ||
| if ((result.status ?? 1) !== 0) { | ||
| const fallback = spawnSync('sysctl', ['-w', `net.ipv4.ip_local_reserved_ports=${reserved}`], { | ||
| encoding: 'utf8', | ||
| }) | ||
| if ((fallback.status ?? 1) !== 0) { | ||
| console.warn(`Could not reserve Supabase ports from the ephemeral pool: ${reserved}`) | ||
| return | ||
| } | ||
| } | ||
| console.error(`Reserved Supabase worktree ports from ephemeral pool: ${reserved}`) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Merge existing port reservations instead of replacing them.
sysctl -w net.ipv4.ip_local_reserved_ports=${reserved} assigns the complete reservation list. It removes reservations configured by another worktree or by the runner. Concurrent starts can also remove each other’s ports.
Read and merge the current value. Serialize the read/modify/write operation across concurrent starts.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@scripts/supabase-worktree.ts` around lines 343 - 356, Update the port
reservation flow around the existing sudo/sysctl calls to read the current
net.ipv4.ip_local_reserved_ports value, merge it with ports, and write the
combined unique reservation list instead of replacing existing entries.
Serialize the read/modify/write sequence across concurrent starts, while
preserving the existing sudo fallback and failure handling.
| function freeHostPorts(ports: number[]): void { | ||
| if (process.platform === 'win32' || ports.length === 0) | ||
| return | ||
|
|
||
| for (const port of ports) { | ||
| spawnSync('fuser', ['-k', `${port}/tcp`], { stdio: 'ignore' }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Restrict cleanup to Supabase-owned processes.
fuser -k terminates every process that uses each <port>/tcp. The code does not check process ownership. A stale process from another worktree or an unrelated service can be terminated during a retry.
Track the PIDs created by this start, or verify that each PID belongs to the failed Supabase stack before sending a signal.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@scripts/supabase-worktree.ts` around lines 359 - 365, Update freeHostPorts to
avoid blindly using fuser -k on shared ports: track the PIDs created by the
current Supabase start or inspect each port’s processes and verify they belong
to the failed Supabase stack before sending termination signals, while
preserving the existing Windows and empty-port early returns.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/dashboard/AppOnboardingFlow.vue (1)
1259-1260: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse regular spaces in copied CLI commands.
inserts U+00A0 into the command text. If a user copies the command, a POSIX shell can treat the subcommand and API key as one token. Use regular spaces in the DOM and control wrapping withwhitespace-nowrapor another visual rule.Proposed fix
-<span class="font-bold text-violet-300"> {{ cliSubcommand }}</span> -<span class="text-emerald-300"> {{ apiKey ?? '[APIKEY]' }}</span> +<span class="font-bold text-violet-300">{{ ' ' }}{{ cliSubcommand }}</span> +<span class="text-emerald-300">{{ ' ' }}{{ apiKey ?? '[APIKEY]' }}</span>Apply the same change to all three command renderings.
Also applies to: 1443-1444, 1577-1578
🤖 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 `@src/components/dashboard/AppOnboardingFlow.vue` around lines 1259 - 1260, Replace the non-breaking-space entities separating cliSubcommand and apiKey in all three command renderings, including the instances near the other referenced locations, with regular spaces so copied commands contain separate shell tokens. Preserve the no-wrap appearance using whitespace-nowrap or an equivalent styling rule.
🤖 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.
Outside diff comments:
In `@src/components/dashboard/AppOnboardingFlow.vue`:
- Around line 1259-1260: Replace the non-breaking-space entities separating
cliSubcommand and apiKey in all three command renderings, including the
instances near the other referenced locations, with regular spaces so copied
commands contain separate shell tokens. Preserve the no-wrap appearance using
whitespace-nowrap or an equivalent styling rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8213b36e-76bf-4c22-9b8a-ec7919880fd1
📒 Files selected for processing (1)
src/components/dashboard/AppOnboardingFlow.vue
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
2 participants
Summary (AI generated)
scripts/setup-bun.shwith download retries so transient GitHub Releases 504s do not fail CI jobs.capgo-local-services-*concurrency groups bygithub.event_nameso push + pull_request no longer cancel each other's pending Playwright/backend shards.address already in usefailures.mainand resolvedAppOnboardingFlow.vueconflicts.Motivation (AI generated)
When a user skips the CLI onboarding step and goes to the dashboard, the app was empty. Demo data should be fed automatically so they can explore Capgo without uploading a real bundle first.
CI was also failing for infrastructure reasons unrelated to the Vue change:
curl504 from GitHub Releases) with no retry loop.main(DIRTY), which blocked clean PR checks.Business Impact (AI generated)
New users who skip CLI setup still land on a populated dashboard, which improves first-session understanding of Capgo and reduces drop-off after onboarding. More stable CI reduces false-negative PR blockers.
Test Plan (AI generated)
app/demois invoked and the app dashboard shows demo versions/channels/devicesRun backend SQL catalog checkspasses (Bun download retries)Generated with AI
Summary by CodeRabbit
New Features
Bug Fixes