Skip to content

fix(ax-bridge): report inactive Flutter semantics#765

Merged
shaun0927 merged 1 commit into
developfrom
fix/693-flutter-semantics-inactive-diagnostic
May 17, 2026
Merged

fix(ax-bridge): report inactive Flutter semantics#765
shaun0927 merged 1 commit into
developfrom
fix/693-flutter-semantics-inactive-diagnostic

Conversation

@shaun0927
Copy link
Copy Markdown
Owner

Summary

  • Adds FLUTTER_SEMANTICS_INACTIVE as a typed final recovery failure for Flutter bundle targets.
  • Promotes exhausted DEVICE_CONTENT_ROOT_EMPTY recovery only when a bundle-scoped reactivation attempt returned false.
  • Preserves the original empty-tree error for non-bundle/native-app calls.

Issue analysis / scope

Issue #693 still contains one unresolved, live-gated slice: release-build Flutter semantics can remain inaccessible after coordinate-space and post-tap improvements. The issue thread explicitly cautions against speculative SpringBoard/Flutter activation changes without live release-build evidence.

This PR therefore implements the deterministic, repo-testable part of that remaining scope: callers now get a stable typed diagnostic (FLUTTER_SEMANTICS_INACTIVE) when the wrapper can prove that:

  1. the target is bundle-scoped,
  2. the native accessibility tree stayed empty through the retry budget, and
  3. forced semantics reactivation returned false.

This makes downstream handling and telemetry reliable while leaving live activation behavior for a separately evidenced follow-up.

Validation

  • npx jest --runTestsByPath tests/unit/ax-bridge-recovery.test.ts --runInBand
  • npx eslint --max-warnings=0 src/native/ax-bridge-recovery.ts tests/unit/ax-bridge-recovery.test.ts
  • npx tsc --noEmit --pretty false

Refs #693

Promote exhausted DEVICE_CONTENT_ROOT_EMPTY recovery for Flutter bundle targets to FLUTTER_SEMANTICS_INACTIVE when semantics reactivation returns false, giving callers a deterministic diagnostic instead of another generic empty-tree failure.

Constraint: Issue #693 still needs live release-build Flutter evidence for a true activation fix, so this PR stays within deterministic repo-testable diagnostics.
Rejected: Adding speculative SpringBoard or Flutter VM-service activation changes | current issue evidence says live verification is required before changing that behavior.
Confidence: high
Scope-risk: narrow
Directive: Only promote bundle-scoped failures; native-app empty trees without bundleId must preserve their original AX error code.
Tested: npx jest --runTestsByPath tests/unit/ax-bridge-recovery.test.ts --runInBand; npx eslint --max-warnings=0 src/native/ax-bridge-recovery.ts tests/unit/ax-bridge-recovery.test.ts; npx tsc --noEmit --pretty false
Not-tested: live iPhone 17 Pro release-mode Flutter app semantics activation
@shaun0927
Copy link
Copy Markdown
Owner Author

PR Review Summary

Verdict

Approve

Scope Reviewed

  • PR intent: provide a deterministic typed diagnostic for the unresolved Flutter semantics-inactive slice of iPhone 17 Pro simulator taps use wrong coordinate space and AX post-tap probe fails #693 without speculative live activation changes.
  • Main changed areas: ax-bridge recovery final error promotion and unit tests.
  • Tests reviewed: typed promotion for bundle-scoped false reactivation and preservation of native/non-bundle empty-tree error.
  • Checks considered: local jest/eslint/tsc passed; GitHub dependency-audit/lint/test/build passed.

Blocking Issues

None.

Warnings

None.

Mutation-Test Thinking

  • Likely mutants that should be killed: removing bundleId guard, changing final code from FLUTTER_SEMANTICS_INACTIVE, removing original native-code preservation.
  • Mutants current tests may not catch: promotion behavior when reactivation throws instead of returning false; current scope intentionally only promotes proven inactive semantics via REACTIVATE_RETURNED_FALSE.
  • Additional tests recommended: none required for this PR scope.

