fix(config): warn when common-config disables Claude Monitor tool - #418
Open
SomSamantray wants to merge 4 commits into
Open
fix(config): warn when common-config disables Claude Monitor tool#418SomSamantray wants to merge 4 commits into
SomSamantray wants to merge 4 commits into
Conversation
Setting CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC in a Claude common-config snippet silently disables Claude Code's Monitor tool. Detect it in the CLI (config common set / extract --save) and TUI common-config editor and surface a non-blocking warning instead of applying it silently. Fixes SaladDay#184
The documented CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC snippet disables Claude Code's Monitor tool. Add an inline comment and a callout next to the example in both READMEs so copying it isn't a silent surprise. Related to SaladDay#184
Both call sites repeated the same "print success, then conditionally print the warning" block. Extract print_monitor_traffic_warning_if_disabled to remove the duplication.
…erwrite Code review findings: - AGENTS.md/CLAUDE.md require durable logic shared between the CLI and TUI to live in src/services/, not src/cli/commands/. Move common_config_snippet_disables_monitor_traffic onto ProviderService and have both the CLI and TUI call it from there. - The TUI save handler could still silently drop the Monitor-traffic warning: a later Codex-form-reconciliation-error toast could overwrite the just-pushed warning toast, since there is no toast queue. Defer the final toast decision until after reconciliation runs, and always fold the warning into whichever toast ends up being shown.
Owner
|
Thanks — this is a useful fix, but it is not ready to merge yet:
Please fix these issues and add assertions for the actual CLI warning output ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Setting
CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFICin a Claude common-config snippet silently disabled Claude Code's Monitor tool — including for anyone who copied the exact example the README recommends. Neither the CLI nor the TUI said anything about it.config common set,config common extract --save, and the TUI's common-config editor now print (or toast) a non-blocking warning when the resulting snippet would disable Monitor traffic for Claude; every other app type and every non-truthy value is unaffected. Both READMEs now flag the same side effect next to the documented example.Added unit tests for the detection helper (truthy/falsy/absent-key/non-Claude cases) and behavioral tests for the CLI and TUI paths, including a regression test for a toast-ordering edge case caught during review (the TUI's single-toast state could otherwise let a later reconciliation-error toast silently overwrite the warning).
cargo testpasses for the touched modules.Fixes #184