fix: make path checks and tool output cross-platform on Windows (#32)

Three production bugs that broke on Windows because path separator handling assumed POSIX. CI is Linux-only so they were invisible there.

1. ingest.ts cloneRepo rejected every repo with `Unsafe clone path` because the safety guard checked `startsWith(`/`)` while `resolve()` produces `\` separators on Windows. Replaced with a path.relative-based helper that works on either platform.

2. web/routes.ts file-download endpoint returned 400 for every legitimate file for the same reason. Same fix.

3. tools.ts search shim `line.replace(repoPath + '/', '')` failed to strip the absolute repo path on Windows, leaking it into LLM-facing output. Switched to running ripgrep with cwd + relative target + --path-separator / so output is always relative forward-slash paths regardless of platform. list_files now also normalizes its displayed path to forward slashes for consistency with the tool description.

Added two helpers in utils.ts: `isPathInsideDir` (resolves both paths internally to avoid misuse) and `toPosixPath` for display normalization, mirroring the pattern already used in impact.ts.

Test fixes: 21 unit + 1 e2e test failed on Windows because they baked in POSIX-only assumptions. Switched literal `/repo/...` expectations to `path.resolve(...)` and normalized separators in fs mock predicates. The e2e helper now mirrors HOME to USERPROFILE on Windows so cache-redirection tests work cross-platform.

All checks pass on Windows now: typecheck, lint, build, 965/965 unit tests, 9/9 CLI e2e tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Arthur742Ramos

src/ingest.ts

@@ -19,7 +19,7 @@ import type {
   MonorepoManager,
   RepoProvider,
 } from "./types.js";
-import { SKIP_DIRS } from "./utils.js";
+import { SKIP_DIRS, isPathInsideDir } from "./utils.js";
 import frameworkMaps from "./data/framework-maps.json" with { type: "json" };
 
 const execFileAsync = promisify(execFile);
@@ -180,7 +180,7 @@ export async function cloneRepo(
   const safeOwner = repoInfo.owner.replace(/[^A-Za-z0-9._/-]/g, "_").replaceAll("/", "__");
   const safeRepo = repoInfo.repo.replace(/[^A-Za-z0-9._-]/g, "_");
   const clonePath = resolve(tempRoot, `${safeOwner}__${safeRepo}`);
-  if (!clonePath.startsWith(`${tempRoot}/`) && clonePath !== tempRoot) {
+  if (!isPathInsideDir(tempRoot, clonePath)) {
     throw new Error("Unsafe clone path");
   }
 

src/tools.ts

@@ -10,7 +10,7 @@ import { readFile, readdir, stat } from "fs/promises";
 import { isAbsolute, join, relative, resolve } from "path";
 import { execFile } from "child_process";
 import { promisify } from "util";
-import { SKIP_DIRS } from "./utils.js";
+import { SKIP_DIRS, toPosixPath } from "./utils.js";
 
 const execFileAsync = promisify(execFile);
 
@@ -175,7 +175,7 @@ function createListFilesTool(context: ToolContext): Tool<ListFilesToolArgs> {
           }
 
           const prefix = entry.isDirectory() ? "[dir]  " : "[file] ";
-          results.push(`${prefix}${relativePath}`);
+          results.push(`${prefix}${toPosixPath(relativePath)}`);
 
           if (entry.isDirectory() && recursive) {
             await scanDir(entryPath, depth + 1);
@@ -230,25 +230,40 @@ function createSearchTool(context: ToolContext): Tool<SearchToolArgs> {
   },
   handler: async (args: SearchToolArgs) => {
     const { pattern, path = "", filePattern, maxResults = 50 } = args;
+    // Validate the requested search path stays inside the repo (path traversal guard).
     const searchPath = safePath(context.repoPath, path);
+    const repoRoot = resolve(context.repoPath);
+    // Express the search target as a forward-slash relative path so ripgrep
+    // emits matches with paths the LLM can feed back into read_file/list_files.
+    const relSearchPath = relative(repoRoot, searchPath);
+    const rgTarget = toPosixPath(relSearchPath) || ".";
 
     context.onToolCall?.("search", { pattern, path, filePattern });
 
     try {
-      const rgArgs = ["--line-number", "--no-heading", "--max-count", String(maxResults)];
+      // --path-separator / normalizes Windows backslashes to forward slashes
+      // in printed paths; combined with cwd + a relative search target, the
+      // output never contains absolute paths.
+      const rgArgs = [
+        "--line-number",
+        "--no-heading",
+        "--path-separator",
+        "/",
+        "--max-count",
+        String(maxResults),
+      ];
       if (filePattern) {
         rgArgs.push("--glob", filePattern);
       }
-      rgArgs.push(pattern, searchPath);
+      rgArgs.push(pattern, rgTarget);
 
-      const { stdout } = await execFileAsync("rg", rgArgs, { timeout: 30000 });
-
-      // Make paths relative
-      const lines = stdout.split("\n").filter(Boolean).map(line => {
-        const relativeLine = line.replace(context.repoPath + "/", "");
-        return relativeLine;
+      const { stdout } = await execFileAsync("rg", rgArgs, {
+        cwd: repoRoot,
+        timeout: 30000,
       });
 
+      const lines = stdout.split("\n").filter(Boolean);
+
       const result = lines.length > 0
         ? lines.join("\n")
         : `No matches found for pattern: ${pattern}`;

src/utils.ts

@@ -3,6 +3,7 @@
  */
 
 import { readFile, access } from "fs/promises";
+import { isAbsolute, relative, resolve } from "path";
 
 /**
  * Common directory names to skip during repository traversal
@@ -47,3 +48,35 @@ export async function readFileSafe(filePath: string): Promise<string | null> {
 export function escapeRegex(str: string): string {
   return str.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
 }
+
+/**
+ * Convert a filesystem path to POSIX form (forward slashes).
+ *
+ * Use for paths that are surfaced to LLMs, written to JSON outputs,
+ * or compared against repo-relative path strings — anywhere the
+ * native Windows backslash separator would cause inconsistency.
+ *
+ * Does not perform path resolution; callers should resolve/normalize first
+ * if needed.
+ */
+export function toPosixPath(p: string): string {
+  return p.replace(/\\/g, "/");
+}
+
+/**
+ * Test whether `child` is the same path as `parent` or strictly inside it.
+ *
+ * Resolves both inputs internally so callers can pass relative or absolute
+ * paths in any normalized form. Cross-platform: works correctly on Windows
+ * where the path separator is `\` and on POSIX where it is `/`.
+ *
+ * NOTE: This is a lexical check only — it does not resolve symlinks. If a
+ * path inside `parent` is a symlink pointing outside, this still returns
+ * true. Use `fs.realpath` first if symlink containment matters.
+ */
+export function isPathInsideDir(parent: string, child: string): boolean {
+  const resolvedParent = resolve(parent);
+  const resolvedChild = resolve(child);
+  const rel = relative(resolvedParent, resolvedChild);
+  return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel));
+}

src/web/routes.ts

@@ -17,6 +17,7 @@ import {
 import { resolveRunConfiguration } from "../services/config-resolution.js";
 import { writeGeneratedOutputs } from "../services/output-writer.js";
 import type { BootcampOptions, RepoFacts } from "../types.js";
+import { isPathInsideDir } from "../utils.js";
 
 /**
  * Progress event for SSE
@@ -444,7 +445,7 @@ export function registerRoutes(app: Application): void {
       try {
         const filePath = resolve(join(job.result.outputDir, filename));
         const outputDirResolved = resolve(job.result.outputDir);
-        if (!filePath.startsWith(outputDirResolved + "/") && filePath !== outputDirResolved) {
+        if (!isPathInsideDir(outputDirResolved, filePath)) {
           res.status(400).json({ error: "Invalid filename" });
           return;
         }

test/agent.test.ts

@@ -1383,7 +1383,7 @@ describe("prompt construction", () => {
   it("uses repoPrompts option as override path", async () => {
     const existsSyncMock = fs.existsSync as Mock;
     existsSyncMock.mockImplementation((p: string) => {
-      return p.endsWith("/custom/my-prompts.md");
+      return p.replace(/\\/g, "/").endsWith("/custom/my-prompts.md");
     });
     (fs.readFileSync as Mock).mockReturnValue("Override prompt content");
 
@@ -1453,7 +1453,7 @@ describe("readCustomPrompt", () => {
 
   it("uses override path when provided", () => {
     (fs.existsSync as Mock).mockImplementation((p: string) =>
-      p.endsWith("/custom/prompts.md")
+      p.replace(/\\/g, "/").endsWith("/custom/prompts.md")
     );
     (fs.readFileSync as Mock).mockReturnValue("Override guidance");
 
@@ -1684,7 +1684,7 @@ describe("fast mode prompt construction", () => {
     const readFileSyncMock = fs.readFileSync as Mock;
 
     existsSyncMock.mockImplementation((p: string) => {
-      return p.endsWith("src/index.ts");
+      return p.replace(/\\/g, "/").endsWith("src/index.ts");
     });
     readFileSyncMock.mockReturnValue("export function main() {}");
 

test/e2e/helpers.ts

@@ -25,6 +25,13 @@ function buildChildEnv(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
     ...env,
   };
 
+  // On Windows, Node's os.homedir() reads USERPROFILE (not HOME). Tests that
+  // override HOME to redirect home-relative paths (e.g. ~/.cache) need the
+  // same value mirrored to USERPROFILE for the override to take effect.
+  if (process.platform === "win32" && env.HOME && !env.USERPROFILE) {
+    childEnv.USERPROFILE = env.HOME;
+  }
+
   delete childEnv.NODE_OPTIONS;
   delete childEnv.VITEST;
 

test/interactive.test.ts

@@ -3,6 +3,7 @@
  */
 
 import { describe, it, expect, vi, beforeEach } from "vitest";
+import { join } from "path";
 import type { RepoInfo, ScanResult, RepoFacts } from "../src/types.js";
 
 const mockCreateSession = vi.fn();
@@ -179,7 +180,9 @@ describe("InteractiveSession", () => {
 
     const outputPath = await session.saveTranscript("/tmp/output");
 
-    expect(outputPath).toBe("/tmp/output/TRANSCRIPT.md");
+    // saveTranscript joins the output dir + filename via path.join, which uses
+    // the platform-native separator. Compare with the same join.
+    expect(outputPath).toBe(join("/tmp/output", "TRANSCRIPT.md"));
     expect(mockWriteFile).toHaveBeenCalledWith(
       outputPath,
       expect.stringContaining("# Interactive Session Transcript"),

test/repo-resolver.test.ts

@@ -98,7 +98,9 @@ describe("resolveLocalPath", () => {
 
   it("should keep absolute paths as-is", () => {
     const result = resolveLocalPath("/tmp/test-repo");
-    expect(result).toBe("/tmp/test-repo");
+    // path.resolve normalizes to platform-native form: "/tmp/test-repo" on POSIX,
+    // "C:\\tmp\\test-repo" (or current-drive equivalent) on Windows.
+    expect(result).toBe(resolve("/tmp/test-repo"));
   });
 });
 
@@ -156,7 +158,8 @@ describe("resolveRepo", () => {
     const result = await resolveRepo(testLocalRepo);
 
     expect(result.isLocal).toBe(true);
-    expect(result.path).toBe(testLocalRepo);
+    // resolveRepo normalizes to platform-native absolute form via path.resolve.
+    expect(result.path).toBe(resolve(testLocalRepo));
     expect(result.repoName).toBe("test-local-repo-resolve");
     expect(result.repoInfo.owner).toBe("local");
     expect(result.repoInfo.repo).toBe("test-local-repo-resolve");

test/tools.test.ts

@@ -3,7 +3,7 @@
  */
 
 import { describe, it, expect, vi, beforeEach } from "vitest";
-import { join } from "path";
+import { resolve } from "path";
 
 // Mock fs/promises
 vi.mock("fs/promises", () => ({
@@ -28,9 +28,11 @@ const mockExecFile = vi.mocked(execFile);
 
 describe("safePath", () => {
   it("allows valid relative paths", () => {
-    expect(safePath("/repo", "src/index.ts")).toBe("/repo/src/index.ts");
-    expect(safePath("/repo", "package.json")).toBe("/repo/package.json");
-    expect(safePath("/repo", "src/deep/nested/file.ts")).toBe("/repo/src/deep/nested/file.ts");
+    expect(safePath("/repo", "src/index.ts")).toBe(resolve("/repo", "src/index.ts"));
+    expect(safePath("/repo", "package.json")).toBe(resolve("/repo", "package.json"));
+    expect(safePath("/repo", "src/deep/nested/file.ts")).toBe(
+      resolve("/repo", "src/deep/nested/file.ts"),
+    );
   });
 
   it("rejects traversal with ..", () => {
@@ -45,16 +47,16 @@ describe("safePath", () => {
   });
 
   it("allows paths that contain .. but stay within root", () => {
-    expect(safePath("/repo", "src/../package.json")).toBe("/repo/package.json");
+    expect(safePath("/repo", "src/../package.json")).toBe(resolve("/repo", "package.json"));
   });
 
   it("allows empty path (resolves to root)", () => {
-    expect(safePath("/repo", "")).toBe("/repo");
+    expect(safePath("/repo", "")).toBe(resolve("/repo"));
   });
 
   it("handles paths with encoded characters safely", () => {
     // These are just regular directory names, not traversal
-    expect(safePath("/repo", "dir%2F..%2Fetc")).toBe("/repo/dir%2F..%2Fetc");
+    expect(safePath("/repo", "dir%2F..%2Fetc")).toBe(resolve("/repo", "dir%2F..%2Fetc"));
   });
 });
 
@@ -109,7 +111,7 @@ describe("read_file tool", () => {
     const result = await tool.handler({ path: "src/index.ts" }, {} as any);
 
     expect(mockReadFile).toHaveBeenCalledWith(
-      join("/test/repo", "src/index.ts"),
+      resolve("/test/repo", "src/index.ts"),
       "utf-8",
     );
     expect(result).toEqual({
@@ -520,13 +522,16 @@ describe("search tool", () => {
   it("searches in a subdirectory", async () => {
     const ctx = makeContext();
     const tool = getTool(ctx, "search");
-    mockExecFileSuccess("/test/repo/src/a.ts:1:hello\n");
+    mockExecFileSuccess("src/a.ts:1:hello\n");
 
     await tool.handler({ pattern: "hello", path: "src" }, {} as any);
 
     const call = mockExecFile.mock.calls[0];
     const args = call[1] as string[];
-    expect(args).toContain(join("/test/repo", "src"));
+    // Search target is now a forward-slash relative path; cwd handles the absolute root.
+    expect(args).toContain("src");
+    const opts = call[2] as { cwd?: string } | undefined;
+    expect(opts?.cwd).toBe(resolve("/test/repo"));
   });
 
   it("returns no matches message when ripgrep exits with code 1", async () => {
@@ -568,8 +573,10 @@ describe("search tool", () => {
   it("makes paths relative in output", async () => {
     const ctx = makeContext();
     const tool = getTool(ctx, "search");
+    // Real ripgrep, with cwd set and --path-separator /, emits relative
+    // forward-slash paths. The mock reflects that contract.
     mockExecFileSuccess(
-      "/test/repo/deep/nested/file.ts:42:found it\n",
+      "deep/nested/file.ts:42:found it\n",
     );
 
     const result = await tool.handler({ pattern: "found" }, {} as any);