feat: add SWAMP_NO_TELEMETRY environment variable to disable telemetry#376
Conversation
Implements the SWAMP_NO_TELEMETRY environment variable mechanism proposed in issue #362. PR #364 added .swamp.yaml telemetryDisabled: true but left out the env var approach, which is the missing piece for CI/UAT environments where editing .swamp.yaml per-run isn't practical. ### Changes src/cli/mod.ts - Added isTelemetryDisabledByEnv() (exported for testing) after isTelemetryDisabledByConfig(), following the same pattern as resolveModelsDir() which already reads SWAMP_MODELS_DIR - Updated runCli() to OR the env var check with the existing flag check: isTelemetryDisabled(args) || isTelemetryDisabledByEnv() src/cli/mod_test.ts - Added 6 tests for isTelemetryDisabledByEnv() covering: not set, "1", "true" (disabled); "0", "false", "" (enabled/explicit opt-in) - Updated import line to include isTelemetryDisabledByEnv README.md - Extended the "Disabling Telemetry" section with the env var usage example and the full priority order ### Precedence (highest to lowest) 1. --no-telemetry CLI flag 2. SWAMP_NO_TELEMETRY env var ← this PR 3. .swamp.yaml telemetryDisabled: true 4. Default (telemetry enabled) The flag and env var checks are evaluated before initTelemetryService() is called, so they short-circuit the config file read entirely. ### Plan vs Implementation The implementation follows the plan exactly with one minor cosmetic difference: deno fmt reflowed the priority sentence in README.md to fit within the line limit. No functional deviation from the plan. ### Verification - deno check — passes - deno lint — passes - deno fmt — passes - deno run test src/cli/mod_test.ts — 15/15 tests pass
There was a problem hiding this comment.
Review Summary
This PR correctly implements the SWAMP_NO_TELEMETRY environment variable feature for disabling telemetry in CI/UAT environments. The implementation is clean and follows the project's established patterns.
No blocking issues found.
What Was Reviewed
Code Quality & TypeScript Strict Mode ✅
- No
anytypes used - Proper type inference with
string | undefinedhandling - Named exports used (not default exports)
Code Style (CLAUDE.md) ✅
- AGPLv3 copyright header present in modified files
- Uses
@std/assertfor test assertions - Tests live next to source file (
mod.ts→mod_test.ts)
Domain-Driven Design ✅
- The
isTelemetryDisabledByEnv()function is correctly placed in the CLI module as application-layer infrastructure - It's a simple configuration check, not domain logic - appropriately not a domain service
- Follows the same pattern as existing
resolveModelsDir()which also reads environment variables
Test Coverage ✅
- 6 comprehensive tests covering all edge cases:
- Env var not set → telemetry enabled
"1"→ telemetry disabled"true"→ telemetry disabled"0"→ telemetry enabled"false"→ telemetry enabled""(empty string) → telemetry enabled
- Tests properly save/restore environment variables in finally blocks
Security ✅
- No security vulnerabilities introduced
- Environment variable handling is safe (just string comparison)
Implementation Notes
- The precedence order (CLI flag → env var → config file) is correctly documented and implemented
- The env var check short-circuits before
initTelemetryService(), avoiding unnecessary file reads - The function follows the existing pattern of
isTelemetryDisabledByConfig()
Suggestions (Non-blocking)
-
Minor inconsistency in test cleanup pattern: The first test (
isTelemetryDisabledByEnv returns false when env var is not set) doesn't restore the env var in the finally block if it was originally undefined (it only restores if it was set). This differs from the other tests that properly callDeno.env.delete()in the else branch. While this works correctly in practice since the test deletes the var, the other tests' pattern is more defensive. -
Case sensitivity: The implementation treats
"TRUE"or"False"(mixed case) as disabling telemetry since only exact matches to"0","false", and""enable it. This is reasonable behavior but could be documented in the JSDoc comment if the behavior is intentional.
Both suggestions are very minor and don't warrant blocking the merge.
Fixes: #375
Implements the SWAMP_NO_TELEMETRY environment variable mechanism proposed in issue #362. PR #364 added
.swamp.yaml telemetryDisabled: true but left out the env var approach, which is the missing piece for CI/UAT
environments where editing .swamp.yaml per-run isn't practical.
Changes
src/cli/mod.ts
same pattern as resolveModelsDir() which already reads SWAMP_MODELS_DIR
isTelemetryDisabledByEnv()
src/cli/mod_test.ts
(enabled/explicit opt-in)
README.md
Precedence (highest to lowest)
The flag and env var checks are evaluated before initTelemetryService() is called, so they short-circuit the
config file read entirely.
Plan vs Implementation
The implementation follows the plan exactly with one minor cosmetic difference: deno fmt reflowed the priority
sentence in README.md to fit within the line limit. No functional deviation from the plan.
Verification