fix(native): preserve plist admission through finalization

docs/native-investigation.md

@@ -474,15 +474,16 @@ estimated 64 MiB decode budget before parsing; this is not a hard process-memory
 bound. JSON decode memory has not been measured and has no equivalent decode
 estimate. A refused input reports non-retryable `resource_constraint` with
 `reported_limits.maximum_input_bytes` or `maximum_decoded_bytes` as applicable.
-Ordinary direct-source inspection admits 998 source nesting levels, derived from
-the existing 1000-level JSON result capacity by reserving one level for typed
-scalar fields and one for the result's `value` field. Deeper input reports
+Ordinary direct-source decoding has a ceiling of 998 source nesting levels,
+derived from the existing 1000-level JSON result capacity by reserving one level
+for typed scalar fields and one for the result's `value` field. Deeper input reports
 `resource_constraint` with `reported_limits.maximum_nesting_depth: 998` and names
-the fixed nesting allowance. Compatibility conversions can encounter an earlier
-native utility refusal; their diagnostics remain explicit. Archive and Interface
-Builder decoding retain their separate 128-level allowance. Worker limits and
-host memory do not raise these fixed allowances, and the unchanged request fails
-the same way.
+the fixed nesting allowance. XML admission also requires native validation, which
+can refuse input below this decoding ceiling. Compatibility conversions can
+likewise encounter an earlier native utility refusal; both retain native
+diagnostics. Archive and Interface Builder decoding retain their separate
+128-level allowance. Worker limits and host memory do not raise these fixed
+allowances, and the unchanged request fails the same way.
 
 A plist explicitly selected through the path input or as the active plist target
 reports an unreadable or malformed selection as `invalid_request`. A missing or

src/contracts/native/nativeToolContracts.ts