Complexity / CRAP-style Risk

  • High-risk functions/modules: moderate importance but low diff complexity.
  • Complexity increase: one predicate and one error-construction helper.
  • Test coverage concern: none for repo-testable diagnostic behavior.
  • Refactoring recommendation: none.

Test Quality Assessment (6/7)

  • Strong tests: cover both promotion and non-promotion boundary.
  • Weak tests: live iPhone 17 Pro Flutter release activation remains untested by design.
  • Missing edge cases: no blocking gap.
  • Mocking concerns: deterministic bridge/reactivate mocks are appropriate.

Security / Operational Risk

None. Diagnostic message contains bundle id already supplied by caller; no secret or external command behavior changes.

Looks Good

  • Narrowly avoids speculative SpringBoard/Flutter behavior changes.
  • Keeps native app empty-tree failures backward-compatible.
  • Exposes a stable code that downstream callers can branch on.

Final Recommendation

Approve.

@shaun0927
Copy link
Copy Markdown
Owner Author

Ready to merge — approve summary

All required CI checks are green on the latest commit 00acadf2 (lint, test, dependency-audit, build — all SUCCESS). No P0/P1 findings from any bot (Codex did not raise any line-level review comments on this PR). Local verification on the same SHA: npx tsc --noEmit --pretty false exits 0, npx eslint --max-warnings=0 on the two touched files exits 0, and npx jest --runTestsByPath tests/unit/ax-bridge-recovery.test.ts --runInBand reports Tests: 14 passed, 14 total. This summary records why the PR is in a mergeable state.

What this PR is

A deterministic, repo-testable slice of the unresolved part of #693. Issue #693 still contains one live-gated failure mode — release-build Flutter targets whose native accessibility tree never rematerialises after coordinate-space and post-tap improvements — and the issue thread explicitly cautions against speculative SpringBoard/Flutter activation changes without live release-build evidence.

This PR therefore lands only the typed-diagnostic half of that scope: callers now get a stable FLUTTER_SEMANTICS_INACTIVE error code when the recovery wrapper can prove that

  1. the target is bundle-scoped (options.bundleId set),
  2. the native tree stayed empty through the entire retry budget (final DEVICE_CONTENT_ROOT_EMPTY), and
  3. forced semantics reactivation returned false at least once (REACTIVATE_RETURNED_FALSE stage).

When any of those preconditions is missing, the original error is preserved unchanged, so native-app and non-bundle paths are completely unaffected.

Scope / direction review

