From 1a42040f67fbbb16570b1668f9d6af18f87480e8 Mon Sep 17 00:00:00 2001
From: Ofek Lev <ofekmeister@gmail.com>
Date: Sun, 19 Jul 2026 12:30:17 -0400
Subject: [PATCH] fix: Support Windows

---
 packages/dotagents-lib/src/index.ts           |  8 ++-
 .../dotagents-lib/src/sources/cache.test.ts   | 69 ++++++++++++++++++-
 packages/dotagents-lib/src/sources/cache.ts   | 43 ++++++++++--
 .../dotagents-lib/src/sources/local.test.ts   | 62 +++++++++++++++++
 packages/dotagents-lib/src/sources/local.ts   |  6 +-
 .../dotagents-lib/src/trust/validator.test.ts | 45 ++++++++++++
 packages/dotagents-lib/src/trust/validator.ts | 50 +++++++++++++-
 packages/dotagents-lib/src/utils/fs.test.ts   | 64 ++++++++++++++++-
 packages/dotagents-lib/src/utils/fs.ts        | 42 ++++++++++-
 .../dotagents/src/cli/commands/add.test.ts    | 12 +++-
 .../dotagents/src/cli/commands/init.test.ts   |  5 +-
 .../src/cli/commands/install-user.test.ts     | 43 +++++++-----
 .../src/cli/commands/install.test.ts          | 20 ++++--
 .../dotagents/src/cli/commands/remove.test.ts | 14 +++-
 .../dotagents/src/cli/commands/sync.test.ts   |  3 +-
 packages/dotagents/src/config/schema.test.ts  | 11 +++
 packages/dotagents/src/config/schema.ts       | 15 ++--
 packages/dotagents/src/scope.test.ts          | 17 +++--
 packages/dotagents/src/subagents/store.ts     |  6 +-
 .../dotagents/src/subagents/writer.test.ts    | 40 +++++------
 .../dotagents/src/symlinks/manager.test.ts    | 40 ++++++++++-
 packages/dotagents/src/symlinks/manager.ts    | 12 ++++
 .../src/targets/skill-symlinks.test.ts        |  6 +-
 packages/dotagents/src/utils/fs.ts            |  7 +-
 24 files changed, 555 insertions(+), 85 deletions(-)
 create mode 100644 packages/dotagents-lib/src/sources/local.test.ts

diff --git a/packages/dotagents-lib/src/index.ts b/packages/dotagents-lib/src/index.ts
index 0646b92..54fdf04 100644
--- a/packages/dotagents-lib/src/index.ts
+++ b/packages/dotagents-lib/src/index.ts
@@ -97,4 +97,10 @@ export type {
 
 // General-purpose utilities used by callers
 export { exec, ExecError } from "./utils/exec.js";
-export { copyDir, stripTrailingSlashes } from "./utils/fs.js";
+export {
+  copyDir,
+  isAbsolutePathString,
+  isInsideDir,
+  isWindowsDrivePath,
+  stripTrailingSlashes,
+} from "./utils/fs.js";
diff --git a/packages/dotagents-lib/src/sources/cache.test.ts b/packages/dotagents-lib/src/sources/cache.test.ts
index d303a3f..e7fe86c 100644
--- a/packages/dotagents-lib/src/sources/cache.test.ts
+++ b/packages/dotagents-lib/src/sources/cache.test.ts
@@ -2,7 +2,13 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest";
 import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
-import { ensureCached, validateCacheKey, CacheError } from "./cache.js";
+import {
+  ensureCached,
+  sanitizeCacheKey,
+  validateCacheKey,
+  assertUrlUsableOnPlatform,
+  CacheError,
+} from "./cache.js";
 import { exec } from "../utils/exec.js";
 
 async function configureTestGitRepo(repoDir: string): Promise<void> {
@@ -37,6 +43,67 @@ describe("validateCacheKey", () => {
   });
 });
 
