fix(contracts): mark foreign execution and device mutation destructive (#1904)

MCP annotations derived destructiveHint only from mayDiscardData, so tools
that run the analysis target or a caller-selected executable
(capture_process_scenario, observe_native_calls) and tools that change an
attached Android device (install/uninstall/push/start/stop) advertised
destructiveHint: false.

Add explicit executesForeignCode and mutatesDevice effect flags, audit them
across all 170 tool effect records, and derive
destructiveHint = mayDiscardData || executesForeignCode || mutatesDevice.
readOnlyHint semantics are unchanged.

Hopper/Ghidra metadata writers (set_address_name, set_addresses_names,
set_bookmark, set_comment, set_inline_comment, annotate_native_function)
can replace or clear existing analyst names and comments, so they now
declare mayDiscardData as well.

A contract test pins the destructive set and requires every tool whose
description mentions running, launching, executing, starting, installing or
uninstalling to declare one of the new flags or carry a reasoned exception,
so wording and effects cannot silently drift. The binary_session effects
output schema carries the new flags.

docs/mcp-contracts.md

@@ -450,7 +450,13 @@ Tool annotations describe effects and are hints, not authorization controls
 ([MCP ToolAnnotations](https://modelcontextprotocol.io/specification/2025-06-18/schema#toolannotations)).
 `readOnlyHint` includes session state: an analysis call that records additive
 Evidence is marked non-read-only even when it leaves the target unchanged.
-`destructiveHint` describes possible data loss, not ordinary Evidence recording.
+`destructiveHint` is true when a tool may discard data (overwriting or clearing
+analyst metadata, replacing files, dropping session state), runs or drives code
+REA does not own (`executesForeignCode`: process, LLDB, Electron, browser, and
+native UI scenarios), or changes an attached Android device (`mutatesDevice`:
+push, install, uninstall, start, and stop). Ordinary Evidence recording is not
+destructive. The effect record in `binary_session` and the generated catalog
+carries each flag separately.
 Effect metadata covers possible behavior across supported inputs: DMG inventory
 can launch `hdiutil` and create an owned temporary mount directory; extraction
 creates a fresh output directory on every call. Comparing supplied web captures

src/contracts/toolEffects.test.ts

@@ -0,0 +1,156 @@
+import { describe, expect, it } from "vitest";
+
+import { TOOL_CONTRACTS } from "./toolContracts.js";
+import { TOOL_EFFECTS, annotationsFromEffects } from "./toolEffects.js";
+
+/**
+ * Descriptions that name running, launching, executing, starting, installing,
+ * or uninstalling something. A matching tool must declare foreign execution or
+ * device mutation unless it is listed below with the reason the wording does
+ * not describe that effect.
+ */
+const EXECUTION_WORDING =
+  /\b(?:run|runs|running|launch|launches|launching|execute|executes|executing|execution|start|starts|install|installs|uninstall|uninstalls)\b/iu;
+
+const EXECUTION_WORDING_EXCEPTIONS: Readonly<Record<string, string>> = {
+  inspect_native_load_image: "states that it does not execute the target",
+  resolve_native_call_targets: "states that it does not execute code",
+  inspect_pe_resources: "states that it does not execute the image",
+  inspect_binary_layout:
+    "parses the binary offline with operator-configured pwntools",
+  inspect_recorded_crash: "inspects a core file without launching its target",
+  inspect_macho: "runs host Mach-O inspection tools, not the target",
+  inspect_evm_interface:
+    "decodes bytecode in REA's bundled worker without executing it",
+  inspect_managed_artifact: "reads metadata without executing target code",
+  inspect_managed_members: "reads metadata without executing target code",
+  inspect_managed_native_boundaries:
+    "reads metadata without executing target code",
+  verify_managed_native_boundaries: "states it does not execute managed code",
+  project_managed_application_graph: "states it does not execute managed code",
+  inspect_android_package: "states it does not execute the APK",
+  search_android_classes: "states it does not execute the APK",
+  inspect_apktool_client:
+    "inspects the operator-selected apktool launcher; never installs it",
+  inspect_jeb_client: "talks to a caller-started JEB; never launches it",
+  open_jeb_project: "the engine analyzes the file; REA does not execute it",
+  inspect_adb_client:
+    "inspects the operator-selected adb client; never starts its server",
+  list_adb_devices: "may start the local adb server, which REA does not own",
+  inspect_firmware_regions: "states there is no target execution",
+  extract_firmware: "states that extracted code is never executed",
+  analyze_javascript_application: "reconstructs the graph without executing",
+  reconcile_javascript_runtime: "never invents execution claims",
+  observe_javascript_runtime: "never evaluates or resumes the target",
+  trace_application_feature: "never executes the application",
+  trace_javascript_semantics: "static reachability never claims execution",
+  compare_javascript_export_shapes: "states it does not execute JavaScript",
+  project_android_application_graph: "execution-free inventory",
+  project_apple_application_graph: "execution-free inventory",
+  trace_web_source_location: "states there is no application execution",
+  trace_web_module_imports:
+    "runs only REA's resolver stub in operator-configured Chromium",
+  export_web_scripts: "states there is no refetch or execution",
+  observe_web_execution:
+    "instruments an externally driven page; it does not run code itself",
+  recover_javascript_sources: "states there is no application code execution",
+  goto_address: "start names an object boundary, not a launch",
+  prev_address: "start names an object boundary, not a launch",
+  trace_call_path: "start names the traversal origin",
+  observe_native_ui: "observes an already-running app without launching it",
+  inspect_adb_package: "reports install times without changing the device",
+  list_adb_processes: "lists running processes without changing the device",
+  inspect_web_event_listeners: "states it does not evaluate JavaScript",
+};
+
+const declaresForeignAuthority = (name: string): boolean => {
+  const effects = TOOL_EFFECTS[name];
+  return (
+    effects !== undefined &&
+    (effects.executesForeignCode || effects.mutatesDevice)
+  );
+};
+
+describe("tool effects", () => {
+  it("audits every public tool exactly once", () => {
+    expect(Object.keys(TOOL_EFFECTS).sort()).toEqual(
+      TOOL_CONTRACTS.map(({ name }) => name).sort(),
+    );
+  });
+
+  it("keeps execution and device wording aligned with declared effects", () => {
+    const undeclared = TOOL_CONTRACTS.filter(
+      ({ name, description }) =>
+        EXECUTION_WORDING.test(description) &&
+        !declaresForeignAuthority(name) &&
+        EXECUTION_WORDING_EXCEPTIONS[name] === undefined,
+    ).map(({ name }) => name);
+    expect(undeclared).toEqual([]);
+    const staleExceptions = Object.keys(EXECUTION_WORDING_EXCEPTIONS).filter(
+      (name) => {
+        const contract = TOOL_CONTRACTS.find(
+          (candidate) => candidate.name === name,
+        );
+        return (
+          contract === undefined ||
+          !EXECUTION_WORDING.test(contract.description) ||
+          declaresForeignAuthority(name)
+        );
+      },
+    );
+    expect(staleExceptions).toEqual([]);
+  });
+
+  it("marks foreign execution and device mutation as destructive", () => {
+    const destructive = TOOL_CONTRACTS.filter(
+      ({ effects }) => effects.executesForeignCode || effects.mutatesDevice,
+    );
+    expect(destructive.map(({ name }) => name).sort()).toEqual([
+      "capture_browser_scenario",
+      "capture_electron_scenario",
+      "capture_native_ui_scenario",
+      "capture_process_scenario",
+      "install_adb_package",
+      "observe_native_calls",
+      "push_adb_file",
+      "start_adb_activity",
+      "start_adb_app",
+      "stop_adb_app",
+      "uninstall_adb_package",
+    ]);
+    for (const contract of destructive)
+      expect(contract.annotations, contract.name).toMatchObject({
+        readOnlyHint: false,
+        destructiveHint: true,
+      });
+  });
+
+  it("derives destructiveHint from each destructive effect alone", () => {
+    const base = TOOL_EFFECTS["binary_session"];
+    if (base === undefined) throw new Error("binary_session is unaudited");
+    expect(annotationsFromEffects(base)).toEqual({
+      readOnlyHint: true,
+      destructiveHint: false,
+      idempotentHint: true,
+      openWorldHint: false,
+    });
+    for (const flag of [
+      "mayDiscardData",
+      "executesForeignCode",
+      "mutatesDevice",
+    ] as const)
+      expect(annotationsFromEffects({ ...base, [flag]: true }), flag).toEqual({
+        readOnlyHint: false,
+        destructiveHint: true,
+        idempotentHint: true,
+        openWorldHint: false,
+      });
+  });
+
+  it("publishes the advertised annotations derived from canonical effects", () => {
+    for (const contract of TOOL_CONTRACTS)
+      expect(contract.annotations, contract.name).toEqual(
+        annotationsFromEffects(contract.effects),
+      );
+  });
+});

src/contracts/toolEffects.ts

@@ -7,7 +7,17 @@ export interface ToolEffects {
   readonly launchesProcess: boolean;
   readonly accessesNetwork: boolean;
   readonly changesUiState: boolean;
+  /** Can overwrite, clear, or drop state that the caller cannot recover from REA. */
   readonly mayDiscardData: boolean;
+  /**
+   * Runs or drives code REA does not own on the host: the analysis target, a
+   * caller-selected executable, or the application loaded into one. Running
+   * REA's own pinned helpers or operator-configured analysis engines does not
+   * count.
+   */
+  readonly executesForeignCode: boolean;
+  /** Changes an attached Android device: packages, files, or running apps. */
+  readonly mutatesDevice: boolean;
   readonly idempotent: boolean;
 }
 
@@ -27,6 +37,8 @@ const effects = (overrides: Partial<ToolEffects> = {}): ToolEffects => ({
   accessesNetwork: false,
   changesUiState: false,
   mayDiscardData: false,
+  executesForeignCode: false,
+  mutatesDevice: false,
   idempotent: true,
   ...overrides,
 });
@@ -178,6 +190,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     launchesProcess: true,
     writesFilesystem: true,
     accessesNetwork: true,
+    mutatesDevice: true,
     idempotent: false,
   }),
   capture_adb_screen: effects({
@@ -239,6 +252,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     mutatesSession: true,
     launchesProcess: true,
     accessesNetwork: true,
+    mutatesDevice: true,
     idempotent: false,
   }),
   uninstall_adb_package: effects({
@@ -246,6 +260,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     mutatesSession: true,
     launchesProcess: true,
     accessesNetwork: true,
+    mutatesDevice: true,
     idempotent: false,
   }),
   start_adb_app: effects({
@@ -253,6 +268,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     mutatesSession: true,
     launchesProcess: true,
     accessesNetwork: true,
+    mutatesDevice: true,
     idempotent: false,
   }),
   start_adb_activity: effects({
@@ -260,6 +276,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     mutatesSession: true,
     launchesProcess: true,
     accessesNetwork: true,
+    mutatesDevice: true,
     idempotent: false,
   }),
   stop_adb_app: effects({
@@ -267,6 +284,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     mutatesSession: true,
     launchesProcess: true,
     accessesNetwork: true,
+    mutatesDevice: true,
     idempotent: false,
   }),
   search_android_classes: effects({
@@ -294,6 +312,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     mutatesSession: true,
     launchesProcess: true,
     writesFilesystem: true,
+    mayDiscardData: true,
   }),
   inspect_native_load_image: evidence,
   inspect_native_dispatch_metadata: evidence,
@@ -330,11 +349,31 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
   resolve_containing_procedure: evidence,
   search_procedures: evidence,
   search_strings: evidence,
-  set_address_name: effects({ mutatesTarget: true, mutatesSession: true }),
-  set_addresses_names: effects({ mutatesTarget: true, mutatesSession: true }),
-  set_bookmark: effects({ mutatesTarget: true, mutatesSession: true }),
-  set_comment: effects({ mutatesTarget: true, mutatesSession: true }),
-  set_inline_comment: effects({ mutatesTarget: true, mutatesSession: true }),
+  set_address_name: effects({
+    mutatesTarget: true,
+    mutatesSession: true,
+    mayDiscardData: true,
+  }),
+  set_addresses_names: effects({
+    mutatesTarget: true,
+    mutatesSession: true,
+    mayDiscardData: true,
+  }),
+  set_bookmark: effects({
+    mutatesTarget: true,
+    mutatesSession: true,
+    mayDiscardData: true,
+  }),
+  set_comment: effects({
+    mutatesTarget: true,
+    mutatesSession: true,
+    mayDiscardData: true,
+  }),
+  set_inline_comment: effects({
+    mutatesTarget: true,
+    mutatesSession: true,
+    mayDiscardData: true,
+  }),
   unset_bookmark: effects({
     mutatesTarget: true,
     mutatesSession: true,
@@ -365,6 +404,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     accessesNetwork: true,
     changesUiState: true,
     mayDiscardData: true,
+    executesForeignCode: true,
     idempotent: false,
   }),
   // The owned process can change files, show UI and use the network.
@@ -374,6 +414,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     launchesProcess: true,
     accessesNetwork: true,
     changesUiState: true,
+    executesForeignCode: true,
     idempotent: false,
   }),
   inspect_macho: nativeEvidence,
