feat(macos): native Connect Client form — preview-first, token-guarded (spec 091)

CLAUDE.md

@@ -158,6 +158,7 @@ tail -f ~/Library/Logs/mcpproxy/main.log  # main log (macOS; Linux: ~/.mcpproxy/
 - **Windows installer**: [docs/github-actions-windows-wix-research.md](docs/github-actions-windows-wix-research.md). **Prerelease** (`next` branch + `v*-rc.*` tags, opt-in, off stable channels): [docs/prerelease-builds.md](docs/prerelease-builds.md).
 
 ## Recent Changes
+- 091-connect-client-form: Added Swift 5.9 (SwiftUI sheet + AppKit menu) + Go 1.25 (core) + existing `internal/connect` package (registry, preview, connect, undo, backup), `internal/httpapi/connect.go` routes; Swift `APIClient` over Unix socket (admin context)
 - 090-tray-glance-v2: Added Swift 5.9 (AppKit, SwiftUI for histogram only) + Go 1.25 (core) + AppKit `NSMenuItem` (incl. `subtitle`, macOS 14.4+), fixture-driven XCTest; Go: chi HTTP API, bbolt storage, zap
 - 088-scanner-trust-ui: Added TypeScript 5.9, Vue 3.5 (Composition API, `<script setup>`), Tailwind v4 + Existing only — hand-written `ApiClient` singleton (`frontend/src/services/api.ts`), Pinia stores (`stores/servers.ts`, `stores/system.ts`), generated `types/contracts.ts` (via `cmd/generate-types`), hand-written `types/api.ts`. **No new npm dependency.**
 - 084-toon-output: Adaptive TOON encoding of `call_tool_*` result text blocks (`internal/toonenc/`, seam in `internal/server/toon_encode.go`). Config `toon_output` (off|adaptive|always, per-server override) + `toon_min_savings_pct`; off by default, hot-reloadable; never-larger by construction; sanitise-before-encode, detection-parity, truncate-after-encode invariants. **New dependency: `github.com/toon-format/toon-go`** (the production encoder; shared with the spec-083 profiler). Docs: [docs/features/toon-output.md](docs/features/toon-output.md).

ROADMAP.md

