feat(ios): route Simulator snapshots through AX bridge - #2279
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
Review at exact head
The route otherwise reuses the Apple runtime/facade and shutdown seams cleanly, and the merged source lifecycle already provides authoritative OS process-start-time checks around acquisition. |
|
Addressed all three actionable review items in fd3c6ac:
Also normalized fallback lineage to the declared targetId/generation fields only. Planted-red failures were observed for items 1 and 2 before the fix. Exact-head pnpm check:affected --run is green: 731 test files, 5,589 tests. The updated iOS native lane is running on this head. |
|
Re-review at exact head |
fd3c6ac to
1519222
Compare
|
Follow-up from the exact-head iOS rerun: after the requested evidence assertion was corrected, the lane exposed a real bridge-depth bug rather than a test-only mismatch. Regular --depth 1 was being sent to the native bridge as maxDepth: 1, so the TypeScript presenter never received the deeper structural nodes needed to project the visible child to presented depth 1. Fixed in 1519222: only rawTraversalDepth bounds native acquisition; regularPresentedDepth remains presentation-only. A planted-red adapter test observed 1 instead of the bounded maximum 10, while raw depth 1 remained 1. Focused tests pass, and the rebased exact-head affected gate is green with 734 files / 5,624 tests. |
|
Exact-head iOS rerun is green at 1519222. It passed clean-install bridge preparation, targeted XCTest regressions, public CLI preflight, Settings replay, the fixture-backed E2E scenario with the corrected depth frontier, and gesture replay. All PR checks are now green. |
|
The exact-head depth fix is correct: regular One readiness item remains: CI reports +35.2 kB npm unpacked (+11.2 kB gzip). Because this exceeds the repository threshold, please itemize the justified package growth and record which materially smaller designs were considered and why they were rejected. Until that rationale is recorded, I’m holding |
1519222 to
37e3ef5
Compare
|
Addressed the remaining size-review item. The CI size report at 1519222 measures +35.2 kB npm unpacked and +11.2 kB JS gzip:
Smaller alternatives considered: retaining XCTest-only routing would omit #2197; deleting XCTest would break required fallback and excluded targets; replacing the source facade with direct helper calls would duplicate or drop its existing deadline, process-generation, cache, and teardown safeguards. Moving the adapter into an optional package would move the installation cost rather than remove it, introduce a separate install/version boundary, and change the packaged-default contract. None is a justified reduction for this slice. The repository-required independent read-only size/design review found no actionable simplification: one runtime-owned lazy source is reused and closed, with the existing presenter and fallback retained. No production-code changes were needed for this comment. Rebased onto 0da105e; git range-diff confirms all three patches are unchanged. The new head is 37e3ef5. Prior green CI/device evidence is stamped to the old head; fresh validation is reported in the PR description. |
|
Reviewed 37e3ef5: no remaining actionable findings. Stale/unknown lineage and regular-depth regressions are resolved; existing lazy bridge, shared presenter and typed fallback retained. The +35.2 kB growth is itemized and smaller designs addressed. Prior live depth/raw-depth evidence covers the unchanged rebased patches. Code-review clean; fresh iOS CI was pending at review, so not an unconditional merge-ready claim. |
|
Delta review at e84bdc1, independently reviewed: no concrete source-level findings. Device/app-scoped target reuse requires matching process start identity; the bridge checks the pinned identity before capture and current identity after it, and stale acquisition explicitly refreshes discovery before fallback. Tests cover PID reuse, unavailable identity, replacement and refresh. Existing source/presenter/lifecycle owners are reused. Readiness remains held for the promised exact-base versus head six-screen benchmark and public-response evidence, still marked in progress; prior green evidence does not cover this new warm-path delta. Exact-head iOS is also pending. Removed the label that this sentinel applied to the previous head; restore only after the required evidence is complete and no failing checks/conflicts remain. |
|
Sentinel evidence update at unchanged e84bdc1: exact-head iOS Smoke is now failing in smoke:automation-input, waiting for Agent Device Tester. Typed reason wait_capture_stalled, readableCaptures=0, 10,000 ms budget / 10,233 ms elapsed (run 33985544618). This does not yet isolate a source defect, but requires investigation or successful exact-head validation. The promised matched six-screen benchmark/public-response evidence is also still marked in progress. Previous code review stands; readiness remains held. |
|
Acceptance update at
Still required before merge-readiness: publish the complete matched benchmark/public-response artifacts and obtain green final-head CI, including iOS. No new performance claim yet. |
|
Sentinel review at 7cf6e7f: Two confirmed readiness gaps: (1) the new native-runtime.test.ts beforeAll hits the 10s hook timeout in iOS run33987768263, so its native regression never executes; use an explicit bounded compile/hook budget or smaller compilation scope and establish green CI. (2) foreground-owner.m calls only isPrimaryForegroundProcess, not snapshotForProcess: removing either new pre/post ownership guard leaves this test green. Add regression proof that deleting either guard fails and covered/changing ownership yields typed failure without publishing the app tree. The foreground-owner implementation otherwise matches the intended fallback policy. Matched benchmark/public-response artifacts remain pending, and Size has the cross-version reporter-schema failure. No readiness label. |
7cf6e7f to
944edfc
Compare
|
Follow-up to the two findings in this review:
Observed with
Refusal cases assert no published tree, the exact typed ownership failure, request ID, and expected native acquisition count. The stable case requires a successfully materialized app tree, preventing an always-refuse implementation from passing. The test-only change was independently handed to the implementation worker and is integrated locally, awaiting the serialized final gate and PR push. Production runtime code is unchanged by this test fix. This records completed regression proof, not final CI acceptance. The worker continues to own final-head iOS validation and the matched six-screen benchmark/public-response artifacts; those remain open until their evidence is published. |
944edfc to
ae26f7a
Compare
|
The remaining acceptance evidence is now published; no benchmark waiver is needed. Immutable evidence corpus, tag
20 warm samples per screen per revision, 240 total; zero failed samples, unreadable trees, anchor mismatches, or admission stops. Head initial traces confirm bridge acquisition followed by TypeScript presentation; measured responses have no fallback warning. These are sequential same-device runs with uncontrolled host load, not a general performance guarantee or cold-start claim. The evidence also preserves live system-dialog fallback and generation-circuit recovery, plus the real native guard-deletion proof requested in review: five cases pass, PRE deletion fails three, POST deletion fails one. Final-head iOS CI executed the five cases successfully. All runnable checks remain green and GitHub reports CLEAN. The earlier stalled capture was documented as connection/discovery exhausting the request budget before bridge acquisition; it is not being presented as a proven bridge failure or a fully root-caused infrastructure incident. The PR body now reflects completed acceptance, with evidence off the source branch. The worker confirmed benchmark daemons stopped and the dedicated simulator removed. No additional code changes or duplicate device runs were needed. #2198 still owns runner-demand/open latency and proxy acceptance. Ready for maintainer merge; not merged by me. |
|
Sentinel at ae26f7a: clean coordinator + independent Terra/high review; merge-ready. The immutable a0e6418/pr-2279 corpus now closes the matched benchmark gap: exact base/head, 240 captures, no failures/anchor mismatches, bridge-to-TypeScript traces; independent checksum/corpus verification passed. The native regression exercises the real capture entry point with PRE/POST deletion proof, and exact-head iOS executes all five cases. All runnable CI checks are green and no conflicts. Size growth (+35,845 unpacked bytes) and scoped growth rationale are recorded; existing presenter/modal resolution are reused. Sequential uncontrolled-host measurements are not a general speed guarantee. Next action: maintainer merge; no merge performed. |
|
Summary
Closes #2197. Eligible local iOS Simulator snapshots use the packaged AX bridge and existing TypeScript presenter, with typed, generation-scoped XCTest fallback. Unknown generations remain incomparable; regular depth is presentation-only and raw depth bounds acquisition.
Target discovery is reused only while OS process-start identity matches. Native acquisition verifies the foreground owner before and after capture, preventing app-only trees beneath system dialogs. XCTest retains modal resolution; fallback disables the bridge until that app generation changes. Provider, physical-device, interaction, and prewarm routes are preserved.
Scope: 34 files, 1,353 gross lines; maintainer-approved exception for required correctness and regression coverage.
Validation
Head:
ae26f7afc064c75c65c71e677d32d77cdf2f9709; matched base:7a2d48d160aaacb582c6f4cde98b6af7531bf7af.evidence/ios-snapshot, taggedevidence/ios-snapshot/ae26f7afc0: raw responses, traces, provenance, checksums, and verification script.pnpm check:affected --runpassed (746 files / 5,691 tests); all runnable CI checks green. Accepted Size growth: +35,845 unpacked bytes.Acceptance evidence is complete. Runner-demand/open latency and proxy acceptance remain #2198 scope.