docs(specs): Spec 116: config commit consistency, atomic removal and upstream retirement (#1558-#1560, #1562-#1564)

[Dumbris]

Summary

This is the design spec (speckit) for the six concurrency and consistency issues Sol 6.1 found while reviewing UX-01 and UX-02: #1558, #1559, #1560, #1562, #1563 and #1564. It is documentation only. The implementation follows as four PRs.

Root cause. A server's state lives in four stores: the live config, bbolt storage, the config file and the upstream manager. Changes reach them in different orders, and only some steps hold configCommitMu. Two flows run in opposite directions: LoadConfiguredServers copies config to storage, and SaveConfiguration rebuilds the config from storage. Asynchronous workers act on stale snapshots. The rejected branch fix/qa070-ux01-create-only-add patched each reader of that model in turn and needed ten review rounds. This spec removes the second source of truth instead.

Design in one paragraph. Every server writer goes through Runtime.Commit. It reads the live config inside configCommitMu, mutates a copy, then runs: gate → prepare generations → disk → one bbolt transaction (purge included on removal) → advance the manager (unregister and retire stale instances) → publish. The live config is the only source, so SaveConfiguration no longer reads storage and there is nothing left to resurrect a removed server from. Each server has a generation, which changes on any change to its entry. Workers and the supervisor check it inside the manager's registration lock. Each server also has an incarnation, which changes only on remove followed by create. Approval writers check it under the per-server approval lock against the storage row. Each client instance owns a dial gate. Retirement closes the gate at the byte level (stdio Start, gated dialer, gated conn Write), and the instance's OAuth client and net/http retries share the same gated transport.

PR split (each one merges from main independently)

main ──► PR1 ──┬──► PR2
               └──► PR3 ──► PR4

PR	Scope	Issues
PR1 Commit critical section + generations	Commit/CommitTx, two-phase configsvc with per-server generations, reconcile inside the commit, SaveConfiguration persists the live config, all writers converted (REST/MCP add, update, patch, enable, quarantine, remove; Apply; Reload; registry sources; sharing), createMu removed, CI write-site guard	#1558, #1560, root of #1559, B1
PR2 Atomic removal + purge + incarnation-bound approvals	full purge set in the commit transaction, exact OAuth key match, one removal path, approval writers and scanner bound to the incarnation, expected_incarnation (optional) plus 409 server_replaced	#1559, #1562, B2
PR3 Generation-bound workers and supervisor	Manager.desired, AddServerConfigAt/RemoveServerAt/ConnectServerAt, AdvanceGenerations at commit step 6, supervisor/restart/capture carry the generation	#1563
PR4 Retirement barrier, OAuth included	internal/upstream/dialgate, per-instance transport, stdio/Docker/launcher admission, OAuth WithBaseTransport, retire before publish	#1564

Each PR has failing-first hook-gated -race tests (quickstart.md T1–T4) and a live check on the qa070 harness (L1–L4). tasks.md groups the tasks by PR in TDD order.

Key decisions

• D2: one source of truth. Storage becomes a projection of the live config written inside the commit. scripts/check-upstream-write-sites.sh fails CI on any upstreams write outside the commit.
• D4: synchronous removal, no tombstones. The async cleanup window is where every resurrection happened, and the rejected branch's tombstones leaked into every new reader.
• D6: generation plus incarnation. Workers need "this exact config". Approvals need "this exact server identity". Binding approvals to the incarnation means an unrelated edit never rejects an approval in flight, while a re-add always does.
• D8: gate the bytes, not the strategy. A conn-level Write gate on a per-instance http.Transport covers TLS, HTTP/2, net/http internal retries and the OAuth client. Sol rejected the RoundTripper and flag approaches in r8–r10.
• D9: lock order (normative). configCommitMu → approval locks (sorted; released before step 6) → storage/index. Then configCommitMu → captureMu(W) → Manager.mu → gate. Then configsvc.updateMu and Runtime.mu as leaves. This respects the existing capture order captureMu(R) → approval lock. createMu is deleted.

New findings recorded while researching (verified on main ee7834e)

• B1: MCP quarantine_security quarantine_server does not purge the index. The tool descriptions were still returned by retrieve_tools 35 s after a successful quarantine. This bypasses the security: quarantining a server via the config file leaves its tools in the search index, so retrieve_tools still discloses their descriptions #1061 TPA control. Fixed in PR1 (FR-008).
• B2: ClearOAuthState("foo") deletes foo_bar's OAuth tokens because it matches on the prefix name_. Reproduced with a throwaway storage test. Fixed in PR2 (FR-011).

Design review (opencode github-copilot/gpt-6.1-sol)

Four rounds were run on this design PR, using 4 of the 10-round cap. 25 findings were each verified against the code and resolved in the spec. research.md §7-§10 has the finding-by-finding table. The main design changes from review:

• Late asynchronous writers. Scan, OAuth token and completion, identity and history writes become incarnation-conditional bbolt writes (D12).
• Commit diffs and boot. A commit diffs against both the base and the stored view, and has Bootstrap/ForceDisk (D13). ReplaceConfig and ReconcileLive are separate operations (D14).
• Discovery and approvals. A discovery pass does its final check and all index writes in one hold of the approval lock (D15). Approval policy is read from storage, and a global policy change takes every server's approval lock (D16).
• Retirement gate. The gate's read lock covers only leaf operations, and WrapTransport is lock-free. stdio launch and registration are one admitted section. Windows processes are created suspended and assigned to their Job. The gate also covers the OAuth preflight helpers, the browser launch, the launcher readiness probe and Docker diagnostic execs. Teardown commands are exempt.
• Generation-bound manager operations. DisconnectServerAt and ReplaceInstanceAt (covering ForceReconnectAll and Docker recovery) are added. Instance generation stamping moves to PR1.

Round 4 still reported findings (2 high, 2 medium), and all four are resolved in the spec. No round has returned PASS yet, so the next rounds run per implementation PR, against the code.

Coordination with Spec 117

Spec 117 (#1565-#1567) is being designed in parallel. research.md §6 defines the integration points:

• server approval becomes Commit + UnderApprovalLock;
• the baseline marker is in the removal purge set;
• there is one Retire() per instance;
• the union of the two lock orders is acyclic.

Open questions (default taken; see research §5)

1. Should expected_incarnation be required on approval endpoints? Default: optional, with the handler falling back to the incarnation read at request start.
2. Should scan reports be purged on removal? Default: yes. Otherwise a leftover clean report lets a same-name replacement be approved without a scan.
3. Should the index purge stay synchronous inside the commit? Default: yes for removal and quarantine. PR2 measures it (SC-004).
4. Should the generation persist across restarts? Default: yes, in meta.config_commit_generation.
5. How long can Retire take? Default: it is bounded by design, and anything over 1 s is logged as a warning.

#1569 (tray) is handled separately in #1572. This spec does not touch native/.

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying mcpproxy-docs with    Cloudflare Pages

Latest commit:	d67c954
Status:	✅  Deploy successful!
Preview URL:	https://13b8fa0e.mcpproxy-docs.pages.dev
Branch Preview URL:	https://116-config-commit-consistenc.mcpproxy-docs.pages.dev

View logs

[github-actions]

📦 Build Artifacts

Workflow Run: View Run
Branch: 116-config-commit-consistency

Available Artifacts

• archive-darwin-amd64 (31 MB)
• archive-darwin-arm64 (28 MB)
• archive-linux-amd64 (19 MB)
• archive-linux-arm64 (17 MB)
• archive-windows-amd64 (31 MB)
• archive-windows-arm64 (27 MB)
• frontend-dist-pr (0 MB)
• installer-dmg-darwin-amd64 (27 MB)
• installer-dmg-darwin-arm64 (24 MB)
• smart-mcp-proxymcpproxy-goN8ZFR8.dockerbuild (0 MB)

How to Download

Option 1: GitHub Web UI (easiest)

1. Go to the workflow run page linked above
2. Scroll to the bottom "Artifacts" section
3. Click on the artifact you want to download

Option 2: GitHub CLI

gh run download 38151645311 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!