You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Two days of running the Bluesky e2e suite on one simulator left a 374,000-line runner.log. The per-command breakdown:
Marker
Lines
FAST_APP_GUARD
35,080
COMMAND_ACCEPTED / COMMAND_COMPLETED
39,415 / 39,408
SYNTHESIZED_GESTURE_POLICY
5,612
PRIVATE_AX_SNAPSHOT_USED
5,639
SNAPSHOT_XCTEST_CHANNEL_DEFERRED
5,467
The accepted/completed pairs are worth every line (the whole investigation was built on them). The guard and policy markers restate, on every command, a fact that had not changed since the previous one: same bundle in the foreground, same gesture policy for the same kind.
Change
RepeatedLogSuppressor (in RunnerTests+Lifecycle.swift): logIfChanged(key:fact:line:) writes line only when fact differs from the last one written under key. Locked, reset() forgets every key.
canUseFastForegroundAppGuard: keyed fast_app_guard, fact is bundle plus state; the line still carries the command that observed the change.
logSynthesizedGesturePolicyDecision: keyed per gesture kind so alternating taps and scrolls do not restate each other; the decision line is the fact.
invalidateCachedTarget resets the suppressor, so a rebind states the guard once more.
Snapshot markers are left alone here: they carry per-capture numbers (nodes, depth) and are the evidence for the busy-runner work in #2804 and #2783.
Validation
Two unit tests under the existing #if AGENT_DEVICE_RUNNER_UNIT_TESTS guard: the suppressor writes on first sight, change, other key and after reset; invalidating the cached target resets it. Both pass on iPhone 17 Pro Max (iOS 27.0) via xcodebuild test-without-building.
End to end, daemon pointed at this runner build, Settings on the same simulator: 8 tap commands wrote 1 FAST_APP_GUARD line and 1 SYNTHESIZED_GESTURE_POLICY line.
pnpm check:xctest-selection (277 methods, 0 unreachable) and pnpm check:packaged-runner-swift (50 files parse, line parity kept) clean.
…they change
Two days of one e2e suite left a 374k-line runner.log: 35,080 FAST_APP_GUARD
and 5,612 SYNTHESIZED_GESTURE_POLICY lines, one per command, each restating a
fact that had not changed since the previous command.
Both markers now go through a small RepeatedLogSuppressor keyed by marker (and
gesture kind for the policy): a line is written when the fact differs from the
last one written under that key, and the suppressor is reset with the cached
target so a rebind states the fact once more. COMMAND_ACCEPTED/COMPLETED pairs
are untouched.
Verified on an iOS simulator against Settings: 8 tap commands wrote 1 guard
line and 1 policy line.
Review follow-ups on the deduped per-command markers:
- the RepeatedLogSuppressor class (string keys, lock, autoclosure) is gone;
the two markers keep a last-written line beside the other target-bound state,
the policy one keyed by SynthesizedGesturePolicyKind
- the target-reset chorus copied at three sites (host activate, invalidate,
activate target) is one resetTargetBoundState(), so a rebind through any of
them restates both markers, as the comment already claimed
- the guard line drops command=, which a deduped line would misreport; the
command is on the adjacent COMMAND_ACCEPTED line
- the policy line is built by one pure function with its own unit test
This test only checks the Boolean return value; it never observes or evaluates line, so it would still pass if the NSLog at lines 499–500 were removed or moved to the wrong branch. The behavior being changed is the actual runner.log emission count/content, so add a logger seam or a side-effecting autoclosure assertion that verifies first/change/reset calls emit and repeated facts do not.
Restructured after an internal quality review (a2f0a5c):
The RepeatedLogSuppressor class is gone. Each marker keeps a last-written line as a plain field beside the other target-bound state; the policy one is keyed by SynthesizedGesturePolicyKind, no string keys, no lock.
The target-reset chorus that was copied at three sites (host activate, invalidate, activate target) is one resetTargetBoundState(), so a rebind through any of them restates both markers. The previous version only reset on invalidate, which the comment overstated.
The guard line drops command=: a deduped line carrying one command would misreport; the command is on the adjacent COMMAND_ACCEPTED line.
The policy line is built by a pure function with its own unit test. Both new tests pass on the simulator; end to end, 8 taps still write 1 guard line and 1 policy line.
refreshCachedTargetIfProcessChanged (https://github.com/callstack/agent-device/blob/a2f0a5c/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift#L203-L224) rebinds currentApp and currentAppProcessIdentifier when the same bundleId's process silently changed, but it clears state with clearRememberedTextEntryTap() instead of the new resetTargetBoundState(), so lastLoggedFastAppGuardLine and lastLoggedGesturePolicyLines carry over from the old process. When the recomputed bundle+state fact or gesture-policy fact matches the stale cached string, the guard or policy line for the new process is silently dropped from runner.log. This is the exact rebind case the PR's own doc comment says every binding site must restate, and it defeats the marker during the kind of investigation this change is meant to support. Every site that binds, rebinds, or drops the target must call resetTargetBoundState() (or clear both fields directly); refreshCachedTargetIfProcessChanged is the one call site that currently doesn't.
testSynthesizedGesturePolicyMarkerWritesOncePerKindUntilTheDecisionChanges (https://github.com/callstack/agent-device/blob/a2f0a5c/apple/runner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedGesturePolicy.swift#L168) only checks the lastLoggedGesturePolicyLines dictionary, never NSLog output, and canUseFastForegroundAppGuard's dedup path has no direct test at all; testResettingTargetBoundStateForgetsTheLastWrittenMarkers only covers the reset, not suppression. Since the dictionary write sits inside the same if as the NSLog call, a change that keeps the write but drops the log gate would still pass everything green, and the reduced-duplicate-logging behavior this PR exists for would be unverified. Every call path that writes lastLoggedFastAppGuardLine or lastLoggedGesturePolicyLines needs a test asserting emission count (not just dictionary state) across a repeated identical fact versus a changed fact.
Not blocking: the PR description says the line still carries the command that observed the change, but the shipped line drops command and the command: CommandType parameter in canUseFastForegroundAppGuard is now unused, so either the description or the dead parameter needs fixing, though this can be taken or left.
I read the diff and the full pre/post RunnerTests+Lifecycle.swift and RunnerTests+SynthesizedGesturePolicy.swift at a2f0a5c; I did not run the Swift unit tests or a live device session. The author reports that 8 tap commands wrote 1 FAST_APP_GUARD line and 1 SYNTHESIZED_GESTURE_POLICY line, but no runner.log excerpt is attached, so that's the author's account, not something I confirmed. What's missing is a live run that forces refreshCachedTargetIfProcessChanged's path — same bundleId, process replaced without an explicit invalidate or relaunch — showing the guard and policy lines restate for the new process once the fix above lands; the steady-state repeat-command case doesn't exercise this path. Smoke Tests were still queued at the time I looked, so I can't say whether they pass; if they do fail, note that Smoke Tests exercise real command dispatch on a simulator and would hit canUseFastForegroundAppGuard, activateTarget, invalidateCachedTarget, and refreshCachedTargetIfProcessChanged directly, so a failure there would be attributable to this diff. Before this can merge, refreshCachedTargetIfProcessChanged needs to call resetTargetBoundState() so a silent process rebind can't suppress its own guard or policy line.
The iOS smoke failure here is the same lane flake as on #2804 and #2811 (regular depth-1 snapshot must disclose the Simulator AX bridge evidence gap), now on five branches today. This PR only dedupes two log lines in the runner; every other check is green. Evidence collected on #2491.
… they write
refreshCachedTargetIfProcessChanged rebinds the target but only cleared the
text-entry tap, so the new process could lose its guard or policy line. It now
runs resetTargetBoundState. Markers write through runnerMarkerWriter so tests
count emitted lines, and the fast app guard dedup is testable on its own. Drops
the unused command parameter of canUseFastForegroundAppGuard.
Co-Authored-By: Claude <noreply@anthropic.com>
Reviewed at f18238c. The fix at RunnerTests+Lifecycle.swift:219 resolves the finding from the earlier review, and the code looks right.
Not blocking: a test that calls refreshCachedTargetIfProcessChanged directly with a new pid for the same bundle would catch a regression on that line that the current tests miss.
Smoke Tests was still running at review time. This diff changes only log output and log-dedup state, so a failure on the known depth-frontier assertion (fixed by #2832) would be unrelated; any other failure needs a look.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ready-for-humanValid work that needs human implementation, judgment, or maintainer merge
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
thymikee
added
the
ready-for-human
Summary
Two days of running the Bluesky e2e suite on one simulator left a 374,000-line
runner.log. The per-command breakdown:FAST_APP_GUARDCOMMAND_ACCEPTED/COMMAND_COMPLETEDSYNTHESIZED_GESTURE_POLICYPRIVATE_AX_SNAPSHOT_USEDSNAPSHOT_XCTEST_CHANNEL_DEFERREDThe accepted/completed pairs are worth every line (the whole investigation was built on them). The guard and policy markers restate, on every command, a fact that had not changed since the previous one: same bundle in the foreground, same gesture policy for the same kind.
Change
RepeatedLogSuppressor(inRunnerTests+Lifecycle.swift):logIfChanged(key:fact:line:)writeslineonly whenfactdiffers from the last one written underkey. Locked,reset()forgets every key.canUseFastForegroundAppGuard: keyedfast_app_guard, fact is bundle plus state; the line still carries the command that observed the change.logSynthesizedGesturePolicyDecision: keyed per gesture kind so alternating taps and scrolls do not restate each other; the decision line is the fact.invalidateCachedTargetresets the suppressor, so a rebind states the guard once more.Snapshot markers are left alone here: they carry per-capture numbers (nodes, depth) and are the evidence for the busy-runner work in #2804 and #2783.
Validation
#if AGENT_DEVICE_RUNNER_UNIT_TESTSguard: the suppressor writes on first sight, change, other key and after reset; invalidating the cached target resets it. Both pass on iPhone 17 Pro Max (iOS 27.0) viaxcodebuild test-without-building.FAST_APP_GUARDline and 1SYNTHESIZED_GESTURE_POLICYline.pnpm check:xctest-selection(277 methods, 0 unreachable) andpnpm check:packaged-runner-swift(50 files parse, line parity kept) clean.