Skip to content

fix: surface UI content warnings in skill preflight - #145

Merged
onevcat merged 5 commits into
mainfrom
fix/preflight-content-warning-takeover
Oct 1, 2026
Merged

onevcat merged 5 commits into
mainfrom
fix/preflight-content-warning-takeover

Conversation

@onevcat

@onevcat onevcat commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Supersedes #139 by @SunsetWan. This branch starts from the #139 head, so the original commits stay unchanged. Merge with a merge commit (not squash or rebase). Then #139 also shows as merged.

From #139

When ui returns ok: true with a remote_content_recovery advisory, preflight now prints a content warning instead of "All checks passed". The exit code stays 0. The warning asks the agent to compare the outline with the screen. On an iOS simulator, it also points at ApplicationAccessibilityEnabled, which an app reads at launch.

Added on top

  • Merge the latest main to resolve the CHANGELOG.md conflict.
  • Empty outline: a successful read with no elements now gets the same warning. Before this change, preflight reported a clean pass for it. This is the worse case because the remote-content retry found nothing, so no advisory is set. Physical iOS reads are excluded: their entries list is always empty, and the outline text is the payload.
  • Skill docs: the guidance moves from the Preflight section of SKILL.md into the Pitfalls symptom index, next to the other [i] advisories, because the advisory can appear on any ui read. The pitfall is renamed to Missing app controls in the outline and covers both cases.
  • Tests: use real advisory kinds (orientation_calibration_fallback, full_screen_tap_target) and add empty-outline cases for iOS, Android and physical iOS.

Validation

🤖 Generated with Claude Code

SunsetWan and others added 5 commits September 10, 2026 01:06
Signed-off-by: Sunset <sunsetwan@gmail.com>
Signed-off-by: Sunset <sunsetwan@gmail.com>
A successful `ui` read with no elements still reported "All checks
passed". This is the worse form of the missing-controls case: the
remote-content retry found nothing, so no advisory is set. Preflight
now gives the same content warning for it. Physical iOS reads are
excluded because their `entries` list is always empty.

The guidance moves from the Preflight section of SKILL.md into the
Pitfalls symptom index, next to the other `[i]` advisories, because
the advisory can appear on any `ui` read. The pitfall now covers both
the recovered and the empty outline. The tests use real advisory
kinds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Signed-off-by: onevcat <onevcat@gmail.com>
Only physical iOS reads have an empty `entries` list by design.
Today, only those reads set `kind`, but the check now names the iOS
platform. If a later change adds `kind: physical` to Android reads,
an empty Android outline still gets the warning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Signed-off-by: onevcat <onevcat@gmail.com>
@onevcat

onevcat commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Review round 1

  • R1-P1 (accepted as hardening, fixed in a2f1a71). The empty-outline exclusion keyed only on data.kind == "physical". The current CLI cannot produce an empty-outline false pass: the top-level ui payload sets kind only for physical iOS. executeAndroid() never sets it, and the field docs at IOSSimDescribeUICommand.swift:85-90 say "absent otherwise". Still, the exclusion now names its real reason (platform == "ios" and kind == "physical"). A new regression case (Android, kind: physical, empty entries) failed before the fix and passes after it.

swift test --filter PreflightScriptTests: 6 tests / 21 cases pass.

@onevcat

onevcat commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Review round 2

  • R1-P1: The reviewer confirmed this is resolved.
  • R2-P2 (CI red on a2f1a71, not caused by this PR). The only failure was cancellableSleep wakes early when the flag is cancelled (9.6 s against a 5 s bound). This PR does not change ProcessControlTests.swift or ProcessControl.swift, and the test passes in isolation. A re-run of the failed job (run 36812590531, attempt 2) passed, and all checks are now green. We will fix this timing-sensitive test separately. The 100 ms pre-cancel Task.sleep is inside the measured window, so starvation of the cooperative thread pool under full-suite load counts against the bound.

@onevcat

onevcat commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Review round 3: clean, with no new findings. R1-P1 and R2-P2 are confirmed resolved. make build passes, and all checks on a2f1a71 are green. The PR is ready to merge with a merge commit.

@onevcat
onevcat merged commit cd6caf9 into main Oct 1, 2026
6 of 7 checks passed
@onevcat
onevcat deleted the fix/preflight-content-warning-takeover branch October 1, 2026 04:33
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.

2 participants