feat(macos): opt-in native app backend beside XCTest - #3189
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 19 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
5257559 to
87340b8
Compare
Size Report
Startup median (7 runs, lower is better):
|
|
|
Thanks for the PR. At 87340b8 there is one problem to fix before merge: With Could this be simpler? If the backend is resolved once in the Apple runtime binding and native limits are runtime facts, admission would refuse the runner-only capabilities (back, orientation, gestures, keyboard dismiss/enter, recording) before dispatch. The wrapper would then need no hand-kept override list. The native interactor could be built from helper-backed methods plus explicitly delegated ones, not by spreading the runner-backed base, so a runner method added later cannot leak in. Both cells in I traced the recording route by reading the code and did not run it on a device. I did not read The failed Smoke Tests check looks unrelated. An iOS simulator On the earlier review threads, one open thread on the repeated env reads still applies, and the same design cost is what the simplicity question above is about. The thread on findText gating does not apply, because Before merge, |
87340b8 to
168af66
Compare
|
Thanks. Addressed in Runner-only capabilities refuse at fact admission (fixed).
Native interactor built explicitly.
Env reads. The daemon no longer reads
Live run on the stack head (
Audited, no runner use on macOS:
Smoke Tests failure. I agree it is unrelated: an iOS simulator |
There was a problem hiding this comment.
4 issues found across 17 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/platform-apple/src/os/macos/native-backend-facts.test.ts">
<violation number="1" location="packages/platform-apple/src/os/macos/native-backend-facts.test.ts:24">
P2: This test omits `performMultiTouchGesturePlan` and `performTargetAuthoredDrag`, so either could become available for the native backend without this “every runner-only” assertion failing. Add both operation names to the cases.</violation>
</file>
<file name="packages/platform-apple/src/runtime-snapshot.test.ts">
<violation number="1" location="packages/platform-apple/src/runtime-snapshot.test.ts:46">
P2: This test only proves that `xctest` skips the helper; it never verifies that the selected interactor actually captures, or that native avoids the XCTest interactor. Assert the selected owner is called and the other is not in each case.</violation>
</file>
<file name="docs/adr/0031-macos-native-app-backend.md">
<violation number="1" location="docs/adr/0031-macos-native-app-backend.md:60">
P3: The helper does not always identify the acted-on window: `windowTitle` is optional and comes from an optional accessibility title. Qualify this as being returned when available.</violation>
</file>
<file name="packages/platform-apple/src/os/macos/native-backend-facts.ts">
<violation number="1" location="packages/platform-apple/src/os/macos/native-backend-facts.ts:22">
P2: Also mark `screenRecordingCleanup` unavailable: cleanup of a macOS runner-backed recording calls `runRunner`, contradicting this backend's no-XCTest guarantee.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
The earlier concerns on 87340b8 are now fixed at 168af66, and I found no code problems that need a change before merge. The backend choice now comes from the owner and is read once, the screenshot crop target classifies by surface only, the native interactor no longer spreads the base interactor, and the abort signals reach the macOS screenshot and text reads. The required live evidence is in. It ran on c91f430, which is this head plus a ghost-cursor commit. That commit does not touch facts, admission or the interactor, so I accepted the run, but I did not rerun it. The open run started no runner, but I did not read close or setSetting on macOS, and those were not run live. CI is green: 21 checks, none failing at 168af66. There are no conflicts. I did not run Not blocking, and you can take or leave it: On the existing threads, two Cubic test-gap threads still apply: the missing multi-touch and drag cells in the facts test (r4176451591), and the weak snapshot routing assertions (r4176451594). One lower-priority ADR wording thread still applies, about the window title (r4176451599). The recording cleanup thread does not apply, because reattach is refused under native, so that cleanup cannot run (r4176451597). These are fixed at this head and can be resolved: r4174630088, r4174630111, r4174630063, r4174630105, r4174630103, r4174630083. I did not check threads r4174630072, r4174630095 and r4174630098 against this head. Nothing blocks merge. Please close the two test-gap threads and fix the ADR wording before merging into feat/macos-native-helper. |
`AGENT_DEVICE_MACOS_APP_BACKEND=native` routes macOS app sessions to the macOS helper's accessibility actions instead of the XCTest runner. The surface routing takes the backend explicitly, the native interactor refuses runner-only commands with a typed reason rather than starting XCTest, and helper refusals surface as UNSUPPORTED_OPERATION. ADR 0031 records the decision; docs cover the env vars and limits. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
168af66 to
4b52912
Compare
|
Thanks. Addressed at
|
|
The code looks good at 4b52912, and I found no new problems. The invalid-backend read is now lazy, and the new unit test covers it. The earlier findings from 168af66 (#3189 (comment)) are fixed. The Smoke Tests check was still running when I looked, so it is not green yet. It drives iOS and Android simulators. The change touches the Apple bind route at packages/platform-apple/src/runtime.ts#L284-L286 (https://github.com/callstack/agent-device/blob/4b52912/packages/platform-apple/src/runtime.ts#L284-L286). For non-macOS devices it still returns xctest without reading the env, so I expect no change on that route. I did not run the lazy-read change live. The live evidence from the earlier round covers valid values, and this change only affects when an invalid value is read. I also did not run There are no conflicts. Before merge, Smoke Tests needs to finish green on 4b52912. The cubic-dev-ai threads that no longer apply can be resolved. Fixed at this commit: the test coverage for performMultiTouchGesturePlan and performTargetAuthoredDrag (#3189 (comment)), the helper-vs-runner owner assertions (#3189 (comment)), the windowTitle wording in ADR 0031 (#3189 (comment)), the click/press/fill, type, and scroll route docs (#3189 (comment)), the ADR 0031 click refusal wording (#3189 (comment)), and the full scroll result assertion (#3189 (comment)). Benign: native-backend-facts.ts refuses start and reattach under native, so no runner recording exists for cleanup to run on (#3189 (comment)). No open threads still apply. |
Summary
AGENT_DEVICE_MACOS_APP_BACKEND=nativedrives macOS app sessions through the helper's accessibility actions (#3188) instead of the XCTest runner. XCTest stays the default. ADR 0031 records the decision.macOsNativeBackendFactsrefuses record,prepare,back, press-and-hold and gestures at admission (UNSUPPORTED_OPERATION,reason: unsupported-device-backend,dispatched: no).macOsNativeAppInteractoris assembled member by member; it does not spread the runner-backed interactor. Double/secondary clicks refuse before the helper runs; helper refusals keephelperReason.27 files. Over the 700-line production threshold; independently reviewed, findings addressed.
Validation
pnpm check:affected --runpassed on4b5291259(3,889 tests).ax-press/windowTitle, no runner process) is in the review reply below.🤖 Generated with Claude Code