@@ -419,6 +460,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     accessesNetwork: true,
     changesUiState: true,
     mayDiscardData: true,
+    executesForeignCode: true,
     idempotent: false,
   }),
   list_electron_targets: browserEvidence,
@@ -431,6 +473,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     accessesNetwork: true,
     changesUiState: true,
     mayDiscardData: true,
+    executesForeignCode: true,
     idempotent: false,
   }),
   list_javascript_runtime_targets: browserEvidence,
@@ -479,6 +522,7 @@ export const TOOL_EFFECTS: Readonly<Record<string, ToolEffects>> = {
     writesFilesystem: true,
     launchesProcess: true,
     accessesNetwork: true,
+    executesForeignCode: true,
     idempotent: false,
   }),
   compare_process_captures: sessionEvidence,
@@ -506,9 +550,15 @@ export const annotationsFromEffects = (
     value.launchesProcess ||
     value.accessesNetwork ||
     value.changesUiState ||
-    value.mayDiscardData
+    value.mayDiscardData ||
+    value.executesForeignCode ||
+    value.mutatesDevice
   ),
-  destructiveHint: value.mayDiscardData,
+  // MCP defines destructive as possibly non-additive changes to the
+  // environment. Running foreign code or changing a device cannot be bounded
+  // to additive effects, so either one is destructive even without data loss.
+  destructiveHint:
+    value.mayDiscardData || value.executesForeignCode || value.mutatesDevice,
   idempotentHint: value.idempotent,
   openWorldHint: value.launchesProcess || value.accessesNetwork,
 });