@@ -792,3 +792,4 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—`
 | [088-scanner-trust-ui](./specs/088-scanner-trust-ui/) | `shipped` | 29/29 (100%) |
 | [089-remote-access-tunnel](./specs/089-remote-access-tunnel/) | — | — |
 | [090-tray-glance-v2](./specs/090-tray-glance-v2/) | `shipped` | 35/35 (100%) |
+| [091-connect-client-form](./specs/091-connect-client-form/) | `shipped` | 26/26 (100%) |

docs/api/rest-api.md

@@ -720,6 +720,29 @@ never overwritten. Backups accumulate one per operation and are **never
 deleted automatically**; there is no retention bound, so an undo (below) can
 always find its backup.
 
+`POST` optionally accepts `precondition_token` (Spec 091) — the opaque token
+from the preview this write was confirmed against:
+
+```json
+{ "server_name": "mcpproxy", "force": true, "precondition_token": "…" }
+```
+
+When supplied, the core recomputes the token at write time and, if it no longer
+matches, responds **`409 Conflict`** having written **nothing** (the check runs
+before any backup or write). `force=true` does not override a stale token. When
+omitted, behavior is exactly as before.
+
+The `409` body carries a top-level `action` discriminating the two conflict
+kinds:
+
+| `action` | Meaning | Caller should |
+|----------|---------|---------------|
+| `precondition_failed` | The preview is stale — the config file, the existing entry, or the entry mcpproxy would now write has changed. | Re-fetch the preview; do not blindly retry. |
+| `already_exists` | Pre-existing semantics: an entry with that name is present and `force` was not set. | Confirm with the user, then retry with `force=true` (and a fresh token). |
+
+See [Connect Clients](../features/connect-clients.md) for the token's contents
+and threat model.
+
 #### GET /api/v1/connect/{client}/preview
 
 Returns the exact change a subsequent connect would make — target config path,
@@ -732,6 +755,17 @@ so on macOS this may raise an App-Data prompt; a denial returns `403` +
 remediation. Optional `?server_name=` mirrors the name a subsequent connect
 would use.
 
+Spec 091 adds three response fields:
+
+| Field | Type | Meaning |
+|-------|------|---------|
+| `existing_entry_summary` | object, present only when `entry_exists=true` | Sanitized description of the entry that would be **replaced**: `entry_name` (the key it actually lives under, which may differ from `server_name` when the write adopts an endpoint-equivalent entry), `type`, `endpoint` (query string, `user:pass@` userinfo and fragment stripped), `command`, `header_names` and `env_names` — **names only, never values**. Built by whitelist projection, so no other config content can reach the response. |
+| `precondition_token` | string, always present | Opaque keyed HMAC binding this preview to the exact pre-write state. Pass it to `POST` (above) to make a stale preview unwritable. Per-core-instance key, never persisted. |
+| `connect_refusal` | string, optional | The verbatim reason a subsequent connect would refuse regardless of user intent (today: a non-create-capable client such as **OpenCode** with no config present). Treat as "Connect unavailable". |
+
+The preview evaluates the refusal with the write's own guard, so the two cannot
+drift. Full semantics: [Connect Clients](../features/connect-clients.md).
+
 #### POST /api/v1/connect/{client}/undo
 
 One-click undo of the immediately-preceding connect (Spec 078 US3). Body:

docs/features/connect-clients.md

@@ -0,0 +1,130 @@
+---
+id: connect-clients
+title: Connect Clients
+sidebar_label: Connect Clients
+sidebar_position: 12
+description: Preview-before-write connect flow that registers mcpproxy in an MCP client's config file, with a sanitized disclosure of what is replaced and a precondition token that makes a stale preview unwritable
+keywords: [connect, client, preview, precondition, claude desktop, cursor, vs code, opencode]
+---
+
+# Connect Clients
+
+**Connect Clients** registers mcpproxy inside another MCP client's configuration
+file (Claude Desktop, Cursor, VS Code, Codex, Gemini, OpenCode, …) so that client
+talks to the proxy instead of to each upstream server directly. It is available
+from the Web UI wizard, the macOS tray ("Connect Client…"), and the
+`mcpproxy connect` CLI.
+
+The UI flows are **preview → confirm → write** (the CLI writes directly, with
+`--force` to overwrite an existing entry):
+
+1. `GET /api/v1/connect/{client}/preview` renders the exact change — target
+   config path, format, server key, and the entry that would be written (any
+   embedded API key masked).
+2. `POST /api/v1/connect/{client}` performs the write, taking a timestamped
+   backup of an existing file first.
+3. `POST /api/v1/connect/{client}/undo` reverts that write byte-for-byte (or
+   removes the file the connect created).
+
+Endpoint-level reference — request/response shapes, backup naming, undo
+semantics, and the macOS App Data privacy prompt — lives in the
+[REST API reference](../api/rest-api.md#connect-client-wizard).
+
+## What the preview discloses (Spec 091)
+
+A preview shows the entry that *will be written*, which mcpproxy constructs and
+can therefore mask. It historically said nothing about the entry being
+**replaced**, which is user-authored content mcpproxy cannot safely echo. Two
+preview fields close that gap without leaking config contents.
+
+### `existing_entry_summary`
+
+Present only when `entry_exists` is true. A fixed, whitelist-built projection of
+the entry the write would replace:
+
+| Field | Meaning |
+|-------|---------|
+| `entry_name` | The key the entry actually lives under. May differ from the requested `server_name` when the write **adopts** an endpoint-equivalent entry stored under a non-canonical name. |
+| `type` | Transport type (`http`, `stdio`, …). |
+| `endpoint` | Endpoint URL with **query string, userinfo (`user:pass@`) and fragment stripped**. |
+| `command` | Command path for stdio/bridge entries. |
+| `header_names` | Header **names** only — never values. Includes names parsed out of `--header` / `-H` bridge arguments. |
+| `env_names` | Environment variable **names** only — never values. |
+
+Secrecy holds *by construction*, not by masking heuristics: no other field of the
+existing entry is copied, header/env values are never read, and a URL-shaped
+field is emitted only after being reparsed into `scheme://host/path`. Backend
+tests feed entries containing rotated API keys, bearer headers, env secrets,
+`?apikey=` URLs and `user:pass@` URLs and assert none of those values appear
+anywhere in the serialized preview response.
+
+The summary is **display-only** and is never used for drift detection — that is
+the precondition token's job, which hashes the raw entry.
+
+### `connect_refusal`
+
+A string, present only when a subsequent connect would refuse **regardless of
+user intent**, carrying the same verbatim reason the write would return. The
+preview runs the write's own guard rather than a copy, so the two can't drift.
+
+Today the single case is a client that mcpproxy will not create a config for
+from scratch: **OpenCode** owns a config schema mcpproxy will not invent, so with
+no `opencode.jsonc` / `opencode.json` present the connect refuses instead of
+creating one. Consumers must treat a non-empty `connect_refusal` as
+"Connect unavailable" and surface the reason — the macOS form hides its Connect
+button entirely in that state.
+
+## Precondition token and the discriminated conflict (Spec 091)
+
+`precondition_token` is an opaque string **always present** in a preview
+response. Passing it back on the write binds the write to the exact state the
+preview described:
+
+```jsonc
+// POST /api/v1/connect/{client}
+{ "server_name": "mcpproxy", "force": true, "precondition_token": "…" }
+```
+
+The token is an HMAC-SHA256 over a canonical **length-prefixed** encoding of:
+
+- the config path,
+- whether the file exists (create vs. update),
+- the **resolved** target entry name — after the same equivalent-entry adoption
+  the write performs — and its **raw** value (so a credential rotation *inside*
+  the existing entry drifts the token even though the sanitized summary hides
+  it), and
+- the exact pending entry mcpproxy would write right now (so proxy-side drift —
+  API-key rotation, `require_mcp_auth` toggle, listen-address change —
+  invalidates the preview too, and a credential can never be embedded without
+  the credential notice having been shown).
+
+It is keyed with a **per-core-instance random in-memory key**: tokens are
+single-session by design, never persisted, and not usable as an offline
+confirmation oracle for masked values.
+
+### Behavior
+
+- **Token matches** → the write proceeds normally.
+- **Token stale** → `409 Conflict` with `"action": "precondition_failed"`, and
+  **nothing is written** — the check runs before any backup or write, so a
+  refusal is completely inert. The caller should re-preview, not retry.
+- **`force=true` with a stale token** → still `precondition_failed`, still no
+  write. The token, not the absence of `force`, is the overwrite safety.
+- **Token absent** → exactly the previous behavior. Existing Web UI and CLI
+  callers are unaffected.
+
+The `409` body is machine-discriminable at the top level via `action`:
+
+| `action` | Meaning | Caller should |
+|----------|---------|---------------|
+| `precondition_failed` | The preview is stale (file, existing entry, or pending entry changed). | Re-fetch the preview and show the user the new change. |
+| `already_exists` | Pre-existing semantics: an entry with that name is present and `force` was not set. | Ask the user, then retry with `force=true` (plus a fresh token). |
+
+### Consumers
+
+- The **macOS Connect Client form** always sends the token from its rendered
+  preview; replace flows send `force=true` **with** the token, never `force`
+  alone. A `precondition_failed` triggers exactly one automatic re-preview.
+- The **Web UI** ConnectModal is unchanged and may adopt the token later.
+- Write gating is unchanged: connect/disconnect routes still reject restricted
+  agent tokens, and the tray's Unix-socket transport carries admin context.

