mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
Fixes #10488 ## Problem On Windows, `c:\Users\me` and `C:\Users\me` are the same directory, but pi compared paths as exact strings (`===`, `startsWith`, map and set keys). `realpathSync` keeps the input casing, so canonicalizing does not help. The reported case, with cwd `c:\Users\me\project` and home `C:\Users\me`: 1. Ancestor discovery finds `c:\Users\me\.agents\skills`. The exclusion of the user dir `C:\Users\me\.agents\skills` does not match, so global skills load as project skills. 2. The same dir loads again as user skills. Dedup does not match either, so every skill is reported as a name collision. 3. `hasTrustRequiringProjectResources` treats `~/.agents/skills` as a project resource that requires trust. The same pattern affected other places: - Saved trust decisions in `trust.json` were not found when cwd casing differed, so pi asked for trust again. - With a custom session dir, `--continue`, resume by id, and the session list hid sessions recorded with different cwd casing. - Git worktree `AGENTS.md` shadowing failed when cwd casing differed from git's paths. - Local package matching (`pi remove c:\pkg` vs configured `C:\pkg`) and package identity across global and project settings. - Extension preload matching, skill scope classification, resource source info, the config and session selector keys, and the session import self-check. Separately, home-to-`~` shortening used an unanchored `startsWith(home)` in four places, so `/home/me2/x` was shown as `~2/x`. ## Fix New helpers in `utils/paths.ts`, all based on one comparison key: - `getPathComparisonKey(path)`: the resolved path, lowercased on Windows. It does not touch the filesystem. - `pathsEqual(a, b)` - `isPathWithin(parent, child)`: true for equal paths or paths inside `parent`, matching whole path segments. - `shortenHomePath(path, home?)`: replaces a leading home dir with `~`. The helpers pick `path.win32` or `path.posix` from `process.platform` at call time, so tests can exercise real Windows paths (drive letters, UNC). `getResourcePathKey(path)` in `source-info.ts` is the dedup key for resources. Synthetic identifiers (`builtin:<name>`, `<inline:...>`) stay unchanged, because they are not filesystem paths and their lookup is case-sensitive. File paths go through realpath plus the comparison key. The helpers are used only for comparison; stored and displayed paths are unchanged. Trust store lookups and `set` match keys case-insensitively on Windows. Unchanged on purpose: - The package install-root guards (`resolveManagedPath`, `pruneEmptyGitParents`, the `node_modules` dependency check) keep exact string checks. Both sides come from the same root string, and exact is the stricter option. - Checks based on `path.relative()` (`getCwdRelativePath`, the changelog containment check) were already correct: Node's `path.win32.relative` ignores case. NTFS can enable per-directory case sensitivity, but it is rarely used outside WSL interop, so it is ignored (see the comment on `getPathComparisonKey`). ## Tests - `test/paths.test.ts`: Windows drive-letter, UNC and root cases, POSIX case sensitivity, segment-boundary containment, and home shortening, including root and UNC homes. - `test/source-info.test.ts`: file paths fold case on Windows; builtin and inline identifiers keep their case. - `test/trust-manager.test.ts`: the `.agents/skills` exclusion with a lowercase `HOME`, plus saved trust decisions matched and replaced across casing (simulated win32). An end-to-end test of the reported loader scenario is not feasible on Linux: the loader resolves paths with POSIX `node:path`, and the filesystem is case-sensitive. `npm run check` passes. The coding-agent suite passes, except for 13 failures that also occur without this change: clipboard tool detection in this environment, and one test that needs built `dist/` output.
108 lines
4.1 KiB
TypeScript
108 lines
4.1 KiB
TypeScript
import { mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
|
import { tmpdir } from "node:os";
|
|
import { join } from "node:path";
|
|
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
|
import { hasTrustRequiringProjectResources, ProjectTrustStore } from "../src/core/trust-manager.ts";
|
|
|
|
describe("ProjectTrustStore", () => {
|
|
let tempDir: string;
|
|
let agentDir: string;
|
|
let cwd: string;
|
|
|
|
beforeEach(() => {
|
|
tempDir = join(tmpdir(), `trust-test-${Date.now()}-${Math.random().toString(36).slice(2)}`);
|
|
agentDir = join(tempDir, "agent");
|
|
cwd = join(tempDir, "project");
|
|
mkdirSync(agentDir, { recursive: true });
|
|
mkdirSync(cwd, { recursive: true });
|
|
});
|
|
|
|
afterEach(() => {
|
|
rmSync(tempDir, { recursive: true, force: true });
|
|
});
|
|
|
|
it("stores decisions and inherits from parent directories", () => {
|
|
const store = new ProjectTrustStore(agentDir);
|
|
const parentDir = join(tempDir, "trusted-parent");
|
|
const childDir = join(parentDir, "project");
|
|
mkdirSync(childDir, { recursive: true });
|
|
|
|
expect(store.get(childDir)).toBeNull();
|
|
store.set(parentDir, true);
|
|
expect(store.get(childDir)).toBe(true);
|
|
store.set(childDir, false);
|
|
expect(store.get(childDir)).toBe(false);
|
|
store.set(childDir, null);
|
|
expect(store.get(childDir)).toBe(true);
|
|
});
|
|
|
|
it("detects trust-requiring project resources", () => {
|
|
const originalHome = process.env.HOME;
|
|
process.env.HOME = tempDir;
|
|
try {
|
|
mkdirSync(join(tempDir, ".pi", "agent"), { recursive: true });
|
|
mkdirSync(join(tempDir, ".agents", "skills"), { recursive: true });
|
|
expect(hasTrustRequiringProjectResources(tempDir)).toBe(false);
|
|
expect(hasTrustRequiringProjectResources(cwd)).toBe(false);
|
|
|
|
writeFileSync(join(tempDir, ".pi", "settings.json"), "{}");
|
|
expect(hasTrustRequiringProjectResources(tempDir)).toBe(true);
|
|
rmSync(join(tempDir, ".pi", "settings.json"), { force: true });
|
|
|
|
mkdirSync(join(cwd, ".pi"), { recursive: true });
|
|
writeFileSync(join(cwd, ".pi", "settings.json"), "{}");
|
|
expect(hasTrustRequiringProjectResources(cwd)).toBe(true);
|
|
|
|
rmSync(join(cwd, ".pi"), { recursive: true, force: true });
|
|
mkdirSync(join(cwd, ".agents", "skills"), { recursive: true });
|
|
expect(hasTrustRequiringProjectResources(cwd)).toBe(true);
|
|
} finally {
|
|
if (originalHome === undefined) {
|
|
delete process.env.HOME;
|
|
} else {
|
|
process.env.HOME = originalHome;
|
|
}
|
|
}
|
|
});
|
|
|
|
// Regression test for https://github.com/earendil-works/pi/issues/10488
|
|
it("ignores the user .agents/skills dir when HOME differs only by case on Windows", () => {
|
|
const originalHome = process.env.HOME;
|
|
const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
|
|
const mixedCaseHome = join(tempDir, "Home");
|
|
const projectDir = join(mixedCaseHome, "project");
|
|
mkdirSync(join(mixedCaseHome, ".agents", "skills"), { recursive: true });
|
|
mkdirSync(projectDir, { recursive: true });
|
|
process.env.HOME = mixedCaseHome.toLowerCase();
|
|
Object.defineProperty(process, "platform", { value: "win32" });
|
|
try {
|
|
expect(hasTrustRequiringProjectResources(projectDir)).toBe(false);
|
|
} finally {
|
|
if (originalPlatform) Object.defineProperty(process, "platform", originalPlatform);
|
|
if (originalHome === undefined) {
|
|
delete process.env.HOME;
|
|
} else {
|
|
process.env.HOME = originalHome;
|
|
}
|
|
}
|
|
});
|
|
|
|
// Regression test for https://github.com/earendil-works/pi/issues/10488
|
|
it("matches saved decisions regardless of casing on Windows", () => {
|
|
const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
|
|
const projectDir = join(tempDir, "Project-Mixed");
|
|
mkdirSync(projectDir, { recursive: true });
|
|
const store = new ProjectTrustStore(agentDir);
|
|
store.set(projectDir, true);
|
|
Object.defineProperty(process, "platform", { value: "win32" });
|
|
try {
|
|
expect(store.get(projectDir.toLowerCase())).toBe(true);
|
|
store.set(projectDir.toLowerCase(), false);
|
|
const saved = JSON.parse(readFileSync(join(agentDir, "trust.json"), "utf-8")) as Record<string, boolean>;
|
|
expect(Object.values(saved)).toEqual([false]);
|
|
expect(store.get(projectDir)).toBe(false);
|
|
} finally {
|
|
if (originalPlatform) Object.defineProperty(process, "platform", originalPlatform);
|
|
}
|
|
});
|
|
});
|