fix(browser): normalize selected script manifest roots on Windows (#1078)

* fix(browser): normalize selected script manifest roots on Windows

* test(browser): keep manifest fixtures valid across Windows drives

.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

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);

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",

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",

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",
+  );
+});