+describe("sanitizeCacheKey", () => {
+  it.each([
+    ["https URL", "https://github.com/owner/repo.git", "github.com/owner/repo"],
+    ["posix absolute path", "/home/foo/skills", "home/foo/skills"],
+    ["file:// URL", "file:///home/foo/skills", "home/foo/skills"],
+    // Windows local sources: a `git:C:\...` spec reaches here as a drive-lettered
+    // path. Both separator styles must reduce to a `/`-separated relative key with
+    // the colon dropped, keeping the drive as a segment so C: and D: don't collide.
+    ["windows backslash path", "C:\\Users\\ofek\\skills", "C/Users/ofek/skills"],
+    ["windows forward-slash path", "C:/Users/ofek/skills", "C/Users/ofek/skills"],
+    ["file:// windows path", "file:///C:/Users/ofek/skills", "C/Users/ofek/skills"],
+    // A doubled separator must not let the drive letter be read as a URL
+    // scheme and stripped; a bare drive must not collapse to an empty key.
+    ["windows doubled separator", "C:\\\\Users\\\\skills", "C/Users/skills"],
+    ["bare windows drive", "C:\\\\", "C"],
+    // Drive-relative paths have no separator after the colon; the colon still
+    // has to go, or the key is an invalid dir on Windows / an scp target.
+    ["drive-relative path", "C:foo", "Cfoo"],
+    ["file:// drive-relative", "file:///C:foo", "Cfoo"],
+  ])("derives %s -> a valid relative key", (_label, url, expected) => {
+    const key = sanitizeCacheKey(url);
+    expect(key).toBe(expected);
+    // The whole point: the derived key must survive validateCacheKey so
+    // ensureCached can join it under stateDir without throwing.
+    expect(() => validateCacheKey(key)).not.toThrow();
+  });
+
+  it("keeps different drives distinct", () => {
+    expect(sanitizeCacheKey("C://team/skills")).toBe("C/team/skills");
+    expect(sanitizeCacheKey("D://team/skills")).toBe("D/team/skills");
+  });
+});
+
+describe("assertUrlUsableOnPlatform", () => {
+  it("rejects a Windows drive path off Windows", () => {
+    // Off win32 git reads `C:` as an scp host and tries to SSH to host "C";
+    // fail with a legible error instead.
+    expect(() => assertUrlUsableOnPlatform("C:\\Users\\me\\repo", "linux")).toThrow(
+      CacheError,
+    );
+    expect(() => assertUrlUsableOnPlatform("C:/Users/me/repo", "darwin")).toThrow(
+      /Windows drive path/,
+    );
+  });
+
+  it("allows a Windows drive path on Windows", () => {
+    expect(() =>
+      assertUrlUsableOnPlatform("C:\\Users\\me\\repo", "win32"),
+    ).not.toThrow();
+  });
+
+  it.each([
+    "https://example.com/repo.git",
+    "/home/me/repo",
+    "git@example.com:owner/repo.git",
+    "file:///home/me/repo",
+  ])("allows %s on any platform", (url) => {
+    expect(() => assertUrlUsableOnPlatform(url, "linux")).not.toThrow();
+  });
+});
+
 describe("ensureCached", () => {
   let tmpDir: string;
   let stateDir: string;
diff --git a/packages/dotagents-lib/src/sources/cache.ts b/packages/dotagents-lib/src/sources/cache.ts
index 4ab99ec..c42ead7 100644
--- a/packages/dotagents-lib/src/sources/cache.ts
+++ b/packages/dotagents-lib/src/sources/cache.ts
@@ -1,6 +1,7 @@
 import { join } from "node:path";
 import { mkdir, rm } from "node:fs/promises";
 import { clone, fetchAndReset, fetchRef, headCommit, headCommitDate, findCommitOlderThan, checkout, isGitRepo } from "./git.js";
+import { isAbsolutePathString, isWindowsDrivePath } from "../utils/fs.js";
 
 export class CacheError extends Error {
   constructor(message: string) {
@@ -10,19 +11,30 @@ export class CacheError extends Error {
 }
 
 /**
- * Derive a safe relative cache-key from a clone URL.
+ * Derive a safe relative cache-key from a clone URL or local path.
  *
- * Strips schemes, leading slashes (so `file:///abs/path` and `/abs/path`
- * both land under `<stateDir>/abs/path/`), and `.git` suffixes. Drops `.`
+ * Normalizes Windows separators to `/`, strips schemes, leading slashes (so
+ * `file:///abs/path` and `/abs/path` both land under `<stateDir>/abs/path/`),
+ * a Windows drive-letter colon (`C:\repo` -> `C/repo`, keeping the drive as a
+ * segment so different drives don't collide), and `.git` suffixes. Drops `.`
  * and `..` segments to keep the result inside the cache root.
  *
+ * The result always satisfies `validateCacheKey` (relative, `/`-separated, no
+ * drive prefix), so a local `git:C:\...` source on Windows caches cleanly.
+ *
  * Callers that build a cacheKey from a hosted owner/repo (e.g. GitHub) can
  * skip this and use the parsed values directly.
  */
 export function sanitizeCacheKey(url: string): string {
   const stripped = url
-    .replace(/^[a-z]+:\/\//i, "")
+    .replaceAll("\\", "/")
+    // Require 2+ scheme letters so a Windows drive (`C://...`) isn't mistaken
+    // for a URL scheme and stripped (which would drop the drive and collide).
+    .replace(/^[a-z]{2,}:\/\//i, "")
     .replace(/^\/+/, "")
+    // Drop a leading drive-letter colon (`C:` -> `C`); no lookahead, so a
+    // drive-relative `C:foo` collapses to `Cfoo` rather than leaking a colon.
+    .replace(/^([a-zA-Z]):/, "$1")
     .replace(/\.git$/, "");
   const segments = stripped
     .split("/")
@@ -43,7 +55,7 @@ export function validateCacheKey(cacheKey: string): void {
   if (!cacheKey) {
     throw new CacheError("cacheKey must be a non-empty string");
   }
-  if (cacheKey.startsWith("/") || /^[a-zA-Z]:[\\/]/.test(cacheKey)) {
+  if (isAbsolutePathString(cacheKey)) {
     throw new CacheError(`cacheKey must be a relative path, got "${cacheKey}"`);
   }
   if (cacheKey.includes("\\")) {
@@ -56,6 +68,26 @@ export function validateCacheKey(cacheKey: string): void {
   }
 }
 
+/**
+ * Reject a Windows drive path (`C:\repo`, `C:/repo`) on a non-Windows host.
+ *
+ * Only Windows git treats a leading drive letter as a local directory.
+ * Everywhere else the colon before the first slash makes `git clone` read it as
+ * scp syntax (`host:path`) and attempt an SSH connection to a host named `C`,
+ * which fails confusingly or hangs on a network timeout.
+ *
+ * The config schema stays platform-independent — it validates the *form* of a
+ * source — so this checks whether that form is usable on *this* machine.
+ */
+export function assertUrlUsableOnPlatform(url: string, platform: string): void {
+  if (platform !== "win32" && isWindowsDrivePath(url)) {
+    throw new CacheError(
+      `Source "${url}" is a Windows drive path and cannot be used on ${platform}. ` +
+        `Use a POSIX absolute path or a remote git URL.`,
+    );
+  }
+}
+
 export interface CacheResult {
   /** Path to the cached repo checkout */
   repoDir: string;
@@ -109,6 +141,7 @@ export async function ensureCached(opts: {
   /** When set, resolve to the newest commit at least this many minutes old. */
   minimumReleaseAge?: number;
 }): Promise<CacheResult> {
+  assertUrlUsableOnPlatform(opts.url, process.platform);
   validateCacheKey(opts.cacheKey);
   const repoDir = join(opts.stateDir, opts.cacheKey);
 
diff --git a/packages/dotagents-lib/src/sources/local.test.ts b/packages/dotagents-lib/src/sources/local.test.ts
new file mode 100644
index 0000000..347eae1
--- /dev/null
+++ b/packages/dotagents-lib/src/sources/local.test.ts
@@ -0,0 +1,62 @@
+import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+import { resolveLocalSource, LocalSourceError } from "./local.js";
+
+describe("resolveLocalSource", () => {
+  let root: string;
+
+  beforeEach(async () => {
+    root = await mkdtemp(join(tmpdir(), "dotagents-local-"));
+    await mkdir(join(root, "skills", "review"), { recursive: true });
+  });
+
+  afterEach(async () => {
+    await rm(root, { recursive: true, force: true });
+  });
+
+  it("resolves a nested child directory", async () => {
+    // The containment check must be separator-agnostic: on Windows `root`
+    // uses `\`, so a `${root}/` string prefix would never match this child.
+    const resolved = await resolveLocalSource(root, "skills/review");
+    expect(resolved).toBe(join(root, "skills", "review"));
+  });
+
+  it("resolves the project root itself", async () => {
+    expect(await resolveLocalSource(root, ".")).toBe(root);
+  });
+
+  it("rejects a parent-traversal path that escapes the root", async () => {
+    await expect(resolveLocalSource(root, "../escape")).rejects.toThrow(
+      LocalSourceError,
+    );
+  });
+
+  it("rejects the bare parent directory", async () => {
+    // Covers the `rel === ".."` guard specifically — the other traversal cases
+    // all produce `..${sep}<child>` and only exercise the startsWith branch.
+    await expect(resolveLocalSource(root, "..")).rejects.toThrow(
+      /resolves outside project root/,
+    );
+  });
+
+  it("rejects traversal that dips out and back in", async () => {
+    await expect(
+      resolveLocalSource(root, "skills/../../elsewhere"),
+    ).rejects.toThrow(/resolves outside project root/);
+  });
+
+  it("errors when the source does not exist", async () => {
+    await expect(resolveLocalSource(root, "skills/missing")).rejects.toThrow(
+      /not found/,
+    );
+  });
+
+  it("errors when the source is a file, not a directory", async () => {
+    await writeFile(join(root, "afile.md"), "not a dir\n");
+    await expect(resolveLocalSource(root, "afile.md")).rejects.toThrow(
+      /not a directory/,
+    );
+  });
+});
diff --git a/packages/dotagents-lib/src/sources/local.ts b/packages/dotagents-lib/src/sources/local.ts
index 10bdfc2..88c497b 100644
--- a/packages/dotagents-lib/src/sources/local.ts
+++ b/packages/dotagents-lib/src/sources/local.ts
@@ -1,5 +1,6 @@
 import { resolve } from "node:path";
 import { stat } from "node:fs/promises";
+import { isInsideDir } from "../utils/fs.js";
 
 export class LocalSourceError extends Error {
   constructor(message: string) {
@@ -16,11 +17,10 @@ export async function resolveLocalSource(
   projectRoot: string,
   relativePath: string,
 ): Promise<string> {
-  const absRoot = resolve(projectRoot);
   const absPath = resolve(projectRoot, relativePath);
 
-  // Prevent path traversal outside the project root
-  if (!absPath.startsWith(`${absRoot}/`) && absPath !== absRoot) {
+  // Prevent path traversal outside the project root.
+  if (!isInsideDir(projectRoot, absPath)) {
     throw new LocalSourceError(
       `Local source "${relativePath}" resolves outside project root`,
     );
diff --git a/packages/dotagents-lib/src/trust/validator.test.ts b/packages/dotagents-lib/src/trust/validator.test.ts
index e06bcbd..d752b55 100644
--- a/packages/dotagents-lib/src/trust/validator.test.ts
+++ b/packages/dotagents-lib/src/trust/validator.test.ts
@@ -141,6 +141,51 @@ describe("validateTrustedSource", () => {
     });
   });
 
+  describe("local filesystem git: sources", () => {
+    // These read files already on the user's disk — the same trust surface as
+    // `path:` — and expose no domain, so gating them on `git_domains` would
+    // make them impossible to allow at all.
+    const trust = makeTrust({ git_domains: ["git.corp.example.com"] });
+
+    it.each([
+      "git:/home/me/local-repo",
+      "git:C:\\Users\\me\\local-repo",
+      "git:C:/Users/me/local-repo",
+      "git:file:///home/me/local-repo",
+    ])("allows %s with no matching domain rule", (source) => {
+      expect(() => validateTrustedSource(source, trust)).not.toThrow();
+    });
+
+    it("still gates a file:// URL that names a host", () => {
+      expect(() =>
+        validateTrustedSource("git:file://evil.com/share/repo", trust),
+      ).toThrow(TrustError);
+    });
+
+    it.each([
+      // UNC is a remote host in absolute-path clothing: `git:` followed by
+      // `//server/...`, any longer run of leading separators, a host-empty
+      // `file:` URL whose path is UNC, and the backslash spelling. Letting any
+      // of these through the local bypass would reach SMB with no git_domains
+      // entry — on Windows, a credential-exposure vector.
+      "git://server/share/repo",
+      "git://///server/share/repo",
+      "git:file:////server/share/repo",
+      "git:\\\\server\\share\\repo",
+    ])("does not treat UNC path %s as local", (source) => {
+      expect(() => validateTrustedSource(source, trust)).toThrow(TrustError);
+    });
+
+    it("still gates remote sources and bare relative paths", () => {
+      expect(() =>
+        validateTrustedSource("git:https://evil.com/repo.git", trust),
+      ).toThrow(TrustError);
+      expect(() =>
+        validateTrustedSource("git:relative/path", trust),
+      ).toThrow(TrustError);
+    });
+  });
+
   describe("git_domains with path prefixes", () => {
     it("allows repos under a trusted domain path (https)", () => {
       const trust = makeTrust({ git_domains: ["gitlab.com/myorg"] });
diff --git a/packages/dotagents-lib/src/trust/validator.ts b/packages/dotagents-lib/src/trust/validator.ts
index 3621b7c..5291316 100644
--- a/packages/dotagents-lib/src/trust/validator.ts
+++ b/packages/dotagents-lib/src/trust/validator.ts
@@ -1,4 +1,5 @@
 import { parseSource } from "../skills/resolver.js";
+import { isAbsolutePathString } from "../utils/fs.js";
 import type { TrustPolicy } from "./policy.js";
 
 /**
@@ -101,6 +102,47 @@ export function extractDomainPath(url: string): string | undefined {
   }
 }
 
+/**
+ * Whether `s` is a UNC network path (`\\server\share`, `//server/share`).
+ *
+ * UNC looks absolute but names a remote host reached over SMB, which is exactly
+ * what `git_domains` exists to gate. Matches any run of two or more leading
+ * separators, in either slash direction.
+ */
+function isUncPath(s: string): boolean {
+  return /^[\\/]{2}/.test(s);
+}
+
+/**
+ * Whether a `git:` URL points at the local filesystem rather than a remote host.
+ *
+ * Local git sources read files the user already has on disk — the same trust
+ * surface as a `path:` source, which is unconditionally allowed — and they
+ * expose no domain for `git_domains` to match, so gating them on domain would
+ * make them impossible to allow at all (short of `allow_all`).
+ *
+ * Only absolute *local* forms count: a bare relative path and any UNC network
+ * path stay subject to the domain rules so neither can be smuggled through.
+ */
+function isLocalFilesystemUrl(url: string): boolean {
+  // Reject UNC before the absolute-path check, which would otherwise accept
+  // `//server/share` on its leading slash.
+  if (isUncPath(url)) {return false;}
+  if (isAbsolutePathString(url)) {return true;}
+  // `file:///abs/path` is local. `file://host/share` names a host, and
+  // `file:////server/share` has an empty host but a UNC path — both stay
+  // subject to the domain rules below.
+  if (/^file:\/\//i.test(url)) {
+    try {
+      const { host, pathname } = new URL(url);
+      return !host && !isUncPath(pathname);
+    } catch {
+      return false;
+    }
+  }
+  return false;
+}
+
 function formatAllowed(trust: TrustPolicy): string {
   const parts: string[] = [];
   if (trust.github_orgs.length > 0) {
@@ -120,7 +162,8 @@ function formatAllowed(trust: TrustPolicy): string {
  *
  * - No trust config → allow all (backward compat)
  * - allow_all = true → allow all
- * - Local path: sources → always allowed
+ * - Local sources → always allowed: `path:`, and `git:` pointing at an
+ *   absolute local path (`git:/abs/repo`, `git:C:\repo`, `file:///abs/repo`)
  * - Otherwise → must match at least one rule (org, repo, or domain)
  */
 export function validateTrustedSource(
@@ -158,6 +201,11 @@ export function validateTrustedSource(
   }
 
   if (parsed.type === "git" || parsed.type === "well-known") {
+    // A `git:` source pointing at a local absolute path has no domain to match,
+    // and the same trust surface as `path:` — allow it rather than throwing an
+    // error the user has no `git_domains` entry to fix.
+    if (parsed.type === "git" && isLocalFilesystemUrl(parsed.url!)) {return;}
+
     const domainPath = extractDomainPath(parsed.url!)?.toLowerCase();
     if (domainPath && trust.git_domains.some((d) => {
       const entry = d.toLowerCase();
diff --git a/packages/dotagents-lib/src/utils/fs.test.ts b/packages/dotagents-lib/src/utils/fs.test.ts
index fda74ae..3570504 100644
--- a/packages/dotagents-lib/src/utils/fs.test.ts
+++ b/packages/dotagents-lib/src/utils/fs.test.ts
@@ -1,9 +1,69 @@
 import { describe, it, expect, beforeEach, afterEach } from "vitest";
 import { mkdtemp, writeFile, mkdir, rm } from "node:fs/promises";
-import { join } from "node:path";
+import { join, resolve } from "node:path";
 import { tmpdir } from "node:os";
 import { existsSync } from "node:fs";
-import { copyDir } from "./fs.js";
+import {
+  copyDir,
+  isAbsolutePathString,
+  isInsideDir,
+  isWindowsDrivePath,
+} from "./fs.js";
+
+describe("isWindowsDrivePath / isAbsolutePathString", () => {
+  it.each(["C:\\repo", "C:/repo", "z:\\repo"])(
+    "treats %s as a drive root",
+    (s) => {
+      expect(isWindowsDrivePath(s)).toBe(true);
+      expect(isAbsolutePathString(s)).toBe(true);
+    },
+  );
+
+  it.each([
+    // Drive-*relative*: no separator after the colon, so not a root. This
+    // boundary is load-bearing — the config schema rejects it, and the cache
+    // must not mistake it for a drive it can strip.
+    "C:repo",
+    "repo",
+    "./repo",
+    "github.com/owner/repo",
+    "https://example.com/repo.git",
+  ])("does not treat %s as a drive root", (s) => {
+    expect(isWindowsDrivePath(s)).toBe(false);
+  });
+
+  it("counts a POSIX absolute path as absolute but not as a drive path", () => {
+    expect(isWindowsDrivePath("/repo")).toBe(false);
+    expect(isAbsolutePathString("/repo")).toBe(true);
+  });
+
+  it("does not count relative paths as absolute", () => {
+    expect(isAbsolutePathString("repo/skills")).toBe(false);
+    expect(isAbsolutePathString("C:repo")).toBe(false);
+  });
+});
+
+describe("isInsideDir", () => {
+  const root = resolve("/project");
+
+  it("accepts the root itself and paths beneath it", () => {
+    expect(isInsideDir(root, root)).toBe(true);
+    expect(isInsideDir(root, join(root, "skills"))).toBe(true);
+    expect(isInsideDir(root, join(root, "skills", "review"))).toBe(true);
+  });
+
+  it("rejects the parent, siblings, and paths that escape", () => {
+    expect(isInsideDir(root, resolve(root, ".."))).toBe(false);
+    expect(isInsideDir(root, resolve(root, "../sibling"))).toBe(false);
+    expect(isInsideDir(root, resolve(root, "skills/../../escape"))).toBe(false);
+  });
+
+  it("accepts a child whose name merely starts with ..", () => {
+    // A bare `rel.startsWith("..")` check misreads this as a traversal; the
+    // whole-segment comparison is what keeps it a legitimate child.
+    expect(isInsideDir(root, join(root, "..foo"))).toBe(true);
+  });
+});
 
 describe("copyDir", () => {
   let srcDir: string;
diff --git a/packages/dotagents-lib/src/utils/fs.ts b/packages/dotagents-lib/src/utils/fs.ts
index 7854cc1..101edfc 100644
--- a/packages/dotagents-lib/src/utils/fs.ts
+++ b/packages/dotagents-lib/src/utils/fs.ts
@@ -1,5 +1,5 @@
 import { cp, rm } from "node:fs/promises";
-import { basename } from "node:path";
+import { basename, isAbsolute, relative, resolve, sep } from "node:path";
 
 /**
  * Copy a directory recursively, excluding .git/.
@@ -13,6 +13,46 @@ export async function copyDir(src: string, dest: string): Promise<void> {
   });
 }
 
+/**
+ * Whether `s` starts with a Windows drive root (`C:\repo`, `C:/repo`).
+ *
+ * Hand-rolled rather than `path.win32.isAbsolute` because that also accepts a
+ * bare leading separator and UNC (`\\server\share`), which callers here treat
+ * differently. Drive-*relative* forms (`C:repo`, no separator) are not roots
+ * and are deliberately excluded.
+ */
+export function isWindowsDrivePath(s: string): boolean {
+  return /^[a-zA-Z]:[\\/]/.test(s);
+}
+
+/**
+ * Whether `s` is an absolute filesystem path in either convention — POSIX
+ * (`/repo`) or a Windows drive root (`C:\repo`).
+ *
+ * `path.isAbsolute` is platform-dependent (it returns false for `C:\repo` when
+ * running on POSIX), so anything that must classify a path string identically
+ * on every host — config validation, trust, cache keys — uses this instead.
+ */
+export function isAbsolutePathString(s: string): boolean {
+  return s.startsWith("/") || isWindowsDrivePath(s);
+}
+
+/**
+ * Whether `target` is `root` itself or a path beneath it.
+ *
+ * Compares with path.relative rather than a string prefix so the check is
+ * separator-agnostic (on Windows `root` uses `\`, so a `${root}/` prefix would
+ * never match a real child), and matches whole `..` segments so a legitimate
+ * child named `..foo` is not mistaken for a traversal.
+ *
+ * Shared by every "does this resolved path escape its root" guard so the
+ * predicate can't drift between call sites.
+ */
+export function isInsideDir(root: string, target: string): boolean {
+  const rel = relative(resolve(root), resolve(target));
+  return rel !== ".." && !rel.startsWith(`..${sep}`) && !isAbsolute(rel);
+}
+
 /** Strip trailing `/` characters from a string. */
 export function stripTrailingSlashes(s: string): string {
   let end = s.length;
diff --git a/packages/dotagents/src/cli/commands/add.test.ts b/packages/dotagents/src/cli/commands/add.test.ts
index 604f08c..f0f00c3 100644
--- a/packages/dotagents/src/cli/commands/add.test.ts
+++ b/packages/dotagents/src/cli/commands/add.test.ts
@@ -26,6 +26,14 @@ description: Test skill ${name}
 # ${name}
 `;
 
+/**
+ * A raw Windows path can't embed in a TOML basic string (backslashes are escape
+ * chars); POSIX separators stay a valid git/fs path on Windows. No-op on POSIX.
+ */
+function posixPath(p: string): string {
+  return p.replaceAll("\\", "/");
+}
+
 describe("runAdd", () => {
   let tmpDir: string;
   let stateDir: string;
@@ -36,7 +44,7 @@ describe("runAdd", () => {
     tmpDir = await mkdtemp(join(tmpdir(), "dotagents-add-"));
     stateDir = join(tmpDir, "state");
     projectRoot = join(tmpDir, "project");
-    repoDir = join(tmpDir, "repo");
+    repoDir = posixPath(join(tmpDir, "repo"));
 
     process.env["DOTAGENTS_STATE_DIR"] = stateDir;
 
@@ -495,7 +503,7 @@ describe("add() CLI parsing", () => {
     tmpDir = await mkdtemp(join(tmpdir(), "dotagents-add-cli-"));
     stateDir = join(tmpDir, "state");
     projectRoot = join(tmpDir, "project");
-    repoDir = join(tmpDir, "repo");
+    repoDir = posixPath(join(tmpDir, "repo"));
 
     process.env["DOTAGENTS_STATE_DIR"] = stateDir;
     originalExitCode = process.exitCode;
diff --git a/packages/dotagents/src/cli/commands/init.test.ts b/packages/dotagents/src/cli/commands/init.test.ts
index cd0e097..07126c5 100644
--- a/packages/dotagents/src/cli/commands/init.test.ts
+++ b/packages/dotagents/src/cli/commands/init.test.ts
@@ -232,7 +232,10 @@ describe("installPostMergeHook", () => {
     expect(content).toContain("dotagents:post-merge");
   });
 
-  it("makes hook executable", async () => {
+  // Windows has no executable bit — chmod(0o755) is a no-op there and the mode
+  // always reads back 0. Nothing is broken: Git for Windows runs hooks through
+  // sh regardless. The property simply doesn't exist to assert.
+  it.skipIf(process.platform === "win32")("makes hook executable", async () => {
     await installPostMergeHook(gitDir);
 
     const stat = await lstat(join(gitDir, "hooks", "post-merge"));
diff --git a/packages/dotagents/src/cli/commands/install-user.test.ts b/packages/dotagents/src/cli/commands/install-user.test.ts
index 1c4ee71..90ef7d9 100644
--- a/packages/dotagents/src/cli/commands/install-user.test.ts
+++ b/packages/dotagents/src/cli/commands/install-user.test.ts
@@ -2,7 +2,7 @@ import { describe, it, expect, afterEach, vi } from "vitest";
 import { mkdtemp, mkdir, readFile, readlink, rm, writeFile, lstat } from "node:fs/promises";
 import { existsSync } from "node:fs";
 import { join, relative } from "node:path";
-import { tmpdir } from "node:os";
+import { homedir, tmpdir } from "node:os";
 
 const SKILL_MD = `---
 name: pdf
@@ -14,25 +14,23 @@ description: Test skill pdf
 
 describe("runInstall user scope", () => {
   let tmpDir: string | undefined;
-  const previousHome = process.env["HOME"];
-  const previousDotagentsHome = process.env["DOTAGENTS_HOME"];
-  const previousStateDir = process.env["DOTAGENTS_STATE_DIR"];
+  // USERPROFILE matters as much as HOME: on Windows `os.homedir()` reads
+  // USERPROFILE and ignores HOME, so overriding HOME alone leaves the target
+  // definitions pointing at the developer's real home.
+  const previousEnv = {
+    HOME: process.env["HOME"],
+    USERPROFILE: process.env["USERPROFILE"],
+    DOTAGENTS_HOME: process.env["DOTAGENTS_HOME"],
+    DOTAGENTS_STATE_DIR: process.env["DOTAGENTS_STATE_DIR"],
+  };
 
   afterEach(async () => {
-    if (previousHome === undefined) {
-      delete process.env["HOME"];
-    } else {
-      process.env["HOME"] = previousHome;
-    }
-    if (previousDotagentsHome === undefined) {
-      delete process.env["DOTAGENTS_HOME"];
-    } else {
-      process.env["DOTAGENTS_HOME"] = previousDotagentsHome;
-    }
-    if (previousStateDir === undefined) {
-      delete process.env["DOTAGENTS_STATE_DIR"];
-    } else {
-      process.env["DOTAGENTS_STATE_DIR"] = previousStateDir;
+    for (const [key, value] of Object.entries(previousEnv)) {
+      if (value === undefined) {
+        delete process.env[key];
+      } else {
+        process.env[key] = value;
+      }
     }
     vi.resetModules();
 
@@ -50,8 +48,17 @@ describe("runInstall user scope", () => {
     const sourceDir = join(dotagentsHome, "skill-source", "pdf");
 
     process.env["HOME"] = homeDir;
+    process.env["USERPROFILE"] = homeDir;
     process.env["DOTAGENTS_HOME"] = dotagentsHome;
     process.env["DOTAGENTS_STATE_DIR"] = stateDir;
+
+    // Hard stop before anything touches the filesystem. The agent target
+    // definitions resolve user dirs from `os.homedir()` at module load, and
+    // runInstall *moves* an existing skills directory aside (rename) and then
+    // deletes it. If this sandbox ever fails again, that destroys the real
+    // ~/.claude/skills — fail the test instead of the developer's machine.
+    expect(homedir()).toBe(homeDir);
+
     vi.resetModules();
 
     const [{ runInstall }, { resolveScope }, { loadLockfile }] = await Promise.all([
diff --git a/packages/dotagents/src/cli/commands/install.test.ts b/packages/dotagents/src/cli/commands/install.test.ts
index 9310e91..cbd0cfb 100644
--- a/packages/dotagents/src/cli/commands/install.test.ts
+++ b/packages/dotagents/src/cli/commands/install.test.ts
@@ -28,6 +28,14 @@ description: Review code for correctness.
 Review the current diff.
 `;
 
+/**
+ * A raw Windows path can't embed in a TOML basic string (backslashes are escape
+ * chars); POSIX separators stay a valid git/fs path on Windows. No-op on POSIX.
+ */
+function posixPath(p: string): string {
+  return p.replaceAll("\\", "/");
+}
+
 async function initTestGitRepo(repoDir: string): Promise<void> {
   await exec("git", ["init"], { cwd: repoDir });
   await exec("git", ["config", "user.email", "test@test.com"], { cwd: repoDir });
@@ -46,7 +54,7 @@ describe("runInstall", () => {
     tmpDir = await mkdtemp(join(tmpdir(), "dotagents-install-"));
     stateDir = join(tmpDir, "state");
     projectRoot = join(tmpDir, "project");
-    repoDir = join(tmpDir, "repo");
+    repoDir = posixPath(join(tmpDir, "repo"));
     repoInitialized = false;
 
     process.env["DOTAGENTS_STATE_DIR"] = stateDir;
@@ -472,7 +480,11 @@ path = "code-reviewer.md"
     expect(existsSync(join(projectRoot, ".agents", "skills", "pdf", "SKILL.md"))).toBe(true);
   });
 
-  it("does not write the lockfile when stale subagent pruning fails", async () => {
+  // Needs chmod(0o000) to make the prune's rm() fail, which only bites on
+  // POSIX. The usual Windows substitute — putting a directory at the path —
+  // doesn't work here: pruneManagedFiles skips non-files before it ever
+  // reaches the rm, so nothing fails and install succeeds.
+  it.skipIf(process.platform === "win32")("does not write the lockfile when stale subagent pruning fails", async () => {
     const sourceDir = join(projectRoot, "agents");
     await mkdir(sourceDir, { recursive: true });
     await writeFile(join(sourceDir, "code-reviewer.md"), SUBAGENT_MD("code-reviewer"));
@@ -930,7 +942,7 @@ path = "reviewer.md"
 
   it("errors on name conflict between two wildcard sources", async () => {
     // Create a second repo that also has a "pdf" skill
-    const repoDir2 = join(tmpDir, "repo2");
+    const repoDir2 = posixPath(join(tmpDir, "repo2"));
     await mkdir(repoDir2, { recursive: true });
     await initTestGitRepo(repoDir2);
     await mkdir(join(repoDir2, "pdf"), { recursive: true });
@@ -985,7 +997,7 @@ path = "reviewer.md"
 
   it("prunes stale managed skills whose source does not match a wildcard", async () => {
     // Create a second repo with a "helper" skill
-    const repoDir2 = join(tmpDir, "repo2");
+    const repoDir2 = posixPath(join(tmpDir, "repo2"));
     await mkdir(repoDir2, { recursive: true });
     await initTestGitRepo(repoDir2);
     await mkdir(join(repoDir2, "helper"), { recursive: true });
diff --git a/packages/dotagents/src/cli/commands/remove.test.ts b/packages/dotagents/src/cli/commands/remove.test.ts
index 9a7332b..85e691e 100644
--- a/packages/dotagents/src/cli/commands/remove.test.ts
+++ b/packages/dotagents/src/cli/commands/remove.test.ts
@@ -17,6 +17,14 @@ description: Test skill ${name}
 ---
 `;
 
+/**
+ * A raw Windows path can't embed in a TOML basic string (backslashes are escape
+ * chars); POSIX separators stay a valid git/fs path on Windows. No-op on POSIX.
+ */
+function posixPath(p: string): string {
+  return p.replaceAll("\\", "/");
+}
+
 describe("runRemove", () => {
   let tmpDir: string;
   let stateDir: string;
@@ -27,7 +35,7 @@ describe("runRemove", () => {
     tmpDir = await mkdtemp(join(tmpdir(), "dotagents-remove-"));
     stateDir = join(tmpDir, "state");
     projectRoot = join(tmpDir, "project");
-    repoDir = join(tmpDir, "repo");
+    repoDir = posixPath(join(tmpDir, "repo"));
 
     process.env["DOTAGENTS_STATE_DIR"] = stateDir;
 
@@ -168,8 +176,8 @@ describe("runRemoveSource", () => {
     tmpDir = await mkdtemp(join(tmpdir(), "dotagents-remove-source-"));
     stateDir = join(tmpDir, "state");
     projectRoot = join(tmpDir, "project");
-    repoDir = join(tmpDir, "repo");
-    otherRepoDir = join(tmpDir, "other-repo");
+    repoDir = posixPath(join(tmpDir, "repo"));
+    otherRepoDir = posixPath(join(tmpDir, "other-repo"));
 
     process.env["DOTAGENTS_STATE_DIR"] = stateDir;
 
diff --git a/packages/dotagents/src/cli/commands/sync.test.ts b/packages/dotagents/src/cli/commands/sync.test.ts
index 39ca9be..8bd8ad6 100644
--- a/packages/dotagents/src/cli/commands/sync.test.ts
+++ b/packages/dotagents/src/cli/commands/sync.test.ts
@@ -185,7 +185,8 @@ describe("runSync", () => {
   });
 
   it("prunes stale managed skills after a collaborator removes the dependency and another collaborator pulls", async () => {
-    const skillRepo = join(tmpDir, "skill-repo");
+    // POSIX separators: a raw Windows path can't embed in a TOML basic string.
+    const skillRepo = join(tmpDir, "skill-repo").replaceAll("\\", "/");
     const projectOrigin = join(tmpDir, "project-origin.git");
     const projectSeed = join(tmpDir, "project-seed");
     const aliceRepo = join(tmpDir, "alice");
diff --git a/packages/dotagents/src/config/schema.test.ts b/packages/dotagents/src/config/schema.test.ts
index ddfcf73..35a1d73 100644
--- a/packages/dotagents/src/config/schema.test.ts
+++ b/packages/dotagents/src/config/schema.test.ts
@@ -110,6 +110,11 @@ describe("agentsConfigSchema", () => {
       expect(parseSkill("git:/tmp/local-repo").success).toBe(true);
     });
 
+    it("accepts git: source with a Windows drive path", () => {
+      expect(parseSkill("git:C:\\Users\\me\\local-repo").success).toBe(true);
+      expect(parseSkill("git:C:/Users/me/local-repo").success).toBe(true);
+    });
+
     it("rejects git: source without protocol", () => {
       expect(parseSkill("git:--upload-pack=evil").success).toBe(false);
     });
@@ -118,6 +123,12 @@ describe("agentsConfigSchema", () => {
       expect(parseSkill("git:relative/path").success).toBe(false);
     });
 
+    it("rejects git: source with a drive-relative path (no separator)", () => {
+      // `C:evil` is drive-relative, not absolute — treat it like a bare
+      // relative path so a leading-dash payload can't sneak past the guard.
+      expect(parseSkill("git:C:relative").success).toBe(false);
+    });
+
     it("accepts path: source", () => {
       expect(parseSkill("path:../relative/dir").success).toBe(true);
     });
diff --git a/packages/dotagents/src/config/schema.ts b/packages/dotagents/src/config/schema.ts
index 933deec..76f7eed 100644
--- a/packages/dotagents/src/config/schema.ts
+++ b/packages/dotagents/src/config/schema.ts
@@ -4,6 +4,7 @@ import {
   GITHUB_SSH_URL,
   GITLAB_HTTPS_URL,
   GITLAB_SSH_URL,
+  isAbsolutePathString,
   type RepositorySource,
 } from "@sentry/dotagents-lib";
 
@@ -24,13 +25,19 @@ export type { RepositorySource };
  *   git:https://...     -- non-GitHub git
  *   path:../relative    -- local filesystem
  */
-const GIT_URL_VALID = /^git:(https:\/\/|git:\/\/|ssh:\/\/|git@|file:\/\/|\/)/;
+// A `git:` value must name a known protocol or an absolute path — never a bare
+// relative path — so a leading `-` can never be forwarded to `git clone` as an
+// argument (injection guard). `isAbsolutePathString` covers POSIX (`/repo`) and
+// Windows drive (`C:\repo`) locals identically on every host, and is the same
+// predicate trust uses to classify a source as local, so what this layer admits
+// and what that layer can classify cannot drift apart.
+const GIT_SCHEME_VALID = /^(https:\/\/|git:\/\/|ssh:\/\/|git@|file:\/\/)/;
 
 const skillSourceSchema = z.string().check(
   z.refine((s) => {
     if (s.startsWith("git:")) {
-      // Require a valid protocol scheme or absolute path to prevent argument injection
-      return GIT_URL_VALID.test(s);
+      const url = s.slice(4);
+      return GIT_SCHEME_VALID.test(url) || isAbsolutePathString(url);
     }
     if (s.startsWith("path:")) {return true;}
     // GitHub HTTPS or SSH URLs
@@ -50,7 +57,7 @@ const skillSourceSchema = z.string().check(
       parts.length === 2 &&
       parts.every((p) => p.length > 0 && !p.startsWith("-"))
     );
-  }, "Must be owner/repo, owner/repo@ref, GitHub/GitLab URL, https://<domain> (well-known), git:<url> (with https/git/ssh protocol), or path:<relative>"),
+  }, "Must be owner/repo, owner/repo@ref, GitHub/GitLab URL, https://<domain> (well-known), git:<url> (https/git/ssh/file protocol or absolute path), or path:<relative>"),
 );
 
 export type SkillSource = z.infer<typeof skillSourceSchema>;
diff --git a/packages/dotagents/src/scope.test.ts b/packages/dotagents/src/scope.test.ts
index bd63381..28bc4d3 100644
--- a/packages/dotagents/src/scope.test.ts
+++ b/packages/dotagents/src/scope.test.ts
@@ -10,13 +10,16 @@ describe("resolveScope", () => {
   });
 
   it("project scope uses projectRoot", () => {
-    const s = resolveScope("project", "/tmp/my-project");
+    // Derived paths come from path.join, so build the expectations the same
+    // way — hardcoded "/" literals only hold on POSIX.
+    const root = "/tmp/my-project";
+    const s = resolveScope("project", root);
     expect(s.scope).toBe("project");
-    expect(s.root).toBe("/tmp/my-project");
-    expect(s.agentsDir).toBe("/tmp/my-project/.agents");
-    expect(s.configPath).toBe("/tmp/my-project/agents.toml");
-    expect(s.lockPath).toBe("/tmp/my-project/agents.lock");
-    expect(s.skillsDir).toBe("/tmp/my-project/.agents/skills");
+    expect(s.root).toBe(root);
+    expect(s.agentsDir).toBe(join(root, ".agents"));
+    expect(s.configPath).toBe(join(root, "agents.toml"));
+    expect(s.lockPath).toBe(join(root, "agents.lock"));
+    expect(s.skillsDir).toBe(join(root, ".agents", "skills"));
   });
 
   it("user scope uses ~/.agents by default", () => {
@@ -35,7 +38,7 @@ describe("resolveScope", () => {
     const s = resolveScope("user");
     expect(s.root).toBe("/tmp/fake-home");
     expect(s.agentsDir).toBe("/tmp/fake-home");
-    expect(s.skillsDir).toBe("/tmp/fake-home/skills");
+    expect(s.skillsDir).toBe(join("/tmp/fake-home", "skills"));
   });
 
   it("user scope: agentsDir equals root (flat layout)", () => {
diff --git a/packages/dotagents/src/subagents/store.ts b/packages/dotagents/src/subagents/store.ts
index 7307f98..f07133b 100644
--- a/packages/dotagents/src/subagents/store.ts
+++ b/packages/dotagents/src/subagents/store.ts
@@ -1,10 +1,11 @@
 import { existsSync } from "node:fs";
 import { mkdir, readdir, readFile, rename, rm, writeFile } from "node:fs/promises";
-import { basename, extname, isAbsolute, join, relative, resolve } from "node:path";
+import { basename, extname, join, relative, resolve } from "node:path";
 import { parse as parseTOML } from "smol-toml";
 import {
   applyDefaultRepositorySource,
   ensureCached,
+  isInsideDir,
   isSourceExcluded,
   loadMarkdownFrontmatter,
   parseSource,
@@ -261,8 +262,7 @@ async function discoverSubagent(
   if (config.path) {
     const sourceRoot = resolve(sourceDir);
     const filePath = resolve(sourceRoot, config.path);
-    const relPath = relative(sourceRoot, filePath);
-    if (relPath.startsWith("..") || isAbsolute(relPath)) {
+    if (!isInsideDir(sourceRoot, filePath)) {
       throw new Error(`Subagent path resolves outside source: ${config.path}`);
     }
     const nativeTarget = inferNativeTarget(config.path);
diff --git a/packages/dotagents/src/subagents/writer.test.ts b/packages/dotagents/src/subagents/writer.test.ts
index 53be208..efc2ab6 100644
--- a/packages/dotagents/src/subagents/writer.test.ts
+++ b/packages/dotagents/src/subagents/writer.test.ts
@@ -666,28 +666,26 @@ describe("reconcileSubagentConfigs", () => {
   });
 
   it("reports unreadable configs during inspection and fails during apply", async () => {
+    // Put a directory at the config path: existsSync still passes, but readFile
+    // fails with EISDIR on every platform. chmod(0o000) would be POSIX-only —
+    // Windows ignores the mode, reads the file, and lands on the "not managed"
+    // branch instead of the read-failure one this test is about.
     const filePath = join(dir, ".claude", "agents", "code-reviewer.md");
-    await mkdir(join(dir, ".claude", "agents"), { recursive: true });
-    await writeFile(filePath, `# ${DOTAGENTS_SUBAGENT_MARKER}\n`, "utf-8");
-    await chmod(filePath, 0o000);
+    await mkdir(filePath, { recursive: true });
 
-    try {
-      const inspected = await reconcileSubagentConfigs(
-        ["claude"],
-        [SUBAGENT],
-        projectSubagentResolver(dir),
-        { mode: "inspect" },
-      );
-      expect(inspected.issues[0]!.issue).toContain("Failed to read");
-
-      await expect(reconcileSubagentConfigs(
-        ["claude"],
-        [SUBAGENT],
-        projectSubagentResolver(dir),
-        { mode: "apply" },
-      )).rejects.toThrow();
-    } finally {
-      await chmod(filePath, 0o600);
-    }
+    const inspected = await reconcileSubagentConfigs(
+      ["claude"],
+      [SUBAGENT],
+      projectSubagentResolver(dir),
+      { mode: "inspect" },
+    );
+    expect(inspected.issues[0]!.issue).toContain("Failed to read");
+
+    await expect(reconcileSubagentConfigs(
+      ["claude"],
+      [SUBAGENT],
+      projectSubagentResolver(dir),
+      { mode: "apply" },
+    )).rejects.toThrow();
   });
 });
diff --git a/packages/dotagents/src/symlinks/manager.test.ts b/packages/dotagents/src/symlinks/manager.test.ts
index 6afa6df..a455cd8 100644
--- a/packages/dotagents/src/symlinks/manager.test.ts
+++ b/packages/dotagents/src/symlinks/manager.test.ts
@@ -5,6 +5,7 @@ import {
   mkdir,
   symlink,
   writeFile,
+  readFile,
   lstat,
   readlink,
   readdir,
@@ -40,7 +41,10 @@ describe("symlinks", () => {
       expect(stat.isSymbolicLink()).toBe(true);
 
       const linkTarget = await readlink(join(targetDir, "skills"));
-      expect(linkTarget).toBe("../.agents/skills");
+      // The link target comes from path.relative, so it uses the platform
+      // separator — `..\.agents\skills` on Windows, which readlink returns
+      // verbatim. verifySymlinks compares against the same computation.
+      expect(linkTarget).toBe(join("..", ".agents", "skills"));
     });
 
     it("creates symlink when target dir exists but skills/ does not", async () => {
@@ -75,7 +79,10 @@ describe("symlinks", () => {
       expect(result.created).toBe(true);
 
       const linkTarget = await readlink(join(targetDir, "skills"));
-      expect(linkTarget).toBe("../.agents/skills");
+      // The link target comes from path.relative, so it uses the platform
+      // separator — `..\.agents\skills` on Windows, which readlink returns
+      // verbatim. verifySymlinks compares against the same computation.
+      expect(linkTarget).toBe(join("..", ".agents", "skills"));
     });
 
     it("migrates existing real directory", async () => {
@@ -100,6 +107,35 @@ describe("symlinks", () => {
       expect(stat.isSymbolicLink()).toBe(true);
     });
 
+    it("refuses to delete skills that collide with an existing managed skill", async () => {
+      const targetDir = join(dir, ".claude");
+      const realSkillsDir = join(targetDir, "skills");
+
+      // "pdf" exists on both sides, so it cannot be migrated...
+      await mkdir(join(realSkillsDir, "pdf"), { recursive: true });
+      await writeFile(join(realSkillsDir, "pdf", "SKILL.md"), "local copy\n");
+      await mkdir(join(agentsDir, "skills", "pdf"), { recursive: true });
+      await writeFile(
+        join(agentsDir, "skills", "pdf", "SKILL.md"),
+        "managed copy\n",
+      );
+      // ...while "other" is free to move.
+      await mkdir(join(realSkillsDir, "other"), { recursive: true });
+
+      await expect(
+        ensureSkillsSymlink(agentsDir, targetDir),
+      ).rejects.toThrow(/pdf/);
+
+      // Neither copy may be destroyed: the local one used to be deleted by the
+      // recursive rm after being skipped for migration.
+      expect(await readFile(join(realSkillsDir, "pdf", "SKILL.md"), "utf-8")).toBe(
+        "local copy\n",
+      );
+      expect(
+        await readFile(join(agentsDir, "skills", "pdf", "SKILL.md"), "utf-8"),
+      ).toBe("managed copy\n");
+    });
+
     it("removes migrated files from git index", async () => {
       // Initialize a git repo in the temp dir
       await exec("git", ["init"], { cwd: dir });
diff --git a/packages/dotagents/src/symlinks/manager.ts b/packages/dotagents/src/symlinks/manager.ts
index 2859445..5d95961 100644
--- a/packages/dotagents/src/symlinks/manager.ts
+++ b/packages/dotagents/src/symlinks/manager.ts
@@ -49,6 +49,18 @@ export async function ensureSkillsSymlink(
   // Real directory - migrate contents then replace with symlink
   if (stat.isDirectory()) {
     const migrated = await migrateDirectory(skillsLink, skillsSource);
+    // migrateDirectory *moves* every entry it can, so whatever is still here
+    // is what it had to skip: a name already taken under skillsSource. The
+    // recursive rm below would delete it, silently destroying a skill the user
+    // never agreed to lose — stop and let them resolve the collision instead.
+    const conflicts = await readdir(skillsLink);
+    if (conflicts.length > 0) {
+      throw new SymlinkError(
+        `Cannot replace ${skillsLink} with a symlink — these already exist in ` +
+          `${skillsSource}: ${conflicts.join(", ")}. Remove or rename them, ` +
+          `then run install again.`,
+      );
+    }
     await removeFromGitIndex(targetDir, "skills");
     await rm(skillsLink, { recursive: true });
     await symlink(relativeTarget, skillsLink);
diff --git a/packages/dotagents/src/targets/skill-symlinks.test.ts b/packages/dotagents/src/targets/skill-symlinks.test.ts
index c6ce124..fe4ee4d 100644
--- a/packages/dotagents/src/targets/skill-symlinks.test.ts
+++ b/packages/dotagents/src/targets/skill-symlinks.test.ts
@@ -15,8 +15,10 @@ describe("skillSymlinkTargets", () => {
         [".legacy", ".claude", ".legacy"],
       ),
     ).toEqual([
-      join(scope.root, ".legacy"),
-      join(scope.root, ".claude"),
+      // skillSymlinkTargets resolves to absolute paths, which on Windows adds
+      // the drive letter that a bare join() leaves off.
+      resolve(join(scope.root, ".legacy")),
+      resolve(join(scope.root, ".claude")),
     ]);
   });
 
diff --git a/packages/dotagents/src/utils/fs.ts b/packages/dotagents/src/utils/fs.ts
index 49d8256..2ca30fc 100644
--- a/packages/dotagents/src/utils/fs.ts
+++ b/packages/dotagents/src/utils/fs.ts
@@ -1,4 +1,5 @@
-import { isAbsolute, relative, resolve } from "node:path";
+import { resolve } from "node:path";
+import { isInsideDir } from "@sentry/dotagents-lib";
 
 const SKILL_NAME_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/;
 
@@ -12,8 +13,8 @@ export function managedSkillPath(skillsDir: string, name: string): string | null
   if (!SKILL_NAME_RE.test(name)) {return null;}
   const root = resolve(skillsDir);
   const target = resolve(root, name);
-  const rel = relative(root, target);
-  if (rel.length === 0 || rel.startsWith("..") || isAbsolute(rel)) {
+  // The skills root itself is not a managed skill path — only entries beneath it.
+  if (target === root || !isInsideDir(root, target)) {
     return null;
   }
   return target;