diff --git a/CHANGELOG.md b/CHANGELOG.md index cb5793a..b354eab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ 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. + - 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..7af6ec5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,6 +42,23 @@ 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(); + + /** + * 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"); @@ -59,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; } @@ -69,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; }); @@ -80,9 +99,11 @@ 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 +152,18 @@ 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) && + !(rule.isSingleFile && injectedSingleFileRules.has(rule.realPath)), ); if (rules.length === 0) { return undefined; @@ -142,6 +173,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 }] }; diff --git a/src/rules/engine.ts b/src/rules/engine.ts index a4346be..7276a4a 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,12 +163,21 @@ 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. + // 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, discoveryTarget, deps.findProjectRoot) + : deps.findProjectRoot(discoveryTarget), + ); const findOptions: Parameters[0] = { projectRoot, - targetFile, + targetFile: discoveryTarget, ...(discoveryCache === undefined ? {} : { cache: discoveryCache }), ...(disabledSources === undefined ? {} : { disabledSources }), }; @@ -193,7 +203,7 @@ export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { const matchReason = matchDynamicRuleCached( dynamicMatchCache, projectRoot, - targetFile, + discoveryTarget, candidate, loadedRule, deps.matchRule ?? matchRule, @@ -235,13 +245,15 @@ export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { const fingerprints: DynamicTargetFingerprint[] = []; for (const targetFile of uniqueStrings(targetPaths)) { - const projectRoot = - cwdProjectRoot !== null && isSameOrChildPath(targetFile, cwdProjectRoot) + const discoveryTarget = canonicalizePath(targetFile); + const projectRoot = widenToRepositoryRoot( + 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 }), }; @@ -683,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 4396e33..8d5f100 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,58 @@ 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 = resolveExistingPath(homedir()); + let currentDirectory = resolveExistingPath(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; + } +} + +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 new file mode 100644 index 0000000..9cd8662 --- /dev/null +++ b/test/engine-monorepo.test.ts @@ -0,0 +1,114 @@ +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, 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 + * 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 engine = createEngine(defaultConfig(), createMonorepoDeps(fs)); + + // when + const result = engine.loadDynamicRules(fs.path("repo"), [targetPath]); + + // then + const paths = result.rules.map((rule) => canonicalPath(rule.path)); + expect(paths).toContain(canonicalPath(rustRule)); + expect(paths).toContain(canonicalPath(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 engine = createEngine(defaultConfig(), createMonorepoDeps(fs)); + + // when + const result = engine.loadDynamicRules(fs.path("repo"), [targetPath]); + + // then + expect(result.rules.map((rule) => canonicalPath(rule.path))).toContain(canonicalPath(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 engine = createEngine(defaultConfig(), createMonorepoDeps(fs)); + + // 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..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,9 +22,17 @@ 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 } 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 +274,104 @@ 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 the repository root is returned", () => { + // given + const tempFs = createTempFs(); + const projectRoot = tempFs.mkdir("repo"); + tempFs.mkdir("repo/.git"); + + try { + // when + const result = widenToRepositoryRoot(projectRoot); + + // then + expect(result).toBe(canonicalPath(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 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); + }); +});