fix(cli): validate plist and dylib-root options with their contract fields (#10557)

* fix(cli): validate plist and dylib-root options with their contract fields

inspect-plist --relative-path and trace-dylib-resolution --root were
declared as bare strings on the CLI. --schema under-advertised them, and an
empty or NUL value reached the operation contract, whose refusal named the
contract field (path, roots) instead of the CLI option.

Declare both options with the contract's own field schemas
(localPathStringSchema; dylibResolutionInputSchema.shape.roots), so the CLI
refuses them first and names --relative-path or --root.

Fixes #9764
Fixes #9823

* fix(mcp): observe SDK initiated cleanup failures

src/android/JadxSession.ts

@@ -1,5 +1,5 @@
 import { throwIfJadxInterrupted } from "./JadxInterruption.js";
-import { Client } from "@modelcontextprotocol/client";
+import { OwnedMcpClient } from "../process/OwnedMcpClient.js";
 import { createAnalysisExecution } from "../application/AnalysisProvider.js";
 import type { AndroidAnalysisExecution } from "../application/android/AndroidAnalysisPort.js";
 import type { AndroidRequest } from "../domain/android/androidAnalysis.js";
@@ -64,7 +64,10 @@ type JadxCallObservation = {
 /** One owned engine connection; its wire protocol stays inside the Android adapter. */
 export class JadxSession {
   readonly transport: JadxMcpTransport;
-  readonly #client = new Client({ name: "rea-android-adapter", version: "1" });
+  readonly #client = new OwnedMcpClient({
+    name: "rea-android-adapter",
+    version: "1",
+  });
   #loaded: ReturnType<typeof jadxLoadSchema.parse> | undefined;
   #connected = false;
   #partialContext:

src/cli/artifactCommands.ts

@@ -13,6 +13,7 @@ import { withCommandCancellation } from "./commandCancellation.js";
 import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
 import { ARTIFACT_GRAPH_PROVIDER } from "../domain/investigationProviders.js";
 import { keyedArchiveInputSchema } from "../domain/apple/keyedArchive.js";
+import { dylibResolutionInputSchema } from "../domain/apple/dylibResolution.js";
 import { localPathStringSchema } from "../domain/localPath.js";
 import { z } from "incur";
 import { cliDecimalField } from "./options.js";
@@ -109,12 +110,9 @@ const registerDylibResolutionCommand = (
       ),
     }),
     options: z.object({
-      root: z
-        .array(z.string())
-        .optional()
-        .describe(
-          "Process root relative to the bundle or the Mach-O's directory (repeatable)",
-        ),
+      root: dylibResolutionInputSchema.shape.roots.describe(
+        "Process root relative to the bundle or the Mach-O's directory (repeatable)",
+      ),
       architecture: z
         .enum(["arm64", "arm64e", "x86_64"])
         .optional()

src/cli/utilityCommands.ts

@@ -229,8 +229,7 @@ const registerNativeCommands = (
       path: localPathStringSchema.describe("App or Mach-O path"),
     }),
     options: z.object({
-      relativePath: z
-        .string()
+      relativePath: localPathStringSchema
         .optional()
         .describe(
           "Plist path relative to the app root, or an absolute path (default: the bundle's Info.plist)",

src/mcpDoctor.ts

@@ -1,4 +1,5 @@
 import { Client, SdkError, SdkErrorCode } from "@modelcontextprotocol/client";
+import { OwnedMcpClient } from "./process/OwnedMcpClient.js";
 import { Help } from "incur";
 import { performance } from "node:perf_hooks";
 import { resolve } from "node:path";
@@ -135,7 +136,7 @@ export const runProductionMcpDoctor = async (options: McpDoctorOptions) => {
     },
     stderr.append,
   );
-  const client = new Client({ name: "rea-mcp-doctor", version: "1" });
+  const client = new OwnedMcpClient({ name: "rea-mcp-doctor", version: "1" });
   let result: ProductionMcpDoctorResult;
   let cleanupFailure: { readonly cause: unknown } | undefined;
   try {

src/process/OwnedMcpClient.ts

@@ -0,0 +1,12 @@
+import { Client } from "@modelcontextprotocol/client";
+
+/** Observe SDK-initiated close failures while keeping them available to callers. */
+export class OwnedMcpClient extends Client {
+  override close(): Promise<void> {
+    const closing = super.close();
+    // The SDK starts close without awaiting it after a legacy handshake fails.
+    // The transport owner still receives the original failure when it joins cleanup.
+    void closing.catch(() => undefined);
+    return closing;
+  }
+}

tests/boundary/cli/dylibResolutionCli.test.ts

@@ -66,6 +66,33 @@ const expectRootInputError = async (
   });
 };
 
+/** The CLI option shares the contract's root format, so it refuses an escaping root itself and names the option. */
+const expectEscapingRootRefusedByCli = async (
+  cli: TestCli,
+  app: string,
+): Promise<void> => {
+  const escaping = await cli.run({
+    arguments: [
+      "trace-dylib-resolution",
+      app,
+      "--root",
+      "../outside",
+      "--json",
+    ],
+    environment: ENVIRONMENT,
+  });
+  expect(escaping.exitCode).toBe(1);
+  expect(escaping.json).toMatchObject({
+    code: "VALIDATION_ERROR",
+    fieldErrors: [
+      expect.objectContaining({
+        path: expect.stringMatching(/^root/u),
+        message: expect.stringContaining("Use a normalized path"),
+      }),
+    ],
+  });
+};
+
 const fixtureApp = async (): Promise<string> => {
   const app = join(
     await createTestTempDirectory("rea-dylib-cli-"),
@@ -534,8 +561,8 @@ describe("trace-dylib-resolution CLI", () => {
       const outside = await createTestTempDirectory("rea-dylib-outside-");
       await writeFiles(outside, { Tool: machoImage({}) });
       await symlink(outside, join(app, "Contents/Linked"), "dir");
+      await expectEscapingRootRefusedByCli(cli, app);
       for (const [root, reason, message] of [
-        ["../outside", "invalid_format", "Use a normalized path"],
         ["Contents/Info.plist", "invalid_value", "is not a Mach-O image"],
         [
           "Contents/Linked/Tool",

tests/boundary/cli/emptyLocalTargetAdmission.test.ts

@@ -1,4 +1,5 @@
 import { Cli } from "incur";
+import { Ajv2020 } from "ajv/dist/2020.js";
 import { expect, it, vi } from "vitest";
 import { registerArtifactCommands } from "../../../src/cli/artifactCommands.js";
 import { registerCoreBinaryCommands } from "../../../src/cli/coreBinaryCommands.js";
@@ -132,6 +133,32 @@ it.each([
   },
 );
 
+it.each([
+  {
+    command: "inspect-plist",
+    field: "relativePath",
+    valid: "Contents/Info.plist",
+    invalid: ["", "Contents/a\x00b"],
+  },
+  {
+    command: "trace-dylib-resolution",
+    field: "root",
+    valid: ["Contents/MacOS/app"],
+    invalid: [[""], ["Contents/a\x00b"], ["../outside"]],
+  },
+])(
+  "advertises the $field option constraints",
+  async ({ command, field, valid, invalid }) => {
+    const { serve } = registry();
+    const schema = (await serve([command, "--schema"])) as { options: object };
+    const validate = new Ajv2020({ strict: false }).compile(schema.options);
+    expect(validate({})).toBe(true);
+    expect(validate({ [field]: valid })).toBe(true);
+    for (const value of invalid)
+      expect(validate({ [field]: value })).toBe(false);
+  },
+);
+
 it("rejects an empty or NUL --snapshot option before analysis", async () => {
   const { serve, runs } = registry();
   expect(await serve(["inspect", ".", "--snapshot", ""])).toMatchObject({
@@ -150,3 +177,24 @@ it("rejects an empty or NUL --snapshot option before analysis", async () => {
   });
   for (const run of runs) expect(run).not.toHaveBeenCalled();
 });
+
+it.each([
+  { argv: ["inspect-plist", ".", "--relative-path"], field: "relativePath" },
+  { argv: ["trace-dylib-resolution", ".", "--root"], field: "root" },
+])(
+  "rejects an empty or NUL $field option on the CLI and names that option",
+  async ({ argv, field }) => {
+    const { serve, runs } = registry();
+    for (const value of ["", "Contents/a\x00b"]) {
+      const result = (await serve([...argv, value])) as {
+        code: string;
+        fieldErrors: readonly { path: string }[];
+      };
+      expect(result.code).toBe("VALIDATION_ERROR");
+      expect(
+        new Set(result.fieldErrors.map(({ path }) => path.split(".")[0])),
+      ).toEqual(new Set([field]));
+    }
+    for (const run of runs) expect(run).not.toHaveBeenCalled();
+  },
+);