Skip to content

fix(posthog): make OMO_DISABLE_POSTHOG env parsing case-insensitive and trim-tolerant (refs #4368)#4485

Merged
code-yeongyu merged 1 commit into
code-yeongyu:devfrom
MoerAI:fix/posthog-env-permissive-parsing
Jun 2, 2026
Merged

fix(posthog): make OMO_DISABLE_POSTHOG env parsing case-insensitive and trim-tolerant (refs #4368)#4485
code-yeongyu merged 1 commit into
code-yeongyu:devfrom
MoerAI:fix/posthog-env-permissive-parsing

Conversation

@MoerAI
Copy link
Copy Markdown
Collaborator

@MoerAI MoerAI commented May 26, 2026

Summary

Hardens telemetry env-var parsing so case-variant and whitespace-padded values are honored. The behavior change is strictly additive: every value already disabled today (true, 1, 0, false, no) is still disabled; only previously-ignored variants (TRUE, Yes, 1, true, etc.) are now respected.

Scope clarification (re: #4368)

Issue #4368 reports OMO_DISABLE_POSTHOG=1 not disabling telemetry. The pre-fix code already strictly matched === "1", so this PR is NOT a guaranteed fix for that specific reproduction — the user's reported value already worked under the old code. A full codebase audit (every new PostHog(...), every capture(), every trackActive(), every CLI entrypoint) confirmed there is no telemetry capture path that bypasses shouldDisablePostHog(), so the residual cause in #4368 is most likely environment-side (env var not propagated to the spawned bun process, or a broken plugin install — the oh-my-openagent unknown in their doctor output is consistent with a missing ~/.cache/opencode/node_modules/oh-my-openagent).

Keeping Refs #4368 rather than Fixes #4368 so the issue stays open until the reporter's actual root cause is identified. The robustness improvement here closes the door on a related class of failures (case/whitespace mismatch on shell/CI env vars) that other users will eventually hit.

Root Cause

shouldDisablePostHog() in src/shared/posthog.ts previously compared OMO_DISABLE_POSTHOG against the strict string literals "true" and "1". Shell environments and config tooling frequently emit case-variant or padded values such as TRUE, True, Yes, 1, or true; those values silently fell through to the default "capture telemetry" branch, even though the user clearly intended to opt out.

The companion var OMO_SEND_ANONYMOUS_TELEMETRY was already normalized via .trim().toLowerCase() before isFalsy() — the disable path just never received the same treatment.

Changes

File Change
src/shared/posthog.ts Add symmetric isTruthy() helper; normalize OMO_DISABLE_POSTHOG with .trim().toLowerCase() before matching (mirror of the existing OMO_SEND_ANONYMOUS_TELEMETRY path)
src/shared/posthog.test.ts 12 new given/when/then cases covering case-variant + whitespace inputs for both env vars

Diff: +67 / −1 across 2 files. fix() only — no behavior change for values already handled (true, 1, 0, false, no).

Reproduction (before fix)

(fail) posthog disable env var parsing > treats OMO_DISABLE_POSTHOG="TRUE" as disabled
(fail) posthog disable env var parsing > treats OMO_DISABLE_POSTHOG="True" as disabled
(fail) posthog disable env var parsing > treats OMO_DISABLE_POSTHOG="Yes" as disabled
(fail) posthog disable env var parsing > treats OMO_DISABLE_POSTHOG="YES" as disabled
(fail) posthog disable env var parsing > treats OMO_DISABLE_POSTHOG=" 1 " as disabled
(fail) posthog disable env var parsing > treats OMO_DISABLE_POSTHOG=" true " as disabled

 10 pass
 6 fail
 32 expect() calls

Verification (after fix)

bun test src/shared/posthog.test.ts
 16 pass
 0 fail
 32 expect() calls
Ran 16 tests across 1 file. [438.00ms]

Test

  • Regression test: src/shared/posthog.test.ts (12 new cases under describe("posthog disable env var parsing"))
  • Related telemetry suite: bun test src/shared/posthog{,-activity-state}.test.ts src/cli/cli-installer.telemetry.test.ts src/cli/run/runner.telemetry.test.ts src/index.telemetry.test.ts — 25 pass
  • Typecheck: bun run typecheck — clean

Refs #4368

…nd trim-tolerant (fixes code-yeongyu#4368)

Users following the documented opt-out (OMO_DISABLE_POSTHOG=1 / OMO_SEND_ANONYMOUS_TELEMETRY=0) report telemetry still firing on some setups.

Root cause: shouldDisablePostHog() compared OMO_DISABLE_POSTHOG with the strict string literals 'true' and '1', so case-variant or whitespace-padded values such as 'TRUE', 'True', 'Yes', ' 1 ', ' true ' were silently treated as 'do not disable'. Shell environments and config files commonly emit such values.

Fix: normalize the env value with .trim().toLowerCase() before matching (mirroring how OMO_SEND_ANONYMOUS_TELEMETRY is already handled), and accept 'true' / '1' / 'yes' as truthy via a new helper isTruthy() that is symmetric with the existing isFalsy().

Verified by 12 new regression tests in posthog.test.ts covering case-variants and surrounding whitespace for both env vars; all 16 posthog.test.ts cases plus the 25 telemetry-related tests across cli-installer/run/index pass.
@MoerAI
Copy link
Copy Markdown
Collaborator Author

MoerAI commented May 26, 2026

I have read the CLA Document and I hereby sign the CLA

@MoerAI MoerAI changed the title fix(posthog): make OMO_DISABLE_POSTHOG env parsing case-insensitive and trim-tolerant (fixes #4368) fix(posthog): make OMO_DISABLE_POSTHOG env parsing case-insensitive and trim-tolerant (refs #4368) May 27, 2026
Copy link
Copy Markdown
Owner

@code-yeongyu code-yeongyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[sisyphus-dev] Reviewed and QA verified. The exact #4368 env values are already honored on current dev, so this is not a guaranteed root-cause fix for that report, but the PR is a clean narrow hardening change: it normalizes case/whitespace for opt-out values without adding config surface or changing telemetry payloads. Local QA on a merge with current dev: telemetry regression suite passed, typecheck passed, build passed.

@code-yeongyu code-yeongyu merged commit 0286a68 into code-yeongyu:dev Jun 2, 2026
8 checks passed
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