@@ -115,7 +115,7 @@ export const NATIVE_TOOL_CONTRACTS = [
   ),
   native(
     "inspect_plist",
-    "Requires an open binary or bundle-resource session; use open_binary or open_bundle_resources first. Parse its Info.plist by default, or select a local plist with path within that session. Supplying path does not open a session. Returns normalized JSON rather than plutil text. native_command_timeout_ms is each file or plutil command's wall clock (omitted means 60000 ms). The 67108864-byte plist input and decode allowances and the 128-level nesting allowance have no runtime override. The 4096-byte markup probe is a probe window, not a document cap, and has no runtime override.",
+    "Requires an open binary or bundle-resource session; use open_binary or open_bundle_resources first. Parse its Info.plist by default, or select a local plist with path within that session. Supplying path does not open a session. Returns normalized JSON rather than plutil text. native_command_timeout_ms is each file or plutil command's wall clock (omitted means 60000 ms). The 67108864-byte plist input and decode allowances and the 998-level nesting allowance have no runtime override. XML sources also require native plutil validation, which can refuse shallower inputs with its own diagnostics. The 4096-byte markup probe is a probe window, not a document cap, and has no runtime override.",
     z.object({
       path: localPathStringSchema
         .optional()

src/native/NativeMacOSProvider.ts

@@ -1003,7 +1003,7 @@ class NativeMacOSClient implements AnalysisClient {
           };
           const selected = await requireRegularPlist(fileInput);
           if (!selected.ok) return selected;
-          const classified = await this.#run(
+          const classified = await this.#runAdmitted(
             "inspect_plist",
             "file",
             ["-b", plist.value.path],
@@ -1096,7 +1096,7 @@ class NativeMacOSClient implements AnalysisClient {
     if (format === "xml") {
       parsed = parsePlistXmlSource(source.value);
       if (parsed.ok) {
-        const validated = await this.#run(
+        const validated = await this.#runAdmitted(
           "inspect_plist",
           "plutil",
           ["-lint", "--", fileInput.selection.path],
@@ -1160,7 +1160,7 @@ class NativeMacOSClient implements AnalysisClient {
   ): Promise<Result<NativeObservation, AnalysisError>> {
     const { path } = fileInput.selection;
     const { signal, timeoutMs } = fileInput;
-    const json = await this.#run(
+    const json = await this.#runAdmitted(
       "inspect_plist",
       "plutil",
       ["-convert", "json", "-o", "-", "--", path],
@@ -1180,7 +1180,7 @@ class NativeMacOSClient implements AnalysisClient {
       AnalysisOutputError | AnalysisResourceConstraintError
     >;
     if (jsonAttempt === undefined || jsonAttempt.needsNumberTypes) {
-      const converted = await this.#run(
+      const converted = await this.#runAdmitted(
         "inspect_plist",
         "plutil",
         ["-convert", "xml1", "-o", "-", "--", path],
@@ -1244,14 +1244,7 @@ class NativeMacOSClient implements AnalysisClient {
     operation: NativeToolName,
     tool: string,
     arguments_: readonly string[],
-    options: {
-      readonly signal?: AbortSignal | undefined;
-      readonly timeoutMs: number;
-      readonly acceptNonZero?: boolean;
-      readonly acceptNonZeroIf?:
-        | ((capture: NativeCommandCapture) => boolean)
-        | undefined;
-    },
+    options: NativeClientCommandOptions,
   ): Promise<Result<NativeCommandCapture, AnalysisError>> {
     if (this.#closed)
       return err(
@@ -1261,6 +1254,16 @@ class NativeMacOSClient implements AnalysisClient {
           "Native provider client is closed",
         ),
       );
+    return this.#runAdmitted(operation, tool, arguments_, options);
+  }
+
+  /** Child commands borrow the admitted file workflow's lifetime through close. */
+  async #runAdmitted(
+    operation: NativeToolName,
+    tool: string,
+    arguments_: readonly string[],
+    options: NativeClientCommandOptions,
+  ): Promise<Result<NativeCommandCapture, AnalysisError>> {
     const command = this.runner.run(tool, arguments_, {
       ...(options.signal === undefined ? {} : { signal: options.signal }),
       timeoutMs: options.timeoutMs,
@@ -1375,6 +1378,15 @@ const nativeOutputFailure = (
 ): AnalysisError =>
   new AnalysisOutputError(operation, "Native output parsing failed", { cause });
 
+interface NativeClientCommandOptions {
+  readonly signal?: AbortSignal | undefined;
+  readonly timeoutMs: number;
+  readonly acceptNonZero?: boolean;
+  readonly acceptNonZeroIf?:
+    | ((capture: NativeCommandCapture) => boolean)
+    | undefined;
+}
+
 interface NativeObservation {
   readonly result: JsonValue;
   readonly limitations: readonly string[];

tests/acceptance/applications/nativePlistNesting.test.ts

@@ -1,3 +1,4 @@
+import { execFile } from "node:child_process";
 import { readFile, writeFile } from "node:fs/promises";
 import { join } from "node:path";
 import { buildBinary, type PlistValue } from "plist";
@@ -11,6 +12,25 @@ import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
 import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
 import { cliTest } from "../../support/cli/cliFixture.js";
 
+const lintXmlPlist = (path: string) =>
+  new Promise<{ exitCode: number; stdout: string; stderr: string }>(
+    (resolve, reject) => {
+      execFile(
+        "/usr/bin/plutil",
+        ["-lint", "--", path],
+        { timeout: 10_000 },
+        (error, stdout, stderr) => {
+          const exitCode = error === null ? 0 : error.code;
+          if (typeof exitCode !== "number") {
+            reject(error);
+            return;
+          }
+          resolve({ exitCode, stdout, stderr });
+        },
+      );
+    },
+  );
+
 describe.skipIf(process.platform !== "darwin")(
   "ordinary plist source nesting",
   () => {
@@ -34,6 +54,7 @@ describe.skipIf(process.platform !== "darwin")(
           { format: "binary", depth: 129, data: true },
           { format: "xml", depth: 129, data: true },
           { format: "binary", depth: 200, data: false },
+          { format: "xml", depth: 200, data: false },
           { format: "json", depth: 200, data: false },
           { format: "binary", depth: 998, data: true },
           { format: "binary", depth: 999, data: true },
@@ -53,9 +74,16 @@ describe.skipIf(process.platform !== "darwin")(
               : Buffer.from(
                   format === "json"
                     ? JSON.stringify(value)
-                    : `<plist>${"<dict><key>level</key>".repeat(depth)}<data>AAE=</data>${"</dict>".repeat(depth)}</plist>`,
+                    : `<plist>${"<dict><key>level</key>".repeat(depth)}${data ? "<data>AAE=</data>" : "<integer>1</integer>"}${"</dict>".repeat(depth)}</plist>`,
                 );
           await writeFile(path, bytes);
+          // The decoder ceiling does not override native XML admission.
+          // Compare its upper-edge admission with the actual utility, while
+          // requiring the historical depth-129/200 controls to succeed below.
+          const native =
+            format === "xml" && depth === 998
+              ? await lintXmlPlist(path)
+              : undefined;
           const output = await cli.run({
             arguments: ["inspect-plist", path, "--json"],
             environment,
@@ -83,6 +111,26 @@ describe.skipIf(process.platform !== "darwin")(
             );
             continue;
           }
+          if (native !== undefined && native.exitCode !== 0) {
+            expect(output.exitCode, output.stderr).toBe(1);
+            const expectedFailure = {
+              code: "invalid_request",
+              retryable: false,
+              details: {
+                operation: "inspect_plist",
+                captured_output: {
+                  exit_code: native.exitCode,
+                  stdout: native.stdout,
+                  stderr: native.stderr,
+                },
+              },
+            };
+            expect(output.json).toMatchObject(expectedFailure);
+            expect(parseMcpToolError(response).error).toMatchObject(
+              expectedFailure,
+            );
+            continue;
+          }
           expect(
             output.exitCode,
             JSON.stringify({
@@ -105,6 +153,7 @@ describe.skipIf(process.platform !== "darwin")(
           expect(cliResult.format).toBe(format === "json" ? "unknown" : format);
           expect(cliResult.provenance.map((item) => item.tool)).toEqual([
             "file",
+            ...(format === "xml" ? ["plutil"] : []),
           ]);
           let leaf: unknown = cliResult.value;
           for (let level = 0; level < depth; level += 1) {

tests/boundary/providers/native/nativePlistFileCleanup.test.ts

@@ -332,41 +332,70 @@ it("preserves an opaque cancellation observed during successful prefix close", a
   expect((await client.close()).ok).toBe(true);
 });
 
-it("joins an admitted prefix read when client close starts concurrently", async () => {
-  const { client, path, prototype } = await fixture();
-  let enter!: () => void;
-  let resume!: () => void;
-  const entered = new Promise<void>((resolve) => {
-    enter = resolve;
-  });
-  const gate = new Promise<void>((resolve) => {
-    resume = resolve;
-  });
-  const original = prototype.read;
-  const read = vi
-    .spyOn(prototype, "read")
-    .mockImplementationOnce(async function (
-      this: FileHandle,
-      ...args: Parameters<FileHandle["read"]>
-    ) {
-      enter();
-      await gate;
-      return Reflect.apply(original, this, args);
+it.for(["source", "json", "xml"] as const)(
+  "joins an admitted prefix read and its %s commands when client close starts concurrently",
+  async (route) => {
+    const { client, path, prototype, runner } = await fixture();
+    if (route !== "source") await writeFile(path, "{ name = value; }");
+    const run = runner.run.bind(runner);
+    const commands = vi
+      .spyOn(runner, "run")
+      .mockImplementation(async (tool, args, options) => {
+        const result = await run(tool, args, options);
+        if (route !== "xml" || tool !== "plutil" || !result.ok) return result;
+        const stdout =
+          args[1] === "xml1" ? "<plist><string>value</string></plist>" : "";
+        return ok({
+          ...result.value,
+          stdout,
+          stdoutBytes: Buffer.byteLength(stdout),
+          exitCode: args[1] === "json" ? 1 : 0,
+        });
+      });
+    onTestFinished(() => commands.mockRestore());
+    let enter!: () => void;
+    let resume!: () => void;
+    const entered = new Promise<void>((resolve) => {
+      enter = resolve;
     });
-  onTestFinished(() => {
+    const gate = new Promise<void>((resolve) => {
+      resume = resolve;
+    });
+    const original = prototype.read;
+    const read = vi
+      .spyOn(prototype, "read")
+      .mockImplementationOnce(async function (
+        this: FileHandle,
+        ...args: Parameters<FileHandle["read"]>
+      ) {
+        enter();
+        await gate;
+        return Reflect.apply(original, this, args);
+      });
+    onTestFinished(() => {
+      resume();
+      read.mockRestore();
+    });
+    const inspecting = client.execute("inspect_plist", { path });
+    await entered;
+    let finished = false;
+    const closing = client.close().then((result) => {
+      finished = true;
+      return result;
+    });
+    await new Promise<void>((resolve) => setImmediate(resolve));
+    expect(finished).toBe(false);
     resume();
-    read.mockRestore();
-  });
-  const inspecting = client.execute("inspect_plist", { path });
-  await entered;
-  let finished = false;
-  const closing = client.close().then((result) => {
-    finished = true;
-    return result;
-  });
-  await new Promise<void>((resolve) => setImmediate(resolve));
-  expect(finished).toBe(false);
-  resume();
-  expect((await inspecting).ok).toBe(true);
-  expect((await closing).ok).toBe(true);
-});
+    const inspected = await inspecting;
+    expect(
+      inspected.ok,
+      inspected.ok ? undefined : inspected.error.message,
+    ).toBe(true);
+    expect((await closing).ok).toBe(true);
+    expect(commands.mock.calls.map(([tool, args]) => [tool, args[0]])).toEqual([
+      ["file", "-b"],
+      ["plutil", route === "source" ? "-lint" : "-convert"],
+      ...(route === "xml" ? [["plutil", "-convert"]] : []),
+    ]);
+  },
+);

tests/boundary/providers/native/nativePlistTextSource.test.ts

@@ -148,6 +148,7 @@ it.for<TextSourceCase>([
   const replay = new NativeFixtureRunner({ file: fixture.description });
   const tools: string[] = [];
   let conversionIndex = 0;
+  let validations = 0;
   const client = new NativeMacOSProvider(
     {},
     {
@@ -155,6 +156,11 @@ it.for<TextSourceCase>([
         tools.push(tool);
         const result = await replay.run(tool, args);
         if (tool !== "plutil" || !result.ok) return result;
+        if (args[0] === "-lint") {
+          expect(args).toEqual(["-lint", "--", path]);
+          validations += 1;
+          return result;
+        }
         const conversion = fixture.conversions[conversionIndex++];
         if (conversion === undefined)
           throw new Error("Unexpected conversion of directly supported source");
@@ -181,7 +187,14 @@ it.for<TextSourceCase>([
     source_path: path,
   });
   expect(result.provenance.map((item) => item.tool)).toEqual(tools);
-  expect(tools).toEqual(["file", ...fixture.conversions.map(() => "plutil")]);
+  const directXml =
+    fixture.format === "xml" && fixture.conversions.length === 0;
+  expect(validations).toBe(directXml ? 1 : 0);
+  expect(tools).toEqual([
+    "file",
+    ...(directXml ? ["plutil"] : []),
+    ...fixture.conversions.map(() => "plutil"),
+  ]);
   expect(conversionIndex).toBe(fixture.conversions.length);
   if (fixture.limitations !== undefined)
     expect(result.limitations).toEqual(fixture.limitations);