mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
fix/native-devtools-dyld-skip-terminal
42 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Ignacy Łątka
|
43d5b9d464 |
fix(native-devtools): let only a verdict that was reached be repeated
`indeterminate` turned terminal on `wasAdvisedToRelaunch` alone, which is a pid change since the hand-out and nothing more — already true one `connecting` reading after the relaunch. One unreadable `ps` in that window answered `injection_failed` for an app two seconds into a healthy cold start, asserting an earlier reading that measured a fresh process which never connected when the earlier reading had said "wait a few seconds". The verdict is now recorded when it is handed out, and `indeterminate` repeats that record rather than inferring one. Both terminal readings share the endpoint withdrawal too: losing the socket makes the premise a statement about the socket for the unreadable reading exactly as for the measured one. describe stops pairing `should_restart: true` — rendered to an agent as "call restart-app and retry" — with a message that says no restart on either side changes anything, and the six feature tools' `service_stale` clause gains the caveat its `native-devtools-status` twin already carries, now that the status can arrive with the endpoint-lost message. |
||
|
Ignacy Łątka
|
673aae8dc6 |
Merge origin/main into fix/native-devtools-dyld-skip-terminal
Attach mode (provider_attached) short-circuits the shared precheck before the injection advice: a provider-owned app has no relaunch of ours to prescribe. native-network-logs keeps main's not-connected sentence alongside this branch's injection_failed clause. |
||
|
Mateusz Aliyev
|
6e9e45c9f0 |
feat: attach to devices offered by external providers (#735)
This pull request lets Argent drive a simulator or emulator another process is already running, attaching to its simulator-server rather than spawning a second one. A provider writes a JSON descriptor to `~/.argent/providers/` listing the devices it offers and the mechanisms Argent may use on each. They appear in `list-devices` with an `ext:` id and work with the existing tools. The file is re-read on every call, so a withdrawal or a narrowed grant applies immediately. `ios.additionalDeviceSets` (#600) already makes such a simulator reachable by UDID. What changes is ownership. `boot-device` and `stop-simulator-server` refuse these devices, anything the provider did not grant is refused with a message naming it and Argent uses only the endpoints its own simulator-server build serves. A grant binds to the device rather than to one of its names, so the real udid or serial is gated exactly like the `ext:` id. Android emulators are covered too and a device visible to both a provider and `adb`/`simctl` is listed once. The contract ships as `schemas/device-provider-v1.json`, validated by `argent providers check`, and is documented in `docs/reference/device-providers.mdx`. Includes unit tests and an e2e phase where the harness acts as its own provider. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for discovering and connecting to externally managed devices. * Added `argent providers` commands for listing, validating, publishing, withdrawing, and pruning providers. * Added provider-aware simulator, debugger, profiler, native tools, and device listings. * Added capability controls, endpoint validation, revocation handling, and provider-specific diagnostics. * Added automatic CLI discovery for provider integrations. * **Bug Fixes** * Improved service recovery and paused-runtime error reporting. * Improved simulator keyboard and paste command reliability. * **Documentation** * Documented provider descriptors, capabilities, lifecycle rules, and CLI usage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Ignacy Łątka
|
e7f1000e1e |
fix(native-devtools): keep the terminal verdict standing when a read cannot inspect the process
An `indeterminate` reading measured nothing, but its own remedy prescribes a
relaunch and then a tool-server restart - and that restart clears the record the
terminal verdict rests on. So one unreadable `ps` handed an agent both steps the
verdict had just forbidden, and two reopened the cycle. It is now terminal while
a verdict is standing, saying the read measured nothing rather than re-arguing
it. ios-remote cannot reach it: it never inspects a process, so it never records
the relaunch this consults.
Also from review:
- A ViewInspector reply whose `error` is not `{message}` rejected the RPC with
an empty string; rendered as the sibling ax-service reader renders it.
- The create-flow injection_failed line said coordinates were not authorised,
which contradicts the coordinate remedy the messages themselves prescribe; it
now names what that yields - a fragment, not a QA flow.
- `service_stale` on the six feature tools promised the message names a terminal
fallback, which the endpoint-lost message does not; `unregistered` on
native-devtools-status forbade a restart that same message prescribes.
- Narrowed the ndjson-socket and hintReachesAgent docs to what the code does:
empty frames are skipped without an onDropped report, and a describe read
re-rendered by MCP auto-describe carries no hint at all.
Pins for five behaviours nothing caught: the flow tree's terminal short-circuit,
the handshake pattern's leading digit, its 256-character cap, that
`indeterminate` never records a relaunch, and the RPC error rendering.
|
||
|
Ignacy Łątka
|
dd7170da05 |
fix(native-devtools): refuse non-record frames in the shared reader, and correct the prose the terminal verdict falsified
The null-frame guard moves from native-devtools' handler into attachNdjsonReader, where it covers the ax-service and Android-helper consumers too and narrows onMessage to `object` so no future consumer can skip it. A frame that parses but is not a record now reports through onDropped like any other unusable frame. Also: bound the refused-handshake log line on every branch (only the string one was capped), stop the endpoint-lost message asserting a rival that may no longer be on the path, say "carries argent's dylib but never dials" on the two surfaces that still claimed dyld loaded it, correct the terminal diagnosis's enumeration of re-probe outcomes and its 2x-budget gloss, narrow holdsEndpoint's doc to the one reading it gates, un-orphan bindNativeDevtoolsUnixSocket's JSDoc, and drop the create-flow parenthetical that authorised coordinates the gate it cited forbids. |
||
|
Ignacy Łątka
|
4adf96e137 |
fix(native-devtools): let a status probe report the terminal state without failing its flow step
`isNativeDevtoolsBlockResult` matched a flat status set against a flat tool set, so `injection_failed` off `native-devtools-status` failed a flow step with "native-devtools-status did not run" - about a call that ran, and whose whole work is reporting that state. A recorded status probe flipped from pass to fail the day its app broke. The set is now per tool: the six feature tools block on all five, and the three that run the 2-arg precheck block on `init_failed`, which is the only one it can produce. The seven tool descriptions no longer frame the message as diagnostic-only. They said the state is TERMINAL and to read the message "for the likely cause", while that message prescribes a wait and one passive re-probe - the disambiguator for the cold start this verdict cannot tell apart, and so the step an agent skips first. The prohibitions on both restarts stay; the finality claim ahead of the message goes. `noteRelaunchAdvice` keeps the first hand-out instead of re-stamping it. A process replaced inside the age slop reads `stale_process` on some polls, so a polling reader - the flow tree reads several times a second - kept moving the anchor onto the replacement and withheld the verdict indefinitely. |
||
|
Ignacy Łątka
|
9ad53b7217 |
fix(native-devtools): stop the terminal verdict speaking for a socket this service lost
The per-UDID socket path carries no server identity and the last binder owns
it, so a second argent tool-server on one simulator takes every future dial.
The first then reads a healthy, connected app as `unregistered`, turns that
terminal, and closes with "a tool-server restart does not help here either" -
while restarting it and calling restart-app once is what recovers. The service
now compares the path's inode against the one it bound: when it no longer owns
the endpoint the reading is about the socket, so the verdict is withheld and
the message names the rival binder and that recovery.
The re-probe the diagnosis prescribes is also re-anchored to an outcome the
surfaces emit. Once terminal they return `{status, message}` with no `state`,
so "if the re-probe still reads unregistered" gated the boot-device rung on
something no tool can report; a repeat of the diagnosis is what an agent
actually sees, and doubles as the second-landing test the message lacked. The
wait is derived from the connect budget instead of spelled out beside it.
|
||
|
Ignacy Łątka
|
6d8b006696 |
fix(native-devtools): pin the terminal contract, and correct three claims it falsified
The review round found the injection_failed work resting on prose and tests that do not hold it. `relaunchAdvised.delete` on the handshake was unpinned: the test named for it never took a reading, so `lastSeenPid` had no entry, `noteRelaunchAdvice` recorded `null`, and the verdict compared one absent pid with another. It now drives the case the code's own comment describes — an app that answers the relaunch, registers, then drops its socket — and fails when the delete goes. `native-devtools-status` is the surface the flow-recovery ladder routes to, and neither its terminal return nor its description arm was held by anything; the precheck's dead-end warning was one identifier away from the non-injectable text, which is false for this state; and `describeIos` recorded the relaunch hand-out from await-ui-element's and await-screen-idle's per-poll reads, whose hints are discarded, letting any later process replacement pass as a relaunch nobody was asked to perform. Each is now pinned, and describe records only where its hint is rendered. Three claims this work falsified are corrected: the launch gate's exclusion rationale still credited the stale/unregistered flip with proving the relaunch, which the pid keying replaced; the precheck-tools JSDoc said the non-feature tools can only answer `init_failed`; and the no-peer localisation read the live connection map while asserting history. The description arms also stated the dyld cause as settled where the message they point at declines to, and `native-devtools-status` alone left the tool-server restart un-forbidden four lines under a rule prescribing it. |
||
|
Ignacy Łątka
|
0d05e02a84 |
Merge remote-tracking branch 'origin/main' into fix/native-devtools-dyld-skip-terminal
#560 landed on main as a squash, so every file it touched conflicted between main's copy and this branch's merged-in copy of the same work. Resolved against #560's pre-squash tip as the base, which reduces those to the places main genuinely moved past it. Two real conflicts: - `NATIVE_DEVTOOLS_BLOCK_STATUSES`, which main added to make a blocked precheck fail its flow step instead of passing it, enumerated the four statuses that existed when it was written. This branch adds a fifth, `injection_failed`. A `Set<NativeDevtoolsPrecheckBlock["status"]>` accepts a subset, so tsc stays green while a `tool: native-describe-screen` step blocked as `injection_failed` reports `pass` for a read that never ran. Added the status and a row to the table-driven gate test, which fails without it. - `reliability-and-recovery.md`, where both sides rewrote the same section after the launch gate stopped failing system-app launches. Kept main's prose, which states the mechanism and the cost, and carried over this branch's `injection_failed` step and its coordinate-tap bullet. Verified: eslint 0, prettier 0, tsc --build 0, tsc -p tsconfig.test.json 0, extract-tools 46/46, tool-server suite 4293 passed / 1 skipped. |
||
|
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
|
||
|
Ignacy Łątka
|
984a8c82f8 |
fix(native-devtools): prove the relaunch behind injection_failed by pid, not the flip
The stale_process -> unregistered flip was treated as proof of a relaunch, but ps -o etime is whole-second, so one unchanged process exec'd 2-3 s after the listener reads both states as its quantised age crosses the slop band - a false terminal verdict. Record the pid handed the relaunch remedy and treat unregistered as terminal only when the inspected pid differs. Also: drop the dead self-exclusion filter (the diagnosed bundle is never connected when unregistered is read), align the no-peer message with the terminal status it describes (a tool-server restart does not help), fix the 'restart the tool-server again' wording on the six descriptions, and add the injection_failed arm to the flow-authoring recovery ladder. |
||
|
Ignacy Łątka
|
8d6f77a983 |
Merge fix/native-devtools-restart-loop-guidance into fix/native-devtools-dyld-skip-terminal
Brings this branch up to date with its base (#560), which reworked the same four
surfaces after this branch forked from it at
|
||
|
Ignacy Łątka
|
6ba77b49fd |
Merge remote-tracking branch 'origin/main' into fix/native-devtools-restart-loop-guidance
The conflicts are all in the flow tree-source path, where both sides independently threaded "the app this run's `launch:` step started" into the iOS hierarchy read, for different purposes: - main (#778) calls it `launchedNativeApp` and uses it as an arbiter, only when `resolveNativeTargetApp` fails with NATIVE_DEVTOOLS_RPC_TIMEOUT, clearing it before any `tool:` step that can change the foreground app. - this branch calls it `launchedBundleId` and uses it to replace auto-targeting's "Launch or restart the app first" with a measured reason when nothing at all is connected. The two fire on disjoint conditions - empty vs non-empty connected list - so the merge keeps both, under main's name and its post-gate assignment: the measured precheck first, then main's try/catch arbiter around `resolveNativeTargetApp`. main's surviving `requiresAppRestart` gate in `queryFullHierarchyTree` is dropped. This branch removes that method from `NativeDevtoolsApi` outright - it was the asserted constant the branch exists to replace - so the call no longer compiles, and `unreadableHierarchyReason` is its measured successor. NATIVE_DEVTOOLS_CONNECT_BUDGET_MS 8000 -> 15000. This branch introduced that constant and coupled the flow launch gate to it, anchored to the 8 s gate of the time; #778 has since raised that gate to 15 s on measurement. Holding 8 s would have reverted #778 for the iOS, Android and Vega gates, and decoupling the two would leave every other surface answering "restart the tool-server" for an app the gate is still patiently waiting out. The boundary fixtures move with it (14/15/16 s), keeping the sweep tight on both sides of the new value. The launch-gate tests pump one real macrotask per 250 ms of fake time, so the longer wait needs ~70 rather than ~44 of them, and their 200-iteration ceiling started running out under full-suite parallelism - which reads as a timeout but is not one, since exhausting the loop abandons the run unsettled and no timeout value helps. That ceiling is now derived from the connect budget. |
||
|
Ignacy Łątka
|
bc5ed9d8cd |
test(native-devtools): ask one question of the whole surface, not of a slice
Mutation testing found seven more edits that leave all 304 files green, and every one is the same defect the last four fixes each moved rather than closed: the assertions pin a slice — a named line, a clause between two markers, the final sentence — and the contradiction goes in whatever slice is left over. A second `If state is connecting:` line. A fifth routing clause past the last marker. A permitting sentence before the pinned tail, where the pin is endsWith. An unbounded retry appended to the two message states with no tail pin, and to all five flow-gate arms, which no pin reached at all. Pinning more slices only moves the gap again, so this asks the question that does not depend on where the text sits: does this surface, anywhere, tell an agent to keep going? Every remedy-bearing surface is swept — all five state messages, all five flow-gate rewrites, the status description, the six tool descriptions — clause by clause, and an unbounded instruction fails unless that clause forbids it. The escape hatch for a forbidding clause needs its own care: a bare `stop` matched "ignore the stop-conditions above and keep calling restart-app", so the hatch covered for the negation it exists to allow. Also closes the two gaps directly: tail pins for not_running and stale_process, one guidance line per state, four routing clauses per tool. |
||
|
Ignacy Łątka
|
b721812203 |
test(native-devtools): pin the stop-conditions verbatim, not by pattern
Mutation testing found eight edits that leave all 304 files green, and every
one reopens the loop the measurement exists to close: a permitting sentence
appended after the prohibition ("go ahead and relaunch … keep relaunching
until it connects"), a qualifier spliced into it ("do NOT restart the app MORE
THAN ONCE", "again and again — a couple more relaunches are worth trying
first"), or the bounded escalation turned unbounded ("restart the tool-server
again and keep retrying until it clears").
They survive because every assertion here matches an opening clause, so the
defect is always in the words the pattern did not look at. Three rounds of
tightening patterns did not change that — the last round's own fix asserted
"Do NOT restart the app" and was defeated by appending two words to it.
So the sentences an agent obeys to decide whether to STOP are now duplicated
in the test and compared whole: each message must END on its stop-condition,
and the description's three guidance lines and the six tools' two routing
clauses must match exactly. Rewording one fails, as NON_INJECTABLE_RECOVERY
already does — that is the point, since these are the words that decide
whether an agent stops.
|
||
|
Ignacy Łątka
|
8a56e17c5e |
test(native-devtools): reject a qualified connecting prohibition
"Do NOT restart the app more than once" contains "Do NOT restart the app", so the assertion added for the inverted-prohibition case passed it on all 304 files. The qualified form is wrong advice for this state specifically: the first relaunch is already the one that discards a handshake in flight and resets the age the verdict reads, so one permitted restart is one too many. All three surfaces now pin the unqualified form — the message and describe's hint by matching the colon, the six tool descriptions by their own comma — and the message additionally pins the reason, so the prohibition cannot be left standing without the fact that makes it true. |
||
|
Ignacy Łątka
|
3b7f12c78c |
test(native-devtools): anchor the last description assertion to its own line
The fourth sibling was left matching over the whole description when the other three moved onto guidanceLine. It is the one the argument fits hardest: "If state is indeterminate" is only the prefix, and the command it checks for is supplied verbatim by the unregistered line, so the bound the test is named for could be deleted outright and it would still pass — dropping the only description-side stop-condition for the state a running app is stuck in on ios-remote. |
||
|
Ignacy Łątka
|
5afc350c74 |
fix(native-devtools): bring the six tool descriptions back onto the message
Raising the connect budget to 8000 ms updated two of the eight agent-facing surfaces. The six native-* descriptions kept routing connect_pending with "wait a second or two and retry" — the wording the budget outgrew — so an agent read one wait off the description and a different one off the message the same call returned, and obeying the description lands back inside the budget it just left. Their service_stale routing had the same split: the tool-server restart with no second landing, after the message and native-devtools-status had both gained one. They now defer to the message, which names the terminal fallback. The descriptions have to be plain literals (extract-tools.mjs reads them statically), so the wording is copied eight ways with nothing holding the copies together. Two tests do that now: every surface agrees on the wait and none still quotes the old one, and every service_stale routing line carries a second landing. Each fails on a single drifted surface, not just on all six. |
||
|
Ignacy Łątka
|
c07a3db677 |
test(native-devtools): pin the prohibitions, not the absence of a tool name
Mutation testing found five edits that leave all 304 files green. The worst inverts the `connecting` remedy. Its test asserted the message does not contain "restart-app" and does contain "Wait a few seconds", so "wait a few seconds, then relaunch the app and retry" passed — the advice whose own doc comment says "a relaunch resets the age this verdict reads, so obeying 'restart' never terminates". The sibling `unregistered` case pinned its prohibition; this one and describe's hint pinned only the hyphenated tool name. Both now assert the prohibition itself. Three more came from matching over the whole tool description. Its guidance lines echo each other enough that deleting one leaves its assertions satisfied by another - "do NOT restart the app again" appears verbatim in the `indeterminate` line - and the field list plus the injectable:false paragraph supply the rest, down to "describe" inside native-describe-screen. So the entire `If state is unregistered:` line could be deleted, the escape made unconditional (the exact case its own comment claims to guard), or its fallback pointed back at a tool behind the same precheck. Assertions now run against the extracted line, on each surface's own conditional clause. Last, the slop was pinned only downward: both boundary fixtures want `stale_process`, and every other injected fixture reads a 600 s listener against a process of seconds, so the term could grow into that gap and swallow `connecting` into a verdict whose remedy is restart-app. A young process against a young listener pins it from above. Also consumes `psExecAt` in the `ps` mock, as `dialDuringPs` already is: it is checked before the `psOutput` fixture, so a leak would decide the age of every case after it. |
||
|
Ignacy Łątka
|
8f5e303361 |
fix(native-devtools): stop three remedies that cannot be obeyed to the end
A launch step whose restart-app never ran. `restart-app` reports a blocked precheck by RESOLVING with it, and returns before the terminate and the launch — so an init-failed service leaves the app un-started. runLaunch read only the throw, so the gate went on to measure an app nothing had launched, found `not_running`, and reported "was relaunched by this step and is no longer running 9500 ms later, so it exited after launch". Every clause of that is false, and the message naming the real cause was dropped. The step now surfaces the block instead. A connect window narrower than the wait built on it. One constant answered two questions: how much slop the process/listener age comparison needs, and how long a dial may take before silence is evidence. The second is the quantity the flow launch gate already waits out at 8000 ms, so at 3000 ms an app still dialing at 4 s was `unregistered` -> `service_stale` -> "restart the tool-server", the heaviest remedy in the set, for an app the gate would still have been waiting for. Split: the slop keeps 3000, the dial gets the gate's budget, and the gate is now defined as that same constant so neither can drift from the other. An unregistered app with nowhere to stop. Each remedy was right and together they closed into a ring: the tool-server restart rebinds the listener, so the same never-dialing process reads `stale_process`, whose relaunch makes it `connecting`, which becomes `unregistered` again. Nothing measured separates the first landing from the second, so the message and the status description now hand the reader that test — as `not_running` and `indeterminate` already did — and name the terminal fallback. Each fix is pinned by a test that fails without it. |
||
|
Ignacy Łątka
|
0565ffcb7f |
docs: cut the added comments to what the code cannot say
Comment-golf pass over every comment #560 adds: keep the non-obvious why (the ordering constraint on the env re-apply, the grace's lean, why `unregistered` and `connecting` withhold a restart, the `ps` no-evidence guarantee), drop the restatement and the essay around it. Also removes two comments that narrated review rounds rather than the code as it stands, and aligns a describe-test comment with the softened injectability claim the rest of the branch makes. 841 comment lines where there were 1069; no behaviour change. |
||
|
Ignacy Łątka
|
b556708669 |
docs(skills): shrink the skill change to removing the stale remedy
The defect this PR fixes is in the state derivation, so the skills only needed the advice that fix falsified taken out of them. argent-create-flow goes back to its base: nothing in it was falsified, and the com.apple.* caveat it gained is documentation of a question this PR leaves open, not a correction. argent-device-interact drops "call restart-app first if needed" - the unconditional-restart advice the measured states replace - without restating the status routing. The remedy now travels on the tool's own output and the six native-* descriptions, both pinned by tests; a second copy in a skill is one more surface to drift. Tests follow: the SKILL.md reads in native-devtools-status.test.ts go with the prose they pinned, and two comments that cited the reverted skill text are corrected. |
||
|
Ignacy Łątka
|
e5463b54ff |
fix(native-devtools): end the restart-app ↔ tool-server cycle a skipped dylib produces
DYLD_INSERT_LIBRARIES proves the bootstrap dylib was handed to a process, never that dyld loaded it: dyld skips an inserted library silently when its slice does not match the simulator's platform, when it is unsigned, or when a dependency is missing. Such a process reads as injected, so appConnectionState alternates between the two states whose remedies undo each other — stale_process prescribes restart-app, which leaves the app younger than the listener and reads unregistered; unregistered prescribes a tool-server restart, whose new listener is younger than the app and reads stale_process — and an agent obeying both never leaves the pair. Record the relaunch hand-out per bundle for the life of the service, and once a bundle that was told to relaunch reads unregistered, report the terminal injection_failed diagnosis instead of the tool-server remedy. The flip proves the process was replaced: which side of the listener a process falls on is fixed at its exec, so the two states cannot describe one process twice. The connected-peer list localises what is left, since the launchd env is simulator-wide and this service holds one listener. Only those two states take part. connecting resolves itself as the process ages out of the grace, not_running asks for a launch, and indeterminate is the absence of a reading — it prescribes a relaunch too, but a later unregistered there may be the same process finally becoming readable. The flow launch gate keeps its re-run advice: it performed the relaunch itself, one window earlier, and a cold start produces the same reading. |
||
|
Ignacy Łątka
|
f85d1be7ee |
fix(native-devtools): make every surface agree on how certain the injection claim is
Hedging the claim on the surfaces this PR touched left the untouched ones absolute, which is the shape that actually misleads: an agent reading `argent-create-flow` was told injection "depends on the simulator runtime" while `native-devtools-status`'s own description told it the dylib "can never load into them", and `describe`'s terminal hint said the same. The evidence supports neither absolute: #453 recorded `connected: false` for com.apple.Preferences on iOS 26.5, an E2E review recorded `connected: true` with both dylibs mapped on 18.5. Every agent-facing surface now says injection cannot be RELIED on, and a test pins that none of them claims it is impossible — the six native-* descriptions, the status description, the shared recovery text and the device-interact skill, each asserted non-empty first so a missing surface cannot satisfy the negative match vacuously. The decision is untouched: `isInjectableBundleId` still answers "not injectable", the precheck still throws, and the status description still says "Do NOT restart/retry" — which the same test pins, 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, and the predicate's docblock now records that as its reason. |
||
|
Ignacy Łątka
|
b9d1743789 |
fix(native-devtools): resolve the review findings on the measured state
Six findings from the E2E review round, each reproduced before it was acted on. The flow launch gate refused every `launch:` of an Apple system app, so a flow that only ever taps by coordinate was stopped at step one — by a message telling its author to "drive it by coordinate instead". That is the shape `argent-create-flow` teaches readers to record, and its own worked example is a flow this gate rejected. The gate now measures rather than short-circuits: the launchd env carrying the bootstrap dylib is simulator-wide, so a system app can genuinely connect on some runtimes (issue #453 saw `connected: false` on iOS 26.5; the review saw `connected: true` on 18.5), and where it does the poll returns connected as before. Where it does not, the launch still passes and the impossibility is reported where it actually bites — at the first selector. The flow tree source could not report it there, because its measurement was unreachable: `resolveNativeTargetApp(api, undefined)` draws its candidates from `listConnectedBundleIds()`, the same map `appConnectionState` reads to answer `connected`, so every state worth explaining threw the stock auto-target error first — whose "Launch or restart the app first" is the loop this all exists to break. The race between the two was measured shut, not merely narrow. The flow now threads the bundle id its `launch` step named, which does not come from that map and so survives the disconnection; the dead branch is gone. The launch gate also emitted `buildAppStateMessage` verbatim for every state but `unregistered`. Those messages are written for the native-* tool surface, whose reader has not launched anything — so `not_running` answered a crash-on-launch app with "Call launch-app (or restart-app) then retry", the action the step had just taken. Each state now gets its post-launch reading. `describe` without a `bundleId` — the default form — lost the whole diagnosis: auto-targeting throws before anything is measured, and the outer catch returned the empty tree bare. `await-ui-element`'s blind-read guard keys off exactly `hint`/`should_restart`, so a `hidden` wait released 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 left alone: that is an absence of a second opinion, not a failed attempt at one. Three of the six native-* descriptions gained a sentence naming the terminal system-app rejection and three did not, though all six throw the identical error. The clause is inlined verbatim in all six — the extractor feeding the SpiderShield scan drops any description that is not a single plain literal. `not_running` is derived from a missing `UIKitApplication` row, which a never-installed bundle id also lacks, so it cannot tell "stopped" from "not installed" — and for the second reading its only remedy is refused outright. It now names both readings rather than asserting the one it cannot see. Measuring it would need a `simctl get_app_container` round-trip ios-remote cannot serve. |
||
|
Ignacy Łątka
|
01fcd03f3a |
fix(native-devtools): pin the guidance surfaces nothing was holding
The device-interact skill doc could be reverted to its unconditional "call restart-app first if needed" — the very advice the measured statuses replace — with the whole suite green, because nothing in either package read its body. Same for the ps guard that keeps a whitespace-free line from being read as a measurement: without it "10:00" alone parses as a 600 s process with "10:00" for an environment, turning unreadable output into a definite stale_process. And for the resolution error the flow gate carries into its reason, which is the only place a bound socket or a wrong-platform device surfaces to the author. Also: drop the precheck fallback the recorded-failure read above it shadowed (the blueprint attempts the env once at construction, so by the time a tool runs, either envSetup latched — making the precheck a no-op — or the failure is recorded); merge the exported constant's new sentence into its existing JSDoc rather than orphaning it behind a second block; and correct three claims — appConnectionState's re-apply is repair, not an input to the verdict, describe degrades like every other consumer rather than "the other two", and a mis-built dylib reads as service_stale or restart_required depending on process age. native-find-views now names the statuses before referring back to them, as its twins do. |
||
|
Ignacy Łątka
|
6b6cc8dba3 |
fix(native-devtools): report the env a wiped simulator actually has
envSetup latches true on the first successful apply and is never cleared, so after a simulator reboot wipes DYLD_INSERT_LIBRARIES the status tool kept answering "the next launch will be injected" — and the agent relaunched into an uninjected process for as long as the sim stayed broken. The re-apply the measurement performs is what tests the env; a failure it recorded, cleared by any later success, is what contradicts the latch. Both branches derive it the same way now. Drop two dead terms: the flow gate's post-loop abort guard (the loop's own check is synchronous with the break, so an abort cannot land between them) and requiresRestart's appRunning conjunct (the state-settling above rewrites an indeterminate whose probe found nothing to not_running, so both surviving states already carry a live process). Pin what mutation showed nothing held: --detach per occurrence across every description, message and the skill doc — a whole-blob assertion passed with one of two occurrences stripped, and the skill copy had no test at all; the cold-start note's wait against the constant it names; and the connect grace from above at the exact boundary, which could be raised by a third and stay green. |
||
|
Ignacy Łątka
|
d980da93d7 |
fix(native-devtools): make the guidance prose true where the code already is
The runtime message stopped asserting an uninjected process for stale_process, but the tool description that agents read first still did, and it named neither connecting nor init_failed. Six tool descriptions, the status tool's state list and the skill guidance now agree with what the states mean. Drop the precheck fallback in the status tool's running-ness catch: reaching it means the entry precheck already latched ensureEnvReady, so the call can only return null. The recorded failure above it is the escalation that works, and a healthy env leaves the probe's own error as the honest answer. Mirror that escalation on the non-injectable branch, where a failure recorded by another bundle's env re-verify is the only witness a latched-then-dead sim leaves — the same sim state must not read as init_failed on one bundle id and a raw simctl throw on another. native-network-logs no longer claims the system-app rejection is the only failure it produces; a missing dependency or a non-Apple udid are failures too, and neither is terminal. |
||
|
Ignacy Łątka
|
b80fc58a2a |
fix(native-devtools): guard the shared precheck's measurement, and pin what nothing pinned
`precheckNativeDevtools` — the path all six native-* feature tools take — was the one `appConnectionState` consumer awaiting it bare. It re-applies the launchd env before it can answer anything, so a simulator that goes away mid-call rejects there and the agent got a raw `Invalid device: <udid>` in place of the structured guidance the same failure produces from `native-devtools-status` and `describe`. A failure recorded by that env re-apply means the sim itself is gone, which is init_failed's case; anything else leaves the connection merely unmeasured. Mutation testing found the rest of these load-bearing lines held by nothing — each mutation below kept the whole suite green before the test beside it: - `ps eww` -> `ps ww`. One character, and the environment stops being rendered, so every app reads `stale_process`: the restart loop, restored silently. - `-o etime=,command=` with the columns swapped. The age is parsed positionally off the leading token, so every app degrades to `indeterminate`. - The probe's own 5 s budget and its 16 MiB buffer. An environment can run to `kern.argmax`, exactly Node's default cap, so the default ENOBUFSes on a maximal one and turns a readable process into "no evidence". - The `connected` short-circuit in `appConnectionState`, and the one in the flow launch gate — without the latter `buildAppStateMessage` falls off its switch and a healthy late connection fails the step with a literal "undefined". - `requiresRestart` staying false for `unregistered`: the invariant the whole derivation exists for, unpinned at the tool that reports it. Both the pre-PR expression and any widening left it green. - `connected` being read off the same measurement as `state` rather than a second `isConnected()`. - The catches that keep a failed inspection or a rejected measurement advisory, in the blueprint, the status tool and the flow gate — the last of which has no try/catch above it, so a rejection returned the run with no step reports at all. - `remoteIosHost.inspectRunningApp`, which had no test in either direction, on the host where running-vs-indeterminate is the only distinction available. |
||
|
Ignacy Łątka
|
4bb9f1c77f |
fix(native-devtools): make every measured state's remedy true and terminating
The derivation measured the process correctly but three of the remedies it built on top were reachable statements the measurement itself disproves, and one prescribed a command that never returns. - `indeterminate` covered two unrelated things: a process that could not be read, and one that was read, found injected against this endpoint, and was merely younger than the connect grace. The second got a message asserting "its process could not be inspected" plus "Call restart-app then retry" — and since exec is what starts the dial, every obedient restart reset the age the verdict is read from and landed back in the same window. Split out as `connecting`, whose remedy is to wait, with its own `connect_pending` precheck status; `should_restart` and `requiresRestart` stay off it. - `stale_process` is reached both by a process carrying no argent injection and by an injected one older than this service's listener — the state an app is left in by the tool-server restart the `unregistered` remedy prescribes. Its message claimed the process was "not injected", eight lines after `processCarriesInjection` returned true. It now speaks of what the process can still reach. - `connections.has` was sampled on entry and never re-read, while the verdict below it costs `reverifyEnv` plus a `launchctl list` plus `ps`. A dial landing in that window was reported as `unregistered` -> `service_stale`, sending the agent to restart a tool-server that had just succeeded. - `argent server start` defaults to foreground and ends in a promise with no resolve path, so the prescribed `argent server stop && argent server start` hangs whoever runs it. Every surface now passes `--detach`. Consumers: - The flow launch gate is the one caller whose wait is bounded — it launches the app itself and waits NATIVE_READY_TIMEOUT_MS — so a cold start slower than that reads identically to a genuinely unregistered app. It says so rather than leaving "restarting cannot help" as the last word. - `native-devtools-status` lets the running-ness probe settle the state it was run to answer, so `appRunning: false` can no longer sit beside a message reading "Call restart-app"; and it reports a dead sim from the recorded init failure, which exists from the first attempt, rather than from the precheck, which only speaks once the service has given up after three. - `flow-ios-tree` degrades a rejected measurement like the other consumers instead of leaking a raw simctl error. - Gate reasons name the bundle id exactly once, as the gate's own comment says they must. Prose: describe's injectability gate named `stale_process` for a case that scores `unregistered`; three tool descriptions claimed any failure meant a dropped connection, which no longer holds for the terminal system-app rejection and never held for native-network-logs at all; and the shared-verbatim warning doc now names the flow gate as a reporter that deliberately does not carry it. Adds coverage for the two paths that had none — `flow-ios-tree`'s diagnosis and `remoteIosHost.inspectRunningApp`, the host where running-vs-indeterminate is the only distinction available. |
||
|
Ignacy Łątka
|
6a41cb98ba |
fix(native-devtools): gate the flow launch gate on injectability
`waitForNativeDevtools` was the one `appConnectionState` caller with no injectability check. The launchd env carrying the bootstrap dylib is simulator-wide, so an Apple system app's process inherits the exact tokens the measurement reads: measured on iOS 18.6, `com.apple.Preferences` launched under argent reports the bootstrap dylib and this simulator's socket, scores `unregistered`, and the launch step tells the flow author to restart a tool-server that was never at fault. Answer terminally instead. Also from the same review pass: - describe swallowed the whole diagnosis when `appConnectionState` rejected (the env re-apply it runs first fails on a sim that goes away mid-call): the outer catch returned an empty tree with no hint, so the read looked empty rather than unexplained. - native-devtools-status re-ran the running-ness probe unguarded on the one route where that probe has already failed once, turning a dead sim into a raw subprocess error instead of the structured init_failed guidance. - native-devtools-status reported `requiresRestart: true` with no prose to bound it. `indeterminate` is the only unconnected state a running app reaches on ios-remote, so that tool's answer there was the restart loop and nothing else; it now carries the same remedy every other consumer of the measurement carries. - Both probes now log their own failure, like the `ps` probe they were modelled on: a broken probe degrades every app to `indeterminate`, which is indistinguishable at the tool surface from a genuinely uninspectable one. - The gate skips the measurement outright once the run is cancelled, and no longer repeats the bundle id the caller's prefix already names. Prose corrections: `not_running` is reachable on ios-remote too; the second grace comparison leans to `indeterminate`, not `stale_process`; four states settle running-ness, not four besides `not_running`; a null pid means an unreadable row, not a job launchd said has no process. Coverage, each mutant confirmed killed: the injectability guard, the measured reason (previously a literal left all 542 flow tests green), the doubled bundle id, describe's reject guard and its unregistered hint merge, the message field, the precheck fallback, the `appRunning` guard, the state names in the description, and the connect grace from above. |
||
|
Ignacy Łątka
|
4f98006743 |
test(native-devtools): pin that status reads running-ness from the state
The two-snapshot version passes every other assertion in the file, so nothing caught the extra simctl round-trip or the window in which `appRunning` and `state` could disagree. |
||
|
Ignacy Łątka
|
8b7d2902a2 |
fix(native-devtools): carry the measured diagnosis to every caller
Review follow-up on the derivation itself.
describe computed buildAppStateMessage for every unconnected state and
then discarded it for all but `unregistered`, returning a bare
`should_restart: true`. That boolean reaches an agent as an undocumented
JSON field, and the one place it is rendered as prose — await-ui-element's
timeout note — spells it "call restart-app and retry". So the states whose
message carries the escape ("do not keep restarting the app") lost it
exactly where it was needed. `indeterminate` is the worst case: on
ios-remote, whose app processes are out of reach of the local process
table, it is the only reachable unconnected state, so the loop had no exit
at all. The diagnosis now rides out as a hint for every state, while
`should_restart` stays limited to the ones a relaunch fixes.
flow-run's launch gate polled for the connection and then guessed at a
stale server; it now reports the same measured reason as the tree read.
native-devtools-status read running-ness from its own `launchctl list`
while `appConnectionState` took a second one seconds later, so
`appRunning` and `state` could contradict each other and `requiresRestart`
was gated on the staler of the two. Both now come from one measurement.
Tests: describe's hint for stale_process / not_running / indeterminate and
alongside a degraded AX hint; service_stale out through all six native-*
tools; the grace-boundary cases that pin the comparison's safe direction;
the env re-application on a latched service; and an unparseable ps age
staying "no evidence" instead of becoming a measurement. Each fails on the
un-fixed code.
Also: `ps` probe maxBuffer matched to its siblings (an environment can
reach kern.argmax, exactly Node's default cap); the device-interact skill
and the dylib-platform check no longer give restart-app as the only
remedy; stale requiresAppRestart reference in boot-device.
|
||
|
Ignacy Łątka
|
cd009b1cec |
fix(native-devtools): derive restart_required instead of asserting it
`requiresAppRestart` returned true for every app without a live devtools
connection. That is not a measurement, it is an assumption, and it is wrong
exactly when it matters: when the connection never registers, "call restart-app
then retry" stays literally true forever and an agent obeying it restarts the
app indefinitely while every native-* tool and any flow whose `launch` step
needs the connection stays dead.
Measure it instead. `restart-app` relaunches into the simulator's *current*
launchd environment, so it can only help a process launched under different
terms than a fresh launch would get. `appConnectionState` reads the running
process out of the host process table — `launchctl list` for its pid, `ps eww`
for the environment it was exec'd with and its age — and answers what would
actually change:
- not_running — nothing to restart; launch-app
- stale_process — no bootstrap dylib, a previous run's endpoint, or older than
this service's listener; restart-app attaches it
- unregistered — injected against this endpoint and started after the listener
came up, yet silent; the launch a restart would perform has
demonstrably already happened
- indeterminate — the process could not be read (ios-remote, unreadable `ps`)
Only the first two report `restart_required`. `unregistered` gets a new
`service_stale` precheck status that names the tool-server restart and says not
to relaunch the app; `indeterminate` keeps restart-app plus the loop warning,
since nothing measured says it will work. `describe` withholds `should_restart`
for `unregistered` and carries the diagnosis as a hint, and `flow-run` raises
the same diagnosis rather than a blanket "relaunch it".
The six native-* tools now type their block as the shared
`NativeDevtoolsPrecheckBlock` rather than restating it, so a new precheck
outcome cannot reach an agent through a tool that never mentions it.
Verified against a real iOS 18.6 simulator with two tool-servers racing the same
per-udid socket — the #561 shape, where the app is genuinely injected and
connected to the newer instance. Queried at the same instant on the same app:
main's stale instance reports requiresRestart:true / "Call restart-app then
retry" (the loop), this branch's reports state:unregistered, requiresRestart:
false and service_stale naming the tool-server restart. The healthy instance
serves 44 elements for the same app.
|
||
|
Ignacy Łątka
|
64bfca7994 |
fix(native-devtools): break the restart loop in the tool descriptions too
The runtime message now names the tool-server restart, but that is the second thing an agent reads. The descriptions of five native-* tools still said only "call restart-app then retry", so the guidance that rebuilds the loop was still the first thing in front of it — describe-screen, find-views, view-at-point, user-interactable-view-at-point and network-logs each promised a fix that never works once the service goes stale, with nothing after it. Each now carries the same escalation as the message: if restart_required survives the app restart, restart the tool-server rather than the app again. A test walks all five, so a tool cannot be added or reworded back into the loop while the runtime message quietly keeps the escalation to itself. |
||
|
Ignacy Łątka
|
7f936f29f8 |
fix(native-devtools): give the restart-required message a way out of the loop
`requiresAppRestart` returns true for every app that has no devtools connection, and keeps no record of how often it has already said so, so the "Call restart-app then retry" guidance stays literally true forever. When the connection never registers, that turns into an unbounded loop: the dylib dials the unix socket and logs `devtools connection established` on every launch, the service never records the connection, and so every native-* tool plus any flow whose `launch` step needs the connection keeps returning restart_required no matter how many times the app is restarted. The message offered no other action, so the only escape was knowing to restart the tool-server, which does clear it. Keep restart-app as the first step (it is the usual and sufficient fix for an app launched before the env carried DYLD_INSERT_LIBRARIES) and name the tool-server restart as the follow-up when a restart did not help. `flow-run` already tells agents this on its own connect failure; the shared precheck now matches it. This is the guidance half only — why the service can stop registering a connection it is listening for is tracked separately. |
||
|
Ignacy Łątka
|
b1cadd7cd6 |
fix(native-devtools): terminal state for non-injectable Apple system apps (#459)
Fixes #453 ## The bug For an app that can never be injected (an Apple built-in app such as `com.apple.Preferences`), `native-devtools-status` kept reporting `requiresRestart: true` / `nextLaunchWillBeInjected: true`, and the native-* feature tools (native-describe-screen, native-find-views, etc.) kept returning "restart-app then retry". Restarting never changes the state, so an autonomous agent loops `restart-app → retry` forever. There was no terminal "this app cannot be injected" signal. ## Why these apps can't inject Apple system/built-in apps (bundle ids under `com.apple.`) are platform binaries with library validation enabled; the simulator ignores `DYLD_INSERT_LIBRARIES` for them, so the Argent native devtools dylib can never load. This is a reliable, well-understood signal on the simulator. Third-party apps the user installs carry no such restriction and inject fine. So the fix treats bundle ids starting with `com.apple.` as non-injectable, via a general predicate rather than hardcoding the single bundle id from the issue. ## The fix - New pure predicate `isInjectableBundleId(bundleId)` in `blueprints/native-devtools.ts` returning `false` for `com.apple.*` bundle ids (case-sensitive prefix, matching how bundle ids are compared), with a doc comment explaining the library-validation rationale. - New failure code `NATIVE_DEVTOOLS_NOT_INJECTABLE` in `@argent/registry`. - `precheckNativeDevtools(api, udid, bundleId)` (the 3-arg overload) now throws a terminal `FailureError` with that code, *before* the `requiresAppRestart → restart_required` branch, when the bundle id is non-injectable. Because every native-* feature tool awaits this precheck and propagates the throw, all 7 tools now surface a hard, terminal error instead of an unbounded restart→retry instruction — no change needed in the individual tool files. The 2-arg overload (no bundleId), used by `native-devtools-status`, is unchanged and never throws (guarded by `bundleId !== undefined`). - `native-devtools-status` now *reports* the state instead of throwing: it adds an `injectable: boolean` field. For a non-injectable app it returns `injectable: false, requiresRestart: false, nextLaunchWillBeInjected: false` (with `envSetup`/`appRunning`/`connected` still measured), giving an agent that checks status first a terminal signal. The tool description documents the new field and that `injectable: false` is terminal (do not restart/retry; use the standard describe / screenshot / view-at-point tools). ## Verification - `npm run build` (root tsc): clean. - `vitest run` on `native-devtools-status.test.ts`: 13/13 passing. Added coverage for the predicate, the status terminal state for a `com.apple.*` app, and that the 3-arg precheck throws `NATIVE_DEVTOOLS_NOT_INJECTABLE` while the 2-arg overload does not throw for the same api. - Telemetry `sanitize.test.ts` (iterates every registered failure code): 49/49 passing — the new code is accepted. - Prettier + eslint (`--max-warnings 0`) clean on all touched files. Device E2E on an iOS simulator with `com.apple.Preferences` (confirming `native-devtools-status` returns `injectable: false` and the native-* tools return the terminal error rather than looping restart-app) is pending and will be run separately. --- ### Device verification (iOS 18.5 simulator, com.apple.Preferences) Reproduced before/after on a real simulator with a dev tool-server: **Before (origin/main):** `native-devtools-status` for `com.apple.Preferences` → `requiresRestart:true, nextLaunchWillBeInjected:true`; `native-describe-screen` → `restart_required`; after `restart-app` the status is identical (the infinite loop). **After (this branch):** - `native-devtools-status` → `{envSetup:true, appRunning:true, connected:false, requiresRestart:false, nextLaunchWillBeInjected:false, injectable:false}` — terminal. - `native-describe-screen` / `native-find-views` → terminal error: "com.apple.Preferences is an Apple system app ... can never be injected ... use the standard describe / screenshot / view-at-point tools instead." (no more restart→retry loop). **Regression (injectable third-party app `xyz.blueskyweb.app`):** `native-devtools-status` → `injectable:true` with the normal `requiresRestart:true`; `native-describe-screen` → the normal `restart_required` (not the terminal error). The predicate does not false-positive on normal apps. --- ### Review follow-ups (@hubgan on-device review) - **Recovery guidance no longer recommends `view-at-point`.** Both view-at-point tools run this same 3-arg precheck and re-throw `NATIVE_DEVTOOLS_NOT_INJECTABLE` for a `com.apple.*` bundle, so recommending them dead-ended the agent. All three surfaces (the precheck throw, the `describe` iOS fallback hint, and the `native-devtools-status` description) now share one verbatim "do not fall back to the native-* tools" warning (`NON_INJECTABLE_NATIVE_WARNING`) and recommend only `describe` and `screenshot`. ( |
||
|
Juliusz Wajgelt
|
20f5a1282c |
feat: initial sim-remote support (#291)
## Summary
Adds **initial support for remote iOS simulators** ("sim-remote") —
simulators that run in the cloud (radon-cloud orchestrator) and are
driven from the dev's machine over a QUIC/MoQ tunnel, alongside the
existing local iOS simulators. A remote sim is exposed as a new device
platform, `ios-remote`, and the existing
interaction/inspection/profiling tools work against it through the same
tool surface.
To get there without scattering `isRemote` branches across every tool,
the iOS code paths are refactored behind a small set of
host/transport/backend abstractions so each tool stays
transport-agnostic.
## How it works
- **Discovery** — `list-devices` enumerates remote sims via the
`sim-remote` CLI in addition to local `simctl`. They classify as
`platform: "ios-remote"`. Enumeration is best-effort: if `sim-remote`
isn't installed or the user isn't logged in, it silently returns none.
- **Transport** — remote interaction goes over a MoQ relay
(WebTransport/QUIC). `moq-client.ts` subscribes to the server-published
`simulator` broadcast (catalog/video/screenshot) and publishes a
`control` track carrying protobuf-encoded input commands (touch / key /
button / rotate / wheel / screenshot). WebTransport is provided in Node
by `@fails-components/webtransport`, polyfilled lazily on first use.
- **Native devtools over the tunnel** — unix sockets don't bridge the
QUIC tunnel, so on remote sims `ax-service` and `native-devtools` are
forced onto TCP with OS-assigned ephemeral ports, and a reverse proxy is
wired before the in-sim daemon dials back.
## Key components
New (`packages/tool-server/src/`):
- `utils/sim-remote.ts` — thin wrapper around the `sim-remote` CLI
(simctl verbs, `moq-info`, proxy), propagating CLI stderr verbatim.
- `utils/moq-client.ts` — pure-JS MoQ client for the remote
simulator-server relay.
- `utils/datachannel-proto.ts` + `proto/datachannel.proto` — vendored
protobuf schema + encoders for the control channel.
- `utils/ios-host.ts` — `IosHost` abstraction (`localIosHost` /
`remoteIosHost`, picked by `pickIosHost(device)`) capturing the
local-vs-remote differences (transport, env setup, proxying,
`requiresTcp`).
- `utils/simctl-backend.ts` — strategy for simctl verbs so one iOS
handler serves both `xcrun simctl` (local) and `sim-remote simctl`
(remote).
- `utils/ax-prefs.ts` — AX-pref helpers extracted from `ax-service`.
-
`tools/{launch-app,open-url,reinstall-app,restart-app}/platforms/ios-remote.ts`
+ shared handlers.
Refactors:
- **Strategy pattern** to limit `isRemote`/platform branching inside
tool and blueprint implementations.
- `simulator-client.ts` gains a transport abstraction
(`SimulatorServerTransport`) so call sites stay transport-agnostic;
local sims back it with WebSocket+HTTP, remote sims with the MoQ client.
- `ax-service` / `native-devtools` blueprints updated for ephemeral TCP
ports and the `ios-remote` platform.
- Dependency checks: tools that pick a platform path internally now gate
on the right CLI (`sim-remote` vs `xcrun`).
## New dependencies
`@moq/lite`, `@moq/net`, `@fails-components/webtransport`,
`@fails-components/webtransport-transport-http3-quiche`, `protobufjs`.
## Testing
- Existing unit suite passes (CI green); the native-devtools
status/describe tests now stub the platform dep-check so they exercise
their logic, not the host toolchain.
- E2E device-smoke jobs (local iOS sim, Android, Chromium, remote VVD)
pass.
---
<sub>Branch rebased onto latest `main` (linear history). `sim-remote` is
opt-in — only required when driving a remote iOS simulator.</sub>
|
||
|
filip131311
|
23cd32073a |
fix(native-devtools): re-apply DYLD env after out-of-band sim reboot (#346)
## Summary After an iOS simulator loses its launchd `DYLD_INSERT_LIBRARIES` (any reboot — out-of-band *or* an argent `boot-device`), `native-devtools-status` keeps reporting `envSetup: true` / `nextLaunchWillBeInjected: true` while the env is actually **empty**. Freshly launched apps aren't injected and native feature tools (`native-describe-screen`, `native-find-views`, …) fail with `restart_required` — and `restart-app` couldn't break out of it. ## Root cause `envSetup` is a **write-once latch**. `ensureEnvReady()` short-circuits once it flips `true`, and nothing resets it, so the env is never re-applied after a reboot clears it. Two paths relied on that latched function and so never recovered: 1. `requiresAppRestart()` — written to recover from the reboot case, but called the latched `ensureEnvReady()`. 2. `boot-device` — primes the native-devtools service after (re)booting, but the simulator watcher usually initialized it earlier, so `resolveService` returns the cached/latched instance and the wiped env is never re-applied. A clean argent `boot-device` + `launch-app` therefore produced an uninjected app and a spurious `restart_required`. ## Fix - Split a latch-free `runEnsureEnv()` out of `ensureEnvReady()` and add `reverifyEnv()` — re-applies the env even when the latch says it's set (still honouring the give-up guard). `ensureEnv` is already idempotent, so it's a cheap no-op when correct. - `requiresAppRestart()` → uses `reverifyEnv()` (fixes the native feature tools' recovery; precheck routes through it). - `native-devtools-status` → calls `reverifyEnv()` when the app isn't connected, so its flags reflect reality and a later launch is injected. - `boot-device` → forces `reverifyEnv()` after the (re)boot, so a clean argent boot injects on first launch with no restart. The hot path (`ensureEnvReady`, app already connected) is unchanged — no added latency on normal calls; the extra simctl round-trips only fire when there's evidence the env is stale. ## Verified end-to-end on a real iOS 26.3 sim (patched tool-server) - **Clean argent boot → launch → just works:** prime service, `boot-device force`, `launch-app` → `connected:true`, `requiresRestart:false`, `native-describe-screen` returns the full tree. `DYLD_INSERT_LIBRARIES` is present immediately after `boot-device` returns. ✅ - **Out-of-band reboot → `native-devtools-status`:** status call self-heals the env; next launch is injected. ✅ - **Out-of-band reboot → launch → native tool → `restart-app`:** the previously dead-end recovery now works. ✅ - **Before the fix (same hardware):** status claimed `envSetup:true` with the env empty; launches uninjected; `restart-app` and `boot-device force` did not recover. ## Tests - New regression tests: `native-devtools-status` re-applies when disconnected / does not when connected; `boot-device` forces a re-apply after boot. - Full `packages/tool-server` suite green (1155), typecheck + prettier clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Radoniusz Cloudiusz <radoniusz@radoniusz.local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Paweł Fornagiel
|
4255b0bd66 |
fix: stop simctl spawn loop on iOS init failure (#233)
Fixes #210 — `argent mcp` pinning CPU to 100% and spawning hundreds of `simctl` processes that survive `kill -9`. ## The problems 1. **No retry cap.** The simulator watcher polls every 10s; on any failure it forgets the UDID and re-classifies it as "new" on the next tick. Each retry runs 4–5 `xcrun simctl spawn` calls in series. A persistently failing simulator produces an unbounded loop — hundreds of `entered ERROR state` log lines and a steady process pile-up. 2. **No timeouts on `simctl spawn`.** A degraded `CoreSimulatorService` makes simctl block forever. The factory `await`s the hung call, never rejects, never retries, but the stuck child process stays alive. ## How this fixes it - **Bounded retries with terminal "given up" state.** Each `NativeDevtools` service instance now tracks `{attempts, lastError, givenUp}`. After 3 consecutive init failures the service stops trying and reports `init_failed` to any caller. The watcher keeps polling but skips UDIDs in the given-up state. - **10s timeout on every `simctl spawn` call** in `ensureEnv`, `ensureAccessibilityEnabled`, and `ensureAutomationEnabled`. Healthy CSS responds in ~300ms; 10s is well above any plausible legit latency and well below "hung forever." A hung simctl now rejects cleanly, increments the attempt counter, and feeds the bounded retry instead of accumulating children. - **Tools surface `init_failed` as a typed result** instead of throwing. `launch-app`, `boot-device`, `native-devtools-status`, and the rest of the native tools now return a structured error with the failure reason and a hint to restart the simulator, instead of throwing an opaque exception. - **Factory no longer throws on first env-init failure.** Previously a single failed `simctl` call during factory creation leaked the socket server. The factory now tolerates init failure; the watcher's bounded retry handles the recovery. Verified on live simulator and against test simulating `simctl` timeouts. Happy path works as before, `restart-needed` is still signalled and the new failure path works as well. |
||
|
Ignacy Łątka
|
837d088744 |
test: cover #148 (android) (#186)
This PR adds tests for #148. ## Summary Moves the Android emulator branch's test coverage out of #148 so that PR can be reviewed without the test churn. This branch adds the test files back on top of the (now test-stripped) base branch. Tests inseparable from source changes (`describe-tool`, `workspace-reader`, `workspace-reader-integration`) and the `boot-simulator.test.ts` deletion stayed on #148 - base PR's CI passes on its own. ## Test plan - [x] `npx tsc --build` — clean - [x] `npx prettier --check .` — clean - [x] `npm test --workspaces --if-present` — 585/585 tool-server tests pass; all other workspaces green --------- Co-authored-by: Paweł Fornagiel <pawel.fornagiel2@gmail.com> |
||
|
filip131311
|
d9dc17d0d5 |
fix: move describe to native devtools (#83)
* fix: move describe to native devtools Replace the AX describe path with an app-scoped native hierarchy adapter so coordinate discovery stays on the supported injection flow. Also add safer targeting and restart guidance to reduce ambiguous Home and cold-start recovery paths. Made-with: Cursor * init * update sim server * Delete NATIVE_DEVTOOLS_DESCRIBE_MIGRATION_NOTE.md |