fix(init): honor RTK_TELEMETRY_DISABLED in consent prompt (#1307)#2477
Open
ousamabenyounes wants to merge 2 commits into
Open
fix(init): honor RTK_TELEMETRY_DISABLED in consent prompt (#1307)#2477ousamabenyounes wants to merge 2 commits into
ousamabenyounes wants to merge 2 commits into
Conversation
`rtk init -g --hook-only --auto-patch` hangs in non-interactive environments (devcontainer postCreateCommand, some CI agents) when `is_terminal()` returns true on a pseudo-TTY that nobody can answer. Setting `RTK_TELEMETRY_DISABLED=1` used to only gate `telemetry::maybe_ping`, so the documented workaround still hit the blocking prompt. Short-circuit `prompt_telemetry_consent` on that env var before the TTY heuristic so the init flow exits cleanly in any non-interactive context. Fixes rtk-ai#1307
Address KuSh review on rtk-ai#2477: export the `RTK_TELEMETRY_DISABLED` opt-out check as a public `telemetry_cmd::telemetry_disabled_by_env()` instead of a private copy in `init.rs`. Collapses the three duplicated `std::env::var("RTK_TELEMETRY_DISABLED") == "1"` checks (init prompt, `telemetry status`, `telemetry::maybe_ping`) into one function, so the accepted values can evolve (e.g. "true"/"y") in a single place without divergence. Behavior-preserving: the rtk-ai#1307 regression test (env opt-out short-circuits the consent prompt) is unchanged and still green; suite 2200/2200. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
KuSh
requested changes
Jun 18, 2026
| /// All cases are bundled in one test to serialize env-var mutations (env is | ||
| /// process-global and cargo runs tests in parallel). | ||
| #[test] | ||
| fn test_telemetry_disabled_by_env_honors_opt_out() { |
Collaborator
There was a problem hiding this comment.
Those tests should go to telemetry_cmd. After that LGTM!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1307.
rtk init's telemetry consent prompt only bailed on a non-TTY check. Some non-interactive environments (devcontainerpostCreateCommand, certain CI agents) hand rtk a pseudo-TTY, sois_terminal()returns true and the prompt blocks forever waiting on stdin.RTK_TELEMETRY_DISABLED=1is the documented opt-out and is honoured bytelemetry::maybe_ping, but the consent prompt ignored it.Fix
Add a
telemetry_disabled_by_env()helper and short-circuitprompt_telemetry_consent()withreturn Ok(())whenRTK_TELEMETRY_DISABLED=1, before the TTY heuristic — mirroring the check already used inmaybe_ping.Test verification (RED → GREEN)
The hang itself only reproduces under a real pseudo-TTY, which the test harness can't provide; the regression is guarded by a unit test on the new
telemetry_disabled_by_env()helper that the fix is built on.RED — patch reverted (helper removed):
GREEN — with the fix:
(Re-proposes the accidentally-closed #1369, rebased onto current
develop.)