fix(ci): stop flaky shard reds and cut wasted runner minutes - #2915
Conversation
Update admin-stats trial fixture expectations after invite-org seeding, gate heavy Docker/CF jobs behind lint, fail-fast shard matrices, run plugin hot-path tests serially, harden worktree port cleanup, and warm edge/plugin endpoints before assertions. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughCI now gates test jobs on lint and dead-code checks, adds serial plugin jobs, warms edge endpoints, reduces concurrency, and improves failure diagnostics. Supabase worktree setup now reserves Cloudflare ports and cleans up containers and sockets before retries. ChangesCI stabilization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant Supabase
participant CloudflareWorker
participant Vitest
GitHubActions->>Supabase: provision isolated stack
GitHubActions->>CloudflareWorker: start worker
GitHubActions->>CloudflareWorker: warm plugin endpoints
GitHubActions->>Vitest: run serial plugin tests
Vitest->>Supabase: execute integration requests
Vitest->>CloudflareWorker: execute plugin requests
GitHubActions-->>GitHubActions: upload logs and clean up services
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Drop unused Severity import that broke oxlint, and stop resolving the supabase package installer stub as the CLI while bun install is still running in parallel with worktree start. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Expand edge warm-up beyond cron_email, warm cron_stat_org/app in beforeAll, log gateway 502/503 from fetchTestRequest, and lower vitest maxConcurrency to 2 so cold Deno isolates stop failing first trigger POSTs. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Move updates/stats/channel_self off the backend shard matrix onto a dedicated serial job with endpoint warm-up, matching the Cloudflare plugin isolation that already stopped workerd 503 flakes. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Keep the invite-org fixture comment and shared toBe(4) expectations from main, and restore cron_stat_app edge warm-up. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
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_c52a8b14-2c89-44db-ab1c-8071ee2b7314) |
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_8200e646-5020-475e-9352-dd22effac958) |
Cloudflare shard 5 failed after supabase eventually started: parallel bun install was OOM-killed during port-bind retries. Free leftover Docker publish holders harder, back off between bind retries, retry deps after start, and fix AdminFilterBar LocationQueryRaw typing from main merge. 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_4c9e30c9-c98b-44f2-a8a5-54fb99047754) |
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_36f3b518-6919-4065-8018-1b85660216ff) |
![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: 4/5
- In
.github/workflows/tests.yml, the plugin job test globs (tests/updates*.test.ts,tests/stats*.test.ts,tests/channel_self*.test.ts) currently pull in broader DB/unit coverage instead of just the intended hot-path integration set, which can inflate CI time and hide the signal from plugin-specific failures—tighten the glob patterns to only the target integration tests. - In
.github/workflows/tests.yml, using a static cache key (key: my_cache_key) for the serial backend plugin job risks stale or cross-job cache reuse because GitHub cache entries are immutable, which can cause inconsistent dependency state and flaky CI outcomes—switch to a versioned/content-based key (with restore keys if needed).
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=".github/workflows/tests.yml">
<violation number="1" location=".github/workflows/tests.yml:481">
P3: The new serial backend plugin job caches the Deno directory under a static `key: my_cache_key`. Because GitHub caches are immutable per key, this entry is written once (by whichever backend job wins the race) and then restored forever — it never refreshes when `supabase/functions/deno.lock` or the runtime version changes, and all 6 shards + the new plugin job resolve to the same frozen cache. This can silently run tests against an outdated Deno dependency set. Consider keying the cache on a dependency hash (e.g. include the deno.lock hash) or at least add `restore-keys`, so the cache is refreshed when dependencies change.</violation>
<violation number="2" location=".github/workflows/tests.yml:896">
P2: The new plugin jobs rerun DB-only and unit suites instead of only the hot-path integration tests: both `tests/updates*.test.ts tests/stats*.test.ts tests/channel_self*.test.ts` globs include `stats-export-cors.test.ts`, and `test:cloudflare:plugin` additionally includes `stats-*.unit.test.ts` without an exclusion. Excluding `tests/*.unit.test.ts` and the files from `tests/tinbase-db-tests.txt` (or invoking Vitest directly with those excludes) would keep these serial jobs focused and avoid duplicating `test_unit`/`test_db` coverage.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| # Serial plugin hot-path suite (/updates, /stats, /channel_self). Kept off the | ||
| # 8-way CF shard matrix because parallel workerd load causes intermittent 503s. | ||
| test_cloudflare_plugin: |
There was a problem hiding this comment.
P2: The new plugin jobs rerun DB-only and unit suites instead of only the hot-path integration tests: both tests/updates*.test.ts tests/stats*.test.ts tests/channel_self*.test.ts globs include stats-export-cors.test.ts, and test:cloudflare:plugin additionally includes stats-*.unit.test.ts without an exclusion. Excluding tests/*.unit.test.ts and the files from tests/tinbase-db-tests.txt (or invoking Vitest directly with those excludes) would keep these serial jobs focused and avoid duplicating test_unit/test_db coverage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/tests.yml, line 896:
<comment>The new plugin jobs rerun DB-only and unit suites instead of only the hot-path integration tests: both `tests/updates*.test.ts tests/stats*.test.ts tests/channel_self*.test.ts` globs include `stats-export-cors.test.ts`, and `test:cloudflare:plugin` additionally includes `stats-*.unit.test.ts` without an exclusion. Excluding `tests/*.unit.test.ts` and the files from `tests/tinbase-db-tests.txt` (or invoking Vitest directly with those excludes) would keep these serial jobs focused and avoid duplicating `test_unit`/`test_db` coverage.</comment>
<file context>
@@ -750,8 +891,133 @@ jobs:
+ # Serial plugin hot-path suite (/updates, /stats, /channel_self). Kept off the
+ # 8-way CF shard matrix because parallel workerd load causes intermittent 503s.
+ test_cloudflare_plugin:
+ needs: [changes, lint_typecheck, dead_code]
+ if: needs.changes.outputs.run_capgo == 'true'
</file context>
| uses: actions/cache@v5 # v5 | ||
| with: | ||
| path: ${{ env.DENO_DIR }} | ||
| key: my_cache_key |
There was a problem hiding this comment.
P3: The new serial backend plugin job caches the Deno directory under a static key: my_cache_key. Because GitHub caches are immutable per key, this entry is written once (by whichever backend job wins the race) and then restored forever — it never refreshes when supabase/functions/deno.lock or the runtime version changes, and all 6 shards + the new plugin job resolve to the same frozen cache. This can silently run tests against an outdated Deno dependency set. Consider keying the cache on a dependency hash (e.g. include the deno.lock hash) or at least add restore-keys, so the cache is refreshed when dependencies change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/tests.yml, line 481:
<comment>The new serial backend plugin job caches the Deno directory under a static `key: my_cache_key`. Because GitHub caches are immutable per key, this entry is written once (by whichever backend job wins the race) and then restored forever — it never refreshes when `supabase/functions/deno.lock` or the runtime version changes, and all 6 shards + the new plugin job resolve to the same frozen cache. This can silently run tests against an outdated Deno dependency set. Consider keying the cache on a dependency hash (e.g. include the deno.lock hash) or at least add `restore-keys`, so the cache is refreshed when dependencies change.</comment>
<file context>
@@ -433,9 +456,127 @@ jobs:
+ uses: actions/cache@v5 # v5
+ with:
+ path: ${{ env.DENO_DIR }}
+ key: my_cache_key
+ - name: Checkout repository
+ uses: actions/checkout@v6 # v6
</file context>
| key: my_cache_key | |
| key: deno-${{ runner.os }}-${{ hashFiles('supabase/functions/deno.lock', 'deno.lock') }} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 @.github/workflows/tests.yml:
- Line 465: Increase the timeout-minutes value for the serial plugin job from 5
minutes to provide sufficient time for startup, warm-up, and all serialized test
suites. Also update the timeout-minutes setting for test_cloudflare_plugin,
accounting for its additional wrangler worker startup.
In `@scripts/supabase-worktree.ts`:
- Around line 390-432: Restrict the container-removal block in freeHostPorts to
CI runs or containers belonging to this worktree, using the same
projectId/project-name filtering as removeLeftoverWorktreeContainers. Do not
force-remove unrelated local Docker containers that happen to publish the target
ports; retain port cleanup only for approved containers.
- Around line 336-344: Centralize the Cloudflare worker and inspector base port
values used by the returned array in the worktree port logic, startup script,
and test script. Introduce or reuse one shared constants source, then update
each consumer to derive its ports from those constants while preserving the
existing offset behavior and port assignments.
In `@src/components/admin/AdminFilterBar.vue`:
- Line 6: Reorder the imports in AdminFilterBar.vue so the vue-router
LocationQueryRaw type import appears before the vue-i18n value import,
satisfying the lint-required import ordering without changing import behavior.
In `@tests/cron_stat_app.test.ts`:
- Around line 28-32: Update the warmEdgeEndpoint call in the cron_stat_app test
setup to send an inert empty object payload instead of appId and orgId, while
preserving the POST method and headers so warming does not execute business
logic.
In `@tests/email-preferences.test.ts`:
- Around line 56-72: Replace the duplicated retry loops in
tests/email-preferences.test.ts lines 56-72 and tests/updates-manifest.test.ts
lines 92-101 with single warmEdgeEndpoint calls, importing the helper from
tests/test-utils.ts. Preserve each request’s existing URL, method, headers, and
JSON body; remove the local retry and delay logic in both sites.
- Around line 58-66: Update the warm-up request in the email-preferences test
around the cron_email POST to avoid using the asserted USER_EMAIL_EMAIL_PREFS
and APPNAME_PREFS identities; provide an inert payload or dedicated warm-up
email/app identity so monthly_create_stats cannot affect the test subject before
assertions.
In `@tests/test-utils.ts`:
- Around line 514-521: Update warmEdgeEndpoint to wrap each fetch attempt and
response handling in try/catch so connection errors such as ECONNREFUSED consume
the same retry and backoff sequence instead of escaping the loop; preserve the
existing immediate return for non-502/503 responses and log retry failures with
the available error or status context.
🪄 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: a0ee8384-89b1-41be-b250-cdb699412e98
📒 Files selected for processing (13)
.github/workflows/tests.ymlscripts/supabase-worktree.tssrc/components/admin/AdminFilterBar.vuetests/admin-stats.test.tstests/cron_stat_app.test.tstests/cron_stat_org.test.tstests/email-preferences.test.tstests/enforce-encrypted-bundles.test.tstests/stats.test.tstests/test-utils.tstests/updates-manifest.test.tsvitest.config.cloudflare.tsvitest.config.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
| needs: [changes, lint_typecheck, dead_code] | ||
| if: needs.changes.outputs.run_capgo == 'true' | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 5 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Raise timeout-minutes for the serial plugin job.
This job performs a cold Supabase start, an edge server bootstrap, a warm-up loop of up to 15 seconds, and then runs three full suites with --maxWorkers=1 --maxConcurrency=1. The sharded test_backend job needs the same 5 minutes for one sixth of the suite with 6-way parallelism. A 5-minute budget will likely fail this job on timeout rather than on a test result.
The same concern applies to test_cloudflare_plugin at line 900, which additionally starts three wrangler workers.
🔧 Proposed change
- timeout-minutes: 5
+ timeout-minutes: 15📝 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.
| timeout-minutes: 5 | |
| timeout-minutes: 15 |
🤖 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 @.github/workflows/tests.yml at line 465, Increase the timeout-minutes value
for the serial plugin job from 5 minutes to provide sufficient time for startup,
warm-up, and all serialized test suites. Also update the timeout-minutes setting
for test_cloudflare_plugin, accounting for its additional wrangler worker
startup.
| // wrangler worker ports + inspector ports from scripts/start-cloudflare-workers.sh | ||
| return [ | ||
| 8787 + offset, | ||
| 8788 + offset, | ||
| 8789 + offset, | ||
| 9230 + offset, | ||
| 9231 + offset, | ||
| 9232 + offset, | ||
| ] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm the wrangler worker and inspector base ports used by the startup script.
fd -t f 'start-cloudflare-workers.sh' -x cat -n {}
fd -t f 'cloudflare-test-config.ts' -x rg -n '87[0-9]{2}|92[0-9]{2}|PORT' {}Repository: Cap-go/capgo.app
Length of output: 9245
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- scripts/cloudflare-test-config.ts ---'
cat -n scripts/cloudflare-test-config.ts | sed -n '1,90p'
printf '%s\n' '--- scripts/supabase-worktree.ts ---'
cat -n scripts/supabase-worktree.ts | sed -n '300,355p'
printf '%s\n' '--- port helper usages ---'
rg -n -C 3 'getCloudflareWorkerPorts|cloudflareWorkerPortOffset|API_INSPECTOR_PORT|PLUGIN_INSPECTOR_PORT|FILES_INSPECTOR_PORT' scriptsRepository: Cap-go/capgo.app
Length of output: 8465
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- cloudflareWorkerUrl call sites ---'
rg -n -C 2 'cloudflareWorkerUrl\(' --glob '!node_modules/**'
printf '%s\n' '--- all Cloudflare worker port literals ---'
rg -n -g '*.ts' -g '*.tsx' -g '*.vue' -g '*.sh' '8787|8788|8789|9230|9231|9232'
printf '%s\n' '--- offset bounds in port reservation ---'
cat -n scripts/supabase-worktree.ts | sed -n '354,380p'
cat -n scripts/supabase-worktree.ts | sed -n '450,475p'Repository: Cap-go/capgo.app
Length of output: 193
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- all Cloudflare worker port literals ---'
rg -n -g '*.ts' -g '*.tsx' -g '*.vue' -g '*.sh' '8787|8788|8789|9230|9231|9232' . || true
printf '%s\n' '--- port reservation call sites ---'
cat -n scripts/supabase-worktree.ts | sed -n '354,380p'
cat -n scripts/supabase-worktree.ts | sed -n '450,475p'Repository: Cap-go/capgo.app
Length of output: 4659
Centralize the Cloudflare port constants.
The ports match scripts/start-cloudflare-workers.sh, but the base values are duplicated across startup, test, and worktree scripts. Use one shared source to prevent drift.
🧰 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 336 - 344, Centralize the
Cloudflare worker and inspector base port values used by the returned array in
the worktree port logic, startup script, and test script. Introduce or reuse one
shared constants source, then update each consumer to derive its ports from
those constants while preserving the existing offset behavior and port
assignments.
| function freeHostPorts(ports: number[]): void { | ||
| if (process.platform === 'win32' || ports.length === 0) | ||
| return | ||
|
|
||
| for (const port of ports) { | ||
| const uniquePorts = [...new Set(ports.filter(port => Number.isFinite(port)))] | ||
| const holders = new Set<string>() | ||
|
|
||
| for (const port of uniquePorts) { | ||
| const byPublish = spawnSync('docker', ['ps', '-aq', '--filter', `publish=${port}`], { | ||
| encoding: 'utf8', | ||
| }) | ||
| for (const id of (byPublish.stdout ?? '').split(/\s+/).filter(Boolean)) | ||
| holders.add(id) | ||
| } | ||
|
|
||
| // Match host-port publish strings docker prints (0.0.0.0:58722->5432/tcp). | ||
| const listed = spawnSync('docker', ['ps', '-a', '--format', '{{.ID}} {{.Ports}}'], { | ||
| encoding: 'utf8', | ||
| }) | ||
| if ((listed.status ?? 1) === 0) { | ||
| for (const line of (listed.stdout ?? '').split('\n')) { | ||
| const trimmed = line.trim() | ||
| if (!trimmed) | ||
| continue | ||
| const spaceIdx = trimmed.indexOf(' ') | ||
| const id = spaceIdx >= 0 ? trimmed.slice(0, spaceIdx) : trimmed | ||
| const published = spaceIdx >= 0 ? trimmed.slice(spaceIdx + 1) : '' | ||
| if (uniquePorts.some(port => published.includes(`:${port}->`) || published.includes(`:${port}/`))) | ||
| holders.add(id) | ||
| } | ||
| } | ||
|
|
||
| if (holders.size > 0) { | ||
| console.error(`Removing Docker containers still publishing worktree ports: ${[...holders].join(', ')}`) | ||
| spawnSync('docker', ['rm', '-f', ...holders], { stdio: 'inherit' }) | ||
| } | ||
|
|
||
| for (const port of uniquePorts) { | ||
| spawnSync('fuser', ['-k', `${port}/tcp`], { stdio: 'ignore' }) | ||
| // Close lingering sockets that still occupy the port after docker-proxy dies. | ||
| spawnSync('ss', ['-K', 'sport', '=', `:${port}`], { stdio: 'ignore' }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Restrict container removal to CI or to this worktree's containers.
freeHostPorts runs on every supabase start through runSupabaseStartWithRetry, including local developer runs. It force-removes any Docker container that publishes one of the target ports. The Cloudflare ports (8787, 8788, 8789) and inspector ports (9230-9232) are common local ports, so an unrelated developer container can be deleted without warning.
removeLeftoverWorktreeContainers already handles this worktree safely by filtering on projectId. Gate the broad port-based removal behind CI, or filter the discovered containers by the worktree project name before removal.
🛡️ Proposed guard
-function freeHostPorts(ports: number[]): void {
+function freeHostPorts(ports: number[]): void {
if (process.platform === 'win32' || ports.length === 0)
return
+ // Removing containers by host port can hit unrelated local stacks; only do it in CI.
+ const allowContainerRemoval = Boolean(process.env.CI)
const uniquePorts = [...new Set(ports.filter(port => Number.isFinite(port)))]
const holders = new Set<string>()
+ if (allowContainerRemoval) {
for (const port of uniquePorts) {
...
}
+ }🧰 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 390 - 432, Restrict the
container-removal block in freeHostPorts to CI runs or containers belonging to
this worktree, using the same projectId/project-name filtering as
removeLeftoverWorktreeContainers. Do not force-remove unrelated local Docker
containers that happen to publish the target ports; retain port cleanup only for
approved containers.
| await warmEdgeEndpoint('/triggers/cron_stat_app', { | ||
| method: 'POST', | ||
| headers: triggerHeaders, | ||
| body: JSON.stringify({ appId, orgId: ORG_ID_CRON_APP }), | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the cron_stat_app test setup and the trigger's write behavior.
fd -t f 'cron_stat_app.test.ts' -x sed -n '1,80p' {}
fd -t f -g '*cron_stat_app*' supabase | head -20Repository: Cap-go/capgo.app
Length of output: 2678
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- test file ---'
sed -n '1,220p' tests/cron_stat_app.test.ts
printf '%s\n' '--- trigger outline/source ---'
wc -l supabase/functions/_backend/triggers/cron_stat_app.ts
sed -n '1,260p' supabase/functions/_backend/triggers/cron_stat_app.ts
printf '%s\n' '--- reset helpers ---'
rg -n -A35 -B10 'resetAndSeedAppDataStats|resetAppDataStats|resetAndSeedAppData|warmEdgeEndpoint' tests/test-utils.ts testsRepository: Cap-go/capgo.app
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- cron_stat_app.test.ts ---'
nl -ba tests/cron_stat_app.test.ts | sed -n '1,180p'
printf '%s\n' '--- cron_stat_app.ts symbols and writes ---'
rg -n '^(export )?(async )?function|from\\(|insert|update|upsert|delete|rpc|queue|stats|return' supabase/functions/_backend/triggers/cron_stat_app.ts
printf '%s\n' '--- cron_stat_app.ts relevant sections ---'
nl -ba supabase/functions/_backend/triggers/cron_stat_app.ts | sed -n '1,240p'
printf '%s\n' '--- helper definitions ---'
rg -n '^(export )?async function (resetAndSeedAppDataStats|resetAppDataStats|resetAndSeedAppData|resetAppData|warmEdgeEndpoint)' tests/test-utils.tsRepository: Cap-go/capgo.app
Length of output: 411
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- cron_stat_app.test.ts ---'
awk '{printf "%5d %s\n", NR, $0}' tests/cron_stat_app.test.ts | sed -n '1,180p'
printf '%s\n' '--- cron_stat_app.ts symbols and writes ---'
rg -n 'function|from\(|insert|update|upsert|delete|rpc|queue|stats|return' supabase/functions/_backend/triggers/cron_stat_app.ts || true
printf '%s\n' '--- cron_stat_app.ts relevant sections ---'
awk '{printf "%5d %s\n", NR, $0}' supabase/functions/_backend/triggers/cron_stat_app.ts | sed -n '1,260p'
printf '%s\n' '--- helper definitions ---'
rg -n 'resetAndSeedAppDataStats|resetAppDataStats|resetAndSeedAppData|resetAppData|warmEdgeEndpoint' tests/test-utils.ts | head -80Repository: Cap-go/capgo.app
Length of output: 20695
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- trigger handler ---'
sed -n '480,690p' supabase/functions/_backend/triggers/cron_stat_app.ts
printf '%s\n' '--- warm-up helper ---'
sed -n '490,535p' tests/test-utils.ts
printf '%s\n' '--- seed and reset helpers ---'
sed -n '650,750p' tests/test-utils.ts
printf '%s\n' '--- stats table references in helpers and migrations ---'
rg -n 'daily_mau|daily_bandwidth|daily_storage|daily_version|daily_storage_hourly|app_metrics_cache|stats_updated_at|plan_calculated_at' tests/test-utils.ts supabase/migrations | head -160Repository: Cap-go/capgo.app
Length of output: 40219
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- seed/reset SQL definitions ---'
rg -n -A90 -B10 'reset_and_seed_app_stats_data|reset_app_stats_data|reset_and_seed_app_data\\(' supabase/migrations | head -420
printf '%s\n' '--- queue and stats-refresh SQL definitions ---'
rg -n -A80 -B15 'queue_cron_stat_org_for_org|mark_app_stats_refreshed|sync.*stats|plan_calculated_at' supabase/migrations | head -500
printf '%s\n' '--- assertions or reads of affected stats rows in the test ---'
rg -n 'daily_|storage_hourly|stats_updated_at|plan_calculated_at|queue_cron|rpc\\(' tests/cron_stat_app.test.tsRepository: Cap-go/capgo.app
Length of output: 48896
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- seed/reset function locations ---'
for term in reset_and_seed_app_stats_data reset_app_stats_data reset_and_seed_app_data; do
printf '\n[%s]\n' "$term"
rg -n -F "$term" supabase/migrations tests/test-utils.ts | head -30
done
printf '%s\n' '--- queue function and refresh logic ---'
sed -n '10760,10855p' supabase/migrations/20260708000000_prod_baseline.sql
printf '%s\n' '--- test affected-state references ---'
for term in 'daily_' 'storage_hourly' 'stats_updated_at' 'plan_calculated_at' 'queue_cron' 'rpc('; do
printf '\n[%s]\n' "$term"
rg -n -F "$term" tests/cron_stat_app.test.ts || true
done
printf '%s\n' '--- targeted SQL extraction ---'
python3 - <<'PY'
from pathlib import Path
text = Path('supabase/migrations/20260708000000_prod_baseline.sql').read_text()
terms = ('reset_and_seed_app_stats_data', 'reset_app_stats_data', 'reset_and_seed_app_data')
for term in terms:
pos = text.find(term)
print(f'\n--- {term}: offset {pos} ---')
if pos >= 0:
start = max(0, text.rfind('CREATE OR REPLACE FUNCTION', 0, pos) - 100)
end = text.find('ALTER FUNCTION', pos)
print(text[start:end if end >= 0 else pos + 6000][:12000])
PYRepository: Cap-go/capgo.app
Length of output: 4275
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- locate reset function definitions across the repository ---'
rg -n -F 'reset_and_seed_app_stats_data' . || true
rg -n -F 'reset_app_stats_data' . || true
rg -n -F 'CREATE OR REPLACE FUNCTION' supabase tests | rg 'reset|seed' | head -120
printf '%s\n' '--- locate sync and pending-refresh definitions ---'
rg -n -F 'syncOrgStatsRefresh' supabase/functions/_backend/triggers/cron_stat_app.ts
sed -n '350,490p' supabase/functions/_backend/triggers/cron_stat_app.ts
rg -n -F 'has_pending_app_stats_refresh' supabase/migrations supabase | head -30
printf '%s\n' '--- identify all assertions in the test ---'
python3 - <<'PY'
from pathlib import Path
for i, line in enumerate(Path('tests/cron_stat_app.test.ts').read_text().splitlines(), 1):
if any(x in line for x in ('expect(', '.select(', '.update(', '.rpc(')):
print(f'{i}: {line}')
PYRepository: Cap-go/capgo.app
Length of output: 10009
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- reset_app_stats_data ---'
sed -n '1110,1165p' supabase/seed.sql
printf '%s\n' '--- reset_and_seed_app_stats_data ---'
sed -n '1150,1228p' supabase/seed.sql
printf '%s\n' '--- reset_and_seed_app_data ---'
sed -n '895,1015p' supabase/seed.sqlRepository: Cap-go/capgo.app
Length of output: 11336
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- shared cron test constants and usages ---'
rg -n -F 'ORG_ID_CRON_APP' tests | head -120
printf '%s\n' '--- stats trigger test constants ---'
rg -n -F 'STRIPE_CUSTOMER_ID_CRON_APP' tests | head -120Repository: Cap-go/capgo.app
Length of output: 1615
Use an inert warm-up payload.
This payload executes the trigger and mutates daily stats, refresh timestamps, and the plan-refresh queue before the tests run. Use {} so warmEdgeEndpoint warms the isolate without running business logic.
🤖 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 `@tests/cron_stat_app.test.ts` around lines 28 - 32, Update the
warmEdgeEndpoint call in the cron_stat_app test setup to send an inert empty
object payload instead of appId and orgId, while preserving the POST method and
headers so warming does not execute business logic.
| const warm = await fetch(`${BASE_URL}/triggers/cron_email`, { | ||
| method: 'POST', | ||
| headers: triggerHeaders, | ||
| body: JSON.stringify({ | ||
| email: USER_EMAIL_EMAIL_PREFS, | ||
| appId: APPNAME_PREFS, | ||
| type: 'monthly_create_stats', | ||
| }), | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The warm-up executes a real monthly_create_stats run for the asserted user.
The payload uses USER_EMAIL_EMAIL_PREFS and APPNAME_PREFS, which are the same identities the tests assert on. The trigger therefore performs its full side effects, including any email send or preference write, before the first assertion. Use an inert payload or a dedicated warm-up identity.
🤖 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 `@tests/email-preferences.test.ts` around lines 58 - 66, Update the warm-up
request in the email-preferences test around the cron_email POST to avoid using
the asserted USER_EMAIL_EMAIL_PREFS and APPNAME_PREFS identities; provide an
inert payload or dedicated warm-up email/app identity so monthly_create_stats
cannot affect the test subject before assertions.
| for (let attempt = 1; attempt <= 5; attempt++) { | ||
| const response = await fetch(url, options) | ||
| await response.text().catch(() => undefined) | ||
| if (response.status !== 502 && response.status !== 503) | ||
| return | ||
| console.error(`[warmEdgeEndpoint] attempt=${attempt} status=${response.status} url=${url}`) | ||
| await new Promise(resolve => setTimeout(resolve, 500 * attempt)) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Retry connection errors, not only gateway statuses.
warmEdgeEndpoint targets a cold isolate. A cold or still-starting server usually rejects the socket, so fetch throws ECONNREFUSED instead of returning 502 or 503. The throw escapes the loop and fails the beforeAll hook. Wrap the request in try/catch so connection errors also consume a retry.
🐛 Proposed fix
for (let attempt = 1; attempt <= 5; attempt++) {
- const response = await fetch(url, options)
- await response.text().catch(() => undefined)
- if (response.status !== 502 && response.status !== 503)
- return
- console.error(`[warmEdgeEndpoint] attempt=${attempt} status=${response.status} url=${url}`)
+ try {
+ const response = await fetch(url, options)
+ await response.text().catch(() => undefined)
+ if (response.status !== 502 && response.status !== 503)
+ return
+ console.error(`[warmEdgeEndpoint] attempt=${attempt} status=${response.status} url=${url}`)
+ }
+ catch (error) {
+ console.error(`[warmEdgeEndpoint] attempt=${attempt} url=${url} error=${String(error)}`)
+ }
await new Promise(resolve => setTimeout(resolve, 500 * attempt))
}📝 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.
| for (let attempt = 1; attempt <= 5; attempt++) { | |
| const response = await fetch(url, options) | |
| await response.text().catch(() => undefined) | |
| if (response.status !== 502 && response.status !== 503) | |
| return | |
| console.error(`[warmEdgeEndpoint] attempt=${attempt} status=${response.status} url=${url}`) | |
| await new Promise(resolve => setTimeout(resolve, 500 * attempt)) | |
| } | |
| for (let attempt = 1; attempt <= 5; attempt++) { | |
| try { | |
| const response = await fetch(url, options) | |
| await response.text().catch(() => undefined) | |
| if (response.status !== 502 && response.status !== 503) | |
| return | |
| console.error(`[warmEdgeEndpoint] attempt=${attempt} status=${response.status} url=${url}`) | |
| } | |
| catch (error) { | |
| console.error(`[warmEdgeEndpoint] attempt=${attempt} url=${url} error=${String(error)}`) | |
| } | |
| await new Promise(resolve => setTimeout(resolve, 500 * attempt)) | |
| } |
🤖 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 `@tests/test-utils.ts` around lines 514 - 521, Update warmEdgeEndpoint to wrap
each fetch attempt and response handling in try/catch so connection errors such
as ECONNREFUSED consume the same retry and backoff sequence instead of escaping
the loop; preserve the existing immediate return for non-502/503 responses and
log retry failures with the available error or status context.
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Confidence score: 3/5
- In
scripts/supabase-worktree.ts, the worktree startup cleanup can force-remove any Docker container bound to the target ports without validating the currentprojectId, so an unrelated local/service container could be deleted and break running workflows — scope container selection to the active project before removal. - In
scripts/supabase-worktree.ts, thess -K sport = :NNNNsocket cleanup is likely ineffective on GitHub Actions without CAP_NET_ADMIN, which can leave lingering sockets and cause flaky port-conflict behavior in CI — gate this step by privilege/environment and add a fallback or explicit warning when it cannot run.
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:424">
P2: Starting a worktree can destroy an unrelated Docker container that happens to publish one of these ports because this cleanup force-removes all matching container IDs without checking the current `projectId`. Scoping removal to this worktree's project/labels, or leaving unrelated holders for the bind error, would avoid destructive cleanup.</violation>
<violation number="2" location="scripts/supabase-worktree.ts:430">
P3: The newly added `ss -K sport = :NNNN` cleanup step (`Close lingering sockets ... after docker-proxy dies`) is likely a silent no-op on GitHub Actions: destroying sockets via `ss -K` needs CAP_NET_ADMIN (root), which the default non-root `runner` user does not have, and the failure is swallowed by `stdio: 'ignore'`. Also, `ss -K` cannot kill TIME_WAIT sockets, which is the exact lingering state after docker-proxy exits. The effective cleanup here is the docker `ps`/`rm -f` block just above it; consider dropping the `ss` call or confirming it actually runs (e.g. run it with `sudo` / capture and log the exit status) so the intended port release is not silently skipped.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| if (holders.size > 0) { | ||
| console.error(`Removing Docker containers still publishing worktree ports: ${[...holders].join(', ')}`) | ||
| spawnSync('docker', ['rm', '-f', ...holders], { stdio: 'inherit' }) |
There was a problem hiding this comment.
P2: Starting a worktree can destroy an unrelated Docker container that happens to publish one of these ports because this cleanup force-removes all matching container IDs without checking the current projectId. Scoping removal to this worktree's project/labels, or leaving unrelated holders for the bind error, would avoid destructive cleanup.
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 424:
<comment>Starting a worktree can destroy an unrelated Docker container that happens to publish one of these ports because this cleanup force-removes all matching container IDs without checking the current `projectId`. Scoping removal to this worktree's project/labels, or leaving unrelated holders for the bind error, would avoid destructive cleanup.</comment>
<file context>
@@ -381,12 +381,53 @@ function reserveWorktreePortsFromEphemeralPool(repoRoot: string): void {
+
+ if (holders.size > 0) {
+ console.error(`Removing Docker containers still publishing worktree ports: ${[...holders].join(', ')}`)
+ spawnSync('docker', ['rm', '-f', ...holders], { stdio: 'inherit' })
+ }
+
</file context>
| for (const port of uniquePorts) { | ||
| spawnSync('fuser', ['-k', `${port}/tcp`], { stdio: 'ignore' }) | ||
| // Close lingering sockets that still occupy the port after docker-proxy dies. | ||
| spawnSync('ss', ['-K', 'sport', '=', `:${port}`], { stdio: 'ignore' }) |
There was a problem hiding this comment.
P3: The newly added ss -K sport = :NNNN cleanup step (Close lingering sockets ... after docker-proxy dies) is likely a silent no-op on GitHub Actions: destroying sockets via ss -K needs CAP_NET_ADMIN (root), which the default non-root runner user does not have, and the failure is swallowed by stdio: 'ignore'. Also, ss -K cannot kill TIME_WAIT sockets, which is the exact lingering state after docker-proxy exits. The effective cleanup here is the docker ps/rm -f block just above it; consider dropping the ss call or confirming it actually runs (e.g. run it with sudo / capture and log the exit status) so the intended port release is not silently skipped.
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 430:
<comment>The newly added `ss -K sport = :NNNN` cleanup step (`Close lingering sockets ... after docker-proxy dies`) is likely a silent no-op on GitHub Actions: destroying sockets via `ss -K` needs CAP_NET_ADMIN (root), which the default non-root `runner` user does not have, and the failure is swallowed by `stdio: 'ignore'`. Also, `ss -K` cannot kill TIME_WAIT sockets, which is the exact lingering state after docker-proxy exits. The effective cleanup here is the docker `ps`/`rm -f` block just above it; consider dropping the `ss` call or confirming it actually runs (e.g. run it with `sudo` / capture and log the exit status) so the intended port release is not silently skipped.</comment>
<file context>
@@ -381,12 +381,53 @@ function reserveWorktreePortsFromEphemeralPool(repoRoot: string): void {
+ for (const port of uniquePorts) {
spawnSync('fuser', ['-k', `${port}/tcp`], { stdio: 'ignore' })
+ // Close lingering sockets that still occupy the port after docker-proxy dies.
+ spawnSync('ss', ['-K', 'sport', '=', `:${port}`], { stdio: 'ignore' })
}
}
</file context>
Root-cause fixes for the post-merge red CI: drop .ts extensions that break cli tsc build, fix AdminFilterBar import sort, install deps before Supabase start on CF/plugin jobs (no parallel OOM), bound CF port cleanup offsets, move channel-rate-limit into serial plugin jobs, and reuse warmEdgeEndpoint with API_SECRET. 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_d7d5ae6d-ff76-4dd1-8e44-5e388ea96119) |
There was a problem hiding this comment.
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 `@scripts/supabase-worktree.ts`:
- Around line 333-335: Update the Cloudflare port-offset parsing in the
surrounding cleanup logic to normalize an unset or empty
CLOUDFLARE_WORKER_PORT_OFFSET value to offset 0, reserving the default worker
port bands 8787–8789 and 9230–9232. Continue returning [] for malformed, unsafe,
negative, or greater-than-50,000 offsets, and keep the existing freeHostPorts
behavior for valid offsets.
🪄 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: a12ac193-0104-417d-844c-5466b16f58b8
📒 Files selected for processing (9)
.github/workflows/tests.ymlcli/src/bundle/auto-bump-ai.tspackage.jsonscripts/supabase-worktree.tssrc/components/admin/AdminFilterBar.vuetests/email-preferences.test.tstests/test-utils.tstests/updates-manifest.test.tsvitest.config.cloudflare.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
| // Match scripts/start-cloudflare-workers.sh / cloudflare-test-config.ts so we never | ||
| // freeHostPorts() derived ports from an invalid offset before the launcher rejects it. | ||
| if (!Number.isSafeInteger(offset) || offset < 0 || offset > 50_000) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Reserve the default Cloudflare port band.
Line 333 states that this logic matches the launcher, but an unset CLOUDFLARE_WORKER_PORT_OFFSET returns [] here. The launcher treats an unset value as offset 0 and binds ports 8787 through 8789 and 9230 through 9232. A default worker run can therefore collide with an existing process that this cleanup did not reserve or release.
Normalize an unset or empty value to 0. Keep [] for malformed or out-of-range values.
Proposed fix
function getCloudflareWorkerPorts(): number[] {
const raw = process.env.CLOUDFLARE_WORKER_PORT_OFFSET
- if (!raw || !/^\d+$/.test(raw))
+ if (raw && !/^\d+$/.test(raw))
return []
- const offset = Number(raw)
+ const offset = raw ? Number(raw) : 0🧰 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 333 - 335, Update the Cloudflare
port-offset parsing in the surrounding cleanup logic to normalize an unset or
empty CLOUDFLARE_WORKER_PORT_OFFSET value to offset 0, reserving the default
worker port bands 8787–8789 and 9230–9232. Continue returning [] for malformed,
unsafe, negative, or greater-than-50,000 offsets, and keep the existing
freeHostPorts behavior for valid offsets.
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_4a6c6d79-51bc-4e7a-bd6f-4f01663661d0) |
Backend shard 6 OOM-killed parallel bun install during Docker start, same failure class as CF shard 5. Make all Supabase-backed jobs install dependencies (and CLI build / pg17 client where needed) before start. 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_cc431c8c-0a3d-4447-b68c-8a0ed60ffccc) |
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)
.github/workflows/tests.yml (1)
392-414: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winFail the warm-up when an endpoint remains unavailable.
At Line 409, a
500response exits the loop immediately. After the fifth502,503, or000response, the loop also completes successfully. The test suite then starts against an unavailable Edge service and hides the startup failure.Require an expected success status before continuing. Print the final response body and exit nonzero when all attempts fail.
Proposed fix
- if [ "${status}" != "502" ] && [ "${status}" != "503" ] && [ "${status}" != "000" ]; then + if [[ "${status}" =~ ^2[0-9]{2}$ ]]; then break fi + if [ "${attempt}" = "5" ]; then + echo "Edge warm-up failed for ${path}: HTTP ${status}" + head -c 4096 /tmp/edge-warm.body || true + exit 1 + fi sleep "${attempt}"🤖 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 @.github/workflows/tests.yml around lines 392 - 414, Update the “Warm edge functions before tests” step to require a successful expected status for each endpoint rather than accepting any non-502/503/000 response. After all retry attempts fail, print the final response body and exit nonzero; ensure 500 responses and exhausted retries both fail the workflow instead of continuing.
🤖 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 @.github/workflows/tests.yml:
- Around line 392-414: Update the “Warm edge functions before tests” step to
require a successful expected status for each endpoint rather than accepting any
non-502/503/000 response. After all retry attempts fail, print the final
response body and exit nonzero; ensure 500 responses and exhausted retries both
fail the workflow instead of continuing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c901e2f4-8016-4377-8f61-d1847bc6406a
📒 Files selected for processing (1)
.github/workflows/tests.yml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 4/5
- In
.github/workflows/tests.yml, movingbun installand PostgreSQL client setup to run strictly beforesupabase-worktree startremoves previous overlap, which can noticeably lengthen CI runtime and increase the chance of job timeout or slower feedback loops—restore the backgrounded/parallelized setup (or raise timeouts if serialization is intentional).
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=".github/workflows/tests.yml">
<violation number="1" location=".github/workflows/tests.yml:362">
P2: The changed jobs now run `bun install` and the PostgreSQL 17 client apt install strictly sequentially *before* `supabase-worktree start`, whereas the previous code launched both installs in the background so they overlapped with the long cold Supabase start. These jobs all carry `timeout-minutes: 5` and do not cache `bun install`/node_modules, so the full `bun install` + apt-get duration is now added serially to each 5-minute job. For a PR whose whole point is eliminating CI timeouts/flaky shard reds, this could push the very jobs it targets back over their 5-minute cap. Recommend confirming (with a couple of runs) that install-then-start stays under the cap, or reintroduce overlap / add a node_modules cache to preserve margin.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| run: bun scripts/supabase-worktree.ts stop --no-backup || true | ||
| - name: Run Supabase Start (deps and PostgreSQL 17 client install in background) | ||
| - name: Install dependencies | ||
| run: bun install |
There was a problem hiding this comment.
P2: The changed jobs now run bun install and the PostgreSQL 17 client apt install strictly sequentially before supabase-worktree start, whereas the previous code launched both installs in the background so they overlapped with the long cold Supabase start. These jobs all carry timeout-minutes: 5 and do not cache bun install/node_modules, so the full bun install + apt-get duration is now added serially to each 5-minute job. For a PR whose whole point is eliminating CI timeouts/flaky shard reds, this could push the very jobs it targets back over their 5-minute cap. Recommend confirming (with a couple of runs) that install-then-start stays under the cap, or reintroduce overlap / add a node_modules cache to preserve margin.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/tests.yml, line 362:
<comment>The changed jobs now run `bun install` and the PostgreSQL 17 client apt install strictly sequentially *before* `supabase-worktree start`, whereas the previous code launched both installs in the background so they overlapped with the long cold Supabase start. These jobs all carry `timeout-minutes: 5` and do not cache `bun install`/node_modules, so the full `bun install` + apt-get duration is now added serially to each 5-minute job. For a PR whose whole point is eliminating CI timeouts/flaky shard reds, this could push the very jobs it targets back over their 5-minute cap. Recommend confirming (with a couple of runs) that install-then-start stays under the cap, or reintroduce overlap / add a node_modules cache to preserve margin.</comment>
<file context>
@@ -358,35 +358,20 @@ jobs:
run: bun scripts/supabase-worktree.ts stop --no-backup || true
- - name: Run Supabase Start (deps and PostgreSQL 17 client install in background)
+ - name: Install dependencies
+ run: bun install
+ - name: Install PostgreSQL 17 client
run: |
</file context>
Main changed the permission backstop message to mention app.build_native; update the prescan gate assertions to match. 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_c9a061fc-063f-45aa-abd8-a31297c268e2) |
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Merge latest main. Fix CLI app-permission mock for appAddHintMessage. Allow observe-tab URL query strings in Playwright. Cap backend workers, warm /webhooks and /organization before sharded suites, and warm those routes in the affected test beforeAll hooks. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
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_0f150b70-0539-41c6-af92-803ca321a1ee) |
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_0839842b-0d1e-43b4-bd2b-0c44b5d70392) |
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Confidence score: 5/5
- In
vitest.config.ts, settingmaxWorkers: 2in the shared base config can unnecessarily throttle all Vitest suites (including large pure-unit jobs), which risks significantly longer CI times and slower feedback for the team—scope the worker cap to only the isolate-constrained environment (or override it per job) instead of applying it globally.
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="vitest.config.ts">
<violation number="1" location="vitest.config.ts:26">
P3: maxWorkers: 2 in the shared base config throttles every vitest run to 2 parallel files, including the 200-file test:unit job and other pure-unit/non-Deno jobs that never spawn edge isolates. The cold-isolate cap is only needed for the sharded backend/plugin runs, which already pass --maxWorkers=1 explicitly in tests.yml; consider scoping the cap to those runs (or a dedicated config) so unit suites keep their default parallelism and don't risk the 5-minute CI cap.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| maxConcurrency: 5, // Reduced to prevent connection exhaustion | ||
| maxConcurrency: 2, // Keep edge-function load under Deno capacity in CI shards | ||
| // Cap workers so shards do not open too many cold isolates at once (502 from Kong). | ||
| maxWorkers: 2, |
There was a problem hiding this comment.
P3: maxWorkers: 2 in the shared base config throttles every vitest run to 2 parallel files, including the 200-file test:unit job and other pure-unit/non-Deno jobs that never spawn edge isolates. The cold-isolate cap is only needed for the sharded backend/plugin runs, which already pass --maxWorkers=1 explicitly in tests.yml; consider scoping the cap to those runs (or a dedicated config) so unit suites keep their default parallelism and don't risk the 5-minute CI cap.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At vitest.config.ts, line 26:
<comment>maxWorkers: 2 in the shared base config throttles every vitest run to 2 parallel files, including the 200-file test:unit job and other pure-unit/non-Deno jobs that never spawn edge isolates. The cold-isolate cap is only needed for the sharded backend/plugin runs, which already pass --maxWorkers=1 explicitly in tests.yml; consider scoping the cap to those runs (or a dedicated config) so unit suites keep their default parallelism and don't risk the 5-minute CI cap.</comment>
<file context>
@@ -22,6 +22,8 @@ export default defineConfig(({ mode }) => ({
retry: 0,
maxConcurrency: 2, // Keep edge-function load under Deno capacity in CI shards
+ // Cap workers so shards do not open too many cold isolates at once (502 from Kong).
+ maxWorkers: 2,
// Vitest 4: pool options are now top-level
isolate: true,
</file context>
|
Bring in #2915 edge warm / vitest worker caps so this branch matches what PR merge CI already runs. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
2 participants
Summary (AI generated)
admin-statstrial_plan_breakdownassertion (3→4) after invite-org fixture from fix(admin): add register→org onboarding funnel step #2883lint_typecheck+dead_code, and setfail-fast: trueon backend + CF matrices/updates/stats/channel_selfoff the 8-way CF shard matrix onto a serial Cloudflare plugin job; lowered CFmaxConcurrencyto 2Motivation (AI generated)
Main CI kept burning money on recurring shard failures (backend 5/6 + 6/6, CF 7/8, historically CF 4/8): a broken admin-stats fixture assertion every push, workerd 503s under parallel plugin load, Docker port binds after cancelled jobs, and cold edge 502s. Lint failures also still spun all heavy Docker shards because they only depended on
changes.Business Impact (AI generated)
Fewer red main pushes and cancelled sibling jobs mean lower GitHub Actions spend and faster feedback for Capgo releases. Stable CI unblocks shipping console/backend changes without retry roulette.
Test Plan (AI generated)
admin-statstrial_plan_breakdown passes on backend shard 5/6 and CF pathsGenerated with AI
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
Bug Fixes
Tests