Repository navigation
fix(ios): stop the XCTest-channel penalty from re-arming itself - #3896
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed 2fd5cd8. The post-response recovery probe can leave abandoned XCTest work on the main thread, so the next tap can fail with RUNNER_BUSY. After an accepted private-AX snapshot, scheduleSnapshotXCTestRecoveryProbeIfNeeded queues a probe on the command queue. The probe runs the whole recursiveTree capture inside one main-thread block, so app.snapshot() runs inline with no slice. On the hostile screen that armed the penalty, that call outlasts 1 s. The waiter gives up, and abandonedMainThreadWorkCount stays above 0 until the call drains. The issue's 72 tree_backend_timeout lines are this same 1 s abandonment. The next command, usually the tap after the snapshot, is refused by executeDispatched with RUNNER_BUSY. Mutating commands get no busy resend. The snapshot response already said runnerMainThreadBusy=false, so the daemon's occupancy view is also stale for retention and handoff. On hostile screens, a snapshot then tap loop would get a tap failure every 30 s that the caller did not cause. Before this PR, an accepted private-AX capture left the main thread idle. The rule the code must satisfy: the runner starts no XCTest main-thread work after it has answered a command, so every response's runnerMainThreadBusy stamp stays true until the next command. Please drop the post-response probe. If periodic recovery is still wanted, can the due tree probe run in-band in the deferred plan instead? Then any abandonment is charged to the snapshot response and its busy stamp. The live runs only measure snapshot sequences. Snapshots skip XCTest tiers while abandoned work drains, so they cannot show the RUNNER_BUSY window. No run sends a mutating command after a scheduled probe. If the probe stays, please run this on the heavy feed. Arm the penalty, wait more than 30 s, and take a snapshot that logs RECOVERY_PROBE_SCHEDULED. Then run Could the PR be smaller? The channelState-gated penalty evidence and the early clear on an accepted fast in-plan tree probe look right and well owned. Could the post-response scheduler, nextRecoveryProbeAt and the stale-generation check go? If light screens still need recovery before expiry, effectiveSnapshotCapturePlan could put the bounded recursiveTree probe ahead of privateAX when the penalty state says a probe is due. Could the penalty fields also become one SnapshotXCTestChannelPenalty type with arm, clear and isProbeDue, so the plan derives the probe order from it and not from a parallel timestamp? The one failing check is lint, on the 1003-line cap in packages/platform-apple/src/snapshot-route.test.ts. This PR touches only apple/runner Swift files and ADR 0004, and the cap breach comes from main (#3890), so it looks unrelated. I could not tell why the occupancy test lost its second-plan assertion that the armed penalty defers the next plan's XCTest tiers. No other test at this head asserts that a deferred plan starts no sweep query. #3895 shares no code with this PR, but both amend ADR 0004 and both reference #3844, so expect a possible text conflict there. I did not run the XCTest suite or a device. The RUNNER_BUSY finding is traced from the code path and the 1.01 s per-probe timing in the PR body. I did not see it in a runner.log. No conflicts are known. Before merge, the probe needs to go in-band or be removed, or the snapshot then press run above needs to pass on the hostile screen. |
Drop the post-response recovery probe: it started XCTest main-thread work after the snapshot had answered, so its abandoned tree work could refuse the next tap with RUNNER_BUSY while the snapshot's occupancy stamp said idle. The penalty is now one SnapshotXCTestChannelPenalty value (arm, clear, isProbeDue, claimProbe). When a probe is due, effectiveSnapshotCapturePlan puts the bounded tree ahead of private AX in the deferred plan, so any abandoned probe work is charged to that capture and its busy stamp. Restore the occupancy test's second-plan assertions that an armed penalty defers the next plan's XCTest tiers and starts no sweep query. Refs #3844 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the post-response probe text with the in-band policy and note that, after the iPad amendment, Simulator captures meet it only when the bridge sends them to XCTest. Refs #3844 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks. Addressed at 1. Post-response probe → dropped. 2. One penalty type. 3. Tests. I restored main's sweep-timeout occupancy test with both second-plan assertions: an armed penalty defers the next plan's XCTest tiers, and a deferred plan starts no sweep query. New tests:
Four planted mutations are detected: ignoring the due flag, not claiming, sweep in the deferred plan, and a plan that ignores the penalty. The last one fails both restored assertions ( 4. ADR 0004. The #3895 amendment merged cleanly. Our paragraph now describes the in-band rule and notes that, after the iPad amendment, Simulator captures meet it only when the bridge sends them to XCTest. 5. Your tap check. iPad Pro M5 / iOS 27.0, Release heavy feed, bridge kept off for iPad by one uncommitted harness line in each checkout. Each iteration: arm the penalty, wait 31 s,
The press does not succeed on either revision. On base and head alike, every runner tap on this ≥27-wrapper feed fails with A client that sends the tap within ~4 ms of such an answer would still be refused. That is the same window that every in-plan XCTest tier has today: the arming query sweep abandons the same way. Closing it would need the plan to wait for its own abandoned work, so I left that out of this PR. 6. Cost. 60 hostile snapshots / 5 min, two runs per revision: PENALIZED/CLEARED/DEFERRED 3/0/57 on both, 7 probes per head run, each ~2.25 s runner-side. Pooled median/p95 1.563/4.923 s → 1.558/4.900 s; base p95 alone moves 0.6 s between runs. Heavy→Settings: 1/0/24 → 1/1/7. The first probe cleared the penalty 32 s after arming (168 ms, the tree answered), where base stayed deferred until 122 s. 7. Lint. The |
|
This PR is ready for human review at 7640bdc. The earlier finding from the first review is fixed, and all 16 checks pass with none failing. The earlier lint failure on the line cap in Not blocking, and fine to take or leave: on a hostile screen whose tree takes longer after the 1 s slice than the private-AX tier, a due probe can answer with On evidence: I did not run the XCTest suite, the mutations, or a device, and the live numbers and the |
|
* origin/main: perf(ios): confirm fill focus from the tapped input before the keyboard (#3906) feat(perf): time every iOS command against the fixture app (#3885) fix(ios): stop the XCTest-channel penalty from re-arming itself (#3896) test(android): pin that a child vanishing mid-read reads as gone (#3905) ci: skip device smoke lanes when a PR changes no lane input (#3904) fix(ios): verify deep-tree back on the sole screen (#3894) fix(ios): synthesize long presses and the gesture viewport on deep trees (#3893) perf(ios): read each resolved text input through one snapshot (#3882) fix(gate): take the host validation lock for heavy gates (#3900) fix(logs): keep an early iOS simulator logs stop from hanging the session (#3892) fix(apple): clean the runner derived path the runner uses (#3897) test(provider-scenarios): open deadline scenarios without the test IME (#3903) test(host-kit): stub os.homedir for the exec-trace late-close test (#3898) feat(test-app): add a Bluesky-depth heavy feed screen (#3884)
Summary
SnapshotXCTestChannelPenaltyvalue (arm / clear / isProbeDue / claimProbe). Every 30 s a due probe runs in-band:effectiveSnapshotCapturePlanputs the 1 s tree ahead of private AX, so abandoned probe work is charged to that capture and its busy stamp. No XCTest work starts after a response. ADR 0004 updated. 8 files, no knobs.Refs #3844
Validation
Head
7640bdc6e6(Swift identical to092d21aa4a); base5f6dfb3e6f. Owned iPad Pro M5 / iOS 27.0, Release heavy feed. Both checkouts carry the same uncommitted harness line that keeps iPad off the host AX bridge (#3895), so the runner penalty is exercised.RUNNER_BUSY command=tap3/3 on2fd5cd8→ 0/10. The press itself fails on base too (deep-treepoint.x != INFINITY, see fix(ios): synthesize long presses and the gesture viewport on deep trees #3893).check:xctest-selectionandcheck:affected --run --head 7640bdc6e6passed (exactCommit=true). CI owns unit-ci and device lanes.🤖 Generated with Claude Code