Track deployment commands in posthog#145
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughDeployment commands now emit structured usage analytics through a new ChangesDeployment Usage Analytics
Payments formatting (minor)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Code Review — Track deployment commands in PostHog
Summary: Clean, focused instrumentation PR that migrates deployment commands from reportCliUsage to a richer PostHog tracker. No blocking issues found.
Requirements context: No /docs/superpowers/ directory exists in this repo. Assessment is based on the PR description and the surrounding code patterns established by existing analytics helpers (trackDiagnose, trackPayments, trackConfig).
Findings
Critical
(none)
Suggestion
Software Engineering — Missing tests for new utils.ts helper
src/commands/deployments/utils.ts introduces trackDeploymentUsage, but no test file is present — the project otherwise has full test coverage of analytics-adjacent helpers (see src/lib/skills.test.ts testing reportCliUsage). Per TDD convention, the following cases should be covered:
- Happy path:
getProjectConfig()returns a config →trackDeployments+shutdownAnalyticsare called. - No-config path:
getProjectConfig()returns null → neithertrackDeploymentsis called,shutdownAnalyticsstill runs. - Error inside inner try: swallowed,
shutdownAnalyticsstill runs.
A test for utils.ts would mirror the existing pattern in src/lib/skills.test.ts and give confidence that future refactors don't accidentally propagate analytics errors into command failures.
Information
Software Engineering — Redundant outer try wrapper in utils.ts:14-28
// utils.ts (current)
try {
try {
const config = getProjectConfig();
if (config) {
trackDeployments(subcommand, config, { success, ...properties });
}
} catch {
// Telemetry should never affect command behavior.
}
} finally {
await shutdownAnalytics();
}The outer try has no matching catch, so it adds no error-isolation — exceptions from shutdownAnalytics in the finally still propagate (although shutdownAnalytics internally swallows its own errors). Simplify to:
try {
const config = getProjectConfig();
if (config) {
trackDeployments(subcommand, config, { success, ...properties });
}
} catch {
// Telemetry should never affect command behavior.
} finally {
await shutdownAnalytics();
}Functionality — Duplicate trackDeploymentUsage call in early-return branches
In src/commands/deployments/list.ts:36,50 and src/commands/deployments/env-vars.ts:38,52, trackDeploymentUsage(..., true) appears both in the early-return "empty results" branch and at the end of the normal path. These branches are mutually exclusive so there is no double-fire, but the duplication is easy to trip over in future edits. Consider restructuring so that tracking always happens in one place after the branching logic completes.
Functionality — Failure events silently dropped when project is not linked
trackDeploymentUsage in utils.ts:16-20 sends the event only when config is truthy. For commands that call requireAuth() before reading the config (e.g., cancel.ts), if auth throws before the config is loaded, the failure path calls trackDeploymentUsage('cancel', false) with getProjectConfig() returning null — so the error event is silently dropped. This is probably intentional (no project ID = no way to attribute the event), but it means auth-failure rates for deployment commands will not appear in the analytics. If that data is wanted, consider falling back to FAKE_PROJECT_ID (the same pattern trackConfig uses in src/lib/analytics.ts:107).
Verdict
approved (informational — human approval still required via the GitHub approve flow)
No correctness, security, or data-loss issues found. The implementation follows the existing pattern established by trackDiagnose and trackPayments. Adding tests for the new utils.ts helper would close the only meaningful gap.
Greptile SummaryThis PR adds PostHog analytics tracking to all five deployment CLI subcommands (
Confidence Score: 5/5Safe to merge — changes are additive telemetry instrumentation that cannot throw or alter command output. The implementation is structurally identical to the already-shipped payments analytics, No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Command as CLI Command<br/>(cancel/deploy/status/list/env-vars)
participant Util as trackDeploymentUsage<br/>(deployments/utils.ts)
participant Analytics as trackDeployments<br/>(lib/analytics.ts)
participant PostHog
User->>Command: run subcommand
Command->>Command: execute business logic
alt success
Command->>Util: trackDeploymentUsage(subcmd, true, props)
else error
Command->>Util: trackDeploymentUsage(subcmd, false, props)
end
Util->>Util: getProjectConfig()
alt project linked
Util->>Analytics: "trackDeployments(subcmd, config, {success, ...props})"
Analytics->>PostHog: captureEvent(project_id, 'cli_deployments_invoked', ...)
else project not linked
Util-->>Util: skip (no event emitted)
end
Util->>Analytics: shutdownAnalytics()
Analytics->>PostHog: client.shutdown()
Command->>Command: handleError(err) [if error path]
Reviews (3): Last reviewed commit: "bump version" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
2 participants
Summary by cubic
Add PostHog tracking for all deployment CLI commands to measure usage and outcomes. Each command emits a
cli_deployments_invokedevent with success status and key props, and analytics are flushed safely.New Features
trackDeploymentsinsrc/lib/analytics.tsto sendcli_deployments_invokedwithsubcommand, project/org/region info, andoss_mode.trackDeploymentUsageinsrc/commands/deployments/utils.tsto read project config, merge custom props, and callshutdownAnalyticsin a finally block.deploy,status,list,cancel, andenv [list|set|delete]with success/failure and props (e.g.,has_env,has_meta,ready,sync).reportCliUsagewith the new tracker for deployment commands.Refactors
shutdownAnalytics; aligned approach insrc/commands/payments/utils.ts.@insforge/cliversion to 0.1.85.Written for commit 17c3bbd. Summary will update on new commits.
Review in cubic
Summary by CodeRabbit
Note
Track deployment command invocations as PostHog analytics events
trackDeploymentsfunction insrc/lib/analytics.tsthat emits acli_deployments_invokedevent with project metadata (org, region, OSS mode).trackDeploymentUsageutility insrc/commands/deployments/utils.tsthat wrapstrackDeployments, records success/failure, and always flushes analytics viashutdownAnalytics.reportCliUsagecalls indeploy,cancel,list,status, andenv-varssubcommands withtrackDeploymentUsage, passing subcommand-specific properties (e.g.has_env,has_meta,ready,sync).Changes since #145 opened
env listanddeployments listsubcommands [2939963]trackDeploymentUsageandtrackPaymentUsageutility functions [2939963]Macroscope summarized 47ce5c0.