diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 413fdb5c62..b7ed573ba9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -445,6 +445,9 @@ jobs: tests/boundary/filesystem/setupUnavailableProvider.test.ts tests/boundary/filesystem/windowsNpxRegistration.test.ts src/application/ClientRegistrationIdentity.test.ts + tests/boundary/filesystem/webScriptManifestPaths.test.ts + tests/acceptance/webModuleCli.test.ts + tests/acceptance/webSourceLocationCli.test.ts tests/boundary/cli/referenceSourceImportStatus.test.ts src/cliReferenceSourceImportStatus.test.ts tests/boundary/filesystem/asarInventoryFreshness.test.ts diff --git a/src/browser/assets/LocalWebScriptArtifacts.ts b/src/browser/assets/LocalWebScriptArtifacts.ts index 625246cebf..985756a738 100644 --- a/src/browser/assets/LocalWebScriptArtifacts.ts +++ b/src/browser/assets/LocalWebScriptArtifacts.ts @@ -1,5 +1,5 @@ import { lstat, realpath } from "node:fs/promises"; -import { dirname, isAbsolute, join, relative, sep } from "node:path"; +import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; import { readStableArtifact } from "../../artifacts/readStableArtifact.js"; import { normalizeArtifactPath } from "../../artifacts/ArtifactPaths.js"; @@ -55,7 +55,7 @@ export class LocalWebScriptArtifacts implements WebScriptArtifactPort { "path", `Selected source path changes during normalization: ${selected.content.relative_path}`, ); - const root = dirname(input.manifest_path); + const root = resolve(dirname(input.manifest_path)); const sourcePath = join(root, "files", portable); targetPath = sourcePath; await assertContained(root, sourcePath); diff --git a/tests/acceptance/webModuleCli.test.ts b/tests/acceptance/webModuleCli.test.ts index fc4d7bd88f..5bab64449f 100644 --- a/tests/acceptance/webModuleCli.test.ts +++ b/tests/acceptance/webModuleCli.test.ts @@ -24,7 +24,7 @@ cliTest( join(root, "files", "modules", "main.js"), artifacts.source, ); - const manifestPath = join(root, "manifest.json"); + const manifestPath = join(root, "manifest.json").replaceAll("\\", "/"); await writeFile(manifestPath, JSON.stringify(manifest)); const environment = { REA_LOG_LEVEL: "silent", diff --git a/tests/acceptance/webSourceLocationCli.test.ts b/tests/acceptance/webSourceLocationCli.test.ts index c7ca688e2d..2e7b3a2fe3 100644 --- a/tests/acceptance/webSourceLocationCli.test.ts +++ b/tests/acceptance/webSourceLocationCli.test.ts @@ -1,4 +1,10 @@ -import { expect } from "vitest"; +import { expect, onTestFinished } from "vitest"; +import { Client } from "@modelcontextprotocol/client"; +import { + StdioClientTransport, + getDefaultEnvironment, +} from "@modelcontextprotocol/client/stdio"; +import { fileURLToPath } from "node:url"; import { mkdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { webSourceLocationFixture } from "../fixtures/webSourceLocation.js"; @@ -12,7 +18,7 @@ cliTest( const fixture = webSourceLocationFixture(); await mkdir(join(root, "files", "modules"), { recursive: true }); await writeFile(join(root, "files", "modules", "main.js"), fixture.source); - const manifest = join(root, "manifest.json"); + const manifest = join(root, "manifest.json").replaceAll("\\", "/"); const map = join(root, "app.js.map"); await writeFile(manifest, JSON.stringify(fixture.manifest)); await writeFile(map, fixture.sourceMap.text); @@ -44,6 +50,40 @@ cliTest( runtime: { id: "node-v8" }, }, }); + const transport = new StdioClientTransport({ + command: process.execPath, + args: [ + fileURLToPath(new URL("../../scripts/rea.mjs", import.meta.url)), + "mcp", + ], + env: { ...getDefaultEnvironment(), REA_LOG_LEVEL: "silent" }, + stderr: "pipe", + }); + const client = new Client({ + name: "source-location-cli-parity", + version: "1", + }); + onTestFinished(async () => { + try { + await client.close(); + } finally { + await transport.close(); + } + }); + await client.connect(transport); + const mcpResponse = await client.callTool({ + name: "trace_web_source_location", + arguments: { + manifest_path: manifest, + script_index: 0, + source_map: { path: map, url: fixture.sourceMap.url }, + generated_position: { line: 1, column: 0 }, + }, + }); + expect(mcpResponse.isError, JSON.stringify(mcpResponse)).not.toBe(true); + expect(mcpResponse.structuredContent).toMatchObject({ + evidence: response.json, + }); const invalid = await cli.run({ arguments: [ "trace-web-source-location", diff --git a/tests/boundary/filesystem/webScriptManifestPaths.test.ts b/tests/boundary/filesystem/webScriptManifestPaths.test.ts new file mode 100644 index 0000000000..9996fa8a99 --- /dev/null +++ b/tests/boundary/filesystem/webScriptManifestPaths.test.ts @@ -0,0 +1,77 @@ +import { mkdir, writeFile } from "node:fs/promises"; +import { isAbsolute, join, relative, sep } from "node:path"; +import { expect, it } from "vitest"; +import { LocalWebScriptArtifacts } from "../../../src/browser/assets/LocalWebScriptArtifacts.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { webModuleArtifactsFixture } from "../../fixtures/webModuleTrace.js"; + +const materialize = async () => { + const root = join( + await createTestTempDirectory("rea-script-manifest-path-"), + "export with spaces", + ); + const artifact = webModuleArtifactsFixture(); + await mkdir(join(root, "files", "modules"), { recursive: true }); + await mkdir(join(root, "nested")); + const sourcePath = join(root, "files", "modules", "main.js"); + const manifestPath = join(root, "manifest.json"); + await writeFile(sourcePath, artifact.source); + await writeFile(manifestPath, JSON.stringify(artifact.manifest)); + return { root, artifact, sourcePath, manifestPath }; +}; + +it("reads equivalent local manifest spellings without changing reported identity", async () => { + const fixture = await materialize(); + const paths = new Set([ + fixture.manifestPath, + fixture.manifestPath.replaceAll("\\", "/"), + `${fixture.root}${sep}nested${sep}..${sep}manifest.json`, + ]); + const fromWorkingDirectory = relative(process.cwd(), fixture.manifestPath); + // A target on another Windows drive has no relative path from this cwd. + if (!isAbsolute(fromWorkingDirectory)) + paths.add(`.${sep}${fromWorkingDirectory}`); + for (const operation of [ + "trace_web_module_imports", + "trace_web_source_location", + ] as const) { + for (const manifestPath of paths) { + const response = await new LocalWebScriptArtifacts(operation).load({ + manifest_path: manifestPath, + script_index: 0, + }); + if (!response.ok) throw response.error; + expect(response.value.source).toBe(fixture.artifact.source); + expect(response.value.sourceFile).toEqual({ + ...fixture.artifact.sourceFile, + path: fixture.sourcePath, + }); + expect(response.value.manifestFile.path).toBe(manifestPath); + expect(response.value.manifest.output_directory).toBe( + fixture.artifact.manifest.output_directory, + ); + expect(response.value.manifest.scripts[0]?.url).toBe( + fixture.artifact.manifest.scripts[0]?.url, + ); + } + } +}); + +it("still rejects changed source bytes after normalizing the selected manifest path", async () => { + const fixture = await materialize(); + await writeFile(fixture.sourcePath, "tampered source"); + const response = await new LocalWebScriptArtifacts( + "trace_web_module_imports", + ).load({ + manifest_path: `${fixture.root}/nested/../manifest.json`.replaceAll( + "\\", + "/", + ), + script_index: 0, + }); + if (response.ok) throw new Error("Expected source integrity mismatch"); + expect(projectAnalysisError(response.error).code).toBe( + "artifact_integrity_mismatch", + ); +});