fix(desktop): preserve cancellation and portable name contracts (#10074)

* fix(desktop): preserve cancellation during reader cleanup

* fix(contracts): advertise portable name restrictions

src/domain/desktop/appTaskReaders.ts

@@ -443,6 +443,9 @@ async def _reader_views(env_url, card):
             except BaseException as error:
                 _PENDING_READERS[token]["abandoned"] = True
                 if failure is not None:
+                    if isinstance(error, asyncio.CancelledError):
+                        error.add_note(f"Reader cleanup remains owned for {lease['path']}; reader failure: {type(failure).__name__}: {failure}")
+                        raise error from failure
                     failure.add_note(f"Reader cleanup remains owned for {lease['path']}: {type(error).__name__}: {error}")
                     raise failure from error
                 error.add_note(f"Reader cleanup remains owned for {lease['path']}")

src/domain/portableBundleName.ts

@@ -4,11 +4,18 @@ import { normalizeHttpHostname } from "./httpHostname.js";
 import { err, ok, type Result } from "./result.js";
 
 /** Agentenv folder and local-id spelling, also used for authored task files. */
-export const PORTABLE_BUNDLE_NAME_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/u;
+export const PORTABLE_BUNDLE_NAME_PATTERN =
+  /^[a-z0-9]+(?:-[a-z0-9]+)*(?![\s\S])/u;
 // A compose service name is one DNS label, whose wire limit is 63 octets.
 export const PORTABLE_BUNDLE_NAME_MAX_LENGTH = 63;
 export const PORTABLE_BUNDLE_NAME_CONSTRAINT = `a kebab-case label of at most ${String(PORTABLE_BUNDLE_NAME_MAX_LENGTH)} characters, excluding Windows device names, that remains unchanged as an HTTP service hostname`;
-const WINDOWS_RESERVED_NAME = /^(?:con|prn|aux|nul|com[1-9]|lpt[1-9])$/u;
+const WINDOWS_RESERVED_PART = "con|prn|aux|nul|com[1-9]|lpt[1-9]";
+const WINDOWS_RESERVED_NAME = new RegExp(`^(?:${WINDOWS_RESERVED_PART})$`, "u");
+// A single label ending in a number is parsed as an IPv4 address by URL.
+// With this module's ASCII label grammar those are decimal and hexadecimal forms.
+const NON_NUMERIC_HOSTNAME_PATTERN = /^(?!(?:[0-9]+|0x[0-9a-f]*)$)/u;
+const NON_RESERVED_NAME_PATTERN =
+  /^(?!(?:con|prn|aux|nul|com[1-9]|lpt[1-9])$)/u;
 
 export type PortableBundleNameFailure = "format" | "reserved" | "hostname";
 
@@ -35,4 +42,6 @@ export const portableBundleNameSchema = z
   .string()
   .regex(PORTABLE_BUNDLE_NAME_PATTERN)
   .max(PORTABLE_BUNDLE_NAME_MAX_LENGTH)
+  .regex(NON_NUMERIC_HOSTNAME_PATTERN)
+  .regex(NON_RESERVED_NAME_PATTERN)
   .refine(isPortableBundleName, `Expected ${PORTABLE_BUNDLE_NAME_CONSTRAINT}`);

tests/boundary/mcp/compactInputSchemaMcp.test.ts

@@ -836,40 +836,62 @@ it("keeps root detail guidance so callers can choose bounded summaries", async (
   expect(detailGuidance(compactTools)).toEqual(expected);
 });
 
-it("advertises portable bundle name spelling and length through the SDK", async () => {
-  const contract = toolContract("export_environment_bundle");
-  const server = fullServer();
-  server.registerTool(
-    contract.name,
-    toolRegistrationOptions(contract),
-    async () => ({ content: [] }),
-  );
-  const tool = (await listToolsThroughTransport(server)).find(
-    ({ name }) => name === contract.name,
-  );
-  expect(tool).toBeDefined();
-  const ajv = new Ajv2020({ strict: false, validateFormats: false });
-  const validate = ajv.compile(tool!.inputSchema);
-  const base = {
-    source: { kind: "evidence-file", path: "/capture.json" },
-    output_directory: "/bundle",
-  };
-  for (const [name, accepted] of [
-    [undefined, true],
-    ["replay-2", true],
-    ["123-replay", true],
-    ["a".repeat(63), true],
-    ["a".repeat(64), false],
-    ["../escape", false],
-    ["Uppercase", false],
-    ["two--hyphens", false],
-  ] as const) {
-    const input = { ...base, ...(name === undefined ? {} : { name }) };
-    expect(validate(input), JSON.stringify(input)).toBe(accepted);
-    expect(contract.inputSchema.safeParse(input).success).toBe(accepted);
-  }
-  for (const name of ["con", "123", "0xffffffff"])
-    expect(contract.inputSchema.safeParse({ ...base, name }).success).toBe(
-      false,
+it.each([
+  [
+    "export_environment_bundle",
+    "name",
+    {
+      source: { kind: "evidence-file", path: "/capture.json" },
+      output_directory: "/bundle",
+    },
+  ],
+  [
+    "measure_build_task",
+    "task",
+    { bundle: "/bundle", model: "test-model", budget_usd: 1 },
+  ],
+] as const)(
+  "advertises portable names for %s through the SDK",
+  async (name, field, base) => {
+    const contract = toolContract(name);
+    const server = fullServer();
+    server.registerTool(
+      contract.name,
+      toolRegistrationOptions(contract),
+      async () => ({ content: [] }),
     );
-});
+    const tool = (await listToolsThroughTransport(server)).find(
+      ({ name }) => name === contract.name,
+    );
+    expect(tool).toBeDefined();
+    const ajv = new Ajv2020({ strict: false, validateFormats: false });
+    const validate = ajv.compile(tool!.inputSchema);
+    for (const [name, accepted] of [
+      [undefined, field === "name"],
+      ["replay-2", true],
+      ["123-replay", true],
+      ["a".repeat(63), true],
+      ["a".repeat(64), false],
+      ["../escape", false],
+      ["Uppercase", false],
+      ["two--hyphens", false],
+      ["trailing\n", false],
+      ["con", false],
+      ["com1", false],
+      ["lpt9", false],
+      ["123", false],
+      ["0128", false],
+      ["0x", false],
+      ["0xffffffff", false],
+      ["0xg", true],
+      ["com10", true],
+    ] as const) {
+      const input = {
+        ...base,
+        ...(name === undefined ? {} : { [field]: name }),
+      };
+      expect(validate(input), JSON.stringify(input)).toBe(accepted);
+      expect(contract.inputSchema.safeParse(input).success).toBe(accepted);
+    }
+  },
+);

tests/boundary/providers/desktopTaskPython.test.ts

@@ -148,7 +148,8 @@ async def run():
             verify_task = asyncio.create_task(module.verify("http://mcp", "http://computer:8080"))
             await verify_task
         except (RuntimeError, asyncio.CancelledError) as error:
-            first_error = str(error) + " ".join(getattr(error, "__notes__", ()))
+            first_error = type(error).__name__ + ": " + str(error) + " ".join(getattr(error, "__notes__", ()))
+            answers.update(scenario.get("retry_answers") or {})
         if scenario.get("reload_verifier"):
             module = load_verifier()
     if scenario.get("parallel_retry"):
@@ -194,6 +195,7 @@ const runVerifier = async (
   computer: {
     readonly readers?: readonly DesktopTaskReader[];
     readonly answers?: Readonly<Record<string, unknown>>;
+    readonly retryAnswers?: Readonly<Record<string, unknown>>;
     readonly exec?: boolean;
     readonly cleanupFailures?: number;
     readonly retry?: boolean;
@@ -233,6 +235,7 @@ const runVerifier = async (
         saves,
         durations,
         answers: computer.answers,
+        retry_answers: computer.retryAnswers,
         exec: computer.exec ?? true,
         cleanup_failures: computer.cleanupFailures,
         retry: computer.retry,
@@ -593,6 +596,25 @@ it("keeps cleanup owned when verification is cancelled during final cleanup", as
   expect(cleanups).toHaveLength(2);
 });
 
+it("retains reader failure when cancellation interrupts its final cleanup", async () => {
+  const output = await runVerifier(READER_CHECKS, [[0, CHECK_VIEWS]], [], {
+    readers: [READERS[0]!],
+    answers: { [READERS[0]!.id]: "not a reader answer" },
+    retryAnswers: ANSWERS,
+    cancelCleanup: true,
+    retry: true,
+    reloadVerifier: true,
+  });
+  expect(output.first_error).toMatch(/^CancelledError:/u);
+  expect(output.first_error).toContain("reader runner did not answer");
+  expect(output.first_error).toContain("Reader cleanup remains owned");
+  const cleanups = output.execs.filter(({ request_id }) =>
+    String(request_id).startsWith("rea-reader-cleanup-"),
+  );
+  expect(cleanups).toHaveLength(2);
+  expect(output.pending_readers).toBe(0);
+});
+
 it("does not adopt another active verifier's reader lease", async () => {
   const output = await runVerifier(READER_CHECKS, [[0, CHECK_VIEWS]], [], {
     readers: [READERS[0]!],