Merged
Conversation
WalkthroughAdds a new telemetry function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
src/commands/storage/buckets.ts (1)
30-33:⚠️ Potential issue | 🟠 MajorEmpty-result success path skips telemetry.
When no buckets exist, Line 32 returns before Line 39. That loses usage events for valid successful executions.
🔧 Proposed fix
if (buckets.length === 0) { console.log('No buckets found.'); + await reportCliUsage('cli.storage.buckets', true); return; } @@ await reportCliUsage('cli.storage.buckets', true);Also applies to: 39-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/storage/buckets.ts` around lines 30 - 33, The early return when buckets.length === 0 in the buckets command skips emitting the success telemetry; update the success path in the function handling the buckets command so that telemetry (e.g., the existing telemetry/usage event emitter) is invoked before any return in the "no buckets" branch and any other early-success return (the return around line 39). Concretely, call the same telemetry success/usage function used for the normal-result path immediately before console.log('No buckets found.') return, or refactor to emit success telemetry in a single post-success step (or finally block) that runs for all successful exits of the buckets command (referencing the buckets variable and the early return locations).src/commands/db/tables.ts (1)
23-26:⚠️ Potential issue | 🟠 MajorEmpty table list path returns before telemetry.
Line 25 exits before Line 32, so successful no-data executions are not reported.
🔧 Proposed fix
if (tables.length === 0) { console.log('No tables found.'); + await reportCliUsage('cli.db.tables', true); return; } @@ await reportCliUsage('cli.db.tables', true);Also applies to: 32-32
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/db/tables.ts` around lines 23 - 26, The early return inside the tables.length === 0 branch prevents the telemetry/send (the block with "console.log('No tables found.'); return;") from executing, so move or invoke the telemetry reporting (the telemetry.trackEvent/sendTelemetry/reportSuccess call currently executed later around the line with the successful execution telemetry) into that empty-list branch before returning, or refactor to set a success flag and call the existing telemetry-reporting code after the if block; specifically modify the tables.length === 0 handling so it logs "No tables found.", then calls the same telemetry routine (the existing telemetry tracking function used later) and only then returns.src/commands/metadata.ts (1)
21-24:⚠️ Potential issue | 🟠 MajorJSON mode currently bypasses telemetry.
Line 23 returns before Line 98, so successful
--jsonruns are never reported.🔧 Proposed fix
if (json) { outputJson(data); + await reportCliUsage('cli.metadata', true); return; } @@ - await reportCliUsage('cli.metadata', true); + await reportCliUsage('cli.metadata', true);Also applies to: 98-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/metadata.ts` around lines 21 - 24, The JSON branch in metadata command returns immediately after calling outputJson, which skips the telemetry success reporting executed later; before returning from the json path in the function containing outputJson, invoke the same telemetry reporting logic used at the successful end of the handler (i.e., call the existing telemetry reporting function or share the success-reporting code path used around line 98), or refactor so both JSON and non-JSON flows run the common success telemetry/reporting code (move it into a shared helper or finally/after-success block) to ensure --json runs are tracked.src/commands/db/functions.ts (1)
27-30:⚠️ Potential issue | 🟠 MajorTelemetry is skipped when there are no functions.
Line 29 returns before Line 36, so successful empty results are not included in usage reporting.
🔧 Proposed fix
if (functions.length === 0) { console.log('No database functions found.'); + await reportCliUsage('cli.db.functions', true); return; } @@ await reportCliUsage('cli.db.functions', true);Also applies to: 36-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/db/functions.ts` around lines 27 - 30, The early return after logging "No database functions found." prevents emitting the successful-lookup telemetry; in the function that lists DB functions (the function containing the console.log 'No database functions found.' and the subsequent telemetry/reporting call), remove the premature return and ensure the same success telemetry path is executed for empty results (or explicitly call the telemetry/reporting method with an empty-results payload) before returning; update the code paths around the console.log and the telemetry/reporting function (the telemetry helper used later in this file) so both empty and non-empty function lists report usage.src/commands/secrets/list.ts (1)
30-33:⚠️ Potential issue | 🟠 MajorNo-secrets success path exits before usage reporting.
Line 32 returns before Line 51, so successful empty list executions are missed by telemetry.
🔧 Proposed fix
if (!secrets.length) { console.log('No secrets found.'); + await reportCliUsage('cli.secrets.list', true); return; } @@ await reportCliUsage('cli.secrets.list', true);Also applies to: 51-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/secrets/list.ts` around lines 30 - 33, The early return in the empty-list branch (the if that checks secrets.length and calls console.log('No secrets found.')) exits before the command's telemetry/usage reporting runs, so move or add the invocation of the telemetry/usage-reporting function (the function used elsewhere to record successful command usage) to execute immediately before returning from that branch; similarly, apply the same change to the other early-return location so the usage-reporting call always runs on successful empty-list paths.src/commands/db/triggers.ts (1)
27-30:⚠️ Potential issue | 🟠 MajorTelemetry is skipped when no triggers are found.
Line 29 returns before Line 44, so successful
db triggersruns with empty results are not reported.Suggested fix
if (json) { outputJson(raw); } else { if (triggers.length === 0) { console.log('No database triggers found.'); + await reportCliUsage('cli.db.triggers', true); return; } outputTable( ['Name', 'Table', 'Timing', 'Events', 'ActionOrientation', 'ActionCondition', 'ActionStatement'], triggers.map((t) => [ t.triggerName, t.tableName, t.actionTiming, t.eventManipulation, t.actionOrientation, t.actionCondition ?? '-', t.actionStatement, ]), ); } await reportCliUsage('cli.db.triggers', true);Also applies to: 44-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/db/triggers.ts` around lines 27 - 30, The early return inside the triggers.length === 0 branch (the console.log('No database triggers found.') followed by return) prevents the telemetry/metric call executed later (the success telemetry at the end of the `db triggers` flow) from running; remove the early return (or call the telemetry success function before returning) so that empty-result runs still emit telemetry. Concretely, modify the triggers.length check in src/commands/db/triggers.ts to either (a) call the existing telemetry/success reporter used later in this file (the same function invoked at the end of the command) immediately after logging 'No database triggers found.' and then return, or (b) remove the return and allow execution to continue to the existing telemetry path — ensure you reference the exact telemetry function used at line ~44 so that empty-result executions are recorded.src/commands/db/indexes.ts (1)
27-30:⚠️ Potential issue | 🟠 MajorNo-results success path skips telemetry.
On Line 29, the early
returnbypasses Line 42, so successful executions with zero indexes are not reported.Suggested fix
if (json) { outputJson(raw); } else { if (indexes.length === 0) { console.log('No database indexes found.'); + await reportCliUsage('cli.db.indexes', true); return; } outputTable( ['Table', 'Index Name', 'Definition', 'Unique', 'Primary'], indexes.map((i) => [ i.tableName, i.indexName, i.indexDef, i.isUnique ? 'Yes' : 'No', i.isPrimary ? 'Yes' : 'No', ]), ); } await reportCliUsage('cli.db.indexes', true);Also applies to: 42-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/db/indexes.ts` around lines 27 - 30, When indexes.length === 0 the function logs "No database indexes found." and returns, which skips the success telemetry emitted later; update the early-exit path by invoking the same telemetry/success-reporting call used at the normal-success path (the telemetry call on line 42) before returning (or refactor so both paths fall through to the existing success-report routine) so that zero-index results are also reported; target the block containing indexes.length === 0, console.log('No database indexes found.'), and the early return and ensure it calls the same telemetry function used for successful executions.src/commands/deployments/list.ts (1)
34-37:⚠️ Potential issue | 🟠 MajorEmpty-list success path is not tracked.
On Line 36, early return skips telemetry on Line 49, so successful runs with no deployments are omitted from usage metrics.
Suggested fix
if (json) { outputJson(raw); } else { if (!deployments.length) { console.log('No deployments found.'); + await reportCliUsage('cli.deployments.list', true); return; } outputTable( ['ID', 'Status', 'Provider', 'URL', 'Created'], deployments.map((d) => [ d.id, d.status, d.provider, d.url ?? '-', new Date(d.createdAt).toLocaleString(), ]), ); } await reportCliUsage('cli.deployments.list', true);Also applies to: 49-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/deployments/list.ts` around lines 34 - 37, The early return when deployments.length === 0 skips the success telemetry sent later in this file; before logging "No deployments found." and returning, invoke the same telemetry success tracking call used on the normal success path (the success telemetry invocation present later in this module) and record the empty-result (zero deployments) outcome so empty-list runs are counted; apply the same change to any other early-return branches that currently bypass the success telemetry.src/commands/db/policies.ts (1)
27-30:⚠️ Potential issue | 🟠 MajorTelemetry misses empty-result runs and all failures.
Line 29 exits before Line 43, so
cli.db.policiesis not reported when no policies exist. Failures in the catch path are also not reported withsuccess=false.Also applies to: 43-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/db/policies.ts` around lines 27 - 30, The telemetry path is skipped when no policies are found and on errors; update the handler in src/commands/db/policies.ts so that the telemetry event for "cli.db.policies" is always emitted: when policies.length === 0 call the same telemetry success reporting used for normal runs (e.g., mark success=true) before returning, and in the catch block emit the telemetry with success=false and include the error details. Locate the early-return branch (policies.length === 0) and the catch handler and add calls to the existing telemetry reporter (the same function used on successful runs) with appropriate success flags and error info.src/commands/storage/list-objects.ts (1)
61-64:⚠️ Potential issue | 🟠 MajorTelemetry is skipped on empty results and never records failures.
Line 63 returns before Line 79, so empty-bucket executions are not tracked. Also, only success is reported; exceptions in Line 80 path never emit
success=false, which skews success metrics.🔧 Suggested fix (move telemetry to
finallyand track status)- try { + let usageSuccess = false; + try { await requireAuth(); @@ - if (objects.length === 0) { - console.log(`No objects found in bucket "${bucket}".`); - return; - } - const total = raw.pagination?.total; - if (total !== undefined) { - console.log(`Showing ${objects.length} of ${total} objects:\n`); - } - outputTable( - ['Key', 'Size', 'Type', 'Uploaded At'], - objects.map((o) => [ - o.key, - formatSize(o.size), - o.mimeType ?? '-', - o.uploadedAt, - ]), - ); + if (objects.length === 0) { + console.log(`No objects found in bucket "${bucket}".`); + } else { + const total = raw.pagination?.total; + if (total !== undefined) { + console.log(`Showing ${objects.length} of ${total} objects:\n`); + } + outputTable( + ['Key', 'Size', 'Type', 'Uploaded At'], + objects.map((o) => [ + o.key, + formatSize(o.size), + o.mimeType ?? '-', + o.uploadedAt, + ]), + ); + } } - await reportCliUsage('cli.storage.list-objects', true); + usageSuccess = true; } catch (err) { handleError(err, json); + } finally { + await reportCliUsage('cli.storage.list-objects', usageSuccess); }Also applies to: 79-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/storage/list-objects.ts` around lines 61 - 64, The telemetry emit is skipped when the bucket is empty and never records failures; fix this by moving the telemetry emission out of the early-return and into a finally block so it always runs, introduce a local success flag (e.g., let success = true) and set success = false inside the catch handler (and optionally set a reason or error message variable), and ensure the empty-results branch (where objects.length === 0) does not return before telemetry is sent—use the same telemetry call used around lines 79-82 but invoked from the finally block with { success, reason, bucket, ... } so empty-bucket runs are tracked and exceptions record success=false (reference variables/functions: objects, bucket, the telemetry emit call currently at lines ~79-82).src/commands/functions/list.ts (1)
30-33:⚠️ Potential issue | 🟠 MajorTelemetry is bypassed on empty lists and never marks failures.
Line 32 returns before Line 44, so usage is not recorded when no functions are found. Error path at Line 45 similarly omits
success=false.Also applies to: 44-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/functions/list.ts` around lines 30 - 33, The empty-list early return in the functions listing code (the if (functions.length === 0) { console.log('No functions found.'); return; } block) skips telemetry and the error path also never records success=false; before returning on an empty list, call the telemetry/usage-recording function with success=true (and relevant metadata) so the usage is recorded, and in the error handling path (the catch/failed branch around the code that logs failures) ensure you call the same telemetry/usage-recording function with success=false and the error details; update both the empty-list branch and the failure branch to invoke telemetry (with consistent keys/metadata) rather than returning/logging without recording.src/commands/schedules/list.ts (1)
25-28:⚠️ Potential issue | 🟠 MajorTelemetry is skipped on empty schedules and on failures.
Line 27 returns before Line 42, so empty-list executions are not tracked. Exceptions in Line 43 path also never report
success=false.Also applies to: 42-45
🧹 Nitpick comments (1)
src/commands/schedules/list.ts (1)
20-20: Add response shape guards to prevent silent failures if API response is wrapped.The code performs a blind cast to
ListSchedulesResponsethen immediately assumes array semantics at line 25 (!schedules.length). This is inconsistent with similar list commands—deployments/list.tsandfunctions/list.tsboth guard against different response shapes (array vs. wrapped object), whilesecrets/list.tsandrecords/list.tsaccess nested properties with nullish coalescing. Update line 20-25 to normalize the response shape similar to deployments or functions pattern to prevent runtime errors if/api/schedulesreturns{ schedules: [...] }or other wrapped structures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/schedules/list.ts` at line 20, Replace the blind cast to ListSchedulesResponse and direct array usage with a normalized guard: check if the returned data is already an array (Array.isArray(data)), else if it is an object with a schedules property use data.schedules, else fall back to other possible wrappers like data.items or an empty array; assign that normalized array to a local variable (e.g. schedulesArray) and use that instead of schedules when testing length and iterating. Update the code around the ListSchedulesResponse cast and subsequent uses so all branches cover array vs wrapped-object shapes (referencing the ListSchedulesResponse type and the schedules variable) to prevent runtime errors when /api/schedules returns { schedules: [...] } or other wrappers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/create.ts`:
- Line 161: The call to reportCliUsage('cli.create', true, 12) inside the create
command is awaited and can block the create flow for ~55s on transient failures
and also reports success too early; change it to a non-blocking/fire-and-forget
call (e.g., invoke without await or schedule in background) or move the
reportCliUsage invocation until after the critical create steps complete so the
success metric reflects true completion—update the usage call in the create
function (the reportCliUsage invocation at line with
reportCliUsage('cli.create', true, 12)) accordingly and preserve existing retry
behavior if running in background.
In `@src/commands/db/query.ts`:
- Around line 48-50: The telemetry call currently always reports success=true
and the catch block doesn't emit any usage event; update the try/catch in the db
query command so reportCliUsage('cli.db.query', ...) is called with true on
success and with false on failure (or call it in a finally with the correct
boolean), and ensure the catch block around the async operation calls
reportCliUsage('cli.db.query', false) before invoking handleError(err, json) so
failures are tracked; reference the existing reportCliUsage invocation and the
handleError(err, json) catch branch when making the change.
In `@src/commands/deployments/deploy.ts`:
- Around line 212-215: The telemetry call only runs on the success path; update
the error handling so reportCliUsage('cli.deployments.deploy', false) is invoked
when exceptions occur before the current success call: inside the catch block
that calls handleError(err, json) (or in a surrounding finally), call
reportCliUsage with success=false (and await it or ensure it completes) before
returning/propagating the error so failed executions are recorded; reference the
reportCliUsage and handleError calls in deploy.ts to locate where to add this
failure telemetry.
In `@src/commands/docs.ts`:
- Line 30: The telemetry call reportCliUsage('cli.docs', true) is emitted before
the docs operations run, causing false-success reporting if listDocs or fetchDoc
later fail; move the success telemetry so it is called only after the successful
completion of the operation(s) (e.g., after listDocs resolves or after fetchDoc
completes), and ensure any error paths call reportCliUsage('cli.docs', false) or
omit the success flag so failures are recorded; update the code paths in the
docs command where reportCliUsage is currently invoked (references:
reportCliUsage, listDocs, fetchDoc) so success telemetry is emitted post-success
and failures are reported appropriately.
In `@src/commands/projects/link.ts`:
- Line 105: The telemetry call reportCliUsage('cli.link', true, 12) is awaited
which can block the link command for long backoff delays; change this to
fire-and-forget (remove the await or prefix with void) or reduce the retries
parameter (e.g., use 0) and ensure any returned promise errors are caught (e.g.,
.catch(()=>{}) ) so reportCliUsage no longer stalls the link command execution.
In `@src/lib/skills.ts`:
- Around line 58-86: Wrap the getProjectConfig() call in a try/catch and bail
silently (no throw) if parsing fails so invalid local JSON cannot bubble up; add
an AbortController with a short timeout (e.g., 2–3s) for the fetch call and pass
its signal so fetch can't hang indefinitely, clear the timeout in finally, and
catch all fetch/AbortError exceptions so the telemetry loop (using maxRetries)
never throws; optionally keep retries but ensure every error path returns
without throwing so callers awaiting this telemetry function cannot fail or hang
(references: getProjectConfig, fetch, maxRetries, toolName, success).
---
Outside diff comments:
In `@src/commands/db/functions.ts`:
- Around line 27-30: The early return after logging "No database functions
found." prevents emitting the successful-lookup telemetry; in the function that
lists DB functions (the function containing the console.log 'No database
functions found.' and the subsequent telemetry/reporting call), remove the
premature return and ensure the same success telemetry path is executed for
empty results (or explicitly call the telemetry/reporting method with an
empty-results payload) before returning; update the code paths around the
console.log and the telemetry/reporting function (the telemetry helper used
later in this file) so both empty and non-empty function lists report usage.
In `@src/commands/db/indexes.ts`:
- Around line 27-30: When indexes.length === 0 the function logs "No database
indexes found." and returns, which skips the success telemetry emitted later;
update the early-exit path by invoking the same telemetry/success-reporting call
used at the normal-success path (the telemetry call on line 42) before returning
(or refactor so both paths fall through to the existing success-report routine)
so that zero-index results are also reported; target the block containing
indexes.length === 0, console.log('No database indexes found.'), and the early
return and ensure it calls the same telemetry function used for successful
executions.
In `@src/commands/db/policies.ts`:
- Around line 27-30: The telemetry path is skipped when no policies are found
and on errors; update the handler in src/commands/db/policies.ts so that the
telemetry event for "cli.db.policies" is always emitted: when policies.length
=== 0 call the same telemetry success reporting used for normal runs (e.g., mark
success=true) before returning, and in the catch block emit the telemetry with
success=false and include the error details. Locate the early-return branch
(policies.length === 0) and the catch handler and add calls to the existing
telemetry reporter (the same function used on successful runs) with appropriate
success flags and error info.
In `@src/commands/db/tables.ts`:
- Around line 23-26: The early return inside the tables.length === 0 branch
prevents the telemetry/send (the block with "console.log('No tables found.');
return;") from executing, so move or invoke the telemetry reporting (the
telemetry.trackEvent/sendTelemetry/reportSuccess call currently executed later
around the line with the successful execution telemetry) into that empty-list
branch before returning, or refactor to set a success flag and call the existing
telemetry-reporting code after the if block; specifically modify the
tables.length === 0 handling so it logs "No tables found.", then calls the same
telemetry routine (the existing telemetry tracking function used later) and only
then returns.
In `@src/commands/db/triggers.ts`:
- Around line 27-30: The early return inside the triggers.length === 0 branch
(the console.log('No database triggers found.') followed by return) prevents the
telemetry/metric call executed later (the success telemetry at the end of the
`db triggers` flow) from running; remove the early return (or call the telemetry
success function before returning) so that empty-result runs still emit
telemetry. Concretely, modify the triggers.length check in
src/commands/db/triggers.ts to either (a) call the existing telemetry/success
reporter used later in this file (the same function invoked at the end of the
command) immediately after logging 'No database triggers found.' and then
return, or (b) remove the return and allow execution to continue to the existing
telemetry path — ensure you reference the exact telemetry function used at line
~44 so that empty-result executions are recorded.
In `@src/commands/deployments/list.ts`:
- Around line 34-37: The early return when deployments.length === 0 skips the
success telemetry sent later in this file; before logging "No deployments
found." and returning, invoke the same telemetry success tracking call used on
the normal success path (the success telemetry invocation present later in this
module) and record the empty-result (zero deployments) outcome so empty-list
runs are counted; apply the same change to any other early-return branches that
currently bypass the success telemetry.
In `@src/commands/functions/list.ts`:
- Around line 30-33: The empty-list early return in the functions listing code
(the if (functions.length === 0) { console.log('No functions found.'); return; }
block) skips telemetry and the error path also never records success=false;
before returning on an empty list, call the telemetry/usage-recording function
with success=true (and relevant metadata) so the usage is recorded, and in the
error handling path (the catch/failed branch around the code that logs failures)
ensure you call the same telemetry/usage-recording function with success=false
and the error details; update both the empty-list branch and the failure branch
to invoke telemetry (with consistent keys/metadata) rather than
returning/logging without recording.
In `@src/commands/metadata.ts`:
- Around line 21-24: The JSON branch in metadata command returns immediately
after calling outputJson, which skips the telemetry success reporting executed
later; before returning from the json path in the function containing
outputJson, invoke the same telemetry reporting logic used at the successful end
of the handler (i.e., call the existing telemetry reporting function or share
the success-reporting code path used around line 98), or refactor so both JSON
and non-JSON flows run the common success telemetry/reporting code (move it into
a shared helper or finally/after-success block) to ensure --json runs are
tracked.
In `@src/commands/secrets/list.ts`:
- Around line 30-33: The early return in the empty-list branch (the if that
checks secrets.length and calls console.log('No secrets found.')) exits before
the command's telemetry/usage reporting runs, so move or add the invocation of
the telemetry/usage-reporting function (the function used elsewhere to record
successful command usage) to execute immediately before returning from that
branch; similarly, apply the same change to the other early-return location so
the usage-reporting call always runs on successful empty-list paths.
In `@src/commands/storage/buckets.ts`:
- Around line 30-33: The early return when buckets.length === 0 in the buckets
command skips emitting the success telemetry; update the success path in the
function handling the buckets command so that telemetry (e.g., the existing
telemetry/usage event emitter) is invoked before any return in the "no buckets"
branch and any other early-success return (the return around line 39).
Concretely, call the same telemetry success/usage function used for the
normal-result path immediately before console.log('No buckets found.') return,
or refactor to emit success telemetry in a single post-success step (or finally
block) that runs for all successful exits of the buckets command (referencing
the buckets variable and the early return locations).
In `@src/commands/storage/list-objects.ts`:
- Around line 61-64: The telemetry emit is skipped when the bucket is empty and
never records failures; fix this by moving the telemetry emission out of the
early-return and into a finally block so it always runs, introduce a local
success flag (e.g., let success = true) and set success = false inside the catch
handler (and optionally set a reason or error message variable), and ensure the
empty-results branch (where objects.length === 0) does not return before
telemetry is sent—use the same telemetry call used around lines 79-82 but
invoked from the finally block with { success, reason, bucket, ... } so
empty-bucket runs are tracked and exceptions record success=false (reference
variables/functions: objects, bucket, the telemetry emit call currently at lines
~79-82).
---
Nitpick comments:
In `@src/commands/schedules/list.ts`:
- Line 20: Replace the blind cast to ListSchedulesResponse and direct array
usage with a normalized guard: check if the returned data is already an array
(Array.isArray(data)), else if it is an object with a schedules property use
data.schedules, else fall back to other possible wrappers like data.items or an
empty array; assign that normalized array to a local variable (e.g.
schedulesArray) and use that instead of schedules when testing length and
iterating. Update the code around the ListSchedulesResponse cast and subsequent
uses so all branches cover array vs wrapped-object shapes (referencing the
ListSchedulesResponse type and the schedules variable) to prevent runtime errors
when /api/schedules returns { schedules: [...] } or other wrappers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c38e6623-bcc2-4ae7-89fb-267a687beae2
📒 Files selected for processing (23)
src/commands/create.tssrc/commands/db/functions.tssrc/commands/db/indexes.tssrc/commands/db/policies.tssrc/commands/db/query.tssrc/commands/db/rpc.tssrc/commands/db/tables.tssrc/commands/db/triggers.tssrc/commands/deployments/deploy.tssrc/commands/deployments/list.tssrc/commands/docs.tssrc/commands/functions/deploy.tssrc/commands/functions/list.tssrc/commands/metadata.tssrc/commands/projects/link.tssrc/commands/schedules/create.tssrc/commands/schedules/list.tssrc/commands/secrets/add.tssrc/commands/secrets/list.tssrc/commands/storage/buckets.tssrc/commands/storage/create-bucket.tssrc/commands/storage/list-objects.tssrc/lib/skills.ts
src/commands/create.ts
Outdated
|
|
||
| // Install InsForge agent skills | ||
| await installSkills(json); | ||
| await reportCliUsage('cli.create', true, 12); |
There was a problem hiding this comment.
Line 161 can introduce long user-visible delay and premature success reporting.
With 12 retries and 5s retry delay, this awaited call can block create for up to ~55s on transient failures. It also marks success before later steps complete.
🔧 Suggested fix
- await reportCliUsage('cli.create', true, 12);
+ void reportCliUsage('cli.create', true, 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/create.ts` at line 161, The call to reportCliUsage('cli.create',
true, 12) inside the create command is awaited and can block the create flow for
~55s on transient failures and also reports success too early; change it to a
non-blocking/fire-and-forget call (e.g., invoke without await or schedule in
background) or move the reportCliUsage invocation until after the critical
create steps complete so the success metric reflects true completion—update the
usage call in the create function (the reportCliUsage invocation at line with
reportCliUsage('cli.create', true, 12)) accordingly and preserve existing retry
behavior if running in background.
src/commands/projects/link.ts
Outdated
|
|
||
| // Install InsForge agent skills | ||
| await installSkills(json); | ||
| await reportCliUsage('cli.link', true, 12); |
There was a problem hiding this comment.
Link command may pause for a long time after success.
Awaiting telemetry with maxRetries=12 can add ~55s backoff delay when endpoint is unavailable.
🔧 Proposed fix
- await reportCliUsage('cli.link', true, 12);
+ void reportCliUsage('cli.link', true, 12);📝 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.
Suggested change
| await reportCliUsage('cli.link', true, 12); | |
| void reportCliUsage('cli.link', true, 12); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/projects/link.ts` at line 105, The telemetry call
reportCliUsage('cli.link', true, 12) is awaited which can block the link command
for long backoff delays; change this to fire-and-forget (remove the await or
prefix with void) or reduce the retries parameter (e.g., use 0) and ensure any
returned promise errors are caught (e.g., .catch(()=>{}) ) so reportCliUsage no
longer stalls the link command execution.
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 (6)
src/commands/metadata.ts (1)
21-24:⚠️ Potential issue | 🟠 MajorSuccess telemetry is skipped in JSON mode.
Line 23 returns before Line 98, so
cli.metadatasuccess is never reported for--json.Suggested fix
if (json) { outputJson(data); + await reportCliUsage('cli.metadata', true); return; }Also applies to: 98-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/metadata.ts` around lines 21 - 24, The JSON output branch returns before emitting the success telemetry, so when --json is used the cli.metadata success event is never reported; modify the handler so that after calling outputJson(data) you still call the same telemetry/success reporting used at the end of the function (the cli.metadata success emission) before returning (or factor the telemetry call into a helper and invoke it from both the JSON branch and the normal flow) so that outputJson(...) does not short-circuit the success telemetry.src/commands/secrets/list.ts (1)
30-33:⚠️ Potential issue | 🟠 MajorSuccessful empty-result runs are not reported.
Line 32 returns before Line 51, so
cli.secrets.listusage is missed when no secrets are found.Suggested fix
if (!secrets.length) { console.log('No secrets found.'); + await reportCliUsage('cli.secrets.list', true); return; }Also applies to: 51-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/secrets/list.ts` around lines 30 - 33, The empty-results branch in the if (!secrets.length) block returns early and skips the telemetry/usage call for cli.secrets.list; modify the block so it still reports usage before exiting (e.g., call the same usage/telemetry function that normally runs at the end, or remove the early return and let execution reach the existing cli.secrets.list invocation). Ensure the change is applied to the if (!secrets.length) branch and any other similar early-return at the end of the file so the cli.secrets.list usage is always reported.src/commands/db/triggers.ts (1)
27-30:⚠️ Potential issue | 🟠 MajorSuccess event is skipped when trigger list is empty.
Line 29 returns before Line 44, so successful
cli.db.triggerscalls are not tracked in this path.Suggested fix
if (triggers.length === 0) { console.log('No database triggers found.'); + await reportCliUsage('cli.db.triggers', true); return; }Also applies to: 44-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/db/triggers.ts` around lines 27 - 30, The early return inside the cli.db.triggers handler when triggers.length === 0 skips the success-tracking path; adjust the branch so the success event is still emitted. Specifically, in the block that logs "No database triggers found." (the check of triggers variable in the cli.db.triggers handler), call the same success-tracking/analytics function used at the normal success path (the event emitted around the current line 44) before returning, or restructure to avoid returning early (e.g., set an empty result and fall through) so the existing success-tracking code always runs.src/commands/storage/buckets.ts (1)
30-33:⚠️ Potential issue | 🟠 MajorSuccess telemetry is skipped when no buckets exist.
Line 32 returns before Line 39, so successful
cli.storage.bucketsexecutions with empty results are not reported.Suggested fix
if (buckets.length === 0) { console.log('No buckets found.'); + await reportCliUsage('cli.storage.buckets', true); return; }Also applies to: 39-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/storage/buckets.ts` around lines 30 - 33, The success telemetry for cli.storage.buckets is never sent when buckets.length === 0 because the function returns early; modify the empty-result branch to either call the same success-telemetry/reporting helper used for non-empty results before returning, or remove the early return so the normal success telemetry path runs (reference the buckets variable and the cli.storage.buckets execution flow to locate the branch and telemetry call to reuse).src/commands/db/indexes.ts (1)
27-30:⚠️ Potential issue | 🟠 Major
cli.db.indexessuccess telemetry is missed on empty results.Line 29 returns before Line 42, so successful calls with no indexes are not reported.
Suggested fix
if (indexes.length === 0) { console.log('No database indexes found.'); + await reportCliUsage('cli.db.indexes', true); return; }Also applies to: 42-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/db/indexes.ts` around lines 27 - 30, The command handler cli.db.indexes currently returns early when indexes.length === 0 (and at another early-return path), which skips emitting the "cli.db.indexes" success telemetry; modify the handler so that it records/sends the success telemetry before any early return (e.g., invoke the telemetry function with the success event right after detecting empty results and also at the other early-return path), or refactor to ensure the telemetry call is executed in a single exit/finally path so all successful outcomes (empty or populated) report "cli.db.indexes" success.src/commands/functions/list.ts (1)
30-33:⚠️ Potential issue | 🟠 MajorEmpty-list success path is not tracked.
Line 32 returns before Line 44, so successful
cli.functions.listcalls with no functions are not reported.Suggested fix
if (functions.length === 0) { console.log('No functions found.'); + await reportCliUsage('cli.functions.list', true); return; }Also applies to: 44-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/functions/list.ts` around lines 30 - 33, The empty-list branch in cli.functions.list returns before emitting the success tracking that the non-empty path triggers (the tracking call present after the normal listing around the code after Line 44). Add the same tracking/reporting call used for the successful list path into the if (functions.length === 0) block — invoke the identical tracking function with the same parameters (status/success event and any metadata) before the console.log('No functions found.') return, and apply the same change to the other empty-list branch noted so both empty-list success paths are reported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/commands/db/indexes.ts`:
- Around line 27-30: The command handler cli.db.indexes currently returns early
when indexes.length === 0 (and at another early-return path), which skips
emitting the "cli.db.indexes" success telemetry; modify the handler so that it
records/sends the success telemetry before any early return (e.g., invoke the
telemetry function with the success event right after detecting empty results
and also at the other early-return path), or refactor to ensure the telemetry
call is executed in a single exit/finally path so all successful outcomes (empty
or populated) report "cli.db.indexes" success.
In `@src/commands/db/triggers.ts`:
- Around line 27-30: The early return inside the cli.db.triggers handler when
triggers.length === 0 skips the success-tracking path; adjust the branch so the
success event is still emitted. Specifically, in the block that logs "No
database triggers found." (the check of triggers variable in the cli.db.triggers
handler), call the same success-tracking/analytics function used at the normal
success path (the event emitted around the current line 44) before returning, or
restructure to avoid returning early (e.g., set an empty result and fall
through) so the existing success-tracking code always runs.
In `@src/commands/functions/list.ts`:
- Around line 30-33: The empty-list branch in cli.functions.list returns before
emitting the success tracking that the non-empty path triggers (the tracking
call present after the normal listing around the code after Line 44). Add the
same tracking/reporting call used for the successful list path into the if
(functions.length === 0) block — invoke the identical tracking function with the
same parameters (status/success event and any metadata) before the
console.log('No functions found.') return, and apply the same change to the
other empty-list branch noted so both empty-list success paths are reported.
In `@src/commands/metadata.ts`:
- Around line 21-24: The JSON output branch returns before emitting the success
telemetry, so when --json is used the cli.metadata success event is never
reported; modify the handler so that after calling outputJson(data) you still
call the same telemetry/success reporting used at the end of the function (the
cli.metadata success emission) before returning (or factor the telemetry call
into a helper and invoke it from both the JSON branch and the normal flow) so
that outputJson(...) does not short-circuit the success telemetry.
In `@src/commands/secrets/list.ts`:
- Around line 30-33: The empty-results branch in the if (!secrets.length) block
returns early and skips the telemetry/usage call for cli.secrets.list; modify
the block so it still reports usage before exiting (e.g., call the same
usage/telemetry function that normally runs at the end, or remove the early
return and let execution reach the existing cli.secrets.list invocation). Ensure
the change is applied to the if (!secrets.length) branch and any other similar
early-return at the end of the file so the cli.secrets.list usage is always
reported.
In `@src/commands/storage/buckets.ts`:
- Around line 30-33: The success telemetry for cli.storage.buckets is never sent
when buckets.length === 0 because the function returns early; modify the
empty-result branch to either call the same success-telemetry/reporting helper
used for non-empty results before returning, or remove the early return so the
normal success telemetry path runs (reference the buckets variable and the
cli.storage.buckets execution flow to locate the branch and telemetry call to
reuse).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: daa7fdd5-7920-49e6-a986-c2710bed03ee
📒 Files selected for processing (21)
src/commands/create.tssrc/commands/db/functions.tssrc/commands/db/indexes.tssrc/commands/db/policies.tssrc/commands/db/query.tssrc/commands/db/tables.tssrc/commands/db/triggers.tssrc/commands/deployments/deploy.tssrc/commands/deployments/list.tssrc/commands/functions/deploy.tssrc/commands/functions/list.tssrc/commands/metadata.tssrc/commands/projects/link.tssrc/commands/schedules/create.tssrc/commands/schedules/list.tssrc/commands/secrets/add.tssrc/commands/secrets/list.tssrc/commands/storage/buckets.tssrc/commands/storage/create-bucket.tssrc/commands/storage/list-objects.tssrc/lib/skills.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- src/commands/functions/deploy.ts
- src/commands/deployments/list.ts
- src/commands/schedules/create.ts
- src/commands/projects/link.ts
- src/commands/schedules/list.ts
- src/commands/db/tables.ts
- src/commands/storage/list-objects.ts
- src/commands/db/query.ts
- src/commands/secrets/add.ts
- src/commands/deployments/deploy.ts
- src/commands/db/policies.ts
- src/commands/db/functions.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit