Skip to content

fix(native-devtools): terminal state for non-injectable Apple system apps#459

Merged
latekvo merged 10 commits into
mainfrom
fix/native-devtools-non-injectable-terminal-state
Jul 10, 2026
Merged

fix(native-devtools): terminal state for non-injectable Apple system apps#459
latekvo merged 10 commits into
mainfrom
fix/native-devtools-non-injectable-terminal-state

Conversation

@latekvo

@latekvo latekvo commented Jul 2, 2026

Copy link
Copy Markdown
Member

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.PreferencesrequiresRestart:true, nextLaunchWillBeInjected:true; native-describe-screenrestart_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-statusinjectable: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. (52b888c)
  • Propagation tests for all six feature tools. Added tests that drive each 3-arg feature tool's execute() with a com.apple.* bundle and assert the throw propagates out as NATIVE_DEVTOOLS_NOT_INJECTABLE, so a future try/catch inside a tool can't silently regress the terminal behavior (proven non-vacuous via a swallow-the-throw mutation). (52b888c)
  • Degraded sim + system app keeps the re-boot hint. When the ax-service is degraded, describe now returns the boot-device force=true guidance instead of the terminal screenshot hint - a proper re-boot may let the ax-service read the system app's full tree. should_restart is still never set, so the restart loop stays removed. (5ace8be)

Second review round (@hubgan) + follow-up hardening

  • Terminal check hoisted above env init in the precheck. Injectability is a static property of the bundle id, so the NATIVE_DEVTOOLS_NOT_INJECTABLE throw now fires before the givenUp/ensureEnvReady plumbing - a broken env can no longer mask the terminal signal behind init_failed's re-boot guidance, and no env work is spent on a system app. Regression tests cover both masked paths. (ca7093c)
  • The same ordering applied to the other two surfaces. native-devtools-status computes the injectable gate before the precheck (a given-up sim no longer hides injectable: false behind init_failed), and describe's gate moved ahead of native-devtools service resolution (a downed ios-remote tunnel or dispose race no longer swallows the terminal hint). (bef6dfc)
  • Unreachable-sim fallback. If the status tool's isAppRunning probe fails on a non-injectable app (shut-down sim), it falls back to the precheck so a broken sim still surfaces structured init_failed - whose re-boot guidance IS corrective there - instead of a raw subprocess error; a healthy env rethrows the probe failure. (722b432)
  • Warning text scoped to the tools it is true of. NON_INJECTABLE_NATIVE_WARNING now names the six injection-based feature tools instead of sweeping in all native-* tools - the native-profiler-* tools never inject and keep working on system apps. (bef6dfc)
  • launch-app / restart-app boundary pinned. New tests drive both tools (local iOS and ios-remote dispatch, all four call sites) with com.apple.Preferences and the real precheck, asserting the terminal throw never reaches them - proven non-vacuous by a 3-arg mutation failing exactly the mutated sites. (bef6dfc, 722b432)

latekvo added 2 commits July 2, 2026 13:31
…em-app loop)

The non-injectable terminal-state gate landed on the low-level native-* tools
and native-devtools-status, but NOT on the public `describe` tool — the most
common consumer. Its iOS native fallback called `requiresAppRestart` directly,
which is always true for a com.apple.* app (it can never connect), so describe
returned `should_restart: true` → the agent restarts the system app → AX is
still empty → describe again → unbounded loop (the exact loop this branch set
out to kill), and it contradicted native-devtools-status ("do NOT restart").

Gate the fallback on `isInjectableBundleId(target.bundleId)`: a system app now
returns the (empty) AX result with a screenshot hint instead of should_restart.
Adds a repro test (fails pre-fix: should_restart was true) and retargets the
existing native-fallback test to an injectable bundle id.
@latekvo
latekvo marked this pull request as ready for review July 2, 2026 17:56

@hubgan hubgan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few inline notes from reviewing and testing this branch on-device (iPhone 16 Pro and Pixel 3a).

Comment thread packages/tool-server/src/blueprints/native-devtools.ts Outdated
Comment thread packages/tool-server/src/blueprints/native-devtools.ts Outdated
Comment thread packages/tool-server/src/tools/native-devtools/native-devtools-status.ts Outdated
Comment thread packages/tool-server/src/tools/describe/platforms/ios/index.ts Outdated
latekvo added 3 commits July 3, 2026 15:24
…ropagation tests

