-
-
Notifications
You must be signed in to change notification settings - Fork 131
fix(onboarding): seed demo data when skipping CLI setup #2854
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8fc0289
d09f3de
5587816
0e0a9d4
1cc10ff
d6d878e
febc1da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -34,7 +34,21 @@ archive_path="$tmp_dir/$asset_name" | |||||||||||||||||||||||||||||
| extract_path="$tmp_dir/extract" | ||||||||||||||||||||||||||||||
| asset_url="https://github.com/oven-sh/bun/releases/download/bun-v${BUN_VERSION}/${asset_name}" | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| curl -fsSL "$asset_url" -o "$archive_path" | ||||||||||||||||||||||||||||||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
|
||||||||||||||||||||||||||||||
| download_ok=1 | ||||||||||||||||||||||||||||||
| break | ||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||
| echo "Bun download failed (attempt ${attempt}/${download_attempts}), retrying..." >&2 | ||||||||||||||||||||||||||||||
| sleep $((attempt * 2)) | ||||||||||||||||||||||||||||||
|
Comment on lines
+43
to
+45
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Skip the delay after the final failed attempt. When 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
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+44
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 Prompt for AI agents
Suggested change
|
||||||||||||||||||||||||||||||
| done | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if [ "$download_ok" -ne 1 ]; then | ||||||||||||||||||||||||||||||
| echo "Failed to download Bun from $asset_url after ${download_attempts} attempts" >&2 | ||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if command -v shasum >/dev/null 2>&1; then | ||||||||||||||||||||||||||||||
| echo "$asset_sha256 $archive_path" | shasum -a 256 -c - | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -322,13 +322,60 @@ function isTransientDockerPortBindFailure(output: string): boolean { | |
| || /failed to bind host port/i.test(output) | ||
| } | ||
|
|
||
| /** | ||
| * Keep worktree host ports out of the kernel ephemeral pool. | ||
| * | ||
| * CI offsets land in 32k–60k, which overlaps Linux's default local port range. | ||
| * Without reserving them, outbound sockets can steal a Supabase port and make | ||
| * `docker` fail with "address already in use" on an otherwise idle runner. | ||
| */ | ||
| function reserveWorktreePortsFromEphemeralPool(repoRoot: string): void { | ||
| if (process.platform !== 'linux') | ||
| return | ||
| if (!process.env.SUPABASE_WORKTREE_PORT_OFFSET && !process.env.CI) | ||
| return | ||
|
|
||
| const { cfg } = ensureWorktreeSupabaseDir(repoRoot) | ||
| const ports = Object.values(cfg.ports).filter(port => Number.isFinite(port)).sort((a, b) => a - b) | ||
| if (ports.length === 0) | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Port reservation can unintentionally break other services on the runner/host because this call overwrites the entire Prompt for AI agents |
||
| 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}`) | ||
|
Comment on lines
+343
to
+356
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift Merge existing port reservations instead of replacing them.
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. (detect-child-process-typescript) 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Retry cleanup can kill unrelated processes because Prompt for AI agents |
||
| } | ||
|
Comment on lines
+359
to
+365
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift Restrict cleanup to Supabase-owned processes.
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. (detect-child-process-typescript) 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| /** | ||
| * `supabase start` can fail on GitHub runners with a transient Docker port bind | ||
| * (`address already in use`) after a partial start/stop. Retry only that class of | ||
| * failure so permanent start errors fail fast. | ||
| */ | ||
| function runSupabaseStartWithRetry(args: string[], repoRoot: string): number { | ||
| const maxAttempts = 3 | ||
| const { cfg } = ensureWorktreeSupabaseDir(repoRoot) | ||
| const ports = Object.values(cfg.ports).filter(port => Number.isFinite(port)) | ||
| reserveWorktreePortsFromEphemeralPool(repoRoot) | ||
|
|
||
| const maxAttempts = 5 | ||
| for (let attempt = 1; attempt <= maxAttempts; attempt++) { | ||
| const { status, output } = runSupabase(args, repoRoot, { captureOutput: true }) | ||
| if (status === 0) | ||
|
|
@@ -338,6 +385,7 @@ function runSupabaseStartWithRetry(args: string[], repoRoot: string): number { | |
| return status | ||
| console.error(`Supabase start hit a transient Docker port bind (attempt ${attempt}/${maxAttempts}); stopping and retrying...`) | ||
| runSupabase(['stop', '--no-backup'], repoRoot) | ||
| freeHostPorts(ports) | ||
| spawnSync(process.platform === 'win32' ? 'timeout' : 'sleep', process.platform === 'win32' ? ['/T', '2', '/NOBREAK'] : ['2']) | ||
| } | ||
| return 1 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -853,11 +853,10 @@ | |
| flowStep.value = 'install' | ||
| } | ||
|
|
||
| function openDashboard() { | ||
| if (!createdApp.value) | ||
| return | ||
|
|
||
| router.push(`/app/${encodeURIComponent(createdApp.value.app_id)}`) | ||
| async function openDashboard() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| // 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() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 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.
🤖 Prompt for AI AgentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: The "Open Dashboard" and "Install later" buttons previously always navigated to Prompt for AI agents |
||
| } | ||
|
|
||
| onMounted(async () => { | ||
|
|
@@ -1468,9 +1467,12 @@ | |
| </div> | ||
|
|
||
| <div class="flex flex-col-reverse gap-3 sm:flex-row sm:items-center sm:justify-end"> | ||
| <button class="d-btn min-h-11" :class="whiteCardPrimaryButtonClass()" @click="openDashboard"> | ||
| {{ t('app-onboarding-explore-dashboard') }} | ||
| <IconArrowRight class="h-4 w-4" /> | ||
| <button class="d-btn min-h-11" :class="whiteCardPrimaryButtonClass()" :disabled="isSeedingDemo" @click="openDashboard"> | ||
|
Check warning on line 1470 in src/components/dashboard/AppOnboardingFlow.vue
|
||
| <IconLoader v-if="isSeedingDemo" class="h-4 w-4 animate-spin" /> | ||
| <template v-else> | ||
| {{ t('app-onboarding-explore-dashboard') }} | ||
| <IconArrowRight class="h-4 w-4" /> | ||
| </template> | ||
|
Comment on lines
+1470
to
+1475
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Keep an accessible name while When 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 |
||
| </button> | ||
| </div> | ||
| </div> | ||
|
|
@@ -1599,12 +1601,15 @@ | |
| </div> | ||
|
|
||
| <div class="flex flex-col-reverse gap-3 sm:flex-row sm:items-center sm:justify-between"> | ||
| <button class="d-btn min-h-11" :class="whiteCardSecondaryButtonClass()" @click="flowStep = 'choice'"> | ||
| <button class="d-btn min-h-11" :class="whiteCardSecondaryButtonClass()" :disabled="isSeedingDemo" @click="flowStep = 'choice'"> | ||
|
Check warning on line 1604 in src/components/dashboard/AppOnboardingFlow.vue
|
||
| {{ t('button-back') }} | ||
| </button> | ||
| <button class="d-btn min-h-11" :class="whiteCardPrimaryButtonClass()" @click="openDashboard"> | ||
| {{ t('app-onboarding-explore-dashboard') }} | ||
| <IconArrowRight class="h-4 w-4" /> | ||
| <button class="d-btn min-h-11" :class="whiteCardPrimaryButtonClass()" :disabled="isSeedingDemo" @click="openDashboard"> | ||
|
Check warning on line 1607 in src/components/dashboard/AppOnboardingFlow.vue
|
||
| <IconLoader v-if="isSeedingDemo" class="h-4 w-4 animate-spin" /> | ||
| <template v-else> | ||
| {{ t('app-onboarding-explore-dashboard') }} | ||
| <IconArrowRight class="h-4 w-4" /> | ||
| </template> | ||
| </button> | ||
| </div> | ||
| </div> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: Cap-go/capgo.app
Length of output: 43523
🏁 Script executed (no clone):
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:
🏁 Script executed (no clone):
Length of output: 1807
🏁 Script executed:
Repository: Cap-go/capgo.app
Length of output: 350
Bound the
curloperation. Without transfer and retry limits, a connected server can stop sending data and keepcurlrunning indefinitely. Add--connect-timeout,--max-time, and--retry-max-timeso the retry loop can reach its final error.🤖 Prompt for AI Agents