Skip to content

fix(cli-repl): correctly return false from enableTelemetry setting when forceDisableTelemetry is set MONGOSH-1141 - #2770

Merged
nbbeeken merged 2 commits into
mainfrom
MONGOSH-1141
Jul 16, 2026
Merged

fix(cli-repl): correctly return false from enableTelemetry setting when forceDisableTelemetry is set MONGOSH-1141#2770
nbbeeken merged 2 commits into
mainfrom
MONGOSH-1141

Conversation

@nbbeeken

Copy link
Copy Markdown
Collaborator

When forceDisableTelemetry is set don't consult the user preferences just return false. Fixes misleading state.

Copilot AI review requested due to automatic review settings July 13, 2026 19:38
@nbbeeken
nbbeeken requested a review from a team as a code owner July 13, 2026 19:38
@nbbeeken
nbbeeken requested a review from addaleax July 13, 2026 19:38
@nbbeeken
nbbeeken requested review from alenakhineika and removed request for addaleax July 13, 2026 19:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the CLI REPL configuration behavior so that when forceDisableTelemetry is in effect, querying enableTelemetry reflects the enforced runtime state (i.e., returns false) rather than the user preference value, preventing misleading configuration output.

Changes:

  • Special-case getConfig('enableTelemetry') to return false when forceDisableTelemetry is set.
  • Refactor the setConfig('enableTelemetry', ...) guard to run before mutating config state (behavior preserved, but clearer control flow).
  • Add a regression test ensuring config.get("enableTelemetry") prints false under a global forceDisableTelemetry: true config.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/cli-repl/src/cli-repl.ts Ensures enableTelemetry reads as false when telemetry is force-disabled; keeps writes blocked.
packages/cli-repl/src/cli-repl.spec.ts Adds coverage verifying config.get("enableTelemetry") returns false under forced-disable telemetry.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alenakhineika

Copy link
Copy Markdown
Collaborator

Can we leverage your PR to clean up this? :) https://github.com/mongodb-js/mongosh/blob/main/packages/cli-repl/src/cli-repl.ts#L221-L224 or crate a follow-up PR/ticket to not forget again.

Would be great to introduce a single isTelemetryEnabled() helper that checks forceDisableTelemetry and enableTelemetry config value, and have all callers defer to it. We have these checks in multiple places already, e.g. the fetch closure, setTelemetryEnabled, enableTelemetry, maybe more. Combining both would give us the single source of truth.

@nbbeeken

Copy link
Copy Markdown
Collaborator Author

Yea that's fair, I'll investigate what that would take

@nbbeeken
nbbeeken requested a review from alenakhineika July 15, 2026 20:12
@nbbeeken

Copy link
Copy Markdown
Collaborator Author

Because of async-ness I had to change some functions return type, what do we think? @alenakhineika

@alenakhineika alenakhineika left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is great 🙌 Thx!

@nbbeeken
nbbeeken merged commit 8633941 into main Jul 16, 2026
151 of 155 checks passed
@nbbeeken
nbbeeken deleted the MONGOSH-1141 branch July 16, 2026 20:04
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.

3 participants