Skip to content

feat(tracking): honor DO_NOT_TRACK and COMFY_NO_TELEMETRY env vars#463

Merged
bigcat88 merged 1 commit into
mainfrom
alexander/telemetry-env-opt-out
May 21, 2026
Merged

feat(tracking): honor DO_NOT_TRACK and COMFY_NO_TELEMETRY env vars#463
bigcat88 merged 1 commit into
mainfrom
alexander/telemetry-env-opt-out

Conversation

@bigcat88

Copy link
Copy Markdown
Contributor

@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. enhancement New feature or request labels May 21, 2026
@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 21b8d4de-05c1-4f0d-bb57-8aa8c1fd1d2a

📥 Commits

Reviewing files that changed from the base of the PR and between 5b014ff and 2c73b8d.

📒 Files selected for processing (2)
  • comfy_cli/tracking.py
  • tests/comfy_cli/test_tracking.py

📝 Walkthrough

Walkthrough

Adds an env-var gate: _telemetry_disabled_by_env() treats non-empty (except "0") DO_NOT_TRACK or COMFY_NO_TELEMETRY as opt-out. When set, track_event() and prompt_tracking_consent() return early and skip telemetry emission and consent flows. Tests exercise both behavior and helper semantics — silent but effective, like a rhyme that mutes a chime.

Changes

Environment Variable Telemetry Opt-Out

Layer / File(s) Summary
Environment variable opt-out gate implementation
comfy_cli/tracking.py
_telemetry_disabled_by_env() checks DO_NOT_TRACK and COMFY_NO_TELEMETRY (range_e4ae46523a26); track_event() early-returns when disabled (range_b6347ab00097); prompt_tracking_consent() skips prompting and session logic when disabled (range_2f7ed0acc3b1).
Environment variable opt-out test coverage
tests/comfy_cli/test_tracking.py
Adds TestEnvVarOptOut and helper tests validating opt-out semantics and edge cases (range_b81994129a08) and unit tests for _telemetry_disabled_by_env() value semantics and single-var sufficiency (range_e500c8f486fe).
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch alexander/telemetry-env-opt-out
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch alexander/telemetry-env-opt-out

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #463      +/-   ##
==========================================
+ Coverage   83.22%   83.25%   +0.02%     
==========================================
  Files          45       45              
  Lines        6791     6801      +10     
==========================================
+ Hits         5652     5662      +10     
  Misses       1139     1139              
Files with missing lines Coverage Δ
comfy_cli/tracking.py 94.03% <100.00%> (+0.42%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Short-circuits both the consent prompt and the per-event dispatch when
either variable is set to a non-empty, non-zero value, per the
consoledonottrack.com spec. Suppresses the non-TTY auto-enable path so
no anonymous user_id is persisted before consent.

Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
@bigcat88
bigcat88 force-pushed the alexander/telemetry-env-opt-out branch from 5b014ff to 2c73b8d Compare May 21, 2026 19:08
@bigcat88
bigcat88 merged commit 30ba66f into main May 21, 2026
15 checks passed
@bigcat88
bigcat88 deleted the alexander/telemetry-env-opt-out branch May 21, 2026 19:36
skishore23 added a commit that referenced this pull request Jun 11, 2026
…nto the run/ package + single-client architecture

Upstream (dffa5c1..a392f55) carried five telemetry/tracking commits; most
of #461/#463 was already on this branch, so the merge mainly ports #465 and
#468 into our architecture:

- PostHog dual-send + lifecycle events (#461): already present in our
  tracking.py (_dispatch fan-out) and cmdline.py run lifecycle; conflicts
  resolved to our superset (keeps _ensure_user_id(persist=), submit_feedback,
  submit_agent_review, _consent_enabled).
- DO_NOT_TRACK / COMFY_NO_TELEMETRY (#463): already present; kept ours.
- cli: event prefix (#465): took upstream's POSTHOG_EVENT_PREFIX in
  tracking.py PostHogProvider.capture + upstream's prefix tests in
  tests/comfy_cli/test_tracking_providers.py.
- Comfy-Usage-Source header (#468): run.py was deleted here, so the header +
  extra_data.comfy_usage_source landed in comfy_cli/command/run/execution.py
  (WorkflowExecution.queue) and comfy_cli/comfy_client.py (Client._request
  header on every local/cloud request, submit_prompt extra_data);
  generate/client.py auto-merged upstream's header. Adapted
  tests/comfy_cli/cloud/test_client.py extra_data assertions accordingly.
- SOC2 unreviewed-merge detector (#464) + pytest.yml codecov bump: taken
  from upstream as-is.
- uv.lock: took upstream then regenerated with `uv lock` (posthog already a
  dependency here).
- Upstream's non-TTY auto-consent assertion in test_tracking.py kept at our
  deliberate behavior (non-interactive sessions do NOT auto-enable tracking).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant