2 Commits

Author	SHA1	Message	Date
Jakub Piasecki	2e182f7971	fix(flows): pin iOS tree reads to the launched app instead of auto-resolving every connection (#772) ... ## The problem Flow runs on CI iOS fail with `ViewInspector RPC timed out: Application.getState` at seemingly random steps - mid-flow after typing, on the first read after a fresh launch, in a probe flow 25 s after the original failure - while the app under test is provably healthy: spindumps taken at failure time show its main thread idle in the runloop, display links firing, and the injected devtools queue empty. The same flows pass locally. The chain: argent's iOS injection is simulator-wide (`launchctl setenv DYLD_INSERT_LIBRARIES` inside the simulator's launchd), so every process the simulator spawns afterwards loads the devtools dylib, not just the app under test. The dylib's constructor connects unconditionally (no UIKit or `com.apple.*` guard) and the tool-server registers whatever bundle id shakes hands, so Spotlight workers (`mdworker_shared`), background-launched system apps doing first-boot work (`MobileCal`), and bare daemons (nil bundle id, registered as `"unknown"`) all become connections indistinguishable from the app under test. Flow tree reads auto-resolved the target on every read: `queryFullHierarchyTree` passed `undefined` to `resolveNativeTargetApp`, which `Promise.all`s `Application.getState` over every connection. The dylib serves `getState` with a `dispatch_sync` onto the main thread, and a background-launched or suspended process never services its main queue, so its `getState` parks until the 5 s RPC timeout and the `Promise.all` rejects. One poisoned sibling connection fails every tree read in the run; the app under test is never even asked. That one mechanism explains everything the failures did. CI spindumps caught the poisoner red-handed (`MobileCal` parked in `HandleViewInspectorRequest -> CurrentApplicationState`) next to a healthy app. "Permanent wedge" vs "recovery" was just poisoner lifetime: a Spotlight worker exits and its connection drops with it, so reads recover; Calendar lingers and the whole job stays poisoned, fresh app launches and probe flows included. And it never reproduced locally because a warm simulator did its first-boot indexing and migrations long ago, while CI boots a pristine device and does exactly that work during the test window - which also made the failures correlate with typing and tapping purely by timing. ## The fix Pin the target instead of resolving it per read. `runLaunch` records the launched app on the run state once the tree-source gate passes, as `ActionEnv.treeTarget` - a `FlowTreeTarget` carrying the `bundleId`, whether the runner still vouches for it (`pinned`), and whether its probe has ever answered in this run (`probeAnswered`). `fetchFlowTree` threads that target down to `queryFullHierarchyTree`, the only platform source that consumes it. A pinned read talks to the pinned app and nobody else: it never calls `resolveNativeTargetApp` at all, and runs four gates of its own instead (`flow-ios-tree.ts`). A `com.apple.*` id is refused first, terminally and before anything is spent - a system app is never the app under test. Then `isConnected`, a map lookup, so a dead pin reports the dropped connection instead of driving env repair from 300 ms read polls, which could otherwise latch the device's process-wide give-up. Then a probe of the pinned app's own `getAppState`, and only that app's: a timeout before this pin has ever answered is ridden out as the cold start it usually is, one after an earlier answer is refused as an app that stopped servicing its main queue. Finally a foreground check on the answered state, as lenient over one app as auto-resolve is, so a flow that left the app fails with the crisp diagnosis instead of describing a screen that is not on screen. A poisoned sibling connection is never touched. The pin is only trusted while the runner can vouch for what is on screen, and it comes off in two tiers. The five `FOREGROUND_CHANGING_TOOLS` (`launch-app`, `restart-app`, `reinstall-app`, `open-url`, `button`) drop the target outright, because the launched app may no longer be on screen at all; `launch-app` and `restart-app` then re-set it from their own `bundleId` arg, unpinned, since they name the app they switched to - which is how a recorded flow switches apps, the recorder keeping them as raw `tool:` steps. Every other raw `tool:` step demotes the pin to an unpinned hint, its effect on the foreground being opaque to the runner; a launch attempt drops the target outright until it succeeds. Either tier also retires a proven tree-outage memo: a verdict proven against the pinned branch's four gates says nothing about the path the next read runs. An unpinned read is the pre-pin behavior with one arbiter added. `resolveNativeTargetApp` fans out over every connection, and only when that fan-out times out - the CI condition this change exists for - does the hint take the read, and only after passing the same terminal `com.apple.*` policy gate, the connections check, and a foreground-checked probe of its own. A resolution that answers always wins, including its deliberate "single app but backgrounded" refusal, so the arbiter never overrides a guard that fired: it only rides out a probe a stalled sibling made unanswerable. Nested `run:` fragments share the run state, so they inherit the pin, its demotion and its clearing. E2e flows begin with a `launch` by definition, so every selector read in a CI run happens pinned. Only iOS consumes the target; the other platforms' tree sources are per-device and never auto-resolve. The review rounds also surfaced that the `isInjectableBundleId` "library validation, the dylib can never load" rationale was disproven by this PR's own spindump evidence, so that text - and every other home of the same claim, `unreadableHierarchyReason` and `treeSourceGate` included - is rewritten as the policy it actually is (system apps are never the app under test; their observed connections are background processes whose reads hang or lie) with the gates' behavior unchanged. ## What this doesn't cover Auto-resolve's own fan-out is still all-or-nothing, so every read that reaches it without a hint the gates accept stays poisonable: top-level fragment runs, `describe` without an explicit bundle id, and the stretch after a raw `tool:` step whose hint is refused or cannot answer. Recording (`captureTapSelector`) now passes the session's last `launch` id as an unpinned hint, so it arbitrates the same timed-out fan-out, but it never pins - the recorder cannot vouch for the foreground app the way a `launch:` step can. Closing the rest means hardening auto-resolve itself (exclude connections that don't answer instead of failing the fan-out) and refusing the connections at the source (dylib constructor bail for non-UIKit and `com.apple.*` processes, plus a server-side handshake filter to cover already-shipped dylibs). Both are follow-ups. ## Tests Three files, and every behavior below is mutation-proven: disabling the seam, dropping the target from any single call site, removing an invalidation, or removing any one of the four gates each fails at least one dedicated test. The reviewers' repros were turned into tests rather than argued away. `flow-ios-tree-pinned-target.test.ts` works at the tree source. It reproduces the failure shape - a healthy app plus a poisoned `com.apple.mobilecal` sibling whose `getState` only times out - and proves a pinned read probes only the pinned app while an unpinned read still fails. It then covers the states auto-resolve could never reach: the terminal `com.apple.*` refusal, including its position ahead of the connections gate so a disconnected system app is not sent into a restart loop; a dead pin, where the dropped connection is reported and no env repair runs; both sides of the `probeAnswered` split; and the foreground guard over the states a real device produces (`inactive` under a permission dialog, `inactive` with no scenes, `background` still holding a foreground-inactive scene, and `background` with none). A second block covers the unpinned arbiter: the hint read on a timeout, refused when its own probe answers from the background, and rethrowing the fan-out timeout when the hint is disconnected, cannot answer, or when the resolution failed for any other reason. `flow-launch-pins-tree-target.test.ts` proves the runner wiring through the real `fetchFlowTree` seam: every read after a `launch` carries the pin across all five read paths (assert, tap, type, rotate, idle), retry polls included; fragments inherit it; a foreground-neutral `tool:` step demotes it and a foreground-changing one that names no app drops it outright; a later launch re-pins on a fresh target with the ride-out re-armed; and a launch-free run keeps the fallback. `flow-tool-step-tree-target-e2e.test.ts` drives `createRunFlowTool` with only `resolveService` stubbed, so the real `fetchFlowTree` and `queryFullHierarchyTree` run. It pins what the two tiers cost against a wedged sibling: the launched app still arbitrating across a foreground-neutral `tool:` step, the outage memo being retired when that step demotes the pin, the `com.apple.*` and foreground refusals staying in force through the demote, and a `hidden:` assert failing after the demote instead of going green against a backgrounded app.	2026-08-25 13:26:46 +02:00
Ignacy Łątka	5ac6805040	fix(native-devtools): derive restart_required from the running process instead of asserting it (#560) ... this is urgent --- ## Problem Every `native-*` tool can get stuck telling the agent to do something that will never work. I hit this during an end-to-end pass over 0.17.0. After a mid-session simulator reboot, `native-devtools-status` reported: ```json { "envSetup": true, "appRunning": true, "connected": false, "requiresRestart": true, "nextLaunchWillBeInjected": true, "injectable": true } ``` and every `native-*` tool returned `restart_required` with "Call restart-app then retry." Following that advice never helped, and the message offered nothing else. `flow-execute` was blocked too, since its `launch` step needs the connection, and flow recording silently degraded a portable `tap: { selector }` to raw coordinates when it couldn't reach native devtools. The app was injected the whole time. All three dylibs were mapped into the process and the simulator log showed a successful dial on every launch, while the tool-server held the listening end of that exact socket and still reported `connected: false`. `argent server stop && argent server start` cleared it instantly. ## The real defect is the state, not the wording `requiresAppRestart` returned `true` for every app without a live connection: ```ts async requiresAppRestart(bundleId) { if (connections.has(bundleId)) return false; await reverifyEnv(); return true; // <- asserted, never measured } ``` That is not a measurement. `restart_required` was a constant wearing a state's clothes, and the advice built on it stays literally true forever, so an agent obeying it restarts the app indefinitely. Softening the message would have left that intact. This PR measures the state instead. ## Fix — derive it from the running process `restart-app` relaunches into the simulator's **current** launchd environment, so it can only ever help a process launched under different terms than a fresh launch would get. That is answerable: read the process. `launchctl list` already gives its pid; `ps eww -p <pid> -o etime=,command=` gives the environment it was exec'd with and its age. Simulator apps are ordinary host processes owned by the same user, so both are readable. `appConnectionState` (replacing `requiresAppRestart`) returns: | state | meaning | remedy | | --- | --- | --- | | `connected` | devtools are live | — | | `not_running` | no process to read | `launch-app` | | `stale_process` | no bootstrap dylib, a previous run's endpoint, or older than this service's listener | `restart-app` | | `unregistered` | injected against **this** endpoint, started after the listener came up, still silent | restart the **tool-server** | | `indeterminate` | process could not be read (ios-remote, unreadable `ps`) | `restart-app`, with the loop warning | Only `not_running` / `stale_process` / `indeterminate` report `restart_required`. `unregistered` gets a new `service_stale` precheck status that names the tool-server restart and explicitly says not to relaunch the app — the launch a restart would perform has demonstrably already happened. The listener-age comparison is load-bearing: after a tool-server restart the unix path is identical (it is derived from the udid) but the inode is new, so an injected process older than the listener still needs relaunching. Without that check this would send people off to restart a tool-server they had just restarted — a new loop in place of the old one. Both grace comparisons lean away from `unregistered` - the first towards `stale_process`, the second towards `indeterminate`, and both of those ask for a relaunch - so an uncertain read costs at most one wasted `restart-app`. Call sites follow the same rule rather than each inventing guidance: - `describe` withholds `should_restart` for `unregistered` (setting it would rebuild the restart-app → describe loop through a live service instead of a system app) and carries the diagnosis as a `hint`, which still marks the empty read untrustworthy for `await-ui-element`. - `flow-run` raises the measured diagnosis instead of a blanket "relaunch it" — both when a flow resolves a selector against the full hierarchy, and at the `launch` step's readiness gate, which previously polled for the connection and then guessed at "a stale or duplicate argent server". - `native-devtools-status` reports `state`, and `requiresRestart` is now false for `unregistered`. - The six `native-*` tools type their block as the shared `NativeDevtoolsPrecheckBlock` instead of restating it, so a new precheck outcome cannot reach an agent through a tool that never mentions it. ## Scope Why the service can stop registering a connection it is listening for is still #561 and is not fixed here. What changes is that the state is now measured, so that defect reports itself accurately instead of masquerading as an app that needs restarting. ## Verification **E2E, real iOS 18.6 simulator, Bluesky (RN 0.81.5).** Two tool-servers were raced onto the same per-udid socket — the #561 shape, where the app is genuinely injected and connected, just to the newer instance. Both were then queried on the same app at the same instant: | | `connected` | `requiresRestart` | `state` | `native-describe-screen` | | --- | --- | --- | --- | --- | | main, stale instance | false | **true** | — | `restart_required` — "Call restart-app then retry." | | this branch, stale instance | false | **false** | `unregistered` | `service_stale` — "Restarting the app cannot change that … restart the tool-server" | | healthy instance | true | false | `connected` | `ok`, 44 elements | Restarting the app in main's row lands on the newer instance again, leaving the queried one stale forever. That is the loop, reproduced on demand. The other states were walked on the same simulator: a pre-existing process against a fresh listener → `stale_process` (`requiresRestart: true`); `restart-app` → `connected`; terminate → `not_running` ("Call launch-app (or restart-app)"). **Unit.** `native-devtools-app-state.test.ts` drives the real blueprint factory with a stubbed process table and covers all five states plus the `etime` and env-token parsers. Mutation-checked — dropping the listener-age comparison, the endpoint match, or the connect grace each fails exactly one test and nothing else. **Suite.** `vitest run` in `packages/tool-server`: 296 files, 3113 passed, 1 skipped. `tsc --build` across the workspace, `tsc --noEmit -p tsconfig.test.json`, `eslint` and `prettier --check` on every changed file: clean. `origin/main` merged in first. ## Review round A review pass over the derivation itself, with each finding reproduced before it was acted on. **`describe` computed the diagnosis and threw it away.** `buildAppStateMessage` ran for every unconnected state, but only `unregistered` carried it out; the rest returned a bare `should_restart: true`. That boolean reaches an agent as an undocumented JSON field — `describe`'s own description never mentions it — and the single place it is rendered as prose is `await-ui-element`'s timeout note, which spells it *"call restart-app and retry"*. So the two states whose message carries the escape hatch lost it exactly where it was needed. `indeterminate` is the sharp end: on `ios-remote`, whose app processes are out of reach of the local process table, it is the **only** reachable unconnected state, so that path had no exit at all. The diagnosis now rides out as a `hint` for every state; `should_restart` still marks only the states a relaunch fixes. **`native-devtools-status` measured the same device twice.** `isAppRunning` ran its own `launchctl list`, and `appConnectionState` ran another one after re-verifying the launchd env — so the two snapshots sat seconds apart and could contradict each other (`appRunning: true` beside `state: "not_running"`), with `requiresRestart` gated on the staler of the pair. Both are derived from one measurement now; only `indeterminate`, which genuinely leaves running-ness unanswered, pays for a second probe. **Coverage.** Mutation testing found the 3 s grace term entirely unpinned — the fixtures sat 200x away from the boundary on either side, so both dropping the term and weakening `>=` to `>` kept the suite green while flipping a relaunchable process to `unregistered`, the one verdict the file exists to withhold. Also unpinned: the `reverifyEnv` call (swapping it for the latching `ensureEnvReady` the comment explicitly forbids passed the whole suite) and an unparseable `ps` age staying "no evidence" rather than becoming a measurement. All three are now covered, and each new test was confirmed to fail on the un-fixed code. **Verified against the real thing.** The `launchctl` and `ps` contracts were re-checked on an iOS 18.6 simulator rather than reasoned about: row layout and pid column, one-line `ps eww` output with the argent tokens present in a live app's environment, `etime` matched against `lstart` to the second, and the four `etime` format strings read out of the `ps` binary. Two corrections came out of it — the environment can reach `kern.argmax`, which is exactly Node's default `maxBuffer`, so the probe now sets the same 16 MiB its siblings use; and `ps` suppresses the environment of a SIP-protected platform binary, which the docstring's "never as 'not injected'" promise did not account for (unreachable here, since `com.apple.*` is rejected as non-injectable upstream, but the promise now says so). **Also corrected:** the `argent-device-interact` skill offered `restart-app` as the only remedy for a missing injection; the dylib-platform check in `download-native-binaries.sh` and a `requiresAppRestart` reference in `boot-device.ts` described states that no longer exist; and three tool descriptions still said *"Fails if native devtools are not connected"* directly beside the new sentence explaining that this case returns a status rather than failing. **Known and deliberately left** (both pre-existing, neither introduced here): `describe` with no `bundleId` auto-targets from the connected list, so an unregistered app throws `NATIVE_TARGET_NO_CONNECTED_APPS` before any state is measured and the message is swallowed by the fallback's catch — surfacing it would newly mark those empty reads blind for `await-ui-element`'s `hidden` condition, which wants its own verification pass. And a dylib that is inserted but silently skipped by dyld (a platform mismatch) reads as injected-but-silent, so `service_stale` → tool-server restart → `stale_process` → `restart-app` is a two-step cycle; distinguishing it needs a load confirmation the process table cannot give. ## Review round 2 A second pass over the derivation, run as four independent sweeps (claims-vs-code, nearest-twin and reachability, non-happy paths and inputs, and absence). Three of the four independently landed on the same defect. **The flow launch gate was the one caller that never asked whether the app could be injected at all.** Every other consumer of `appConnectionState` gates `com.apple.*` first - the precheck throws, `describe` returns its terminal hint, `native-devtools-status` short-circuits, `flow-ios-tree` only ever auto-resolves a connected app. `waitForNativeDevtools` measured the raw launch id. That matters because the launchd environment carrying the bootstrap dylib is simulator-wide, so a system app's process inherits the very tokens the measurement reads. Confirmed on an iOS 18.6 simulator: `com.apple.Preferences`, launched under argent, reports ``` 71940 0 UIKitApplication:com.apple.Preferences[9a63][rb-legacy] etime=00:03 DYLD_INSERT_LIBRARIES=.../libArgentInjectionBootstrap.dylib NATIVE_DEVTOOLS_IOS_CDP_SOCKET=/tmp/argent-nd-39B9C547.sock ``` Run through the shipped algorithm that is `processCarriesInjection: true` against an older listener, i.e. **`unregistered`** - so a flow with an `ios:` launch entry of `com.apple.Preferences` failed its launch step with *"restart the tool-server"*, for an app no tool-server restart can ever help. Library validation means it can never connect, so the gate now says that terminally. **`describe` dropped the entire diagnosis when the probe rejected.** `appConnectionState` re-applies the launchd env before it can answer anything, so it throws outright on a sim that goes away mid-call; that landed in the native fallback's outer `catch` and returned an empty tree with no hint. An agent saw "nothing on screen" rather than "could not be read", and `await-ui-element` had nothing to append to its timeout note. The two other consumers of the same call already degraded to `indeterminate` instead - this one now matches them. **`native-devtools-status` re-ran the probe that had just failed.** One route into `indeterminate` is `inspectRunningApp` rejecting, i.e. `simctl spawn <udid> launchctl list` failing. The running-ness fallback then issues that same command unguarded, so a shut-down simulator surfaced a raw subprocess error rather than the structured `init_failed` guidance - which the branch 40 lines above it handles correctly, for exactly this reason. **`native-devtools-status` was the only surface withholding the loop escape.** Every other consumer carries `buildAppStateMessage`, whose `indeterminate` text ends *"do not keep restarting the app; restart the tool-server"*. This tool returned booleans only, and `requiresRestart: true` is the whole of what an `indeterminate` app tells you. On ios-remote, where no app process can ever be inspected, that is the only state a running app reaches - so the tool whose stated job is verifying readiness answered with the restart loop and nothing else. It now returns `message`. **Also fixed:** both probes log their own failure (a broken probe degrades every app to `indeterminate`, indistinguishable at the tool surface from a genuinely uninspectable one - the sibling `ps` probe in `vega-process.ts` logs for the same reason); the gate skips its several-second measurement once the run is cancelled; and the gate no longer repeats the bundle id its caller's prefix already names. **Prose that had drifted from the code:** `not_running` is reachable on ios-remote too, so `indeterminate` is only the sole unconnected state a *running* app reaches there; the second grace comparison leans to `indeterminate`, not `stale_process`; four of the five states settle running-ness, not four besides `not_running`; a null pid means a row that did not parse, not a job launchd reported as having no process (measured: a `UIKitApplication` row is removed outright when the app exits, and `-` appears only on daemon labels); `waitForNativeDevtools`'s documented return contract described the branch the change had rewritten; and a rationale comment was attached to the test above the one it described. **Coverage.** The gate's measured reason had none at all - replacing the whole diagnosis with a literal left all 542 flow tests green. Eleven mutations were run against the fixes and every one is now killed: the injectability guard, the measured reason, the doubled bundle id, `describe`'s reject guard, `describe`'s `unregistered` hint merge, the `message` field, the precheck fallback, the `appRunning` guard on `requiresRestart`, a state renamed in the description only, the connect grace widened from above (the old fixtures all sat 10x clear of it, so it could have grown to 25 s unnoticed), and the `indeterminate` guidance line. --- ## Review round 3 — external review findings Six findings from @hubgan's E2E review, each reproduced before it was acted on. Fixed in b9d17437 and 3eb59ef7. **This round supersedes several claims made above — they are listed at the end.** **The launch gate refused every `launch:` of an Apple system app.** A coordinate-only flow gave `["launch:error","tool:skip"]` with `resolveService` called 0 times — the guard short-circuited before any measurement — and the reason it emitted said *"Drive it by coordinate instead"*, from the gate that had just killed a purely coordinate-driven flow. `argent-create-flow`'s worked example (SKILL.md:171, :199-209) is exactly that shape. The premise underneath it did not survive checking. #453, which motivated `isInjectableBundleId`, recorded `connected: false` for `com.apple.Preferences` on an iOS 26.5 sim; the review recorded `connected: true`, with both dylibs mapped, on 18.5. So injection into a simulator system app appears to be runtime-dependent rather than categorically impossible, and neither reading is checkable from CI. The gate therefore no longer decides: it polls exactly as before — where the app does connect, nothing changes — and only on timeout does a non-injectable id return "no failure" instead of a reason. The launch step's job is to start the app; it succeeded. **Whether `isInjectableBundleId`'s blanket prefix rule is correct at all is now an open question, and a separate one from this PR.** **The flow tree source could not report it either, because its measurement was unreachable.** `resolveNativeTargetApp(api, undefined)` draws candidates from `listConnectedBundleIds()` — the same map `appConnectionState` reads to answer `connected` — so a non-connected app threw the stock auto-target error first, whose "Launch or restart the app first" is the loop this PR exists to break. Driving the real factory: `appConnectionState === "unregistered"` and `listConnectedBundleIds() === []` at the same instant. The race between the two is closed, not narrow: no macrotask boundary separates them, while the socket `close` handler is one. The branch's own test passed only because its stub let the two accessors disagree. The flow now threads the id its `launch` step named — which does not come from that map — through `ActionEnv` → `fetchFlowTree` → `queryFullHierarchyTree`; the unreachable branch is gone. **The gate emitted `buildAppStateMessage` verbatim for every state but `unregistered`.** Those are written for the native-* tool surface, whose reader has not launched anything, so a crash-on-launch app was answered with "Call launch-app (or restart-app) then retry" — the action the step had taken 9.5 s earlier. `flowLaunchGateReason` now rewrites each state for a caller that has just launched, exhaustively, so a state added later cannot inherit the tool-surface remedy. **`describe` without a `bundleId` — the default form — lost the whole diagnosis.** Auto-targeting throws before anything is measured and the outer catch returned the tree bare, so `await-ui-element`'s blind-read guard (which keys off exactly `hint`/`should_restart`) let a `hidden` wait release in 0 ms against an element still on screen, where the same call *with* a `bundleId` correctly refused. An empty read the hierarchy could not corroborate now says so. A device with **no** native-devtools service at all is deliberately left unqualified — that is an absence of a second opinion, not a failed attempt at one, and marking it blind broke a real `await-ui-element` case. **Three of the six `native-*` descriptions named the terminal system-app rejection and three did not**, though all six throw the identical error (verified: one byte-identical message across all six). The clause is inlined verbatim in all six — `scripts/extract-tools.mjs` accepts only a single plain literal and silently drops anything else from the SpiderShield scan. **`not_running` cannot tell "stopped" from "not installed"** — both simply lack a `UIKitApplication:<id>` row — and for the second reading its only remedy is refused outright by `simctl launch`. It now names both readings instead of asserting the one it cannot see. A real probe was considered and not taken: `get_app_container` needs a booted sim and ios-remote has no such verb, so it would answer `undefined` on exactly the host where `indeterminate` is already the only reachable state. ### Claims above that this round supersedes - *"Library validation means it can never connect, so the gate now says that terminally."* — the gate now lets system-app launches through; the premise is contested (see above). - *"The flow launch gate was the one caller that never asked whether the app could be injected at all"* and *"`flow-ios-tree` only ever auto-resolves a connected app"* — both inverted here. - *"Known and deliberately left"* — the `describe` no-`bundleId` path was closed, with tests, so that paragraph now over-counts by one. - The suite figures in **Verification** and **Review round 2** are pre-round-3 counts. --- ## Review round 4 — reviewing round 3 Three lenses over 856efd8e's two parent commits (claims-vs-code, non-happy paths + reachability, absence + mutation). Round 3 did not survive intact, and what follows is what was wrong with it. **Three statements were false in prose an agent acts on.** `connecting` requires a process inside the 3 s connect grace, while this gate takes its verdict 9.5 s after launching the app — so its remedy claimed the step had launched the very process it was measuring, next to a base message reading "it launched moments ago". The figure that arm and `not_running` quoted was the connect wait alone, omitting the 1.5 s post-launch settle that precedes it (`LAUNCH_TO_VERDICT_MS` now names the whole spend). And `stale_process` blamed the launchd environment, which is false for the producer reached only when `processCarriesInjection` is **true** — that process carries this endpoint and is merely older than the listener. The measured text it wraps names both producers, so the two contradicted each other in one message. **A `hidden` wait still released in 1 ms.** Round 3 carved out a failed native-devtools resolution as "no service on this device at all". The blueprint is registered unconditionally, so on iOS that rejection only ever means the service failed to come up — a socket bind losing to a concurrent same-udid server, which this repo documents as the dominant crash source in 0.16.0 telemetry. Reproduced: `describe` → no hint, then `await-ui-element hidden` → `{"success":true,"elapsed":1}`. It is now as unexplained as its siblings. **The failure-code discrimination was defeatable.** The flow tree source classified auto-targeting's throw by reading a code off a module-local `Symbol`, so a duplicate `@argent/registry` instance would read it as absent and fall back to the exact message the change replaces. It tests the empty connected list directly now — the same condition, structurally, with no symbol involved. The `connected` arm added in round 3 was unreachable for the same reason the branch it replaced was, and is gone. **Coverage the mutation lens found missing.** Three of the four sites threading the launched bundle id were unpinned — including the `settleTree` read every `tap`/`type`/`scroll-to` uses, so an action step could have silently lost the terminal reason with the suite green. Also unpinned: `unexplainedHint` dropping a pre-existing degraded hint (where its exact twin fifty lines up **is** pinned), the tree source's "Flows resolve selectors" half, and both of the gate's withholdings. **Test cost.** The four per-state gate tests drove the whole runner in pumped fake time, ~9 s each, which is what pushed this file past vitest's default under parallel load. The mapping is a pure function and is tested as one; one end-to-end test still proves the gate reaches it. 46 s → 16 s, with more tests than before. Widening the fake-timer step was tried and is worse (52 s) — the pumps are what let the run's disk I/O proceed. **Also corrected:** comments the round-3 commits had falsified in the very files they edited; the `ActionEnv` claim that a fragment never carries a launch step (a flow is classified by its FIRST non-echo step, so a later launch is legal and does thread its id); `isBlindRead`'s enumeration of what sets its flags; and change narration that had leaked into permanent prose. `argent-create-flow` now states what a `com.apple.*` target costs a flow — its worked example is selector-driven, and round 3's justification comment claimed the opposite. **Verification.** 17 mutations across both rounds, every one killed by a specifically-named test. Two consecutive full runs of `packages/tool-server`: 3181 passed, 1 skipped, and 6 failures in `boot-device-hotboot.test.ts` that are pre-existing on this host (it asserts `-gpu darwin`/`swiftshader` where a Linux box resolves `-gpu host`; the file is untouched by this PR). `tsc --build`, `tsc --noEmit -p tsconfig.test.json`, eslint and prettier clean on every changed file; `scripts/extract-tools.mjs` emits all 75 tools with zero warnings. --- ## Review round 5 — reviewing round 4 Round 4 left two of the defects it set out to fix and introduced one. It also broke CI. **ESLint failed on the push.** Two `` \` `` escapes inside a double-quoted string (`no-useless-escape`). The pre-push check missed it because it was piped through `tail`, so the shell read the pipe's exit status rather than eslint's and reported success over a failure. Gates are run unpiped now, and repo-wide — `eslint .` is what CI runs, and a changed-files invocation covers a subset of it. **`unregistered` still quoted the connect wait alone** — the exact understatement `LAUNCH_TO_VERDICT_MS` was introduced for, applied in round 4 to `not_running` only. And the end-to-end test asserted `8000 ms` while its sibling fifty lines down asserted the whole spend, so the file enforced two contradictory rules and the *correct* figure would have failed CI. The post-launch settle counts: the connection is read off the live map and the poll checks it once before its first sleep, so a dial landing inside those 1.5 s is caught. All three arms that quote a figure now quote the same one, pinned together. **`stale_process` named the launchd environment as fact** four lines below a new comment saying that is false for one of the state's two producers. It is in fact sound — but only where the message already scopes it, to a *second* landing, whose relaunch makes the process younger than any established listener and so rules that producer out (it needs `processAge + grace >= listenerAge`). The comment now says which producer and why. The test named "does not blame the launchd environment" asserted only the phrase it kept, so it was green with the blame present; it now pins the ordering it is named for. **The `connected` arm is reachable — round 4's "unreachable" claim was wrong.** `appConnectionState` re-reads the live connections map *after* its env re-apply and process probe (`native-devtools.ts`), several simctl round-trips past the empty list that routed the call there, precisely so a dial landing in that window is not reported as unregistered. Folding the arm away left that case throwing a bare "Flows resolve selectors…" with no app named and no remedy. It is a branch again, and says the connection arrived mid-read. **`unexplainedHint` dropped `screenshot` along with the `bundleId` suggestion**, so a caller that passed a bundle id received a hint with no next action at all — while `argent-device-interact` tells the agent to follow that hint. `screenshot` is unconditional now. Its JSDoc also claimed two of three call sites are reached only after an explicit id resolved and measured connected; all three are reachable with an explicit id, as the file's own comment twenty lines above says. **Also:** `argent-create-flow` recommended `tool: gesture-tap` where the same file documents the first-class, equally tree-free `tap: { x, y }`; and change narration removed from four more comments. ### Standing caveats - **The `com.apple.*` premise is still unsettled and is not settled by this PR.** #453 recorded `connected: false` on iOS 26.5; the review recorded `connected: true` with both dylibs mapped on 18.5. Every change here is correct under either reading, but `isInjectableBundleId` remains a blanket prefix predicate and may be wrong on some runtimes. Worth its own issue. - `boot-electron-spawn-error.test.ts` and `boot-device-hotboot.test.ts` fail on the Linux host used for this work; both were confirmed failing at the PR base `b2e5c54d` and neither file is touched here. --- ## Review round 6 — reviewing round 5 Round 5's own headline fix was both broken and unpinned. **The hint stopped parsing.** Moving `screenshot` out of the conditional clause without rejoining the sentence produced, on the default no-`bundleId` form: > … not evidence that nothing is on screen. Pass `bundleId` to have the connection state measured, or Take a `screenshot` to see what is there. That is the exact string `argent-device-interact` instructs an agent to act on. Both forms are one sentence again, and both are now pinned by a test anchored to the end of the string rather than matching a substring. **And deleting the action entirely left the suite green.** The defect round 5 describes — "a caller that passed a bundle id got a hint with no next action at all" — could be reintroduced with 635 tests passing. Nothing asserted the hint named a next step. It does now. **Two claims round 5 corrected in the source survived verbatim in tests.** The "two of the three call sites are reached only after an explicit id resolved and measured connected" partition (all three are reachable with an explicit id), and "waited only NATIVE_READY_TIMEOUT_MS" — sitting ten lines above the assertion that now forbids that figure. **A guard test did not test its own name.** `does not blame the launchd environment for a stale process` pinned only *where* the blame sits. Restating it as fact on a first landing — `It lands here twice, so the …` — passed. It asserts the conditional now, which is the whole distinction: only a repeat rules out the producer that carries this endpoint. **The `connected` branch still had no coverage** — which is how round 4 came to delete it as dead in the first place. It is exercised now, including that its message still names the app and an action. **Also:** the terminal system-app reason recommended `tool: gesture-tap` where the same round-5 fix in `argent-create-flow` had moved to the first-class, equally tree-free `tap: { x, y }`. The twin was missed. **Verification.** Four mutations against these fixes, all killed: dropping the `screenshot` action; reintroducing the broken join; stating the launchd blame as fact; folding the `connected` branch away. Gates run unpiped and read by exit code — `eslint .` 0, `tsc --build` 0, `tsc --noEmit -p tsconfig.test.json` 0, `prettier --check .` 0. --- ## Review round 7 — reviewing round 6 **The gate asserted the contested fact as settled.** Round 3's justification for keeping the 8 s wait read "a system app's process inherits the injection tokens **and does connect on some runtimes**". Eight other places in this repo say the opposite — `blueprints/native-devtools.ts` ("our dylib can never load", and the agent-facing precheck throw), `native-devtools-status`, `describe`'s iOS hint, and `test/describe-tool.test.ts` ("a com.apple.* app never connects"), a file these commits edit. `flow-ios-tree.ts` contradicted *itself*, its docblock saying "can never load" above a message saying "cannot be relied on to inject". The fact is not settleable here, so nothing asserts it now. The comment cites both observations — #453's `connected: false` on iOS 26.5, the review's `connected: true` with both dylibs mapped on 18.5 — and justifies the wait without picking: polling costs this step its timeout under the pessimistic reading and is the only thing that makes selectors work under the optimistic one, so waiting spends time on an open question where skipping would bake an answer into behaviour. The tree source's docblock is hedged to match the message it already had. **This PR does not resolve that inconsistency**, and `isInjectableBundleId` remains a blanket `com.apple.` prefix predicate. It deserves its own issue. **A guard let the thing it guards be deleted.** Round 6 made the repeat-landing assertion conditional (`if (blame !== -1)`), so removing the whole clause — leaving a reader who lands there twice with "re-run the flow" and no escape — passed 36/36. Unconditional again, wording pinned. **A rationale that was false in both directions.** "Keeps the `not.toContain` guards non-degenerate: they would pass vacuously if the constants were made equal" — equal constants make them *fail*, and 500 vs 1500 makes them fail for the wrong reason (`"1500 ms".includes("500 ms")`). It pins the substring property those guards actually need. **Two more presence-only guards.** "keeps the ax-degraded boot guidance **ahead of** the unreadable-hierarchy note" asserted only that both strings appear, and passed with them swapped. And round 6's own `tap: { x, y }` twin fix had no test at all — the same absence that got the `connected` branch deleted as dead two rounds earlier. Both pinned, and the `tap: { x, y }` claim was verified end-to-end: that step form parses, resolves through `resolveTargetPoint` with no tree read, and passes in a flow whose selector step fails with this very message. **Two test names promised more than they checked** — a remedy "that is not the action the step just took", asserted for states whose remedy is another relaunch; and a crash-on-launch guard forbidding only the literal `Call launch-app` while the arm legitimately suggests launch-app for watching the exit. **Verification.** Three mutations, all killed: deleting the repeat-landing clause, reverting the coordinate form to `tool: gesture-tap`, and putting the read-note ahead of the boot guidance. Gates read by exit code, unpiped: `eslint .` 0, `tsc --build` 0, `tsc --noEmit -p tsconfig.test.json` 0, `prettier --check .` 0. Suite: 3187 passed, 1 skipped, and the 6 pre-existing `boot-device-hotboot` failures on this Linux host. --- ## Review round 8 — reviewing round 7 **The gate's own justification was false, and the disproof was already in this PR's review.** Round 7's comment claimed polling "is the only thing that makes selectors work at all" under the optimistic reading. It is not: `settleTree` retries a failed tree read for `SETTLE_TIMEOUT_MS` on top of `POST_LAUNCH_SETTLE_MS`, so roughly the first 4.5 s of connect latency is absorbed with no gate at all — which is exactly why the review's fragment (no `launch` step, therefore no gate) resolved selectors 4/4 against `com.apple.Preferences` on 18.5. Confirmed with an in-repo counterfactual: at 2000 ms connect latency the flow passes with or without the gate; at 6000 ms only with it. What the poll adds is the band from ~4.5 s out to `LAUNCH_TO_VERDICT_MS`, and the comment now says that. **Hedging one surface left its twins absolute — the same failure mode, a fourth time.** The precheck throw, 230 lines from the message this PR hedged and in the same file, still told agents Argent "can never be injected into it"; so did the predicate's docblock, `native-devtools-status`'s description ("the dylib can never load into them", "TERMINAL state — the app can never be injected"), and `describe`'s terminal hint. An agent reading `argent-create-flow` was told "depends on the simulator runtime" and the status tool told it "can never". Every agent-facing surface now says injection cannot be RELIED on, and a test pins that none of them claims impossibility — the six native-* descriptions, the status description, the shared recovery text, the device-interact skill, and (separately, since it is module-private and reachable only through the tool's output) `describe`'s terminal hint. Each surface is asserted non-empty first, so a missing one cannot satisfy the negative match vacuously. **The decision is untouched.** `isInjectableBundleId` still answers "not injectable", the precheck still throws, and the description still says `Do NOT restart/retry` — pinned by the same test, because softening the claim must not soften the behaviour or #453's restart loop returns. An app that MIGHT connect cannot be the basis of a retry loop either, which is now the predicate's recorded reason. **Also:** the round-7 reword left "The flow / flow path" duplicated; withholding the verdict before the measurement (promoted to its own paragraph last round) was asserted nowhere, so moving it after `appConnectionState` left the suite green — a test now proves the measurement is not taken for an app whose verdict is withheld either way; and two more test names were narrowed to what they actually check. **Verification.** Mutations run against every decision these commits added. Killed: re-asserting "can never load" in the status description; dropping its terminal instruction; re-asserting impossibility in `describe`'s hint; moving the withholding after the measurement; and the earlier rounds' decisions re-checked and still pinned. One survivor is unpinnable by construction — a claim inside a code comment, which no test can assert. Gates read by exit code, unpiped: `eslint .` 0, `tsc --build` 0, `tsc --noEmit -p tsconfig.test.json` 0 (this one rejected a tuple vitest had accepted), `prettier --check .` 0. --- ## Review round 9 — scope: the skill docs This is a state-derivation fix. It should not be rewriting skill docs, and the earlier rounds had it doing exactly that. **`argent-create-flow` is back to its base.** Nothing in it was falsified by this change. The `com.apple.*` caveat rounds 4-6 grew there documents the question this PR explicitly declines to settle — whether `isInjectableBundleId`'s blanket prefix rule is right at all — and that belongs to its own issue, not to a paragraph in a skill an agent reads before recording a flow. **`argent-device-interact` only got smaller.** It said *"call `restart-app` first if needed"* — the unconditional-restart advice the measured states exist to replace — and that clause is deleted. The four-status routing table the first round put in its place is gone with it: the remedy already travels on the tool's own output and on the six `native-*` descriptions, both pinned by tests, and a second copy in a doc no code reads is one more surface to drift. Tests follow the prose. The two `SKILL.md` reads in `native-devtools-status.test.ts` go with the text they pinned, and two comments that cited the reverted skill text are corrected. ### Claims above that this round supersedes - The first round's *"the `argent-device-interact` skill offered `restart-app` as the only remedy for a missing injection"* — the clause is deleted, not replaced. - Round 4's *"`argent-create-flow` now states what a `com.apple.*` target costs a flow"*, round 5's *"`argent-create-flow` recommended `tool: gesture-tap`"*, and round 6's twin fix in the skill — all in the reverted paragraph. The matching source-side fix in `flow-ios-tree.ts` stands; only its stated twin is gone. - Round 8's cross-surface certainty test no longer reads the skill. It covers the status description, the six `native-*` descriptions and the shared recovery text — the surfaces that make the claim. **`origin/main` merged in, and #519 needed one test to follow.** It landed after this branch last merged main and makes a `getFullHierarchy` reply with no windows an error — an untrustworthy read, not a blank screen. `does not measure when auto-targeting resolved an app` had been using an empty `windows` as a don't-care return, so the call now ended before it could observe anything. The stub serves a real window, which is the state the test names. Mutation-checked: dropping the empty-connected-list term from the guard makes the measurement fire and fails that test alone. **Verification.** `packages/tool-server`: 304 files, 3242 passed, 1 skipped, on a macOS host (the `boot-device-hotboot` failures noted in the standing caveats are Linux-only and do not reproduce here). Gates read by exit code, unpiped: `eslint .` 0, `tsc --build` 0, `tsc --noEmit -p tsconfig.test.json` 0, `prettier --check .` 0.	2026-08-17 21:06:26 +02:00