diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fed260e0fc..ebbe866c09 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -318,8 +318,8 @@ jobs: - run: npm run verify:asset-catalog - run: npm run verify:macos-bundle - run: npm run verify:installer-package - - name: Native compiled NIB hierarchy regressions - run: npm run test:focused -- tests/boundary/filesystem/nibNativeHierarchy.test.ts tests/boundary/filesystem/interfaceBuilderBundle.test.ts + - name: Native installer and compiled NIB hierarchy regressions + run: npm run test:focused -- tests/boundary/filesystem/installScript.test.ts tests/boundary/filesystem/nibNativeHierarchy.test.ts tests/boundary/filesystem/interfaceBuilderBundle.test.ts - name: Real binary metadata and Interface Builder integration run: npm run verify:apple-dispatch && npm run verify:interface-builder - name: Native plist CLI selection regressions diff --git a/install.sh b/install.sh index c236fc8ab5..d059694844 100755 --- a/install.sh +++ b/install.sh @@ -107,7 +107,7 @@ if [[ "$dry_run" == true ]]; then fi printf 'Installing %s@%s...\n' "$PACKAGE" "$version" -npm install --global "${prefix_args[@]}" "$PACKAGE@$version" || fail "npm could not install REA. Check registry access and npm permissions, then retry." +npm install --global ${prefix_args[@]+"${prefix_args[@]}"} "$PACKAGE@$version" || fail "npm could not install REA. Check registry access and npm permissions, then retry." [[ -x "$install_bin" ]] || fail "npm completed without installing the rea command. Check the npm global bin directory and PATH, then retry." installed_version="$("$install_bin" --version 2>/dev/null | tr -d '[:space:]')" || fail "the installed REA version could not be read. Reinstall the requested version, then retry." [[ "$installed_version" == "$version" ]] || fail "installed version $installed_version does not match $version. Reinstall the requested version, then retry." diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts index c647dfe48e..f798ebfc3e 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts @@ -362,6 +362,35 @@ describe("contextual JavaScript package path resolution", () => { }); }); +describe("generic package entry heuristics", () => { + it.each([ + ['{"main":"actual.cjs","module":"bundler.mjs"}', "actual.cjs"], + ['{"module":"bundler.mjs"}', "bundler.mjs"], + ])( + "preserves generic entry heuristics without a module kind for %s", + (metadata, entry) => { + const files = fileMap([ + file("app/consumer.js", "root"), + file("app/node_modules/fixture/package.json", "root", metadata), + file("app/node_modules/fixture/actual.cjs", "root"), + file("app/node_modules/fixture/bundler.mjs", "root"), + file("app/node_modules/fixture/index.js", "root"), + ]); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/consumer.js", + context: "module-specifier", + files, + }), + ).toMatchObject({ + resolution_status: "resolved", + resolved_path: `app/node_modules/fixture/${entry}`, + }); + }, + ); +}); + describe("directory package entrypoint precedence", () => { it.each([ ["require", '{"main":"actual.cjs"}', "actual.cjs"], diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.ts b/src/application/javascript/JavaScriptArtifactPathResolution.ts index 143d4503ab..dca7ac8e7e 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.ts @@ -461,10 +461,12 @@ const packageEntry = ( ? { ...exported, source: "exports" } : exported; } - const preferred = - moduleKind === "import" - ? [Reflect.get(value, "module"), Reflect.get(value, "main")] - : [Reflect.get(value, "main"), Reflect.get(value, "module")]; + // Node imports and requires ignore the bundler-only module field. Keep + // that fallback for callers without an explicit module-loading kind. + const preferred = [ + Reflect.get(value, "main"), + ...(moduleKind === undefined ? [Reflect.get(value, "module")] : []), + ]; const entry = preferred.find((candidate) => candidate !== undefined); if (entry === undefined) return { status: "missing" }; const legacy = packagePathValue(entry); diff --git a/src/cliJsonInput.ts b/src/cliJsonInput.ts index 44199b79ef..8bb2d105a5 100644 --- a/src/cliJsonInput.ts +++ b/src/cliJsonInput.ts @@ -37,9 +37,9 @@ export const parseCliJsonInput = async ( /** * Resolve named string fields of CLI-supplied JSON against the operator * working directory. Each entry is a key path from the document root; only - * fields holding strings are resolved, everything else passes through - * untouched. Shared MCP contracts reject relative paths, so CLI workflows - * resolve operator-relative values before validation. + * fields holding nonblank strings are resolved, everything else passes through + * untouched so shared schemas can reject blank paths. Shared MCP contracts reject + * relative paths, so CLI workflows resolve operator-relative values before validation. */ export const resolveCliJsonPaths = ( value: unknown, @@ -61,7 +61,12 @@ const resolveJsonPath = ( ([key, entry]: readonly [string, unknown]): readonly [string, unknown] => { if (key !== head) return [key, entry]; if (tail.length === 0) - return [key, typeof entry === "string" ? resolve(entry) : entry]; + return [ + key, + typeof entry === "string" && entry.trim().length > 0 + ? resolve(entry) + : entry, + ]; return [key, resolveJsonPath(entry, tail)]; }, ); diff --git a/src/cliJsonPaths.test.ts b/src/cliJsonPaths.test.ts new file mode 100644 index 0000000000..936bfacea1 --- /dev/null +++ b/src/cliJsonPaths.test.ts @@ -0,0 +1,59 @@ +import { resolve } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { resolveCliJsonPaths } from "./cliJsonInput.js"; +import { electronActiveObservationInputSchema } from "./domain/javascript/electronActiveObservation.js"; + +const paths = [ + ["executable_path"], + ["application_path"], + ["application_root"], +] as const; + +describe("CLI JSON path normalization", () => { + it.each([ + ["executable_path", ""], + ["executable_path", " \t "], + ["application_path", ""], + ["application_path", " \t "], + ["application_root", ""], + ["application_root", " \t "], + ])( + "keeps blank %s (%j) invalid at the shared input boundary", + (field, value) => { + const input = { + executable_path: "/synthetic/electron", + application_path: "/synthetic/app", + application_root: "/synthetic", + [field]: value, + }; + const parsed = electronActiveObservationInputSchema.safeParse( + resolveCliJsonPaths(input, paths), + ); + expect(parsed.success).toBe(false); + if (parsed.success) throw new Error("Expected blank path rejection"); + expect(parsed.error.issues).toEqual( + expect.arrayContaining([expect.objectContaining({ path: [field] })]), + ); + expect(input).toHaveProperty(field, value); + }, + ); + + it("resolves selected relative paths without trimming names or changing unrelated fields", () => { + const input = { + browser: { executable_path: " browser ", label: "relative-label" }, + application_path: "app", + }; + expect( + resolveCliJsonPaths(input, [["browser", "executable_path"]]), + ).toEqual({ + browser: { + executable_path: resolve(" browser "), + label: "relative-label", + }, + application_path: "app", + }); + expect(input.browser.executable_path).toBe(" browser "); + }); +}); diff --git a/src/contracts/toolSchemaMetadata.ts b/src/contracts/toolSchemaMetadata.ts index 90762695b2..bed9526ba0 100644 --- a/src/contracts/toolSchemaMetadata.ts +++ b/src/contracts/toolSchemaMetadata.ts @@ -56,10 +56,11 @@ export const toolInputSchemaWithMetadata = ( withAdvertisedJsonSchema( contract.inputSchema, "input", - (project) => (options) => ({ - ...describeProperties(project(options)), - examples: contract.examples.map(({ input }) => input), - }), + (project) => (options) => + flattenRootUnion({ + ...describeProperties(project(options)), + examples: contract.examples.map(({ input }) => input), + }), ); /** Advertise a canonical output schema without reconverting it per listing. */ @@ -113,6 +114,57 @@ const memoizeByTarget = ( }; }; +/** Keep MCP tool roots object-shaped for clients that reject root unions. */ +const flattenRootUnion = ( + value: Record, +): Record => { + if (!Array.isArray(value.anyOf)) return value; + const branches = value.anyOf.filter(isObject); + if (branches.length === 0 || branches.length !== value.anyOf.length) + return value; + + const properties: Record = {}; + for (const branch of branches) { + if (!isObject(branch.properties)) return value; + for (const [name, schema] of Object.entries(branch.properties)) { + const previous = properties[name]; + if (previous === undefined) properties[name] = schema; + else if (JSON.stringify(previous) !== JSON.stringify(schema)) + properties[name] = { + anyOf: [previous, schema], + description: fallbackPropertyDescription(name), + }; + } + } + + // The canonical Zod parser still enforces branch validation at invocation. + const { anyOf: _anyOf, required: _required, ...root } = value; + const requiredByBranch = branches.map((branch) => + Array.isArray(branch.required) + ? branch.required.filter( + (item): item is string => typeof item === "string", + ) + : [], + ); + const required = requiredByBranch.reduce((common, current) => + common.filter((name) => current.includes(name)), + ); + const minProperties = Math.max( + typeof root.minProperties === "number" ? root.minProperties : 0, + Math.min(...requiredByBranch.map((names) => names.length)), + ); + return { + ...root, + type: "object", + properties, + ...(minProperties > 0 ? { minProperties } : {}), + ...(required.length > 0 ? { required } : {}), + ...(branches.every((branch) => branch.additionalProperties === false) + ? { additionalProperties: false } + : {}), + }; +}; + const describeProperties = ( value: Readonly>, ): Record => diff --git a/tests/acceptance/applications/electronBlankPathCli.test.ts b/tests/acceptance/applications/electronBlankPathCli.test.ts new file mode 100644 index 0000000000..f060aed34d --- /dev/null +++ b/tests/acceptance/applications/electronBlankPathCli.test.ts @@ -0,0 +1,32 @@ +import { expect } from "vitest"; + +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest.for(["", " \t "])( + "rejects blank Electron application_root (%j) before filesystem or provider work", + async (applicationRoot, { cli }) => { + const response = await cli.run({ + arguments: [ + "capture-electron-scenario", + JSON.stringify({ + executable_path: "/synthetic/missing-electron", + application_path: "/synthetic/missing-app", + application_root: applicationRoot, + }), + "--json", + ], + environment: { REA_LOG_LEVEL: "silent" }, + timeoutMs: 15000, + }); + expect(response.exitCode).toBe(1); + expect(response.json).toMatchObject({ + code: "invalid_request", + details: { + operation: "capture_electron_scenario", + issues: expect.arrayContaining([ + { path: ["application_root"], reason: "out_of_range", minimum: 1 }, + ]), + }, + }); + }, +); diff --git a/tests/boundary/filesystem/installScript.test.ts b/tests/boundary/filesystem/installScript.test.ts index 3abdf6a4ff..57450fee4d 100644 --- a/tests/boundary/filesystem/installScript.test.ts +++ b/tests/boundary/filesystem/installScript.test.ts @@ -172,6 +172,22 @@ describe("curl installer scenarios", { timeout: 20_000 }, () => { }); }); +describe("native macOS installer", { timeout: 20_000 }, () => { + it("installs on Darwin with an empty prefix argument array", async () => { + const fixture = await createFixture(); + const result = await runInstaller(fixture, ["--version", "0.3.0"], { + FAKE_PLATFORM: "Darwin", + FAKE_NPM_PREFIX: join(fixture.home, "npm global prefix"), + }); + expect(result.stdout).toContain("REA 0.3.0 is installed"); + expect(await readFile(fixture.npmLog, "utf8")).toBe( + "prefix --global\ninstall --global rea-agents@0.3.0\n", + ); + expect(await readFile(fixture.reaLog, "utf8")).toBe("--version\n"); + expect(await readdir(fixture.temporary)).toEqual([]); + }); +}); + describe("installer semantic version parsing", { timeout: 20_000 }, () => { it.each(["1.2.3-01"])( "rejects malformed semantic versions before invoking npm: %s", @@ -309,6 +325,7 @@ printf '%s\n' "$*" >> "$FAKE_NPM_LOG" [ "$1" = "prefix" ] && { [ "\${FAKE_NPM_PREFIX_FAIL:-}" = "1" ] && exit 1; printf '%s\n' "$FAKE_NPM_PREFIX"; exit 0; } [ "\${FAKE_NPM_FAIL:-}" = "1" ] && exit 1 prefix="$HOME/.local" +[ "\${FAKE_PLATFORM:-Linux}" = "Darwin" ] && prefix="$FAKE_NPM_PREFIX" while [ "$#" -gt 0 ]; do if [ "$1" = "--prefix" ]; then shift; prefix="$1"; fi shift diff --git a/tests/boundary/filesystem/javascriptPackagePrecedence.test.ts b/tests/boundary/filesystem/javascriptPackagePrecedence.test.ts index 14c0561663..4c730fb73c 100644 --- a/tests/boundary/filesystem/javascriptPackagePrecedence.test.ts +++ b/tests/boundary/filesystem/javascriptPackagePrecedence.test.ts @@ -1,7 +1,11 @@ -import { mkdir, writeFile } from "node:fs/promises"; +import { execFile } from "node:child_process"; +import { mkdir, realpath, writeFile } from "node:fs/promises"; import { createRequire } from "node:module"; import { dirname, join, relative, sep } from "node:path"; +import { fileURLToPath } from "node:url"; +import { promisify } from "node:util"; + import { expect, it } from "vitest"; import { reconstructJavaScriptArtifact } from "../../../src/application/javascript/JavaScriptArtifactReconstruction.js"; @@ -160,3 +164,108 @@ it.each(cases)( expect(result.graph.coverage.status).toBe("complete"); }, ); + +const runNode = promisify(execFile); + +it.each([ + { + name: "import uses main before bundler module", + kind: "import", + metadata: { main: "actual.cjs", module: "bundler.mjs" }, + }, + { + name: "import ignores a module-only entry", + kind: "import", + metadata: { module: "bundler.mjs" }, + }, + { + name: "require ignores a module-only entry", + kind: "require", + metadata: { module: "bundler.mjs" }, + }, + { + name: "import preserves a main-only entry", + kind: "import", + metadata: { main: "actual.cjs" }, + }, + { + name: "require preserves main before bundler module", + kind: "require", + metadata: { main: "actual.cjs", module: "bundler.mjs" }, + }, + { + name: "import preserves declared exports before legacy entries", + kind: "import", + metadata: { + main: "actual.cjs", + module: "bundler.mjs", + exports: { import: "./exported.mjs", require: "./actual.cjs" }, + }, + }, +] as const)( + "matches the native Node loader when $name", + async ({ kind, metadata }) => { + const root = await createTestTempDirectory("rea-node-package-fields-"); + const entry = kind === "import" ? "main.mjs" : "main.cjs"; + const source = + kind === "import" + ? `import value from "fixture"; console.log(JSON.stringify({value, resolved: import.meta.resolve("fixture")}));` + : `const value = require("fixture"); console.log(JSON.stringify({value, resolved: require.resolve("fixture")}));`; + const files = { + "package.json": JSON.stringify({ + name: "app", + main: entry, + type: kind === "import" ? "module" : "commonjs", + }), + [entry]: source, + "node_modules/fixture/package.json": JSON.stringify({ + name: "fixture", + ...metadata, + }), + "node_modules/fixture/actual.cjs": "module.exports = 'actual.cjs';", + "node_modules/fixture/index.js": "module.exports = 'index.js';", + "node_modules/fixture/bundler.mjs": "export default 'bundler.mjs';", + "node_modules/fixture/exported.mjs": "export default 'exported.mjs';", + }; + await Promise.all( + Object.entries(files).map(async ([path, text]) => { + await mkdir(dirname(join(root, path)), { recursive: true }); + await writeFile(join(root, path), text); + }), + ); + const { stdout } = await runNode(process.execPath, [join(root, entry)], { + cwd: root, + }); + const oracle: unknown = JSON.parse(stdout); + expect(oracle).toMatchObject({ + value: expect.any(String), + resolved: expect.any(String), + }); + if (typeof oracle !== "object" || oracle === null) + throw new Error("Node returned no package identity"); + const resolved: unknown = Reflect.get(oracle, "resolved"); + if (typeof resolved !== "string") + throw new Error("Node returned no resolved path"); + const expected = relative( + await realpath(root), + kind === "import" ? fileURLToPath(resolved) : resolved, + ) + .split(sep) + .join("/"); + expect(Reflect.get(oracle, "value")).toBe(expected.split("/").at(-1)); + const result = await reconstructJavaScriptArtifact({ + input_path: root, + format: "directory", + }); + const edge = result.graph.edges.find( + ({ properties }) => + properties.module_link_kind === kind && + properties.specifier === "fixture", + ); + expect(edge?.properties).toMatchObject({ + resolution_status: "resolved", + resolved_path: expected, + }); + expect(result.graph.coverage.status).toBe("complete"); + }, +); diff --git a/tests/boundary/mcp/toolSchemaValidity.test.ts b/tests/boundary/mcp/toolSchemaValidity.test.ts index 708f8d40dc..d2ac26ddfe 100644 --- a/tests/boundary/mcp/toolSchemaValidity.test.ts +++ b/tests/boundary/mcp/toolSchemaValidity.test.ts @@ -328,8 +328,10 @@ describe("MCP JSON Schema validity", () => { await Promise.allSettled([client.close(), server.close()]); } }); +}); - it("advertises the structural request requirements enforced at runtime", async () => { +describe("MCP root input schemas", () => { + it("advertises object roots while enforcing union requirements at runtime", async () => { const server = new McpServer({ name: "schema-constraints", version: "0" }); const client = new Client({ name: "schema-constraints", version: "0" }); const [clientTransport, serverTransport] = @@ -353,12 +355,18 @@ describe("MCP JSON Schema validity", () => { if (graphContract === undefined || graphTool === undefined) throw new Error("Managed application graph tool was not advertised"); expect(graphContract.inputSchema.safeParse({}).success).toBe(false); + expect(graphTool.inputSchema).toMatchObject({ type: "object" }); expect(ajv.compile(graphTool.inputSchema)({})).toBe(false); + expect(ajv.compile(graphTool.inputSchema)({ unrelated: true })).toBe( + false, + ); for (const contract of TOOL_CONTRACTS) { - const validate = ajv.compile( - advertised.get(contract.name)!.inputSchema, - ); + const inputSchema = advertised.get(contract.name)!.inputSchema; + expect(inputSchema.type, contract.name).toBe("object"); + expect(inputSchema.properties, contract.name).toBeDefined(); + expect(inputSchema.anyOf, contract.name).toBeUndefined(); + const validate = ajv.compile(inputSchema); for (const example of contract.examples) expect( validate(example.input),