Addresses review on #459.

- Recovery guidance no longer recommends `view-at-point`: both view-at-point
  tools run the same 3-arg precheck and re-throw NATIVE_DEVTOOLS_NOT_INJECTABLE
  for a com.apple.* bundle, so recommending them dead-ends the agent right back
  on this error. Recovery now points only at `describe` (reads these apps via
  the ax-service, no injection) and `screenshot`.

- Unify the three surfaces that report this terminal state (the precheck throw,
  the describe iOS fallback hint, and the native-devtools-status description).
  The "do not fall back to the native-* tools" warning is now one shared const
  (NON_INJECTABLE_NATIVE_WARNING) embedded verbatim by all three, so none can
  drift into recommending a dead-end. The describe fallback hint keeps leading
  with `screenshot` (it is reached only after describe's own AX path returned
  empty, so re-recommending describe there would be circular).

- Add tests driving every 3-arg feature tool's execute() with a com.apple.*
  bundle, asserting the throw propagates out as NATIVE_DEVTOOLS_NOT_INJECTABLE.
  Previously only the precheck itself was tested; a later try/catch inside a
  tool could have silently swallowed the throw while the suite stayed green.

- Add a consistency test asserting all three surfaces carry the same warning and
  that the recommendation never names a native-* tool outside it.
…system apps

When the ax-service is degraded (the simulator was not booted through argent) it
returns an empty tree, so describe reaches the non-injectable branch for a
com.apple.* app. The empty tree there is a fixable sim-config problem, not proof
the system app is undescribable - a proper `boot-device force=true` may let the
ax-service read its full tree (Settings et al. expose a rich AX tree). The branch
was overriding the DEGRADED_HINT ("you MUST boot-device force=true") with the
terminal "use screenshot" hint, so the agent never learned its sim was degraded
(which affects every describe call in the session) and missed that a re-boot
would yield the full tree.

Return `hint ?? NON_INJECTABLE_HINT`: on a degraded sim the re-boot guidance wins;
on a healthy sim `hint` is undefined and it falls back to the terminal
non-injectable hint. should_restart is still never set, so the restart loop this
branch removed stays removed. Adds a test (fails pre-fix: hint was the screenshot
hint, not the boot-device hint).
…N_INJECTABLE_RECOVERY jsdoc

The jsdoc predated the degraded-hint change and read as if the describe surface
always carries the dead-end warning at runtime. NON_INJECTABLE_HINT still always
contains it, but describeIos substitutes the sim re-boot hint when the ax-service
is degraded. Clarify so the contract is precise.

@hubgan hubgan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the diff and exercised it end-to-end against this branch's tool-server on iOS 26 simulators (iPad Pro 13" M5 and iPhone 17 Pro) plus an Android tablet. The behavior matches the intent: a com.apple.* app reports injectable:false with no requiresRestart, the native-* feature tools return the terminal error instead of "restart then retry", and describe never sets should_restart for a system app (confirmed both the empty-tree hint path and the real-tree path). Injectable apps are unaffected and Android is untouched. No correctness issues found.

A few smaller notes inline. The one worth acting on is the missing test coverage for a system app with a non-empty accessibility tree; the rest are comment/doc-accuracy nits.

Comment thread packages/tool-server/test/describe-tool.test.ts
Comment thread packages/tool-server/src/blueprints/native-devtools.ts Outdated
Comment thread packages/tool-server/src/blueprints/native-devtools.ts Outdated
Comment thread packages/tool-server/src/tools/native-devtools/native-devtools-status.ts Outdated
Comment thread packages/tool-server/test/native-devtools-status.test.ts Outdated
Comment thread packages/tool-server/test/describe-tool.test.ts Outdated
Comment thread packages/tool-server/test/native-devtools-status.test.ts Outdated
latekvo added 2 commits July 7, 2026 14:43
- isInjectableBundleId matches the com.apple. prefix case-insensitively so a
  stray mixed-case system-app id (com.Apple.Preferences) can't slip through as
  injectable and drop the agent into a restart loop; corrected the "bundle ids
  are case-sensitive" rationale (iOS treats them case-insensitively).
