From 3790d767cdca3f610ec99b5a9f20ae982ea0374a Mon Sep 17 00:00:00 2001 From: Toby Date: Thu, 3 Sep 2026 07:28:01 +0200 Subject: [PATCH 1/3] feat(monorepo): widen dynamic rule walk to the git repository root findProjectRoot stops at the innermost project marker. In monorepos with nested markers (Cargo workspaces: a Cargo.toml per member crate, pnpm workspaces: per-package package.json) a target file inside a member resolves its project root to the member directory, so the rule walk never reaches workspace- or repository-level .github/instructions/ directories and those rules silently never fire for member code. Widen the dynamic (per-target) project root to the innermost enclosing .git repository root instead: - engine.loadDynamicRules and fingerprintDynamicTargets wrap the per-target findProjectRoot result with widenToRepositoryRoot. - Globs stay keyed to the directory owning the rule file via the existing scopeRelative path base: a rule in backend/.github/instructions/ matches backend-relative patterns, a rule in the repository-root .github/instructions/ matches repo-relative patterns. - A .git in the user's home directory (dotfiles repositories) is ignored so rules never leak from ~ into marker-less projects. - Static (always-on) discovery is unchanged. - index.ts now dedups natively loaded context files in the tool_result path too: with the walk reaching repository level, single-file rules (AGENTS.md/CLAUDE.md) from levels pi already loaded natively must not be re-injected per matching read. Tests: unit tests for widenToRepositoryRoot (project-root.test.ts) and end-to-end engine tests with a real Cargo-workspace layout (engine-monorepo.test.ts) covering workspace-level rules, repository -level rules via repo-relative globs, negative scoping, and discovery of single-file rules on the walk. --- CHANGELOG.md | 11 +++ README.md | 11 +++ package-lock.json | 1 - src/index.ts | 27 ++++++-- src/rules/engine.ts | 18 +++-- src/rules/project-root.ts | 48 +++++++++++++ test/engine-monorepo.test.ts | 130 +++++++++++++++++++++++++++++++++++ test/project-root.test.ts | 62 ++++++++++++++++- 8 files changed, 297 insertions(+), 11 deletions(-) create mode 100644 test/engine-monorepo.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cb5793a..5fa61df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Monorepo / Cargo-workspace support for dynamic rule discovery: the per-target project + root is widened to the enclosing git repository root (`widenToRepositoryRoot`). Target + files inside workspace members (nested `Cargo.toml`/`package.json` markers) now discover + `.github/instructions/` rule directories from the workspace and repository levels as + well; each rule's glob semantics stay keyed to the directory owning the rule file via the + scopeRelative path base. Static (always-on) discovery is unchanged. +- Native context dedup in the `tool_result` path: single-file rules (AGENTS.md/CLAUDE.md) + that pi already loaded natively into the system prompt are no longer re-injected per + matching file read when dynamic discovery walks to the repository root. +### Added + - Matcher cache reset and stats helpers for deterministic cache verification. ### Changed diff --git a/README.md b/README.md index 3a94440..ce8e687 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,17 @@ After installation, restart pi (or run `/reload` inside an interactive session). These use **walk-up stack semantics**: from the target file's directory up to the project root, rules at every level are collected. Closer directories win in precedence. +**Monorepos / workspaces:** `findProjectRoot` stops at the innermost project marker, so a +target inside a workspace member (e.g. `crates//src/*.rs`, where each member has +its own `Cargo.toml`) would never reach workspace-level rule directories. For dynamic +(per-target) discovery the walk is therefore widened to the innermost enclosing **git +repository root** (`.git`): rules placed at the repository, workspace or member level all +participate, and each rule's globs match relative to the directory that owns the rule file +(a rule in `backend/.github/instructions/` matches `backend`-relative globs, a rule in the +repository root's `.github/instructions/` matches repo-relative globs). A `.git` in the +user's home directory (dotfiles repositories) is ignored. Static (session-level) discovery +is unchanged. + ### Project single-file rules | File | Style | diff --git a/package-lock.json b/package-lock.json index 1762119..e1dacc8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -207,7 +207,6 @@ "version": "0.82.1", "resolved": "https://registry.npmjs.org/@earendil-works/pi-coding-agent/-/pi-coding-agent-0.82.1.tgz", "integrity": "sha512-zbkAhoIuDPMF3pKuja0ajZabrMWU29FUMV9A/XMXT/XC1yXs5xt6t6t13GogQFsDrDqbFP4DkZQO1w8rWRAzYA==", - "hasShrinkwrap": true, "license": "MIT", "peer": true, "dependencies": { diff --git a/src/index.ts b/src/index.ts index 0435e61..ea4b36f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,6 +42,15 @@ export default function piRulesExtension(pi: ExtensionAPI): void { }); registerSlashCommands(pi, engine); + /** + * Absolute paths (and realpaths) of context files pi loaded natively into the system + * prompt. Rebuilt on every before_agent_start. Dynamic discovery walks to the repository + * root, so single-file rules (AGENTS.md/CLAUDE.md) from levels the agent has natively + * loaded must be deduplicated in the tool_result path as well — otherwise each matching + * read re-injects a context file that is already in the system prompt. + */ + const nativeContextPaths = new Set(); + function syncConfigFromFlags(): void { const disabled = pi.getFlag("pi-rules-disabled"); const mode = pi.getFlag("pi-rules-mode"); @@ -80,9 +89,10 @@ export default function piRulesExtension(pi: ExtensionAPI): void { } const loaded = engine.loadStaticRules(ctx.cwd); - const nativeContextPaths = new Set( - event.systemPromptOptions.contextFiles?.flatMap((contextFile) => pathKeys(contextFile.path)) ?? [], - ); + nativeContextPaths.clear(); + for (const path of event.systemPromptOptions.contextFiles?.flatMap((contextFile) => pathKeys(contextFile.path)) ?? []) { + nativeContextPaths.add(path); + } for (const rule of loaded.rules) { if (nativeContextPaths.has(rule.path) || nativeContextPaths.has(rule.realPath)) { engine.markStaticInjected(rule); @@ -131,8 +141,17 @@ export default function piRulesExtension(pi: ExtensionAPI): void { pendingFingerprints.map((target) => target.targetPath), ); engine.commitDynamicTargetFingerprints(fingerprints); + for (const rule of loaded.rules) { + if (nativeContextPaths.has(rule.path) || nativeContextPaths.has(rule.realPath)) { + engine.markStaticInjected(rule); + } + } const rules = loaded.rules.filter( - (rule) => !engine.isStaticInjected(rule) && !engine.isDynamicInjected(firstTargetPath, rule), + (rule) => + !nativeContextPaths.has(rule.path) && + !nativeContextPaths.has(rule.realPath) && + !engine.isStaticInjected(rule) && + !engine.isDynamicInjected(firstTargetPath, rule), ); if (rules.length === 0) { return undefined; diff --git a/src/rules/engine.ts b/src/rules/engine.ts index a4346be..3aa9bc6 100644 --- a/src/rules/engine.ts +++ b/src/rules/engine.ts @@ -22,6 +22,7 @@ import { formatDynamicBlock, formatStaticBlock } from "./formatter.js"; import { hashContent, matchRule } from "./matcher.js"; import { sortCandidates } from "./ordering.js"; import { parseRule } from "./parser.js"; +import { widenToRepositoryRoot } from "./project-root.js"; import type { LoadedRule, MatchReason, PiRulesConfig, RuleCandidate, RuleDiagnostic, SessionState } from "./types.js"; interface LoadedRuleContent { @@ -162,9 +163,15 @@ export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { const realPathCache: RealPathCache = new Map(); const rootSingleFileSelections = new Set(); for (const targetFile of targetFiles) { - const projectRoot = shouldCacheLookups - ? findProjectRootCached(projectRootCache, targetFile, deps.findProjectRoot) - : deps.findProjectRoot(targetFile); + // Cargo/pnpm workspaces nest project markers (e.g. a per-member Cargo.toml), so the + // marker-based root would stop the walk below workspace-level rule directories. + // Widen to the enclosing git repository root so rules at repository and workspace + // level participate; scopeRelative keeps globs keyed to the rule file's scope. + const projectRoot = widenToRepositoryRoot( + shouldCacheLookups + ? findProjectRootCached(projectRootCache, targetFile, deps.findProjectRoot) + : deps.findProjectRoot(targetFile), + ); const findOptions: Parameters[0] = { projectRoot, targetFile, @@ -235,10 +242,11 @@ export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { const fingerprints: DynamicTargetFingerprint[] = []; for (const targetFile of uniqueStrings(targetPaths)) { - const projectRoot = + const projectRoot = widenToRepositoryRoot( cwdProjectRoot !== null && isSameOrChildPath(targetFile, cwdProjectRoot) ? cwdProjectRoot - : deps.findProjectRoot(targetFile); + : deps.findProjectRoot(targetFile), + ); const findOptions: Parameters[0] = { projectRoot, targetFile, diff --git a/src/rules/project-root.ts b/src/rules/project-root.ts index 4396e33..e5282f0 100644 --- a/src/rules/project-root.ts +++ b/src/rules/project-root.ts @@ -1,4 +1,5 @@ import { existsSync, realpathSync, statSync } from "node:fs"; +import { homedir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { PROJECT_MARKERS } from "./constants.js"; @@ -40,3 +41,50 @@ export function findProjectRoot(startPath: string, markers: ReadonlyArray/src/` resolves its project root to the member + * crate (`crates//Cargo.toml`), which stops the rule walk before it + * reaches workspace-level `.github/instructions/` directories. Walking up to + * the innermost `.git` instead lets rules placed at repository, workspace or + * member level all participate — the scopeRelative path base keeps each rule's + * glob semantics keyed to the directory that owns the rule file. + * + * A `.git` in the user's home directory (dotfiles repositories) is ignored: it + * is not a meaningful rule boundary and would otherwise leak rules from `~` + * into every marker-less project below it. When no `.git` exists above, the + * project root is returned unchanged (plain upstream behaviour). + */ +export function widenToRepositoryRoot(projectRoot: string | null): string | null { + if (projectRoot === null) { + return null; + } + + const repositoryFilesystemRoot = resolve("/"); + const homeDirectory = resolve(homedir()); + let currentDirectory = resolve(projectRoot); + + while (true) { + if (currentDirectory === homeDirectory) { + return projectRoot; + } + + if (existsSync(join(currentDirectory, ".git"))) { + return currentDirectory; + } + + if (currentDirectory === repositoryFilesystemRoot) { + return projectRoot; + } + + const parentDirectory = dirname(currentDirectory); + if (parentDirectory === currentDirectory) { + return projectRoot; + } + + currentDirectory = parentDirectory; + } +} diff --git a/test/engine-monorepo.test.ts b/test/engine-monorepo.test.ts new file mode 100644 index 0000000..4a80b77 --- /dev/null +++ b/test/engine-monorepo.test.ts @@ -0,0 +1,130 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +import type { EngineDeps } from "../src/rules/engine.js"; +import { createEngine, defaultConfig } from "../src/rules/engine.js"; +import { findRuleCandidates } from "../src/rules/finder.js"; +import { findProjectRoot } from "../src/rules/project-root.js"; +import { createTempFs } from "./helpers/temp-fs.js"; + +/** + * Monorepo semantics: a Cargo/pnpm workspace member (nested project marker) must still + * discover rule directories from the workspace and repository levels because + * `loadDynamicRules` widens the per-target project root to the enclosing git root + * (see `widenToRepositoryRoot`). + */ +describe("loadDynamicRules (monorepo workspace)", () => { + it("#given workspace member target with workspace-level rules #when loadDynamicRules #then workspace rule and single-file rules from walk levels are returned", () => { + // given + const fs = createTempFs("pi-rules-monorepo-"); + try { + fs.mkdir("repo/.git"); + fs.write("repo/backend/Cargo.toml", ""); + const rustRule = fs.write( + "repo/backend/.github/instructions/rust.instructions.md", + '---\napplyTo: "crates/**/*.rs"\n---\nBackend Rust rule.\n', + ); + const agents = fs.write("repo/backend/AGENTS.md", "# Backend AGENTS\n"); + fs.write("repo/backend/crates/member/Cargo.toml", ""); + const targetPath = fs.write("repo/backend/crates/member/src/lib.rs", "pub fn f() {}\n"); + + const deps: EngineDeps = { + findProjectRoot, + findCandidates: findRuleCandidates, + readFile: (path) => { + try { + return readFileSync(path, "utf-8"); + } catch { + return null; + } + }, + extractToolPaths: () => [], + }; + const engine = createEngine(defaultConfig(), deps); + + // when + const result = engine.loadDynamicRules(fs.path("repo"), [targetPath]); + + // then + const paths = result.rules.map((rule) => rule.path); + expect(paths).toContain(rustRule); + expect(paths).toContain(agents); + } finally { + fs.cleanup(); + } + }); + + it("#given workspace member target #when loadDynamicRules #then repository-level rules match via repo-relative globs", () => { + // given + const fs = createTempFs("pi-rules-monorepo-"); + try { + fs.mkdir("repo/.git"); + fs.write("repo/backend/Cargo.toml", ""); + const rootLevelRule = fs.write( + "repo/.github/instructions/scripts.instructions.md", + '---\napplyTo: "backend/crates/**/*.rs"\n---\nRepository Rust rule.\n', + ); + fs.write("repo/backend/crates/member/Cargo.toml", ""); + const targetPath = fs.write("repo/backend/crates/member/src/lib.rs", "pub fn f() {}\n"); + + const deps: EngineDeps = { + findProjectRoot, + findCandidates: findRuleCandidates, + readFile: (path) => { + try { + return readFileSync(path, "utf-8"); + } catch { + return null; + } + }, + extractToolPaths: () => [], + }; + const engine = createEngine(defaultConfig(), deps); + + // when + const result = engine.loadDynamicRules(fs.path("repo"), [targetPath]); + + // then + expect(result.rules.map((rule) => rule.path)).toContain(rootLevelRule); + } finally { + fs.cleanup(); + } + }); + + it("#given workspace-level globs scoped to another subtree #when loadDynamicRules #then rule does not match", () => { + // given + const fs = createTempFs("pi-rules-monorepo-"); + try { + fs.mkdir("repo/.git"); + fs.write("repo/backend/Cargo.toml", ""); + fs.write( + "repo/backend/.github/instructions/scripts.instructions.md", + '---\napplyTo: "scripts/**"\n---\nScripts rule.\n', + ); + fs.write("repo/backend/crates/member/Cargo.toml", ""); + const targetPath = fs.write("repo/backend/crates/member/src/lib.rs", "pub fn f() {}\n"); + + const deps: EngineDeps = { + findProjectRoot, + findCandidates: findRuleCandidates, + readFile: (path) => { + try { + return readFileSync(path, "utf-8"); + } catch { + return null; + } + }, + extractToolPaths: () => [], + }; + const engine = createEngine(defaultConfig(), deps); + + // when + const result = engine.loadDynamicRules(fs.path("repo"), [targetPath]); + + // then + expect(result.rules.map((rule) => rule.path)).toHaveLength(0); + } finally { + fs.cleanup(); + } + }); +}); diff --git a/test/project-root.test.ts b/test/project-root.test.ts index 2c8a3f0..a194f3c 100644 --- a/test/project-root.test.ts +++ b/test/project-root.test.ts @@ -22,7 +22,7 @@ vi.mock("node:fs", async (importOriginal) => { import { realpathSync } from "node:fs"; -import { findProjectRoot } from "../src/rules/project-root.js"; +import { findProjectRoot, widenToRepositoryRoot } from "../src/rules/project-root.js"; import { createTempFs } from "./helpers/temp-fs.js"; function canonicalPath(path: string): string { @@ -264,3 +264,63 @@ describe("findProjectRoot", () => { } }); }); + +describe("widenToRepositoryRoot", () => { + it("#given project root with no .git above #when widening #then project root is returned unchanged", () => { + // given + const tempFs = createTempFs(); + const projectRoot = tempFs.mkdir("project"); + + try { + // when + const result = widenToRepositoryRoot(projectRoot); + + // then + expect(result).toBe(projectRoot); + } finally { + tempFs.cleanup(); + } + }); + + it("#given project root is the repository root itself #when widening #then project root is returned unchanged", () => { + // given + const tempFs = createTempFs(); + const projectRoot = tempFs.mkdir("repo"); + tempFs.mkdir("repo/.git"); + + try { + // when + const result = widenToRepositoryRoot(projectRoot); + + // then + expect(result).toBe(projectRoot); + } finally { + tempFs.cleanup(); + } + }); + + it("#given nested workspace member inside a git repository #when widening #then repository root is returned", () => { + // given + const tempFs = createTempFs(); + const repositoryRoot = tempFs.mkdir("repo"); + tempFs.mkdir("repo/.git"); + tempFs.write("repo/backend/Cargo.toml", ""); + const memberRoot = tempFs.mkdir("repo/backend/crates/member"); + tempFs.write("repo/backend/crates/member/Cargo.toml", ""); + + try { + // when + const result = widenToRepositoryRoot(memberRoot); + + // then + expect(result).toBe(canonicalPath(repositoryRoot)); + } finally { + tempFs.cleanup(); + } + }); + + it("#given null project root #when widening #then null is returned", () => { + // given / when / then + expect(widenToRepositoryRoot(null)).toBe(null); + }); +}); From 6b4f18577b2aac75386acf032d49b61722f4bdb2 Mon Sep 17 00:00:00 2001 From: Toby Date: Thu, 3 Sep 2026 07:28:49 +0200 Subject: [PATCH 2/3] fix(monorepo): inject single-file rules once per session, not per target Single-file rules (AGENTS.md/CLAUDE.md) match every dynamic target. With the walk now reaching repository level, a root-launched session that reads N backend files would re-inject backend/AGENTS.md N times (the per-target dynamic cache keys on the target path). Track realpaths of already-injected single-file rules at session level and clear the set on session start and compaction alongside the engine session state. Glob rules keep per-target re-injection semantics (a pipeline file and a photo module file each get their matching instructions). --- src/index.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index ea4b36f..2036888 100644 --- a/src/index.ts +++ b/src/index.ts @@ -51,6 +51,14 @@ export default function piRulesExtension(pi: ExtensionAPI): void { */ const nativeContextPaths = new Set(); + /** + * Realpaths of single-file rules (AGENTS.md/CLAUDE.md) already injected dynamically in + * this session. Single-file rules match every target, so without session-level dedup + * each newly read file would re-inject the same context document. Cleared on session + * start and compaction together with the engine session state. + */ + const injectedSingleFileRules = new Set(); + function syncConfigFromFlags(): void { const disabled = pi.getFlag("pi-rules-disabled"); const mode = pi.getFlag("pi-rules-mode"); @@ -68,6 +76,7 @@ export default function piRulesExtension(pi: ExtensionAPI): void { pi.on("session_start", async (event, ctx) => { syncConfigFromFlags(); engine.resetSession(ctx.cwd); + injectedSingleFileRules.clear(); if (engine.config.disabled) { return undefined; } @@ -78,6 +87,7 @@ export default function piRulesExtension(pi: ExtensionAPI): void { pi.on("session_compact", async (_event, ctx) => { engine.resetSession(ctx.cwd); + injectedSingleFileRules.clear(); pi.appendEntry("pi-rules.scan", { cwd: ctx.cwd, reason: "compact" }); return undefined; }); @@ -151,7 +161,8 @@ export default function piRulesExtension(pi: ExtensionAPI): void { !nativeContextPaths.has(rule.path) && !nativeContextPaths.has(rule.realPath) && !engine.isStaticInjected(rule) && - !engine.isDynamicInjected(firstTargetPath, rule), + !engine.isDynamicInjected(firstTargetPath, rule) && + !(rule.isSingleFile && injectedSingleFileRules.has(rule.realPath)), ); if (rules.length === 0) { return undefined; @@ -161,6 +172,9 @@ export default function piRulesExtension(pi: ExtensionAPI): void { const block = engine.formatDynamic(rules, displayPath(ctx.cwd, firstPendingTarget)); for (const rule of rules) { engine.markDynamicInjected(firstTargetPath, rule); + if (rule.isSingleFile) { + injectedSingleFileRules.add(rule.realPath); + } } return { content: [...event.content, { type: "text", text: block }] }; From 1c7a86f9a8efe24740571e2bb48041d63abf5063 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Thu, 24 Sep 2026 15:36:42 +0900 Subject: [PATCH 3/3] fix(monorepo): realpath repository walk and isolate tests Canonicalize paths so the git-root walk and candidate discovery agree on macOS symlink roots, and stop asserting against non-canonical tmp paths. --- CHANGELOG.md | 1 - src/index.ts | 5 ++- src/rules/engine.ts | 26 +++++++++---- src/rules/project-root.ts | 12 +++++- test/engine-monorepo.test.ts | 74 ++++++++++++++---------------------- test/project-root.test.ts | 55 ++++++++++++++++++++++++++- 6 files changed, 114 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fa61df..b354eab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Native context dedup in the `tool_result` path: single-file rules (AGENTS.md/CLAUDE.md) that pi already loaded natively into the system prompt are no longer re-injected per matching file read when dynamic discovery walks to the repository root. -### Added - Matcher cache reset and stats helpers for deterministic cache verification. diff --git a/src/index.ts b/src/index.ts index 2036888..7af6ec5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -100,7 +100,8 @@ export default function piRulesExtension(pi: ExtensionAPI): void { const loaded = engine.loadStaticRules(ctx.cwd); nativeContextPaths.clear(); - for (const path of event.systemPromptOptions.contextFiles?.flatMap((contextFile) => pathKeys(contextFile.path)) ?? []) { + for (const path of event.systemPromptOptions.contextFiles?.flatMap((contextFile) => pathKeys(contextFile.path)) ?? + []) { nativeContextPaths.add(path); } for (const rule of loaded.rules) { @@ -153,7 +154,7 @@ export default function piRulesExtension(pi: ExtensionAPI): void { engine.commitDynamicTargetFingerprints(fingerprints); for (const rule of loaded.rules) { if (nativeContextPaths.has(rule.path) || nativeContextPaths.has(rule.realPath)) { - engine.markStaticInjected(rule); + engine.markStaticInjected(rule); } } const rules = loaded.rules.filter( diff --git a/src/rules/engine.ts b/src/rules/engine.ts index 3aa9bc6..7276a4a 100644 --- a/src/rules/engine.ts +++ b/src/rules/engine.ts @@ -167,14 +167,17 @@ export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { // marker-based root would stop the walk below workspace-level rule directories. // Widen to the enclosing git repository root so rules at repository and workspace // level participate; scopeRelative keeps globs keyed to the rule file's scope. + // Canonicalize so macOS /var vs /private/var (and other symlink roots) still sit + // inside the realpathed project root used by findProjectRoot. + const discoveryTarget = canonicalizePath(targetFile); const projectRoot = widenToRepositoryRoot( shouldCacheLookups - ? findProjectRootCached(projectRootCache, targetFile, deps.findProjectRoot) - : deps.findProjectRoot(targetFile), + ? findProjectRootCached(projectRootCache, discoveryTarget, deps.findProjectRoot) + : deps.findProjectRoot(discoveryTarget), ); const findOptions: Parameters[0] = { projectRoot, - targetFile, + targetFile: discoveryTarget, ...(discoveryCache === undefined ? {} : { cache: discoveryCache }), ...(disabledSources === undefined ? {} : { disabledSources }), }; @@ -200,7 +203,7 @@ export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { const matchReason = matchDynamicRuleCached( dynamicMatchCache, projectRoot, - targetFile, + discoveryTarget, candidate, loadedRule, deps.matchRule ?? matchRule, @@ -242,14 +245,15 @@ export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { const fingerprints: DynamicTargetFingerprint[] = []; for (const targetFile of uniqueStrings(targetPaths)) { + const discoveryTarget = canonicalizePath(targetFile); const projectRoot = widenToRepositoryRoot( - cwdProjectRoot !== null && isSameOrChildPath(targetFile, cwdProjectRoot) + cwdProjectRoot !== null && isSameOrChildPath(discoveryTarget, cwdProjectRoot) ? cwdProjectRoot - : deps.findProjectRoot(targetFile), + : deps.findProjectRoot(discoveryTarget), ); const findOptions: Parameters[0] = { projectRoot, - targetFile, + targetFile: discoveryTarget, cache: discoveryCache, ...(disabledSources === undefined ? {} : { disabledSources }), }; @@ -691,6 +695,14 @@ function uniqueStrings(values: ReadonlyArray): string[] { return [...new Set(values)]; } +function canonicalizePath(path: string): string { + try { + return realpathSync.native(path); + } catch { + return resolve(path); + } +} + function storeLastLoad( state: SessionState, rules: ReadonlyArray, diff --git a/src/rules/project-root.ts b/src/rules/project-root.ts index e5282f0..8d5f100 100644 --- a/src/rules/project-root.ts +++ b/src/rules/project-root.ts @@ -64,8 +64,8 @@ export function widenToRepositoryRoot(projectRoot: string | null): string | null } const repositoryFilesystemRoot = resolve("/"); - const homeDirectory = resolve(homedir()); - let currentDirectory = resolve(projectRoot); + const homeDirectory = resolveExistingPath(homedir()); + let currentDirectory = resolveExistingPath(projectRoot); while (true) { if (currentDirectory === homeDirectory) { @@ -88,3 +88,11 @@ export function widenToRepositoryRoot(projectRoot: string | null): string | null currentDirectory = parentDirectory; } } + +function resolveExistingPath(path: string): string { + try { + return realpathSync.native(resolve(path)); + } catch { + return resolve(path); + } +} diff --git a/test/engine-monorepo.test.ts b/test/engine-monorepo.test.ts index 4a80b77..9cd8662 100644 --- a/test/engine-monorepo.test.ts +++ b/test/engine-monorepo.test.ts @@ -1,11 +1,31 @@ -import { readFileSync } from "node:fs"; +import { readFileSync, realpathSync } from "node:fs"; import { describe, expect, it } from "vitest"; import type { EngineDeps } from "../src/rules/engine.js"; import { createEngine, defaultConfig } from "../src/rules/engine.js"; import { findRuleCandidates } from "../src/rules/finder.js"; import { findProjectRoot } from "../src/rules/project-root.js"; -import { createTempFs } from "./helpers/temp-fs.js"; +import { createTempFs, type TempFs } from "./helpers/temp-fs.js"; + +function canonicalPath(path: string): string { + return realpathSync.native(path); +} + +function createMonorepoDeps(fs: TempFs): EngineDeps { + const homeDir = fs.mkdir("home"); + return { + findProjectRoot, + findCandidates: (options) => findRuleCandidates({ ...options, homeDir }), + readFile: (path) => { + try { + return readFileSync(path, "utf-8"); + } catch { + return null; + } + }, + extractToolPaths: () => [], + }; +} /** * Monorepo semantics: a Cargo/pnpm workspace member (nested project marker) must still @@ -28,27 +48,15 @@ describe("loadDynamicRules (monorepo workspace)", () => { fs.write("repo/backend/crates/member/Cargo.toml", ""); const targetPath = fs.write("repo/backend/crates/member/src/lib.rs", "pub fn f() {}\n"); - const deps: EngineDeps = { - findProjectRoot, - findCandidates: findRuleCandidates, - readFile: (path) => { - try { - return readFileSync(path, "utf-8"); - } catch { - return null; - } - }, - extractToolPaths: () => [], - }; - const engine = createEngine(defaultConfig(), deps); + const engine = createEngine(defaultConfig(), createMonorepoDeps(fs)); // when const result = engine.loadDynamicRules(fs.path("repo"), [targetPath]); // then - const paths = result.rules.map((rule) => rule.path); - expect(paths).toContain(rustRule); - expect(paths).toContain(agents); + const paths = result.rules.map((rule) => canonicalPath(rule.path)); + expect(paths).toContain(canonicalPath(rustRule)); + expect(paths).toContain(canonicalPath(agents)); } finally { fs.cleanup(); } @@ -67,25 +75,13 @@ describe("loadDynamicRules (monorepo workspace)", () => { fs.write("repo/backend/crates/member/Cargo.toml", ""); const targetPath = fs.write("repo/backend/crates/member/src/lib.rs", "pub fn f() {}\n"); - const deps: EngineDeps = { - findProjectRoot, - findCandidates: findRuleCandidates, - readFile: (path) => { - try { - return readFileSync(path, "utf-8"); - } catch { - return null; - } - }, - extractToolPaths: () => [], - }; - const engine = createEngine(defaultConfig(), deps); + const engine = createEngine(defaultConfig(), createMonorepoDeps(fs)); // when const result = engine.loadDynamicRules(fs.path("repo"), [targetPath]); // then - expect(result.rules.map((rule) => rule.path)).toContain(rootLevelRule); + expect(result.rules.map((rule) => canonicalPath(rule.path))).toContain(canonicalPath(rootLevelRule)); } finally { fs.cleanup(); } @@ -104,19 +100,7 @@ describe("loadDynamicRules (monorepo workspace)", () => { fs.write("repo/backend/crates/member/Cargo.toml", ""); const targetPath = fs.write("repo/backend/crates/member/src/lib.rs", "pub fn f() {}\n"); - const deps: EngineDeps = { - findProjectRoot, - findCandidates: findRuleCandidates, - readFile: (path) => { - try { - return readFileSync(path, "utf-8"); - } catch { - return null; - } - }, - extractToolPaths: () => [], - }; - const engine = createEngine(defaultConfig(), deps); + const engine = createEngine(defaultConfig(), createMonorepoDeps(fs)); // when const result = engine.loadDynamicRules(fs.path("repo"), [targetPath]); diff --git a/test/project-root.test.ts b/test/project-root.test.ts index a194f3c..2b440f5 100644 --- a/test/project-root.test.ts +++ b/test/project-root.test.ts @@ -5,6 +5,8 @@ const fsMock = vi.hoisted(() => ({ statError: null as NodeJS.ErrnoException | null, })); +const homeMock = vi.hoisted(() => ({ path: "" })); + vi.mock("node:fs", async (importOriginal) => { const actual = await importOriginal(); return { @@ -20,6 +22,14 @@ vi.mock("node:fs", async (importOriginal) => { }; }); +vi.mock("node:os", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + homedir: () => (homeMock.path === "" ? actual.homedir() : homeMock.path), + }; +}); + import { realpathSync } from "node:fs"; import { findProjectRoot, widenToRepositoryRoot } from "../src/rules/project-root.js"; @@ -282,7 +292,7 @@ describe("widenToRepositoryRoot", () => { } }); - it("#given project root is the repository root itself #when widening #then project root is returned unchanged", () => { + it("#given project root is the repository root itself #when widening #then the repository root is returned", () => { // given const tempFs = createTempFs(); const projectRoot = tempFs.mkdir("repo"); @@ -293,7 +303,7 @@ describe("widenToRepositoryRoot", () => { const result = widenToRepositoryRoot(projectRoot); // then - expect(result).toBe(projectRoot); + expect(result).toBe(canonicalPath(projectRoot)); } finally { tempFs.cleanup(); } @@ -319,6 +329,47 @@ describe("widenToRepositoryRoot", () => { } }); + it("#given nested git repo inside a parent git repo #when widening #then innermost repository root is returned", () => { + // given + const tempFs = createTempFs(); + const outerRoot = tempFs.mkdir("outer"); + tempFs.mkdir("outer/.git"); + const innerRoot = tempFs.mkdir("outer/inner"); + tempFs.mkdir("outer/inner/.git"); + const memberRoot = tempFs.mkdir("outer/inner/crates/member"); + + try { + // when + const result = widenToRepositoryRoot(memberRoot); + + // then + expect(result).toBe(canonicalPath(innerRoot)); + expect(result).not.toBe(canonicalPath(outerRoot)); + } finally { + tempFs.cleanup(); + } + }); + + it("#given project under a home directory that is itself a git repo #when widening #then home .git is ignored", () => { + // given + const tempFs = createTempFs(); + const homeDirectory = tempFs.mkdir("home"); + tempFs.mkdir("home/.git"); + const projectRoot = tempFs.mkdir("home/project"); + homeMock.path = homeDirectory; + + try { + // when + const result = widenToRepositoryRoot(projectRoot); + + // then + expect(result).toBe(projectRoot); + } finally { + homeMock.path = ""; + tempFs.cleanup(); + } + }); + it("#given null project root #when widening #then null is returned", () => { // given / when / then expect(widenToRepositoryRoot(null)).toBe(null);