Axis Verdict
Aligned with project direction ✅ Closes the typed-diagnostic gap from #693 without touching the live-activation surface that the issue explicitly gates behind release-build evidence.
Single concern ✅ One file (src/native/ax-bridge-recovery.ts, +28/−3) and its dedicated unit-test file (tests/unit/ax-bridge-recovery.test.ts, +50/−0). No drive-by changes.
Over-engineering risk ✅ Considered and rejected: (a) auto-issuing a SpringBoard reactivation here (out of scope per the #693 caution about live evidence), (b) widening the promotion to any failed reactivation (would mask legitimate REACTIVATE_TIMEOUT / REACTIVATE_THREW failures whose recovery story is different), (c) splitting the predicate and the constructor into a class hierarchy (two small helpers are enough — total +28 LOC of source).
Public API surface change ✅ Additive only: a new exported constant FLUTTER_SEMANTICS_INACTIVE. The thrown-error shape is unchanged (still AccessibilityBridgeError with an attached recovery report); only the code value on the specific path widens to a new typed value. Existing try { ... } catch (err) { if (err.code === 'DEVICE_CONTENT_ROOT_EMPTY') ... } blocks for native-app callers still match because the native path preserves the original code.

Why merge is OK

  • Scope discipline. The diff is 85+/3−. Two helpers (shouldPromoteFlutterSemanticsInactive, promoteFlutterSemanticsInactive) and a constant. The dumpTreeWithRecovery orchestration loop is touched in exactly one place at the final-throw site, computed into surfacedError once and used consistently for both attachRecoveryReport(...) and throw.
  • Backward compatibility is asserted by tests. persistent empty native tree without bundleId preserves original error code locks the native-app behaviour at code: 'DEVICE_CONTENT_ROOT_EMPTY', message: 'empty 3'. The promotion test asserts that the recovery report's lastErrorCode is also promoted to FLUTTER_SEMANTICS_INACTIVE, so telemetry and the throw stay consistent.
  • No new runtime behavior in the happy path. When reactivate returns true or no REACTIVATE_RETURNED_FALSE stage is recorded, shouldPromoteFlutterSemanticsInactive short-circuits to false and the original error is thrown verbatim.
  • No security / operational surface change. The error message embeds only options.bundleId (caller-supplied) and the original error message; no secrets, no shell-outs, no new I/O.

Verification on 00acadf2

$ npx tsc --noEmit --pretty false          # exit 0
$ npx eslint --max-warnings=0 \
    src/native/ax-bridge-recovery.ts \
    tests/unit/ax-bridge-recovery.test.ts  # exit 0
$ npx jest --runTestsByPath tests/unit/ax-bridge-recovery.test.ts --runInBand
Tests:       14 passed, 14 total

GitHub CI on the same commit: lint ✅ · test ✅ · dependency-audit ✅ · build ✅.

Out-of-scope follow-ups (not blocking this PR)

  1. Live release-build activation work for iPhone 17 Pro simulator taps use wrong coordinate space and AX post-tap probe fails #693. Still gated behind real device evidence per the issue's own guidance — not part of this PR.
  2. Downstream consumer switch-over to FLUTTER_SEMANTICS_INACTIVE. Telemetry/error-routing callers can branch on the new code at their own cadence; the typed constant is now stable and exported.

Approving on the strength of green CI, zero bot findings, the scope-discipline above, and the two-test coverage that locks both the promotion and the native-path preservation.

@shaun0927
Copy link
Copy Markdown
Owner Author

PR #765: fix(ax-bridge): report inactive Flutter semantics

/pr-review-os skill pass — 6-axis (WebKit / iOS / Simulator / MCP / Architecture / Reliability) sweep on the latest commit 00acadf2.

P0 — Blockers (must fix before merge)

None.

P1 — Must Fix (should fix in this PR)

None.

P2 — Improve (can be follow-up)

None at the >= 60/100 confidence threshold.

Per-axis findings

Axis Finding
WebKit Protocol Not applicable — this PR only touches the Swift accessibility-bridge wrapper (dumpTreeWithRecovery). No WebKit Remote Debugging Protocol calls are added or modified. No Page.* / Network.* / Runtime.* invocations are introduced.
iOS Safari Compatibility Not applicable — no DOM, no Touch / TouchEvent, no CSS or input behaviour changes. The code path runs in Node, invoking AccessibilityBridge.dumpTree() over the Swift bridge socket.
Simulator Not applicable — no simctl invocations are added. Reactivation continues to delegate to ensureSemanticsActive(...) which is unchanged by this PR.
MCP Protocol src/native/ax-bridge-recovery.ts contains zero console.log calls (only the existing // comments). The thrown-error shape is unchanged (AccessibilityBridgeError with attached recovery report); only the code field on one specific path widens to a new typed value (FLUTTER_SEMANTICS_INACTIVE). MCP responses that surface this code via meta.errorCode keep their JSON-RPC envelope structure intact.
Architecture ✅ No puppeteer-core / CDP / Chrome imports. The new FLUTTER_SEMANTICS_INACTIVE constant is exported as a typed string literal alongside the existing RECOVERABLE_ERROR_CODES / NON_RECOVERABLE_ERROR_CODES sets. shouldPromoteFlutterSemanticsInactive is a pure boolean predicate (no any), promoteFlutterSemanticsInactive returns a new AccessibilityBridgeError instance (preserves instanceof checks downstream). No dead code — both helpers have exactly one call-site at the final-throw point of dumpTreeWithRecovery.
Reliability ✅ Promotion is fully guarded by three preconditions (bundleId set, final code DEVICE_CONTENT_ROOT_EMPTY, at least one REACTIVATE_RETURNED_FALSE stage in the recovery report). When any precondition is missing the original error is thrown verbatim, so native-app and non-bundle callers are unaffected (locked by persistent empty native tree without bundleId preserves original error code). The recovery report's lastErrorCode is computed from surfacedError.code, keeping telemetry consistent with the thrown error. Reactivation continues to be best-effort (catch + record stage, do not abort the retry loop).

Test coverage delta

Test Mutant killed
persistent empty Flutter tree with failed reactivation surfaces a typed semantics-inactive error (a) changing the final code from FLUTTER_SEMANTICS_INACTIVE, (b) failing to set lastErrorCode to the promoted code, (c) dropping the original-error suffix in the message.
persistent empty native tree without bundleId preserves original error code (a) removing the bundleId guard in shouldPromoteFlutterSemanticsInactive, (b) accidentally promoting native-app failures.
(Existing) records outcome: error when reactivate resolves false Anchors the REACTIVATE_RETURNED_FALSE stage that the new predicate depends on.
(Existing) thrown error carries the recovery report for diagnostics Confirms attachRecoveryReport is invoked on the throw path.

Verification

$ npx tsc --noEmit --pretty false          # exit 0
$ npx eslint --max-warnings=0 \
    src/native/ax-bridge-recovery.ts \
    tests/unit/ax-bridge-recovery.test.ts  # exit 0
$ npx jest --runTestsByPath tests/unit/ax-bridge-recovery.test.ts --runInBand
Test Suites: 1 passed, 1 total
Tests:       14 passed, 14 total

GitHub CI on 00acadf2: lint ✅ · test ✅ · dependency-audit ✅ · build ✅.

Summary

Priority Count
P0 0
P1 0
P2 0

Verdict

APPROVE.

Rationale, in order of weight:

  1. Zero blocking findings on any of the six review axes; the diff is 2 files, 85+/3− net, with the entire orchestration loop touched in exactly one place.
  2. Scope discipline. The PR implements only the typed-diagnostic slice of iPhone 17 Pro simulator taps use wrong coordinate space and AX post-tap probe fails #693 — the live-build SpringBoard / Flutter activation work that the issue explicitly gates behind release-build evidence is not touched here. This is the right scope split.
  3. Backward compatibility is asserted, not assumed. Two tests lock the symmetric pair: native-app callers still see DEVICE_CONTENT_ROOT_EMPTY, bundle-scoped callers with a proven REACTIVATE_RETURNED_FALSE stage now see FLUTTER_SEMANTICS_INACTIVE.
  4. No new runtime surface in the happy path. When reactivate returns true or no REACTIVATE_RETURNED_FALSE stage is recorded, shouldPromoteFlutterSemanticsInactive short-circuits to false and the original error is thrown verbatim. CPU cost is one array .some(...) scan over a bounded stage list, only on the throw path.
  5. No security / operational surface change. The new error message embeds only caller-supplied options.bundleId and the original error message; no shell-outs, no new I/O, no secret-bearing fields.

Merge Notes

No file overlap with PR #764 (fix/763-network-interceptor-session-evict) — independent merge order; either can land first. No file overlap with PR #762 (release/0.6.1) on this change (ax-bridge-recovery.ts is unchanged by the release PR's net-new commits). Safe to merge immediately once a maintainer click-through is in place.

/pr-review-os skill, senior-engineer pass.

@shaun0927 shaun0927 merged commit ff416a3 into develop May 17, 2026
4 checks passed
@shaun0927 shaun0927 deleted the fix/693-flutter-semantics-inactive-diagnostic branch May 17, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant