Skip to content

fix(diagnostics): never send diagnostics from test environments#34

Merged
naji247 merged 1 commit into
mainfrom
fix/disable-diagnostics-in-test-envs
Jul 2, 2026
Merged

fix(diagnostics): never send diagnostics from test environments#34
naji247 merged 1 commit into
mainfrom
fix/disable-diagnostics-in-test-envs

Conversation

@naji247

@naji247 naji247 commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

MCPCat's internal SDK diagnostics exist to catch broken installs — they should never fire from a test run, neither ours nor yours. Before this change, diagnostics were enabled by default with no test-environment awareness, so any test suite that called track() could ship real OTLP metadata (host/OS/runtime info) to the diagnostics collector. This PR makes staying silent in tests the default, enforced at two layers:

  • SDK-level guard (protects every consumer): diagnostics now auto-disable whenever a test environment is detected — PYTEST_CURRENT_TEST or PYTEST_VERSION set. No configuration needed; your CI sends nothing. Anyone who genuinely needs diagnostics active in a test context can opt back in explicitly with DISABLE_DIAGNOSTICS=false.
  • Repo-level guard (protects this repo's CI): a top-level tests/conftest.py sets DISABLE_DIAGNOSTICS=true before any test runs, so no test ordering or future change to the detection logic can leak traffic from our own suite. Diagnostics-specific tests opt back in per-file with mocked HTTP, keeping them network-safe.

Also tightens DISABLE_DIAGNOSTICS parsing: explicit falsy values (false/0/no/off) are now a deliberate opt-in that overrides test detection, and whitespace-only values are treated as unset. The disable_diagnostics docstring documents the new behavior.

Counterpart of the TypeScript fix: agentcathq/agentcat-typescript-sdk#44.

Test plan

  • Regression test written first, confirmed failing (RED), then passing (GREEN). tests/test_diagnostics_test_env.py::test_track_does_not_enable_diagnostics_in_pytest — before the SDK guard, track() under pytest latched diagnostics on (assert True is False); after the guard it stays disabled and flush_diagnostics() posts nothing.
  • New opt-out cases per detection signal (tests/test_diagnostics_optout.py): enabled by default in a simulated non-test env; disabled by default when PYTEST_CURRENT_TEST set; disabled by default when PYTEST_VERSION set; DISABLE_DIAGNOSTICS=false force-enables even under pytest; whitespace-only treated as unset; falsy values force-enable.
  • Full suite: uv run pytest → 479 passed, 4 skipped, 3 xfailed.
  • Dead-endpoint run: DIAGNOSTICS_ENDPOINT=http://127.0.0.1:9 uv run pytest → 479 passed, 4 skipped, 3 xfailed (no hangs, no leaked traffic).
  • Lint: uv run ruff check . — no new findings (new files clean; pre-existing repo findings unchanged).
  • Types: uv run mypy src/mcpcat --strict — error count unchanged from baseline (130), no new errors introduced.

Internal SDK diagnostics were enabled by default with no test-environment
awareness, so any test suite that called track() could ship real OTLP
metadata (host/OS/runtime info) to the diagnostics collector. Staying silent
in tests is now the default, enforced at two layers:

- SDK-level guard (protects every consumer): diagnostics auto-disable whenever
  a test environment is detected (PYTEST_CURRENT_TEST or PYTEST_VERSION set).
  No configuration needed; consumer CI sends nothing.
- Repo-level guard (protects this repo's CI): a top-level tests/conftest.py
  sets DISABLE_DIAGNOSTICS=true before any test runs, so no test ordering or
  future change to the detection logic can leak traffic from our own suite.

DISABLE_DIAGNOSTICS parsing is tightened: explicit falsy values
(false/0/no/off) are now a deliberate opt-in that overrides test detection
(the escape hatch to re-enable diagnostics under pytest), and whitespace-only
values are treated as unset. Diagnostics-specific tests opt back in per-file
with mocked HTTP. README and the disable_diagnostics docstring document the
new behavior.

Counterpart of the TypeScript fix: agentcathq/agentcat-typescript-sdk#44.
@naji247
naji247 force-pushed the fix/disable-diagnostics-in-test-envs branch from 5188b67 to 2e6c402 Compare July 2, 2026 19:50
@naji247
naji247 merged commit 1132377 into main Jul 2, 2026
43 checks passed
@naji247
naji247 deleted the fix/disable-diagnostics-in-test-envs branch July 2, 2026 20: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.

2 participants