From 1ab1b54ce6807cb326fe871b860be755a2d36688 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 18 May 2026 11:52:44 +0900 Subject: [PATCH 1/4] perf(rules-injector): cache compiled glob matchers Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/hooks/rules-injector/matcher.test.ts | 70 ++++++++++++++++++++++++ src/hooks/rules-injector/matcher.ts | 28 +++++++++- 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 src/hooks/rules-injector/matcher.test.ts diff --git a/src/hooks/rules-injector/matcher.test.ts b/src/hooks/rules-injector/matcher.test.ts new file mode 100644 index 000000000..009c19645 --- /dev/null +++ b/src/hooks/rules-injector/matcher.test.ts @@ -0,0 +1,70 @@ +/// + +import { beforeEach, describe, expect, it } from "bun:test" +import { + createContentHash, + getMatcherCacheStats, + isDuplicateByContentHash, + isDuplicateByRealPath, + resetMatcherCache, + shouldApplyRule, +} from "./matcher" + +describe("shouldApplyRule", () => { + beforeEach(() => { + resetMatcherCache() + }) + + it("#given repeated glob metadata #when matching many files #then compiles each pattern once", () => { + // given + const metadata = { globs: ["src/**/*.ts", "test/**/*.ts"] } + const projectRoot = "/workspace/project" + + // when + for (let index = 0; index < 20; index += 1) { + shouldApplyRule(metadata, `${projectRoot}/src/file-${index}.ts`, projectRoot) + shouldApplyRule(metadata, `${projectRoot}/test/file-${index}.ts`, projectRoot) + } + + // then + expect(getMatcherCacheStats()).toEqual({ entries: 2 }) + }) + + it("#given matching glob #when path is under project root #then returns matching reason", () => { + // given / when + const result = shouldApplyRule({ globs: "src/**/*.ts" }, "/workspace/project/src/index.ts", "/workspace/project") + + // then + expect(result).toEqual({ applies: true, reason: "glob: src/**/*.ts" }) + }) + + it("#given always apply metadata #when no globs exist #then applies without compiling matchers", () => { + // given / when + const result = shouldApplyRule({ alwaysApply: true }, "/workspace/project/src/index.ts", "/workspace/project") + + // then + expect(result).toEqual({ applies: true, reason: "alwaysApply" }) + expect(getMatcherCacheStats()).toEqual({ entries: 0 }) + }) +}) + +describe("rule duplicate helpers", () => { + it("#given real path cache #when path exists #then reports duplicate", () => { + // given + const cache = new Set(["/workspace/project/AGENTS.md"]) + + // when / then + expect(isDuplicateByRealPath("/workspace/project/AGENTS.md", cache)).toBe(true) + expect(isDuplicateByRealPath("/workspace/project/src/AGENTS.md", cache)).toBe(false) + }) + + it("#given content #when hashing #then duplicate helper uses truncated hash", () => { + // given + const hash = createContentHash("rule-content") + const cache = new Set([hash]) + + // when / then + expect(hash).toHaveLength(16) + expect(isDuplicateByContentHash(hash, cache)).toBe(true) + }) +}) diff --git a/src/hooks/rules-injector/matcher.ts b/src/hooks/rules-injector/matcher.ts index 13f6d51c5..d68dae109 100644 --- a/src/hooks/rules-injector/matcher.ts +++ b/src/hooks/rules-injector/matcher.ts @@ -3,11 +3,37 @@ import { relative } from "node:path" import picomatch from "picomatch" import type { RuleMetadata } from "./types" +type PathMatcher = (path: string) => boolean + export interface MatchResult { applies: boolean reason?: string } +export interface MatcherCacheStats { + entries: number +} + +const PICOMATCH_OPTIONS = { dot: true, bash: true } as const +const matcherCache = new Map() + +function matcherFor(pattern: string): PathMatcher { + const cached = matcherCache.get(pattern) + if (cached) return cached + + const matcher = picomatch(pattern, PICOMATCH_OPTIONS) + matcherCache.set(pattern, matcher) + return matcher +} + +export function resetMatcherCache(): void { + matcherCache.clear() +} + +export function getMatcherCacheStats(): MatcherCacheStats { + return { entries: matcherCache.size } +} + /** * Check if a rule should apply to the current file based on metadata */ @@ -33,7 +59,7 @@ export function shouldApplyRule( const relativePath = projectRoot ? relative(projectRoot, currentFilePath) : currentFilePath for (const pattern of patterns) { - if (picomatch.isMatch(relativePath, pattern, { dot: true, bash: true })) { + if (matcherFor(pattern)(relativePath)) { return { applies: true, reason: `glob: ${pattern}` } } } From 46b965b9daa2ca3e9c4bcc4d74defbbe4deb10c2 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 18 May 2026 12:36:33 +0900 Subject: [PATCH 2/4] fix(rules-injector): bound matcher cache Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/hooks/rules-injector/matcher.test.ts | 13 +++++++++++++ src/hooks/rules-injector/matcher.ts | 13 ++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/hooks/rules-injector/matcher.test.ts b/src/hooks/rules-injector/matcher.test.ts index 009c19645..fce36bc4e 100644 --- a/src/hooks/rules-injector/matcher.test.ts +++ b/src/hooks/rules-injector/matcher.test.ts @@ -30,6 +30,19 @@ describe("shouldApplyRule", () => { expect(getMatcherCacheStats()).toEqual({ entries: 2 }) }) + it("#given many unique globs #when matching repeatedly #then matcher cache stays bounded", () => { + // given + const projectRoot = "/workspace/project" + + // when + for (let index = 0; index < 300; index += 1) { + shouldApplyRule({ globs: `src/file-${index}.ts` }, `${projectRoot}/src/file-${index}.ts`, projectRoot) + } + + // then + expect(getMatcherCacheStats().entries <= 256).toBe(true) + }) + it("#given matching glob #when path is under project root #then returns matching reason", () => { // given / when const result = shouldApplyRule({ globs: "src/**/*.ts" }, "/workspace/project/src/index.ts", "/workspace/project") diff --git a/src/hooks/rules-injector/matcher.ts b/src/hooks/rules-injector/matcher.ts index d68dae109..cd6995f6e 100644 --- a/src/hooks/rules-injector/matcher.ts +++ b/src/hooks/rules-injector/matcher.ts @@ -15,13 +15,24 @@ export interface MatcherCacheStats { } const PICOMATCH_OPTIONS = { dot: true, bash: true } as const +const MAX_MATCHER_CACHE_ENTRIES = 256 const matcherCache = new Map() function matcherFor(pattern: string): PathMatcher { const cached = matcherCache.get(pattern) - if (cached) return cached + if (cached) { + matcherCache.delete(pattern) + matcherCache.set(pattern, cached) + return cached + } const matcher = picomatch(pattern, PICOMATCH_OPTIONS) + if (matcherCache.size >= MAX_MATCHER_CACHE_ENTRIES) { + const oldestPattern = matcherCache.keys().next().value + if (oldestPattern !== undefined) { + matcherCache.delete(oldestPattern) + } + } matcherCache.set(pattern, matcher) return matcher } From 61890f08575942391efecb62c99832d55e6ce722 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 18 May 2026 12:40:12 +0900 Subject: [PATCH 3/4] fix(rules-injector): bound parsed rule cache Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/hooks/rules-injector/hook.ts | 3 +- src/hooks/rules-injector/injector.test.ts | 27 +++++++++++++++-- src/hooks/rules-injector/injector.ts | 36 +++++++++++++++++++++-- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/hooks/rules-injector/hook.ts b/src/hooks/rules-injector/hook.ts index 3b62d5e01..51cd31146 100644 --- a/src/hooks/rules-injector/hook.ts +++ b/src/hooks/rules-injector/hook.ts @@ -3,7 +3,7 @@ import { createDynamicTruncator } from "../../shared/dynamic-truncator"; import { resolveSessionEventID } from "../../shared/event-session-id"; import { getRuleInjectionFilePath } from "./output-path"; import { createSessionCacheStore, createSessionRuleScanCacheStore } from "./cache"; -import { createRuleInjectionProcessor } from "./injector"; +import { clearParsedRuleCache, createRuleInjectionProcessor } from "./injector"; import { clearProjectRootCache } from "./project-root-finder"; interface ToolExecuteInput { @@ -53,6 +53,7 @@ export function createRulesInjectorHook( function clearSessionState(sessionID: string): void { clearSessionCache(sessionID); clearSessionRuleScanCache(sessionID); + clearParsedRuleCache(); } const toolExecuteAfter = async ( diff --git a/src/hooks/rules-injector/injector.test.ts b/src/hooks/rules-injector/injector.test.ts index 88b8076d4..8eb23c029 100644 --- a/src/hooks/rules-injector/injector.test.ts +++ b/src/hooks/rules-injector/injector.test.ts @@ -5,7 +5,7 @@ import * as os from "node:os"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { RULES_INJECTOR_STORAGE } from "./constants"; -import { createRuleInjectionProcessor } from "./injector"; +import { clearParsedRuleCache, createRuleInjectionProcessor, getParsedRuleCacheStats } from "./injector"; type StatSnapshot = { mtimeMs: number; size: number }; @@ -55,11 +55,11 @@ async function createProcessor(projectRoot: string): Promise<{ } return cache; }, - readFileSync: (filePath: fs.PathOrFileDescriptor, options?: Parameters[1]) => { + readFileSync: (filePath: string, encoding: "utf-8") => { if (filePath === trackedRulePath) { trackedReadFileCount += 1; } - return originalReadFileSync(filePath, options as never); + return originalReadFileSync(filePath, encoding); }, statSync: (filePath: fs.PathLike) => { if (filePath === trackedRulePath) { @@ -98,6 +98,7 @@ describe("createRuleInjectionProcessor", () => { let ruleRealPath: string; beforeEach(() => { + clearParsedRuleCache(); testRoot = join(tmpdir(), `rules-injector-injector-${Date.now()}`); projectRoot = join(testRoot, "project"); homeRoot = join(testRoot, "home"); @@ -125,6 +126,7 @@ describe("createRuleInjectionProcessor", () => { }); afterEach(() => { + clearParsedRuleCache(); if (fs.existsSync(testRoot)) { rmSync(testRoot, { recursive: true, force: true }); } @@ -178,6 +180,25 @@ describe("createRuleInjectionProcessor", () => { expect(trackedReadFileCount).toBe(2); }); + it("does not cache oversized parsed rule bodies", async () => { + // given + const largeBody = "x".repeat(70 * 1024); + writeFileSync(ruleFile, largeBody); + statSnapshots = [ + { mtimeMs: 1000, size: largeBody.length }, + { mtimeMs: 1000, size: largeBody.length }, + ]; + const processor = await createProcessor(projectRoot); + + // when + await processor.processFilePathForInjection(targetFile, "session-1", createOutput()); + await processor.processFilePathForInjection(targetFile, "session-2", createOutput()); + + // then + expect(trackedReadFileCount).toBe(2); + expect(getParsedRuleCacheStats()).toEqual({ entries: 0, bodyBytes: 0 }); + }); + it("does not save injected rules when all candidates are already cached", async () => { // given const sessionID = `dirty-no-new-${Date.now()}`; diff --git a/src/hooks/rules-injector/injector.ts b/src/hooks/rules-injector/injector.ts index 0cd64be5b..8dc517c58 100644 --- a/src/hooks/rules-injector/injector.ts +++ b/src/hooks/rules-injector/injector.ts @@ -35,6 +35,8 @@ type DynamicTruncator = { ) => Promise<{ result: string; truncated: boolean }>; }; +type RuleFileReader = (path: string, encoding: "utf-8") => string; + interface ParsedRuleEntry { mtimeMs: number; size: number; @@ -42,8 +44,38 @@ interface ParsedRuleEntry { body: string; } +export interface ParsedRuleCacheStats { + entries: number; + bodyBytes: number; +} + +const MAX_PARSED_RULE_CACHE_ENTRIES = 256; +const MAX_PARSED_RULE_CACHE_BODY_BYTES = 64 * 1024; const parsedRuleCache = new Map(); +export function clearParsedRuleCache(): void { + parsedRuleCache.clear(); +} + +export function getParsedRuleCacheStats(): ParsedRuleCacheStats { + let bodyBytes = 0; + for (const entry of parsedRuleCache.values()) { + bodyBytes += Buffer.byteLength(entry.body, "utf8"); + } + return { entries: parsedRuleCache.size, bodyBytes }; +} + +function setParsedRuleCacheEntry(realPath: string, entry: ParsedRuleEntry): void { + if (Buffer.byteLength(entry.body, "utf8") > MAX_PARSED_RULE_CACHE_BODY_BYTES) return; + if (parsedRuleCache.size >= MAX_PARSED_RULE_CACHE_ENTRIES) { + const oldestRealPath = parsedRuleCache.keys().next().value; + if (oldestRealPath !== undefined) { + parsedRuleCache.delete(oldestRealPath); + } + } + parsedRuleCache.set(realPath, entry); +} + function resolveFilePath( workspaceDirectory: string, path: string @@ -59,7 +91,7 @@ export function createRuleInjectionProcessor(deps: { getSessionCache: (sessionID: string) => SessionInjectedRulesCache; getSessionRuleScanCache?: (sessionID: string) => RuleScanCache; ruleFinderOptions?: FindRuleFilesOptions; - readFileSync?: typeof readFileSync; + readFileSync?: RuleFileReader; statSync?: typeof statSync; homedir?: typeof homedir; shouldApplyRule?: typeof shouldApplyRule; @@ -101,7 +133,7 @@ export function createRuleInjectionProcessor(deps: { const rawContent = readRuleFileSync(filePath, "utf-8"); const { metadata, body } = parseRuleFrontmatter(rawContent); - parsedRuleCache.set(realPath, { + setParsedRuleCacheEntry(realPath, { mtimeMs: stat.mtimeMs, size: stat.size, metadata, From 50ae2e110e01ce34e27d224a392ca8416ba00761 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 18 May 2026 13:30:23 +0900 Subject: [PATCH 4/4] perf(rules-injector): cache match decisions Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/hooks/rules-injector/injector.test.ts | 544 ++++++++++++++-------- src/hooks/rules-injector/injector.ts | 446 +++++++++++------- 2 files changed, 623 insertions(+), 367 deletions(-) diff --git a/src/hooks/rules-injector/injector.test.ts b/src/hooks/rules-injector/injector.test.ts index 8eb23c029..67fa2b9b9 100644 --- a/src/hooks/rules-injector/injector.test.ts +++ b/src/hooks/rules-injector/injector.test.ts @@ -5,13 +5,18 @@ import * as os from "node:os"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { RULES_INJECTOR_STORAGE } from "./constants"; -import { clearParsedRuleCache, createRuleInjectionProcessor, getParsedRuleCacheStats } from "./injector"; +import { + clearParsedRuleCache, + createRuleInjectionProcessor, + getParsedRuleCacheStats, +} from "./injector"; type StatSnapshot = { mtimeMs: number; size: number }; let trackedRulePath = ""; let statSnapshots: Array = []; let trackedReadFileCount = 0; +let trackedShouldApplyRuleCount = 0; let mockedHomeDir = ""; const originalReadFileSync = fs.readFileSync.bind(fs); @@ -19,246 +24,377 @@ const originalStatSync = fs.statSync.bind(fs); const originalHomedir = os.homedir.bind(os); function createOutput(): { title: string; output: string; metadata: unknown } { - return { title: "tool", output: "", metadata: {} }; + return { title: "tool", output: "", metadata: {} }; } async function createProcessor(projectRoot: string): Promise<{ - processFilePathForInjection: ( - filePath: string, - sessionID: string, - output: { title: string; output: string; metadata: unknown } - ) => Promise; + processFilePathForInjection: ( + filePath: string, + sessionID: string, + output: { title: string; output: string; metadata: unknown }, + ) => Promise; }> { - const sessionCaches = new Map< - string, - { contentHashes: Set; realPaths: Set } - >(); + const sessionCaches = new Map< + string, + { contentHashes: Set; realPaths: Set } + >(); - return createRuleInjectionProcessor({ - workspaceDirectory: projectRoot, - truncator: { - truncate: async (_sessionID: string, content: string) => ({ - result: content, - truncated: false, - }), - }, - getSessionCache: (sessionID: string) => { - if (!sessionCaches.has(sessionID)) { - sessionCaches.set(sessionID, { - contentHashes: new Set(), - realPaths: new Set(), - }); - } - const cache = sessionCaches.get(sessionID); - if (!cache) { - throw new Error("Session cache should exist"); - } - return cache; - }, - readFileSync: (filePath: string, encoding: "utf-8") => { - if (filePath === trackedRulePath) { - trackedReadFileCount += 1; - } - return originalReadFileSync(filePath, encoding); - }, - statSync: (filePath: fs.PathLike) => { - if (filePath === trackedRulePath) { - const next = statSnapshots.shift(); - if (next instanceof Error) { - throw next; - } - if (next) { - return { - mtimeMs: next.mtimeMs, - size: next.size, - isFile: () => true, - } as ReturnType; - } - } - return originalStatSync(filePath); - }, - homedir: () => mockedHomeDir || originalHomedir(), - shouldApplyRule: () => ({ applies: true, reason: "matched" }), - isDuplicateByRealPath: (realPath: string, cache: Set) => cache.has(realPath), - createContentHash: (content: string) => `hash:${content}`, - isDuplicateByContentHash: (hash: string, cache: Set) => cache.has(hash), - }); + return createRuleInjectionProcessor({ + workspaceDirectory: projectRoot, + truncator: { + truncate: async (_sessionID: string, content: string) => ({ + result: content, + truncated: false, + }), + }, + getSessionCache: (sessionID: string) => { + if (!sessionCaches.has(sessionID)) { + sessionCaches.set(sessionID, { + contentHashes: new Set(), + realPaths: new Set(), + }); + } + const cache = sessionCaches.get(sessionID); + if (!cache) { + throw new Error("Session cache should exist"); + } + return cache; + }, + readFileSync: (filePath: string, encoding: "utf-8") => { + if (filePath === trackedRulePath) { + trackedReadFileCount += 1; + } + return originalReadFileSync(filePath, encoding); + }, + statSync: (filePath: fs.PathLike) => { + if (filePath === trackedRulePath) { + const next = statSnapshots.shift(); + if (next instanceof Error) { + throw next; + } + if (next) { + return { + mtimeMs: next.mtimeMs, + size: next.size, + isFile: () => true, + } as ReturnType; + } + } + return originalStatSync(filePath); + }, + homedir: () => mockedHomeDir || originalHomedir(), + shouldApplyRule: () => { + trackedShouldApplyRuleCount += 1; + return { applies: true, reason: "matched" }; + }, + isDuplicateByRealPath: (realPath: string, cache: Set) => + cache.has(realPath), + createContentHash: (content: string) => `hash:${content}`, + isDuplicateByContentHash: (hash: string, cache: Set) => + cache.has(hash), + }); } function getInjectedRulesPath(sessionID: string): string { - return join(RULES_INJECTOR_STORAGE, `${sessionID}.json`); + return join(RULES_INJECTOR_STORAGE, `${sessionID}.json`); } describe("createRuleInjectionProcessor", () => { - let testRoot: string; - let projectRoot: string; - let homeRoot: string; - let targetFile: string; - let ruleFile: string; - let ruleRealPath: string; + let testRoot: string; + let projectRoot: string; + let homeRoot: string; + let targetFile: string; + let ruleFile: string; + let ruleRealPath: string; - beforeEach(() => { - clearParsedRuleCache(); - testRoot = join(tmpdir(), `rules-injector-injector-${Date.now()}`); - projectRoot = join(testRoot, "project"); - homeRoot = join(testRoot, "home"); - targetFile = join(projectRoot, "src", "index.ts"); - ruleFile = join( - projectRoot, - ".github", - "instructions", - "typescript.instructions.md" - ); + beforeEach(() => { + clearParsedRuleCache(); + testRoot = join(tmpdir(), `rules-injector-injector-${Date.now()}`); + projectRoot = join(testRoot, "project"); + homeRoot = join(testRoot, "home"); + targetFile = join(projectRoot, "src", "index.ts"); + ruleFile = join( + projectRoot, + ".github", + "instructions", + "typescript.instructions.md", + ); - mkdirSync(join(projectRoot, ".git"), { recursive: true }); - mkdirSync(join(projectRoot, "src"), { recursive: true }); - mkdirSync(join(projectRoot, ".github", "instructions"), { recursive: true }); - mkdirSync(homeRoot, { recursive: true }); + mkdirSync(join(projectRoot, ".git"), { recursive: true }); + mkdirSync(join(projectRoot, "src"), { recursive: true }); + mkdirSync(join(projectRoot, ".github", "instructions"), { + recursive: true, + }); + mkdirSync(homeRoot, { recursive: true }); - writeFileSync(targetFile, "export const value = 1;\n"); - writeFileSync(ruleFile, "rule-content\n"); + writeFileSync(targetFile, "export const value = 1;\n"); + writeFileSync(ruleFile, "rule-content\n"); - ruleRealPath = fs.realpathSync(ruleFile); - trackedRulePath = ruleFile; - statSnapshots = []; - trackedReadFileCount = 0; - mockedHomeDir = homeRoot; - }); + ruleRealPath = fs.realpathSync(ruleFile); + trackedRulePath = ruleFile; + statSnapshots = []; + trackedReadFileCount = 0; + trackedShouldApplyRuleCount = 0; + mockedHomeDir = homeRoot; + }); - afterEach(() => { - clearParsedRuleCache(); - if (fs.existsSync(testRoot)) { - rmSync(testRoot, { recursive: true, force: true }); - } - }); + afterEach(() => { + clearParsedRuleCache(); + if (fs.existsSync(testRoot)) { + rmSync(testRoot, { recursive: true, force: true }); + } + }); - it("reads and parses same file once when stat is unchanged", async () => { - // given - statSnapshots = [ - { mtimeMs: 1000, size: 13 }, - { mtimeMs: 1000, size: 13 }, - ]; - const processor = await createProcessor(projectRoot); + it("reads and parses same file once when stat is unchanged", async () => { + // given + statSnapshots = [ + { mtimeMs: 1000, size: 13 }, + { mtimeMs: 1000, size: 13 }, + ]; + const processor = await createProcessor(projectRoot); - // when - await processor.processFilePathForInjection(targetFile, "session-1", createOutput()); - await processor.processFilePathForInjection(targetFile, "session-2", createOutput()); + // when + await processor.processFilePathForInjection( + targetFile, + "session-1", + createOutput(), + ); + await processor.processFilePathForInjection( + targetFile, + "session-2", + createOutput(), + ); - // then - expect(trackedReadFileCount).toBe(1); - }); + // then + expect(trackedReadFileCount).toBe(1); + }); - it("re-reads file when mtime changes", async () => { - // given - statSnapshots = [ - { mtimeMs: 1000, size: 13 }, - { mtimeMs: 2000, size: 13 }, - ]; - const processor = await createProcessor(projectRoot); + it("re-reads file when mtime changes", async () => { + // given + statSnapshots = [ + { mtimeMs: 1000, size: 13 }, + { mtimeMs: 2000, size: 13 }, + ]; + const processor = await createProcessor(projectRoot); - // when - await processor.processFilePathForInjection(targetFile, "session-1", createOutput()); - await processor.processFilePathForInjection(targetFile, "session-2", createOutput()); + // when + await processor.processFilePathForInjection( + targetFile, + "session-1", + createOutput(), + ); + await processor.processFilePathForInjection( + targetFile, + "session-2", + createOutput(), + ); - // then - expect(trackedReadFileCount).toBe(2); - }); + // then + expect(trackedReadFileCount).toBe(2); + }); - it("re-reads file when size changes", async () => { - // given - statSnapshots = [ - { mtimeMs: 1000, size: 13 }, - { mtimeMs: 1000, size: 21 }, - ]; - const processor = await createProcessor(projectRoot); + it("re-reads file when size changes", async () => { + // given + statSnapshots = [ + { mtimeMs: 1000, size: 13 }, + { mtimeMs: 1000, size: 21 }, + ]; + const processor = await createProcessor(projectRoot); - // when - await processor.processFilePathForInjection(targetFile, "session-1", createOutput()); - await processor.processFilePathForInjection(targetFile, "session-2", createOutput()); + // when + await processor.processFilePathForInjection( + targetFile, + "session-1", + createOutput(), + ); + await processor.processFilePathForInjection( + targetFile, + "session-2", + createOutput(), + ); - // then - expect(trackedReadFileCount).toBe(2); - }); + // then + expect(trackedReadFileCount).toBe(2); + }); - it("does not cache oversized parsed rule bodies", async () => { - // given - const largeBody = "x".repeat(70 * 1024); - writeFileSync(ruleFile, largeBody); - statSnapshots = [ - { mtimeMs: 1000, size: largeBody.length }, - { mtimeMs: 1000, size: largeBody.length }, - ]; - const processor = await createProcessor(projectRoot); + it("reuses match decision when stat fingerprint and target are unchanged", async () => { + // given + statSnapshots = [ + { mtimeMs: 1000, size: 13 }, + { mtimeMs: 1000, size: 13 }, + ]; + const processor = await createProcessor(projectRoot); - // when - await processor.processFilePathForInjection(targetFile, "session-1", createOutput()); - await processor.processFilePathForInjection(targetFile, "session-2", createOutput()); + // when + await processor.processFilePathForInjection( + targetFile, + "session-1", + createOutput(), + ); + await processor.processFilePathForInjection( + targetFile, + "session-2", + createOutput(), + ); - // then - expect(trackedReadFileCount).toBe(2); - expect(getParsedRuleCacheStats()).toEqual({ entries: 0, bodyBytes: 0 }); - }); + // then + expect(trackedShouldApplyRuleCount).toBe(1); + }); - it("does not save injected rules when all candidates are already cached", async () => { - // given - const sessionID = `dirty-no-new-${Date.now()}`; - const injectedPath = getInjectedRulesPath(sessionID); - if (fs.existsSync(injectedPath)) { - fs.unlinkSync(injectedPath); - } + it("re-evaluates match decision when stat fingerprint changes", async () => { + // given + statSnapshots = [ + { mtimeMs: 1000, size: 13 }, + { mtimeMs: 2000, size: 13 }, + ]; + const processor = await createProcessor(projectRoot); - const { createRuleInjectionProcessor } = await import("./injector"); - const processor = createRuleInjectionProcessor({ - workspaceDirectory: projectRoot, - truncator: { - truncate: async (_sessionID: string, content: string) => ({ - result: content, - truncated: false, - }), - }, - getSessionCache: () => ({ - contentHashes: new Set(), - realPaths: new Set([ruleRealPath]), - }), - }); + // when + await processor.processFilePathForInjection( + targetFile, + "session-1", + createOutput(), + ); + await processor.processFilePathForInjection( + targetFile, + "session-2", + createOutput(), + ); - // when - await processor.processFilePathForInjection(targetFile, sessionID, createOutput()); + // then + expect(trackedShouldApplyRuleCount).toBe(2); + }); - // then - expect(fs.existsSync(injectedPath)).toBe(false); - }); + it("keeps match decisions separate for different target files", async () => { + // given + const secondTargetFile = join(projectRoot, "src", "other.ts"); + writeFileSync(secondTargetFile, "export const other = 2;\n"); + statSnapshots = [ + { mtimeMs: 1000, size: 13 }, + { mtimeMs: 1000, size: 13 }, + ]; + const processor = await createProcessor(projectRoot); - it("saves injected rules when a new rule is added", async () => { - // given - const sessionID = `dirty-new-${Date.now()}`; - const injectedPath = getInjectedRulesPath(sessionID); - if (fs.existsSync(injectedPath)) { - fs.unlinkSync(injectedPath); - } - const processor = await createProcessor(projectRoot); + // when + await processor.processFilePathForInjection( + targetFile, + "session-1", + createOutput(), + ); + await processor.processFilePathForInjection( + secondTargetFile, + "session-2", + createOutput(), + ); - // when - await processor.processFilePathForInjection(targetFile, sessionID, createOutput()); + // then + expect(trackedShouldApplyRuleCount).toBe(2); + }); - // then - expect(fs.existsSync(injectedPath)).toBe(true); + it("does not cache oversized parsed rule bodies", async () => { + // given + const largeBody = "x".repeat(70 * 1024); + writeFileSync(ruleFile, largeBody); + statSnapshots = [ + { mtimeMs: 1000, size: largeBody.length }, + { mtimeMs: 1000, size: largeBody.length }, + ]; + const processor = await createProcessor(projectRoot); - if (fs.existsSync(injectedPath)) { - fs.unlinkSync(injectedPath); - } - }); + // when + await processor.processFilePathForInjection( + targetFile, + "session-1", + createOutput(), + ); + await processor.processFilePathForInjection( + targetFile, + "session-2", + createOutput(), + ); - it("falls back to direct read and parse when statSync throws", async () => { - // given - statSnapshots = [new Error("stat failed"), new Error("stat failed")]; - const processor = await createProcessor(projectRoot); + // then + expect(trackedReadFileCount).toBe(2); + expect(getParsedRuleCacheStats()).toEqual({ entries: 0, bodyBytes: 0 }); + }); - // when - await processor.processFilePathForInjection(targetFile, "session-1", createOutput()); - await processor.processFilePathForInjection(targetFile, "session-2", createOutput()); + it("does not save injected rules when all candidates are already cached", async () => { + // given + const sessionID = `dirty-no-new-${Date.now()}`; + const injectedPath = getInjectedRulesPath(sessionID); + if (fs.existsSync(injectedPath)) { + fs.unlinkSync(injectedPath); + } - // then - expect(trackedReadFileCount).toBe(2); - }); + const { createRuleInjectionProcessor } = await import("./injector"); + const processor = createRuleInjectionProcessor({ + workspaceDirectory: projectRoot, + truncator: { + truncate: async (_sessionID: string, content: string) => ({ + result: content, + truncated: false, + }), + }, + getSessionCache: () => ({ + contentHashes: new Set(), + realPaths: new Set([ruleRealPath]), + }), + }); + + // when + await processor.processFilePathForInjection( + targetFile, + sessionID, + createOutput(), + ); + + // then + expect(fs.existsSync(injectedPath)).toBe(false); + }); + + it("saves injected rules when a new rule is added", async () => { + // given + const sessionID = `dirty-new-${Date.now()}`; + const injectedPath = getInjectedRulesPath(sessionID); + if (fs.existsSync(injectedPath)) { + fs.unlinkSync(injectedPath); + } + const processor = await createProcessor(projectRoot); + + // when + await processor.processFilePathForInjection( + targetFile, + sessionID, + createOutput(), + ); + + // then + expect(fs.existsSync(injectedPath)).toBe(true); + + if (fs.existsSync(injectedPath)) { + fs.unlinkSync(injectedPath); + } + }); + + it("falls back to direct read and parse when statSync throws", async () => { + // given + statSnapshots = [new Error("stat failed"), new Error("stat failed")]; + const processor = await createProcessor(projectRoot); + + // when + await processor.processFilePathForInjection( + targetFile, + "session-1", + createOutput(), + ); + await processor.processFilePathForInjection( + targetFile, + "session-2", + createOutput(), + ); + + // then + expect(trackedReadFileCount).toBe(2); + expect(trackedShouldApplyRuleCount).toBe(2); + }); }); diff --git a/src/hooks/rules-injector/injector.ts b/src/hooks/rules-injector/injector.ts index 8dc517c58..cd23195d3 100644 --- a/src/hooks/rules-injector/injector.ts +++ b/src/hooks/rules-injector/injector.ts @@ -4,10 +4,10 @@ import { relative, resolve } from "node:path"; import { findProjectRoot, findRuleFiles } from "./finder"; import type { FindRuleFilesOptions } from "./rule-file-finder"; import { - createContentHash, - isDuplicateByContentHash, - isDuplicateByRealPath, - shouldApplyRule, + createContentHash, + isDuplicateByContentHash, + isDuplicateByRealPath, + shouldApplyRule, } from "./matcher"; import { parseRuleFrontmatter } from "./parser"; import { saveInjectedRules } from "./storage"; @@ -16,216 +16,336 @@ import type { RuleScanCache } from "./rule-scan-cache"; import type { RuleMetadata } from "./types"; type ToolExecuteOutput = { - title: string; - output: string; - metadata: unknown; + title: string; + output: string; + metadata: unknown; }; type RuleToInject = { - relativePath: string; - matchReason: string; - content: string; - distance: number; + relativePath: string; + matchReason: string; + content: string; + distance: number; }; type DynamicTruncator = { - truncate: ( - sessionID: string, - content: string - ) => Promise<{ result: string; truncated: boolean }>; + truncate: ( + sessionID: string, + content: string, + ) => Promise<{ result: string; truncated: boolean }>; }; type RuleFileReader = (path: string, encoding: "utf-8") => string; interface ParsedRuleEntry { - mtimeMs: number; - size: number; - metadata: RuleMetadata; - body: string; + mtimeMs: number; + size: number; + metadata: RuleMetadata; + body: string; } +type ParsedRule = { + metadata: RuleMetadata; + body: string; + statFingerprint: string | null; +}; + +type MatchDecisionCache = Map; + export interface ParsedRuleCacheStats { - entries: number; - bodyBytes: number; + entries: number; + bodyBytes: number; } const MAX_PARSED_RULE_CACHE_ENTRIES = 256; const MAX_PARSED_RULE_CACHE_BODY_BYTES = 64 * 1024; +const MAX_MATCH_DECISION_CACHE_ENTRIES = 4096; const parsedRuleCache = new Map(); export function clearParsedRuleCache(): void { - parsedRuleCache.clear(); + parsedRuleCache.clear(); } export function getParsedRuleCacheStats(): ParsedRuleCacheStats { - let bodyBytes = 0; - for (const entry of parsedRuleCache.values()) { - bodyBytes += Buffer.byteLength(entry.body, "utf8"); - } - return { entries: parsedRuleCache.size, bodyBytes }; + let bodyBytes = 0; + for (const entry of parsedRuleCache.values()) { + bodyBytes += Buffer.byteLength(entry.body, "utf8"); + } + return { entries: parsedRuleCache.size, bodyBytes }; } -function setParsedRuleCacheEntry(realPath: string, entry: ParsedRuleEntry): void { - if (Buffer.byteLength(entry.body, "utf8") > MAX_PARSED_RULE_CACHE_BODY_BYTES) return; - if (parsedRuleCache.size >= MAX_PARSED_RULE_CACHE_ENTRIES) { - const oldestRealPath = parsedRuleCache.keys().next().value; - if (oldestRealPath !== undefined) { - parsedRuleCache.delete(oldestRealPath); - } - } - parsedRuleCache.set(realPath, entry); +function setParsedRuleCacheEntry( + realPath: string, + entry: ParsedRuleEntry, +): void { + if (Buffer.byteLength(entry.body, "utf8") > MAX_PARSED_RULE_CACHE_BODY_BYTES) + return; + if (parsedRuleCache.size >= MAX_PARSED_RULE_CACHE_ENTRIES) { + const oldestRealPath = parsedRuleCache.keys().next().value; + if (oldestRealPath !== undefined) { + parsedRuleCache.delete(oldestRealPath); + } + } + parsedRuleCache.set(realPath, entry); } function resolveFilePath( - workspaceDirectory: string, - path: string + workspaceDirectory: string, + path: string, ): string | null { - if (!path) return null; - if (path.startsWith("/")) return path; - return resolve(workspaceDirectory, path); + if (!path) return null; + if (path.startsWith("/")) return path; + return resolve(workspaceDirectory, path); } export function createRuleInjectionProcessor(deps: { - workspaceDirectory: string; - truncator: DynamicTruncator; - getSessionCache: (sessionID: string) => SessionInjectedRulesCache; - getSessionRuleScanCache?: (sessionID: string) => RuleScanCache; - ruleFinderOptions?: FindRuleFilesOptions; - readFileSync?: RuleFileReader; - statSync?: typeof statSync; - homedir?: typeof homedir; - shouldApplyRule?: typeof shouldApplyRule; - isDuplicateByRealPath?: typeof isDuplicateByRealPath; - createContentHash?: typeof createContentHash; - isDuplicateByContentHash?: typeof isDuplicateByContentHash; - saveInjectedRules?: typeof saveInjectedRules; + workspaceDirectory: string; + truncator: DynamicTruncator; + getSessionCache: (sessionID: string) => SessionInjectedRulesCache; + getSessionRuleScanCache?: (sessionID: string) => RuleScanCache; + ruleFinderOptions?: FindRuleFilesOptions; + readFileSync?: RuleFileReader; + statSync?: typeof statSync; + homedir?: typeof homedir; + shouldApplyRule?: typeof shouldApplyRule; + isDuplicateByRealPath?: typeof isDuplicateByRealPath; + createContentHash?: typeof createContentHash; + isDuplicateByContentHash?: typeof isDuplicateByContentHash; + saveInjectedRules?: typeof saveInjectedRules; }): { - processFilePathForInjection: ( - filePath: string, - sessionID: string, - output: ToolExecuteOutput - ) => Promise; + processFilePathForInjection: ( + filePath: string, + sessionID: string, + output: ToolExecuteOutput, + ) => Promise; } { - const { - workspaceDirectory, - truncator, - getSessionCache, - getSessionRuleScanCache, - ruleFinderOptions, - readFileSync: readRuleFileSync = readFileSync, - statSync: statRuleSync = statSync, - homedir: getHomeDir = homedir, - shouldApplyRule: shouldApplyRuleImpl = shouldApplyRule, - isDuplicateByRealPath: isDuplicateByRealPathImpl = isDuplicateByRealPath, - createContentHash: createContentHashImpl = createContentHash, - isDuplicateByContentHash: isDuplicateByContentHashImpl = isDuplicateByContentHash, - saveInjectedRules: saveInjectedRulesImpl = saveInjectedRules, - } = deps; + const { + workspaceDirectory, + truncator, + getSessionCache, + getSessionRuleScanCache, + ruleFinderOptions, + readFileSync: readRuleFileSync = readFileSync, + statSync: statRuleSync = statSync, + homedir: getHomeDir = homedir, + shouldApplyRule: shouldApplyRuleImpl = shouldApplyRule, + isDuplicateByRealPath: isDuplicateByRealPathImpl = isDuplicateByRealPath, + createContentHash: createContentHashImpl = createContentHash, + isDuplicateByContentHash: + isDuplicateByContentHashImpl = isDuplicateByContentHash, + saveInjectedRules: saveInjectedRulesImpl = saveInjectedRules, + } = deps; - function getParsedRule(filePath: string, realPath: string): { metadata: RuleMetadata; body: string } { - try { - const stat = statRuleSync(filePath); - const cached = parsedRuleCache.get(realPath); + const matchDecisionCache: MatchDecisionCache = new Map(); - if (cached && cached.mtimeMs === stat.mtimeMs && cached.size === stat.size) { - return { metadata: cached.metadata, body: cached.body }; - } + function getParsedRule(filePath: string, realPath: string): ParsedRule { + try { + const stat = statRuleSync(filePath); + const statFingerprint = `${stat.mtimeMs}:${stat.size}`; + const cached = parsedRuleCache.get(realPath); - const rawContent = readRuleFileSync(filePath, "utf-8"); - const { metadata, body } = parseRuleFrontmatter(rawContent); - setParsedRuleCacheEntry(realPath, { - mtimeMs: stat.mtimeMs, - size: stat.size, - metadata, - body, - }); - return { metadata, body }; - } catch { - const rawContent = readRuleFileSync(filePath, "utf-8"); - return parseRuleFrontmatter(rawContent); - } - } + if ( + cached && + cached.mtimeMs === stat.mtimeMs && + cached.size === stat.size + ) { + return { + metadata: cached.metadata, + body: cached.body, + statFingerprint, + }; + } - async function processFilePathForInjection( - filePath: string, - sessionID: string, - output: ToolExecuteOutput - ): Promise { - const resolved = resolveFilePath(workspaceDirectory, filePath); - if (!resolved) return; + const rawContent = readRuleFileSync(filePath, "utf-8"); + const { metadata, body } = parseRuleFrontmatter(rawContent); + setParsedRuleCacheEntry(realPath, { + mtimeMs: stat.mtimeMs, + size: stat.size, + metadata, + body, + }); + return { metadata, body, statFingerprint }; + } catch { + const rawContent = readRuleFileSync(filePath, "utf-8"); + const { metadata, body } = parseRuleFrontmatter(rawContent); + return { metadata, body, statFingerprint: null }; + } + } - const projectRoot = findProjectRoot(resolved); - const cache = getSessionCache(sessionID); - const ruleScanCache = getSessionRuleScanCache?.(sessionID); - const home = getHomeDir(); + async function processFilePathForInjection( + filePath: string, + sessionID: string, + output: ToolExecuteOutput, + ): Promise { + const resolved = resolveFilePath(workspaceDirectory, filePath); + if (!resolved) return; - const ruleFileCandidates = findRuleFiles( - projectRoot, - home, - resolved, - ruleFinderOptions, - ruleScanCache, - ); - const toInject: RuleToInject[] = []; - let dirty = false; + const projectRoot = findProjectRoot(resolved); + const cache = getSessionCache(sessionID); + const ruleScanCache = getSessionRuleScanCache?.(sessionID); + const home = getHomeDir(); - for (const candidate of ruleFileCandidates) { - if (isDuplicateByRealPathImpl(candidate.realPath, cache.realPaths)) continue; + const ruleFileCandidates = findRuleFiles( + projectRoot, + home, + resolved, + ruleFinderOptions, + ruleScanCache, + ); + const toInject: RuleToInject[] = []; + let dirty = false; - try { - const { metadata, body } = getParsedRule( - candidate.path, - candidate.realPath - ); + for (const candidate of ruleFileCandidates) { + if (isDuplicateByRealPathImpl(candidate.realPath, cache.realPaths)) + continue; - let matchReason: string; - if (candidate.isSingleFile) { - matchReason = "copilot-instructions (always apply)"; - } else { - const matchResult = shouldApplyRuleImpl(metadata, resolved, projectRoot); - if (!matchResult.applies) continue; - matchReason = matchResult.reason ?? "matched"; - } + try { + const { metadata, body, statFingerprint } = getParsedRule( + candidate.path, + candidate.realPath, + ); - const contentHash = createContentHashImpl(body); - if (isDuplicateByContentHashImpl(contentHash, cache.contentHashes)) continue; + let matchReason: string; + if (candidate.isSingleFile) { + matchReason = "copilot-instructions (always apply)"; + } else { + const cachedMatchReason = getCachedMatchReason( + matchDecisionCache, + projectRoot, + resolved, + candidate.realPath, + statFingerprint, + ); + if (cachedMatchReason !== undefined) { + if (cachedMatchReason === null) continue; + matchReason = cachedMatchReason; + } else { + const matchResult = shouldApplyRuleImpl( + metadata, + resolved, + projectRoot, + ); + if (!matchResult.applies) { + setCachedMatchReason( + matchDecisionCache, + projectRoot, + resolved, + candidate.realPath, + statFingerprint, + null, + ); + continue; + } + matchReason = matchResult.reason ?? "matched"; + setCachedMatchReason( + matchDecisionCache, + projectRoot, + resolved, + candidate.realPath, + statFingerprint, + matchReason, + ); + } + } - const relativePath = projectRoot - ? relative(projectRoot, candidate.path) - : candidate.path; + const contentHash = createContentHashImpl(body); + if (isDuplicateByContentHashImpl(contentHash, cache.contentHashes)) + continue; - toInject.push({ - relativePath, - matchReason, - content: body, - distance: candidate.distance, - }); + const relativePath = projectRoot + ? relative(projectRoot, candidate.path) + : candidate.path; - cache.realPaths.add(candidate.realPath); - cache.contentHashes.add(contentHash); - dirty = true; - } catch {} - } + toInject.push({ + relativePath, + matchReason, + content: body, + distance: candidate.distance, + }); - if (toInject.length === 0) return; + cache.realPaths.add(candidate.realPath); + cache.contentHashes.add(contentHash); + dirty = true; + } catch {} + } - toInject.sort((a, b) => a.distance - b.distance); + if (toInject.length === 0) return; - for (const rule of toInject) { - const { result, truncated } = await truncator.truncate( - sessionID, - rule.content - ); - const truncationNotice = truncated - ? `\n\n[Note: Content was truncated to save context window space. For full context, please read the file directly: ${rule.relativePath}]` - : ""; - output.output += `\n\n[Rule: ${rule.relativePath}]\n[Match: ${rule.matchReason}]\n${result}${truncationNotice}`; - } + toInject.sort((a, b) => a.distance - b.distance); - if (dirty) { - saveInjectedRulesImpl(sessionID, cache); - } - } + for (const rule of toInject) { + const { result, truncated } = await truncator.truncate( + sessionID, + rule.content, + ); + const truncationNotice = truncated + ? `\n\n[Note: Content was truncated to save context window space. For full context, please read the file directly: ${rule.relativePath}]` + : ""; + output.output += `\n\n[Rule: ${rule.relativePath}]\n[Match: ${rule.matchReason}]\n${result}${truncationNotice}`; + } - return { processFilePathForInjection }; + if (dirty) { + saveInjectedRulesImpl(sessionID, cache); + } + } + + return { processFilePathForInjection }; +} + +function getCachedMatchReason( + cache: MatchDecisionCache, + projectRoot: string | null, + resolvedFilePath: string, + realPath: string, + statFingerprint: string | null, +): string | null | undefined { + const cacheKey = matchDecisionCacheKey( + projectRoot, + resolvedFilePath, + realPath, + statFingerprint, + ); + if (cacheKey === null || !cache.has(cacheKey)) return undefined; + + const cached = cache.get(cacheKey) ?? null; + cache.delete(cacheKey); + cache.set(cacheKey, cached); + return cached; +} + +function setCachedMatchReason( + cache: MatchDecisionCache, + projectRoot: string | null, + resolvedFilePath: string, + realPath: string, + statFingerprint: string | null, + matchReason: string | null, +): void { + const cacheKey = matchDecisionCacheKey( + projectRoot, + resolvedFilePath, + realPath, + statFingerprint, + ); + if (cacheKey === null) return; + + if (cache.size >= MAX_MATCH_DECISION_CACHE_ENTRIES) { + const oldestCacheKey = cache.keys().next().value; + if (oldestCacheKey !== undefined) { + cache.delete(oldestCacheKey); + } + } + cache.set(cacheKey, matchReason); +} + +function matchDecisionCacheKey( + projectRoot: string | null, + resolvedFilePath: string, + realPath: string, + statFingerprint: string | null, +): string | null { + if (statFingerprint === null) return null; + return [projectRoot ?? "", resolvedFilePath, realPath, statFingerprint].join( + "\0", + ); }