src/contracts/toolOutputSchemaPrimitives.ts

@@ -293,6 +293,8 @@ const toolAvailabilityFacts = {
     accessesNetwork: z.boolean(),
     changesUiState: z.boolean(),
     mayDiscardData: z.boolean(),
+    executesForeignCode: z.boolean(),
+    mutatesDevice: z.boolean(),
     idempotent: z.boolean(),
   }),
   annotations: z.object({

tests/boundary/mcp/toolResult.test.ts

@@ -50,6 +50,8 @@ const contract: ToolContract = {
     accessesNetwork: false,
     changesUiState: false,
     mayDiscardData: false,
+    executesForeignCode: false,
+    mutatesDevice: false,
     idempotent: true,
   },
   annotations: {

tests/boundary/mcp/toolSchemaValidity.test.ts

@@ -73,7 +73,7 @@ function expectKnownAuthorityHints(tools: readonly ToolSchemas[]): void {
     },
     annotate_native_function: {
       readOnlyHint: false,
-      destructiveHint: false,
+      destructiveHint: true,
       idempotentHint: true,
       openWorldHint: true,
     },
@@ -102,8 +102,22 @@ function expectKnownAuthorityHints(tools: readonly ToolSchemas[]): void {
       openWorldHint: false,
     },
     capture_process_scenario: {
+      readOnlyHint: false,
+      destructiveHint: true,
       openWorldHint: true,
     },
+    observe_native_calls: {
+      readOnlyHint: false,
+      destructiveHint: true,
+    },
+    uninstall_adb_package: {
+      readOnlyHint: false,
+      destructiveHint: true,
+    },
+    start_adb_app: {
+      readOnlyHint: false,
+      destructiveHint: true,
+    },
     export_evidence_bundle: {
       readOnlyHint: false,
       destructiveHint: true,