feat(cli): add Sentry error telemetry + stabilize flaky stat tests by designcode · Pull Request #200 · tigrisdata/storage

designcode · 2026-07-20T14:48:10Z

What

Two related reliability changes to the CLI, in separate commits.

1. Stabilize flaky `stat` integration tests (`test(cli)`)

The org-wide stat command issues a single getStats request against a live, eventually-consistent gateway. A transient network blip made it exit non-zero (network/general), failing two overall-stats tests that assert exitCode === 0 (expected 5 to be 0 / expected 1 to be 0). The CLI vitest config had no retry, unlike packages/storage which uses retry: 2 for exactly this reason.

Adds retry: 2 to packages/cli/vitest.config.ts. Deterministic failures still fail all attempts, so real regressions aren't masked.

2. Add Sentry error telemetry (`feat(cli)`)

Reports errors to Sentry so we can debug customer issues.

Scope: crashes (uncaught/unhandled) + unexpected general and network errors. Expected user-facing conditions (auth, permission, not_found, rate_limit) are not reported.
Delivery: exitWithError() stays synchronous (preserves the never contract command code relies on), so handled errors are captured best-effort. The global crash handlers capture + await flush before exit, so crashes are delivered reliably.
Context: command, error category, exit code, CLI version, platform/arch.
Privacy: sendDefaultPii: false; beforeSend scrubs secrets (access keys, tokens, credential flags) and the machine hostname; argv values after credential flags are redacted. Unit-tested (test/utils/telemetry.test.ts, 12 tests).
Off by default when unconfigured, in dev/test, and on opt-out (TIGRIS_NO_TELEMETRY=1 / DO_NOT_TRACK=1). Never throws into the command path.

Also folds in a pre-existing biome 2.5.4 format drift fix (two test files + biome.json schema bump) so the pre-commit formatter passes.

Validation

tsc, biome, and unit tests pass.
Sentry delivery confirmed end-to-end (real event accepted, flush returned true).
Validated under both the npm build and the bun-compiled binary (brew distribution) — no OTel/compile issues, graceful degradation if init ever fails.
Steady-state overhead ~30–80ms init + ~60ms capture; inert path unchanged.

Follow-ups (not in this PR)

Guaranteed handled-error delivery — a boundary refactor (route handled errors to a single awaited command boundary; requires auditing the 19 try/catch blocks in src/lib) so general/network errors flush reliably too.
Docs for the opt-out env vars.

🤖 Generated with Claude Code

Note

Medium Risk
New outbound telemetry and embedded DSN affect all production CLI users unless opted out; privacy relies on scrubbing and disabled integrations. Crash-handler async exit path is a behavioral change from immediate synchronous exits.

Overview
The CLI now reports uncaught crashes and selected handled failures (general / network) to Sentry via a new telemetry module (@sentry/node), wired through setupErrorHandlers() (capture + flush before exit) and best-effort capture in exitWithError(). Events get command, category, exit code, and platform context; argv and payloads are scrubbed for credentials/PII and hostname is dropped. Telemetry stays off in dev/test and with TIGRIS_NO_TELEMETRY=1 / DO_NOT_TRACK=1.

Test reliability: packages/cli/vitest.config.ts gains retry: 2 for live-gateway flakes. Integration cleanups in agent-kit, storage, and CLI now delete buckets LIFO so fork/source ordering does not leak resources; CLI rebase/merge integration cases are it.skip until snapshot teardown is fixed. Minor Biome 2.5.4 formatting and lockfile updates for Sentry/OpenTelemetry deps.

^{Reviewed by Cursor Bugbot for commit 50802c5. Bugbot is set up for automated code reviews on this repo. Configure here.}

The org-wide `stat` command issues a single getStats request against a live, eventually-consistent gateway; a transient network blip made it exit non-zero (network/general) and fail two overall-stats tests that assert exitCode === 0. Add retry: 2 to the CLI vitest config, mirroring packages/storage, so transient failures are absorbed while deterministic failures still fail every attempt. Assisted-by: Opus 4.8 via Claude Code