internal/connect/adoption_test.go

@@ -0,0 +1,167 @@
+package connect
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+// Adoption resolution must be DETERMINISTIC and resolved exactly ONCE per
+// operation (Spec 091 FR-005). The precondition token promises it hashes "the
+// entry the write would actually replace"; if the preview, the token check and
+// the write each pick a different equivalent entry out of a randomly-ordered Go
+// map, that promise is void — the token covers one entry while the write
+// deletes another.
+
+// TestFindEquivalentJSONServerName_Deterministic pins the tie-break: an exact
+// name match wins, otherwise the lowest name in sorted order. Repeated over a
+// map whose iteration order Go deliberately randomizes.
+func TestFindEquivalentJSONServerName_Deterministic(t *testing.T) {
+	const baseURL = "http://127.0.0.1:8080/mcp"
+
+	t.Run("two url-equivalent entries resolve to the sorted-first name", func(t *testing.T) {
+		for i := 0; i < 200; i++ {
+			serversMap := map[string]interface{}{
+				"b-legacy": map[string]interface{}{"url": baseURL + "?apikey=old"},
+				"a-clean":  map[string]interface{}{"url": baseURL},
+				"unrelated": map[string]interface{}{
+					"url": "http://127.0.0.1:9999/mcp",
+				},
+			}
+			got, found := findEquivalentJSONServerName(serversMap, baseURL, "mcpproxy")
+			if !found {
+				t.Fatal("expected a match")
+			}
+			if got != "a-clean" {
+				t.Fatalf("iteration %d: resolved %q, want the deterministic sorted-first match %q", i, got, "a-clean")
+			}
+		}
+	})
+
+	t.Run("the requested name wins over a url-equivalent entry", func(t *testing.T) {
+		for i := 0; i < 200; i++ {
+			serversMap := map[string]interface{}{
+				"mcpproxy": map[string]interface{}{"url": "http://127.0.0.1:9999/mcp"},
+				"legacy":   map[string]interface{}{"url": baseURL},
+			}
+			got, found := findEquivalentJSONServerName(serversMap, baseURL, "mcpproxy")
+			if !found {
+				t.Fatal("expected a match")
+			}
+			if got != "mcpproxy" {
+				t.Fatalf("iteration %d: resolved %q, want the exact requested name (it is what the token and the summary cover)", i, got)
+			}
+		}
+	})
+}
+
+// TestConnect_OpenCodeAdoptionNeverDeletesAnUnnamedEntry is scenario B of the
+// nondeterminism: the requested name already exists, so preview/token/summary
+// cover exactly that key — yet the write's own independent adoption lookup used
+// to reach a URL-equivalent entry under another key and silently delete it.
+func TestConnect_OpenCodeAdoptionNeverDeletesAnUnnamedEntry(t *testing.T) {
+	for i := 0; i < 40; i++ {
+		svc, home := testService(t)
+		cfgPath := ConfigPath("opencode", home)
+		writeFileT(t, cfgPath, `{"mcp":{`+
+			`"mcpproxy":{"type":"remote","url":"http://127.0.0.1:9999/mcp"},`+
+			`"legacy":{"type":"remote","url":"http://127.0.0.1:8080/mcp?apikey=old"}}}`)
+
+		preview, err := svc.Preview("opencode", "mcpproxy")
+		if err != nil {
+			t.Fatalf("Preview: %v", err)
+		}
+		if preview.ExistingEntrySummary == nil || preview.ExistingEntrySummary.EntryName != "mcpproxy" {
+			t.Fatalf("iteration %d: preview must name the exact-name entry, got %+v", i, preview.ExistingEntrySummary)
+		}
+
+		res, err := svc.ConnectWithPrecondition("opencode", "mcpproxy", true, preview.PreconditionToken)
+		if err != nil {
+			t.Fatalf("ConnectWithPrecondition: %v", err)
+		}
+		if !res.Success {
+			t.Fatalf("iteration %d: expected success, got %+v", i, res)
+		}
+		if !strings.Contains(readConfigT(t, cfgPath), `"legacy"`) {
+			t.Fatalf("iteration %d: the write deleted an entry the token never hashed and the summary never named:\n%s",
+				i, readConfigT(t, cfgPath))
+		}
+	}
+}
+
+// TestConnect_OpenCodeAdoptionMatchesThePreviewedEntry is scenario A: two
+// URL-equivalent entries and a requested name that matches neither. The write
+// must adopt exactly the entry the preview resolved — never flip to the other
+// one (which would be a spurious precondition_failed, or a silent delete of an
+// entry the user was never shown).
+func TestConnect_OpenCodeAdoptionMatchesThePreviewedEntry(t *testing.T) {
+	for i := 0; i < 40; i++ {
+		svc, home := testService(t)
+		cfgPath := ConfigPath("opencode", home)
+		writeFileT(t, cfgPath, `{"mcp":{`+
+			`"a-clean":{"type":"remote","url":"http://127.0.0.1:8080/mcp"},`+
+			`"b-legacy":{"type":"remote","url":"http://127.0.0.1:8080/mcp?apikey=old"}}}`)
+
+		preview, err := svc.Preview("opencode", "proxy")
+		if err != nil {
+			t.Fatalf("Preview: %v", err)
+		}
+		if preview.ExistingEntrySummary == nil {
+			t.Fatalf("iteration %d: expected an adopted existing entry", i)
+		}
+		adopted := preview.ExistingEntrySummary.EntryName
+		if adopted != "a-clean" {
+			t.Fatalf("iteration %d: preview adopted %q, want the deterministic %q", i, adopted, "a-clean")
+		}
+
+		res, err := svc.ConnectWithPrecondition("opencode", "proxy", true, preview.PreconditionToken)
+		if err != nil {
+			t.Fatalf("ConnectWithPrecondition: %v", err)
+		}
+		if !res.Success {
+			t.Fatalf("iteration %d: a token minted from an unchanged config must not fail: %+v", i, res)
+		}
+
+		var data map[string]interface{}
+		if err := json.Unmarshal([]byte(readConfigT(t, cfgPath)), &data); err != nil {
+			t.Fatal(err)
+		}
+		servers, _ := data["mcp"].(map[string]interface{})
+		if _, gone := servers[adopted]; gone {
+			t.Fatalf("iteration %d: the adopted entry %q should have been replaced", i, adopted)
+		}
+		if _, kept := servers["b-legacy"]; !kept {
+			t.Fatalf("iteration %d: the write deleted %q, which the preview never named:\n%s",
+				i, "b-legacy", readConfigT(t, cfgPath))
+		}
+		if _, written := servers["proxy"]; !written {
+			t.Fatalf("iteration %d: expected the new entry under the requested name", i)
+		}
+	}
+}
+
+// TestUndoReplay_AdoptionIsDeterministic covers undo.go's use of the same
+// matcher: the replay must reconstruct the same bytes the write produced, so it
+// must reach the same adoption decision every time.
+func TestUndoReplay_AdoptionIsDeterministic(t *testing.T) {
+	svc, _ := testService(t)
+	client := FindClient("opencode")
+	backup := []byte(`{"mcp":{` +
+		`"a-clean":{"type":"remote","url":"http://127.0.0.1:8080/mcp"},` +
+		`"b-legacy":{"type":"remote","url":"http://127.0.0.1:8080/mcp?apikey=old"}}}`)
+
+	first, err := svc.replayConnectWrite(client, "proxy", backup)
+	if err != nil {
+		t.Fatalf("replayConnectWrite: %v", err)
+	}
+	for i := 0; i < 100; i++ {
+		again, err := svc.replayConnectWrite(client, "proxy", backup)
+		if err != nil {
+			t.Fatalf("replayConnectWrite: %v", err)
+		}
+		if string(again) != string(first) {
+			t.Fatalf("iteration %d: replay is nondeterministic, so undo would refuse a file it wrote itself:\n%s\n--- vs ---\n%s",
+				i, first, again)
+		}
+	}
+}

internal/connect/clients.go

@@ -204,7 +204,12 @@ func opencodeConfigCandidates(homeDir string) []string {
 func (s *Service) configPath(clientID string) string {
 	if clientID == "opencode" {
 		for _, p := range opencodeConfigCandidates(s.homeDir) {
-			if _, err := os.Stat(p); err == nil {
+			// A candidate we cannot stat counts as PRESENT: the only reason to
+			// prefer .jsonc is that it shadows .json, and skipping an
+			// unstattable one would silently target the shadowed file, where
+			// the write has no effect. Targeting it instead surfaces the real
+			// permission error on the read that follows.
+			if _, err := s.stat(p); err == nil || !os.IsNotExist(err) {
 				return p
 			}
 		}