fix(init): opt-out for project-local editor files - #1836
Conversation
…e-identical repowise init wrote the repo-shared, tracked .mcp.json by re-serialising the whole document from a dict, so pre-existing unrelated server entries (e.g. a SonarQube entry with compact ["-e", "VAR"] args) came back reformatted on every run. Only the repowise entry is touched now: the new merge_json_object_member performs a surgical, indentation-aware edit that leaves every other byte of the file untouched, while still preserving user-added keys (an env block) on the repowise entry and staying idempotent across re-runs. Closes repowise-dev#1603.
|
✅ Health of changed files: 5.5 (unchanged) 📋 At a glance Files & modules (2)
✅ Health gate: passed 📌 Before you merge
🔎 More signals (2)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (3 with dependents)"]
f_packages_cli_src_repowise_cli_agent_targets_formats_json_merge_py[".../formats/json_merge.py 🔥"]:::changed
f_packages_cli_src_repowise_cli_agent_targets_targets_claude_code_py[".../targets/claude_code.py 🔥"]:::changed
f_tests_unit_cli_test_agent_targets_py[".../cli/test_agent_targets.py"]:::changed
end
f_packages_cli_src_repowise_cli_agent_targets_formats_marker_block_py[".../formats/marker_block.py"]
f_packages_cli_src_repowise_cli_agent_targets_formats_json_merge_py --> f_packages_cli_src_repowise_cli_agent_targets_formats_marker_block_py
f_packages_cli_src_repowise_cli_agent_targets_formats_toml_merge_py[".../formats/toml_merge.py"]
f_packages_cli_src_repowise_cli_agent_targets_formats_json_merge_py --> f_packages_cli_src_repowise_cli_agent_targets_formats_toml_merge_py
f_packages_cli_src_repowise_cli_agent_targets_formats_yaml_merge_py[".../formats/yaml_merge.py"]
f_packages_cli_src_repowise_cli_agent_targets_formats_json_merge_py --> f_packages_cli_src_repowise_cli_agent_targets_formats_yaml_merge_py
f_packages_cli_src_repowise_cli_agent_targets_targets_codex_py[".../targets/codex.py"]
f_packages_cli_src_repowise_cli_agent_targets_formats_json_merge_py --> f_packages_cli_src_repowise_cli_agent_targets_targets_codex_py
f_packages_cli_src_repowise_cli_editor_integrations_claude_py[".../editor_integrations/claude.py"]
f_packages_cli_src_repowise_cli_agent_targets_targets_claude_code_py --> f_packages_cli_src_repowise_cli_editor_integrations_claude_py
f_packages_cli_src_repowise_cli_mcp_config_py[".../cli/mcp_config.py"]
f_packages_cli_src_repowise_cli_agent_targets_targets_claude_code_py --> f_packages_cli_src_repowise_cli_mcp_config_py
f_packages_cli_src_repowise_cli_uninstall_inventory_py[".../uninstall/inventory.py"]
f_packages_cli_src_repowise_cli_agent_targets_targets_claude_code_py --> f_packages_cli_src_repowise_cli_uninstall_inventory_py
f_tests_unit_cli_test_agent_targets_py --> f_packages_cli_src_repowise_cli_agent_targets_formats_toml_merge_py
more(["+3 more dependents"])
PR --> more
t_tests_unit_cli_test_agent_drift_control_py(["✅ .../cli/test_agent_drift_control.py"]):::guard
t_tests_unit_cli_test_agent_drift_control_py -.-> f_packages_cli_src_repowise_cli_agent_targets_targets_claude_code_py
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (10 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (3)
👀 Suggested reviewers @RaghavChamadiya 📊 See the full report for this PR |
Ayush7614
left a comment
There was a problem hiding this comment.
Verified: packages/cli/src/repowise/cli/agent_targets/formats/json_merge.py:221 adds surgical merge_json_object_member that upserts only mcpServers.repowise preserving unrelated entries byte-identical, with env preservation and idempotency, fixing #1603 reformatting bug. claude_code.py now uses it. Tests cover byte-identical, idempotent, env. Solid minimal-edit fix — LGTM.
|
Thanks @sloemo01. The minimal-edit approach is the right answer to the reformat half of #1603. Re-serialising a tracked, repo-shared file end to end so that one contributor's Careful detail worth calling out: returning The caller can destroy a user's file. In action = merge_json_object_member(config_path, "mcpServers", "repowise", new_entry)
if action is FileAction.KEPT:
action = write_json_config(
config_path, {"mcpServers": {"repowise": new_entry}}
The helper's own comment says the opposite of what the caller does: # container_key is present but not an object; leave the file alone
# rather than guessing what a rewrite would mean.
return FileAction.KEPTIt leaves the file alone, and then the caller guesses. The old code did not have this hole: it did Reaching it needs a Two smaller things: The PR title and body say "opt-out for project-local editor files", and there is no opt-out anywhere in the diff. It is entirely the no-reformat half. #1603 has both halves and this closes it on merge, which would mark the opt-out as done when it is not. Worth retitling and changing Non-blocking: 270 lines of hand-written JSON position scanning is a lot of new surface in a write path. Fix the fallback and I will take this. |
Fixes #1603
What
When
repowise initwrites the repo-shared, tracked.mcp.json, it re-serialised the whole document from a dict. Pre-existing, unrelated server entries (e.g. a SonarQube entry with compact["-e", "SONARQUBE_TOKEN"]args) therefore came back reformatted — one arg per line — in a file repowise had no business re-rendering end to end.This PR makes the
.mcp.jsonwrite a minimal edit. Only themcpServers.repowiseentry is touched; every other byte of the file stays identical.Changes
json_merge.merge_json_object_member— a surgical, indentation-aware upsert that inserts/replaces only the owned member while preserving the rest of the document byte-for-byte. It also:envblock) so BYOK survives re-registration (mirrorsmerge_server_entries, [Bug] Re-registration wipes user-addedenv(BYOK keys) on the repowise MCP entry — shallow servers.update() replaces the whole entry #307);UNCHANGEDand writes nothing;The opt-out half of #1603 (project-local files behind
--no-editor-setup) was already resolved in #1572; this closes the remaining reformatting bug the issue calls out as the one to fix first.