fix(cli): make CLI usage analytics PR pass CI - #2945
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Keep both builder_capacity and cli_usage admin metrics, and prefer the HTTP app existence check with the shared app-add hint message. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve merge conflicts with main, restamp cli_usage migration after latest main timestamps, and fix getOrganizationId call sites after the HTTP API refactor so CLI build/lint pass. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Align init conflict and permission helper tests with findAppInOrganization and checkAppExists now calling Capgo HTTP APIs instead of PostgREST RPCs. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
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: 30 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 (2)
📝 WalkthroughWalkthroughThe CLI migration adds shared request headers and Capgo HTTP paths across app, channel, bundle, organization, onboarding, and preview flows. The backend records CLI usage through Analytics Engine or Postgres and exposes statistics in a new admin dashboard. ChangesCLI HTTP migration
CLI usage analytics
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ConfigEndpoint
participant UsageTracker
participant AdminStats
participant CliDashboard
CLI->>ConfigEndpoint: Send CLI usage headers
ConfigEndpoint->>UsageTracker: Record usage metadata
UsageTracker->>AdminStats: Provide aggregated CLI usage
CliDashboard->>AdminStats: Request date-filtered statistics
AdminStats-->>CliDashboard: Return totals and breakdowns
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Forward supabase host options into checkAppExists so integration tests hit local functions, fix admin tab import order, and bump cli-mcp-tests to model GET /app/:id after the Capgo HTTP org lookup change. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
supabase-js marks supabaseUrl/supabaseKey protected; cast before reading so local existence checks compile under tsc. 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_3a71e210-4028-4141-b533-c07ee1723022) |
HTTP GET /channel requires app.read_channels, which app_preview keys lack by design. Keep findChannel on PostgREST so RLS matches delete/set permission checks, normalize create responses without promote metadata, and satisfy drizzle execute row typing for onboarding metrics. 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_14aca8e4-5091-44af-be8f-623af9e069e3) |
delete_bundle=true is preview-key only. App-admin cleanup soft-deletes the linked bundle then deletes the channel without that flag. 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_be21eb6d-8481-4ddf-8186-86e07a1a9dfc) |
HTTP DELETE /bundle rejects versions still linked to a channel. Admin channel+bundle cleanup soft-deletes through PostgREST first, then deletes the channel over HTTP without the preview-only delete_bundle flag. 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_b7cd7afb-9384-427c-a724-df87888410a2) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Fail closed on app-delete ownership checks, keep hosted app existence checks on api.capgo.app, restore PostgREST channel-link reads for preview keys, soft-delete only unshared admin cleanup bundles, forward AbortSignal through Capgo HTTP helpers, isolate alpha CLI analytics, and tighten related host/indent/dashboard/test follow-ups from review. 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_d0c409ee-84f2-4ff0-9e79-a334b07f7013) |
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_8ee05a52-485a-4c20-84b8-8c64cfc3fc91) |
There was a problem hiding this comment.
5 issues found across 26 files (changes from recent commits).
Confidence score: 2/5
- In
cli/src/app/add.ts, theapp addfallback uses an API key instead of the configured anon JWT forlocalSupa/localSupaAnonwhen--supa-anonis omitted, so self-hosted adds can fail unexpectedly for valid setups — resolve and apply the configured anon token in that path. - In
cli/src/app/delete.ts,skipConfirmationstill triggers anorg.readdependency, which breaks non-interactive deletions and violates the flag’s contract for automation workflows — only fetch/validatecreated_bywhen confirmation is actually required. - In
cli/src/api/app.ts, explicit cloud--supa-host/ SDK clients inside self-hosted projects are routed to local Edge Function existence checks, so real cloud apps can be reported missing and downstream commands fail — keep existence checks bound to the explicitly selected host/client context. - In
cli/src/api/channels.ts, linked-version lookup failures are treated as “no linked bundles,” allowingchannel delete --delete-bundleto report success after deleting only the channel and leaving bundle state inconsistent — propagate lookup errors and reserve empty results for truly absent links.
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="cli/src/analytics/org-resolver.ts">
<violation number="1" location="cli/src/analytics/org-resolver.ts:60">
P3: Remove unreachable rejection handler: `promise` resolves `undefined` for every failure, so this cleanup branch never runs.</violation>
</file>
<file name="cli/src/api/channels.ts">
<violation number="1" location="cli/src/api/channels.ts:237">
P2: A failed linked-version lookup is treated as no linked bundles, so `channel delete --delete-bundle` can succeed after deleting only the channel. Propagate lookup errors; reserve the empty result for an actually absent channel.</violation>
</file>
<file name="cli/src/app/add.ts">
<violation number="1" location="cli/src/app/add.ts:111">
P1: `app add` fails for self-hosted projects configured through `localSupa`/`localSupaAnon` but invoked without `--supa-anon`, because this fallback sends the API key instead of the configured anon JWT. Resolve and use the local anon key (or route this call through `invokeCapgoCliApi`) whenever the resolved host is `/functions/v1`.</violation>
</file>
<file name="cli/src/app/delete.ts">
<violation number="1" location="cli/src/app/delete.ts:74">
P2: Non-interactive `skipConfirmation` deletes now fail on an unrelated `org.read` lookup, despite the SDK contract saying this flag skips owner confirmation. Only fetch/validate `created_by` when confirmation is needed, or make the backend expose an ownership check authorized by `app.delete`.</violation>
</file>
<file name="cli/src/api/app.ts">
<violation number="1" location="cli/src/api/app.ts:191">
P2: Explicit cloud `--supa-host`/SDK clients inside a self-hosted project now send app-existence checks to the project’s local Edge Function, so valid cloud apps are reported missing and dependent commands fail. Keep the managed client host options so `invokeCapgoCliApi` can apply its existing managed-host routing guard.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const usesFunctionsV1 = apiHost.includes('/functions/v1') | ||
| const authorization = usesFunctionsV1 && params.supaAnon | ||
| ? `Bearer ${params.supaAnon}` | ||
| : apikey |
There was a problem hiding this comment.
P1: app add fails for self-hosted projects configured through localSupa/localSupaAnon but invoked without --supa-anon, because this fallback sends the API key instead of the configured anon JWT. Resolve and use the local anon key (or route this call through invokeCapgoCliApi) whenever the resolved host is /functions/v1.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/app/add.ts, line 111:
<comment>`app add` fails for self-hosted projects configured through `localSupa`/`localSupaAnon` but invoked without `--supa-anon`, because this fallback sends the API key instead of the configured anon JWT. Resolve and use the local anon key (or route this call through `invokeCapgoCliApi`) whenever the resolved host is `/functions/v1`.</comment>
<file context>
@@ -105,9 +105,17 @@ async function createAppViaApi(
+ const usesFunctionsV1 = apiHost.includes('/functions/v1')
+ const authorization = usesFunctionsV1 && params.supaAnon
+ ? `Bearer ${params.supaAnon}`
+ : apikey
const response = await fetch(`${apiHost}/app`, {
method: 'POST',
</file context>
| if (error || !data) | ||
| return { stable: null, rollout: null } |
There was a problem hiding this comment.
P2: A failed linked-version lookup is treated as no linked bundles, so channel delete --delete-bundle can succeed after deleting only the channel. Propagate lookup errors; reserve the empty result for an actually absent channel.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/api/channels.ts, line 237:
<comment>A failed linked-version lookup is treated as no linked bundles, so `channel delete --delete-bundle` can succeed after deleting only the channel. Propagate lookup errors; reserve the empty result for an actually absent channel.</comment>
<file context>
@@ -230,18 +216,53 @@ export function findChannel(supabase: SupabaseClient<Database>, appId: string, n
- .then(({ data }) => data?.version)
+ .maybeSingle()
+
+ if (error || !data)
+ return { stable: null, rollout: null }
+
</file context>
| if (error || !data) | |
| return { stable: null, rollout: null } | |
| if (error) | |
| throw error | |
| if (!data) | |
| return { stable: null, rollout: null } |
| const appOwner = appOwnerRaw as { owner_org: { created_by: string, id: string } } | null | ||
| // Owner confirmation previously joined orgs.created_by; GET app does not include that. | ||
| // TODO(cli-http): GET organization returns created_by — use it for owner confirmation | ||
| const { data: orgData, error: orgError } = await invokeCapgoCliApi<{ created_by?: string }>( |
There was a problem hiding this comment.
P2: Non-interactive skipConfirmation deletes now fail on an unrelated org.read lookup, despite the SDK contract saying this flag skips owner confirmation. Only fetch/validate created_by when confirmation is needed, or make the backend expose an ownership check authorized by app.delete.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/app/delete.ts, line 74:
<comment>Non-interactive `skipConfirmation` deletes now fail on an unrelated `org.read` lookup, despite the SDK contract saying this flag skips owner confirmation. Only fetch/validate `created_by` when confirmation is needed, or make the backend expose an ownership check authorized by `app.delete`.</comment>
<file context>
@@ -62,25 +62,34 @@ export async function deleteAppInternal(
- },
- )
- orgCreatedBy = orgData?.created_by
+ const { data: orgData, error: orgError } = await invokeCapgoCliApi<{ created_by?: string }>(
+ `organization?orgId=${encodeURIComponent(orgId)}`,
+ {
</file context>
| const client = supabase as SupabaseClient<Database> & { supabaseUrl?: string, supabaseKey?: string } | ||
| const supaHost = typeof client.supabaseUrl === 'string' ? client.supabaseUrl : undefined | ||
| const supaAnon = typeof client.supabaseKey === 'string' ? client.supabaseKey : undefined | ||
| if (supaHost && supaAnon && !isCapgoManagedSupabaseHost(supaHost)) |
There was a problem hiding this comment.
P2: Explicit cloud --supa-host/SDK clients inside a self-hosted project now send app-existence checks to the project’s local Edge Function, so valid cloud apps are reported missing and dependent commands fail. Keep the managed client host options so invokeCapgoCliApi can apply its existing managed-host routing guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/api/app.ts, line 191:
<comment>Explicit cloud `--supa-host`/SDK clients inside a self-hosted project now send app-existence checks to the project’s local Edge Function, so valid cloud apps are reported missing and dependent commands fail. Keep the managed client host options so `invokeCapgoCliApi` can apply its existing managed-host routing guard.</comment>
<file context>
@@ -169,10 +183,12 @@ export async function check2FAComplianceForApp(
const supaHost = typeof client.supabaseUrl === 'string' ? client.supabaseUrl : undefined
const supaAnon = typeof client.supabaseKey === 'string' ? client.supabaseKey : undefined
- if (supaHost && supaAnon)
+ if (supaHost && supaAnon && !isCapgoManagedSupabaseHost(supaHost))
return { supaHost, supaAnon }
return undefined
</file context>
| void promise.then((value) => { | ||
| if (!value) | ||
| ownerOrgCache.delete(cacheKey) | ||
| }).catch(() => { | ||
| ownerOrgCache.delete(cacheKey) | ||
| }) |
There was a problem hiding this comment.
P3: Remove unreachable rejection handler: promise resolves undefined for every failure, so this cleanup branch never runs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/analytics/org-resolver.ts, line 60:
<comment>Remove unreachable rejection handler: `promise` resolves `undefined` for every failure, so this cleanup branch never runs.</comment>
<file context>
@@ -55,5 +56,12 @@ export function resolveOwnerOrgId(apikey: string, appId: string, deps: OrgResolv
ownerOrgCache.set(cacheKey, promise)
+ // Do not cache aborted/failed/negative lookups for the process lifetime.
+ void promise.then((value) => {
+ if (!value)
+ ownerOrgCache.delete(cacheKey)
</file context>
| void promise.then((value) => { | |
| if (!value) | |
| ownerOrgCache.delete(cacheKey) | |
| }).catch(() => { | |
| ownerOrgCache.delete(cacheKey) | |
| }) | |
| void promise.then((value) => { | |
| if (!value) | |
| ownerOrgCache.delete(cacheKey) | |
| }) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cli/src/utils.ts (1)
866-941: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winForward
options.signaltogetRemoteConfig.Line 908 starts configuration resolution without
options.signal. If/private/configstalls, an aborted API call waits for that fetch before it can stop. This breaks cancellation for everyinvokeCapgoCliApicaller.Proposed fix
- const localConfig = await getRemoteConfig(true) + const localConfig = await getRemoteConfig(true, options.signal)🤖 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 `@cli/src/utils.ts` around lines 866 - 941, Pass options.signal into the getRemoteConfig call within invokeCapgoCliApi’s configuration-resolution branch, preserving the signal through to the remote configuration fetch so aborted API requests cancel promptly.cli/src/analytics/org-resolver.ts (1)
17-65: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not cache requests that use a caller AbortSignal.
Line 19 returns a cached promise before Line 25 checks
signal.aborted. An already aborted caller can receive a successful cached result. More importantly, when the first caller aborts, every concurrent caller sharing this cache entry receivesundefined.Bypass the shared cache when
signalis present. Cache only signal-free lookups.Proposed fix
export function resolveOwnerOrgId(apikey: string, appId: string, deps: OrgResolverDeps = {}, signal?: AbortSignal): Promise<string | undefined> { + if (signal?.aborted) + return Promise.resolve(undefined) + const cacheKey = `${apikey}:${appId}` - const cached = ownerOrgCache.get(cacheKey) + const cached = signal ? undefined : ownerOrgCache.get(cacheKey) if (cached) return cached const promise = (async () => { try { - if (signal?.aborted) - return undefined if (deps.fetchOwnerOrg) return await deps.fetchOwnerOrg(apikey, appId, signal)🤖 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 `@cli/src/analytics/org-resolver.ts` around lines 17 - 65, Update resolveOwnerOrgId so requests with a caller-provided signal bypass ownerOrgCache entirely, including both cache reads and writes. Retain the existing shared-cache behavior only for signal-free lookups, while preserving the current abort handling for signaled requests.
🤖 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 `@cli/src/api/app.ts`:
- Around line 183-194: Update the caller of hostOptionsFromSupabase to receive
the Supabase host and key explicitly from the configuration instead of reading
protected supabase.supabaseUrl and supabase.supabaseKey internals. Refactor
hostOptionsFromSupabase to accept those explicit values, retain the
isCapgoManagedSupabaseHost filtering, and ensure self-hosted clients continue
receiving { supaHost, supaAnon } while managed clients preserve default
resolution.
In `@cli/src/api/channels.ts`:
- Around line 263-265: Update isVersionLinkedToOtherChannel to distinguish query
failures from confirmed links by returning the error state separately rather
than treating any error as true. Adjust the caller in the channel deletion flow
to inspect that error and log its message when present; only log that the bundle
remains linked when the query succeeds and finds another channel.
In `@cli/src/api/versions.ts`:
- Around line 179-212: Update getChannelsVersion to use the existing
BUNDLE_PAGE_SIZE constant instead of the hard-coded 50 when determining whether
the fetched batch is the final page. Preserve the current pagination and
fetchChannelsPage boundary semantics so page traversal remains consistent with
the shared page size.
- Around line 179-212: Centralize the CLI pagination page-size value by defining
a shared constant in a neutral CLI module, then replace the duplicated literal
50 in all seven pagination loops, including getChannelsVersion and the loops in
cli/src/api/channels.ts (301-322) and cli/src/app/list.ts (21-55), plus
cli/src/preview/qr.ts and cli/src/api/app.ts. Import and reuse that constant at
each site; do not export it from versions.ts.
- Around line 28-31: Update isEmptyBundleListError to determine emptiness solely
from payload?.error matching cannot_get_bundle; remove the payload?.message
comparison so backend display-text changes do not affect empty bundle-list
detection.
- Around line 74-89: Update the Supabase branch in the surrounding delete
function to request the affected-row count and treat a zero count as “App
version ${appid}@${bundle} not found in database,” while preserving the existing
silent logging and formatted-error behavior. Ensure the success path returns
only when a bundle was actually soft-deleted, so delete.ts records names for
rollback only after an update affects a row.
- Around line 214-232: Update getVersionData to pass bundle as the version
filter when calling getActiveAppVersions, allowing the GET /bundle handler to
query by version name instead of filtering all pages in memory. Preserve the
existing pagination behavior for list callers and retain the current
missing-version error handling.
In `@cli/src/build/onboarding/ui/shell.tsx`:
- Around line 226-230: Update the Appflow build flow around AppflowApp and
requestBuildInternal to forward the supaAnon value alongside supaHost when both
CLI options are provided. Preserve the existing behavior for self-hosted builds
and ensure the same API configuration reaches the internal build request.
In `@cli/src/bundle/delete.ts`:
- Around line 44-51: Move the getOrganizationId call before
deleteSpecificVersion so organization lookup completes before the irreversible
deletion, while preserving the existing parameters and analytics flow after
deletion.
In `@cli/src/channel/delete.ts`:
- Around line 113-121: The rollback in the deleteChannel flow discards the
restore update result. Capture the result from the supabase app_versions update
and, when it reports an error, log the restore failure with the error details
while preserving the existing channel-delete error handling.
In `@cli/src/channel/set.ts`:
- Around line 599-697: Extend tests/cli-channel.test.ts for the channel set flow
to cover rollout and auto-pause options emitted by the channelBody construction,
including rollout_version, rollout_percentage_bps, rollout_enabled,
rollout_paused_at, rollout_pause_reason, rollout_cache_ttl_seconds, and all
auto_pause_* fields. After invoking channel set, assert the persisted channel
state matches the supplied values.
In `@cli/src/organization/members.ts`:
- Line 146: Update the organization member status mapping around the 2FA and
password-policy status RPC results so RPC errors remain an explicit unknown
state rather than becoming false. Ensure affected non-compliance totals are
suppressed, or terminate the report when either status collection is
unavailable; preserve normal true/false mapping for successful responses.
In `@cli/src/preview/qr.ts`:
- Around line 160-163: Update the error handling around getCapgoCliHttpStatus in
the channel-loading flow so it returns null only when the endpoint response
matches the documented missing-channel error payload. Continue throwing the
formatted error for other HTTP 400 responses, including invalid_app_id and
cannot_access_app.
In `@cli/src/sdk.ts`:
- Around line 607-611: Remove the redundant apikey property from the options
object passed to getActiveAppVersions in the surrounding method, keeping the
existing first apikey argument and the supaHost and supaAnon options unchanged.
In `@src/pages/admin/dashboard/cli.vue`:
- Around line 35-51: Update loadCliUsage to track a monotonically increasing
request identifier for each fetch, and only apply cliUsage or error state
changes when the response belongs to the latest request. Likewise, only clear
isLoadingStats in finally for the latest request so stale requests cannot
overwrite data or loading state.
In `@supabase/functions/_backend/private/config.ts`:
- Around line 12-22: Bound all six request-header values when constructing the
event in the config handler: cli_version, command, node_version, os_platform,
api_version, and the capgkey value used by trackCliUsage. Truncate each value to
a small fixed maximum at collection time, while preserving the existing defaults
and event structure.
- Around line 24-31: Add application or upstream rate limiting to the
/private/config request flow before resolveCliUsageIdentity, checkKey, or the
background trackCliUsage operation runs. Use the existing route/middleware
rate-limit mechanism if available, and ensure excess requests are rejected
without triggering Analytics Engine writes, pg pool creation, or cli_usage
inserts.
In `@supabase/functions/_backend/utils/cli_usage.ts`:
- Around line 77-102: Replace the per-event getPgClient/closeClient flow inside
the backgroundTask callback with a single-statement service-role insert through
supabaseAdmin(c), preserving all cli_usage field values and existing error
logging. Remove the dedicated pool lifecycle for this write while keeping the
asynchronous tracking behavior unchanged.
In `@supabase/migrations/20260808172409_cli_usage.sql`:
- Around line 14-16: Remove the low-cardinality cli_usage_cli_version_idx and
cli_usage_api_version_idx definitions from the migration, retaining
cli_usage_apikey_id_idx. Only keep either index if EXPLAIN (ANALYZE, BUFFERS)
for getAdminCliUsageFromPostgres demonstrates that the planner selects it.
- Around line 1-12: Plan and implement a new migration that updates
process_all_cron_tasks to purge obsolete rows from public.cli_usage using an
appropriate retention period, while preserving the existing cron-task flow. Do
not create a separate cron job; ensure the cleanup is executed as part of
process_all_cron_tasks.
In `@tests/cli-usage.unit.test.ts`:
- Around line 56-59: Add unit tests in tests/cli-usage.unit.test.ts covering
getAdminCliUsage for the Analytics Engine branch, Postgres branch, and
rejected-query fallback returning emptyAdminCliUsageStats(). Add a trackCliUsage
failure test asserting cloudlogErr is called and closeClient still executes,
reusing the existing mocks and fixtures.
---
Outside diff comments:
In `@cli/src/analytics/org-resolver.ts`:
- Around line 17-65: Update resolveOwnerOrgId so requests with a caller-provided
signal bypass ownerOrgCache entirely, including both cache reads and writes.
Retain the existing shared-cache behavior only for signal-free lookups, while
preserving the current abort handling for signaled requests.
In `@cli/src/utils.ts`:
- Around line 866-941: Pass options.signal into the getRemoteConfig call within
invokeCapgoCliApi’s configuration-resolution branch, preserving the signal
through to the remote configuration fetch so aborted API requests cancel
promptly.
🪄 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: aab14e25-0171-41be-b0d9-3bf5bfffdb85
📒 Files selected for processing (60)
cli/TODO_HTTP_API.mdcli/src/ai/analyze.tscli/src/analytics/cli-headers.tscli/src/analytics/org-resolver.tscli/src/api/app.tscli/src/api/channels.tscli/src/api/versions.tscli/src/app/add.tscli/src/app/debug.tscli/src/app/delete.tscli/src/app/list.tscli/src/app/set.tscli/src/app/updateProbe.tscli/src/build/credentials-command.tscli/src/build/onboarding/android/ui/app.tsxcli/src/build/onboarding/command.tscli/src/build/onboarding/mcp/broker-oauth.tscli/src/build/onboarding/ui/app.tsxcli/src/build/onboarding/ui/shell.tsxcli/src/build/request.tscli/src/bundle/cleanup.tscli/src/bundle/delete.tscli/src/bundle/list.tscli/src/bundle/partial.tscli/src/bundle/unlink.tscli/src/bundle/upload.tscli/src/channel/add.tscli/src/channel/currentBundle.tscli/src/channel/delete.tscli/src/channel/list.tscli/src/channel/set.tscli/src/index.tscli/src/init/command.tscli/src/init/replay.tscli/src/notifications/send-update.tscli/src/organization/delete.tscli/src/organization/members.tscli/src/organization/set.tscli/src/preview/qr.tscli/src/sdk.tscli/src/support/support-upload.tscli/src/utils.tscli/test/test-app-permission-helper.mjscli/test/test-init-app-conflict.mjscli/test/test-preview-qr.mjscloudflare_workers/api/wrangler.jsoncmessages/en.jsonprivate/cli-mcp-testssrc/constants/adminTabs.tssrc/pages/admin/dashboard/cli.vuesrc/stores/adminDashboard.tssupabase/functions/_backend/private/admin_stats.tssupabase/functions/_backend/private/config.tssupabase/functions/_backend/utils/cli_usage.tssupabase/functions/_backend/utils/cloudflare.tssupabase/functions/_backend/utils/hono.tssupabase/migrations/20260808172409_cli_usage.sqltests/admin-stats.unit.test.tstests/cli-app-permission-helper.test.tstests/cli-usage.unit.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
| function hostOptionsFromSupabase(supabase: SupabaseClient<Database>) { | ||
| // supabase-js keeps these as protected fields; local/self-host tests still | ||
| // need the same host when Capgo HTTP existence checks replace PostgREST RPCs. | ||
| // Hosted Capgo clients must keep default api.capgo.app resolution — their | ||
| // supabaseUrl points at PostgREST, not the public Capgo HTTP API. | ||
| const client = supabase as SupabaseClient<Database> & { supabaseUrl?: string, supabaseKey?: string } | ||
| const supaHost = typeof client.supabaseUrl === 'string' ? client.supabaseUrl : undefined | ||
| const supaAnon = typeof client.supabaseKey === 'string' ? client.supabaseKey : undefined | ||
| if (supaHost && supaAnon && !isCapgoManagedSupabaseHost(supaHost)) | ||
| return { supaHost, supaAnon } | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
supabase-js v2 SupabaseClient protected supabaseUrl supabaseKey properties
💡 Result:
In supabase-js v2, the supabaseUrl and supabaseKey properties are defined as protected properties within the constructor of the SupabaseClient class [1][2]. Because they are marked as protected, they are not intended to be accessed directly from an instance of the SupabaseClient object outside of the class itself or its subclasses [3][1][2]. These values are required arguments when initializing the client via the createClient function [4][5]. While they are stored internally to configure the client's various services—such as Auth, Realtime, and Rest—they are not exposed as public properties for external consumption [3][1]. If you need to access these values after the client has been initialized, you should maintain a separate reference to the URL and key strings in your own application code at the time of client creation, rather than attempting to retrieve them from the SupabaseClient instance [4][5].
Citations:
- 1: https://github.com/supabase/supabase-js/blob/b8a5d7137de9985d09fb5820b444b1f7a8a580f3/src/SupabaseClient.ts
- 2: https://github.com/supabase/supabase-js/blob/86fe3511ce5f6d4c7ccf85db8e9a824e4db60fb6/src/SupabaseClient.ts
- 3: https://github.com/supabase/supabase-js/blob/21e9d4b7/packages/core/supabase-js/src/SupabaseClient.ts
- 4: https://supabase.com/docs/reference/javascript/initializing?example=with-additional-parameters
- 5: https://github.com/supabase/supabase-js/blob/21e9d4b7/packages/core/supabase-js/src/index.ts
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Supabase dependency declarations ---'
rg -n --glob 'package.json' --glob 'package-lock.json' --glob 'pnpm-lock.yaml' --glob 'yarn.lock' 'supabase-js|2\.110\.8' . || true
printf '%s\n' '--- hostOptionsFromSupabase usages and surrounding code ---'
rg -n -C 8 'hostOptionsFromSupabase|supabaseUrl|supabaseKey' cli/src/api/app.ts cli/src || true
printf '%s\n' '--- relevant file context ---'
cat -n cli/src/api/app.ts | sed -n '130,225p'Repository: Cap-go/capgo.app
Length of output: 10224
🏁 Script executed (no clone):
#!/bin/bash
set -eu
url='https://raw.githubusercontent.com/supabase/supabase-js/v2.110.8/src/SupabaseClient.ts'
printf '%s\n' '--- SupabaseClient v2.110.8 source ---'
curl -fsSL "$url" | rg -n -C 5 'supabaseUrl|supabaseKey|constructor'
printf '%s\n' '--- tag/ref metadata ---'
curl -fsSL 'https://api.github.com/repos/supabase/supabase-js/git/ref/tags/v2.110.8' |
jq '{ref, object}'
Length of output: 213
🏁 Script executed (no clone):
#!/bin/bash
set -eu
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
printf '%s\n' '--- npm metadata for `@supabase/supabase-js` 2.110.8 ---'
curl -fsSL 'https://registry.npmjs.org/@supabase%2fsupabase-js/2.110.8' |
jq '{version, dist: {tarball, shasum}, types, main, module}'
tarball="$(curl -fsSL 'https://registry.npmjs.org/@supabase%2fsupabase-js/2.110.8' | jq -r '.dist.tarball')"
curl -fsSL "$tarball" -o "$tmpdir/package.tgz"
tar -xzf "$tmpdir/package.tgz" -C "$tmpdir"
printf '%s\n' '--- published package files containing SupabaseClient ---'
find "$tmpdir/package" -type f \( -iname '*SupabaseClient*' -o -iname '*.d.ts' \) -print
printf '%s\n' '--- supabaseUrl and supabaseKey definitions/usages ---'
rg -n -C 6 'supabaseUrl|supabaseKey' "$tmpdir/package" || true
printf '%s\n' '--- exact package version and package exports ---'
cat "$tmpdir/package/package.json" | jq '{version, main, module, types, exports}'
Length of output: 50344
🏁 Script executed (no clone):
#!/bin/bash
set -eu
printf '%s\n' '--- GitHub tag metadata for v2.110.8 ---'
curl -fsSL 'https://api.github.com/repos/supabase/supabase-js/git/ref/tags/v2.110.8' |
jq '{ref, object}'
printf '%s\n' '--- npm versions after 2.110.8 ---'
curl -fsSL 'https://registry.npmjs.org/@supabase%2fsupabase-js' |
jq -r '.versions | keys[]' |
awk '$0 >= "2.110.8"' |
tail -n 10
Length of output: 577
🏁 Script executed (no clone):
#!/bin/bash
set -eu
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
metadata="$(curl -fsSL 'https://registry.npmjs.org/@supabase%2fsupabase-js/2.110.8')"
tarball="$(printf '%s' "$metadata" | jq -r '.dist.tarball')"
curl -fsSL "$tarball" | tar -xzf - -C "$tmpdir"
printf '%s\n' '--- package metadata ---'
jq '{name, version, main, module, types, exports}' "$tmpdir/package/package.json"
printf '%s\n' '--- candidate source and declaration files ---'
find "$tmpdir/package" -type f \
\( -name 'SupabaseClient.ts' -o -name 'SupabaseClient.d.ts' -o -name '*.d.ts' \) \
-print | head -n 80
printf '%s\n' '--- non-minified matches ---'
find "$tmpdir/package" -type f \
\( -name '*.js' -o -name '*.mjs' -o -name '*.cjs' -o -name '*.d.ts' \) \
! -name '*.min.js' ! -name '*umd*' ! -name '*browser*' -print0 |
xargs -0 grep -n -E -C 3 'supabaseUrl|supabaseKey' || true
Length of output: 50346
🏁 Script executed (no clone):
#!/bin/bash
set -eu
printf '%s\n' '--- dereferenced v2.110.8 tag object ---'
tag_object='7c00ee52d75dec6fa107d471018875a33dff20e5'
curl -fsSL "https://api.github.com/repos/supabase/supabase-js/git/tags/$tag_object" |
jq '{object, tagger, message}'
printf '%s\n' '--- commit validation ---'
commit_sha="$(curl -fsSL "https://api.github.com/repos/supabase/supabase-js/git/tags/$tag_object" | jq -r '.object.sha')"
curl -fsSL "https://api.github.com/repos/supabase/supabase-js/commits/$commit_sha" |
jq '{sha, commit: {message: .commit.message}}'
Length of output: 747
🏁 Script executed (no clone):
#!/bin/bash
set -eu
commit='6b8ff73d56770715ff6e687efe2de24ba7bb68bc'
url="https://raw.githubusercontent.com/supabase/supabase-js/$commit/src/SupabaseClient.ts"
printf '%s\n' '--- tagged TypeScript declarations ---'
curl -fsSL "$url" |
grep -n -E -C 4 'protected supabaseUrl|protected supabaseKey|supabaseUrl:|supabaseKey:' || true
printf '%s\n' '--- tagged constructor assignments ---'
curl -fsSL "$url" |
grep -n -E -C 3 'this\.supabaseUrl|this\.supabaseKey' || true
Length of output: 301
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
from pathlib import Path
package = json.loads(Path("cli/package.json").read_text())
dependency = package["dependencies"]["`@supabase/supabase-js`"]
print({"cli_dependency": dependency, "allows_versions_above_2_110_8": dependency.startswith("^")})
PYRepository: Cap-go/capgo.app
Length of output: 261
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- CLI Supabase dependency context ---'
cat -n cli/package.json | sed -n '232,252p'
rg -n -C 3 '"`@supabase/supabase-js`"' cli/package.json package.jsonRepository: Cap-go/capgo.app
Length of output: 1966
🏁 Script executed (no clone):
#!/bin/bash
set -eu
commit='6b8ff73d56770715ff6e687efe2de24ba7bb68bc'
url="https://raw.githubusercontent.com/supabase/supabase-js/$commit/packages/core/supabase-js/src/SupabaseClient.ts"
printf '%s\n' '--- tagged TypeScript field declarations ---'
curl -fsSL "$url" |
grep -n -E -C 4 'protected supabaseUrl|protected supabaseKey|supabaseUrl:|supabaseKey:' || true
printf '%s\n' '--- tagged constructor assignments ---'
curl -fsSL "$url" |
grep -n -E -C 3 'this\.supabaseUrl|this\.supabaseKey' || true
Length of output: 1200
Pass host options explicitly
@supabase/supabase-js 2.110.8 creates runtime properties named supabaseUrl and supabaseKey. However, cli/package.json permits newer versions with ^2.110.8, and these fields are protected internals. Pass host options explicitly to avoid silent fallback to api.capgo.app if the fields change.
🤖 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 `@cli/src/api/app.ts` around lines 183 - 194, Update the caller of
hostOptionsFromSupabase to receive the Supabase host and key explicitly from the
configuration instead of reading protected supabase.supabaseUrl and
supabase.supabaseKey internals. Refactor hostOptionsFromSupabase to accept those
explicit values, retain the isCapgoManagedSupabaseHost filtering, and ensure
self-hosted clients continue receiving { supaHost, supaAnon } while managed
clients preserve default resolution.
| if (error) | ||
| return true | ||
| return (data?.length ?? 0) > 0 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Distinguish a query failure from a real cross-channel link.
isVersionLinkedToOtherChannel returns true on any query error. The caller in cli/src/channel/delete.ts then logs Keeping bundle <name>; it is still linked to another channel. That message is wrong when the cause is a failed query. Return the error state so the caller can report the real reason.
♻️ Proposed change to surface the failure reason
export async function isVersionLinkedToOtherChannel(
supabase: SupabaseClient<Database>,
appId: string,
versionId: number,
excludeChannelName: string,
-): Promise<boolean> {
+): Promise<{ linked: boolean, error: unknown }> {
const { data, error } = await supabase
.from('channels')
.select('id')
.eq('app_id', appId)
.neq('name', excludeChannelName)
.or(`version.eq.${versionId},rollout_version.eq.${versionId}`)
.limit(1)
if (error)
- return true
- return (data?.length ?? 0) > 0
+ return { linked: true, error }
+ return { linked: (data?.length ?? 0) > 0, error: null }
}Update the call site in cli/src/channel/delete.ts to log the error message when error is set.
🤖 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 `@cli/src/api/channels.ts` around lines 263 - 265, Update
isVersionLinkedToOtherChannel to distinguish query failures from confirmed links
by returning the error state separately rather than treating any error as true.
Adjust the caller in the channel deletion flow to inspect that error and log its
message when present; only log that the bundle remains linked when the query
succeeds and finds another channel.
| async function isEmptyBundleListError(error: unknown) { | ||
| const payload = await readCapgoCliApiErrorPayload(error) | ||
| return payload?.error === 'cannot_get_bundle' && payload?.message === 'Cannot get bundle' | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Match the error code only, not the message text.
isEmptyBundleListError compares both error and message. The message string is display text and can change on the backend without a contract change. A change breaks the empty-list detection and turns an empty bundle list into a thrown error. Match on the cannot_get_bundle code alone.
♻️ Proposed change
async function isEmptyBundleListError(error: unknown) {
const payload = await readCapgoCliApiErrorPayload(error)
- return payload?.error === 'cannot_get_bundle' && payload?.message === 'Cannot get bundle'
+ return payload?.error === 'cannot_get_bundle'
}📝 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.
| async function isEmptyBundleListError(error: unknown) { | |
| const payload = await readCapgoCliApiErrorPayload(error) | |
| return payload?.error === 'cannot_get_bundle' && payload?.message === 'Cannot get bundle' | |
| } | |
| async function isEmptyBundleListError(error: unknown) { | |
| const payload = await readCapgoCliApiErrorPayload(error) | |
| return payload?.error === 'cannot_get_bundle' | |
| } |
🤖 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 `@cli/src/api/versions.ts` around lines 28 - 31, Update isEmptyBundleListError
to determine emptiness solely from payload?.error matching cannot_get_bundle;
remove the payload?.message comparison so backend display-text changes do not
affect empty bundle-list detection.
| if (supabase) { | ||
| const { error: delAppSpecVersionError } = await supabase | ||
| .from('app_versions') | ||
| .update({ deleted: true }) | ||
| .eq('app_id', appid) | ||
| .eq('deleted', false) | ||
| .eq('name', bundle) | ||
|
|
||
| if (delAppSpecVersionError) { | ||
| const message = `App version ${appid}@${bundle} not found in database` | ||
| if (!silent) | ||
| log.error(message) | ||
| throw new Error(`${message}: ${formatError(delAppSpecVersionError)}`) | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The Supabase branch reports success when no bundle matches.
A PostgREST update that matches zero rows returns no error. The App version ... not found in database error is therefore unreachable on this branch, while the HTTP branch below still fails for a missing bundle. The same function then has two different contracts.
This also affects cli/src/channel/delete.ts: it records the bundle name in softDeletedBundleNames even when nothing was soft-deleted, so the rollback can set deleted = false on a bundle that was already deleted before the command ran.
Request the affected rows and check the count.
🐛 Proposed fix
if (supabase) {
- const { error: delAppSpecVersionError } = await supabase
+ const { data: updated, error: delAppSpecVersionError } = await supabase
.from('app_versions')
.update({ deleted: true })
.eq('app_id', appid)
.eq('deleted', false)
.eq('name', bundle)
+ .select('name')
- if (delAppSpecVersionError) {
+ if (delAppSpecVersionError || !updated?.length) {
const message = `App version ${appid}@${bundle} not found in database`
if (!silent)
log.error(message)
throw new Error(`${message}: ${formatError(delAppSpecVersionError)}`)
}
return
}📝 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.
| if (supabase) { | |
| const { error: delAppSpecVersionError } = await supabase | |
| .from('app_versions') | |
| .update({ deleted: true }) | |
| .eq('app_id', appid) | |
| .eq('deleted', false) | |
| .eq('name', bundle) | |
| if (delAppSpecVersionError) { | |
| const message = `App version ${appid}@${bundle} not found in database` | |
| if (!silent) | |
| log.error(message) | |
| throw new Error(`${message}: ${formatError(delAppSpecVersionError)}`) | |
| } | |
| return | |
| } | |
| if (supabase) { | |
| const { data: updated, error: delAppSpecVersionError } = await supabase | |
| .from('app_versions') | |
| .update({ deleted: true }) | |
| .eq('app_id', appid) | |
| .eq('deleted', false) | |
| .eq('name', bundle) | |
| .select('name') | |
| if (delAppSpecVersionError || !updated?.length) { | |
| const message = `App version ${appid}@${bundle} not found in database` | |
| if (!silent) | |
| log.error(message) | |
| throw new Error(`${message}: ${formatError(delAppSpecVersionError)}`) | |
| } | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/src/api/versions.ts` around lines 74 - 89, Update the Supabase branch in
the surrounding delete function to request the affected-row count and treat a
zero count as “App version ${appid}@${bundle} not found in database,” while
preserving the existing silent logging and formatted-error behavior. Ensure the
success path returns only when a bundle was actually soft-deleted, so delete.ts
records names for rollback only after an update affects a row.
| export async function getChannelsVersion( | ||
| supabase: SupabaseClient<Database>, | ||
| options: CapgoHttpOptions, | ||
| appid: string, | ||
| options: VersionOptions = {}, | ||
| ) { | ||
| const { silent = false } = options | ||
|
|
||
| const { data: channels, error: channelsError } = await supabase | ||
| .from('channels') | ||
| .select('version') | ||
| .eq('app_id', appid) | ||
|
|
||
| if (channelsError) { | ||
| const message = `App ${appid} not found in database` | ||
| if (!silent) | ||
| log.error(message) | ||
| throw new Error(`${message}: ${formatError(channelsError)}`) | ||
| const versions: Array<number | null> = [] | ||
| let page = 0 | ||
| while (true) { | ||
| const params = new URLSearchParams({ app_id: appid, page: String(page) }) | ||
| const { data: channels, error: channelsError } = await invokeCapgoCliApi<Array<{ version?: { id?: number } | null }>>( | ||
| `channel?${params.toString()}`, | ||
| { | ||
| apikey: options.apikey, | ||
| method: 'GET', | ||
| body: undefined, | ||
| supaHost: options.supaHost, | ||
| supaAnon: options.supaAnon, | ||
| }, | ||
| ) | ||
| if (channelsError) { | ||
| const message = `App ${appid} not found in database` | ||
| if (!options.silent) | ||
| log.error(message) | ||
| throw new Error(`${message}: ${formatError(channelsError)}`) | ||
| } | ||
| const batch = Array.isArray(channels) ? channels : [] | ||
| if (!batch.length) | ||
| break | ||
| versions.push(...batch.map(c => c.version?.id ?? null)) | ||
| if (batch.length < 50) | ||
| break | ||
| page += 1 | ||
| } | ||
|
|
||
| return (channels ?? []).map(c => c.version) | ||
| return versions | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Reuse BUNDLE_PAGE_SIZE and fetchChannelsPage semantics for the page size.
getChannelsVersion hard-codes 50 as the page size. The same literal exists in cli/src/api/channels.ts and the named constant BUNDLE_PAGE_SIZE exists at the top of this file. Use one shared constant so a backend page-size change needs one edit.
🤖 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 `@cli/src/api/versions.ts` around lines 179 - 212, Update getChannelsVersion to
use the existing BUNDLE_PAGE_SIZE constant instead of the hard-coded 50 when
determining whether the fetched batch is the final page. Preserve the current
pagination and fetchChannelsPage boundary semantics so page traversal remains
consistent with the shared page size.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm the backend page size for the app, bundle, and channel list endpoints.
set -euo pipefail
rg -n -C 8 '\.range\(|limit\(|paginat' supabase/functions/_backend --glob '**/*.ts' | rg -n -C 4 'app|bundle|channel'Repository: Cap-go/capgo.app
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- CLI page-size definitions and imports ---'
rg -n -C 5 'BUNDLE_PAGE_SIZE|fetchLimit|batch\.length < 50|page[=:]' cli/src/api/versions.ts cli/src/api/channels.ts cli/src/app/list.ts cli/src 2>/dev/null | head -n 240
printf '%s\n' '--- Backend pagination constants and handlers ---'
rg -n -C 6 'fetchLimit|itemsPerPage|range\(from, to\)|body\.page|page.*50|50.*page' supabase/functions/_backend/public/app/get.ts supabase/functions/_backend/public/channel/get.ts supabase/functions/_backend/public/bundle/get.ts supabase/functions/_backend/utils/utils.ts
printf '%s\n' '--- API invocation and endpoint callers ---'
rg -n -C 8 'function invokeCapgoCliApi|const invokeCapgoCliApi|export .*invokeCapgoCliApi|fetchChannelsPage|channel\?|app\?page=' cli/srcRepository: Cap-go/capgo.app
Length of output: 37148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
backend_files = [
Path('supabase/functions/_backend/utils/utils.ts'),
Path('supabase/functions/_backend/public/app/get.ts'),
Path('supabase/functions/_backend/public/channel/get.ts'),
Path('supabase/functions/_backend/public/bundle/get.ts'),
]
cli_files = list(Path('cli/src').rglob('*.ts')) + list(Path('cli/src').rglob('*.tsx'))
utils = backend_files[0].read_text()
m = re.search(r'export const fetchLimit\s*=\s*(\d+)', utils)
backend_limit = int(m.group(1)) if m else None
print(f'backend.fetchLimit={backend_limit}')
for path in backend_files[1:]:
text = path.read_text()
print(f'{path}: uses fetchLimit={text.count("fetchLimit")} range calls={text.count(".range(")}')
pattern = re.compile(r'if\s*\(batch\.length\s*<\s*([A-Za-z_][A-Za-z0-9_]*|\d+)\)')
for path in cli_files:
text = path.read_text()
matches = list(pattern.finditer(text))
for match in matches:
line = text.count('\n', 0, match.start()) + 1
print(f'{path}:{line}: termination-size={match.group(1)}')
PYRepository: Cap-go/capgo.app
Length of output: 761
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
backend_files = [
Path('supabase/functions/_backend/utils/utils.ts'),
Path('supabase/functions/_backend/public/app/get.ts'),
Path('supabase/functions/_backend/public/channel/get.ts'),
Path('supabase/functions/_backend/public/bundle/get.ts'),
]
cli_files = list(Path('cli/src').rglob('*.ts')) + list(Path('cli/src').rglob('*.tsx'))
utils = backend_files[0].read_text()
m = re.search(r'export const fetchLimit\s*=\s*(\d+)', utils)
backend_limit = int(m.group(1)) if m else None
print(f'backend.fetchLimit={backend_limit}')
for path in backend_files[1:]:
text = path.read_text()
print(f'{path}: uses fetchLimit={text.count("fetchLimit")} range calls={text.count(".range(")}')
pattern = re.compile(r'if\s*\(batch\.length\s*<\s*([A-Za-z_][A-Za-z0-9_]*|\d+)\)')
for path in cli_files:
text = path.read_text()
for match in pattern.finditer(text):
line = text.count('\n', 0, match.start()) + 1
print(f'{path}:{line}: termination-size={match.group(1)}')
PYRepository: Cap-go/capgo.app
Length of output: 761
Centralize the CLI page-size constant. The backend uses fetchLimit = 50, so the current loops paginate correctly. Replace the duplicated 50 values in all seven CLI loops, including cli/src/preview/qr.ts and cli/src/api/app.ts, with a shared constant from a neutral CLI module. Do not export the constant from versions.ts.
📍 Affects 3 files
cli/src/api/versions.ts#L179-L212(this comment)cli/src/api/channels.ts#L301-L322cli/src/app/list.ts#L21-L55
🤖 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 `@cli/src/api/versions.ts` around lines 179 - 212, Centralize the CLI
pagination page-size value by defining a shared constant in a neutral CLI
module, then replace the duplicated literal 50 in all seven pagination loops,
including getChannelsVersion and the loops in cli/src/api/channels.ts (301-322)
and cli/src/app/list.ts (21-55), plus cli/src/preview/qr.ts and
cli/src/api/app.ts. Import and reuse that constant at each site; do not export
it from versions.ts.
| backgroundTask(c, (async () => { | ||
| const identity = await resolveCliUsageIdentity(c, capgkey) | ||
| trackCliUsage(c, { | ||
| ...event, | ||
| apikey_id: identity.apikey_id, | ||
| org_id: identity.org_id, | ||
| }) | ||
| })()) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check middleware and rate limiting applied to the /private/config route.
set -euo pipefail
fd -t f 'config.ts' supabase/functions/_backend/private --exec cat -n {}
# Find where the private config route is mounted and which middleware wraps it.
rg -n -C 5 "private/config|'/config'" --glob '*.ts' supabase/functions
# Look for rate limiting helpers in the backend.
rg -n -C 3 'rateLimit|rate_limit|ratelimit' --glob '*.ts' supabase/functions/_backend | head -50Repository: Cap-go/capgo.app
Length of output: 12714
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- private function entrypoint ---'
cat -n supabase/functions/private/index.ts | sed -n '1,90p'
printf '%s\n' '--- CLI usage implementation ---'
cat -n supabase/functions/_backend/utils/cli_usage.ts | sed -n '1,220p'
printf '%s\n' '--- referenced helpers and call sites ---'
rg -n -C 5 'function (resolveCliUsageIdentity|trackCliUsage)|const (resolveCliUsageIdentity|trackCliUsage)|checkKey|new Pool|createPool|backgroundTask' supabase/functions/_backend supabase/functions/private --glob '*.ts'Repository: Cap-go/capgo.app
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- createHono middleware ---'
rg -n -C 12 'export function createHono|function createHono|middlewareAuth|middlewareKey|middlewareAPISecret|app\.use' supabase/functions/_backend/utils/hono.ts
printf '%s\n' '--- pool creation and background task ---'
rg -n -C 12 'export function getPgClient|function getPgClient|export async function closeClient|function closeClient|export function backgroundTask|function backgroundTask' \
supabase/functions/_backend/utils/pg.ts supabase/functions/_backend/utils/utils.ts
printf '%s\n' '--- config route and route-specific middleware ---'
cat -n supabase/functions/_backend/private/config.ts
rg -n -C 8 'appGlobal\.(use|route)|middleware(Auth|Key|APISecret).*config|config.*middleware' supabase/functions/private/index.ts supabase/functions/_backend/private/config.ts supabase/functions/_backend/utils/hono.tsRepository: Cap-go/capgo.app
Length of output: 17790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- remaining createHono middleware ---'
cat -n supabase/functions/_backend/utils/hono.ts | sed -n '291,390p'
printf '%s\n' '--- exact pool implementation ---'
cat -n supabase/functions/_backend/utils/pg.ts | sed -n '360,430p'
printf '%s\n' '--- infrastructure and route guards ---'
rg -n -C 4 'private|rate.limit|rate_limit|rateLimit|firewall|WAF|throttle|shield|x-cli-version' \
--glob '!supabase/functions/_backend/**' \
--glob '!node_modules' \
--glob '!dist' \
--glob '!build' .
printf '%s\n' '--- all middleware on the private app ---'
rg -n -C 3 'appGlobal\.use|appGlobal\.route|middleware(Auth|Key|APISecret)|useCors' \
supabase/functions/private supabase/functions/_backend/private/config.ts supabase/functions/_backend/utils/hono.tsRepository: Cap-go/capgo.app
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- createHono middleware ---'
cat -n supabase/functions/_backend/utils/hono.ts | sed -n '291,380p'
printf '%s\n' '--- pool implementation ---'
cat -n supabase/functions/_backend/utils/pg.ts | sed -n '360,410p'
printf '%s\n' '--- infrastructure references, excluding generated output ---'
rg -n -C 3 'private|rate.limit|rate_limit|rateLimit|firewall|WAF|throttle|shield|x-cli-version' \
--glob '!graphify-out/**' \
--glob '!node_modules/**' \
--glob '!dist/**' \
--glob '!build/**' \
--glob '!coverage/**' \
--glob '!supabase/functions/_backend/**' \
. | head -300
printf '%s\n' '--- private app middleware references ---'
rg -n -C 3 'appGlobal\.use|appGlobal\.route|middleware(Auth|Key|APISecret)|useCors' \
supabase/functions/private supabase/functions/_backend/private/config.ts supabase/functions/_backend/utils/hono.tsRepository: Cap-go/capgo.app
Length of output: 35784
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
config = Path("supabase/functions/_backend/private/config.ts").read_text()
usage = Path("supabase/functions/_backend/utils/cli_usage.ts").read_text()
entry = Path("supabase/functions/private/index.ts").read_text()
hono = Path("supabase/functions/_backend/utils/hono.ts").read_text()
pg = Path("supabase/functions/_backend/utils/pg.ts").read_text()
checks = {
"config route is mounted at /config": "appGlobal.route('/config', config)" in entry,
"config app has no auth middleware": not re.search(r"middleware(?:Auth|Key|APISecret)", config),
"x-cli-version gates background work": "if (cliVersion)" in config and "backgroundTask(c" in config,
"capgkey is optional": "?? undefined" in config,
"API-key identity uses checkKey": "checkKey(c, capgkey, supabaseAdmin(c))" in usage,
"Analytics Engine is conditional": "getRuntimeKey() === 'workerd' && !!c.env.CLI_USAGE" in usage,
"Postgres path creates a Pool": "const pool = new Pool(options)" in pg,
"Postgres path inserts cli_usage": "INSERT INTO public.cli_usage" in usage,
"private app has no rate-limit middleware in its entrypoint": not re.search(r"rateLimit|rate_limit|RateLimit", entry),
}
for label, result in checks.items():
print(f"{'PASS' if result else 'FAIL'}: {label}")
print("\nBehavior matrix:")
print("- x-cli-version absent: no identity lookup and no usage write")
print("- x-cli-version present, capgkey absent: no checkKey lookup; usage write is still scheduled")
print("- x-cli-version present, capgkey present: checkKey lookup is scheduled, then usage write is scheduled")
print("- workerd with CLI_USAGE: Analytics Engine write")
print("- otherwise: new Pool plus cli_usage INSERT, then client close")
PYRepository: Cap-go/capgo.app
Length of output: 902
Rate-limit CLI usage tracking on /private/config.
x-cli-version is caller-controlled, and the route has no authentication or rate-limit middleware. Requests can trigger checkKey and then an Analytics Engine write or a new pg pool plus cli_usage insert. Add an application or upstream rate limit.
🤖 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 `@supabase/functions/_backend/private/config.ts` around lines 24 - 31, Add
application or upstream rate limiting to the /private/config request flow before
resolveCliUsageIdentity, checkKey, or the background trackCliUsage operation
runs. Use the existing route/middleware rate-limit mechanism if available, and
ensure excess requests are rejected without triggering Analytics Engine writes,
pg pool creation, or cli_usage inserts.
| backgroundTask(c, (async () => { | ||
| const pgClient = getPgClient(c, false) | ||
| try { | ||
| await pgClient.query( | ||
| `INSERT INTO public.cli_usage | ||
| (cli_version, command, node_version, os_platform, apikey_id, org_id, source, api_version) | ||
| VALUES ($1, $2, $3, $4, $5::uuid, $6::uuid, $7, $8)`, | ||
| [ | ||
| event.cli_version, | ||
| event.command, | ||
| event.node_version, | ||
| event.os_platform, | ||
| event.apikey_id, | ||
| event.org_id, | ||
| event.source, | ||
| event.api_version, | ||
| ], | ||
| ) | ||
| } | ||
| catch (error) { | ||
| cloudlogErr({ requestId: c.get('requestId'), message: 'trackCliUsage insert error', error: serializeError(error) }) | ||
| } | ||
| finally { | ||
| await closeClient(c, pgClient) | ||
| } | ||
| })()) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Avoid a new Postgres pool for each tracked event.
getPgClient constructs a new Pool on every call. This path runs once per /private/config request on non-workerd runtimes. Each event therefore pays a full connect and teardown for one INSERT.
A single-statement service-role insert through supabaseAdmin(c) is the cheaper abstraction here, and it matches the existing convention for one-statement internal writes. Alternatively, buffer events and flush them in batches.
Based on learnings: "it's acceptable to use supabaseAdmin(c) for single-statement, non-transactional service-role writes... Prefer getPgClient()/getDrizzleClient() when the write is multi-step, involves joins, needs transactions, or is schema-heavy."
🤖 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 `@supabase/functions/_backend/utils/cli_usage.ts` around lines 77 - 102,
Replace the per-event getPgClient/closeClient flow inside the backgroundTask
callback with a single-statement service-role insert through supabaseAdmin(c),
preserving all cli_usage field values and existing error logging. Remove the
dedicated pool lifecycle for this write while keeping the asynchronous tracking
behavior unchanged.
Source: Learnings
| CREATE TABLE public.cli_usage ( | ||
| id bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, | ||
| created_at timestamptz NOT NULL DEFAULT now(), | ||
| cli_version text NOT NULL, | ||
| command text NOT NULL DEFAULT '', | ||
| node_version text NOT NULL DEFAULT '', | ||
| os_platform text NOT NULL DEFAULT '', | ||
| apikey_id uuid NULL, | ||
| org_id uuid NULL, | ||
| source text NOT NULL DEFAULT 'config', | ||
| api_version text NOT NULL DEFAULT '' | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial
Plan a retention policy for public.cli_usage.
The table is append-only. One row is written for each /private/config request that carries a CLI version header, and nothing deletes old rows. The admin dashboard reads only a bounded date range, so old rows have no reader.
Add the purge to process_all_cron_tasks in a new migration. Do not add a separate cron job.
Do you want me to draft the retention migration?
🤖 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 `@supabase/migrations/20260808172409_cli_usage.sql` around lines 1 - 12, Plan
and implement a new migration that updates process_all_cron_tasks to purge
obsolete rows from public.cli_usage using an appropriate retention period, while
preserving the existing cron-task flow. Do not create a separate cron job;
ensure the cleanup is executed as part of process_all_cron_tasks.
Source: Coding guidelines
| CREATE INDEX cli_usage_cli_version_idx ON public.cli_usage (cli_version); | ||
| CREATE INDEX cli_usage_apikey_id_idx ON public.cli_usage (apikey_id) WHERE apikey_id IS NOT NULL; | ||
| CREATE INDEX cli_usage_api_version_idx ON public.cli_usage (api_version); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Reconsider the two low-cardinality indexes.
getAdminCliUsageFromPostgres filters only on created_at and groups by cli_version and api_version. A grouped aggregate over a date range uses cli_usage_created_at_idx, not a plain b-tree on a low-cardinality text column. cli_usage_cli_version_idx and cli_usage_api_version_idx therefore add write cost to a high-volume insert path without serving a known read.
Drop both indexes unless an EXPLAIN (ANALYZE, BUFFERS) run shows that the planner selects them.
Based on the coding guidelines: "Before adding or changing PostgreSQL functions used by policies, views, triggers, RPCs, or hot endpoints, document execution frequency, roles, cardinalities, indexes, and validate worst cases with EXPLAIN (ANALYZE, BUFFERS)."
🧰 Tools
🪛 SQLFluff (4.2.2)
[error] 14-14: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 15-15: Line is too long (97 > 80).
(LT05)
[error] 15-15: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 16-16: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
🤖 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 `@supabase/migrations/20260808172409_cli_usage.sql` around lines 14 - 16,
Remove the low-cardinality cli_usage_cli_version_idx and
cli_usage_api_version_idx definitions from the migration, retaining
cli_usage_apikey_id_idx. Only keep either index if EXPLAIN (ANALYZE, BUFFERS)
for getAdminCliUsageFromPostgres demonstrates that the planner selects it.
Source: Coding guidelines
| vi.mock('../supabase/functions/_backend/utils/cloudflare.ts', () => ({ | ||
| formatDateCF: (value: string) => value.replace('T', ' ').replace('Z', ''), | ||
| runQueryToCFA: vi.fn(), | ||
| })) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add coverage for the aggregation path.
The file already mocks formatDateCF and runQueryToCFA, but no test calls getAdminCliUsage. That function feeds the admin dashboard, and it swallows every error and returns empty statistics. A regression there is silent.
Add tests for three cases: the Analytics Engine branch, the Postgres branch, and the fallback that returns emptyAdminCliUsageStats() when a query rejects. Also cover the trackCliUsage failure path, which must call cloudlogErr and still call closeClient.
🤖 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/cli-usage.unit.test.ts` around lines 56 - 59, Add unit tests in
tests/cli-usage.unit.test.ts covering getAdminCliUsage for the Analytics Engine
branch, Postgres branch, and rejected-query fallback returning
emptyAdminCliUsageStats(). Add a trackCliUsage failure test asserting
cloudlogErr is called and closeClient still executes, reusing the existing mocks
and fixtures.
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_b60423e7-1006-4239-8715-0034718f32bb) |
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Main added 20260808175552_app_preview_public_channel_guard.sql, so the cli_usage migration timestamp must sort after it for db push / CI order. 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_d7aa3de0-dfbd-4939-8a42-3e7561a1608b) |
|
2 participants
Summary (AI generated)
cli_usage_alpha, dashboard distinct counts, config identity off hot pathMotivation (AI generated)
Review threads on #2906 / #2945 called out correctness gaps after the Capgo HTTP CLI migration.
Business Impact (AI generated)
Keeps preview-key and self-host flows correct while shipping CLI analytics.
Test Plan (AI generated)
bun run --cwd cli typecheck/lintbun run typecheck:backendbunx vitest run tests/cli-usage.unit.test.ts9874f6c1cGenerated with AI
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Bug Fixes