From 1495aae0cdc94878d91679d516f03b35dd0e2d45 Mon Sep 17 00:00:00 2001 From: Kyle Date: Thu, 29 Oct 2026 18:35:00 +0800 Subject: [PATCH 1/2] fix(mcp): give virtual nodes unique identifiers and readable details Use deferred detail IDs for virtual nodes and namespaced handles for older inline payloads. Route lookup through the same identifier mapping, serve inline details without an invalid RPC, and recognize custom attribute groups as cached details. Validation: Host build and MCP bridge tests passed, including 105 virtual nodes. iOS 27 end-to-end checks found 117 unique detailed-mode nodes and 30 unique compact-mode nodes; all seven Text nodes returned details and cached attributes. --- LookInside.xcodeproj/project.pbxproj | 1 + .../MCPBridge/MCPBridgeDetailsService.swift | 112 ++++++++++-------- .../MCPBridgeDisplayItemAccess.swift | 41 +++++++ .../MCPBridge/MCPBridgeInspectionModels.swift | 4 +- .../MCPBridgeInspectionService.swift | 2 +- .../MCPBridgeLiveDocumentLookup.swift | 29 ++--- Scripts/test-mcp-bridge.sh | 21 ++++ .../MCPBridgeDisplayItemAccessTests.swift | 65 ++++++++++ 8 files changed, 200 insertions(+), 75 deletions(-) create mode 100644 LookInside/MCPBridge/MCPBridgeDisplayItemAccess.swift create mode 100644 Tests/MCPBridge/MCPBridgeDisplayItemAccessTests.swift diff --git a/LookInside.xcodeproj/project.pbxproj b/LookInside.xcodeproj/project.pbxproj index 7b010033..4ddca8ee 100644 --- a/LookInside.xcodeproj/project.pbxproj +++ b/LookInside.xcodeproj/project.pbxproj @@ -191,6 +191,7 @@ MCPBridge/MCPBridgeAttributeValueDecoder.swift, MCPBridge/MCPBridgeConnection.swift, MCPBridge/MCPBridgeDetailsService.swift, + MCPBridge/MCPBridgeDisplayItemAccess.swift, MCPBridge/MCPBridgeEventPublisher.swift, MCPBridge/MCPBridgeFrame.swift, MCPBridge/MCPBridgeInspectionModels.swift, diff --git a/LookInside/MCPBridge/MCPBridgeDetailsService.swift b/LookInside/MCPBridge/MCPBridgeDetailsService.swift index 61a87787..7671ee6a 100644 --- a/LookInside/MCPBridge/MCPBridgeDetailsService.swift +++ b/LookInside/MCPBridge/MCPBridgeDetailsService.swift @@ -28,8 +28,8 @@ import AppKit import CoreGraphics import Foundation -import os import FoundationToolbox +import os @Loggable(subsystem: "com.lookinside.app", category: "MCPBridge.Details") @MainActor @@ -40,7 +40,6 @@ final class MCPBridgeDetailsService { /// need more issue multiple calls. private static let maximumObjectIdentifiersPerCall = 100 - init() {} // MARK: - Entry point @@ -82,9 +81,8 @@ final class MCPBridgeDetailsService { case let .success(parsed): requestedIdentifiers = parsed.identifiers if parsed.droppedDuplicateCount > 0 { - #log(.debug, - "details.read collapsed \(parsed.droppedDuplicateCount, privacy: .public) duplicate object identifier(s)" - ) + #log(.debug, + "details.read collapsed \(parsed.droppedDuplicateCount, privacy: .public) duplicate object identifier(s)") } case .failure(.notAllStrings), .failure(.empty): return .failure(identifier: identifier, error: .invalidParameters) @@ -98,11 +96,10 @@ final class MCPBridgeDetailsService { ) } - let includeUserCustom: Bool - if case let .bool(raw)? = parameters["includeUserCustom"] { - includeUserCustom = raw + let includeUserCustom: Bool = if case let .bool(raw)? = parameters["includeUserCustom"] { + raw } else { - includeUserCustom = true + true } // Live-document lookup @@ -134,24 +131,42 @@ final class MCPBridgeDetailsService { var resolvedItems: [(identifier: String, displayItem: DisplayItem, nativeOid: UInt)] = [] resolvedItems.reserveCapacity(requestedIdentifiers.count) var failedIdentifiers: [String] = [] + var emittedDetails: [MCPBridgeViewDetail] = [] for requested in requestedIdentifiers { guard let displayItem = MCPBridgeLiveDocumentLookup.findDisplayItem( amongRoots: roots, matchingObjectIdentifier: requested - ), - case let nativeOid = displayItem.detailRequestOid, - nativeOid != 0 - else { + ) else { failedIdentifiers.append(requested) continue } - resolvedItems.append((identifier: requested, displayItem: displayItem, nativeOid: nativeOid)) + let nativeOid = displayItem.detailRequestOid + if displayItem.customInfo != nil, nativeOid == 0 { + // Older Servers send virtual-node attributes with the tree. + // They have no object for RPC 203; return that inline detail. + if displayItem.customAttrGroupList == nil { + displayItem.customAttrGroupList = [] + } + emittedDetails.append(encodeDetail( + identifier: requested, displayItem: displayItem, + inbuiltGroups: displayItem.attributesGroupList, + customGroups: displayItem.customAttrGroupList, + includeUserCustom: includeUserCustom + )) + } else if nativeOid != 0 { + resolvedItems.append((identifier: requested, displayItem: displayItem, nativeOid: nativeOid)) + } else { + failedIdentifiers.append(requested) + } } // If everything is gone, surface a structured error instead of // an empty success response — the agent likely passed stale oids. if resolvedItems.isEmpty { + if !emittedDetails.isEmpty { + return successResponse(identifier: identifier, details: emittedDetails, failedIdentifiers: failedIdentifiers) + } return .failure( identifier: identifier, error: MCPBridgeErrorPayload( @@ -215,8 +230,7 @@ final class MCPBridgeDetailsService { // to its owning document. let staticDataSource = document.hierarchyDataSource - var emittedDetails: [MCPBridgeViewDetail] = [] - emittedDetails.reserveCapacity(allDetails.count) + emittedDetails.reserveCapacity(emittedDetails.count + allDetails.count) var seenOids: Set = [] for detail in allDetails { @@ -246,40 +260,11 @@ final class MCPBridgeDetailsService { // Encode the attribute groups carried in this detail // through the read-side encoder so the wire shape matches // attributes.read. - let redactSecureContent = MCPBridgeSecureContentDetector.isSecure(displayItem: resolved.displayItem) - var rawGroups: [AttributesGroup] = [] - if let inbuiltGroups = detail.attributesGroupList { - rawGroups.append(contentsOf: inbuiltGroups) - } - if includeUserCustom, let customGroups = detail.customAttrGroupList { - rawGroups.append(contentsOf: customGroups) - } - let encodedGroups = rawGroups.map { group -> MCPBridgeAttributeGroup in - let sections = (group.attrSections ?? []).map { section -> MCPBridgeAttributeSection in - let attributes = (section.attributes ?? []).map { attribute in - MCPBridgeAttributeEncoder.encode( - attribute, - redactingSecureContent: redactSecureContent - ) - } - return MCPBridgeAttributeSection( - identifier: section.identifier ?? "", - attributes: attributes - ) - } - let groupIdentifier = group.userCustomTitle ?? group.identifier ?? "" - return MCPBridgeAttributeGroup( - identifier: groupIdentifier, - isUserCustom: group.userCustomTitle != nil, - isSwiftUIGroup: group.isSwiftUIGroup, - sections: sections - ) - } - - emittedDetails.append(MCPBridgeViewDetail( - objectIdentifier: resolved.identifier, - groups: encodedGroups, - secureContent: redactSecureContent + emittedDetails.append(encodeDetail( + identifier: resolved.identifier, displayItem: resolved.displayItem, + inbuiltGroups: detail.attributesGroupList, + customGroups: detail.customAttrGroupList, + includeUserCustom: includeUserCustom )) } @@ -299,6 +284,33 @@ final class MCPBridgeDetailsService { // MARK: - Helpers + private func encodeDetail( + identifier: String, + displayItem: DisplayItem, + inbuiltGroups: [AttributesGroup]?, + customGroups: [AttributesGroup]?, + includeUserCustom: Bool + ) -> MCPBridgeViewDetail { + let redact = MCPBridgeSecureContentDetector.isSecure(displayItem: displayItem) + let groups = (inbuiltGroups ?? []) + (includeUserCustom ? customGroups ?? [] : []) + let encoded = groups.map { group in + MCPBridgeAttributeGroup( + identifier: group.userCustomTitle ?? group.identifier ?? "", + isUserCustom: group.userCustomTitle != nil, + isSwiftUIGroup: group.isSwiftUIGroup, + sections: (group.attrSections ?? []).map { section in + MCPBridgeAttributeSection( + identifier: section.identifier ?? "", + attributes: (section.attributes ?? []).map { + MCPBridgeAttributeEncoder.encode($0, redactingSecureContent: redact) + } + ) + } + ) + } + return MCPBridgeViewDetail(objectIdentifier: identifier, groups: encoded, secureContent: redact) + } + private func successResponse( identifier: String, details: [MCPBridgeViewDetail], diff --git a/LookInside/MCPBridge/MCPBridgeDisplayItemAccess.swift b/LookInside/MCPBridge/MCPBridgeDisplayItemAccess.swift new file mode 100644 index 00000000..6e89ba6d --- /dev/null +++ b/LookInside/MCPBridge/MCPBridgeDisplayItemAccess.swift @@ -0,0 +1,41 @@ +import Foundation + +/// Identifiers belong to a hierarchy snapshot. A virtual node is addressable +/// even when it has no UIView/CALayer and an older Server sent details inline. +@MainActor +enum MCPBridgeDisplayItemAccess { + static func objectIdentifier(for item: DisplayItem) -> String { + if let customInfo = item.customInfo { + if customInfo.detailObjectOid != 0 { + return "0x" + String(customInfo.detailObjectOid, radix: 16) + } + if let identifier = customInfo.swiftUIDisplayItemID, !identifier.isEmpty { + return identifier + } + } else if let oid = item.displayingObject()?.oid, oid != 0 { + return "0x" + String(oid, radix: 16) + } + // Generic custom nodes predate deferred detail IDs. Keep their + // host-local identity in a namespace separate from target object IDs. + let address = UInt(bitPattern: Unmanaged.passUnretained(item).toOpaque()) + return "custom:0x" + String(address, radix: 16) + } + + static func find(in roots: [DisplayItem], identifier: String) -> DisplayItem? { + var queue = roots + var index = 0 + while index < queue.count { + let item = queue[index] + index += 1 + if objectIdentifier(for: item) == identifier { + return item + } + queue.append(contentsOf: item.subitems ?? []) + } + return nil + } + + static func hasCachedDetails(_ item: DisplayItem) -> Bool { + item.attributesGroupList != nil || item.customAttrGroupList != nil + } +} diff --git a/LookInside/MCPBridge/MCPBridgeInspectionModels.swift b/LookInside/MCPBridge/MCPBridgeInspectionModels.swift index aa2fabb7..b7c45007 100644 --- a/LookInside/MCPBridge/MCPBridgeInspectionModels.swift +++ b/LookInside/MCPBridge/MCPBridgeInspectionModels.swift @@ -82,8 +82,8 @@ struct MCPBridgeTargetInfo: Sendable, Codable { /// screenshot URIs live in separate methods to be added in subsequent /// commits. struct MCPBridgeViewNode: Sendable, Codable { - /// Hex-encoded `InspectedObject.oid`, prefixed with `0x` (for example, - /// `0x600000abc123`). Stable within a single connected app instance. + /// Opaque handle: a hex native/deferred-detail oid, or a namespaced + /// virtual-node handle for older SDKs. Rediscover after hierarchy refresh. let objectIdentifier: String /// Leaf Objective-C class name (head of `InspectedObject.classChainList`), diff --git a/LookInside/MCPBridge/MCPBridgeInspectionService.swift b/LookInside/MCPBridge/MCPBridgeInspectionService.swift index 86ded6ad..ea9c5a5d 100644 --- a/LookInside/MCPBridge/MCPBridgeInspectionService.swift +++ b/LookInside/MCPBridge/MCPBridgeInspectionService.swift @@ -297,7 +297,7 @@ final class MCPBridgeInspectionService { // see an empty `groups` array fetch the details with // `read_view_details` (the `details.read` route). `secureContent` // tells the agent why textual values may be `null` here. - let hasCachedDetails = displayItem.attributesGroupList?.isEmpty == false + let hasCachedDetails = MCPBridgeDisplayItemAccess.hasCachedDetails(displayItem) return .success( identifier: identifier, result: .object([ diff --git a/LookInside/MCPBridge/MCPBridgeLiveDocumentLookup.swift b/LookInside/MCPBridge/MCPBridgeLiveDocumentLookup.swift index b6d3fa95..a0a07461 100644 --- a/LookInside/MCPBridge/MCPBridgeLiveDocumentLookup.swift +++ b/LookInside/MCPBridge/MCPBridgeLiveDocumentLookup.swift @@ -2,11 +2,8 @@ // // Shared lookup helpers that translate the bridge wire identifiers // (`targetIdentifier`, `objectIdentifier`) into the host's live runtime -// objects (`LiveDocument`, `DisplayItem`). Both the inspection -// service (read-only queries) and the invocation service (round-trips to -// the server over the connection channel) share this surface so the wire shape — hex -// `oid` strings, decimal `appInfoIdentifier` strings — stays one -// canonical implementation. +// objects (`LiveDocument`, `DisplayItem`). The services share one canonical +// implementation for native and virtual node handles and decimal target IDs. // // All entry points are `@MainActor`: `NSDocumentController` and // `LiveDocument` require main-thread isolation, and the bridge @@ -39,23 +36,12 @@ enum MCPBridgeLiveDocumentLookup { /// Breadth-first search across the supplied root display items for an /// item whose wire `objectIdentifier` matches `identifier`. The wire - /// form is the hex-encoded `InspectedObject.oid`, prefixed with `0x` - /// (e.g. `0x600000abc123`). + /// form is an opaque native or virtual node handle returned by this bridge. static func findDisplayItem( amongRoots roots: [DisplayItem], matchingObjectIdentifier identifier: String ) -> DisplayItem? { - var queue: [DisplayItem] = roots - while queue.isEmpty == false { - let current = queue.removeFirst() - if objectIdentifierString(for: current) == identifier { - return current - } - if let subitems = current.subitems { - queue.append(contentsOf: subitems) - } - } - return nil + MCPBridgeDisplayItemAccess.find(in: roots, identifier: identifier) } /// Top-level display items (windows / scenes) inside a live document. @@ -72,12 +58,11 @@ enum MCPBridgeLiveDocumentLookup { return flatItems.filter { $0.indentLevel() == 0 } } - /// Canonical hex-encoded `objectIdentifier` string for a display item. + /// Canonical `objectIdentifier` for native and virtual display items. /// Stays in sync with the form returned by `hierarchy.read` so /// callers can round-trip identifiers across bridge methods. static func objectIdentifierString(for item: DisplayItem) -> String { - let oid = item.displayingObject()?.oid ?? 0 - return String(format: "0x%lx", oid) + MCPBridgeDisplayItemAccess.objectIdentifier(for: item) } /// The item's rectangle in the root coordinate space. @@ -93,6 +78,6 @@ enum MCPBridgeLiveDocumentLookup { /// both values are equally meaningless and the raw one at least matches /// what the host inspector displays. static func rootSpaceFrame(for item: DisplayItem) -> CGRect { - return item.hasValidFrameToRoot() ? item.calculateFrameToRoot() : item.frame + item.hasValidFrameToRoot() ? item.calculateFrameToRoot() : item.frame } } diff --git a/Scripts/test-mcp-bridge.sh b/Scripts/test-mcp-bridge.sh index cc093218..ef3af56b 100755 --- a/Scripts/test-mcp-bridge.sh +++ b/Scripts/test-mcp-bridge.sh @@ -42,4 +42,25 @@ RECOVERY_HINT_TEST="$ROOT/Tests/MCPBridge/MCPBridgeRecoveryHintTests.swift" swiftc -parse-as-library "$RECOVERY_HINT" "$RECOVERY_HINT_TEST" -o "$TMPDIR/recovery-hint-test" "$TMPDIR/recovery-hint-test" +# Exercise virtual-node identity against the real wire model. Only the C +# declarations are imported; Swift model implementations compile alongside it. +CORE="$ROOT/Sources/LookinCore" +SERVER_BASE="$ROOT/Sources/LookinServerBase" +mkdir -p "$TMPDIR/core" +for source_file in "$CORE"/*.m "$CORE"/Shim/*.m "$SERVER_BASE"/*.m; do + [ -e "$source_file" ] || continue + xcrun clang -c -fobjc-arc -w -DSHOULD_COMPILE_LOOKIN_SERVER=1 \ + -mmacosx-version-min=14.0 -I "$CORE" -I "$CORE/include" -I "$SERVER_BASE" \ + "$source_file" -o "$TMPDIR/core/$(basename "$source_file" .m).o" +done +swiftc -parse-as-library -target "$(uname -m)-apple-macos14.0" \ + -import-objc-header "$ROOT/Tests/Dashboard/LKDashboardTests-Bridging-Header.h" \ + -Xcc -DSHOULD_COMPILE_LOOKIN_SERVER=1 -D SHOULD_COMPILE_LOOKIN_SERVER -D LOOKIN_CORE_STANDALONE \ + -Xcc -I"$CORE" -Xcc -I"$CORE/include" -Xcc -I"$SERVER_BASE" \ + $(rg --files "$ROOT/Sources/LookinCoreImpl" -g '*.swift' | sort) \ + "$ROOT/LookInside/MCPBridge/MCPBridgeDisplayItemAccess.swift" \ + "$ROOT/Tests/MCPBridge/MCPBridgeDisplayItemAccessTests.swift" \ + "$TMPDIR"/core/*.o -framework AppKit -framework QuartzCore -o "$TMPDIR/display-item-test" +"$TMPDIR/display-item-test" + echo "MCPBridge tests passed" diff --git a/Tests/MCPBridge/MCPBridgeDisplayItemAccessTests.swift b/Tests/MCPBridge/MCPBridgeDisplayItemAccessTests.swift new file mode 100644 index 00000000..57b46a6e --- /dev/null +++ b/Tests/MCPBridge/MCPBridgeDisplayItemAccessTests.swift @@ -0,0 +1,65 @@ +import AppKit +import Foundation + +@main +@MainActor +struct MCPBridgeDisplayItemAccessTests { + static func main() { + let root = DisplayItem() + let native = InspectedObject() + native.oid = 42 + root.viewObject = native + precondition(MCPBridgeDisplayItemAccess.objectIdentifier(for: root) == "0x2a") + + let nodes = (0 ..< 105).map { index in + let item = DisplayItem() + let info = CustomDisplayItemInfo() + info.isSwiftUI = true + info.title = "Text" + info.swiftUIDisplayItemID = "swiftui:host:\(index)" + info.detailObjectOid = UInt(index + 1000) + item.customInfo = info + return item + } + root.subitems = nodes + let identifiers = nodes.map(MCPBridgeDisplayItemAccess.objectIdentifier) + precondition(Set(identifiers).count == nodes.count) + precondition(!identifiers.contains("0x0")) + for (item, identifier) in zip(nodes, identifiers) { + precondition(MCPBridgeDisplayItemAccess.find(in: [root], identifier: identifier) === item) + precondition(UInt(identifier.dropFirst(2), radix: 16) == item.customInfo?.detailObjectOid) + } + precondition(MCPBridgeDisplayItemAccess.find(in: [root], identifier: "0x0") == nil) + + // Old SDKs have no deferred source, but send attributes inline. + for item in nodes { + item.customInfo?.detailObjectOid = 0 + } + let inlineIdentifiers = nodes.map(MCPBridgeDisplayItemAccess.objectIdentifier) + precondition(Set(inlineIdentifiers).count == nodes.count) + for (item, identifier) in zip(nodes, inlineIdentifiers) { + precondition(MCPBridgeDisplayItemAccess.find(in: [root], identifier: identifier) === item) + } + + // Generic custom nodes don't even carry a SwiftUI ID. Their local + // handles must stay unique and cannot collide with native IDs. + for item in nodes { + item.customInfo?.swiftUIDisplayItemID = nil + } + let genericIdentifiers = nodes.map(MCPBridgeDisplayItemAccess.objectIdentifier) + precondition(Set(genericIdentifiers).count == nodes.count) + precondition(genericIdentifiers.allSatisfy { $0.hasPrefix("custom:") }) + precondition(MCPBridgeDisplayItemAccess.objectIdentifier(for: nodes[0]) == genericIdentifiers[0]) + + let text = nodes[0] + precondition(!MCPBridgeDisplayItemAccess.hasCachedDetails(text)) + text.customAttrGroupList = [AttributesGroup()] + precondition(MCPBridgeDisplayItemAccess.hasCachedDetails(text)) + text.customAttrGroupList = [] + precondition(MCPBridgeDisplayItemAccess.hasCachedDetails(text)) + text.customAttrGroupList = nil + text.attributesGroupList = [AttributesGroup()] + precondition(MCPBridgeDisplayItemAccess.hasCachedDetails(text)) + print("MCP display item identity and cache tests passed (105 virtual nodes)") + } +} From 612aabad462785076120b261cfa55da4a3a16453 Mon Sep 17 00:00:00 2001 From: Kyle Date: Thu, 29 Oct 2026 18:50:37 +0800 Subject: [PATCH 2/2] test: remove ripgrep dependency from MCP bridge checks Use the system find command to enumerate core model sources on CI runners without ripgrep. Validated the entire MCP bridge test script with PATH restricted to macOS system directories. --- Scripts/test-mcp-bridge.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Scripts/test-mcp-bridge.sh b/Scripts/test-mcp-bridge.sh index ef3af56b..3004a6dd 100755 --- a/Scripts/test-mcp-bridge.sh +++ b/Scripts/test-mcp-bridge.sh @@ -57,7 +57,7 @@ swiftc -parse-as-library -target "$(uname -m)-apple-macos14.0" \ -import-objc-header "$ROOT/Tests/Dashboard/LKDashboardTests-Bridging-Header.h" \ -Xcc -DSHOULD_COMPILE_LOOKIN_SERVER=1 -D SHOULD_COMPILE_LOOKIN_SERVER -D LOOKIN_CORE_STANDALONE \ -Xcc -I"$CORE" -Xcc -I"$CORE/include" -Xcc -I"$SERVER_BASE" \ - $(rg --files "$ROOT/Sources/LookinCoreImpl" -g '*.swift' | sort) \ + $(find "$ROOT/Sources/LookinCoreImpl" -name '*.swift' -type f | sort) \ "$ROOT/LookInside/MCPBridge/MCPBridgeDisplayItemAccess.swift" \ "$ROOT/Tests/MCPBridge/MCPBridgeDisplayItemAccessTests.swift" \ "$TMPDIR"/core/*.o -framework AppKit -framework QuartzCore -o "$TMPDIR/display-item-test"