mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(adb): evaluate the full activity start report
Evaluate stdout and stderr together, giving hard errors and non-ok wait statuses precedence over an attempted Starting line. Retain raw streams and typed partial launch facts when Activity Manager refuses or times out; retain successful raw output too. Validation: parser/provider tests and public MCP app/activity starts through a fake executable at the real process boundary. Real Android device behavior was not verified; device-reported timeout remains distinct from a host deadline. Fixes #2225
This commit is contained in:
+42
-13
@@ -23,9 +23,11 @@ import {
|
||||
type AnalysisExecution,
|
||||
} from "../application/AnalysisProvider.js";
|
||||
import type {
|
||||
AdbActivityStartPartialObservation,
|
||||
AdbAcquiredFilePartialObservation,
|
||||
AdbRequest,
|
||||
} from "../domain/adb/adbDeviceAnalysis.js";
|
||||
import { adbResultSchemas } from "../domain/adb/adbDeviceAnalysis.js";
|
||||
import {
|
||||
AnalysisArtifactChangedError,
|
||||
AnalysisCancelledError,
|
||||
@@ -1985,28 +1987,55 @@ export class AdbProvider {
|
||||
signal,
|
||||
);
|
||||
if (!started.ok) return started;
|
||||
const verdict = parseAmStartOutput(started.value.stdout);
|
||||
if (verdict.status === "refused")
|
||||
const verdict = parseAmStartOutput(
|
||||
started.value.stdout,
|
||||
started.value.stderr,
|
||||
);
|
||||
const result = {
|
||||
...resultFields,
|
||||
started_activity: verdict.startedActivity,
|
||||
reported_activity: verdict.activity,
|
||||
reported_status: verdict.reportedStatus,
|
||||
};
|
||||
if (verdict.status === "refused") {
|
||||
const partialObservation: AdbActivityStartPartialObservation =
|
||||
operation === "start_adb_app"
|
||||
? {
|
||||
provider_id: "adb",
|
||||
operation,
|
||||
result: adbResultSchemas.start_adb_app.parse(result),
|
||||
}
|
||||
: {
|
||||
provider_id: "adb",
|
||||
operation,
|
||||
result: adbResultSchemas.start_adb_activity.parse(result),
|
||||
};
|
||||
return err(
|
||||
new AnalysisUnsupportedTargetError(
|
||||
operation,
|
||||
serial,
|
||||
verdict.reason ?? "am start refused the intent",
|
||||
{
|
||||
capturedOutput: {
|
||||
stdout: started.value.stdout,
|
||||
stderr: started.value.stderr,
|
||||
truncated: false,
|
||||
},
|
||||
partialObservation,
|
||||
},
|
||||
),
|
||||
);
|
||||
}
|
||||
return ok(
|
||||
createAnalysisExecution(
|
||||
{
|
||||
...resultFields,
|
||||
started_activity: verdict.startedActivity,
|
||||
createAnalysisExecution(result, provider, {
|
||||
rawResult: {
|
||||
stdout: started.value.stdout,
|
||||
stderr: started.value.stderr,
|
||||
},
|
||||
provider,
|
||||
{
|
||||
limitations: [
|
||||
"Starting an activity changes device state; am start -W waits for launch completion but no lifecycle observation is captured",
|
||||
],
|
||||
},
|
||||
) as AnalysisExecution,
|
||||
limitations: [
|
||||
"Starting an activity changes device state; am start -W waits for launch completion but no lifecycle observation is captured",
|
||||
],
|
||||
}) as AnalysisExecution,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { parseAmStartOutput } from "./AdbShellOutput.js";
|
||||
|
||||
const starting =
|
||||
"Starting: Intent { act=android.intent.action.MAIN cmp=com.example/.Main }";
|
||||
|
||||
describe("parseAmStartOutput", () => {
|
||||
it("recognizes a CRLF wait-timeout report", () => {
|
||||
expect(
|
||||
parseAmStartOutput(
|
||||
`${starting}\r\nStatus: timeout\r\nActivity: com.example/.Main\r\nComplete\r\n`,
|
||||
),
|
||||
).toMatchObject({
|
||||
status: "refused",
|
||||
activity: "com.example/.Main",
|
||||
reportedStatus: "timeout",
|
||||
reason: "am start reported status: timeout",
|
||||
});
|
||||
});
|
||||
|
||||
it("treats every non-ok status as refusal, including a later status after ok", () => {
|
||||
expect(
|
||||
parseAmStartOutput(`${starting}\nStatus: ok\nStatus: timeout\n`),
|
||||
).toMatchObject({
|
||||
status: "refused",
|
||||
reportedStatus: "timeout",
|
||||
reason: "am start reported status: timeout",
|
||||
});
|
||||
expect(
|
||||
parseAmStartOutput(`${starting}\nStatus: future-value\n`),
|
||||
).toMatchObject({
|
||||
status: "refused",
|
||||
reportedStatus: "future-value",
|
||||
});
|
||||
});
|
||||
|
||||
it("gives hard errors in either stream priority over status and attempt", () => {
|
||||
expect(
|
||||
parseAmStartOutput(
|
||||
`${starting}\nStatus: timeout\nActivity: com.example/.Main\n`,
|
||||
"Exception: Activity class does not exist\n",
|
||||
),
|
||||
).toEqual({
|
||||
status: "refused",
|
||||
startedActivity: "act=android.intent.action.MAIN cmp=com.example/.Main",
|
||||
activity: "com.example/.Main",
|
||||
reportedStatus: "timeout",
|
||||
reason: "Activity class does not exist",
|
||||
});
|
||||
expect(
|
||||
parseAmStartOutput(
|
||||
`${starting}\nStatus: ok\nError: Activity class does not exist\n`,
|
||||
),
|
||||
).toMatchObject({
|
||||
status: "refused",
|
||||
reason: "Activity class does not exist",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps Starting-only output compatible and refuses no-attempt output", () => {
|
||||
expect(parseAmStartOutput(starting)).toMatchObject({
|
||||
status: "started",
|
||||
reportedStatus: null,
|
||||
activity: null,
|
||||
});
|
||||
expect(
|
||||
parseAmStartOutput("Status: ok\nWarning: Activity not started\n"),
|
||||
).toMatchObject({
|
||||
status: "refused",
|
||||
startedActivity: null,
|
||||
reportedStatus: "ok",
|
||||
});
|
||||
});
|
||||
});
|
||||
+63
-15
@@ -320,32 +320,80 @@ export const parseResolveActivityOutput = (stdout: string): string | null => {
|
||||
return lines.length === 0 ? null : lines[lines.length - 1]!;
|
||||
};
|
||||
|
||||
/** Parsed facts and verdict from the full `am start -W` report. */
|
||||
export interface AmStartOutput {
|
||||
readonly status: "started" | "refused";
|
||||
readonly startedActivity: string | null;
|
||||
readonly activity: string | null;
|
||||
readonly reportedStatus: string | null;
|
||||
readonly reason: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse `am start` output. Success prints `Starting: Intent { ... }`;
|
||||
* refusals print lines starting with `Error:` or `Warning:` and activity
|
||||
* classes that do not exist name the component.
|
||||
* Parse the complete `am start -W` report. `Starting:` records an attempt;
|
||||
* hard errors and any non-ok Activity Manager status determine the verdict.
|
||||
*/
|
||||
export const parseAmStartOutput = (
|
||||
stdout: string,
|
||||
): {
|
||||
readonly status: "started" | "refused";
|
||||
readonly startedActivity: string | null;
|
||||
readonly reason: string | null;
|
||||
} => {
|
||||
const text = stdout.trim();
|
||||
const starting = /^Starting:\s+Intent\s*\{(.*)\}\s*$/mu.exec(text);
|
||||
if (starting !== null)
|
||||
stderr = "",
|
||||
): AmStartOutput => {
|
||||
const normalizedStdout = stdout.replaceAll("\r\n", "\n");
|
||||
const starting = /^Starting:[ \t]+Intent[ \t]*\{(.*)\}[ \t]*$/mu.exec(
|
||||
normalizedStdout,
|
||||
);
|
||||
const startedActivity = starting?.[1]?.trim() ?? null;
|
||||
const activityMatch = /^Activity:[ \t]*(.*)$/mu.exec(normalizedStdout);
|
||||
const activity = activityMatch?.[1]?.trim() || null;
|
||||
const statuses = [...normalizedStdout.matchAll(/^Status:[ \t]*(.*)$/gmu)].map(
|
||||
(match) => match[1]!.trim(),
|
||||
);
|
||||
const nonOkStatus = statuses.find((status) => status.toLowerCase() !== "ok");
|
||||
const reportedStatus = nonOkStatus ?? statuses[0] ?? null;
|
||||
const error = [normalizedStdout, stderr.replaceAll("\r\n", "\n")]
|
||||
.flatMap((stream) => stream.split("\n"))
|
||||
.map((line) => line.trim())
|
||||
.map((line) => ({
|
||||
line,
|
||||
match: /^(?:Error|Exception)(?::|\b)(.*)$/iu.exec(line),
|
||||
}))
|
||||
.find(({ match }) => match !== null);
|
||||
const errorReason = error?.match?.[1]?.trim() || error?.line || null;
|
||||
const statusReason =
|
||||
nonOkStatus === ""
|
||||
? "am start reported an empty status"
|
||||
: nonOkStatus === undefined
|
||||
? null
|
||||
: `am start reported status: ${nonOkStatus}`;
|
||||
const reason =
|
||||
errorReason ??
|
||||
statusReason ??
|
||||
(/^Warning:/mu.test(normalizedStdout.trim())
|
||||
? normalizedStdout.trim().slice(0, 512)
|
||||
: null);
|
||||
if (errorReason !== null || nonOkStatus !== undefined)
|
||||
return {
|
||||
status: "refused",
|
||||
startedActivity,
|
||||
activity,
|
||||
reportedStatus,
|
||||
reason: reason ?? "am start did not report a successful launch",
|
||||
};
|
||||
if (startedActivity !== null)
|
||||
return {
|
||||
status: "started",
|
||||
startedActivity: starting[1]!.trim(),
|
||||
startedActivity,
|
||||
activity,
|
||||
reportedStatus,
|
||||
reason: null,
|
||||
};
|
||||
const error = /^(?:Error|Warning|Exception)(?::|\b)(.*)$/mu.exec(text);
|
||||
const text = `${normalizedStdout}\n${stderr.replaceAll("\r\n", "\n")}`.trim();
|
||||
return {
|
||||
status: "refused",
|
||||
startedActivity: null,
|
||||
startedActivity,
|
||||
activity,
|
||||
reportedStatus,
|
||||
reason:
|
||||
error?.[1]?.trim() ??
|
||||
reason ??
|
||||
(text === "" ? "am start returned no activity" : text.slice(0, 512)),
|
||||
};
|
||||
};
|
||||
|
||||
@@ -326,7 +326,7 @@ export const ADB_TOOL_CONTRACTS = [
|
||||
name: "start_adb_app",
|
||||
...toolContractMetadata("start_adb_app"),
|
||||
description:
|
||||
"Start one installed app by launching its declared launcher activity: the component is resolved through `cmd package resolve-activity --brief`, then started with `am start -n`. A device-mutating tool for dynamic-analysis workflows; pair with `read_adb_logcat` (pid filter) and `list_adb_processes` to observe the started app, and `stop_adb_app` to end the run.",
|
||||
"Start one installed app's resolved launcher activity with `am start -W -n`. A device-mutating tool for dynamic analysis. The full wait report determines success; failures retain launch-attempt facts and raw output. Pair with `read_adb_logcat` (pid filter) and `list_adb_processes` to observe the app, and `stop_adb_app` to end the run.",
|
||||
kind: "adb-provider",
|
||||
inputSchema: adbInputSchemas.start_adb_app,
|
||||
outputSchema: evidenceResultOf(adbResultSchemas.start_adb_app),
|
||||
@@ -341,7 +341,7 @@ export const ADB_TOOL_CONTRACTS = [
|
||||
name: "start_adb_activity",
|
||||
...toolContractMetadata("start_adb_activity"),
|
||||
description:
|
||||
"Start an intent with `am start`: one action, optional data URI, optional explicit component, and up to 16 typed extras (string, boolean, int, long, float). A device-mutating tool; the activity manager's Starting/Error verdict is preserved verbatim.",
|
||||
"Start an intent with `am start -W`: one action, optional data URI, optional explicit component, and up to 16 typed extras (string, boolean, int, long, float). A device-mutating tool; the full wait report determines success, with failed-attempt facts and raw output retained.",
|
||||
kind: "adb-provider",
|
||||
inputSchema: adbInputSchemas.start_adb_activity,
|
||||
outputSchema: evidenceResultOf(adbResultSchemas.start_adb_activity),
|
||||
|
||||
@@ -679,6 +679,8 @@ export const adbResultSchemas = {
|
||||
package_name: packageNameSchema,
|
||||
component: z.string().min(1),
|
||||
started_activity: z.string().nullable(),
|
||||
reported_activity: z.string().nullable().optional(),
|
||||
reported_status: z.string().nullable().optional(),
|
||||
}),
|
||||
start_adb_activity: z.strictObject({
|
||||
client,
|
||||
@@ -694,6 +696,8 @@ export const adbResultSchemas = {
|
||||
}),
|
||||
),
|
||||
started_activity: z.string().nullable(),
|
||||
reported_activity: z.string().nullable().optional(),
|
||||
reported_status: z.string().nullable().optional(),
|
||||
}),
|
||||
stop_adb_app: z.strictObject({
|
||||
client,
|
||||
@@ -702,6 +706,19 @@ export const adbResultSchemas = {
|
||||
}),
|
||||
} as const;
|
||||
|
||||
/** Launch-attempt facts retained when Activity Manager does not report success. */
|
||||
export type AdbActivityStartPartialObservation =
|
||||
| {
|
||||
readonly provider_id: "adb";
|
||||
readonly operation: "start_adb_app";
|
||||
readonly result: z.infer<typeof adbResultSchemas.start_adb_app>;
|
||||
}
|
||||
| {
|
||||
readonly provider_id: "adb";
|
||||
readonly operation: "start_adb_activity";
|
||||
readonly result: z.infer<typeof adbResultSchemas.start_adb_activity>;
|
||||
};
|
||||
|
||||
/** Acquired package facts retained when cancellation leaves caller-owned output. */
|
||||
export type AdbPackagePullPartialObservation = {
|
||||
readonly provider_id: "adb";
|
||||
|
||||
@@ -114,6 +114,7 @@ import type { Evidence } from "./evidence.js";
|
||||
import type { AndroidPartialObservation } from "./android/androidPartialObservation.js";
|
||||
import type { FirmwarePartialObservation } from "./firmware/firmwareAnalysis.js";
|
||||
import type {
|
||||
AdbActivityStartPartialObservation,
|
||||
AdbAcquiredFilePartialObservation,
|
||||
AdbPackagePullPartialObservation,
|
||||
} from "./adb/adbDeviceAnalysis.js";
|
||||
@@ -143,5 +144,6 @@ export type AnalysisPartialObservation =
|
||||
| AndroidPartialObservation
|
||||
| FirmwarePartialObservation
|
||||
| NativeUiObservationResult
|
||||
| AdbActivityStartPartialObservation
|
||||
| AdbAcquiredFilePartialObservation
|
||||
| AdbPackagePullPartialObservation;
|
||||
|
||||
@@ -0,0 +1,189 @@
|
||||
import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
|
||||
import { chmod, readFile, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { expect, it, onTestFinished } from "vitest";
|
||||
|
||||
import { AdbProvider } from "../../../src/adb/AdbProvider.js";
|
||||
import { analysisErrorProjectionSchema } from "../../../src/contracts/errorSchemas.js";
|
||||
import { toolContract } from "../../../src/contracts/toolContracts.js";
|
||||
import { parseEvidence } from "../../../src/domain/evidence.js";
|
||||
import { createServer } from "../../../src/server/createServer.js";
|
||||
import { createTestBinarySession } from "../../fixtures/binarySession.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
|
||||
const responseText = (response: { readonly content: readonly unknown[] }) =>
|
||||
response.content
|
||||
.flatMap((part) =>
|
||||
typeof part === "object" &&
|
||||
part !== null &&
|
||||
"text" in part &&
|
||||
typeof part.text === "string"
|
||||
? [part.text]
|
||||
: [],
|
||||
)
|
||||
.join("");
|
||||
|
||||
const createFakeAdb = async () => {
|
||||
const directory = await createTestTempDirectory("rea-adb-start-mcp-");
|
||||
const binary = join(directory, "adb");
|
||||
const stdoutFile = join(directory, "stdout");
|
||||
const stderrFile = join(directory, "stderr");
|
||||
const callsFile = join(directory, "calls");
|
||||
const fakeAdbSource = `#!/usr/bin/env node
|
||||
const fs = require("node:fs");
|
||||
const [stdoutPath, stderrPath, callsPath] = ${JSON.stringify([stdoutFile, stderrFile, callsFile])};
|
||||
const args = process.argv.slice(2);
|
||||
fs.appendFileSync(callsPath, args.join(" ") + "\\n");
|
||||
if (args[0] === "version") {
|
||||
process.stdout.write("Android Debug Bridge version 1.0.41\\nVersion 34.0.5-stub\\n");
|
||||
} else if (args.includes("resolve-activity")) {
|
||||
process.stdout.write("com.example/.Main\\n");
|
||||
} else {
|
||||
process.stdout.write(fs.readFileSync(stdoutPath, "utf8"));
|
||||
process.stderr.write(fs.readFileSync(stderrPath, "utf8"));
|
||||
}
|
||||
`;
|
||||
await writeFile(binary, fakeAdbSource);
|
||||
await chmod(binary, 0o755);
|
||||
return {
|
||||
binary,
|
||||
calls: async () => (await readFile(callsFile, "utf8")).trim().split("\n"),
|
||||
setReport: async (stdout: string, stderr: string) => {
|
||||
await writeFile(stdoutFile, stdout);
|
||||
await writeFile(stderrFile, stderr);
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
it
|
||||
.skipIf(process.platform === "win32")
|
||||
.each(["start_adb_app", "start_adb_activity"] as const)(
|
||||
"%s preserves successful reports and failed launch attempts through MCP (ADB process output simulated)",
|
||||
async (operation) => {
|
||||
const fakeAdb = await createFakeAdb();
|
||||
const { binary } = fakeAdb;
|
||||
const starting =
|
||||
"Starting: Intent { act=android.intent.action.MAIN cmp=com.example/.Main }\n";
|
||||
let stdout = `${starting}Warning: Activity not started, intent has been delivered to currently running top-most instance.\nStatus: ok\nActivity: com.example/.Main\nWaitTime: 315\nComplete\n`;
|
||||
let stderr = "";
|
||||
await fakeAdb.setReport(stdout, stderr);
|
||||
|
||||
const server = createServer(
|
||||
{
|
||||
kind: "session",
|
||||
session: createTestBinarySession(() => {
|
||||
throw new Error("ADB start must not open a deep-analysis provider");
|
||||
}),
|
||||
},
|
||||
{
|
||||
adbDeviceAnalysis: new AdbProvider({
|
||||
environment: { REA_ADB_PATH: binary },
|
||||
}),
|
||||
},
|
||||
);
|
||||
onTestFinished(() => server.close());
|
||||
const [clientTransport, serverTransport] =
|
||||
InMemoryTransport.createLinkedPair();
|
||||
await server.connect(serverTransport);
|
||||
const client = new Client({ name: "adb-start-boundary", version: "0.0.0" });
|
||||
onTestFinished(() => client.close());
|
||||
await client.connect(clientTransport);
|
||||
|
||||
const input =
|
||||
operation === "start_adb_app"
|
||||
? { serial: "emulator-5554", package: "com.example" }
|
||||
: {
|
||||
serial: "emulator-5554",
|
||||
action: "android.intent.action.MAIN",
|
||||
data_uri: "https://example.invalid/launch",
|
||||
component: "com.example/.Main",
|
||||
extras: [{ key: "mode", type: "string" as const, value: "test" }],
|
||||
};
|
||||
const success = await client.callTool({
|
||||
name: operation,
|
||||
arguments: input,
|
||||
});
|
||||
expect(success.isError).not.toBe(true);
|
||||
const evidence = toolContract(operation).outputSchema.parse(
|
||||
parseEvidence(JSON.parse(responseText(success))),
|
||||
);
|
||||
expect(evidence.confidence).toBe("observed");
|
||||
expect(evidence.normalized_result).toMatchObject({
|
||||
started_activity: "act=android.intent.action.MAIN cmp=com.example/.Main",
|
||||
reported_activity: "com.example/.Main",
|
||||
reported_status: "ok",
|
||||
});
|
||||
expect(evidence.raw_result).toEqual({ stdout, stderr });
|
||||
|
||||
stdout = `${starting}Status: timeout\nActivity: com.example/.Main\nWaitTime: 10004\nComplete\n`;
|
||||
await fakeAdb.setReport(stdout, stderr);
|
||||
const timeout = await client.callTool({
|
||||
name: operation,
|
||||
arguments: input,
|
||||
});
|
||||
expect(timeout.isError).toBe(true);
|
||||
const timeoutError = analysisErrorProjectionSchema.parse(
|
||||
JSON.parse(responseText(timeout)).error,
|
||||
);
|
||||
expect(timeoutError).toMatchObject({
|
||||
code: "unsupported_target",
|
||||
details: {
|
||||
path: "emulator-5554",
|
||||
reason: "am start reported status: timeout",
|
||||
captured_output: { stdout, stderr, truncated: false },
|
||||
partial_observation: {
|
||||
provider_id: "adb",
|
||||
operation,
|
||||
result: {
|
||||
started_activity:
|
||||
"act=android.intent.action.MAIN cmp=com.example/.Main",
|
||||
reported_activity: "com.example/.Main",
|
||||
reported_status: "timeout",
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(timeoutError).not.toHaveProperty("timeout_ms");
|
||||
|
||||
stdout = `${starting}Status: ok\nActivity: com.example/.Main\nComplete\n`;
|
||||
stderr = "Exception: Activity class does not exist\n";
|
||||
await fakeAdb.setReport(stdout, stderr);
|
||||
const hardError = await client.callTool({
|
||||
name: operation,
|
||||
arguments: input,
|
||||
});
|
||||
expect(hardError.isError).toBe(true);
|
||||
const hardErrorProjection = analysisErrorProjectionSchema.parse(
|
||||
JSON.parse(responseText(hardError)).error,
|
||||
);
|
||||
expect(hardErrorProjection.message).toContain(
|
||||
"Activity class does not exist",
|
||||
);
|
||||
expect(hardErrorProjection.details).toMatchObject({
|
||||
captured_output: { stdout, stderr, truncated: false },
|
||||
partial_observation: {
|
||||
operation,
|
||||
result: { reported_status: "ok" },
|
||||
},
|
||||
});
|
||||
|
||||
const calls = await fakeAdb.calls();
|
||||
expect(
|
||||
calls.filter((arguments_) => arguments_.includes("start -W")),
|
||||
).toEqual(
|
||||
Array.from({ length: 3 }, () =>
|
||||
[
|
||||
"-s emulator-5554 shell am start -W",
|
||||
...(operation === "start_adb_app"
|
||||
? ["-n com.example/.Main"]
|
||||
: [
|
||||
"-a android.intent.action.MAIN",
|
||||
"-d https://example.invalid/launch",
|
||||
"-n com.example/.Main",
|
||||
"--es mode test",
|
||||
]),
|
||||
].join(" "),
|
||||
),
|
||||
);
|
||||
},
|
||||
);
|
||||
Reference in New Issue
Block a user