Skip to content

fix(docs): restore telemetry and root pre-run for docs commands - #1982

Merged
hngo-stripe merged 1 commit into
masterfrom
fix/docs-cmd-telemetry-persistent-prerun
Sep 2, 2026
Merged

fix(docs): restore telemetry and root pre-run for docs commands#1982
hngo-stripe merged 1 commit into
masterfrom
fix/docs-cmd-telemetry-persistent-prerun

Conversation

@tomer-stripe

@tomer-stripe tomer-stripe commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Problem

stripe docs declares its own PersistentPreRunE (pkg/cmd/docs/root.go) — a shape it inherited from the docs plugin it was inlined from in v1.43.3 (#1663). In a standalone plugin binary that's correct: there's no CLI root above you, so nothing is shadowed.

Inside the CLI it is not. Cobra runs only the closest PersistentPreRun hook in the chain (EnableTraverseRunHooks is off), so that declaration shadows rootCmd.PersistentPreRunE and silently disables, for every command under stripe docs:

  • command-invocation telemetry and all of its metadata (merchant, command path, user agent, machine UUID, flags)
  • config file migration
  • --access-base validation, plus the OAuth access base URL that the token refresher inside ResolveCredentials relies on
  • Sentry command context via reporting.SetCommandPath (bears on Stop Sentry noise from rate-limit errors and fix uninformative issue titles #1970)
  • the legacy profile name warning

stripe docs is currently the only command in the CLI that declares a pre-run hook, which is why it's the only one affected.

Reproducing

Only the root hook rejects a non-Stripe --access-base, so it makes a clean probe:

$ stripe config --list --access-base https://evil.example.com
invalid access base URL: must be exactly https://access.stripe.com or https://qa-access.stripe.com

$ stripe docs prefs list --access-base https://evil.example.com
• api-version
  Your preferred API version for viewing version-specific content...

Rejected for config, silently accepted for docs.

Fix

Normalize the docs tree to match every other command in the CLI, none of which declares a pre-run hook. The setup moves out of PersistentPreRunE and into RunE via a withSetup wrapper — the same place runListenCmd does its post-flag --api-base validation. That leaves the PersistentPreRun slot to the root command, so docs gets telemetry for free like everything else.

Setup still runs after flag parsing and before the command body, so behavior is unchanged; it simply no longer occupies a slot it was never entitled to. The root hook now also runs before docs resolves credentials, which is the correct order — it is what validates and publishes the access base URL the OAuth refresher uses.

Considered and rejected: cobra.EnableTraverseRunHooks = true (fixes the class, but changes hook semantics for every command to fix one), and having the docs hook reach up to invoke its ancestor's (contained, but inverted control flow that the next inlined plugin has to remember to repeat).

Scope

This restores Command Invoked events. Docs page fetches still emit no API Request events, because pkg/docs/client.go builds its own http.Client instead of using the instrumented pkg/stripe client, so SendAPIRequestEvent is never reached. That's a separate change.

Testing

  • Added TestDocsRunsRootPersistentPreRun, which uses the --access-base probe through a nested subcommand (docs prefs list) so the hook is reached through more than one parent. Verified it fails on master and passes with this change.
  • go test ./pkg/cmd/ ./pkg/cmd/docs/... passes, except TestSearchCommand, which fails identically on master (unrelated, pre-existing, and not agent-env dependent).
  • Manually confirmed the probe is now rejected for docs, and that stripe docs and stripe docs prefs list still work normally.

🤖 Generated with Claude Code

`stripe docs` declared its own PersistentPreRunE, a shape it inherited from
the docs plugin it was inlined from in v1.43.3. Cobra runs only the closest
PersistentPreRun hook in the chain, so that declaration shadowed the root
command's hook and silently disabled, for every command under `stripe docs`:

- command-invocation telemetry and all of its metadata (merchant, command
  path, user agent, machine UUID, flags)
- config file migration
- --access-base validation, and the OAuth access base URL that the token
  refresher inside ResolveCredentials depends on
- Sentry command context via reporting.SetCommandPath
- the legacy profile name warning

The telemetry gap is measurable: of 14,266 `stripe docs` telemetry events in
a recent 7-day window, 14,264 came from pre-1.43.3 builds and none from any
release at or after 1.43.3, while `/v2/docs/page` traffic from 1.45+ clients
grew steadily and went entirely unrecorded.

Normalize the tree to match every other command in the CLI, none of which
declares a pre-run hook: move the docs setup out of PersistentPreRunE and
into RunE through a withSetup wrapper, the same place runListenCmd does its
post-flag validation. This leaves the PersistentPreRun slot to the root
command, so docs gets telemetry for free like everything else.

Note this restores `Command Invoked` events only. Docs page fetches still
emit no `API Request` events, because pkg/docs/client.go uses its own
http.Client rather than the instrumented pkg/stripe client. That is a
separate change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Committed-By-Agent: claude
@tomer-stripe
tomer-stripe requested review from a team as code owners September 1, 2026 22:25
@hngo-stripe
hngo-stripe added this pull request to the merge queue Sep 2, 2026
Merged via the queue into master with commit 6056ace Sep 2, 2026
14 checks passed
@hngo-stripe
hngo-stripe deleted the fix/docs-cmd-telemetry-persistent-prerun branch September 2, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants