From b26d3c917af23bfcb8b7947375f83d85dab85cd6 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 5 Oct 2026 16:02:39 +0300 Subject: [PATCH 1/2] test(macos): make Swift guard tests able to fail (refs #1458, #1451, #1466) Pin ToolTier wire spellings, feed SC006 a decoy superset, drive the credential test through AppState's deferred source, and slice the Token Savings card before asserting the shared estimate label. --- .../ClientBindingModelTests.swift | 47 +++++++++++++++---- .../HomeTokenSavingsBadgeTests.swift | 5 +- .../ProfilesEnumsLabelsTests.swift | 13 +++++ .../MCPProxyTests/SC006RecordSetTests.swift | 18 +++++-- 4 files changed, 70 insertions(+), 13 deletions(-) diff --git a/native/macos/MCPProxy/MCPProxyTests/ClientBindingModelTests.swift b/native/macos/MCPProxy/MCPProxyTests/ClientBindingModelTests.swift index 26f980beb..aca9ec09a 100644 --- a/native/macos/MCPProxy/MCPProxyTests/ClientBindingModelTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/ClientBindingModelTests.swift @@ -525,20 +525,49 @@ final class ClientBindingModelTests: XCTestCase { } /// The secret never reaches `AppState` (and so never a UserDefaults, a - /// keychain item or a log line: the sheet model is the only holder). + /// keychain item or a log line: the sheet model is the only holder). The + /// model is built exactly as the sheet builds it: from the AppState's own + /// deferred source, over an API client the AppState holds, so the POST + /// really travels through the app wiring and its response is what we scan. func testTheCredentialNeverLandsInAppState() async throws { - let source = StubSource() - source.customResult = .success(customResponse(credential: "mcp_cli_SECRETSECRET")) + ConnectStubURLProtocol.reset() + defer { ConnectStubURLProtocol.reset() } + ConnectStubURLProtocol.responseBody = ConnectStubURLProtocol.envelope(""" + {"client":{"id":"dev-laptop","display_name":"Dev Laptop","kind":"other","state":"other", + "installed":false,"connected":false,"active_sessions":0,"calls_24h":0}, + "credential":"mcp_cli_SECRETSECRET", + "snippet":{"generic_http":"Authorization: Bearer mcp_cli_SECRETSECRET","header_name":"Authorization"}} + """) let appState = AppState() - let model = CustomClientModel(source: source) + appState.apiClient = ConnectStubURLProtocol.makeClient() + let model = CustomClientModel(source: appState.deferredClientSource) model.id = "dev-laptop" await model.create() - XCTAssertNotNil(model.credential) - let mirror = String(reflecting: appState) - XCTAssertFalse(mirror.contains("mcp_cli_")) - XCTAssertFalse(appState.clients.contains { $0.tokenName?.hasPrefix("mcp_cli_") == true }) - XCTAssertNil(UserDefaults.standard.dictionaryRepresentation().values.first { "\($0)".contains("mcp_cli_SECRET") }) + XCTAssertEqual(ConnectStubURLProtocol.recorded.last?.method, "POST", "the create went through the app wiring") + XCTAssertEqual(model.credential, "mcp_cli_SECRETSECRET") + XCTAssertNotNil(model.createdClient) + + func assertClean(_ phase: String) { + XCTAssertFalse(String(reflecting: appState).contains("mcp_cli_"), "\(phase): AppState dump") + XCTAssertFalse(appState.clients.contains { "\($0)".contains("mcp_cli_") }, "\(phase): appState.clients") + XCTAssertNil(UserDefaults.standard.dictionaryRepresentation().values.first { "\($0)".contains("mcp_cli_SECRET") }, + "\(phase): UserDefaults") + } + assertClean("after create") + model.dismiss() + XCTAssertNil(model.credential) + assertClean("after dismiss") + } + + func testACustomClientCreateWithoutACoreConnectionShowsNoCredential() async { + let appState = AppState() + appState.apiClient = nil + let model = CustomClientModel(source: appState.deferredClientSource) + model.id = "dev-laptop" + await model.create() + XCTAssertNil(model.credential) + XCTAssertNotNil(model.errorMessage) } func testAWeakReferenceSeesTheModelReleasedAfterDismiss() async { diff --git a/native/macos/MCPProxy/MCPProxyTests/HomeTokenSavingsBadgeTests.swift b/native/macos/MCPProxy/MCPProxyTests/HomeTokenSavingsBadgeTests.swift index fa55bdf36..8b662467d 100644 --- a/native/macos/MCPProxy/MCPProxyTests/HomeTokenSavingsBadgeTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/HomeTokenSavingsBadgeTests.swift @@ -51,7 +51,10 @@ final class HomeTokenSavingsBadgeTests: XCTestCase { "hubSection must render the shared HomeTokenSavingsBadge") XCTAssertTrue(hub.contains("HomeTokenSavingsBadge.estimateHelp"), "the hub estimate capsule must use the shared help text") - XCTAssertTrue(source.contains("HomeTokenSavingsBadge.estimateLabel"), + let card = try XCTUnwrap(section(of: source, from: "private var tokenSavingsSection", to: "// MARK: - Token Distribution"), + "HomeView has no tokenSavingsSection") + XCTAssertTrue(card.contains("Token Savings"), "the slice must be the Token Savings card") + XCTAssertTrue(card.contains("HomeTokenSavingsBadge.estimateLabel"), "the Token Savings card must reuse the shared estimate label") XCTAssertEqual(source.components(separatedBy: "simulated estimate from the current tool catalog").count - 1, 1, "the estimate help text must live in one place") diff --git a/native/macos/MCPProxy/MCPProxyTests/ProfilesEnumsLabelsTests.swift b/native/macos/MCPProxy/MCPProxyTests/ProfilesEnumsLabelsTests.swift index 3f02a5c5e..bcde87fbe 100644 --- a/native/macos/MCPProxy/MCPProxyTests/ProfilesEnumsLabelsTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/ProfilesEnumsLabelsTests.swift @@ -67,6 +67,19 @@ final class ProfilesEnumsLabelsTests: XCTestCase { func testFixActionCoversTheGoValues() throws { try assertTolerantFamily("fix_action", FixAction.self) } func testExplainVerdictCoversTheGoValues() throws { try assertTolerantFamily("explain_verdict", ExplainVerdict.self) } + /// `ToolTier` has no key in enums.json (a Go contract change is out of scope), + /// so the Go tier spellings are pinned here: each must be a KNOWN case. + func testToolTierCoversTheGoTiers() { + for wire in ["read", "write", "destructive", "unannotated"] { + let decoded = ToolTier(wire: wire) + XCTAssertEqual(decoded.wire, wire, "tool tier \(wire) must round-trip") + XCTAssertFalse(isUnknown(decoded), "tool tier \(wire) decodes to .unknown: the Swift enum is missing a case") + } + if case .unannotated = ToolTier(wire: "unannotated") {} else { XCTFail("unannotated must be a known ToolTier case") } + XCTAssertTrue(isUnknown(ToolTier(wire: "brand_new_tier"))) + XCTAssertEqual(ToolTier(wire: "brand_new_tier").wire, "brand_new_tier") + } + func testTheComparisonCanFail() { XCTAssertTrue(isUnknown(FixAction(wire: "brand_new_fix"))) XCTAssertFalse(isUnknown(FixAction(wire: "move_client"))) diff --git a/native/macos/MCPProxy/MCPProxyTests/SC006RecordSetTests.swift b/native/macos/MCPProxy/MCPProxyTests/SC006RecordSetTests.swift index d3bd8b3a7..26bc4e026 100644 --- a/native/macos/MCPProxy/MCPProxyTests/SC006RecordSetTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/SC006RecordSetTests.swift @@ -74,7 +74,10 @@ final class SC006RecordSetTests: XCTestCase { func testEveryCombinationSendsItsQueryAndShowsItsIds() throws { let now = Date(timeIntervalSince1970: 1_790_000_000) - for set in try golden().recordsets { + let recordsets = try golden().recordsets + var seen = Set() + let allIds = recordsets.flatMap(\.ids).filter { seen.insert($0).inserted } + for set in recordsets { let filter = ScopeFilter(query: parameters(set.urlQuery)) let request = filter.restRequest(for: .activity, scopeFiltersAvailable: true, now: now) XCTAssertEqual(request?.path, "/api/v1/activity", set.urlQuery) @@ -90,8 +93,17 @@ final class SC006RecordSetTests: XCTestCase { // The list the view builds from the server's answer: exactly the ids, // in the server's order, whatever the folding does to the rows. let status = parameters(set.restQuery)["status"] ?? "success" - let list = try entries(set.ids, status: status) - let shown = ActivityFolding.fold(list).flatMap { $0.members.map(\.id) } + // The input is a SUPERSET (this combination's ids interleaved with the + // ids of every other combination as decoys), so the assertion cannot + // pass just because the input was built from the expected output. + let decoys = allIds.filter { !set.ids.contains($0) }.map { "decoy-\($0)" } + var mixed: [String] = [] + for (index, id) in set.ids.enumerated() { mixed.append(id); if index < decoys.count { mixed.append(decoys[index]) } } + mixed.append(contentsOf: decoys.dropFirst(set.ids.count)) + let list = try entries(mixed, status: status) + let folded = ActivityFolding.fold(list).flatMap { $0.members.map(\.id) } + XCTAssertEqual(folded, mixed, "folding must neither drop nor reorder a record (\(set.urlQuery))") + let shown = folded.filter { !$0.hasPrefix("decoy-") } XCTAssertEqual(shown, set.ids, "macOS must show exactly the ids of \(set.urlQuery)") } } From 91676285ee5bf7b46771eac6c087fcbac2e16409 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 5 Oct 2026 21:12:28 +0300 Subject: [PATCH 2/2] test(macos): make the AppState credential leak check actually inspect stored values String(reflecting:) on a plain Swift class prints only the type name, so the assertClean AppState leg could never fail. Walk the stored properties with a recursive Mirror dump instead; verified it fails when a secret is seeded. --- .../ClientBindingModelTests.swift | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/native/macos/MCPProxy/MCPProxyTests/ClientBindingModelTests.swift b/native/macos/MCPProxy/MCPProxyTests/ClientBindingModelTests.swift index aca9ec09a..4b8be41e5 100644 --- a/native/macos/MCPProxy/MCPProxyTests/ClientBindingModelTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/ClientBindingModelTests.swift @@ -549,7 +549,9 @@ final class ClientBindingModelTests: XCTestCase { XCTAssertNotNil(model.createdClient) func assertClean(_ phase: String) { - XCTAssertFalse(String(reflecting: appState).contains("mcp_cli_"), "\(phase): AppState dump") + // String(reflecting:) on a plain class prints only the type name, so walk the + // stored properties (including @Published storage) with Mirror instead. + XCTAssertFalse(Self.mirrorDump(appState).contains("mcp_cli_"), "\(phase): AppState stored values") XCTAssertFalse(appState.clients.contains { "\($0)".contains("mcp_cli_") }, "\(phase): appState.clients") XCTAssertNil(UserDefaults.standard.dictionaryRepresentation().values.first { "\($0)".contains("mcp_cli_SECRET") }, "\(phase): UserDefaults") @@ -560,6 +562,29 @@ final class ClientBindingModelTests: XCTestCase { assertClean("after dismiss") } + /// Recursively renders every stored value reachable from `value` via Mirror. + static func mirrorDump(_ value: Any, depth: Int = 0, seen: inout Set) -> String { + if depth > 8 { return "" } + let m = Mirror(reflecting: value) + if m.displayStyle == .class, let o = value as AnyObject? { + if !seen.insert(ObjectIdentifier(o)).inserted { return "" } + } + var out = m.children.isEmpty ? "\(value)" : "" + var cur: Mirror? = m + while let mm = cur { + for c in mm.children { + out += "\(c.label ?? ""):" + mirrorDump(c.value, depth: depth + 1, seen: &seen) + "\n" + } + cur = mm.superclassMirror + } + return out + } + + static func mirrorDump(_ value: Any) -> String { + var seen = Set() + return mirrorDump(value, seen: &seen) + } + func testACustomClientCreateWithoutACoreConnectionShowsNoCredential() async { let appState = AppState() appState.apiClient = nil