feat(cli): replace HashiCorp checkpoint telemetry with Sentry usage metrics and consent

[so0k]

Part 3 of 6 in a stack; review order S1 → S6; base is the previous slice.

1. chore(deps): upgrade @sentry/node to 10.x with unchanged reporting behaviour
2. fix(cli): stop the top-level error handler racing yargs
3. feat(cli): replace HashiCorp checkpoint telemetry with Sentry usage metrics and consent (this PR)
4. feat(cli): report the installed binary, target versions and platform in usage metrics
5. feat(cli): report per-stack, per-provider and per-command usage metrics
6. chore(gha): run the telemetry delivery e2e on every build

Related issue

Resolves #48

Description

This is the slice that answers the issue: the CLI stops sending anything to checkpoint-api.hashicorp.com, and usage analytics go to the project's own Sentry instead, behind their own consent flag. S4 and S5 add attributes and per-item metrics on top of the contract established here; nothing in this slice depends on them.

What changes:

• The HashiCorp transport is deleted. packages/@cdktn/commons/src/checkpoint.ts and packages/@cdktn/cli-core/src/test/checkpoint.test.ts are removed, and nock leaves packages/@cdktn/cli-core/package.json (that test was its only user), so the lockfile moves again. The preserved getUserId / getProjectId relocate to packages/@cdktn/commons/src/identity.ts.
• The telemetry contract, packages/@cdktn/commons/src/telemetry.ts: the consent reader, the decision captured at command start plus hasCapturedUsageTelemetryDecision, isUsageTelemetryEnabled, the single bounded flushTelemetry, sendErrorTelemetry, startCommandTelemetry emitting cli.command.invoked at the start of a run, and sendTelemetry emitting cli.command.completed, cli.command.error and cli.synth.duration, all with attributes {command, ci, language}. The rule those three command metrics follow is written out under One emission rule for the whole stack below.
• Consent, packages/@cdktn/cli-core/src/lib/error-reporting.ts: a per-flag interactive prompt (TTY, not CI), persistence back to cdktf.json, a projectPath parameter so a project directory that does not exist yet is handled, and the setUsageTelemetryEnabled capture. packages/@cdktn/commons/src/config.ts gains sendUsageTelemetry alongside sendCrashReports on ConfigBase, and the six templates' cdktf.json carry the flag. Because the templates render the flags as "{{sendUsageTelemetry}}", an on-disk value may be the string "true" or "false"; parseConfig now normalises both consent flags to booleans, and drops any other value as unset with a debug log, so the declared boolean type is true of everything the rest of the codebase reads.
• Emission sites: packages/@cdktn/commons/src/errors.ts (factories call sendErrorTelemetry(type, scope) at call time, plus Errors.getScope()), packages/@cdktn/cli-core/src/lib/synth-stack.ts (command metric and duration, and failure paths that emit the error metric, flush, then exit), packages/@cdktn/cli-core/src/lib/cdktf-project.ts, packages/cdktn-cli/src/bin/cmds/handlers.ts, packages/cdktn-cli/src/bin/cmds/ui/get.ts, and the init path (packages/@cdktn/cli-core/src/lib/init.ts, packages/cdktn-cli/src/bin/cmds/init.ts, packages/cdktn-cli/src/bin/cmds/helper/init.ts, packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts).
• convert collects consent like every other interactive handler: convert() passes askForCrashReportingConsent and askForUsageTelemetryConsent, and does so before it builds its temporary project and before any process.chdir, so the decision is captured against the user's real cwd and hasCapturedUsageTelemetryDecision keeps the nested init from overwriting it. A handler-level test covers the prompt, the persistence, the no-re-prompt case and the non-interactive path.
• Failure paths throw instead of exiting. The synth --check-code-maker-output guard, the watch --auto-approve guard and the Terraform availability/version check in packages/cdktn-cli/src/bin/cmds/helper/terraform-check.ts now throw Errors.Usage(...) rather than printing and calling process.exit(1), so each of them reaches the single failure path: one line of output, one cli.command.error{error_type: Usage}, one bounded flush, one exit. The direct exits that remain all run before reporting is initialised (an unknown command, and init's empty-directory, template-download and login checks), and the two self-exiting synth paths emit and flush themselves.
• Process-global state for the two bundles. esbuild builds two entry bundles, each with its own copy of @cdktn/commons, so module-level state is per bundle rather than per process. The command scope, the captured consent decision, the target attributes and the started-command record all live in a store keyed by Symbol.for, which makes them one value per run whichever copy reads them. This is what used to make an error raised inside @cdktn/hcl2cdk count cli.error with command: "unknown"; the delivery e2e never caught it because it only asserted metric names, and it now asserts attribute values as well.
• The entrypoint reroute: packages/cdktn-cli/src/bin/error-handling.ts and cdktn.ts drop their direct Sentry.flush / Sentry.close calls in favour of flushTelemetry(), and reportFailure gains a sendCommandErrorTelemetry dep and the commandErrorType computation.

Start with packages/@cdktn/commons/src/telemetry.ts. It is the whole contract in one file: the consent gate, what is emitted, and the only flush. Then read packages/@cdktn/cli-core/src/lib/error-reporting.ts for how consent is asked and stored.

Reading order for this slice:

1. packages/@cdktn/commons/src/telemetry.ts
2. packages/@cdktn/cli-core/src/lib/error-reporting.ts
3. packages/@cdktn/commons/src/errors.ts and identity.ts
4. packages/@cdktn/cli-core/src/lib/synth-stack.ts and cdktf-project.ts
5. packages/cdktn-cli/src/bin/error-handling.ts, cdktn.ts (the flush reroute and the failure metric)
6. packages/@cdktn/commons/src/telemetry.test.ts and packages/@cdktn/cli-core/src/test/error-reporting.test.ts

Things worth calling out

• The typed CdktfConfig.sendUsageTelemetry getter was removed rather than kept. It had zero production callers and parsed the flag differently from the reader that actually gates emission, throwing on a malformed value instead of treating it as unset. Both consent readers (packages/@cdktn/commons/src/telemetry.ts and cli-core's crash-reporting reader) use normalizeConsentFlag, so a malformed value reads as unset. ConfigBase keeps the field so cdktf.json's schema is still documented.
• cli.command.error is emitted at most once per failed run. Synth paths that exit inside cli-core count themselves and then exit; a synth error that is rethrown instead is counted by the entrypoint. A failed run is therefore never counted twice, and never counted zero times when it reaches the reporter. The tests pin both halves of that rule.

One emission rule for the whole stack

This is the rule for S3 through S6, stated here because the metrics are defined here.

• cli.command.invoked is emitted exactly once per run, at command start, inside initializErrorReporting right after Sentry is initialised and under the command scope. It counts attempts, not successes.
• cli.command.completed is emitted once at the end of a successful run, and is the metric that carries the scalars only known at the end: the convert stats, the init template, the get counts, and in S5 the synth origin.
• cli.command.error is emitted once per failed run.

error / invoked is therefore a true failure rate for every command, and invoked no longer means one thing for synth and another for deploy. Consequences worth knowing:

• Nested operations add only their own metrics. A deploy's synth, init's get and convert's throwaway init do not count as runs of their own.
• Commands that never initialise reporting emit nothing at all: login, debug, provider list and provider upgrade. provider add counts as one invoked and one completed, whether or not its nested get runs.
• The watch start-event metric is gone. invoked at command start already records exactly that, and a gracefully stopped watch, list, output and a get with nothing to fetch each count as completed.
• The two self-exiting synth failures count under the command the run was started as, so a failed cdktn deploy counts as one deploy invoked and one deploy error, because deploy calls synth non-gracefully.

What is collected

Everything below is gated by isUsageTelemetryEnabled() (CHECKPOINT_DISABLE > the decision captured at command start > sendUsageTelemetry in cdktf.json > default on) and a Sentry DSN in the build. Crash reports are a separate consent (sendCrashReports) and are unchanged. S4 and S5 extend this set; this is what the slice emits on its own.

Base attributes on every metric below: command; ci (the CI provider's name, or false); language (one of the supported languages, else omitted). language is read from cdktf.json at command start for cli.command.invoked and from the run's own payload for cli.command.completed and cli.command.error, which is why a run outside a project carries no language on any of them.

Metric	Value	Additional attributes
cli.command.invoked	1 per run, at command start	none in this slice
cli.command.completed	1 per successful run, at the end	none in this slice; S5 adds the end-of-run scalars
cli.command.error	1 per failed run	error_type: Usage, External, Internal, unexpected
cli.synth.duration	synth wall time in ms (distribution)	none
cli.error	1 per error built by the Errors factories	type (Usage, External, Internal), command (the Errors scope, read at call time)

Stamped by the Sentry SDK itself: a random, persistent user id kept in ~/.cdktf/config.json (delete the file and a new one is generated); the release cdktn-cli-<version>, which is how the CLI version is known; a fixed environment production; the SDK name and version; the fixed server.address cdktn-cli; a random per-process trace id and the emission timestamp; and one session record per process carrying the Node major version. Crash reports additionally carry the projectId from cdktf.json.

What is never sent

Stack names; resource ids and addresses; file paths and the working directory; the machine hostname or username; error messages and error context; any user code; the values of SENTRY_* environment variables. Unit tests assert these against the raw envelope bytes.

Migration notes (user-facing)

• cdktn no longer sends any data to HashiCorp. The checkpoint transport is gone entirely. Usage analytics go to the project's own Sentry, and only when enabled.
• New sendUsageTelemetry flag in cdktf.json, independent of sendCrashReports. Everything it covers is listed above.
• One-time interactive prompt on upgrade. On the first run in an interactive terminal (TTY, not CI), a project whose cdktf.json lacks sendUsageTelemetry is asked once and the answer persisted. You may also be asked about crash reporting if sendCrashReports is missing: that prompt existed before but never fired due to a bug (a missing key was silently treated as "no"), and it now works as originally intended.
• Non-interactive runs (CI, piped) are unchanged. No prompts; usage telemetry stays on by default as it always was, now routed to Sentry, and crash reporting stays off by default.
• CHECKPOINT_DISABLE keeps working. It still disables usage telemetry everywhere and still does not affect crash reporting. Telemetry is fully inert when the build has no Sentry DSN, for example in self-built binaries.

Situation	Before	After
Usage data recipient	HashiCorp checkpoint API	the project's own Sentry
sendUsageTelemetry unset, interactive	sent silently, no prompt existed	prompted once, choice saved
sendUsageTelemetry unset, CI or non-interactive	sent	sent, to Sentry
CHECKPOINT_DISABLE set	no usage data	no usage data
sendCrashReports missing, interactive	treated as "no", the prompt was broken	prompted once, choice saved
Machine hostname in crash reports	sent (Sentry default)	scrubbed (serverName: cdktn-cli, landed in S1)
Per-stack usage data	backend, provider and override details posted to HashiCorp	not sent at all in this slice; restored as enumerated values and counts in S5
Errors built by the CLI	{type, message, context} posted to HashiCorp	counted by type and command only, no message, no context

Test plan

• Unit tests green across @cdktn/commons, @cdktn/cli-core and cdktn-cli on this slice against S2
• Delivery oracle: a real v10 client with a capturing transport asserts the emitted metric names and the exact sorted attribute key set of cli.command.invoked
• The emission rule: exactly one cli.command.invoked per run at start, one cli.command.completed on success, one cli.command.error on failure; a nested operation (a deploy's synth, init's get, convert's throwaway init) adds no second run
• convert consent at the handler level: a missing sendUsageTelemetry under an interactive terminal invokes the usage-consent callback and persists the answer, a flag already set is not re-prompted, and the non-interactive path stays prompt-free
• The synth code-maker guard, the watch --auto-approve guard and the Terraform availability check each yield one cli.command.error{error_type: Usage} and one flush before the exit
• parseConfig normalisation rows: booleans pass through, "true" / "false" become booleans, any other value is dropped as unset with a debug log, for both consent flags
• No-host-identity invariant asserted on the raw envelope bytes
• Consent gating matrix: per-flag prompt, precedence, CHECKPOINT_DISABLE, no-project, CI, the tri-state reader, persistence, and the init-time capture of the decision for a project directory that does not exist yet
• cli.error cases including the call-time scope read, so a later setScope is honored
• cli.command.error counted exactly once per failed run: self-exiting synth paths and the rethrow-to-entrypoint path both covered
• Inertness with no Sentry client, and the bounded flushTelemetry behaviour
• The consent, prompt and CHECKPOINT_DISABLE cases moved into their own describe, leaving the Sentry init hardening cases from S1 in theirs
• Lint and pnpm prettier --check . clean
• Maintainer, one time: enable the Sentry "Logs and Metrics" product (org-side, SDK 10 and above gate)

Review threads from #62 answered here

• "doesn't really add any value", on the static no-hashicorp-egress.test.ts: agreed, and this stack never introduces it. A source scan for a string that no longer exists in the tree asserts what a build failure already asserts. The grep over the real bundle artifact lands in S6, which is the only place that assertion can still be wrong.
• "doesn't seem to add value", on the nock no-hashicorp-runtime-egress.test.ts: agreed, also never introduced here. The nock canary only re-proved that Sentry's own transport posts to the DSN host, which is not a fact about this change. That is why nock is dropped from cli-core in this slice, where it was the only user.
• "at least 2 other spots doing something similar", on the duplicated flush: correct, and they are one helper now. The beforeExit, yargs failure and self-exiting synth paths all route through flushTelemetry(). Nothing outside packages/@cdktn/commons/src/telemetry.ts calls Sentry.flush or Sentry.close, so the timeout lives in exactly one place.
• "dropped Errors reporting": fair, dropping it outright was too far. The signal comes back as a count: every error the factories build emits cli.error with the error type and the command scope, read at call time. The message and the context stay out, and it goes through the usage-telemetry gate like every other metric, so it is a counter rather than the old fire-and-forget POST of error text.
• "isTTY is not accurate in every terminal": noted, taken as informational. The check stays as it is, consistent with the rest of the codebase.

Follow-ups (documented, not in this PR)

1. cdktn debug prints neither consent flag. Both sendUsageTelemetry and sendCrashReports should show up there so a user can see what a given project sends without reading cdktf.json.
2. yargs pre-handler validation failures emit no metric (invalid choice, unknown option): Sentry is initialized inside the command handlers, so the reporter counts them against a client that does not exist yet. Same on main.

Checklist

• I have updated the PR title to match CDKTN's style guide
• I have run the linter on my code locally
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation if applicable (follow-up in cdk-terrain-docs)
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works if applicable
• New and existing unit tests pass locally with my changes

[jsteinich]

Reviewed as part of the full tele/s1–tele/s6 stack. The consent model here is the part I scrutinised hardest and I think it's right:

• initializErrorReporting initialises Sentry when either flag is on, and beforeSend returns null when only usage telemetry is enabled — so metrics flow without a single error event leaking to a user who never opted into crash reporting. That's the correct decomposition and it's easy to get wrong.
• The hasCapturedUsageTelemetryDecision() guard genuinely works: convert calls initializErrorReporting() at handlers.ts:96 before the process.chdir into the throwaway project, so the decision is captured against the user's real cwd and the nested init correctly declines to overwrite it. I traced both the TypeScript (needsProject === false) and non-TypeScript paths.
• Fixing shouldReportCrash to return undefined for an absent key — rather than silently false — is the right call even though it means existing users see a new prompt on upgrade. Good that it's called out in the description.

Two things.

sendUsageTelemetry is typed boolean but rendered as a string

The templates emit "sendUsageTelemetry": "{{sendUsageTelemetry}}", so a freshly inited project has the string "true" / "false" on disk, while ConfigBase declares:

readonly sendUsageTelemetry?: boolean;

getUsageTelemetryConsent handles the string explicitly (and the comment explains why), so nothing is broken today. But the declared type is now a lie about what's actually in the file, and the trap is asymmetric: any future if (config.sendUsageTelemetry) through the typed path reads the string "false" as true — i.e. it fails open, silently enabling telemetry for a user who opted out.

This mirrors the pre-existing sendCrashReports trap rather than introducing a new one, so I don't think it blocks. But since this PR is the one adding both fields to ConfigBase, it's the natural place to normalise them in parseConfig and let the rest of the codebase trust the type.

Heads-up for #413: this sets up an inconsistency in cli.command.invoked

Not actionable in this PR, recording it here because the divergence starts with the paths introduced here. synth-stack.ts:181 and :203 emit cli.command.error and then hard-exit, so a failed synth never emits cli.command.invoked. In #413, finishStackRun sends telemetry before throwing, so a failed deploy emits both. Details and a suggested fix are on #413.

[sakul-learning]

Review: usage-consent and failed-command telemetry gaps

Reviewed at head e3f991439401c7cf8a3b8a437418e4265c7da234 against this stacked layer's base, tele/s2-error-handler (8131b2158f296dc147812c61ab8e0baf04152f87). I found two handler-level gaps in the telemetry contract introduced by this layer.

1. convert() does not collect usage-telemetry consent

packages/cdktn-cli/src/bin/cmds/handlers.ts:90-97 starts convert() with:

await initializErrorReporting();

Unlike deploy, destroy, diff, get, list, synth, watch, and output, it does not pass askForUsageTelemetryConsent. For an interactive conversion in a project whose cdktf.json has no sendUsageTelemetry value, this skips the one-time decision/persistence flow while the usage-telemetry default remains enabled.

I reproduced this with an isolated temporary project under a pseudo-TTY, with SENTRY_DSN empty so the test could not deliver telemetry. cdktn convert --language typescript main.tf proceeded directly to its Terraform-source input prompt:

? Enter your Terrafrom code to convert to cdktn ...

No usage-telemetry consent prompt appeared first. The process was stopped at that unrelated conversion-input prompt. This confirms that the interactive convert path is live and bypasses consent collection; it is not merely a source-level inference.

Please make convert() pass the same crash- and usage-consent callbacks as the other interactive command handlers, with the decision captured before its temporary-project work begins. A focused handler-level regression test should assert that a missing sendUsageTelemetry setting invokes and persists the usage-consent callback, while an existing setting does not re-prompt.

2. Direct handler exits bypass cli.command.error emission and telemetry flushing

The entrypoint's reportFailure() sends the cli.command.error metric and awaits flushTelemetry(SENTRY_FLUSH_TIMEOUT_MS). However, these command paths initialize telemetry and then terminate the Node process directly instead of throwing into the entrypoint:

• packages/cdktn-cli/src/bin/cmds/handlers.ts:487-492 — synth --check-code-maker-output when the configured code-maker directory is absent
• packages/cdktn-cli/src/bin/cmds/handlers.ts:520-524 — watch without --auto-approve
• packages/cdktn-cli/src/bin/cmds/helper/terraform-check.ts:67-69 — failed Terraform availability/version validation used by multiple commands

For example, I ran cdktn watch without --auto-approve in an isolated valid project with both telemetry flags disabled. It deterministically reached the guard and exited 1 with:

ERROR: The watch command always automatically deploys and approves changes. To make this behaviour explicit the --auto-approve flag must be set

That probe establishes the direct-exit path is reachable. Because telemetry was intentionally disabled, it does not attempt to observe an outbound metric; the control flow establishes the delivery gap: process.exit(1) prevents runCli() from reaching reportFailure(), so neither the failed-command metric nor the bounded flush can occur. This is the same class of hard-exit handling that SynthStack.synth() now treats specially in this PR.

Please replace these direct exits with typed errors handled by reportFailure(), or use a single helper that emits the failed-command metric, flushes telemetry, and only then exits. Add focused regression coverage for at least the watch guard and the Terraform-check failure path, asserting one error metric and one flush before exit.

Focused validation

After building the changed packages (@cdktn/commons, @cdktn/cli-core, and cdktn-cli), the following targeted suites passed:

packages/@cdktn/cli-core/src/test/lib/synth-stack.test.ts
packages/cdktn-cli/src/bin/cmds/helper/__tests__/init-telemetry.test.ts
packages/cdktn-cli/src/bin/__tests__/error-handling.integration.test.ts

Result: 3 suites passed, 11 tests passed. These tests validate the entrypoint failure reporter and the dedicated SynthStack hard-exit path, but do not exercise convert() consent wiring or the handler-level direct exits above. The proposed regressions would cover those remaining paths.

[so0k]

@sakul-learning both gaps are fixed on the branch, one commit each.

fix(cli): collect crash and usage consent in convert before the temporary project makes convert() pass askForCrashReportingConsent and askForUsageTelemetryConsent like the other interactive handlers, and does it before the temporary project is built and before any process.chdir, so the decision is captured against the user's real cwd and the hasCapturedUsageTelemetryDecision guard keeps the nested init from overwriting it; the handler-level regression test covers the three cases you named, a missing sendUsageTelemetry under an interactive terminal invoking and persisting the usage-consent callback, an existing setting not re-prompting, and the non-interactive path staying prompt-free.

fix(cli): throw usage errors instead of exiting from the synth, watch and terraform checks replaces the three console.error plus process.exit(1) sites with throw Errors.Usage(...), so each reaches the single failure path in reportFailure: one line of output, one cli.command.error{error_type: Usage}, one bounded flush, one exit. I swept the rest of packages/cdktn-cli/src and packages/@cdktn/cli-core/src/lib for process.exit( reachable after reporting is initialised; what is left all runs before reporting exists (an unknown command, and init's empty-directory, template-download and login checks), and the two self-exiting synth paths already emit and flush themselves, so they stay as they are. Regression coverage drives the watch guard and the Terraform-check failure and asserts exactly one error metric and one flush before the exit.

@jsteinich on the string-typed flags: parseConfig now normalises both sendCrashReports and sendUsageTelemetry, a boolean passing through, "true" and "false" becoming booleans, and anything else dropped as unset with a debug log, so the declared type is true of what the rest of the codebase reads and the fail-open trap is closed here rather than left for the next caller.

On the cli.command.invoked divergence you flagged for #413: the semantic is picked here at the root rather than patched downstream, invoked is now emitted exactly once per run at command start, a successful run ends with one cli.command.completed carrying the scalars only known at the end, and a failed run ends with one cli.command.error, which makes error / invoked a true failure rate for every command; the rule is in a comment next to the metric names and written out in the PR description.

[sakul-learning]

Re-review at 6fba3893: both prior findings are fixed, two remaining lifecycle/consent gaps

Re-reviewed at head 6fba3893d6f0eb3a1835f78d27487091e16b0c8f against this stacked layer's base, tele/s2-error-handler (8131b2158f296dc147812c61ab8e0baf04152f87).

Both previously reported gaps are resolved:

• convert() now passes askForCrashReportingConsent and askForUsageTelemetryConsent (packages/cdktn-cli/src/bin/cmds/handlers.ts:99-100), alongside the other interactive handlers, before its temporary project is created.
• The three direct exits now throw typed errors that reach reportFailure(): packages/cdktn-cli/src/bin/cmds/helper/terraform-check.ts:52 and packages/cdktn-cli/src/bin/cmds/handlers.ts:494 and :525.

Two gaps remain in this layer.

1. Malformed consent values are treated as an explicit opt-out instead of an unset decision

parseConfig in packages/@cdktn/commons/src/config.ts:312-326 normalises a boolean or "true"/"false" and drops every other value as unset. But the two paths that actually decide consent do not call it: each reads the raw cdktf.json field itself and folds every present non-boolean value except exactly "true" down to false.

• Usage telemetry — packages/@cdktn/commons/src/telemetry.ts:52-54
• Crash reporting — packages/@cdktn/cli-core/src/lib/error-reporting.ts:35-37

return typeof cdktfJson.sendUsageTelemetry === "boolean"
  ? cdktfJson.sendUsageTelemetry
  : cdktfJson.sendUsageTelemetry === "true";

For a project containing { "sendCrashReports": 1, "sendUsageTelemetry": "yes" } the observable behaviour is:

• Interactive terminal: both readers return false rather than undefined, so neither === undefined guard in initializErrorReporting fires. The flags are present so the guard also skips the prompt-and-persist path. A project that most likely meant "unset" is silently read as an explicit refusal and no corrected decision is ever written back to cdktf.json.
• Non-interactive/CI: crashReportingEnabled and usageTelemetryEnabled are both false, so usage telemetry is off where the documented non-interactive default is on.

This contradicts the stated contract that an unparseable value is "dropped as unset" with the declared type holding true for everything the rest of the codebase reads. It fails closed rather than leaking data, which is why I am flagging it as a correctness/contract gap rather than a privacy one — but it also means the promised one-time prompt never happens for a project with a malformed flag.

Existing tests only cover booleans, "true", "false", and absent fields; telemetry.test.ts:476-493 (usage) and error-reporting.test.ts (crash) do not exercise a malformed present value, so the parser tests in config.test.ts:730-765 do not protect this behaviour.

Suggested fix: have getUsageTelemetryConsent and shouldReportCrash share the same tri-state normaliser as parseConfig (returning undefined for anything unparseable), and add a case asserting a malformed present value behaves exactly like an absent one through initializErrorReporting.

2. A successful watch run emits cli.command.invoked but no terminal event

The lifecycle rule added in this PR is that a run emits cli.command.invoked once at start, then exactly one of cli.command.completed or cli.command.error, so that error / invoked is a true failure rate per command.

For watch the second half never happens:

1. handlers.ts:510-513 calls initializErrorReporting, which emits cli.command.invoked.
2. runWatch is awaited (handlers.ts:531).
3. packages/@cdktn/cli-core/src/lib/watch.ts:188 resolves only when the abort signal fires; a graceful SIGINT/SIGTERM/SIGQUIT closes the watcher, settles the in-flight run, and returns normally.
4. The successful path through runCli() only flushes and exits (packages/cdktn-cli/src/bin/error-handling.ts:186-195); it never calls sendTelemetry, and the old sendTelemetry("watch", { event: "start" }) line was removed by this PR.

So a normal cdktn watch session that the user interrupts ends with one cli.command.invoked and no completed or error. That makes error / invoked understate failures for watch specifically, since invocations accumulate and never resolve. The comment next to the metric names does not list watch as an exception.

watch is not covered for this either: watch.test.ts asserts the pending-until-aborted control flow, and handlers-watch.test.ts covers the --auto-approve failure and its error metric, but no test asserts a terminal event after graceful shutdown.

Suggested fix: emit the completion metric after await stopped in watch.ts (or in the handler immediately after runWatch resolves), and add a test asserting one cli.command.completed for a gracefully aborted watch.

Narrower note on provider add

The PR description documents provider add as the deliberate invoked-without-completed exception. That exception holds when the nested get() runs, since sendTelemetry("get", …) is suppressed when startedCommand is "provider add". But when providerAddLib returns false and no nested get runs, providerAdd() (handlers.ts:650-687) never initialises reporting, so it emits no invocation at all — which contradicts the description's claim that provider add counts as invoked. Either initialise reporting directly in that handler or tighten the stated exception to note it only applies when a nested get runs.

Checks

The three configured commands target the untouched packages/cdktn project (validations.test.ts, nx build cdktn, nx test cdktn) and are not applicable to this layer's changes in @cdktn/commons, @cdktn/cli-core, and cdktn-cli; I did not substitute other test commands. git diff --check against the layer base is clean. GitHub reports 200 checks passing, 58 pending, 1 skipped, and no failures. No .github/workflows changes, so no workflow-security comparison applies. The complexity hotspots named in the preflight (cdktf-project.test.ts, runInit, telemetry.test.ts) did not yield a separate defect.

[jsteinich]

Re-reviewed at 6fba3893d. The bundle-copy fixes (process-state.ts plus sharing the scope and the consent decision) are a good catch — in the shipped bundle Errors.getScope() would always have returned "unknown" and the consent capture would never have crossed from handlers.js to cdktn.js, and no unit test could have seen it. The invoked / completed / error triad is a better answer than what I suggested, and asserting the exact multiset of run metrics in the e2e is the right guard for it.

One new finding, distinct from the two @sakul-learning has open.

aa4dc6a9d makes convert prompt on the stdin it is about to read

convert() now passes both consent prompts to initializErrorReporting, which runs before readStreamAsString(process.stdin). But canPrompt (error-reporting.ts:100-104) gates on stdout only:

const canPrompt =
  Boolean(process.stdout.isTTY) &&
  !ciInfo.isCI && !process.env.CI &&
  fs.existsSync(path.resolve(projectPath, "cdktf.json"));

@inquirer/prompts' confirm reads from process.stdin. So cat main.tf | cdktn convert, run in an interactive terminal inside a project whose flags are unset, shows a prompt that consumes the Terraform source as its answer — persisting a consent decision derived from the user's HCL (anything not matching y/yes resolves false), and leaving convert with truncated or empty input. Both halves fail silently.

The e2e's convert trigger pipes stdin but pre-seeds both flags in cdktf.json, so it stops short of the prompt.

Fix is one clause — Boolean(process.stdin.isTTY) && — and it's right generally rather than as a convert special case: prompting when stdin is not a terminal is never correct. Worth an e2e case that pipes HCL into convert with the flags absent and asserts the source still converts.

[so0k]

Thanks for the re-review.

1. Malformed consent values (0483757): getUsageTelemetryConsent and shouldReportCrash now use parseConfig's normalizeConsentFlag. A value such as 1, "yes" or null now reads as unset, not as an opt-out. It prompts when interactive and falls back to the non-interactive defaults otherwise.

2. watch without a terminal event (c72e7ba): you were right, and list, output and a no-op get (no providers or modules) had the same gap. Each of those handlers now sends its own cli.command.completed on success. A nested synth still reports as synth, not as the outer command.

3. provider add (adb9ef2): providerAdd now initializes reporting itself and sends its own completion, with or without a nested get. The nested get's completion does not count separately. We also flush telemetry before the nested get, in both provider add and init, because get re-initializes Sentry, and without the flush the buffered invoked metric could be lost on a fast run. A test covers this ordering.

[so0k]

Good catch on the piped stdin (2a021f1). canPrompt (cli-core) and isInteractiveTerminal (cli) now also require process.stdin.isTTY, so cat main.tf | cdktn convert no longer takes the piped HCL as the consent answer. Unit tests cover convert with piped stdin, canPrompt, and isInteractiveTerminal (stdout TTY with stdin piped, stdin TTY with stdout piped, CI set). An end-to-end check needs a pty wrapper in CI, so it belongs with the e2e script in #414 if we want one.

[jsteinich]

Re-reviewed at b6f6f0fd4. The stdin fix (2a021f139) is right, and hardening isInteractiveTerminal() the same way covers init's prompts too — good catch beyond what I reported. Sharing normalizeConsentFlag between parseConfig, getUsageTelemetryConsent and shouldReportCrash is the correct resolution of the tri-state read.

One defect, and two things I'd rather ask than assert.

A run of init can emit both completed and error

runInit sends the init completion metric at its end, and only then does handlers.ts run the nested get:

const { needsGet, codeMakerOutput, language } = await runInit(argv);  // invoked + completed
if (needsGet) {
  await flushTelemetry();
  await get({ ... });                                                 // can throw
}

The nested call is handlers.get, not the yargs module, so Errors.setScope("get") never runs and the scope stays "init". A failing nested get — a provider that will not generate, a jsii failure, a schema fetch error — therefore produces invoked{init} + completed{init} + error{init}.

That breaks this round's headline invariant ("exactly once per run at start, then exactly one of completed or error") for the one command where the nested operation is most likely to fail, and it inflates init's completion rate: completed/invoked and error/invoked will sum above 1 for init alone.

provider add already has the right shape — adb9ef2f8 put its sendTelemetry("provider add", {}) after the nested get, so a failure skips it. Giving init the same shape (emit the init completion at the end of handlers.init, after the needsGet block) fixes it and makes the two read alike.

The e2e's exact-multiset assertion is the right guard for exactly this class of bug, but it has no init trigger, so it does not cover this. An init trigger whose nested get fails would be a good addition alongside the fix.

Comment density

This is a house-style question rather than a finding, so treat it as one. telemetry.ts is 139 of 665 non-blank lines comments and error-handling.ts 38 of 191, and most of that is rationale sitting at the declaration site rather than in the commit body — the opposite of how the rest of this repo reads.

A subset genuinely earns its place because it stops a future regression rather than explaining a decision: the "must stay synchronous, an await here races Node's unhandled-rejection reporter" note in runCli, the bundle-copy warnings on processState and the scope store, and the "telemetry.ts must never import this module" note in errors.ts. Those are warnings to the next editor and should stay whatever else happens.

Not asking for a sweep across the stack at this point if you would rather keep the momentum — but worth a deliberate call, because once merged this becomes the reference style for the next person touching telemetry.

Is the untracked-command set the one you want?

Line 56 documents login, debug, provider list and provider upgrade emitting nothing at all, so this is a stated decision rather than a gap. Flagging it only because the new consent prompt added in 7484b9a09 tells users the data exists so the project can "focus on what is actually used by the community to prioritize development".

Those four will read as zero usage indefinitely, while their sibling provider add is counted — so the one comparison a reader is most likely to make within the provider subcommands is the one the data cannot support. If that is acceptable for now, it is worth saying so in the docs the prompt links to, so the gap is visible to whoever reads the dashboards later rather than only to whoever reads this PR.