-
-
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 3 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 |
|---|---|---|
|
|
@@ -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 () => { | ||
|
|
@@ -1428,7 +1427,8 @@ | |
| {{ setupSubtitle }} | ||
| </p> | ||
| </div> | ||
| <button class="d-btn min-h-11" :class="whiteCardSecondaryButtonClass()" @click="openDashboard"> | ||
| <button class="d-btn min-h-11" :class="whiteCardSecondaryButtonClass()" :disabled="isSeedingDemo" @click="openDashboard"> | ||
|
Check warning on line 1430 in src/components/dashboard/AppOnboardingFlow.vue
|
||
| <IconLoader v-if="isSeedingDemo" class="h-4 w-4 animate-spin" /> | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
|
||
| {{ t('app-onboarding-open-dashboard') }} | ||
| </button> | ||
| </div> | ||
|
|
@@ -1473,9 +1473,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-install-later') }} | ||
| <IconArrowRight class="h-4 w-4" /> | ||
| <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> | ||
|
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> | ||
|
|
@@ -1565,7 +1568,8 @@ | |
| {{ t('app-onboarding-install-subtitle') }} | ||
| </p> | ||
| </div> | ||
| <button class="d-btn min-h-11" :class="whiteCardSecondaryButtonClass()" @click="openDashboard"> | ||
| <button class="d-btn min-h-11" :class="whiteCardSecondaryButtonClass()" :disabled="isSeedingDemo" @click="openDashboard"> | ||
|
Check warning on line 1571 in src/components/dashboard/AppOnboardingFlow.vue
|
||
| <IconLoader v-if="isSeedingDemo" class="h-4 w-4 animate-spin" /> | ||
| {{ t('app-onboarding-open-dashboard') }} | ||
| </button> | ||
| </div> | ||
|
|
@@ -1609,12 +1613,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 1616 in src/components/dashboard/AppOnboardingFlow.vue
|
||
| {{ t('button-back') }} | ||
| </button> | ||
| <button class="d-btn min-h-11" :class="whiteCardPrimaryButtonClass()" @click="openDashboard"> | ||
| {{ t('app-onboarding-install-later') }} | ||
| <IconArrowRight class="h-4 w-4" /> | ||
| <button class="d-btn min-h-11" :class="whiteCardPrimaryButtonClass()" :disabled="isSeedingDemo" @click="openDashboard"> | ||
|
Check warning on line 1619 in src/components/dashboard/AppOnboardingFlow.vue
|
||
| <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> | ||
| </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