Skip to content

chore(ios-runner): remove unset env knobs, the dead back wire command, and unreachable paths - #2828

Merged
thymikee merged 4 commits into
chore/2790-drop-continuous-dragfrom
chore/2791-runner-dead-paths
Sep 23, 2026
Merged

thymikee merged 4 commits into
chore/2790-drop-continuous-dragfrom
chore/2791-runner-dead-paths

Conversation

@thymikee

@thymikee thymikee commented Sep 23, 2026

Copy link
Copy Markdown
Member

Summary

Removes runner code nothing can reach. Commands the daemon sends behave the same. Stacked on #2825.

  • Deletes the env knobs DISABLE_READONLY_RETRY, DISABLE_SAFE_MODAL_PROBE and NOOP_STARTUP, plus the now-unused RunnerEnv.isTruthy. git grep confirmed nothing set them. The nightly comment that named NOOP_STARTUP is reworded.
  • Deletes the runner back wire command: TS union member, trait row (the manifest is still runner-command-traits.ts here), Swift case and handler.
  • Deletes selector-keyed type and its element-focus entry point.
  • Deletes the Thread.isMainThread branch in executeDispatched and the prepared-path .snapshot case. Also deletes the main-thread shortcuts only those reached (modal-routing check, post-snapshot mark, snapshot-failure invalidation). The issue did not list these three.
  • The test-only execute(command:) now calls the transport's own inlineResponse/enqueueAccepted, so tests run on the command queue.
  • Custom-action coverage: truncated/blocked are now required.
  • AppleToolProvider.simctl/devicectl are now required; the runXcrun fallback is gone.

Closes #2791. 18 files, +120/−220.

Validation

Rebased onto main at a913cb222. Signed macOS host lane: 236/236 at b7a4ffbc0. Resolved a conflict with #2822: back is removed from the merged trait table and its test. check:affected --run is green at the stack head 60e533284. The evidence below is from before the rebase.
Tested at d53defc2f.

  • pnpm check:affected --run: all runnable checks passed (59 selected; everything else is GitHub-authoritative).
  • check:production-exports, check:xctest-selection (host reaches 234), check:packaged-runner-swift, typecheck: green.
  • macOS host lane, signed unit-test build: 234 tests, 0 failures.
  • iOS Simulator (iPhone 17 Pro, iOS 27 beta), changed tests that don't launch the host app: pass. Tests that call app.launch() fail the same way on this branch and on the base build ("Application … is not running"), so this host can't give evidence for them. That gap stays open for the ios.yml lane.
  • Live: open Settings, then snapshot, press, back --in-app, back --system, fill. Runner log shows backInApp/backSystem/type/snapshot ok=1. Session closed, daemon stopped --clean.
  • Review (code-review skill, 9 findings): fixed 2 (helper now reuses the transport's routing; standalone expectation). Rejected 7: out of scope (TV-remote env knob, captureSnapshotRootBounded), sibling-owned (shouldRetryCommandrefactor(ios-runner): Swift retry decisions keyed on message text → typed codes #2786), tests follow production dispatch on purpose, no sender (selector type), existing refusal pattern (.snapshot switch case), file already over 1,000 lines before this PR (refactor(ios-runner): move ~3.9k lines of inline unit tests out of production Swift files; split CommandExecution under 1,000 lines #2792 splits it).

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 23, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.80 MB 4.80 MB -3.7 kB
Package (unpacked) 4.80 MB 4.80 MB -3.7 kB
Package (download) 1.43 MB 1.43 MB -518 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 18.7 ms 19.1 ms +0.5 ms
CLI --help 54.0 ms 54.1 ms +0.1 ms

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at d53defc. The removed branches (main-thread executeDispatched, the prepared .snapshot case, the main-thread shortcuts) are not reachable from production, and executeSnapshotDispatched/executeSnapshotPrepared only lose a default argument. The code looks right.

Small notes, not blocking: the tool-provider resolver now reads provider.simctl.run and provider.devicectl.run before defaults apply, so an untyped scoped provider without them would throw (no in-repo producer does this); the alert-dispatch deadline test no longer bounds wall time the way the old 1 s wait did; and testCustomActionCoverageParsesOnlyCompletePairs now tests a four-key record, so ...CompleteRecords would be a better name.

The Smoke Tests failure at live-snapshot-depth-frontier.ts:126 is unrelated. It also fails on main and #2832 fixes it.

No conflicts. This needs #2825 to merge first.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 23, 2026
@thymikee

Copy link
Copy Markdown
Member Author

The code review of d53defc is still clean. This branch now has merge conflicts with main, so I removed ready-for-human for now. Please rebase onto main; I will look at the conflict resolution when the new head is up.

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 23, 2026
thymikee and others added 4 commits September 23, 2026 19:47
AGENT_DEVICE_RUNNER_DISABLE_READONLY_RETRY, AGENT_DEVICE_RUNNER_DISABLE_SAFE_MODAL_PROBE and
AGENT_DEVICE_RUNNER_NOOP_STARTUP were only reachable through the process.env pass-through; no
repo code, CI job or doc sets them. The safe modal probe is now unconditional, and RunnerEnv loses
its only-caller isTruthy helper.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…dispatch paths

- `back`: the daemon resolves only `backInApp`/`backSystem`, so the RunnerCommand union member,
  its trait row, the Swift case and its handler go.
- Selector-keyed `type`: runner adoption checks the artifact fingerprint, so no older daemon can
  send it; the element-focus entry point it alone used goes with it.
- Main-thread dispatch: the transport always executes on `commandExecutionQueue`, so the
  `Thread.isMainThread` branch in `executeDispatched`, the prepared-path `.snapshot` case, and the
  main-thread shortcuts that only those reached are removed. The test-only `execute(command:)`
  now dispatches like the transport, so tests exercise the production path.
- Custom-action coverage: the bridge in the same target always writes all four keys, so the
  older-bridge defaults for `truncated`/`blocked` go and a partial dictionary is malformed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… provider

Every resolved provider is normalized through createLocalAppleToolProvider, which always fills
both subcommand providers, so runXcrun's `?? runCommand('xcrun', …)` fallback was unreachable.
The fields are now required and runXcrun calls them directly.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… dispatch

The transport's inline status/uptime answer and its journal-accept-then-enqueue step are now named
functions, and the test-only execute(command:) calls them rather than re-stating them, so unit
tests take the same dispatch path as a daemon request. The wait uses a standalone expectation so a
command that outlives it cannot fulfill into an ended wait context.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@thymikee
thymikee force-pushed the chore/2791-runner-dead-paths branch from d53defc to b7a4ffb Compare September 23, 2026 18:08
@thymikee
thymikee merged commit 7939464 into main Sep 23, 2026
22 of 34 checks passed
@thymikee
thymikee deleted the chore/2791-runner-dead-paths branch September 23, 2026 18:13
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-23 18:18 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.

chore(ios-runner): remove env knobs nobody sets, the dead back wire command, and unreachable paths

1 participant