From be7cfe19d48dd8bc2cb4db189328f6bb3a7dc767 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Sun, 27 Sep 2026 07:30:23 +0300 Subject: [PATCH 1/6] fix(catalog): restore added-server navigation (Spec 109-j) --- frontend/src/components/CatalogSearch.vue | 50 ++++++++++- frontend/src/components/SecretToggle.vue | 13 --- frontend/src/types/api.ts | 2 + .../tests/unit/add-server-catalog.spec.ts | 36 ++++++++ .../MCPProxy/MCPProxy/API/APIClient.swift | 5 ++ .../MCPProxy/MCPProxy/API/CatalogModels.swift | 12 +++ .../macos/MCPProxy/MCPProxy/API/Models.swift | 4 + .../MCPProxy/Models/SecretFieldResolver.swift | 9 +- .../MCPProxy/Views/AddServerView.swift | 6 +- .../MCPProxy/MCPProxy/Views/CatalogView.swift | 87 ++++++++++++++++--- .../MCPProxy/Views/PasteServerView.swift | 23 ++++- .../Views/SecretFieldToggleView.swift | 9 ++ .../MCPProxy/MCPProxy/Views/ServersView.swift | 12 ++- .../MCPProxy/MCPProxyTests/CatalogTests.swift | 20 +++++ .../ResolverStubURLProtocol.swift | 27 ++++++ 15 files changed, 278 insertions(+), 37 deletions(-) create mode 100644 native/macos/MCPProxy/MCPProxyTests/ResolverStubURLProtocol.swift diff --git a/frontend/src/components/CatalogSearch.vue b/frontend/src/components/CatalogSearch.vue index 4b49654b7..9f796d22a 100644 --- a/frontend/src/components/CatalogSearch.vue +++ b/frontend/src/components/CatalogSearch.vue @@ -17,6 +17,7 @@
{{ error }}
diff --git a/frontend/src/types/api.ts b/frontend/src/types/api.ts index 9ceb96962..45620da6f 100644 --- a/frontend/src/types/api.ts +++ b/frontend/src/types/api.ts @@ -238,6 +238,8 @@ export interface ServerIsolationDefaults { export interface Server { name: string + /** Catalog provenance, used only to resolve an already-added catalog card. */ + source_registry_id?: string // Human-friendly display label from the source registry (MCP-1112). When // present it is preferred over `name` for display; `name` stays the stable // identifier used for routing and API calls (it may be a reverse-DNS id such diff --git a/frontend/tests/unit/add-server-catalog.spec.ts b/frontend/tests/unit/add-server-catalog.spec.ts index fb773a504..4ceccd607 100644 --- a/frontend/tests/unit/add-server-catalog.spec.ts +++ b/frontend/tests/unit/add-server-catalog.spec.ts @@ -11,6 +11,7 @@ vi.mock('@/services/api', () => ({ getSecretRefs: vi.fn(), setSecret: vi.fn(), deleteSecret: vi.fn(), + getServers: vi.fn(), }, })) import api from '@/services/api' @@ -49,6 +50,7 @@ describe('CatalogSearch', () => { vi.mocked(api.catalogSearch).mockReset() vi.mocked(api.getConfigSecrets).mockReset() vi.mocked(api.addServerFromRegistry).mockReset() + vi.mocked(api.getServers).mockReset() }) it('defaults to the Catalog source (empty query) and renders sections', async () => { @@ -80,6 +82,40 @@ describe('CatalogSearch', () => { expect(updated.text()).toContain('Added ✓') }) + it('resolves a prior-session Added/Open card only by visible source and install target', async () => { + vi.mocked(api.catalogSearch).mockResolvedValue({ + success: true, + data: { query: '', results: [], sections: { official: [githubResult({ added: true })], popular: [] }, unavailable: [] }, + }) + vi.mocked(api.getServers).mockResolvedValue({ + success: true, + data: { servers: [{ name: 'installed-github', source_registry_id: 'official', url: 'https://api.githubcopilot.com/mcp/', protocol: 'http' }] }, + }) + const wrapper = await mountCatalog() + await wrapper.find('[data-test="catalog-add-official-io.github.github/github-mcp-server"]').trigger('click') + await flushPromises() + expect(router.currentRoute.value.path).toBe('/servers/installed-github') + }) + + it('does not navigate an ambiguous prior-session Added/Open card', async () => { + vi.mocked(api.catalogSearch).mockResolvedValue({ + success: true, + data: { query: '', results: [], sections: { official: [githubResult({ added: true })], popular: [] }, unavailable: [] }, + }) + vi.mocked(api.getServers).mockResolvedValue({ + success: true, + data: { servers: [ + { name: 'github-a', source_registry_id: 'official', url: 'https://api.githubcopilot.com/mcp/', protocol: 'http' }, + { name: 'github-b', source_registry_id: 'official', url: 'https://api.githubcopilot.com/mcp/', protocol: 'http' }, + ] }, + }) + const wrapper = await mountCatalog() + await wrapper.find('[data-test="catalog-add-official-io.github.github/github-mcp-server"]').trigger('click') + await flushPromises() + expect(wrapper.find('[data-test="catalog-error"]').text()).toContain('More than one') + expect(router.currentRoute.value.path).not.toMatch(/github-[ab]/) + }) + it('renders a description containing and markdown INERT — no element is created (D19)', async () => { vi.mocked(api.catalogSearch).mockResolvedValue({ success: true, diff --git a/native/macos/MCPProxy/MCPProxy/API/APIClient.swift b/native/macos/MCPProxy/MCPProxy/API/APIClient.swift index 60c692ec9..9d5bdbc0b 100644 --- a/native/macos/MCPProxy/MCPProxy/API/APIClient.swift +++ b/native/macos/MCPProxy/MCPProxy/API/APIClient.swift @@ -314,6 +314,11 @@ actor APIClient { return response.refs } + /// Keyring capability for add forms. The response has no secret values. + func keyringAvailability() async throws -> KeyringAvailability { + try await fetchWrapped(path: "/api/v1/secrets/config") + } + /// Delete a keyring secret via `DELETE /api/v1/secrets/{name}?type=keyring`. /// Used to roll back a secret this session's own Add Server flow just /// wrote, when the add itself then fails (FR-065) — never a pre-existing diff --git a/native/macos/MCPProxy/MCPProxy/API/CatalogModels.swift b/native/macos/MCPProxy/MCPProxy/API/CatalogModels.swift index 907675108..9cead7ab9 100644 --- a/native/macos/MCPProxy/MCPProxy/API/CatalogModels.swift +++ b/native/macos/MCPProxy/MCPProxy/API/CatalogModels.swift @@ -107,6 +107,18 @@ struct SecretRefsResponse: Codable { let count: Int? } +/// The only keyring fields the Catalog form needs from `/secrets/config`. +/// Values and configured references deliberately stay outside this DTO. +struct KeyringAvailability: Codable, Equatable { + let keyringAvailable: Bool + let keyringReason: String? + + enum CodingKeys: String, CodingKey { + case keyringAvailable = "keyring_available" + case keyringReason = "keyring_reason" + } +} + // MARK: - Import preview (Spec 109 FR-064), content-based // // `POST /api/v1/servers/import/json?preview=true` — the Paste tab's "detect diff --git a/native/macos/MCPProxy/MCPProxy/API/Models.swift b/native/macos/MCPProxy/MCPProxy/API/Models.swift index 2c7f516f6..62334a20f 100644 --- a/native/macos/MCPProxy/MCPProxy/API/Models.swift +++ b/native/macos/MCPProxy/MCPProxy/API/Models.swift @@ -421,6 +421,9 @@ struct IsolationDefaultsStatus: Codable, Equatable { struct ServerStatus: Codable, Identifiable, Equatable { let id: String let name: String + /// Catalog provenance, used to resolve an existing Added/Open card without + /// exposing configured server names in the catalog DTO itself. + let sourceRegistryID: String? let url: String? let command: String? let args: [String]? @@ -467,6 +470,7 @@ struct ServerStatus: Codable, Identifiable, Equatable { enum CodingKeys: String, CodingKey { case id, name, url, command, args + case sourceRegistryID = "source_registry_id" case workingDir = "working_dir" case headers case env diff --git a/native/macos/MCPProxy/MCPProxy/Models/SecretFieldResolver.swift b/native/macos/MCPProxy/MCPProxy/Models/SecretFieldResolver.swift index 520d446ba..0af91149d 100644 --- a/native/macos/MCPProxy/MCPProxy/Models/SecretFieldResolver.swift +++ b/native/macos/MCPProxy/MCPProxy/Models/SecretFieldResolver.swift @@ -82,10 +82,11 @@ enum SecretFieldResolver { // One taken-name check up front, then tracked locally as this call // writes its own refs, so two fields in the same call that would // otherwise compute the same name get -2, not a silent collision. - var taken: Set = [] - if let refs = try? await client.getSecretRefs() { - taken = Set(refs.filter { $0.type == "keyring" }.map(\.name)) - } + // Never infer an empty set after a failed list. POST /secrets can + // overwrite, so proceeding after this GET fails could destroy a + // credential the current add did not create. + let refs = try await client.getSecretRefs() + var taken = Set(refs.filter { $0.type == "keyring" }.map(\.name)) var written: [String: String] = [:] // field.id -> ref var writtenRefs: [String] = [] diff --git a/native/macos/MCPProxy/MCPProxy/Views/AddServerView.swift b/native/macos/MCPProxy/MCPProxy/Views/AddServerView.swift index e5874609e..36ac8d680 100644 --- a/native/macos/MCPProxy/MCPProxy/Views/AddServerView.swift +++ b/native/macos/MCPProxy/MCPProxy/Views/AddServerView.swift @@ -28,14 +28,16 @@ struct AddServerView: View { /// Narrows the Catalog tab only (mirrors the Web UI's `?source=`); never /// selects a tab itself (FR-062). let catalogSourceFilter: String? + let onOpenServer: (ServerStatus) -> Void private var apiClient: APIClient? { appState.apiClient } - init(appState: AppState, isPresented: Binding, initialTab: AddServerTab = .catalog, catalogSourceFilter: String? = nil) { + init(appState: AppState, isPresented: Binding, initialTab: AddServerTab = .catalog, catalogSourceFilter: String? = nil, onOpenServer: @escaping (ServerStatus) -> Void = { _ in }) { self.appState = appState self._isPresented = isPresented self._selectedTab = State(initialValue: initialTab) self.catalogSourceFilter = catalogSourceFilter + self.onOpenServer = onOpenServer } var body: some View { @@ -70,7 +72,7 @@ struct AddServerView: View { switch selectedTab { case .catalog: - CatalogView(appState: appState, sourceFilter: catalogSourceFilter, onAdded: { _ in isPresented = false }) + CatalogView(appState: appState, sourceFilter: catalogSourceFilter, onOpenServer: onOpenServer) case .paste: PasteServerView(appState: appState, onAdded: { _ in isPresented = false }) case .importConfig: diff --git a/native/macos/MCPProxy/MCPProxy/Views/CatalogView.swift b/native/macos/MCPProxy/MCPProxy/Views/CatalogView.swift index 21cd77a1f..56d8dc82a 100644 --- a/native/macos/MCPProxy/MCPProxy/Views/CatalogView.swift +++ b/native/macos/MCPProxy/MCPProxy/Views/CatalogView.swift @@ -17,7 +17,6 @@ struct CatalogView: View { /// `?source=` query param, which narrows the Catalog tab only — it never /// selects a tab, FR-062). Nil searches every enabled source. var sourceFilter: String? - let onAdded: (String) -> Void @Environment(\.fontScale) var fontScale @@ -39,6 +38,9 @@ struct CatalogView: View { @State private var pendingFields: [SecretFieldInput] = [] @State private var addError: String? @State private var confirming = false + @State private var keyringAvailable = false + @State private var keyringReason = "Checking OS keyring availability…" + let onOpenServer: (ServerStatus) -> Void private var apiClient: APIClient? { appState.apiClient } @@ -65,7 +67,10 @@ struct CatalogView: View { } .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) .accessibilityIdentifier("catalog-view") - .task { await search() } + .task { + await loadKeyringAvailability() + await search() + } .sheet(item: $pendingResult) { result in secretsSheet(result) } @@ -162,7 +167,11 @@ struct CatalogView: View { Spacer() if added { Button("Added ✓ · Open") { - if let addedName { openServer(addedName) } + if let addedName { + Task { await openAddedServer(named: addedName) } + } else { + Task { await openPreviouslyAdded(r) } + } } .controlSize(.small) .accessibilityIdentifier("catalog-added-\(key)") @@ -216,7 +225,7 @@ struct CatalogView: View { .foregroundStyle(.secondary) ForEach($pendingFields) { $field in - SecretFieldToggleView(name: field.name, value: $field.value, mode: $field.mode) + SecretFieldToggleView(name: field.name, value: $field.value, mode: $field.mode, keyringAvailable: keyringAvailable, keyringReason: keyringReason) } if let addError { @@ -236,7 +245,7 @@ struct CatalogView: View { if confirming { ProgressView().controlSize(.small) } else { Text("Add to MCPProxy") } } .buttonStyle(.borderedProminent) - .disabled(confirming || !allPendingValuesFilled) + .disabled(confirming || !allPendingValuesFilled || hasUnavailableSecret) .accessibilityIdentifier("catalog-secrets-confirm") } } @@ -248,6 +257,10 @@ struct CatalogView: View { pendingFields.allSatisfy { !$0.value.trimmingCharacters(in: .whitespaces).isEmpty } } + private var hasUnavailableSecret: Bool { + !keyringAvailable && pendingFields.contains(where: { $0.mode == .secret }) + } + // MARK: - Search private func scheduleSearch() { @@ -277,6 +290,18 @@ struct CatalogView: View { } } + private func loadKeyringAvailability() async { + guard let client = apiClient else { return } + do { + let status = try await client.keyringAvailability() + keyringAvailable = status.keyringAvailable + keyringReason = status.keyringReason ?? "OS keyring unavailable" + } catch { + keyringAvailable = false + keyringReason = "Could not verify OS keyring availability" + } + } + // MARK: - Add private func handleAddTapped(_ result: CatalogResult) { @@ -329,7 +354,10 @@ struct CatalogView: View { if outcome.success { let assignedName = outcome.serverName ?? result.title addedNames[key] = assignedName - onAdded(assignedName) + // Adding succeeded independently of navigation. Do not let a + // refresh failure make confirmAdd roll back secrets now referenced + // by the persisted server configuration. + await openAddedServer(named: assignedName) return true } addError = outcome.message ?? "Failed to add server" @@ -337,10 +365,49 @@ struct CatalogView: View { } /// Spec 109 FR-063: "Added ✓ · Open" opens the server it just added. - private func openServer(_ name: String) { - NotificationCenter.default.post(name: .switchToServers, object: nil) - DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { - NotificationCenter.default.post(name: .showServerDetail, object: name) + private func openPreviouslyAdded(_ result: CatalogResult) async { + guard let client = apiClient else { return } + do { + let refreshed = try await client.servers() + let target = catalogTarget(result.install) + let matches = refreshed.filter { + serverTarget($0) == target && ($0.sourceRegistryID == result.source || $0.sourceRegistryID == nil) + } + guard matches.count == 1, let server = matches.first else { + addError = matches.isEmpty + ? "This catalog entry is marked added, but its installed server is not visible. Open it from Servers." + : "More than one installed server matches this catalog entry. Open the intended server from Servers." + return + } + appState.updateServers(refreshed) + onOpenServer(server) + } catch { + addError = "Could not resolve the installed server. Refresh and try again." } } + + private func openAddedServer(named name: String) async { + guard let client = apiClient else { return } + do { + let refreshed = try await client.servers() + appState.updateServers(refreshed) + guard let server = refreshed.first(where: { $0.name == name }) else { + addError = "Added to MCPProxy, but it is not visible yet. Open it from Servers." + return + } + onOpenServer(server) + } catch { + addError = "Added to MCPProxy, but it could not be opened. Open it from Servers." + } + } + + private func catalogTarget(_ install: CatalogInstall) -> String { + if let url = install.url { return "url:\(url)" } + return "stdio:\(install.command ?? "")\u{0}\((install.args ?? []).joined(separator: "\u{0}"))" + } + + private func serverTarget(_ server: ServerStatus) -> String { + if let url = server.url { return "url:\(url)" } + return "stdio:\(server.command ?? "")\u{0}\((server.args ?? []).joined(separator: "\u{0}"))" + } } diff --git a/native/macos/MCPProxy/MCPProxy/Views/PasteServerView.swift b/native/macos/MCPProxy/MCPProxy/Views/PasteServerView.swift index 29d128845..a4ea18bd1 100644 --- a/native/macos/MCPProxy/MCPProxy/Views/PasteServerView.swift +++ b/native/macos/MCPProxy/MCPProxy/Views/PasteServerView.swift @@ -28,6 +28,8 @@ struct PasteServerView: View { // re-parses the same input the preview was computed from, even if a // debounced re-preview for newer text hasn't landed yet. @State private var previewRawContent = "" + @State private var keyringAvailable = false + @State private var keyringReason = "Checking OS keyring availability…" private var apiClient: APIClient? { appState.apiClient } @@ -60,6 +62,7 @@ struct PasteServerView: View { } .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) .accessibilityIdentifier("paste-server") + .task { await loadKeyringAvailability() } } @ViewBuilder @@ -97,7 +100,7 @@ struct PasteServerView: View { } ForEach($fields) { $field in - SecretFieldToggleView(name: field.name, value: $field.value, mode: $field.mode) + SecretFieldToggleView(name: field.name, value: $field.value, mode: $field.mode, keyringAvailable: keyringAvailable, keyringReason: keyringReason) } if let addError { @@ -113,7 +116,7 @@ struct PasteServerView: View { if adding { ProgressView().controlSize(.small) } else { Text("Add to MCPProxy") } } .buttonStyle(.borderedProminent) - .disabled(adding) + .disabled(adding || hasUnavailableSecret) .accessibilityIdentifier("paste-add-button") } .padding() @@ -121,6 +124,22 @@ struct PasteServerView: View { } } + private var hasUnavailableSecret: Bool { + !keyringAvailable && fields.contains(where: { $0.mode == .secret }) + } + + private func loadKeyringAvailability() async { + guard let client = apiClient else { return } + do { + let status = try await client.keyringAvailability() + keyringAvailable = status.keyringAvailable + keyringReason = status.keyringReason ?? "OS keyring unavailable" + } catch { + keyringAvailable = false + keyringReason = "Could not verify OS keyring availability" + } + } + private func badge(_ text: String) -> some View { Text(text) .font(.scaled(.caption2, scale: fontScale)) diff --git a/native/macos/MCPProxy/MCPProxy/Views/SecretFieldToggleView.swift b/native/macos/MCPProxy/MCPProxy/Views/SecretFieldToggleView.swift index 9185ba891..89567f511 100644 --- a/native/macos/MCPProxy/MCPProxy/Views/SecretFieldToggleView.swift +++ b/native/macos/MCPProxy/MCPProxy/Views/SecretFieldToggleView.swift @@ -12,6 +12,8 @@ struct SecretFieldToggleView: View { let name: String @Binding var value: String @Binding var mode: SecretFieldInput.Mode + let keyringAvailable: Bool + let keyringReason: String @Environment(\.fontScale) var fontScale var body: some View { @@ -25,11 +27,18 @@ struct SecretFieldToggleView: View { Text("Value").tag(SecretFieldInput.Mode.value) Text("Secret").tag(SecretFieldInput.Mode.secret) } + .disabled(!keyringAvailable && mode == .secret) .pickerStyle(.segmented) .labelsHidden() .frame(width: 140) .accessibilityIdentifier("secret-toggle-mode-\(name)") } + if !keyringAvailable { + Text(keyringReason.isEmpty ? "OS keyring unavailable" : keyringReason) + .font(.scaled(.caption2, scale: fontScale)) + .foregroundStyle(.orange) + .accessibilityIdentifier("secret-toggle-keyring-unavailable-\(name)") + } if mode == .secret { SecureField("Secret value", text: $value) .textFieldStyle(.roundedBorder) diff --git a/native/macos/MCPProxy/MCPProxy/Views/ServersView.swift b/native/macos/MCPProxy/MCPProxy/Views/ServersView.swift index 11e702059..14bb1eef6 100644 --- a/native/macos/MCPProxy/MCPProxy/Views/ServersView.swift +++ b/native/macos/MCPProxy/MCPProxy/Views/ServersView.swift @@ -20,7 +20,7 @@ struct ServersView: View { @State private var selectedServer: ServerStatus? @State private var selectedServerInitialTab: ServerDetailTab = .tools @State private var showAddServer = false - @State private var addServerInitialTab: AddServerTab = .manual + @State private var addServerInitialTab: AddServerTab = .catalog var body: some View { VStack(alignment: .leading, spacing: 0) { @@ -36,7 +36,7 @@ struct ServersView: View { } } .sheet(isPresented: $showAddServer) { - AddServerView(appState: appState, isPresented: $showAddServer, initialTab: addServerInitialTab) + AddServerView(appState: appState, isPresented: $showAddServer, initialTab: addServerInitialTab, onOpenServer: openServerAfterAddSheetDismisses) .id(addServerInitialTab) } // Review finding (this round): these two `.onReceive` handlers used to @@ -57,7 +57,7 @@ struct ServersView: View { if let tab = notification.object as? AddServerTab { addServerInitialTab = tab } else { - addServerInitialTab = .manual + addServerInitialTab = .catalog } showAddServer = true } @@ -82,6 +82,12 @@ struct ServersView: View { } } + private func openServerAfterAddSheetDismisses(_ server: ServerStatus) { + showAddServer = false + selectedServerInitialTab = .tools + selectedServer = server + } + @ViewBuilder private var serverListView: some View { VStack(alignment: .leading, spacing: 0) { diff --git a/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift b/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift index b61405832..de2eb7dc9 100644 --- a/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift @@ -243,4 +243,24 @@ final class CatalogTests: XCTestCase { let header = SecretFieldInput(name: "API_KEY", kind: .header, value: "b", mode: .secret) XCTAssertNotEqual(env.id, header.id) } + + /// A failed GET must stop before POST /secrets. This guards the data-loss + /// case where POST would overwrite a canonical name we failed to list. + func testSecretResolverDoesNotPostAfterSecretRefsFailure() async throws { + ResolverStubURLProtocol.reset() + ResolverStubURLProtocol.handler = { request in + if request.url?.path == "/api/v1/secrets/refs" { + return (500, Data("{\"success\":false,\"error\":\"temporary failure\"}".utf8)) + } + return (200, Data("{\"success\":true,\"data\":{}}".utf8)) + } + let client = ResolverStubURLProtocol.makeClient() + do { + _ = try await SecretFieldResolver.resolve(client: client, serverName: "github", fields: [ + SecretFieldInput(name: "TOKEN", value: "not-a-real-secret", mode: .secret), + ]) + XCTFail("expected getSecretRefs failure") + } catch {} + XCTAssertEqual(ResolverStubURLProtocol.requests.map(\.httpMethod), ["GET"]) + } } diff --git a/native/macos/MCPProxy/MCPProxyTests/ResolverStubURLProtocol.swift b/native/macos/MCPProxy/MCPProxyTests/ResolverStubURLProtocol.swift new file mode 100644 index 000000000..76a5d7a2d --- /dev/null +++ b/native/macos/MCPProxy/MCPProxyTests/ResolverStubURLProtocol.swift @@ -0,0 +1,27 @@ +import Foundation +@testable import MCPProxy + +final class ResolverStubURLProtocol: URLProtocol { + static var handler: ((URLRequest) -> (Int, Data))? + static var requests: [URLRequest] = [] + + static func reset() { handler = nil; requests = [] } + + static func makeClient() -> APIClient { + let config = URLSessionConfiguration.ephemeral + config.protocolClasses = [ResolverStubURLProtocol.self] + return APIClient(session: URLSession(configuration: config), baseURL: "http://resolver.test") + } + + override class func canInit(with request: URLRequest) -> Bool { true } + override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } + override func startLoading() { + Self.requests.append(request) + let (status, body) = Self.handler?(request) ?? (500, Data()) + let response = HTTPURLResponse(url: request.url!, statusCode: status, httpVersion: nil, headerFields: nil)! + client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed) + client?.urlProtocol(self, didLoad: body) + client?.urlProtocolDidFinishLoading(self) + } + override func stopLoading() {} +} From 2fd27b7d1dad9b5ae9311dfc495787f18e328082 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 28 Sep 2026 09:04:23 +0300 Subject: [PATCH 2/6] fix(catalog): allow value mode without keyring --- .../Views/SecretFieldToggleView.swift | 20 +++++++++++++++++-- .../MCPProxy/MCPProxyTests/CatalogTests.swift | 12 +++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/native/macos/MCPProxy/MCPProxy/Views/SecretFieldToggleView.swift b/native/macos/MCPProxy/MCPProxy/Views/SecretFieldToggleView.swift index 89567f511..f4706e604 100644 --- a/native/macos/MCPProxy/MCPProxy/Views/SecretFieldToggleView.swift +++ b/native/macos/MCPProxy/MCPProxy/Views/SecretFieldToggleView.swift @@ -16,6 +16,23 @@ struct SecretFieldToggleView: View { let keyringReason: String @Environment(\.fontScale) var fontScale + /// Value mode is always safe: it lets a secret-like field recover from a + /// keyring outage. Secret mode is only selectable while the keyring can + /// accept the value, preserving the add-time fail-closed guard. + static func canSelect(mode: SecretFieldInput.Mode, keyringAvailable: Bool) -> Bool { + mode == .value || keyringAvailable + } + + private var modeSelection: Binding { + Binding( + get: { mode }, + set: { proposedMode in + guard Self.canSelect(mode: proposedMode, keyringAvailable: keyringAvailable) else { return } + mode = proposedMode + } + ) + } + var body: some View { VStack(alignment: .leading, spacing: 4) { HStack { @@ -23,11 +40,10 @@ struct SecretFieldToggleView: View { .font(.scaledMonospaced(.caption, scale: fontScale)) .foregroundStyle(.secondary) Spacer() - Picker("", selection: $mode) { + Picker("", selection: modeSelection) { Text("Value").tag(SecretFieldInput.Mode.value) Text("Secret").tag(SecretFieldInput.Mode.secret) } - .disabled(!keyringAvailable && mode == .secret) .pickerStyle(.segmented) .labelsHidden() .frame(width: 140) diff --git a/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift b/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift index de2eb7dc9..dd5de6f81 100644 --- a/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift @@ -153,6 +153,18 @@ final class CatalogTests: XCTestCase { } } + // MARK: - Secret toggle availability (FR-065) + + /// A field detected as secret-like begins in Secret mode. If the OS + /// keyring is unavailable, the user must still be able to move it back to + /// Value mode so the Catalog/Paste Add button is no longer trapped behind + /// the secret-mode keyring gate. The unavailable keyring must still stop a + /// new Secret selection. + func testUnavailableKeyringAllowsValueSelectionButRejectsSecretSelection() { + XCTAssertTrue(SecretFieldToggleView.canSelect(mode: .value, keyringAvailable: false)) + XCTAssertFalse(SecretFieldToggleView.canSelect(mode: .secret, keyringAvailable: false)) + } + // MARK: - Secret refs decode (FR-065 taken-name check) func testDecodesSecretRefsResponse() throws { From ac634591f39712170bd4f35f0ef7431f3d01b573 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 28 Sep 2026 11:12:21 +0300 Subject: [PATCH 3/6] Fix catalog Added Open resolution --- frontend/src/components/CatalogSearch.vue | 9 ++- frontend/src/types/api.ts | 2 + .../tests/unit/add-server-catalog.spec.ts | 24 ++++++++ internal/httpapi/catalog.go | 56 ++++++++++++------- internal/httpapi/catalog_test.go | 52 +++++++++++++++++ internal/registries/catalog.go | 5 ++ .../MCPProxy/MCPProxy/API/CatalogModels.swift | 5 ++ .../MCPProxy/MCPProxy/Views/CatalogView.swift | 22 +++++++- .../MCPProxy/MCPProxyTests/CatalogTests.swift | 10 ++++ .../CatalogViewOpenFeedbackTests.swift | 27 +++++++++ .../contracts/rest-api.md | 2 +- .../data-model.md | 3 +- 12 files changed, 194 insertions(+), 23 deletions(-) create mode 100644 native/macos/MCPProxy/MCPProxyTests/CatalogViewOpenFeedbackTests.swift diff --git a/frontend/src/components/CatalogSearch.vue b/frontend/src/components/CatalogSearch.vue index 9f796d22a..3b1ed458c 100644 --- a/frontend/src/components/CatalogSearch.vue +++ b/frontend/src/components/CatalogSearch.vue @@ -292,6 +292,13 @@ function serverTarget(server: { url?: string; command?: string; args?: string[] } async function openPreviouslyAdded(result: CatalogResult): Promise { + // GET /servers redacts credential-bearing URL query values and argv, so an + // exact install-target comparison can fail even though the server-authoritative + // catalog response already established a unique visible match. + if (result.added_server_name) { + await router.push(serverDetailPath(result.added_server_name)) + return + } const response = await api.getServers() if (!response.success || !response.data) { error.value = response.error || 'Could not resolve the installed server. Refresh and try again.' @@ -307,7 +314,7 @@ async function openPreviouslyAdded(result: CatalogResult): Promise { return } error.value = matches.length === 0 - ? 'This catalog entry is marked added, but its installed server is not visible. Open it from Servers.' + ? 'This catalog entry is marked added, but MCPProxy could not identify one visible installed server. Open it from Servers.' : 'More than one installed server matches this catalog entry. Open the intended server from Servers.' } diff --git a/frontend/src/types/api.ts b/frontend/src/types/api.ts index a16619ca6..668b7d99f 100644 --- a/frontend/src/types/api.ts +++ b/frontend/src/types/api.ts @@ -879,6 +879,8 @@ export interface CatalogResult { required_inputs?: CatalogInput[] source_code_url?: string added: boolean + /** Unique visible installed server selected by the backend before redaction. */ + added_server_name?: string } export interface CatalogSourceError { diff --git a/frontend/tests/unit/add-server-catalog.spec.ts b/frontend/tests/unit/add-server-catalog.spec.ts index 4ceccd607..6173c9b50 100644 --- a/frontend/tests/unit/add-server-catalog.spec.ts +++ b/frontend/tests/unit/add-server-catalog.spec.ts @@ -97,6 +97,30 @@ describe('CatalogSearch', () => { expect(router.currentRoute.value.path).toBe('/servers/installed-github') }) + it('opens a prior-session credential-bearing Added/Open card from the server-authoritative name', async () => { + vi.mocked(api.catalogSearch).mockResolvedValue({ + success: true, + data: { + query: '', + results: [], + sections: { official: [githubResult({ + added: true, + added_server_name: 'installed-github-with-secret', + install: { url: 'https://api.githubcopilot.com/mcp/?access_token=catalog-value' }, + })], popular: [] }, + unavailable: [], + }, + }) + const wrapper = await mountCatalog() + await wrapper.find('[data-test="catalog-add-official-io.github.github/github-mcp-server"]').trigger('click') + await flushPromises() + + expect(router.currentRoute.value.path).toBe('/servers/installed-github-with-secret') + // GET /servers redacts credential-bearing URLs, so it is not a safe join + // source. The authoritative catalog name makes no second request needed. + expect(api.getServers).not.toHaveBeenCalled() + }) + it('does not navigate an ambiguous prior-session Added/Open card', async () => { vi.mocked(api.catalogSearch).mockResolvedValue({ success: true, diff --git a/internal/httpapi/catalog.go b/internal/httpapi/catalog.go index e843aa918..3568557bc 100644 --- a/internal/httpapi/catalog.go +++ b/internal/httpapi/catalog.go @@ -56,7 +56,7 @@ func (s *Server) handleCatalogSearch(w http.ResponseWriter, r *http.Request) { // source for one of the truncated top-`limit` slots. hits, sections, unavailable := registries.SearchAll(r.Context(), q, tag, limit, registries.SearchOptions{Source: source}) - added := s.catalogAddedPredicate(r.Context()) + added := s.catalogAddedResolver(r.Context()) resp := catalogSearchResponse{ Query: q, @@ -81,22 +81,25 @@ func (s *Server) handleCatalogSearch(w http.ResponseWriter, r *http.Request) { s.writeSuccess(w, resp) } -func toCatalogResults(hits []registries.CatalogHit, added func(registries.CatalogHit) bool) []registries.CatalogResult { +func toCatalogResults(hits []registries.CatalogHit, added func(registries.CatalogHit) (bool, string)) []registries.CatalogResult { out := make([]registries.CatalogResult, 0, len(hits)) for _, h := range hits { - out = append(out, registries.ToCatalogResult(h, added(h))) + isAdded, addedServerName := added(h) + result := registries.ToCatalogResult(h, isAdded) + result.AddedServerName = addedServerName + out = append(out, result) } return out } -// catalogAddedPredicate returns a function reporting whether a catalog hit is -// already configured, joined only against the servers visible to ctx's caller -// (FR-007, contracts/rest-api.md#catalog): "added" is computed only over -// servers passing CanEnumerateServer, so a scoped caller never learns from -// "added" that an out-of-scope server exists. A configured server with a -// source_registry_id matches on (source, install target); one without (a -// manual add) matches on install target alone. -func (s *Server) catalogAddedPredicate(ctx context.Context) func(registries.CatalogHit) bool { +// catalogAddedResolver returns a function reporting whether a catalog hit is +// already configured and, only for a unique visible match, that server's name. +// The join uses raw configuration before GET /servers redacts credential-bearing +// URLs and arguments. It considers only servers visible to ctx's caller (FR-007), +// so it cannot reveal an out-of-scope server name. A configured server with a +// source_registry_id matches on (source, install target); one without (a manual +// add) matches on install target alone. +func (s *Server) catalogAddedResolver(ctx context.Context) func(registries.CatalogHit) (bool, string) { servers := s.getVisibleServersForCatalog(ctx) // byRegistryAndTarget indexes servers that declare which registry they @@ -106,26 +109,41 @@ func (s *Server) catalogAddedPredicate(ctx context.Context) func(registries.Cata // byTargetOnly: without this split it would falsely read added:true for // every OTHER source whose entry happens to share the same install // target (contracts/rest-api.md#catalog "added"). - byRegistryAndTarget := make(map[string]bool, len(servers)) - byTargetOnly := make(map[string]bool, len(servers)) + byRegistryAndTarget := make(map[string][]string, len(servers)) + byTargetOnly := make(map[string][]string, len(servers)) for _, srv := range servers { target := catalogInstallTargetForServer(srv) if srv.SourceRegistryID == "" { - byTargetOnly[target] = true + byTargetOnly[target] = append(byTargetOnly[target], srv.Name) } else { - byRegistryAndTarget[srv.SourceRegistryID+"\x00"+target] = true + key := srv.SourceRegistryID + "\x00" + target + byRegistryAndTarget[key] = append(byRegistryAndTarget[key], srv.Name) } } - return func(h registries.CatalogHit) bool { + return func(h registries.CatalogHit) (bool, string) { // added is never itself computed from the DTO's Added field (still // false here) — only its Install target, which is pure derived data // from the catalog entry. target := registries.CatalogInstallTarget(registries.ToCatalogResult(h, false).Install) - if byRegistryAndTarget[h.Source+"\x00"+target] { - return true + names := append([]string(nil), byRegistryAndTarget[h.Source+"\x00"+target]...) + names = append(names, byTargetOnly[target]...) + if len(names) == 0 { + return false, "" } - return byTargetOnly[target] + // Names are normally unique configuration keys. De-duplicate defensively + // so a malformed legacy configuration cannot turn one logical server into + // a false ambiguity. + unique := make(map[string]struct{}, len(names)) + for _, name := range names { + unique[name] = struct{}{} + } + if len(unique) == 1 { + for name := range unique { + return true, name + } + } + return true, "" } } diff --git a/internal/httpapi/catalog_test.go b/internal/httpapi/catalog_test.go index 0df15e2cb..1be5873e5 100644 --- a/internal/httpapi/catalog_test.go +++ b/internal/httpapi/catalog_test.go @@ -146,6 +146,48 @@ func TestCatalogSearch_AddedRequiresMatchingSourceForRegistryAdd(t *testing.T) { "lookalike-tool (source=other) shares delta's install target but must NOT read added=true — delta was added from a different source") } +// TestCatalogSearch_AddedServerNameIsVisibleScopedAndUnique pins the +// server-authoritative join used by Added/Open. Server status redacts URL query +// values and command arguments, so clients cannot safely reproduce this join +// after GET /servers. The catalog response may name the matching server only +// when exactly one server visible to the caller matches it. +func TestCatalogSearch_AddedServerNameIsVisibleScopedAndUnique(t *testing.T) { + withCatalogFixtureRegistry(t) + ctrl := &scopeController{cfg: scopeFixtureConfig(false), servers: catalogFixtureServers(), withManagement: true} + srv, token := scopedAgentServer(t, ctrl, []string{"gamma"}) + + adminRec := scopeGet(t, srv, "/api/v1/catalog/search?q=tool", scopeAdminAPIKey) + require.Equal(t, http.StatusOK, adminRec.Code, adminRec.Body.String()) + adminResults := catalogDecodeResults(t, adminRec) + assert.Equal(t, "gamma", catalogAddedServerNameFor(adminResults, "gamma-tool")) + assert.Equal(t, "delta", catalogAddedServerNameFor(adminResults, "delta-tool")) + + // The scoped caller may only learn the name of gamma, which it can already + // enumerate. Delta remains neither added nor name-resolvable. + agentRec := scopeGet(t, srv, "/api/v1/catalog/search?q=tool", token) + require.Equal(t, http.StatusOK, agentRec.Code, agentRec.Body.String()) + agentResults := catalogDecodeResults(t, agentRec) + assert.Equal(t, "gamma", catalogAddedServerNameFor(agentResults, "gamma-tool")) + assert.Empty(t, catalogAddedServerNameFor(agentResults, "delta-tool")) + + // A second matching visible server keeps added=true but intentionally omits + // the target name. The UI must ask the user to choose from Servers rather + // than silently opening either server. + ctrl.servers = append(ctrl.servers, contracts.Server{ + ID: "delta-copy", + Name: "delta-copy", + Command: "npx", + Args: []string{"delta-server"}, + SourceRegistryID: "official", + Enabled: true, + }) + ambiguousRec := scopeGet(t, srv, "/api/v1/catalog/search?q=tool", scopeAdminAPIKey) + require.Equal(t, http.StatusOK, ambiguousRec.Code, ambiguousRec.Body.String()) + ambiguousResults := catalogDecodeResults(t, ambiguousRec) + assert.True(t, catalogAddedFor(ambiguousResults, "delta-tool")) + assert.Empty(t, catalogAddedServerNameFor(ambiguousResults, "delta-tool")) +} + func catalogDecodeResults(t *testing.T, rec *httptest.ResponseRecorder) []map[string]interface{} { t.Helper() data := scopeDecodeData(t, rec) @@ -170,6 +212,16 @@ func catalogAddedFor(results []map[string]interface{}, id string) bool { return false } +func catalogAddedServerNameFor(results []map[string]interface{}, id string) string { + for _, r := range results { + if r["id"] == id { + name, _ := r["added_server_name"].(string) + return name + } + } + return "" +} + // TestCatalogSearch_AddedScopedForNonAdminUserContext pins that the FR-007 // scoping is not agent-token-specific: a non-admin AuthTypeUser session // (server edition's OAuth user identity, e.g. a tenant) is scoped by the same diff --git a/internal/registries/catalog.go b/internal/registries/catalog.go index 53b2429c1..52abfdba4 100644 --- a/internal/registries/catalog.go +++ b/internal/registries/catalog.go @@ -74,6 +74,11 @@ type CatalogResult struct { RequiredInputs []CatalogInput `json:"required_inputs,omitempty"` SourceCodeURL string `json:"source_code_url,omitempty"` Added bool `json:"added"` + // AddedServerName identifies the one visible installed server that made + // Added true. It is intentionally omitted when no unique match exists. + // This lets clients open a credential-bearing install without attempting to + // join against redacted GET /servers fields. + AddedServerName string `json:"added_server_name,omitempty"` } // CatalogSections groups the empty-query landing results (FR-060): the diff --git a/native/macos/MCPProxy/MCPProxy/API/CatalogModels.swift b/native/macos/MCPProxy/MCPProxy/API/CatalogModels.swift index 9cead7ab9..9538f1ae3 100644 --- a/native/macos/MCPProxy/MCPProxy/API/CatalogModels.swift +++ b/native/macos/MCPProxy/MCPProxy/API/CatalogModels.swift @@ -53,11 +53,16 @@ struct CatalogResult: Codable, Identifiable, Equatable { let requiredInputs: [CatalogInput]? let sourceCodeURL: String? let added: Bool + /// The server-authoritative unique visible match for an Added card. This + /// avoids comparing the catalog install target with redacted server URLs + /// or command arguments after a prior session. + let addedServerName: String? enum CodingKeys: String, CodingKey { case source, id, title, publisher, verified, official, popularity, description, transport, install, added case requiredInputs = "required_inputs" case sourceCodeURL = "source_code_url" + case addedServerName = "added_server_name" } } diff --git a/native/macos/MCPProxy/MCPProxy/Views/CatalogView.swift b/native/macos/MCPProxy/MCPProxy/Views/CatalogView.swift index 56d8dc82a..f9e0dc11a 100644 --- a/native/macos/MCPProxy/MCPProxy/Views/CatalogView.swift +++ b/native/macos/MCPProxy/MCPProxy/Views/CatalogView.swift @@ -63,6 +63,19 @@ struct CatalogView: View { .accessibilityIdentifier("catalog-unavailable-notice") } + // Navigation failures can happen without opening the secrets + // sheet (for a prior-session Added/Open card), or after the user + // dismisses it. Keep the actionable error in the catalog itself + // whenever the sheet is not currently presenting it. + if let addError, pendingResult == nil { + Text(addError) + .font(.scaled(.caption, scale: fontScale)) + .foregroundStyle(.red) + .padding(.horizontal) + .padding(.bottom, 6) + .accessibilityIdentifier("catalog-add-error") + } + content } .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) @@ -367,6 +380,13 @@ struct CatalogView: View { /// Spec 109 FR-063: "Added ✓ · Open" opens the server it just added. private func openPreviouslyAdded(_ result: CatalogResult) async { guard let client = apiClient else { return } + // The catalog handler performed this join over raw visible + // configuration before GET /servers redacts credential-bearing query + // values and argv. Use its name only when it established uniqueness. + if let name = result.addedServerName { + await openAddedServer(named: name) + return + } do { let refreshed = try await client.servers() let target = catalogTarget(result.install) @@ -375,7 +395,7 @@ struct CatalogView: View { } guard matches.count == 1, let server = matches.first else { addError = matches.isEmpty - ? "This catalog entry is marked added, but its installed server is not visible. Open it from Servers." + ? "This catalog entry is marked added, but MCPProxy could not identify one visible installed server. Open it from Servers." : "More than one installed server matches this catalog entry. Open the intended server from Servers." return } diff --git a/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift b/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift index dd5de6f81..c50005930 100644 --- a/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/CatalogTests.swift @@ -51,6 +51,16 @@ final class CatalogTests: XCTestCase { XCTAssertTrue(result.added) } + func testDecodesServerAuthoritativeAddedName() throws { + let json = """ + {"source": "official", "id": "credentialed", "title": "Credentialed", "verified": true, "official": true, + "description": "d", "transport": "http", "install": {"url": "https://example.test/mcp?token=catalog-value"}, + "added": true, "added_server_name": "credentialed-installed"} + """ + let result = try decode(CatalogResult.self, from: json) + XCTAssertEqual(result.addedServerName, "credentialed-installed") + } + func testDecodesSearchResponseWithNullSections() throws { let json = """ {"query": "github", "results": [], "sections": null, diff --git a/native/macos/MCPProxy/MCPProxyTests/CatalogViewOpenFeedbackTests.swift b/native/macos/MCPProxy/MCPProxyTests/CatalogViewOpenFeedbackTests.swift new file mode 100644 index 000000000..15df1cf54 --- /dev/null +++ b/native/macos/MCPProxy/MCPProxyTests/CatalogViewOpenFeedbackTests.swift @@ -0,0 +1,27 @@ +import XCTest +@testable import MCPProxy + +/// The package has no SwiftUI interaction harness. Pin the two state branches +/// that make Added/Open failures actionable: errors outside a dismissed sheet, +/// and the server-authoritative name before the redacted-target fallback. +final class CatalogViewOpenFeedbackTests: XCTestCase { + func testCatalogViewKeepsNavigationErrorVisibleAfterSheetDismissal() throws { + let source = try catalogViewSource() + XCTAssertTrue(source.contains("if let addError, pendingResult == nil")) + XCTAssertTrue(source.contains("accessibilityIdentifier(\"catalog-add-error\")")) + } + + func testCatalogViewUsesUniqueServerNameBeforeRedactedTargetFallback() throws { + let source = try catalogViewSource() + let authoritative = try XCTUnwrap(source.range(of: "if let name = result.addedServerName")) + let fallback = try XCTUnwrap(source.range(of: "let target = catalogTarget(result.install)")) + XCTAssertLessThan(authoritative.lowerBound, fallback.lowerBound) + } + + private func catalogViewSource() throws -> String { + let testDirectory = URL(fileURLWithPath: #filePath).deletingLastPathComponent() + let source = testDirectory.deletingLastPathComponent() + .appendingPathComponent("MCPProxy/Views/CatalogView.swift") + return try String(contentsOf: source) + } +} diff --git a/specs/109-ux-navigation-consistency/contracts/rest-api.md b/specs/109-ux-navigation-consistency/contracts/rest-api.md index e872b87c6..2f0f06346 100644 --- a/specs/109-ux-navigation-consistency/contracts/rest-api.md +++ b/specs/109-ux-navigation-consistency/contracts/rest-api.md @@ -229,7 +229,7 @@ No new route: the Web UI and macOS write each secret with the existing `POST /se - Result objects are the `CatalogResult` **DTO** (data-model §9), built from the internal `CatalogHit` by `toCatalogResult`; the Spec 070 `registries.ServerEntry` JSON (`url`, `installCmd`, `registry`, `required_inputs[].secret`) is not renamed and keeps serving `GET /registries/{id}/servers` and MCP `search_servers` unchanged (codex round 3: embedding `ServerEntry` could not produce this example). Golden test T101a. - Empty `q` → `results: []`, `sections: {"official": [...], "popular": [...]}` (≤ 12 each). - Ranking (pure `registries.Rank`): `official` desc, `verified` desc, popularity desc (missing = 0), relevance desc, `title` asc, `id` asc. -- `added: true` when a configured server has `source_registry_id == source` **and** the same install target (`install.url`, or the command + args) as this result → surfaces render "Added ✓ · Open". The config does not store the registry's own server `id`, so `id` is not part of the join (data-model.md §9). A manually added server matches on the install target alone. +- `added: true` when a configured server has `source_registry_id == source` **and** the same install target (`install.url`, or the command + args) as this result → surfaces render "Added ✓ · Open". The config does not store the registry's own server `id`, so `id` is not part of the join (data-model.md §9). A manually added server matches on the install target alone. When exactly one matching server is visible to the caller, the optional `added_server_name` identifies it; clients use that authoritative name before any comparison with redacted `GET /servers` URL/argv fields. It is omitted for ambiguous matches. - Adding stays `POST /registries/{id}/servers/{serverId}/add` (Spec 070), always quarantined. ## Activity summary (additive) diff --git a/specs/109-ux-navigation-consistency/data-model.md b/specs/109-ux-navigation-consistency/data-model.md index 530001ea1..2db80a19a 100644 --- a/specs/109-ux-navigation-consistency/data-model.md +++ b/specs/109-ux-navigation-consistency/data-model.md @@ -155,6 +155,7 @@ type CatalogResult struct { RequiredInputs []CatalogInput `json:"required_inputs,omitempty"` // {name, description?, secret_like} ← RequiredInput{Name, Description, Secret}; secret_like = Secret OR the research D13 secret-like-name rule (the one function the import preview's SecretLike uses), so a registry that omits or falsifies isSecret on GITHUB_TOKEN still defaults to Secret (FR-065) SourceCodeURL string `json:"source_code_url,omitempty"` Added bool `json:"added"` + AddedServerName string `json:"added_server_name,omitempty"` // unique caller-visible installed match; omitted when ambiguous } type SearchOptions struct{ SourceTimeout time.Duration } // default 5 s; only tests set another value (T109a) func SearchAll(ctx, q, tag string, limit int, opts SearchOptions) (results []CatalogHit, sections *CatalogSections, unavailable []SourceError) @@ -162,7 +163,7 @@ func Rank(a, b CatalogHit, q string) bool // pure, deterministic func toCatalogResult(h CatalogHit, added bool) CatalogResult // REST only; golden-tested against the contracts/rest-api.md#catalog example ``` -`Added` is computed per caller: only configured servers the caller can enumerate (`visibleServers(ctx, …)`, FR-007) take part in the join, so a scoped caller never learns from `added` that an out-of-scope server is configured. `Added` is true when such a configured server has `source_registry_id == Source` (existing field, `config.go:756`) **and** the same install URL or command (the config does not record the registry's server id, so the install target is the join key). Manually added servers match on install URL or command alone. +`Added` is computed per caller: only configured servers the caller can enumerate (`visibleServers(ctx, …)`, FR-007) take part in the join, so a scoped caller never learns from `added` that an out-of-scope server is configured. `Added` is true when such a configured server has `source_registry_id == Source` (existing field, `config.go:756`) **and** the same install URL or command (the config does not record the registry's server id, so the install target is the join key). Manually added servers match on install URL or command alone. `AddedServerName` is sent only when that visible join has one matching server; clients use it to open credential-bearing installs without comparing the catalog target to redacted `GET /servers` values. ## 10. Token metrics (extended) — `contracts.ServerTokenMetrics` From ecfb060de7b650901620afd73f96707cc84a28dc Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 28 Sep 2026 11:57:23 +0300 Subject: [PATCH 4/6] Fix tray add server default tab --- .../macos/MCPProxy/MCPProxy/MCPProxyApp.swift | 4 +++- .../MCPProxyTests/MenuStructureTests.swift | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift index cb994143b..2bd7a110f 100644 --- a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift +++ b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift @@ -686,7 +686,9 @@ final class AppController: NSObject, NSApplicationDelegate, NSWindowDelegate, NS // Then post the showAddServer notification after the tab switch completes // and ServersView has fully registered its notification observer. DispatchQueue.main.asyncAfter(deadline: .now() + 0.8) { - NotificationCenter.default.post(name: .showAddServer, object: AddServerTab.manual) + // FR-062: this generic tray entry point follows the same + // catalog-first default as the Servers and Home add actions. + NotificationCenter.default.post(name: .showAddServer, object: AddServerTab.catalog) } } diff --git a/native/macos/MCPProxy/MCPProxyTests/MenuStructureTests.swift b/native/macos/MCPProxy/MCPProxyTests/MenuStructureTests.swift index 12133622c..55e4b26b5 100644 --- a/native/macos/MCPProxy/MCPProxyTests/MenuStructureTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/MenuStructureTests.swift @@ -118,6 +118,21 @@ final class MenuStructureTests: XCTestCase { XCTAssertTrue(addServer.target === controller) } + /// FR-062: the tray's generic Add Server action is another entry point to + /// the catalog-first add flow. It must not silently bypass discovery by + /// selecting Manual while every in-window generic add action selects + /// Catalog. + func testTrayAddServerPostsCatalogAsItsInitialTab() throws { + let source = try appControllerSource() + let start = try XCTUnwrap(source.range(of: "@objc private func showAddServer()")) + let end = try XCTUnwrap(source[start.lowerBound...].range(of: "\n // Inject our View menu items")) + let body = String(source[start.lowerBound.. String { + let testDirectory = URL(fileURLWithPath: #filePath).deletingLastPathComponent() + let source = testDirectory.deletingLastPathComponent() + .appendingPathComponent("MCPProxy/MCPProxyApp.swift") + return try String(contentsOf: source) + } } From 20433ed7b07e646cebb917bdd311e9c01b672999 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 28 Sep 2026 13:21:16 +0300 Subject: [PATCH 5/6] fix(ui): route add-server entry points through catalog (Spec 109-j) --- ROADMAP.md | 2 +- frontend/src/components/ManualServerForm.vue | 7 +- frontend/src/components/OnboardingWizard.vue | 17 +-- frontend/src/components/TopHeader.vue | 13 +- frontend/src/views/Home.vue | 27 +--- frontend/src/views/Servers.vue | 19 +-- .../unit/add-server-handoff-consumers.spec.ts | 117 ------------------ .../unit/dashboard-tenant-gating.spec.ts | 35 ++++++ .../manual-server-form-navigation.spec.ts | 44 +++++++ .../onboarding-wizard-servers-step.spec.ts | 42 +++++-- .../unit/servers-first-run-empty.spec.ts | 8 +- specs/109-ux-navigation-consistency/tasks.md | 3 +- 12 files changed, 138 insertions(+), 196 deletions(-) delete mode 100644 frontend/tests/unit/add-server-handoff-consumers.spec.ts create mode 100644 frontend/tests/unit/manual-server-form-navigation.spec.ts diff --git a/ROADMAP.md b/ROADMAP.md index e781f6836..ddf9d1930 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1036,4 +1036,4 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—` | [106-security-residual-fixes](./specs/106-security-residual-fixes/) | `shipped` | 18/19 (95%) | | [107-server-edition-sso-hardening](./specs/107-server-edition-sso-hardening/) | `shipped` | 126/126 (100%) | | [108-profiles-v3](./specs/108-profiles-v3/) | `in-flight` | 23/153 (15%) | -| [109-ux-navigation-consistency](./specs/109-ux-navigation-consistency/) | `in-flight` | 2/179 (1%) | +| [109-ux-navigation-consistency](./specs/109-ux-navigation-consistency/) | `in-flight` | 3/180 (2%) | diff --git a/frontend/src/components/ManualServerForm.vue b/frontend/src/components/ManualServerForm.vue index 1c1becc37..10effbc12 100644 --- a/frontend/src/components/ManualServerForm.vue +++ b/frontend/src/components/ManualServerForm.vue @@ -96,6 +96,9 @@ import { resolveSecretFields, rollbackSecrets } from '@/composables/useSecretFie import { useServersStore } from '@/stores/servers' import { serverDetailPath } from '@/utils/serverRoute' +const props = withDefaults(defineProps<{ navigateAfterAdd?: boolean }>(), { + navigateAfterAdd: true, +}) const emit = defineEmits<{ added: [name: string] }>() const router = useRouter() @@ -158,7 +161,9 @@ async function handleSubmit() { await serversStore.addServer(serverData) emit('added', name.value) - void router.push(serverDetailPath(name.value)) + if (props.navigateAfterAdd) { + void router.push(serverDetailPath(name.value)) + } } catch (e) { // The secret write (if any) succeeded but something after it failed — // don't leave an orphaned keyring entry behind, and let a retry reuse diff --git a/frontend/src/components/OnboardingWizard.vue b/frontend/src/components/OnboardingWizard.vue index 1f7773cbd..d72d51c4c 100644 --- a/frontend/src/components/OnboardingWizard.vue +++ b/frontend/src/components/OnboardingWizard.vue @@ -281,6 +281,9 @@ Add a server manually +
+ +

✓ Server added — it's currently in quarantine. Review it on the Servers page after this wizard.

@@ -400,12 +403,16 @@
+
+ +

✓ Server added — it's currently in quarantine. Review it on the Servers page after this wizard.

@@ -673,12 +680,6 @@
- -