morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #5307] [CLOSED] fix(native): report unresolved native UI PID and window selections as invalid input #6870

New issue

Closed

opened 2026-11-01 21:43:42 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:43:42 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5307
Author: @cathrynlavery
Created: 10/19/2026
Status: ❌ Closed

Base: main ← Head: fix/native-ui-selection-errors

📝 Commits (1)

bbc0061 fix(native): report unresolved native UI PID and window selections as invalid input

📊 Changes

6 files changed (+259 additions, -11 deletions)

View changed files

📝 bridge/native/ReaNativeUI.swift (+17 -4)
📝 docs/native-investigation.md (+4 -1)
📝 scripts/verify-native-ui.mjs (+19 -0)
📝 src/native/NativeUiObservation.test.ts (+69 -1)
📝 src/native/NativeUiObservation.ts (+36 -5)
➕ tests/boundary/mcp/nativeUiSelectionMcp.test.ts (+114 -0)

📄 Description

Summary

Fixes #5306.

A wrong pid or window_id passed to observe_native_ui or capture_native_ui_scenario failed as capability_unavailable with "This analysis is unavailable for the current target. Choose another analysis or target." and marked the operation unavailable in binary_session until a later call succeeded. It now fails as invalid_input with an issue on the argument that did not resolve, and the tool stays advertised as available.

Problem and expected behavior

Smallest trigger: open /System/Applications/Calculator.app, then call observe_native_ui with the PID of a sleep process and Calculator's window ID.

The Swift helper used target-mismatch for three different conditions: NSRunningApplication(processIdentifier:) returned nil, the application had no executableURL, or the PID ran another executable. The message did not say which.
NativeUiObservation turned every helper refusal into AnalysisCapabilityUnavailableError without a userMessage, so the helper's reason was only in details.reason.
BinarySession marks an operation unavailable after any AnalysisCapabilityUnavailableError, so one wrong PID withdrew the tool in session discovery.

Change and scope

