Skip to content

fix: prevent private AX subtree leaks at source - #1807

Merged
thymikee merged 3 commits into
mainfrom
fix/private-ax-followup
Aug 18, 2026
Merged

fix: prevent private AX subtree leaks at source#1807
thymikee merged 3 commits into
mainfrom
fix/private-ax-followup

Conversation

@thymikee

@thymikee thymikee commented Aug 18, 2026

Copy link
Copy Markdown
Member

Summary

Prevent private-AX descendants of an offscreen scroll item from reappearing at a clamped visible edge. Element settings rows such as Theme, Notifications, and About now stay excluded from interactive snapshots when their ancestor is offscreen.

This follow-up removes the downstream collapsed-cluster detector and second iOS presentation pass introduced in #1798 while retaining Maestro source-index provenance. XCTest settle projection now uses the canonical visibility rules with linear behavior for nested and flat scroll trees.

Related to #1797; this does not implement or close the broader acquire/present migration.

Scope: 22 files across Apple runner presentation, canonical snapshot visibility and settle, daemon iOS presentation, and Maestro provenance.

Validation

The new XCTest regression was proven red before the fix: an offscreen Theme cell leaked two clamped Theme descendants. It passes after the fix alongside the geometryless-content and scroll-hint regressions.

Two settle-signal regressions were proven red independently: a visible node changing only its value incorrectly compared stable, while whitespace-only semantic changes incorrectly compared unstable. The signal retains value and preserves canonical semantic trimming; the focused settle/wait suite passes 24 tests.

iOS and macOS runner builds with unit regressions enabled passed. Focused unit and integration tests cover composed replacement updates, tree and flat-tree settle linearity, iOS presentation, snapshot capture, and Maestro target provenance. The full affected gate passed on the pushed head: 556 test files and 4,724 tests passed with 100% changed-line coverage. A final Sol high review found no remaining in-scope issues on the current head.

No docs or skills changed because this is an internal correction to existing snapshot behavior; the XCTest CI allowlist was updated for the new regression.

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.27 MB 2.26 MB -5.5 kB
JS gzip 746.2 kB 744.6 kB -1.5 kB
npm tarball 865.4 kB 864.0 kB -1.5 kB
npm unpacked 3.02 MB 3.01 MB -5.3 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.6 ms 26.5 ms -0.1 ms
CLI --help 65.9 ms 65.6 ms -0.4 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/prepare-kind.js -5.1 kB -1.4 kB
dist/src/internal/daemon.js -349 B -136 B
dist/src/session2.js -56 B -7 B
dist/src/interaction2.js -2 B -4 B
dist/src/agent-device-client.js 0 B -2 B

@thymikee

Copy link
Copy Markdown
Member Author

Code-quality review

This is the right shape: the leak is closed where the information lives (appendPrivateAXNode now carries ancestorOffscreen, so an excluded offscreen parent's clamped-frame children can't re-enter), and everything that existed downstream only to detect that leak is gone — the collapsed-cluster heuristic, the second presentation pass, viewport.ts, and settle's private forked viewport resolver. Net −1080 lines with no behavior lost that we want. Settle now goes through the canonical isNodeVisibleOnScreen, and the updateReplacement(current => patch) change fixes the derive-from-raw-node footgun at the API instead of at one call site. 👍

Verified locally on the PR head: the touched unit suites pass (88 tests: stable-capture-signal, snapshot-presentation/*, snapshot-capture, daemon-runtime-port-observation, contracts snapshot, maestro runtime-targets-typed). Swift regression reads correctly for the Theme/clamped-children case.

Two things worth tightening; neither blocks:

1. precomputedViewportRects lets a caller change semantics, not just cache them. resolveViewportRect collects root rects with hasValidRect (finite), while settle precomputes them with isPositiveFiniteRect (positive size). Today the difference is only a zero-area Application rect (fail-open before, falls through to the "largest containing rect of any node" fallback now), but the parameter is documented as a precompute and it's threaded as a 4th optional positional through four functions. Suggest contracts exports the collector — collectViewportRootRects(nodes) — and callers pass that result. Then the arg is provably a cache of the same computation and the predicate can't drift between call sites.

2. mergeReplacement and updateReplacement are two exports for one operation. mergeReplacement(r, n, patch) is updateReplacement(r, n, () => patch). Either define it that way so the relationship is explicit, or migrate the remaining mergeReplacement sites and drop it.

Notes, no action needed:

  • The hint registration now fires only for geometrically-offscreen nodes (!ancestorOffscreen && hasFrame && !onScreen), not for negligibleDecoration — that's a quiet correction over fix: unify private AX scroll visibility #1798 and it's the right one; just confirming it's intentional.
  • Settle no longer has a TS-side guard for sub-pixel private-AX decoration or value churn; it now relies on the runner's negligibleDecoration from fix: unify private AX scroll visibility #1798. Fine since the runner ships with the package, but it does mean settle-vs-runner version skew is a thing to keep in mind if the runner build ever lags a CLI release.

@thymikee
thymikee merged commit 0d3b741 into main Aug 18, 2026
31 checks passed
@thymikee
thymikee deleted the fix/private-ax-followup branch August 18, 2026 07:23
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-18 07:24 UTC

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