feat: report each command to PostHog, redacted; opt out with INSTA_NO_TELEMETRY - #165
Conversation
There was a problem hiding this comment.
Reviewed by Yang Dong
This adds best-effort PostHog command telemetry, local redaction and opt-outs, while preserving subprocess exit codes. I would not merge it yet because storage-list prefixes bypass the payload redaction and expose object-key metadata.
storage list sends object-key prefixes to PostHog unredacted
important · defect · correctness · src/telemetry.ts:26
Object keys are treated as opaque for storage get and storage delete, but storage list --prefix reaches cmd.opts() and only receives pattern-based scrubbing. A normal call such as insta storage list --prefix customers/acme/contracts/ therefore sends that user-controlled portion of the object-key namespace verbatim; redact prefix wholesale, preferably through command-specific option redaction.
Evidence
read-the-code — src/telemetry.ts:25-57, src/index.ts:287-305, src/commands/storage.ts:53-71, test/telemetry.test.ts:69-97
|
Round 1 (fdccc48): |
There was a problem hiding this comment.
Reviewed by Wang Miao
This adds a per-command PostHog event (command, redacted flags/args, outcome, environment) behind INSTA_NO_TELEMETRY/DO_NOT_TRACK, and along the way converts the last two process.exit() call sites into process.exitCode/CliExit so the wrapper can report before the process ends. The redaction is the load-bearing part and it holds up — I traced every command's positionals and options against REDACTED_OPTIONS/PAYLOAD_ARGS_FROM, and the payload-carrying surfaces (secrets set, run, db query, storage get|delete, --set, --password/--api-key/--email, feedback free text) are all covered, with compute exec's remote argv never reaching commander in the first place. I'd merge it; two minor notes below.
Anonymous CLI events are sent as identified events, so every machine mints a permanent person profile
minor · judgement · conventions · src/telemetry.ts:118
When there's no session, distinct_id falls back to a random UUID from ~/.insta/telemetry.json, and the event carries no $process_person_profile. PostHog's server-side ingest treats that as an identified event: it creates a person profile per machine, in the same project the console deliberately runs with person_profiles: 'identified_only' (insta-frontend src/lib/api/hooks.ts:27-35 explains why), and identified events are billed up to 4× anonymous ones. Those persons never merge with the real user either — nothing aliases the UUID to user.id at login, so a machine's pre-login setup agent / build / status events stay orphaned. Named fix: put $process_person_profile: false in properties when the id came from the anonymous fallback.
Evidence
read-the-code — src/telemetry.ts:113-153, src/telemetry.ts:167-179, insta-frontend src/lib/analytics.ts (same two project keys), insta-frontend src/lib/api/hooks.ts:26-37; PostHog docs on anonymous vs identified events (server-side default, $process_person_profile).
success reports the child process's exit status for the passthrough commands
minor · judgement · correctness · src/telemetry.ts:124
success/exit_code are read from process.exitCode after the action returns (src/index.ts:48), and for run, db connect and compute exec that value is deliberately the child's status — src/commands/run.ts:45, src/commands/db.ts:331, src/commands/compute.ts:552-554. So insta run npm test with a failing test, or a psql session ended with a non-zero status, lands as a failed insta run with no error_type, and the failure rate for exactly the commands agents use most reads high for reasons that have nothing to do with the CLI. Named alternative: for those three, carry the passthrough status as its own property and leave success for the CLI's own outcome.
Evidence
read-the-code — src/index.ts:41-49, src/telemetry.ts:113-153, src/commands/run.ts:35-45, src/commands/db.ts:298-331, src/commands/compute.ts:528-555.
There was a problem hiding this comment.
Reviewed by Yang Dong
This adds command-level PostHog telemetry with local redaction and routes several long-running commands back through the reporting wrapper. I would not merge it yet: sensitive payloads can escape through captured error messages, and common interactive cancellations bypass telemetry entirely.
Captured error messages can reintroduce values that the argument redaction removed
critical · defect · correctness · src/telemetry.ts:99
Arguments and options receive command-aware redaction, but error_message only gets generic pattern scrubbing. Consequently, storage get customer/acme/ sends the full object key when filename inference fails, while an invalid template assignment such as --set lower=<sensitive-value> sends the entire assignment from the validation error. These values cross the local redaction boundary into PostHog; omit error text or apply the same command-aware payload redaction before capturing it.
Evidence
read-the-code — src/telemetry.ts:25-57, src/telemetry.ts:92-127, src/redact.ts:8-69, src/commands/storage.ts:97-103, src/commands/storage.ts:149-160, src/commands/template.ts:107-115, src/commands/template.ts:332-333, src/index.ts:43-49, test/storage.test.ts:84-86, test/template.test.ts:260-267
Interactive cancellation exits before the reporting wrapper can send an event
important · defect · correctness · src/index.ts:47
The new mechanism depends on every action unwinding back into guard, but existing prompt helpers call process.exit(0) on cancellation. Cancelling interactive services add, feedback, or a prompted template variable therefore terminates immediately and never reports the command, systematically hiding abandoned flows. Return or throw a cancellation sentinel so guard can record the outcome before exit.
Evidence
read-the-code — src/index.ts:41-49, src/resolve-service.ts:120-180, src/commands/feedback.ts:91-130, src/commands/template.ts:304-312
|
Round 2 (aacd4c3), Wang Miao's two notes: anonymous events now carry |
There was a problem hiding this comment.
1 issue found and verified against the latest diff
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="src/telemetry.ts">
<violation number="1" location="src/telemetry.ts:33">
P2: `DO_NOT_TRACK=0` or `INSTA_NO_TELEMETRY=0` currently disables telemetry because the guard tests truthiness instead of the documented `=1` value. Check each variable for equality with `'1'`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const PAYLOAD_ARGS_FROM: Record<string, number> = { 'secrets set': 1, run: 1, 'db query': 1, 'storage get': 0, 'storage delete': 0 } | ||
|
|
||
| export function telemetryDisabled(env: NodeJS.ProcessEnv = process.env): boolean { | ||
| return !!(env.DO_NOT_TRACK || env.INSTA_NO_TELEMETRY) |
There was a problem hiding this comment.
P2: DO_NOT_TRACK=0 or INSTA_NO_TELEMETRY=0 currently disables telemetry because the guard tests truthiness instead of the documented =1 value. Check each variable for equality with '1'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/telemetry.ts, line 33:
<comment>`DO_NOT_TRACK=0` or `INSTA_NO_TELEMETRY=0` currently disables telemetry because the guard tests truthiness instead of the documented `=1` value. Check each variable for equality with `'1'`.</comment>
<file context>
@@ -0,0 +1,211 @@
+const PAYLOAD_ARGS_FROM: Record<string, number> = { 'secrets set': 1, run: 1, 'db query': 1, 'storage get': 0, 'storage delete': 0 }
+
+export function telemetryDisabled(env: NodeJS.ProcessEnv = process.env): boolean {
+ return !!(env.DO_NOT_TRACK || env.INSTA_NO_TELEMETRY)
+}
+
</file context>
| return !!(env.DO_NOT_TRACK || env.INSTA_NO_TELEMETRY) | |
| return env.DO_NOT_TRACK === '1' || env.INSTA_NO_TELEMETRY === '1' |
|
Round 3 (29de003), Yang Dong's two findings:
|
There was a problem hiding this comment.
Reviewed by Yang Dong
This adds best-effort PostHog reporting around each Commander action while preserving cancellation, approval, and child-process exit behavior. The execution changes are sound, but the redaction boundary still transmits arbitrary user-chosen names and paths, so I would request changes before merging.
The telemetry allowlist exposes branch names and executable paths
important · defect · correctness · src/telemetry.ts:25
Branch names are unrestricted user strings, yet they are retained as branch-command arguments, --branch/--into values, and ambient context on every linked command. The first insta run argument is likewise an arbitrary executable and may be an absolute or project-specific path. These values routinely encode feature, customer, or filesystem information and are sent to PostHog by default; redact them, or locally pseudonymize branch values if cross-event correlation is required.
Evidence
read-the-code — src/telemetry.ts:23-37, src/telemetry.ts:127-152, src/index.ts:93-97, src/index.ts:128-170, src/config.ts:122-135, test/telemetry.test.ts:70-98, test/telemetry.test.ts:113-121
There was a problem hiding this comment.
4 issues found across 10 files (changes from recent commits).
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="src/commands/compute.ts">
<violation number="1" location="src/commands/compute.ts:554">
P3: In applyExecResult the in-range branch now calls relayExitCode(exitCode), but the out-of-range branch (exitCode < 0 || > 255, which includes the platform's -1 "unknown exit" sentinel) still sets process.exitCode = 1 directly without relaying. Because relayedCode is only populated by relayExitCode, an out-of-range remote code reports child_exit_code: null and success: false — even though the CLI itself ran the command. That contradicts the change's intent that "success means the CLI itself ran the command". Consider relaying the clamped code (e.g. relayExitCode(1)) in the out-of-range branch too, so the child's abnormal exit is reported distinctly from a CLI failure.</violation>
</file>
<file name="src/telemetry.ts">
<violation number="1" location="src/telemetry.ts:26">
P1: When `insta run` receives a path-valued command, telemetry sends the first positional verbatim, including the local username/path. Do not allowlist `run`'s arbitrary executable, or scrub and cap it before emitting the event.</violation>
<violation number="2" location="src/telemetry.ts:27">
P2: Branch names are user-controlled strings, but this allowlist sends them in telemetry without redaction. Remove branch-name positionals and the `branch`/`into` option values from the safe allowlists, or validate and encode them as an actual enum/ID before retaining them.</violation>
</file>
<file name="src/index.ts">
<violation number="1" location="src/index.ts:51">
P2: When a prompt is cancelled on Windows while `maybeUpdate()` has spawned its detached child, this direct `process.exit(0)` can race child-handle teardown and abort the CLI with `UV_HANDLE_CLOSING`. Preserve the zero exit while using a cleanup path that does not force process teardown.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Round 4 (6ec1956). |
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Reviewed by Yang Dong
This adds best-effort PostHog reporting around CLI command execution with opt-out support. The lifecycle integration is sound, but the redactor still transmits user-controlled names, so I would not merge it as it stands.
The redactor sends template variable names to PostHog
important · defect · correctness · src/telemetry.ts:55
Every template deploy --set NAME=value event preserves NAME, even though variable names are application-specific and can contain customer or integration identifiers such as CUSTOMER_ACME_TOKEN. This occurs on every use of --set; redact the entire assignment, or retain only a count, and validate any other supposedly safe strings against explicit ID/enum/number formats before sending them.
Evidence
read-the-code — src/telemetry.ts:23-64, src/index.ts:307-317, src/commands/template.ts:107-117, test/telemetry.test.ts:84-85
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…_TELEMETRY Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…/delete keys Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ed apart from the CLI outcome Co-Authored-By: Claude Fable 5.1 <noreply@antml:parameter>anthropic.com>
…instead of exiting past the wrapper Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t exits through the wrapper Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
6ec1956 to
fa1fd5c
Compare
|
Round 5 (6079a28): |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed by Yang Dong
This adds a redacted PostHog event after each guarded CLI action while preserving child and cancellation exit semantics. The redaction is careful, but failed logins can report to the wrong environment—including sending a self-hosted attempt to production—so I would not merge it as written.
Failed login attempts are reported to the previously configured environment
important · defect · correctness · src/telemetry.ts:203
login --api-url <custom-host> and login --env staging change only the action’s in-memory ApiClient, persisting that target only after authentication succeeds. When authentication fails, the guard invokes trackCommand, which rereads the old configuration—or defaults to production on a fresh install—so a custom-host attempt emits telemetry and a staging attempt can be attributed to the production PostHog project, including the prior production user and project IDs. Route from the target actually used by the command rather than rereading persisted state after completion.
Evidence
read-the-code — src/index.ts:43-49, src/commands/auth.ts:37-44, src/commands/auth.ts:80-86, src/api.ts:22-32, src/config.ts:27-60, src/telemetry.ts:197-216
…s configuration Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Round 6 (d1df986): |
There was a problem hiding this comment.
Reviewed by Yang Dong
This adds command-level PostHog telemetry and restructures exit handling so events can flush without changing command outcomes. The redaction boundary does not hold for rejected inputs, so I would request changes before merging.
The allowlists send rejected free-form values to PostHog without redaction
important · defect · correctness · src/telemetry.ts:49
Allowlisting is based only on an option name or positional index, without checking that the value is actually an ID, enum, or number. For example, insta services add compute app --port yesterday is rejected by parsePort, but the guard still tracks the command and redactOptions sends "yesterday" verbatim; the same issue applies to every allowlisted field receiving malformed or mistakenly pasted content. Validate/normalize each allowlisted value before retaining it, and redact values that fail that check.
Evidence
read-the-code — src/telemetry.ts:23-61, src/telemetry.ts:202-224, src/index.ts:41-49, src/commands/services.ts:31-38, src/commands/services.ts:99-107, src/commands/metrics.ts:145-160
…ember, number, id, region) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Round 7 (21f5919): every allowlisted value is now shape-checked before it is kept, and dropped otherwise — enums by membership ( |
There was a problem hiding this comment.
Reviewed by Wang Miao
The redaction has been rebuilt since round 1 as an allowlist — only positionals a command declares as ids or enums, plus a fixed set of option names, survive; everything else, including every error message, is dropped before the event is built. I walked every command's positionals and options against SAFE_ARGS/SAFE_OPTIONS and found nothing user-owned that still escapes, and both findings I raised last round are genuinely fixed (anonymous events now carry $process_person_profile: false; a relayed child status is reported as child_exit_code and no longer read as a CLI failure). One narrow routing gap in the new login handling is the only thing left, and it does not block.
login --env STAGING routes its failure event to the wrong PostHog project
minor · defect · correctness · src/telemetry.ts:69
loginTarget tests isEnvName(opts.env) against the raw flag, but the command itself accepts the flag case-insensitively — src/commands/auth.ts:13 does opts.env.trim().toLowerCase() before validating. So insta login --env STAGING is a valid staging login, yet loginTarget returns undefined for it and the event falls back to the previously persisted apiUrl — a failed attempt from a prod-configured machine lands in the prod project, which is the exact case commit d1df986 exists to prevent. Same normalization at the call site closes it.
Evidence
read-the-code — src/telemetry.ts:63-70, src/telemetry.ts:206-211, src/commands/auth.ts:11-15, src/env.ts:isEnvName/envFromEnvVar (which does normalize), test/telemetry.test.ts:229-244 (covers 'staging', not the cased form).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Wang Miao's note (1ecafef): |
There was a problem hiding this comment.
Reviewed by Yang Dong
This adds best-effort, redacted PostHog reporting around CLI actions while preserving command and child-process exit behavior. I would request changes because project attribution is not scoped to the selected deployment, causing cross-environment identifier leakage and corrupted analytics.
Ambient project IDs are sent to a different deployment after an environment switch
important · defect · correctness · src/telemetry.ts:231
From a directory linked to a production project, insta env use staging, INSTA_ENV=staging insta status, or a successful insta login --env staging selects the staging PostHog key but leaves .insta/project.json unchanged. trackCommand then unconditionally sends the production project and organization IDs to staging, despite the repository explicitly treating these deployments and databases as separate; subsequent events are both misattributed and cross that deployment boundary. Store the originating environment with the project link and verify it before attribution, or omit project metadata whenever the association cannot be established.
Evidence
read-the-code — src/telemetry.ts:226-241, src/config.ts:27-59, src/config.ts:122-136, src/commands/env.ts:37-79, src/commands/auth.ts:37-44, src/commands/auth.ts:67-86, test/telemetry.test.ts:238-255
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…commands do Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Round 8 (c5c4a8a). Declining the ambient project-id finding, by owner decision: cubic's note is taken: |
What
Every finished
instacommand now sends onecli_commandevent to PostHog — the same two projects the console reports to (src/telemetry.tsPROJECT_KEYS, chosen byenvForApiUrl; a customINSTA_API_URLsuch as insta-oss reports nothing).Event properties: command path, positional args, options,
success/exit_code/duration_ms,error_type(plushttp_statusfor API errors,error_codefor network errors, and thedie()text for CLI errors),cli_version,channel(binary/npm/source),node_version,os/os_release/arch,env(prod/staging/custom),api_host,logged_in,auth_kind(api_key/session),project_id/org_id/branchfrom.insta/project.json,tty,ci,agent(claude-code/cursor, from env),term_program.distinct_idis the signed-in user id (what the console identifies with), else an anonymous id kept in~/.insta/telemetry.json. No email is sent.Redaction (
src/telemetry.ts): options that are credentials, identity or free text (--password,--api-key,--email, feedback's--title/--detail/--error/--expected/--workaround/--doc/--command/--area) are dropped wholesale;--set NAME=valuekeeps the name only; payload positionals are dropped by index (secrets setvalue,runanddb queryargv,storage get|deletekeys); everything else goes throughredact.tsclean()(tokens, JWTs, URL credentials, emails, home dirs, public IPs; 200-char cap).Opt out with
DO_NOT_TRACK=1orINSTA_NO_TELEMETRY=1(README § Configuration).How
guardinsrc/index.tstimes the action and callstrackCommandafter it settles. The send is one bounded attempt (1.5 s) so a blackholed network cannot hang the CLI, andtrackCommandnever throws or touches the exit code.insta run,insta db connect, and the approval paths ofdeploy/runcalledprocess.exit()inside the action, which would skip the report. They now setprocess.exitCodeor throwCliExit; observable exit codes are unchanged (buildFromSourcehas one caller with no try/catch between it andguard;runWithSecretspropagates the rejection).util.tsfail()remembers its last message so aCliExitcan be reported with the text the user saw.Not covered
Commands that call
process.exit()before returning — the interactive prompt cancellations (feedback,template deploy, theservices addpicker) andobserve hook— emit no event.Verification
npm run typecheck && npm test: 48 files, 688 tests green (test/telemetry.test.tsis new).npx tsx src/index.tsand the ingestion host temporarily pointed at a local capture server:org listagainst a dead API (error event witherror_type: TypeError),secrets set K value(value arrives as[REDACTED]),DO_NOT_TRACK=1(nothing sent),--help(nothing sent),build(exit 1, no error text). No secret value in the capture.🤖 Generated with Claude Code
Summary by cubic
Adds usage analytics to the CLI: previously no command data was collected; now every finished
instacommand sends onecli_commandevent to the same PostHog projects the console uses. Sensitive values are redacted locally,DO_NOT_TRACK=1orINSTA_NO_TELEMETRY=1opts out, and custom API hosts report nothing.Redaction and reporting
[REDACTED], and--setassignments are dropped whole.--env, accepted case-insensitively, or--api-url), not by the previous configuration.db connect,compute exec,deploy, andrunnow avoidprocess.exit()so the report is not skipped; observable exit codes are unchanged, and relayed child exit codes are reported separately from the CLI outcome.cancelledevent; anonymous events stay personless ($process_person_profile: false).observe hook) still emit no event.Written for commit c5c4a8a. Summary will update on new commits.