Repository navigation
Description
Problem
Runtime.LoadConfiguredServers(cfg) reconciles storage, the search index and upstream connections from a captured config snapshot, and does so outside configCommitMu. It is scheduled asynchronously by ApplyConfig (internal/runtime/runtime.go:2092-2093). When the snapshot is older than a concurrent commit, every phase acts on stale data:
- Orphan GC uses the snapshot's name set:
PruneOrphanToolApprovals(internal/runtime/lifecycle.go:1461) andPruneOrphanToolCalls(:1472) delete the approval records, explicit tool blocks and call history of a server added after the snapshot. - Removal (
:1600-1640) treats any stored row missing from the snapshot as "no longer in config" and callsDeleteUpstreamServer(:1627), deleting a server that was just created through REST/MCP. A followingSaveConfigurationthen drops it from runtime and disk. - Storage save loop (
:1557,SaveUpstreamServer(serverCfg)) and the connect goroutines (:1573-1574) write the stale snapshot over a server that was removed and re-created (new args/env/quarantine/blocks), or revert a newer explicit quarantine decision. ApplyConfigpublishes the new config before it updates storage; any later commit that rebuilds the server list from storage (SaveConfiguration,lifecycle.go:1667) restores the stale quarantine/disable state of server A when an unrelated server B is toggled before the queued reconcile runs.- Same class on the add path:
Server.AddServerbuilds its config snapshot froms.runtime.Config()beforeUpdateConfigtakes the commit lock (internal/server/server.go:1995-1996; MCPinternal/server/mcp.go:5978), so a concurrentApplyConfigbetween the two is overwritten (can undo profile restrictions or other security settings).
Failure scenario
ApplyConfig(config A) schedules reconcile; the worker is delayed.- A server B is added via REST. It is approved, one tool is blocked, a call is recorded.
- The delayed reconcile runs with the pre-B snapshot: B's approvals/blocks/history are pruned and B's row is deleted. On an auto-trust server, rediscovery recreates the blocked tool as approved+enabled.
Suggested direction
Make reconcile a commit: take configCommitMu, re-read the live config inside it and reconcile against that (or discard the snapshot when the live pointer moved), and make ApplyConfig publish + storage update one commit. Never delete a row/records unless the name is absent from the current live config. Cover with deterministic hook-gated -race tests (pause before storage read / before save loop / before prune).
Where Sol found it
ux01-r2.txt #1, ux01-r3.txt #1, ux01-r4.txt #1, ux01-r5b.txt #1 (+ ux01-r5a.txt), ux01-r9-B.txt #1, ux01-r1.txt #1 (add-path snapshot).
Found by OpenCode Sol 6.1 cross-model review of the (now split) oversized PRs #1553 (UX-01) and #1555 (UX-02); verified still present on origin/main (704795c) by reading the cited sites. Pre-existing, deliberately out of scope of the minimal core PRs. Review transcripts: docs/qa/0.70.0-ux-2026-10-10/remediation/reviews/.
Activity
fix: make server add create-only across REST, MCP and Web UI (UX-01)
docs(specs): Spec 116: config commit consistency, atomic removal and …
fix: single config commit section with per-server generations (Spec 1…