diff --git a/src/domain/desktop/appTaskReaders.ts b/src/domain/desktop/appTaskReaders.ts index dee95d6d46..82522fe104 100644 --- a/src/domain/desktop/appTaskReaders.ts +++ b/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']}") diff --git a/src/domain/portableBundleName.ts b/src/domain/portableBundleName.ts index d496de09ae..e6f2ebe77f 100644 --- a/src/domain/portableBundleName.ts +++ b/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}`); diff --git a/tests/boundary/mcp/compactInputSchemaMcp.test.ts b/tests/boundary/mcp/compactInputSchemaMcp.test.ts index 6b76db61e4..5a8b43dad3 100644 --- a/tests/boundary/mcp/compactInputSchemaMcp.test.ts +++ b/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); + } + }, +); diff --git a/tests/boundary/providers/desktopTaskPython.test.ts b/tests/boundary/providers/desktopTaskPython.test.ts index 0c91ad6e81..2241097d88 100644 --- a/tests/boundary/providers/desktopTaskPython.test.ts +++ b/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>; + readonly retryAnswers?: Readonly>; 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]!],