Report crashes plus unexpected (general) and network errors to Sentry so we can debug customer issues, with the command, error category, exit code, CLI version, and platform attached. Expected user-facing conditions (auth, permission, not_found, rate_limit) are not reported. Telemetry is initialized in setupErrorHandlers() and is a strict no-op unless a DSN is configured; it stays off in dev/test and on opt-out (TIGRIS_NO_TELEMETRY / DO_NOT_TRACK) and never throws into the command path. Secrets (access keys, tokens, credential flags) and the machine hostname are scrubbed in beforeSend. exitWithError() stays synchronous so the never-returning contract that command code relies on is preserved; handled errors are captured best-effort. The global crash handlers (uncaught/unhandled) capture and await a bounded flush before exiting, so crashes are delivered reliably. Validated under both the npm build and the bun-compiled binary (brew). Also reformat two test files and bump the biome schema to 2.5.4 to satisfy the pre-commit formatter (pre-existing drift from an earlier biome bump). Assisted-by: Opus 4.8 via Claude Code

greptile-apps · 2026-07-20T14:50:35Z

Greptile Summary

This PR adds error telemetry and improves CLI test reliability. The main changes are:

Sentry capture and bounded flushing for CLI crashes.
Best-effort reporting for handled general and network errors.
Secret redaction, telemetry opt-out controls, and CLI context.
Two retries for CLI tests that use a live gateway.
Dependency, changeset, and formatter metadata updates.

Confidence Score: 4/5

Invocation telemetry can disclose user and customer-resource data and should be narrowed before merging.

Crash capture, filtering, and flushing preserve existing exit behavior.
Known credential forms are redacted.
Explicitly attached argv values bypass default PII suppression.

packages/cli/src/utils/telemetry.ts

Security Review

Telemetry can send user-identifying and customer-resource data from CLI arguments. Known credentials are redacted, but usernames, paths, bucket names, and object keys remain in the explicitly added invocation context.

Important Files Changed

Filename	Overview
packages/cli/src/utils/telemetry.ts	Adds Sentry initialization, filtering, context, redaction, capture, and flushing; arbitrary argument values remain in invocation context.
packages/cli/src/cli-core.ts	Initializes telemetry and flushes captured crashes before preserving classified CLI exits.
packages/cli/src/utils/exit.ts	Adds synchronous handled-error capture while preserving the existing immediate-exit contract.
packages/cli/vitest.config.ts	Adds two retries across the CLI test suite for transient live-gateway failures.
packages/cli/package.json	Adds Sentry Node SDK as a runtime dependency.

_{Reviews (1): Last reviewed commit: "feat(cli): add Sentry error telemetry" | Re-trigger Greptile}

greptile-apps · 2026-07-20T14:50:38Z