bridge/native/ReaNativeUI.swift: separate codes process-not-application (message includes the PID's proc_pidpath, or says no process with that PID is running), executable-unavailable, and process-mismatch. Changed executable bytes keep target-mismatch. The path is diagnostic text only and plays no part in identity checks.
src/native/NativeUiObservation.ts: on the first capture, process-not-application and process-mismatch become AnalysisInputError with an issue on pid, and window-mismatch with an issue on window_id. After a capture has succeeded inside a scenario, the same codes mean the process or window changed, so they stay AnalysisCapabilityUnavailableError with the partial observation. All other helper refusals keep their category and now pass the helper's message as userMessage, so Accessibility and Screen Recording denials show their remediation.
docs/native-investigation.md: one sentence on the new failure shape.

Not included: element-path failures inside scenario steps (element-missing and similar) keep their current classification. I could not establish why a Developer ID-signed third-party app in /Applications produced the old "does not expose an application executable URL" message (see #5306). With this change, that case reports which LaunchServices lookup failed and the PID's executable path.

Contribution scope: not applicable; ordinary fix within the existing native UI boundary.

Contract and boundary impact

Semantic owner and earliest changed stage: native UI helper refusal codes (bridge/native/ReaNativeUI.swift) and their classification in src/native/NativeUiObservation.ts.
Contribution scope fit and linked maintainer decision: not applicable.
CLI and MCP/tool-catalog contract: no schema or catalog change. Error category for unresolved initial PID/window selections changes from unsupported_provider to invalid_input. Other native UI helper refusals keep their category with a more specific message.
Provider, bridge, target-format, or platform compatibility: macOS native UI helper only.
Evidence, artifact, provenance, or reconstruction contract: unchanged. Scenario failures after a completed capture still retain partial_observation.
Target selection, authority, upstream behavior, and cleanup details: unchanged. The helper only reads proc_pidpath for the selected PID when LaunchServices has no application for it.
Generated metadata, package, or installation impact: none.

Expected effects (unchanged by this PR; the helper already reads the selected executable and process metadata):

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated:
- src/native/NativeUiObservation.test.ts: initial process-not-application, process-mismatch and window-mismatch refusals become AnalysisInputError with the matching issue path; a window-mismatch after a completed capture stays a capability failure with partial observation; permission refusals carry the helper message as userMessage.
- tests/boundary/mcp/nativeUiSelectionMcp.test.ts: over MCP, an unresolved PID returns invalid_input with details.issues[0].path == ["pid"], binary_session still lists observe_native_ui as available, and a correct PID then succeeds.
- scripts/verify-native-ui.mjs: the real-helper lane now also observes the fixture window with the verifier's own Node PID and requires AnalysisInputError with process-not-application on pid.
Base reproduction: with only the two source files reverted to b4938aed5, 7 of the new or updated tests fail (the 3 permission userMessage cases, the 3 initial selection cases, and the MCP test). The post-capture window-mismatch case passes on base, as intended.

User-visible MCP output, Calculator open, sleep PID, stdio MCP client against the built branch:

Before (b4938aed5):

{"error":{"code":"capability_unavailable","category":"unsupported_provider","message":"This analysis is unavailable for the current target. Choose another analysis or target.","retryable":false,"remediation":{"action":"This analysis is unavailable for the current target. Choose another analysis or target."},"details":{"provider_id":"native-macos","operation":"observe_native_ui","reason":"target-mismatch: Selected PID does not expose an application executable URL"}}}

followed by binary_session reporting {"names":["observe_native_ui"],"available":false,"reason":"provider_unavailable",...}.

After:

{"error":{"code":"invalid_request","category":"invalid_input","message":"LaunchServices has no running application for selected PID 71105 (/bin/sleep); select the PID that owns the window","retryable":true,"remediation":{"action":"Correct the listed arguments and retry."},"details":{"operation":"observe_native_ui","issues":[{"path":["pid"],"reason":"invalid_value","message":"process-not-application: LaunchServices has no running application for selected PID 71105 (/bin/sleep); select the PID that owns the window"}]}}}

binary_session keeps {"names":["observe_native_ui"],"available":true,"reason":"available","remediation":null}. The same run with Calculator's PID and window_id: 1 returns invalid_input on window_id, and TextEdit's PID returns invalid_input on pid with process-mismatch.

Remaining proof gaps: the executable-unavailable branch is not exercised by a real process; I do not have one that LaunchServices reports without an executable URL.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

All on macOS 27.0 (26A428) arm64, Node 24.18.0, npm 11.16.0.

npm run test:focused -- src/native/NativeUiObservation.test.ts tests/boundary/mcp/nativeUiSelectionMcp.test.ts tests/boundary/mcp/nativeUiFinalizationMcp.test.ts tests/boundary/providers/native/nativeUiFinalization.test.ts tests/boundary/providers/native/nativeUiRuntimeOwnership.test.ts tests/boundary/mcp/sessionAvailabilityRecovery.test.ts tests/composition/analysis-sessions/binarySession.test.ts: 7 files, 94 tests passed.
npm run verify:native-ui: passed with "verification_status":"passed","positive_e2e":true,"scenario":"selected-AX-button-action-and-CLI-MCP-parity-observed","mismatch_rejected":true,"unregistered_pid_rejected":true. Accessibility and Screen Recording were granted to the host shell.
npm run check: passed (lint 0 errors; the warning count is unchanged at 1790).
npm run check:changed: passed (4 files, 68 tests).
npm run verify:test-discovery: "Discovered all 1688 test files exactly once."
npm run docs:check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none to schemas. Callers that matched unsupported_provider for a wrong PID or window now get invalid_input.
Real Hopper/Ghidra, browser, or OS coverage: real macOS native UI helper lane above. No Hopper, Ghidra or IDA involvement.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the helper reads proc_pidpath for the caller-selected PID only when LaunchServices has no application for it, and only for the error message. It sends no signals and starts no processes.

Author readiness

Human owner:

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5307 **Author:** [@cathrynlavery](https://github.com/cathrynlavery) **Created:** 10/19/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/native-ui-selection-errors` --- ### 📝 Commits (1) - [`bbc0061`](https://github.com/morluto/rea/commit/bbc006152b9f24f43f819be4cbe215ad81db75a2) fix(native): report unresolved native UI PID and window selections as invalid input ### 📊 Changes **6 files changed** (+259 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `bridge/native/ReaNativeUI.swift` (+17 -4) 📝 `docs/native-investigation.md` (+4 -1) 📝 `scripts/verify-native-ui.mjs` (+19 -0) 📝 `src/native/NativeUiObservation.test.ts` (+69 -1) 📝 `src/native/NativeUiObservation.ts` (+36 -5) ➕ `tests/boundary/mcp/nativeUiSelectionMcp.test.ts` (+114 -0) </details> ### 📄 Description ## Summary Fixes #5306. A wrong `pid` or `window_id` passed to `observe_native_ui` or `capture_native_ui_scenario` failed as `capability_unavailable` with "This analysis is unavailable for the current target. Choose another analysis or target." and marked the operation unavailable in `binary_session` until a later call succeeded. It now fails as `invalid_input` with an issue on the argument that did not resolve, and the tool stays advertised as available. ## Problem and expected behavior Smallest trigger: open `/System/Applications/Calculator.app`, then call `observe_native_ui` with the PID of a `sleep` process and Calculator's window ID. - The Swift helper used `target-mismatch` for three different conditions: `NSRunningApplication(processIdentifier:)` returned nil, the application had no `executableURL`, or the PID ran another executable. The message did not say which. - `NativeUiObservation` turned every helper refusal into `AnalysisCapabilityUnavailableError` without a `userMessage`, so the helper's reason was only in `details.reason`. - `BinarySession` marks an operation unavailable after any `AnalysisCapabilityUnavailableError`, so one wrong PID withdrew the tool in session discovery. ## Change and scope - `bridge/native/ReaNativeUI.swift`: separate codes `process-not-application` (message includes the PID's `proc_pidpath`, or says no process with that PID is running), `executable-unavailable`, and `process-mismatch`. Changed executable bytes keep `target-mismatch`. The path is diagnostic text only and plays no part in identity checks. - `src/native/NativeUiObservation.ts`: on the first capture, `process-not-application` and `process-mismatch` become `AnalysisInputError` with an issue on `pid`, and `window-mismatch` with an issue on `window_id`. After a capture has succeeded inside a scenario, the same codes mean the process or window changed, so they stay `AnalysisCapabilityUnavailableError` with the partial observation. All other helper refusals keep their category and now pass the helper's message as `userMessage`, so Accessibility and Screen Recording denials show their remediation. - `docs/native-investigation.md`: one sentence on the new failure shape. Not included: element-path failures inside scenario steps (`element-missing` and similar) keep their current classification. I could not establish why a Developer ID-signed third-party app in /Applications produced the old "does not expose an application executable URL" message (see #5306). With this change, that case reports which LaunchServices lookup failed and the PID's executable path. Contribution scope: not applicable; ordinary fix within the existing native UI boundary. ## Contract and boundary impact - Semantic owner and earliest changed stage: native UI helper refusal codes (`bridge/native/ReaNativeUI.swift`) and their classification in `src/native/NativeUiObservation.ts`. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: no schema or catalog change. Error category for unresolved initial PID/window selections changes from `unsupported_provider` to `invalid_input`. Other native UI helper refusals keep their category with a more specific `message`. - Provider, bridge, target-format, or platform compatibility: macOS native UI helper only. - Evidence, artifact, provenance, or reconstruction contract: unchanged. Scenario failures after a completed capture still retain `partial_observation`. - Target selection, authority, upstream behavior, and cleanup details: unchanged. The helper only reads `proc_pidpath` for the selected PID when LaunchServices has no application for it. - Generated metadata, package, or installation impact: none. Expected effects (unchanged by this PR; the helper already reads the selected executable and process metadata): - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: - `src/native/NativeUiObservation.test.ts`: initial `process-not-application`, `process-mismatch` and `window-mismatch` refusals become `AnalysisInputError` with the matching issue path; a `window-mismatch` after a completed capture stays a capability failure with partial observation; permission refusals carry the helper message as `userMessage`. - `tests/boundary/mcp/nativeUiSelectionMcp.test.ts`: over MCP, an unresolved PID returns `invalid_input` with `details.issues[0].path == ["pid"]`, `binary_session` still lists `observe_native_ui` as available, and a correct PID then succeeds. - `scripts/verify-native-ui.mjs`: the real-helper lane now also observes the fixture window with the verifier's own Node PID and requires `AnalysisInputError` with `process-not-application` on `pid`. - Base reproduction: with only the two source files reverted to b4938aed5, 7 of the new or updated tests fail (the 3 permission `userMessage` cases, the 3 initial selection cases, and the MCP test). The post-capture `window-mismatch` case passes on base, as intended. - User-visible MCP output, Calculator open, `sleep` PID, stdio MCP client against the built branch: Before (b4938aed5): ```json {"error":{"code":"capability_unavailable","category":"unsupported_provider","message":"This analysis is unavailable for the current target. Choose another analysis or target.","retryable":false,"remediation":{"action":"This analysis is unavailable for the current target. Choose another analysis or target."},"details":{"provider_id":"native-macos","operation":"observe_native_ui","reason":"target-mismatch: Selected PID does not expose an application executable URL"}}} ``` followed by `binary_session` reporting `{"names":["observe_native_ui"],"available":false,"reason":"provider_unavailable",...}`. After: ```json {"error":{"code":"invalid_request","category":"invalid_input","message":"LaunchServices has no running application for selected PID 71105 (/bin/sleep); select the PID that owns the window","retryable":true,"remediation":{"action":"Correct the listed arguments and retry."},"details":{"operation":"observe_native_ui","issues":[{"path":["pid"],"reason":"invalid_value","message":"process-not-application: LaunchServices has no running application for selected PID 71105 (/bin/sleep); select the PID that owns the window"}]}}} ``` `binary_session` keeps `{"names":["observe_native_ui"],"available":true,"reason":"available","remediation":null}`. The same run with Calculator's PID and `window_id: 1` returns `invalid_input` on `window_id`, and TextEdit's PID returns `invalid_input` on `pid` with `process-mismatch`. - Remaining proof gaps: the `executable-unavailable` branch is not exercised by a real process; I do not have one that LaunchServices reports without an executable URL. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed All on macOS 27.0 (26A428) arm64, Node 24.18.0, npm 11.16.0. - `npm run test:focused -- src/native/NativeUiObservation.test.ts tests/boundary/mcp/nativeUiSelectionMcp.test.ts tests/boundary/mcp/nativeUiFinalizationMcp.test.ts tests/boundary/providers/native/nativeUiFinalization.test.ts tests/boundary/providers/native/nativeUiRuntimeOwnership.test.ts tests/boundary/mcp/sessionAvailabilityRecovery.test.ts tests/composition/analysis-sessions/binarySession.test.ts`: 7 files, 94 tests passed. - `npm run verify:native-ui`: passed with `"verification_status":"passed","positive_e2e":true,"scenario":"selected-AX-button-action-and-CLI-MCP-parity-observed","mismatch_rejected":true,"unregistered_pid_rejected":true`. Accessibility and Screen Recording were granted to the host shell. - `npm run check`: passed (lint 0 errors; the warning count is unchanged at 1790). - `npm run check:changed`: passed (4 files, 68 tests). - `npm run verify:test-discovery`: "Discovered all 1688 test files exactly once." - `npm run docs:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none to schemas. Callers that matched `unsupported_provider` for a wrong PID or window now get `invalid_input`. - Real Hopper/Ghidra, browser, or OS coverage: real macOS native UI helper lane above. No Hopper, Ghidra or IDA involvement. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the helper reads `proc_pidpath` for the caller-selected PID only when LaunchServices has no application for it, and only for the error message. It sends no signals and starts no processes. ## Author readiness  Human owner:  - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:43:42 +00:00