- NON_INJECTABLE_NATIVE_WARNING jsdoc now notes native-devtools-status is the
  lone native-* tool that reports injectable:false instead of re-throwing.
- native-devtools-status comment no longer implies envSetup is freshly measured
  on the non-injectable path (it reads the cached latch).
- describe: add the missing regression test for a non-injectable system app with
  a NON-EMPTY accessibility tree (returns the real tree via the early return,
  before the injectability gate) — proven to fail if the gate is hoisted above
  the return.
- Test-comment cleanups: drop the "finding #4" review references and the inert
  {appRunning,connected} mock config in the propagation tests.
…test

`expect(result.description).not.toContain(NON_INJECTABLE_NATIVE_WARNING)` can
never fail: the warning only ever lands in the `hint` result field, never in the
rendered tree `description`. The load-bearing guard on that path is the
`expect(result.hint).toBeUndefined()` line above it.

@hubgan hubgan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor, non-blocking notes inline.

Comment thread packages/tool-server/src/blueprints/native-devtools.ts Outdated
Comment thread packages/tool-server/src/blueprints/native-devtools.ts Outdated
Comment thread packages/tool-server/test/native-devtools-status.test.ts Outdated
latekvo added 3 commits July 9, 2026 23:59
…nv init

Injectability is a static property of the bundle id, so the terminal
NATIVE_DEVTOOLS_NOT_INJECTABLE throw now fires before the givenUp /
ensureEnvReady plumbing: a broken env can no longer mask the terminal
signal behind init_failed's re-boot-the-simulator guidance, and no
env-setup work is spent on an app that can never load the dylib.

Also scopes the 2-arg-overload comment to all its consumers (status,
launch-app, restart-app) and retitles the guidance-consistency test to
the two surfaces it actually asserts.
…face

Two surfaces could still hide the terminal signal the precheck now hoists:

- native-devtools-status ran the 2-arg precheck before the injectable
  gate, so a sim with failing/given-up env init returned init_failed
  (re-boot guidance that can never help a system app) instead of
  injectable:false. The gate now runs first, mirroring the precheck.
- describeIos gated on injectability only after resolving the
  native-devtools service, so a resolution failure (downed ios-remote
  tunnel, dispose race) swallowed the terminal hint into the generic
  catch. The gate now sits before the fallback try, keyed on the
  explicit bundleId; auto-resolution needs no gate since it only yields
  connected (hence injected) apps.

Also: scope NON_INJECTABLE_NATIVE_WARNING to the six injection-based
feature tools it is true of (the native-profiler-* tools never inject
and keep working on system apps), fix the 'canonically lowercase' jsdoc
claim (com.apple.Preferences is not lowercase), note the always-false
fields and the init_failed return shape in the status description, and
pin the launch-app/restart-app tool boundary with system-app tests that
run the real precheck (proven non-vacuous by a 3-arg mutation).
…nreachable sim

The status tool's terminal branch probes isAppRunning (a simctl spawn)
before the precheck, so on a shut-down/unreachable sim a com.apple.*
query threw the raw subprocess error where the parent returned the
structured init_failed block. On probe failure the branch now falls
back to the precheck — a broken sim surfaces init_failed (whose re-boot
guidance IS corrective for a dead sim) and a healthy env rethrows the
probe failure.

Also covers the ios-remote launch-app/restart-app call sites
(platforms/shared.ts) in the system-app boundary tests — a remote:
udid dispatches them with the real precheck (proven by a 3-arg
mutation failing exactly those two) — and drops the 'repeatedly' /
'injectable apps only' overclaims from the status description.
@latekvo
latekvo merged commit b1cadd7 into main Jul 10, 2026
6 checks passed
@latekvo
latekvo deleted the fix/native-devtools-non-injectable-terminal-state branch July 10, 2026 10:51
latekvo added a commit that referenced this pull request Jul 20, 2026
…ion stays a plain literal

The native-devtools-status description interpolated NON_INJECTABLE_RECOVERY
(#459), which the extract-tools policy gate rejects: an interpolated
description cannot be read statically, so the tool would silently leave the
spidershield scan. Inline the text verbatim (escaped backticks cook to the
identical runtime string) and keep the existing native-devtools-status.test.ts
assertion as the drift pin.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

native-devtools injection status never reaches a terminal state for non-injectable apps (restart→retry loops forever)

2 participants