mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
b1cadd7cd6
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`. (52b888ce) - **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). (52b888ce) - **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. (5ace8be3) --- ### 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. (ca7093c6) - **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). (bef6dfcb) - **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. (722b432a) - **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. (bef6dfcb) - **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. (bef6dfcb,722b432a)
125 lines
4.6 KiB
TypeScript
125 lines
4.6 KiB
TypeScript
import { describe, it, expect, vi, beforeEach } from "vitest";
|
|
|
|
// launch-app / restart-app must keep working for Apple system apps: launching
|
|
// or restarting com.apple.* is legitimate — the app just never injects. They
|
|
// stay safe from the terminal NATIVE_DEVTOOLS_NOT_INJECTABLE throw only
|
|
// because all four iOS call sites use the 2-arg precheckNativeDevtools
|
|
// overload (no bundleId). Unlike launch-restart-tvos.test.ts, the REAL
|
|
// precheck runs here — these tests pin the tool boundary so a call site
|
|
// "improved" to pass params.bundleId (3-arg) starts failing loudly instead of
|
|
// shipping a launch-app that throws on Settings/Safari. A plain udid
|
|
// dispatches the local iOS sites (platforms/ios.ts); a `remote:`-prefixed
|
|
// udid dispatches the ios-remote sites (platforms/shared.ts) — all four are
|
|
// covered.
|
|
|
|
const execFileMock = vi.fn(
|
|
(
|
|
_cmd: string,
|
|
_args: readonly string[],
|
|
opts: unknown,
|
|
cb?: (err: Error | null, out: { stdout: string; stderr: string }) => void
|
|
) => {
|
|
const callback = typeof opts === "function" ? opts : cb!;
|
|
callback(null, { stdout: "", stderr: "" });
|
|
}
|
|
);
|
|
|
|
vi.mock("node:child_process", async () => {
|
|
const actual = await vi.importActual<typeof import("node:child_process")>("node:child_process");
|
|
return { ...actual, execFile: (...a: unknown[]) => (execFileMock as any)(...a) };
|
|
});
|
|
|
|
vi.mock("../src/utils/ios-devices", () => ({
|
|
isTvOsSimulator: vi.fn(async () => false),
|
|
}));
|
|
|
|
import { createLaunchAppTool } from "../src/tools/launch-app";
|
|
import { createRestartAppTool } from "../src/tools/restart-app";
|
|
import type { NativeDevtoolsApi } from "../src/blueprints/native-devtools";
|
|
import { __primeDepCacheForTests, __resetDepCacheForTests } from "../src/utils/check-deps";
|
|
|
|
const IOS_UDID = "AAAAAAAA-AAAA-AAAA-AAAA-AAAAAAAAAAAA";
|
|
const REMOTE_UDID = `remote:${IOS_UDID}`;
|
|
const SYSTEM_APP = "com.apple.Preferences";
|
|
|
|
// Minimal api satisfying what the real 2-arg precheck consults
|
|
// (getInitFailure + ensureEnvReady); the rest must stay untouched.
|
|
function makeNativeApi(): NativeDevtoolsApi {
|
|
return {
|
|
isEnvSetup: () => true,
|
|
socketPath: "/tmp/test.sock",
|
|
ensureEnvReady: async () => {},
|
|
reverifyEnv: async () => {},
|
|
getInitFailure: () => null,
|
|
isConnected: () => false,
|
|
isAppRunning: async () => false,
|
|
listConnectedBundleIds: () => [],
|
|
requiresAppRestart: async () => false,
|
|
activateNetworkInspection: () => {},
|
|
getNetworkLog: () => [],
|
|
clearNetworkLog: () => {},
|
|
getAppState: async () => {
|
|
throw new Error("not implemented");
|
|
},
|
|
detectFrontmostBundleId: async () => null,
|
|
queryViewHierarchy: async () => ({}),
|
|
} as NativeDevtoolsApi;
|
|
}
|
|
|
|
function makeRegistry() {
|
|
return { resolveService: vi.fn(async () => makeNativeApi() as unknown) } as any;
|
|
}
|
|
|
|
beforeEach(() => {
|
|
execFileMock.mockClear();
|
|
// The local branch requires xcrun, the remote branch sim-remote; prime both
|
|
// so neither probes the host toolchain (Linux CI has neither).
|
|
__resetDepCacheForTests();
|
|
__primeDepCacheForTests(["xcrun", "sim-remote"]);
|
|
});
|
|
|
|
describe("launch-app / restart-app — Apple system apps stay launchable (real precheck)", () => {
|
|
it("launch-app launches com.apple.* without the non-injectable terminal throw", async () => {
|
|
const tool = createLaunchAppTool(makeRegistry());
|
|
|
|
await expect(tool.execute!({}, { udid: IOS_UDID, bundleId: SYSTEM_APP })).resolves.toEqual({
|
|
launched: true,
|
|
bundleId: SYSTEM_APP,
|
|
});
|
|
});
|
|
|
|
it("restart-app restarts com.apple.* without the non-injectable terminal throw", async () => {
|
|
const tool = createRestartAppTool(makeRegistry());
|
|
|
|
await expect(tool.execute!({}, { udid: IOS_UDID, bundleId: SYSTEM_APP })).resolves.toEqual({
|
|
restarted: true,
|
|
bundleId: SYSTEM_APP,
|
|
});
|
|
});
|
|
|
|
it("launch-app (ios-remote) launches com.apple.* without the terminal throw", async () => {
|
|
// The remote branch reads services.nativeDevtools instead of resolving
|
|
// lazily; the shared handler (platforms/shared.ts) runs the same real
|
|
// 2-arg precheck.
|
|
const tool = createLaunchAppTool(makeRegistry());
|
|
|
|
await expect(
|
|
tool.execute!(
|
|
{ nativeDevtools: makeNativeApi() },
|
|
{ udid: REMOTE_UDID, bundleId: SYSTEM_APP }
|
|
)
|
|
).resolves.toEqual({ launched: true, bundleId: SYSTEM_APP });
|
|
});
|
|
|
|
it("restart-app (ios-remote) restarts com.apple.* without the terminal throw", async () => {
|
|
const tool = createRestartAppTool(makeRegistry());
|
|
|
|
await expect(
|
|
tool.execute!(
|
|
{ nativeDevtools: makeNativeApi() },
|
|
{ udid: REMOTE_UDID, bundleId: SYSTEM_APP }
|
|
)
|
|
).resolves.toEqual({ restarted: true, bundleId: SYSTEM_APP });
|
|
});
|
|
});
|