+      arch: process.arch,
+    });
+    Sentry.setContext('invocation', {
+      command: scrubArgv(process.argv.slice(2)).join(' '),


Invocation Context Leaks User Data

sendDefaultPii: false does not sanitize context added by this code. scrubArgv() removes known credentials but still sends values such as notification usernames, local paths, bucket names, and object keys; for example, --username=user@example.com reaches contexts.invocation.command unchanged on every captured error.

The CLI tsconfig relies on automatic @types discovery for Node globals (process, console). `tsc` run from the package resolves the root-hoisted @types/node by walking up, but the editor's TypeScript server does not pick it up through pnpm's symlinked workspace layout, so it reported "Cannot find name 'process'/'console'" (and a cascade of any/unknown) even though the build was green. Declare `types: ["node"]` so the TS server and tsc agree. tsconfig.binary.json extends this config, so it inherits the setting. Assisted-by: Opus 4.8 via Claude Code

Address PR review: the invocation context sent the full argv (only known secret flags redacted), so positionals and non-secret flag values — bucket names, object keys, file paths, emails — reached Sentry, and contexts are not run through beforeSend. Capture only flag names now (invocationFlags), dropping every value and positional. Also drop Sentry default integrations that capture unscrubbed user data: LocalVariables/LocalVariablesAsync (local variable values, e.g. credentials, into stack frames) and Console/ChildProcess (breadcrumbs carrying printed output and spawned command lines). Assisted-by: Opus 4.8 via Claude Code

…e tests Integration teardowns removed tracked buckets in push order, but a fork is pushed after its source and a source cannot be removed while a dependent fork still exists — so the source delete failed (error unchecked) and the source bucket leaked. Delete in reverse (LIFO) so forks go before sources (bucket-create and agent-kit checkpoint/restore suites). Also disable the CLI fork rebase/merge tests: those operations snapshot the fork, and the snapshots block the fork's teardown, leaking the fork and its source bucket. Assisted-by: Opus 4.8 via Claude Code

redactSecrets used one replacer that treated the second arg as a capture group, but for the group-less patterns (JWT, Bearer, AKIA) that arg is the match offset — so a secret found mid-string was replaced with `<offset>[redacted]` (e.g. `9[redacted]`). The secret was still removed, but the scrubbed text was corrupted, and the tests missed it because they only asserted toContain('[redacted]'). Only treat the replacer arg as a prefix when it is a string, and assert exact output in the tests so the offset injection can't regress. Assisted-by: Opus 4.8 via Claude Code

…etry Address PR review: crash events carried no command identity (the invocation context only had flag names) and events never included the exit code despite the docs claiming so. Send the full command with arguments in the invocation context — kept for debugging — but redact credentials and PII: credential/PII flag values (access-key/secret, token, password, api-key, name, username, email, owner) detected by pattern rather than a brittle list, plus credential- or PII-shaped values anywhere (tid_/tsec_ keys, JWTs, AKIA ids, Bearer tokens, emails including inside object keys/paths). Also set a searchable top-level command tag and an exit_code tag on both the handled and crash paths. Assisted-by: Opus 4.8 via Claude Code

A non-sensitive `--flag=value` token fell through to the space-form branch, which tested the whole token against the sensitive-flag regex. A sensitive substring in the value then pushed the token verbatim (bypassing redactSecrets, leaking a secret value like `--note=tsec_...`) and redacted the next positional instead. Handle `--flag=value` entirely up front and always continue: redact the value for a sensitive flag name, otherwise scrub the value through redactSecrets. The space-form branch now only sees bare flags. Adds a regression test covering both the leak and the mis-redaction. Assisted-by: Opus 4.8 via Claude Code

SENSITIVE_FLAG_RE matched access-key/api-key but not the CLI's documented `--key` alias for --access-key, so an access-key passed as `--key <value>` only got redacted when the value already looked like tid_/AKIA/JWT — a custom key could reach the Sentry invocation context. Match flag names ending in `key` (covers --key, --access-key, --secret-key) without matching non-secret flags like --key-marker. Object keys are positional args, so they never hit this flag check. Adds tests for both. Assisted-by: Opus 4.8 via Claude Code

beforeSend only redacted exception values, the message, and breadcrumb messages, leaving structured fields (breadcrumb `data` such as request URLs, and anything default integrations attach) unscrubbed. Walk the whole event and run redactSecrets on every string leaf, preserving non-string values. Also route the beforeSend tests through a typed helper so they type-check outside the DOM-lib inferred project. Assisted-by: Opus 4.8 via Claude Code

reportCrashAndExit runs async (to await the telemetry flush) and is fired with void from the global handlers. If exitWithError throws before process.exit — e.g. console.error hitting EPIPE on a closed stderr — the rejection re-enters via unhandledRejection and loops forever. Add a re-entrancy guard that exits hard on the second entry. Assisted-by: Opus 4.8 via Claude Code

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b5c611d. Configure here.}

scrubArgv only matched long flag names, so the webhook --token short alias -t was never treated as sensitive; `-t <token>` / `-t=<token>` skipped redaction, and arbitrary webhook tokens also miss the secret-shape patterns, so they could reach the Sentry invocation context. Add a SENSITIVE_SHORT_FLAGS set (-t) checked alongside the name regex via a new isSensitiveFlag helper. Adds tests for both forms. Assisted-by: Opus 4.8 via Claude Code

designcode added 2 commits July 20, 2026 13:57

cursor Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread packages/cli/src/utils/telemetry.ts

greptile-apps Bot reviewed Jul 20, 2026

View reviewed changes