fix(mcpinstall): stop destroying user config on install/uninstall

[mfacenet]

Summary

Fixes verified audit findings where sting install / sting uninstall could
silently corrupt or destroy user config files. All findings are listed below
with file:line (pre-change references), what changed, and any caveats.

Verification: go build ./..., go vet ./..., go test ./..., and
golangci-lint v2.12.2 run ./... all pass. Per-package coverage gate
(scripts/check-coverage.sh) passes; internal/mcpinstall is at 88.4%.

---

P0 — JSON numeric corruption of ~/.claude.json

internal/mcpinstall/claude.go:123-156 (readJSONDoc)
json.Unmarshal into map[string]any coerced every number to float64, and the
Claude user-scope adapter rewrites the entire ~/.claude.json, so any integer
above 2^53 anywhere in the file was silently rounded on install/uninstall.
Change: decode with json.Decoder + UseNumber() so json.Number
round-trips losslessly. This also protects .mcp.json and opencode.json.
Test: TestClaudeLargeIntegerRoundTrip (9007199254740993 survives a write).

P0 — TOML content destruction (~/.codex/config.toml, ~/.grok/config.toml)

internal/mcpinstall/codex.go:116-146 (readTOMLDoc/writeTOMLDoc, shared by grok.go)
go-toml v2 has no comment model, so the read-then-remarshal cycle deleted all
comments, reordered keys, rewrote quoting, and exploded inline tables.
Change: implemented a surgical, format-preserving text editor
(internal/mcpinstall/tomledit.go). It locates only the [mcp_servers.sting]
table and its [mcp_servers.sting.*] subtables and inserts/replaces/removes just
those bytes, leaving every other byte untouched. A string- and array-aware
scanner tracks multiline strings and array-bracket depth so a [ inside a value
or multiline string is never mistaken for a table header. The comment-destroying
writeTOMLDoc remarshal path is removed; WriteEntry/RemoveEntry now use the
surgical editor. ReadEntry still uses go-toml (read-only, no data loss).

• No new dependency added (reuses the existing go-toml/v2 only to render the
  small sting block and to detect malformed files / read existing keys).
  Limitations (documented in the file header, and safe):
• The entry must be a standard [mcp_servers.sting] table header — which is how
  sting and every runtime write it. An entry hand-authored as an inline/dotted
  key (mcp_servers.sting = {...}) is refused with a clear "edit manually"
  error rather than silently duplicated or corrupted.
• A trailing multiline value whose final lines look blank/comment-like is an
  untested edge; sting's own tables never produce this.
  Tests: TestCodexWritePreservesComments, TestGrokWritePreservesEnvOnPathChange,
  TestCodexRemovePreservesNeighborComment, TestScannerIgnoresBracketsInValues,
  TestScannerHandlesArrayTables, TestInlineStingEntryRefused,
  TestQuotedStingHeaderMatches, TestSplitTOMLKeyQuoted, and others.

P1 — WriteEntry clobbered user-added keys on upgrade

claude.go:94, grok.go:118 (grokServer.Env dead field at grok.go:74), opencode.go:142
WriteEntry assigned a fresh struct, so a user entry carrying env (a token),
type, cwd, timeout, or headers lost those keys whenever the command path
changed — the exact upgrade path.
Change: merge into the existing entry instead of replacing it.

• claude.go / opencode.go: read the existing entry map, set
  command/args/enabled/type, preserve all other keys.
• grok.go: env and other extra keys are preserved structurally by the TOML
  surgical merge (reads the existing [mcp_servers.sting] table, overwrites only
  command/args/enabled, keeps env and the rest).
• Dead-field note: because env is now preserved structurally by the merge
  (not round-tripped through the struct), the grokServer.Env field had no
  consumer, so it was removed to resolve the dead-field finding. I did not
  add Env to the shared Entry because that would spuriously break the
  reflect.DeepEqual idempotency check in runInstall (install never supplies
  env, so existing.Env != desired.Env would report "updated" on every run).
  Test: TestGrokWritePreservesEnvOnPathChange, TestClaudeWritePreservesUserKeys,
  TestOpencodeWritePreservesUserKeys (env survives a path-changing update).

P1 — WriteAtomic never fsync'd

internal/mcpinstall/atomic.go:31-48
write→chmod→close→rename with no fsync could leave a zero-length/truncated config
after a crash.
Change: Sync() the temp file before Close(), and fsync the parent
directory after os.Rename (dir sync guarded/skipped on Windows). The existing
same-dir temp file and mode preservation are unchanged.

---

P2 — landed

• uninstall aborts all runtimes on one malformed entry (internal/cli/uninstall.go:100-102):
  a present-but-undecodable entry is treated as removable (RemoveEntry only needs
  key existence), and per-runtime errors are collected with errors.Join and
  named, so one bad config no longer aborts removal from the others.
  Test: TestRunUninstallMalformedEntryDoesNotAbortOthers.
• install.go fail-fast, no aggregation (install.go:78-107): continue
  per-runtime, aggregate with errors.Join, name the failing runtime.
  Test: TestRunInstallAggregatesErrors.
• Detect false positives (opencode.go:37-39, grok.go:32-34): stat the
  resolved config dir before declaring detection so a stale
  OPENCODE_CONFIG_DIR/GROK_CONFIG_DIR cannot fabricate a config tree.
  Tests: TestOpencodeDetectStaleEnvVar, TestGrokDetectStaleEnvVar.
• claude.go:96 creates fresh ~/.claude.json as 0644 → now 0600 (OAuth
  material). WriteAtomic still preserves an existing file's mode.
  Test: TestClaudeCreatesPrivateFile.
• jsonObjectAt rejects "mcpServers": null (claude.go:160-169): explicit
  null is now treated as absent. Test: TestClaudeNullMcpServers.
• preserve an existing enabled:false on reinstall (install.go:197 /
  opencode / grok): reinstall no longer forces enabled back to true.
  Tests: TestRunInstallPreservesDisabled, TestGrokWritePreservesDisabled.

P2 — deferred (with reason)

• read-only permissions "cannot drift" tautological test (install.go:171 +
  mcpserver server_test.go): the weak test lives in the mcpserver package,
  which is out of scope here and owned by a parallel PR. install.go's
  permission block already derives from mcpserver.ReadOnlyTools(), so it does
  not drift from the annotations; only the test is tautological. Left untouched
  to avoid colliding with the mcpserver PR, per instructions.

Notes for maintainer

• The surgical TOML editor is the main judgment call. It is the real fix for the
  P0 (vs. the fallback of a .bak + doc correction), and it makes the
  format-preserving promise in ADR 0003 and runtime.go honest — so I updated
  the runtime.go package doc to describe the actual semantics rather than
  weakening the ADR. Please sanity-check the inline-entry refusal behavior:
  it is conservative (refuse rather than risk corruption) but means a user with a
  hand-authored inline mcp_servers.sting = {...} must edit manually.
• grokServer.Env removal (see P1) is a deliberate deviation from the finding's
  "populate it" suggestion; rationale above. Happy to instead add Env to
  Entry if you prefer that trade-off.

[Copilot]

Pull request overview

Fixes multiple audit findings where sting install / sting uninstall could corrupt or clobber user config files by improving JSON numeric fidelity, preserving TOML formatting, making writes crash-safe, and making per-runtime operations more resilient.

Changes:

• Add a format-preserving TOML “surgical editor” for Codex/Grok configs and migrate adapters to use it.
• Improve JSON handling (lossless numbers + merge-with-existing semantics) and tighten permissions for newly created ~/.claude.json.
• Make atomic writes more crash-safe (fsync temp file + parent dir) and aggregate install/uninstall errors per runtime.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/mcpinstall/tomledit.go	New surgical TOML editor to insert/replace/remove only the mcp_servers.sting subtree without rewriting unrelated bytes.
internal/mcpinstall/tomledit_test.go	Regression tests for TOML format preservation, header scanning correctness, and refusal of unsafe inline/dotted entries.
internal/mcpinstall/snippet.go	Updates Grok snippet generation to match updated Grok adapter structures.
internal/mcpinstall/runtime.go	Package doc updated to reflect atomic+fsync semantics and format-preserving behavior.
internal/mcpinstall/opencode.go	Avoid false-positive detection from stale env vars; merge writes to preserve user-added keys.
internal/mcpinstall/json_fidelity_test.go	Regression tests for JSON numeric round-tripping, null handling, permission mode, and merge preservation.
internal/mcpinstall/grok.go	Avoid false-positive detection; migrate Grok writes/removals to surgical TOML edits; clarify struct intent.
internal/mcpinstall/detect_falsepos_test.go	Tests for stale GROK_CONFIG_DIR / OPENCODE_CONFIG_DIR not fabricating detection.
internal/mcpinstall/codex.go	Migrate Codex writes/removals to surgical TOML edits; remove remarshal writer.
internal/mcpinstall/claude.go	JSON UseNumber() decoding; merge writes to preserve user keys; set newly created file mode to 0600; treat null objects as absent.
internal/mcpinstall/atomic.go	Improve atomic write durability via Sync() on temp file and fsync parent directory after rename (non-Windows).
internal/cli/uninstall.go	Continue uninstall across runtimes and return aggregated errors; treat undecodable entries as candidates.
internal/cli/install.go	Continue install across runtimes and return aggregated errors; preserve user-disabled entries.
internal/cli/install_more_test.go	Tests for enabled-state preservation and per-runtime error aggregation behavior.