From cc90771af14f045538651615147d8bf86b576549 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 04:16:35 +0800 Subject: [PATCH] fix(native)!: preserve UI scalar facts and selection failures BREAKING CHANGE: Accessibility node value can also be a JSON number or boolean. Strings and null retain their prior meaning. --- bridge/native/NativeUIValues.swift | 26 +++ bridge/native/ReaNativeUI.swift | 11 +- docs/native-investigation.md | 11 + package.json | 1 + scripts/verify-native-ui.mjs | 3 + src/domain/native/nativeUiObservation.ts | 2 +- src/native/NativeUiHelperRuntime.ts | 3 + src/native/NativeUiObservation.test.ts | 17 +- src/native/NativeUiObservation.ts | 26 ++- tests/boundary/mcp/nativeUiFactsMcp.test.ts | 221 ++++++++++++++++++ .../native/native-ui-children/main.swift | 15 ++ 11 files changed, 330 insertions(+), 6 deletions(-) create mode 100644 bridge/native/NativeUIValues.swift create mode 100644 tests/boundary/mcp/nativeUiFactsMcp.test.ts diff --git a/bridge/native/NativeUIValues.swift b/bridge/native/NativeUIValues.swift new file mode 100644 index 0000000000..f0e6629f8a --- /dev/null +++ b/bridge/native/NativeUIValues.swift @@ -0,0 +1,26 @@ +import Foundation +import CoreFoundation + +struct AccessibilityValue { + let value: Any + let gap: String? +} + +/** Preserve JSON scalars; report successful reads that JSON cannot represent faithfully. */ +func captureAccessibilityValue(_ source: Any?) -> AccessibilityValue { + guard let source else { return AccessibilityValue(value: NSNull(), gap: nil) } + if let text = source as? String { return AccessibilityValue(value: text, gap: nil) } + if let number = source as? NSNumber { + if CFGetTypeID(number) == CFBooleanGetTypeID() { + return AccessibilityValue(value: number.boolValue, gap: nil) + } + let value = number.doubleValue + let encoding = String(cString: number.objCType) + let floating = encoding == "f" || encoding == "d" + if number is NSDecimalNumber || !value.isFinite || (value == 0 && value.sign == .minus) || (!floating && abs(value) > 9_007_199_254_740_991) { + return AccessibilityValue(value: NSNull(), gap: "AXValue number \(number.stringValue) cannot be represented faithfully as a JSON number") + } + return AccessibilityValue(value: number, gap: nil) + } + return AccessibilityValue(value: NSNull(), gap: "AXValue type \(type(of: source)) is not a supported JSON scalar") +} diff --git a/bridge/native/ReaNativeUI.swift b/bridge/native/ReaNativeUI.swift index 2e4cc83414..eed5bfd7f5 100644 --- a/bridge/native/ReaNativeUI.swift +++ b/bridge/native/ReaNativeUI.swift @@ -43,8 +43,11 @@ func windowBounds(_ element: AXUIElement) -> CGRect? { return CGRect(origin: point, size: size) } func observe(_ request: Request) async throws -> [String: Any] { - guard let app = NSRunningApplication(processIdentifier: request.pid), let url = app.executableURL else { - try fail("target-mismatch", "Selected PID does not expose an application executable URL") + guard let app = NSRunningApplication(processIdentifier: request.pid) else { + try fail("pid-not-application", "LaunchServices does not expose an application for the selected PID; this does not establish that the process is absent") + } + guard let url = app.executableURL else { + try fail("application-executable-unavailable", "The selected application does not expose an executable URL; select a PID with an identifiable application executable") } guard let canonicalPointer = realpath(url.path, nil) else { try fail("target-mismatch", "Running executable path cannot be resolved") } defer { free(canonicalPointer) } @@ -117,7 +120,9 @@ func observe(_ request: Request) async throws -> [String: Any] { let childCountResult = childCount(element) var actions: CFArray? AXUIElementCopyActionNames(element, &actions) - nodes.append(["path": path, "role": text(element, kAXRoleAttribute), "title": text(element, kAXTitleAttribute), "value": text(element, kAXValueAttribute), "actions": actions as? [String] ?? [], "children_count": childCountResult.value.map { $0 as Any } ?? NSNull()]) + let observedValue = captureAccessibilityValue(attribute(element, kAXValueAttribute)) + if let gap = observedValue.gap { gaps.append("\(gap) at path \(path)"); truncated = true } + nodes.append(["path": path, "role": text(element, kAXRoleAttribute), "title": text(element, kAXTitleAttribute), "value": observedValue.value, "actions": actions as? [String] ?? [], "children_count": childCountResult.value.map { $0 as Any } ?? NSNull()]) guard let totalChildren = childCountResult.value else { truncated = true gaps.append("AX child count unavailable at path \(path): AXError \(childCountResult.error ?? -1)") diff --git a/docs/native-investigation.md b/docs/native-investigation.md index 87644cbdca..a41180af69 100644 --- a/docs/native-investigation.md +++ b/docs/native-investigation.md @@ -732,6 +732,17 @@ capture or automatic permission prompts. Executable bytes and process launch time guard against a different target or PID reuse. AX selection requires one unique geometry match; ambiguity fails closed. +Accessibility node values preserve strings, finite numbers and booleans as their +original JSON scalar types. Values that JSON cannot represent faithfully, +including unsafe integers, negative zero and non-finite numbers, remain null +with a path-specific gap and incomplete capture coverage. Null alone does not +establish that an accessibility value was successfully read or absent. + +An initial PID or window selection refusal is `invalid_request` on `pid` or +`window_id` and leaves provider availability intact. Identity or window loss +after the initial capture remains an observation failure with collected facts. +Permission and other helper failures retain their specific remediation text. + `capture_native_ui_scenario` takes AX child-index paths for press, increment/ decrement scrolling and text-value entry, or bounded waits. No global event injection is used. Unsupported AX actions fail explicitly. The result preserves diff --git a/package.json b/package.json index 841bdb69db..4720dbd50e 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,7 @@ "bridge/hopper_bridge.py", "bridge/cutter/rea_bridge.py", "bridge/native/NativeUIChildren.swift", + "bridge/native/NativeUIValues.swift", "bridge/native/ReaNativeUI.swift", "bridge/native/rea_lldb_tracer.py", "bridge/process/ProcessRunTokenReader.swift", diff --git a/scripts/verify-native-ui.mjs b/scripts/verify-native-ui.mjs index f94cf1c174..91a9f6a169 100644 --- a/scripts/verify-native-ui.mjs +++ b/scripts/verify-native-ui.mjs @@ -28,6 +28,9 @@ try { fileURLToPath( new URL("../bridge/native/NativeUIChildren.swift", import.meta.url), ), + fileURLToPath( + new URL("../bridge/native/NativeUIValues.swift", import.meta.url), + ), fileURLToPath( new URL( "../tests/conformance/native/native-ui-children/main.swift", diff --git a/src/domain/native/nativeUiObservation.ts b/src/domain/native/nativeUiObservation.ts index c5f8ce2e73..f209818dde 100644 --- a/src/domain/native/nativeUiObservation.ts +++ b/src/domain/native/nativeUiObservation.ts @@ -129,7 +129,7 @@ export const nativeUiSnapshotSchema = z.strictObject({ path: z.array(z.int().nonnegative()), role: z.string().nullable(), title: z.string().nullable(), - value: z.string().nullable(), + value: z.union([z.string(), z.number(), z.boolean()]).nullable(), actions: z.array(z.string()), children_count: z.int().nonnegative().nullable(), }), diff --git a/src/native/NativeUiHelperRuntime.ts b/src/native/NativeUiHelperRuntime.ts index f7bacb2225..07a71c418c 100644 --- a/src/native/NativeUiHelperRuntime.ts +++ b/src/native/NativeUiHelperRuntime.ts @@ -67,6 +67,9 @@ export const createNativeUiHelperRuntime = ( import.meta.url, ), ), + fileURLToPath( + new URL("../../bridge/native/NativeUIValues.swift", import.meta.url), + ), main, "-o", output, diff --git a/src/native/NativeUiObservation.test.ts b/src/native/NativeUiObservation.test.ts index ce3982d5c1..11e682c34a 100644 --- a/src/native/NativeUiObservation.test.ts +++ b/src/native/NativeUiObservation.test.ts @@ -561,7 +561,22 @@ describe("native UI target and cancellation failures", () => { }), }); expect(result.ok).toBe(false); - if (!result.ok) expect(result.error.message).toContain(code); + if (!result.ok) { + const projected = projectAnalysisError(result.error); + expect(projected.message).toBe( + "Grant permission or select an unambiguous window", + ); + if (code === "ambiguous-window") + expect(projected).toMatchObject({ + code: "invalid_request", + details: { + issues: [ + { path: ["window_id"], message: expect.stringContaining(code) }, + ], + }, + }); + else expect(projected.details?.reason).toContain(code); + } }); it("preserves the underlying helper failure reason in diagnostics", async () => { const result = await observeNativeUi(target, "observe_native_ui", scope, { diff --git a/src/native/NativeUiObservation.ts b/src/native/NativeUiObservation.ts index e785e8a368..c18ae5034c 100644 --- a/src/native/NativeUiObservation.ts +++ b/src/native/NativeUiObservation.ts @@ -50,6 +50,13 @@ const responseSchema = z.discriminatedUnion("ok", [ message: z.string(), }), ]); +const initialSelectionFields = new Map([ + ["pid-not-application", "pid"], + ["application-executable-unavailable", "pid"], + ["target-mismatch", "pid"], + ["window-mismatch", "window_id"], + ["ambiguous-window", "window_id"], +]); /** Narrow process seam preserves the native helper's OS and target failures. */ export type NativeUiHelper = ( parameters: Readonly>, @@ -179,14 +186,31 @@ const observeWithHelper = async ( signal, ), ); - if (!response.ok) + if (!response.ok) { + const field = initialSelectionFields.get(response.code); + if (launchTime === undefined && field !== undefined) + return err( + new AnalysisInputError( + operation, + { userMessage: response.message }, + [ + { + path: [field], + reason: "invalid_value", + message: `${response.code}: ${response.message}`, + }, + ], + ), + ); return err( new AnalysisCapabilityUnavailableError( "native-macos", operation, `${response.code}: ${response.message}`, + { userMessage: response.message }, ), ); + } if ( response.result.window.pid !== input.pid || response.result.window.window_id !== input.window_id || diff --git a/tests/boundary/mcp/nativeUiFactsMcp.test.ts b/tests/boundary/mcp/nativeUiFactsMcp.test.ts new file mode 100644 index 0000000000..6073aa8d17 --- /dev/null +++ b/tests/boundary/mcp/nativeUiFactsMcp.test.ts @@ -0,0 +1,221 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { expect, it, onTestFinished, vi } from "vitest"; + +import { AnalysisCleanupLease } from "../../../src/artifacts/AnalysisCleanupLease.js"; +import { toolContract } from "../../../src/contracts/toolContracts.js"; +import { expandToolAvailability } from "../../../src/contracts/toolOutputSchemaPrimitives.js"; +import { parseEvidence } from "../../../src/domain/evidence.js"; +import * as runtimes from "../../../src/native/NativeUiHelperRuntime.js"; +import { NativeMacOSProvider } from "../../../src/native/NativeMacOSProvider.js"; +import { createServer } from "../../../src/server/createServer.js"; +import { createTestBinarySession } from "../../fixtures/binarySession.js"; +import { NativeFixtureRunner } from "../../fixtures/nativeCommands.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; + +const scope = { + pid: 123, + window_id: 456, + screenshot: false, + accessibility: true, +}; +const connectNativeUi = async () => { + const directory = await createTestTempDirectory("rea-native-ui-facts-"); + const path = join(directory, "Inert.macho"); + const header = Buffer.alloc(32); + header.writeUInt32LE(0xfeedfacf, 0); + header.writeUInt32LE(0x0100000c, 4); + header.writeUInt32LE(2, 12); + await writeFile(path, header); + const replies: unknown[] = []; + const factory = vi + .spyOn(runtimes, "createNativeUiHelperRuntime") + .mockImplementation(() => { + const cleanupLease = new AnalysisCleanupLease(async () => {}); + return { + signal: new AbortController().signal, + cleanupLease, + invoke: async () => { + if (replies.length === 0) + throw new Error("Missing fixture helper reply"); + return replies.shift(); + }, + close: () => cleanupLease.release(), + resources: () => [], + }; + }); + const session = createTestBinarySession( + new NativeMacOSProvider({}, new NativeFixtureRunner(), "darwin"), + ); + const server = createServer({ kind: "session", session }); + const client = new Client({ name: "native-ui-facts", version: "1" }); + const [ct, st] = InMemoryTransport.createLinkedPair(); + onTestFinished(async () => { + factory.mockRestore(); + try { + await client.close(); + } finally { + await server.close(); + await session.close(); + } + }); + await server.connect(st); + await client.connect(ct); + expect( + (await client.callTool({ name: "open_binary", arguments: { path } })) + .isError, + ).not.toBe(true); + const snapshot = (values: (string | number | boolean | null)[]) => ({ + ok: true, + result: { + window: { + pid: 123, + window_id: 456, + executable: path, + launch_time: 1, + title: "Fixture", + }, + nodes: values.map((value, index) => ({ + path: [index], + role: "AXSlider", + title: null, + value, + actions: [], + children_count: 0, + })), + screenshot: null, + gaps: [], + truncated: false, + }, + }); + return { client, replies, snapshot }; +}; + +it("preserves scalar accessibility values in passive and scenario Evidence", async () => { + const { client, replies, snapshot } = await connectNativeUi(); + replies.push(snapshot([0.25, true, "selected", null])); + const passive = await client.callTool({ + name: "observe_native_ui", + arguments: scope, + }); + expect(passive.isError).not.toBe(true); + expect( + parseEvidence(passive.structuredContent).normalized_result, + ).toMatchObject({ + initial: { + nodes: [ + { value: 0.25 }, + { value: true }, + { value: "selected" }, + { value: null }, + ], + }, + }); + replies.push(snapshot([0.25, false]), snapshot([0.75, true])); + const scenario = await client.callTool({ + name: "capture_native_ui_scenario", + arguments: { ...scope, steps: [{ kind: "wait", milliseconds: 0 }] }, + }); + expect(scenario.isError).not.toBe(true); + expect( + parseEvidence(scenario.structuredContent).normalized_result, + ).toMatchObject({ + initial: { nodes: [{ value: 0.25 }, { value: false }] }, + steps: [ + { + before: { nodes: [{ value: 0.25 }, { value: false }] }, + after: { nodes: [{ value: 0.75 }, { value: true }] }, + outcome: "completed", + }, + ], + }); +}); + +it("keeps native UI availability after initial selection errors and preserves later failures", async () => { + const { client, replies, snapshot } = await connectNativeUi(); + for (const [code, field] of [ + ["pid-not-application", "pid"], + ["application-executable-unavailable", "pid"], + ["target-mismatch", "pid"], + ["window-mismatch", "window_id"], + ] as const) + for (const tool of [ + "observe_native_ui", + "capture_native_ui_scenario", + ] as const) { + const message = `Fixture ${code} reason`; + replies.push({ ok: false, code, message }); + const response = await client.callTool({ + name: tool, + arguments: { + ...scope, + ...(tool === "capture_native_ui_scenario" + ? { steps: [{ kind: "wait", milliseconds: 0 }] } + : {}), + }, + }); + expect(parseMcpToolError(response)).toMatchObject({ + error: { + code: "invalid_request", + retryable: false, + message, + details: { + issues: [{ path: [field], message: `${code}: ${message}` }], + }, + }, + }); + const inventory = toolContract("binary_session").outputSchema.parse( + (await client.callTool({ name: "binary_session", arguments: {} })) + .structuredContent, + ); + expect( + expandToolAvailability(inventory.result.tool_availability).find( + (entry) => entry.name === tool, + ), + ).toMatchObject({ available: true }); + } + replies.push(snapshot([0.25])); + expect( + (await client.callTool({ name: "observe_native_ui", arguments: scope })) + .isError, + ).not.toBe(true); + replies.push(snapshot([0.25]), { + ok: false, + code: "window-mismatch", + message: "Selected window disappeared after initial capture", + }); + const changed = await client.callTool({ + name: "capture_native_ui_scenario", + arguments: { ...scope, steps: [{ kind: "wait", milliseconds: 0 }] }, + }); + expect(parseMcpToolError(changed)).toMatchObject({ + error: { + code: "capability_unavailable", + message: "Selected window disappeared after initial capture", + details: { + partial_observation: { + initial: { nodes: [{ value: 0.25 }] }, + steps: [{ outcome: "failed" }], + }, + }, + }, + }); + replies.push({ + ok: false, + code: "accessibility-denied", + message: "Grant Accessibility to the REA host in System Settings", + }); + const denied = await client.callTool({ + name: "observe_native_ui", + arguments: scope, + }); + expect(parseMcpToolError(denied)).toMatchObject({ + error: { + code: "capability_unavailable", + message: "Grant Accessibility to the REA host in System Settings", + }, + }); + await client.ping(); +}); diff --git a/tests/conformance/native/native-ui-children/main.swift b/tests/conformance/native/native-ui-children/main.swift index bbe416107c..25d7d88d64 100644 --- a/tests/conformance/native/native-ui-children/main.swift +++ b/tests/conformance/native/native-ui-children/main.swift @@ -1,5 +1,20 @@ import Foundation +let numericValue = captureAccessibilityValue(NSNumber(value: 0.25)) +precondition((numericValue.value as? NSNumber)?.doubleValue == 0.25 && numericValue.gap == nil) +let booleanValue = captureAccessibilityValue(NSNumber(value: true)) +precondition((booleanValue.value as? Bool) == true && booleanValue.gap == nil) +let stringValue = captureAccessibilityValue("selected") +precondition((stringValue.value as? String) == "selected" && stringValue.gap == nil) +let absentValue = captureAccessibilityValue(nil) +precondition(absentValue.value is NSNull && absentValue.gap == nil) +for unrepresentable in [NSNumber(value: Double.infinity), NSNumber(value: -0.0), NSNumber(value: UInt64.max), NSDecimalNumber(string: "0.1234567890123456789012345")] { + let result = captureAccessibilityValue(unrepresentable) + precondition(result.value is NSNull && result.gap != nil) +} +let compoundValue = captureAccessibilityValue(["nested": "value"]) +precondition(compoundValue.value is NSNull && compoundValue.gap != nil) + let zero = captureChildCount(status: 0, success: 0, value: 0) precondition(zero.value == 0, "A successful empty child list must remain a known zero") precondition(zero.error == nil, "A successful child count must not report an error")