diff --git a/src/__tests__/perf/fixtures/in-tree/AGENTS.md b/src/__tests__/perf/fixtures/in-tree/AGENTS.md new file mode 100644 index 000000000..22257f9ad --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/AGENTS.md @@ -0,0 +1 @@ +# fixture root diff --git a/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/AGENTS.md b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/AGENTS.md new file mode 100644 index 000000000..6bc3f0b2c --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/AGENTS.md @@ -0,0 +1 @@ +# fixture package diff --git a/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-16.ts b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-16.ts new file mode 100644 index 000000000..dad26290a --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-16.ts @@ -0,0 +1 @@ +export const file16 = 16 diff --git a/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-17.ts b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-17.ts new file mode 100644 index 000000000..01e60135f --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-17.ts @@ -0,0 +1 @@ +export const file17 = 17 diff --git a/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-18.ts b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-18.ts new file mode 100644 index 000000000..000ce187b --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-18.ts @@ -0,0 +1 @@ +export const file18 = 18 diff --git a/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-19.ts b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-19.ts new file mode 100644 index 000000000..43ebccb94 --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-19.ts @@ -0,0 +1 @@ +export const file19 = 19 diff --git a/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-20.ts b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-20.ts new file mode 100644 index 000000000..763bfe44f --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/packages/pkg-one/src/file-20.ts @@ -0,0 +1 @@ +export const file20 = 20 diff --git a/src/__tests__/perf/fixtures/in-tree/src/AGENTS.md b/src/__tests__/perf/fixtures/in-tree/src/AGENTS.md new file mode 100644 index 000000000..df55bdcda --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/AGENTS.md @@ -0,0 +1 @@ +# fixture src diff --git a/src/__tests__/perf/fixtures/in-tree/src/app/file-01.ts b/src/__tests__/perf/fixtures/in-tree/src/app/file-01.ts new file mode 100644 index 000000000..8a4e4907d --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/app/file-01.ts @@ -0,0 +1 @@ +export const file01 = 1 diff --git a/src/__tests__/perf/fixtures/in-tree/src/app/file-02.ts b/src/__tests__/perf/fixtures/in-tree/src/app/file-02.ts new file mode 100644 index 000000000..20ca96c14 --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/app/file-02.ts @@ -0,0 +1 @@ +export const file02 = 2 diff --git a/src/__tests__/perf/fixtures/in-tree/src/app/file-03.ts b/src/__tests__/perf/fixtures/in-tree/src/app/file-03.ts new file mode 100644 index 000000000..b7a0ab9bd --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/app/file-03.ts @@ -0,0 +1 @@ +export const file03 = 3 diff --git a/src/__tests__/perf/fixtures/in-tree/src/app/file-04.ts b/src/__tests__/perf/fixtures/in-tree/src/app/file-04.ts new file mode 100644 index 000000000..5917ea7a4 --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/app/file-04.ts @@ -0,0 +1 @@ +export const file04 = 4 diff --git a/src/__tests__/perf/fixtures/in-tree/src/app/file-05.ts b/src/__tests__/perf/fixtures/in-tree/src/app/file-05.ts new file mode 100644 index 000000000..7c842b808 --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/app/file-05.ts @@ -0,0 +1 @@ +export const file05 = 5 diff --git a/src/__tests__/perf/fixtures/in-tree/src/app/file-06.ts b/src/__tests__/perf/fixtures/in-tree/src/app/file-06.ts new file mode 100644 index 000000000..b48d2d1cd --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/app/file-06.ts @@ -0,0 +1 @@ +export const file06 = 6 diff --git a/src/__tests__/perf/fixtures/in-tree/src/app/file-07.ts b/src/__tests__/perf/fixtures/in-tree/src/app/file-07.ts new file mode 100644 index 000000000..9de6f660f --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/app/file-07.ts @@ -0,0 +1 @@ +export const file07 = 7 diff --git a/src/__tests__/perf/fixtures/in-tree/src/app/file-08.ts b/src/__tests__/perf/fixtures/in-tree/src/app/file-08.ts new file mode 100644 index 000000000..2f24a3912 --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/app/file-08.ts @@ -0,0 +1 @@ +export const file08 = 8 diff --git a/src/__tests__/perf/fixtures/in-tree/src/app/file-09.ts b/src/__tests__/perf/fixtures/in-tree/src/app/file-09.ts new file mode 100644 index 000000000..2c4cddbd4 --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/app/file-09.ts @@ -0,0 +1 @@ +export const file09 = 9 diff --git a/src/__tests__/perf/fixtures/in-tree/src/app/file-10.ts b/src/__tests__/perf/fixtures/in-tree/src/app/file-10.ts new file mode 100644 index 000000000..1d329a0dc --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/app/file-10.ts @@ -0,0 +1 @@ +export const file10 = 10 diff --git a/src/__tests__/perf/fixtures/in-tree/src/lib/file-11.ts b/src/__tests__/perf/fixtures/in-tree/src/lib/file-11.ts new file mode 100644 index 000000000..eb1a64844 --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/lib/file-11.ts @@ -0,0 +1 @@ +export const file11 = 11 diff --git a/src/__tests__/perf/fixtures/in-tree/src/lib/file-12.ts b/src/__tests__/perf/fixtures/in-tree/src/lib/file-12.ts new file mode 100644 index 000000000..6dbff13ec --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/lib/file-12.ts @@ -0,0 +1 @@ +export const file12 = 12 diff --git a/src/__tests__/perf/fixtures/in-tree/src/lib/file-13.ts b/src/__tests__/perf/fixtures/in-tree/src/lib/file-13.ts new file mode 100644 index 000000000..5a46ab064 --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/lib/file-13.ts @@ -0,0 +1 @@ +export const file13 = 13 diff --git a/src/__tests__/perf/fixtures/in-tree/src/lib/file-14.ts b/src/__tests__/perf/fixtures/in-tree/src/lib/file-14.ts new file mode 100644 index 000000000..32824f748 --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/lib/file-14.ts @@ -0,0 +1 @@ +export const file14 = 14 diff --git a/src/__tests__/perf/fixtures/in-tree/src/lib/file-15.ts b/src/__tests__/perf/fixtures/in-tree/src/lib/file-15.ts new file mode 100644 index 000000000..c0d19485f --- /dev/null +++ b/src/__tests__/perf/fixtures/in-tree/src/lib/file-15.ts @@ -0,0 +1 @@ +export const file15 = 15 diff --git a/src/__tests__/perf/plugin-init.test.ts b/src/__tests__/perf/plugin-init.test.ts new file mode 100644 index 000000000..1450b6557 --- /dev/null +++ b/src/__tests__/perf/plugin-init.test.ts @@ -0,0 +1,121 @@ +import { cpSync, mkdirSync, mkdtempSync, rmSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" + +import type { PluginInput } from "@opencode-ai/plugin" +import { createOpencodeClient } from "@opencode-ai/sdk" +import { describe, expect, it } from "bun:test" + +type InitMetrics = { + coldMs: number + warmMs: [number, number] + medianMs: number +} + +function getMedian(values: number[]): number { + const sorted = [...values].sort((left, right) => left - right) + return sorted[Math.floor(sorted.length / 2)] ?? 0 +} + +function createPluginInput(directory: string): PluginInput { + const client = createOpencodeClient({ directory }) + + return { + client, + project: { + id: `perf-${Date.now()}`, + worktree: directory, + time: { created: Date.now() }, + }, + directory, + worktree: directory, + serverUrl: new URL("http://localhost"), + $: Bun.$, + } +} + +async function importFreshPluginModule(): Promise<(typeof import("../../index"))["default"]> { + const token = `${Date.now()}-${Math.random()}` + return (await import(`../../index?perf=${token}`)).default +} + +async function measureInitMetrics(directory: string): Promise { + const pluginModule = await importFreshPluginModule() + const measurements: number[] = [] + + for (let index = 0; index < 3; index += 1) { + const input = createPluginInput(directory) + const start = performance.now() + await pluginModule.server(input, {}) + measurements.push(performance.now() - start) + } + + return { + coldMs: measurements[0] ?? 0, + warmMs: [measurements[1] ?? 0, measurements[2] ?? 0], + medianMs: getMedian(measurements), + } +} + +async function measureScenario( + label: string, + populateDirectory: (directory: string) => void, +): Promise { + const rootDirectory = mkdtempSync(join(tmpdir(), "perf-d09-")) + const projectDirectory = join(rootDirectory, label) + const configDirectory = join(rootDirectory, "opencode-config") + const previousConfigDirectory = process.env.OPENCODE_CONFIG_DIR + + mkdirSync(configDirectory, { recursive: true }) + process.env.OPENCODE_CONFIG_DIR = configDirectory + + try { + populateDirectory(projectDirectory) + return await measureInitMetrics(projectDirectory) + } finally { + if (previousConfigDirectory === undefined) { + delete process.env.OPENCODE_CONFIG_DIR + } else { + process.env.OPENCODE_CONFIG_DIR = previousConfigDirectory + } + + rmSync(rootDirectory, { recursive: true, force: true }) + } +} + +function logMetrics(label: string, metrics: InitMetrics): void { + console.info( + `${label}: cold=${metrics.coldMs.toFixed(1)}ms warm=[${metrics.warmMs.map((value) => value.toFixed(1)).join(", ")}] median=${metrics.medianMs.toFixed(1)}ms`, + ) +} + +describe("plugin init performance", () => { + it("stays within the empty project init budget", async () => { + // given + const metrics = await measureScenario("empty-project", (directory) => { + mkdirSync(directory, { recursive: true }) + }) + + // when + logMetrics("empty-project", metrics) + + // then + // regression budget + expect(metrics.medianMs).toBeLessThan(500) + }) + + it("stays within the in-tree fixture init budget", async () => { + // given + const fixtureDirectory = new URL("./fixtures/in-tree/", import.meta.url) + const metrics = await measureScenario("in-tree-fixture", (directory) => { + cpSync(fixtureDirectory, directory, { recursive: true }) + }) + + // when + logMetrics("in-tree-fixture", metrics) + + // then + // regression budget + expect(metrics.medianMs).toBeLessThan(700) + }) +}) diff --git a/src/features/claude-code-command-loader/loader-cache.ts b/src/features/claude-code-command-loader/loader-cache.ts new file mode 100644 index 000000000..9f0d4d195 --- /dev/null +++ b/src/features/claude-code-command-loader/loader-cache.ts @@ -0,0 +1,37 @@ +import { promises as fs } from "fs" +import { resolve } from "path" + +import type { CommandDefinition } from "./types" + +const commandLoaderCache = new Map>>() + +export async function getCommandLoaderCacheKey(directory?: string): Promise { + const resolvedDirectory = resolve(directory ?? process.cwd()) + + try { + return await fs.realpath(resolvedDirectory) + } catch { + return resolvedDirectory + } +} + +export function getCachedCommands( + cacheKey: string, +): Promise> | undefined { + return commandLoaderCache.get(cacheKey) +} + +export function setCachedCommands( + cacheKey: string, + commands: Promise>, +): void { + commandLoaderCache.set(cacheKey, commands) +} + +export function deleteCachedCommands(cacheKey: string): void { + commandLoaderCache.delete(cacheKey) +} + +export function clearCommandLoaderCache(): void { + commandLoaderCache.clear() +} diff --git a/src/features/claude-code-command-loader/loader.test.ts b/src/features/claude-code-command-loader/loader.test.ts index be7928d3f..b674f8ff9 100644 --- a/src/features/claude-code-command-loader/loader.test.ts +++ b/src/features/claude-code-command-loader/loader.test.ts @@ -1,9 +1,10 @@ import { execFileSync } from "node:child_process" -import { afterEach, beforeEach, describe, expect, it } from "bun:test" +import { promises as fs } from "node:fs" +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test" import { mkdirSync, rmSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" -import { loadOpencodeGlobalCommands, loadOpencodeProjectCommands } from "./loader" +import * as loader from "./loader" const TEST_DIR = join(tmpdir(), `claude-code-command-loader-${Date.now()}`) @@ -16,19 +17,41 @@ function writeCommand(directory: string, name: string, description: string): voi } describe("claude-code command loader", () => { + let originalClaudeConfigDir: string | undefined let originalOpencodeConfigDir: string | undefined beforeEach(() => { mkdirSync(TEST_DIR, { recursive: true }) + originalClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR originalOpencodeConfigDir = process.env.OPENCODE_CONFIG_DIR + + const claudeConfigDir = join(TEST_DIR, "claude-config") + const opencodeConfigDir = join(TEST_DIR, "opencode-config") + process.env.CLAUDE_CONFIG_DIR = claudeConfigDir + process.env.OPENCODE_CONFIG_DIR = opencodeConfigDir + + if ("clearCommandLoaderCache" in loader && typeof loader.clearCommandLoaderCache === "function") { + loader.clearCommandLoaderCache() + } }) afterEach(() => { + if (originalClaudeConfigDir === undefined) { + delete process.env.CLAUDE_CONFIG_DIR + } else { + process.env.CLAUDE_CONFIG_DIR = originalClaudeConfigDir + } + if (originalOpencodeConfigDir === undefined) { delete process.env.OPENCODE_CONFIG_DIR } else { process.env.OPENCODE_CONFIG_DIR = originalOpencodeConfigDir } + + if ("clearCommandLoaderCache" in loader && typeof loader.clearCommandLoaderCache === "function") { + loader.clearCommandLoaderCache() + } + rmSync(TEST_DIR, { recursive: true, force: true }) }) @@ -39,7 +62,7 @@ describe("claude-code command loader", () => { writeCommand(join(projectDir, ".opencode", "commands"), "ancestor", "Ancestor command") // when - const commands = await loadOpencodeProjectCommands(childDir) + const commands = await loader.loadOpencodeProjectCommands(childDir) // then expect(commands.ancestor?.description).toBe("(opencode-project) Ancestor command") @@ -50,7 +73,7 @@ describe("claude-code command loader", () => { writeCommand(join(TEST_DIR, ".opencode", "command"), "singular", "Singular command") // when - const commands = await loadOpencodeProjectCommands(TEST_DIR) + const commands = await loader.loadOpencodeProjectCommands(TEST_DIR) // then expect(commands.singular?.description).toBe("(opencode-project) Singular command") @@ -66,7 +89,7 @@ describe("claude-code command loader", () => { writeCommand(projectDir, "duplicate", "Nearest command") // when - const commands = await loadOpencodeProjectCommands(childDir) + const commands = await loader.loadOpencodeProjectCommands(childDir) // then expect(commands.duplicate?.description).toBe("(opencode-project) Nearest command") @@ -79,7 +102,7 @@ describe("claude-code command loader", () => { writeCommand(join(opencodeConfigDir, "commands"), "global-plural", "Global plural command") // when - const commands = await loadOpencodeGlobalCommands() + const commands = await loader.loadOpencodeGlobalCommands() // then expect(commands["global-plural"]?.description).toBe("(opencode) Global plural command") @@ -94,7 +117,7 @@ describe("claude-code command loader", () => { writeCommand(join(profileConfigDir, "commands"), "duplicate-global", "Profile global command") // when - const commands = await loadOpencodeGlobalCommands() + const commands = await loader.loadOpencodeGlobalCommands() // then expect(commands["duplicate-global"]?.description).toBe("(opencode) Profile global command") @@ -114,7 +137,7 @@ describe("claude-code command loader", () => { writeCommand(join(TEST_DIR, ".opencode", "commands"), "outside", "Outside command") // when - const commands = await loadOpencodeProjectCommands(nestedDirectory) + const commands = await loader.loadOpencodeProjectCommands(nestedDirectory) // then expect(commands["deploy/staging"]?.description).toBe("(opencode-project) Deploy staging") @@ -122,4 +145,38 @@ describe("claude-code command loader", () => { expect(commands.outside).toBeUndefined() expect(commands["deploy:staging"]).toBeUndefined() }) + + it("#given commands nested under an excluded basename #when loadProjectCommands is called #then it skips the excluded directory contents", async () => { + // given + writeCommand(join(TEST_DIR, ".claude", "commands"), "real", "Real command") + writeCommand( + join(TEST_DIR, ".claude", "commands", "node_modules"), + "fake", + "Fake command", + ) + + // when + const commands = await loader.loadProjectCommands(TEST_DIR) + + // then + expect(commands.real?.description).toBe("(project) Real command") + expect(commands.fake).toBeUndefined() + }) + + it("#given a previously loaded directory #when loadAllCommands is called twice #then the second call reuses the cached result without readdir calls", async () => { + // given + writeCommand(join(TEST_DIR, ".claude", "commands"), "cached", "Cached command") + const readdirSpy = spyOn(fs, "readdir") + + // when + const firstCommands = await loader.loadAllCommands(TEST_DIR) + const firstReaddirCount = readdirSpy.mock.calls.length + const secondCommands = await loader.loadAllCommands(TEST_DIR) + + // then + expect(firstCommands.cached?.description).toBe("(project) Cached command") + expect(secondCommands).toEqual(firstCommands) + expect(firstReaddirCount).toBeGreaterThan(0) + expect(readdirSpy.mock.calls.length).toBe(firstReaddirCount) + }) }) diff --git a/src/features/claude-code-command-loader/loader.ts b/src/features/claude-code-command-loader/loader.ts index b052f56bd..6ee178b66 100644 --- a/src/features/claude-code-command-loader/loader.ts +++ b/src/features/claude-code-command-loader/loader.ts @@ -4,13 +4,23 @@ import { parseFrontmatter } from "../../shared/frontmatter" import { sanitizeModelField } from "../../shared/model-sanitizer" import { isMarkdownFile } from "../../shared/file-utils" import { + EXCLUDED_DIRS, findProjectOpencodeCommandDirs, getClaudeConfigDir, getOpenCodeCommandDirs, } from "../../shared" import { log } from "../../shared/logger" +import { + clearCommandLoaderCache, + deleteCachedCommands, + getCachedCommands, + getCommandLoaderCacheKey, + setCachedCommands, +} from "./loader-cache" import type { CommandScope, CommandDefinition, CommandFrontmatter, LoadedCommand } from "./types" +export { clearCommandLoaderCache } + async function loadCommandsFromDir( commandsDir: string, scope: CommandScope, @@ -48,6 +58,7 @@ async function loadCommandsFromDir( for (const entry of entries) { if (entry.isDirectory()) { + if (EXCLUDED_DIRS.has(entry.name)) continue if (entry.name.startsWith(".")) continue const subDirPath = join(commandsDir, entry.name) const subPrefix = prefix ? `${prefix}/${entry.name}` : entry.name @@ -159,11 +170,26 @@ export async function loadOpencodeProjectCommands(directory?: string): Promise> { - const [user, project, global, projectOpencode] = await Promise.all([ + const cacheKey = await getCommandLoaderCacheKey(directory) + const cachedCommands = getCachedCommands(cacheKey) + if (cachedCommands) { + return cachedCommands + } + + const loadCommandsPromise = Promise.all([ loadUserCommands(), loadProjectCommands(directory), loadOpencodeGlobalCommands(), loadOpencodeProjectCommands(directory), ]) - return { ...projectOpencode, ...global, ...project, ...user } + .then(([user, project, global, projectOpencode]) => { + return { ...projectOpencode, ...global, ...project, ...user } + }) + .catch((error) => { + deleteCachedCommands(cacheKey) + throw error + }) + + setCachedCommands(cacheKey, loadCommandsPromise) + return loadCommandsPromise } diff --git a/src/features/opencode-skill-loader/skill-content.test.ts b/src/features/opencode-skill-loader/skill-content.test.ts index 64d6d5bf4..dedf74413 100644 --- a/src/features/opencode-skill-loader/skill-content.test.ts +++ b/src/features/opencode-skill-loader/skill-content.test.ts @@ -3,12 +3,19 @@ import { describe, it, expect, beforeEach, afterEach } from "bun:test" import { join } from "node:path" import { tmpdir } from "node:os" -import { resolveSkillContent, resolveMultipleSkills, resolveSkillContentAsync, resolveMultipleSkillsAsync } from "./skill-content" +import { + clearSkillCache, + resolveSkillContent, + resolveMultipleSkills, + resolveSkillContentAsync, + resolveMultipleSkillsAsync, +} from "./skill-content" let originalEnv: Record let testConfigDir: string beforeEach(() => { + clearSkillCache() originalEnv = { CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR, OPENCODE_CONFIG_DIR: process.env.OPENCODE_CONFIG_DIR, @@ -20,6 +27,7 @@ beforeEach(() => { }) afterEach(() => { + clearSkillCache() for (const [key, value] of Object.entries(originalEnv)) { if (value !== undefined) { process.env[key] = value diff --git a/src/hooks/auto-slash-command/executor.test.ts b/src/hooks/auto-slash-command/executor.test.ts index 246557275..0fe169cb6 100644 --- a/src/hooks/auto-slash-command/executor.test.ts +++ b/src/hooks/auto-slash-command/executor.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test" import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" +import { clearCommandLoaderCache } from "../../features/claude-code-command-loader" import { executeSlashCommand } from "./executor" const ENV_KEYS = [ @@ -95,6 +96,7 @@ describe("auto-slash command executor plugin dispatch", () => { let envSnapshot: EnvSnapshot beforeEach(() => { + clearCommandLoaderCache() tempDir = mkdtempSync(join(tmpdir(), "omo-executor-plugin-test-")) envSnapshot = { CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR, @@ -106,6 +108,7 @@ describe("auto-slash command executor plugin dispatch", () => { }) afterEach(() => { + clearCommandLoaderCache() for (const key of ENV_KEYS) { const previousValue = envSnapshot[key] if (previousValue === undefined) { diff --git a/src/hooks/auto-slash-command/index.test.ts b/src/hooks/auto-slash-command/index.test.ts index 543341b0b..cda63bf8c 100644 --- a/src/hooks/auto-slash-command/index.test.ts +++ b/src/hooks/auto-slash-command/index.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, beforeEach, afterEach, spyOn, mock } from "bun:te import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" +import { clearCommandLoaderCache } from "../../features/claude-code-command-loader" import type { LoadedSkill } from "../../features/opencode-skill-loader/types" import type { AutoSlashCommandHookInput, @@ -43,6 +44,7 @@ describe("createAutoSlashCommandHook", () => { let createAutoSlashCommandHook: AutoSlashCommandModule["createAutoSlashCommandHook"] beforeEach(async () => { + clearCommandLoaderCache() mock.restore() logCalls = [] spyOn(shared, "log").mockImplementation((message: string, data?: unknown) => { @@ -56,6 +58,7 @@ describe("createAutoSlashCommandHook", () => { }) afterEach(() => { + clearCommandLoaderCache() process.chdir(originalWorkingDirectory) rmSync(tempDir, { recursive: true, force: true }) mock.restore() diff --git a/src/hooks/auto-update-checker/hook.test.ts b/src/hooks/auto-update-checker/hook.test.ts new file mode 100644 index 000000000..a6cacd54d --- /dev/null +++ b/src/hooks/auto-update-checker/hook.test.ts @@ -0,0 +1,261 @@ +import type { PluginInput } from "@opencode-ai/plugin" +import { describe, expect, mock, test } from "bun:test" + +type CreateAutoUpdateCheckerHook = typeof import("./hook").createAutoUpdateCheckerHook +type HookOptions = Parameters[1] +type HookDeps = NonNullable[2]> + +let latestVersionCallCount = 0 +let scheduleDeferredStartupCheckCallCount = 0 + +const flushMicrotasks = async (count: number): Promise => { + for (let index = 0; index < count; index += 1) { + await Promise.resolve() + } +} + +const latestVersionMock = async () => { + latestVersionCallCount += 1 + return "3.0.1" +} + +const scheduleDeferredStartupCheckMock = (runCheck: () => void) => { + scheduleDeferredStartupCheckCallCount += 1 + scheduledCheck = runCheck +} + +let scheduledCheck: (() => void) | null = null + +mock.module("./checker/latest-version", () => ({ + getLatestVersion: latestVersionMock, +})) + +mock.module("./hook/deferred-startup-check", () => ({ + scheduleDeferredStartupCheck: scheduleDeferredStartupCheckMock, +})) + +const createPluginInput = (): PluginInput => ({ + client: {} as PluginInput["client"], + directory: "/tmp/project", + project: {} as PluginInput["project"], + worktree: "/tmp/project", + serverUrl: new URL("https://example.com"), + $: {} as PluginInput["$"], +} satisfies PluginInput) + +const createDeps = (overrides: Partial = {}) => { + const showConfigErrorsIfAny = mock(async () => undefined) + const updateAndShowConnectedProvidersCacheStatus = mock(async () => undefined) + const refreshModelCapabilitiesOnStartup = mock(async () => undefined) + const showModelCacheWarningIfNeeded = mock(async () => undefined) + const showLocalDevToast = mock(async () => undefined) + const showVersionToast = mock(async () => undefined) + const runBackgroundUpdateCheck = mock(async () => { + await latestVersionMock() + }) + + const deps: HookDeps = { + getCachedVersion: () => "3.0.0", + getLocalDevVersion: () => null, + showConfigErrorsIfAny, + updateAndShowConnectedProvidersCacheStatus, + refreshModelCapabilitiesOnStartup, + showModelCacheWarningIfNeeded, + showLocalDevToast, + showVersionToast, + runBackgroundUpdateCheck, + log: () => undefined, + ...overrides, + } + + return { + deps, + mocks: { + showConfigErrorsIfAny, + updateAndShowConnectedProvidersCacheStatus, + refreshModelCapabilitiesOnStartup, + showModelCacheWarningIfNeeded, + showLocalDevToast, + showVersionToast, + runBackgroundUpdateCheck, + }, + } +} + +const createHook = async ( + options: HookOptions = {}, + overrides: Partial = {}, +) => { + const module = await import("./hook") + const { deps, mocks } = createDeps(overrides) + + return { + hook: module.createAutoUpdateCheckerHook( + createPluginInput(), + { + showStartupToast: true, + autoUpdate: false, + ...options, + }, + deps, + ), + mocks, + } +} + +const resetDeferredState = (): void => { + latestVersionCallCount = 0 + scheduleDeferredStartupCheckCallCount = 0 + scheduledCheck = null +} + +const runScheduledCheck = async (): Promise => { + scheduledCheck?.() + await flushMicrotasks(8) +} + +const triggerSessionCreated = ( + hook: ReturnType, + properties?: { info?: { parentID?: string } }, +): void => { + hook.event({ event: { type: "session.created", properties } }) +} + +const triggerSessionIdle = (hook: ReturnType): void => { + hook.event({ event: { type: "session.idle" } }) +} + +describe("auto-update-checker hook", () => { + test("schedules deferred check on session.created without parentID", async () => { + // given + resetDeferredState() + const { hook, mocks } = await createHook() + + // when + triggerSessionCreated(hook) + + // then + expect(scheduleDeferredStartupCheckCallCount).toBe(1) + expect(mocks.showVersionToast).not.toHaveBeenCalled() + expect(mocks.runBackgroundUpdateCheck).not.toHaveBeenCalled() + expect(latestVersionCallCount).toBe(0) + + // when + await runScheduledCheck() + + // then + expect(mocks.showVersionToast).toHaveBeenCalledTimes(1) + expect(mocks.runBackgroundUpdateCheck).toHaveBeenCalledTimes(1) + expect(latestVersionCallCount).toBe(1) + }) + + test("does not schedule deferred check on session.created with parentID", async () => { + // given + resetDeferredState() + const { hook, mocks } = await createHook() + + // when + triggerSessionCreated(hook, { info: { parentID: "parent-123" } }) + + // then + expect(scheduleDeferredStartupCheckCallCount).toBe(0) + expect(mocks.showVersionToast).not.toHaveBeenCalled() + expect(mocks.runBackgroundUpdateCheck).not.toHaveBeenCalled() + }) + + test("does not schedule deferred check on session.idle without session.created", async () => { + // given + resetDeferredState() + const { hook, mocks } = await createHook() + + // when + triggerSessionIdle(hook) + + // then + expect(scheduleDeferredStartupCheckCallCount).toBe(0) + expect(mocks.showVersionToast).not.toHaveBeenCalled() + expect(mocks.runBackgroundUpdateCheck).not.toHaveBeenCalled() + }) + + test("runs all startup checks after deferred session.created check executes", async () => { + // given + resetDeferredState() + const { hook, mocks } = await createHook() + + // when + triggerSessionCreated(hook) + await runScheduledCheck() + + // then + expect(mocks.showConfigErrorsIfAny).toHaveBeenCalledTimes(1) + expect(mocks.updateAndShowConnectedProvidersCacheStatus).toHaveBeenCalledTimes(1) + expect(mocks.refreshModelCapabilitiesOnStartup).toHaveBeenCalledTimes(1) + expect(mocks.showModelCacheWarningIfNeeded).toHaveBeenCalledTimes(1) + expect(mocks.showVersionToast).toHaveBeenCalledTimes(1) + expect(mocks.runBackgroundUpdateCheck).toHaveBeenCalledTimes(1) + }) + + test("guards double execution across repeated session.created events", async () => { + // given + resetDeferredState() + const { hook, mocks } = await createHook() + + // when + triggerSessionCreated(hook) + triggerSessionCreated(hook) + + // then + expect(scheduleDeferredStartupCheckCallCount).toBe(1) + + // when + await runScheduledCheck() + triggerSessionCreated(hook) + + // then + expect(scheduleDeferredStartupCheckCallCount).toBe(1) + expect(mocks.showConfigErrorsIfAny).toHaveBeenCalledTimes(1) + expect(mocks.updateAndShowConnectedProvidersCacheStatus).toHaveBeenCalledTimes(1) + expect(mocks.showModelCacheWarningIfNeeded).toHaveBeenCalledTimes(1) + expect(mocks.showVersionToast).toHaveBeenCalledTimes(1) + expect(mocks.runBackgroundUpdateCheck).toHaveBeenCalledTimes(1) + }) + + test("shows localDevToast when local dev version exists", async () => { + // given + resetDeferredState() + const { hook, mocks } = await createHook({}, { + getLocalDevVersion: () => "3.0.0-dev", + }) + + // when + triggerSessionCreated(hook) + await runScheduledCheck() + + // then + expect(mocks.showConfigErrorsIfAny).toHaveBeenCalledTimes(1) + expect(mocks.updateAndShowConnectedProvidersCacheStatus).toHaveBeenCalledTimes(1) + expect(mocks.showModelCacheWarningIfNeeded).toHaveBeenCalledTimes(1) + expect(mocks.showLocalDevToast).toHaveBeenCalledTimes(1) + expect(mocks.showVersionToast).not.toHaveBeenCalled() + expect(mocks.runBackgroundUpdateCheck).not.toHaveBeenCalled() + expect(latestVersionCallCount).toBe(0) + }) + + test("passes correct toast message with sisyphus enabled", async () => { + // given + resetDeferredState() + const { hook, mocks } = await createHook({ isSisyphusEnabled: true }) + + // when + triggerSessionCreated(hook) + await runScheduledCheck() + + // then + expect(mocks.showVersionToast).toHaveBeenCalledTimes(1) + expect(mocks.showVersionToast).toHaveBeenCalledWith( + expect.anything(), + "3.0.0", + expect.stringContaining("Sisyphus"), + ) + }) +}) diff --git a/src/hooks/auto-update-checker/hook.ts b/src/hooks/auto-update-checker/hook.ts index fbe3998da..2306c03a0 100644 --- a/src/hooks/auto-update-checker/hook.ts +++ b/src/hooks/auto-update-checker/hook.ts @@ -3,6 +3,7 @@ import { log } from "../../shared/logger" import type { AutoUpdateCheckerOptions } from "./types" import { getCachedVersion, getLocalDevVersion } from "./checker" import { runBackgroundUpdateCheck } from "./hook/background-update-check" +import { scheduleDeferredStartupCheck } from "./hook/deferred-startup-check" import { showConfigErrorsIfAny } from "./hook/config-errors-toast" import { updateAndShowConnectedProvidersCacheStatus } from "./hook/connected-providers-status" import { refreshModelCapabilitiesOnStartup } from "./hook/model-capabilities-status" @@ -35,6 +36,20 @@ const defaultDeps: AutoUpdateCheckerDeps = { log, } +const isRecord = (value: unknown): value is Record => { + return typeof value === "object" && value !== null +} + +const getParentID = (properties: unknown): string | undefined => { + if (!isRecord(properties)) return undefined + + const { info } = properties + if (!isRecord(info)) return undefined + + const { parentID } = info + return typeof parentID === "string" && parentID.length > 0 ? parentID : undefined +} + export function createAutoUpdateCheckerHook( ctx: PluginInput, options: AutoUpdateCheckerOptions = {}, @@ -60,44 +75,46 @@ export function createAutoUpdateCheckerHook( } let hasChecked = false + let hasScheduled = false return { event: ({ event }: { event: { type: string; properties?: unknown } }) => { if (event.type !== "session.created") return if (isCliRunMode) return - if (hasChecked) return + if (hasChecked || hasScheduled) return + if (getParentID(event.properties)) return - const props = event.properties as { info?: { parentID?: string } } | undefined - if (props?.info?.parentID) return + hasScheduled = true + scheduleDeferredStartupCheck(() => { hasChecked = true + void (async () => { + const cachedVersion = deps.getCachedVersion() + const localDevVersion = deps.getLocalDevVersion(ctx.directory) + const displayVersion = localDevVersion ?? cachedVersion - setTimeout(async () => { - const cachedVersion = deps.getCachedVersion() - const localDevVersion = deps.getLocalDevVersion(ctx.directory) - const displayVersion = localDevVersion ?? cachedVersion + await deps.showConfigErrorsIfAny(ctx) + await deps.updateAndShowConnectedProvidersCacheStatus(ctx) + await deps.refreshModelCapabilitiesOnStartup(modelCapabilities) + await deps.showModelCacheWarningIfNeeded(ctx) - await deps.showConfigErrorsIfAny(ctx) - await deps.updateAndShowConnectedProvidersCacheStatus(ctx) - await deps.refreshModelCapabilitiesOnStartup(modelCapabilities) - await deps.showModelCacheWarningIfNeeded(ctx) - - if (localDevVersion) { - if (showStartupToast) { - deps.showLocalDevToast(ctx, displayVersion, isSisyphusEnabled).catch(() => {}) + if (localDevVersion) { + if (showStartupToast) { + deps.showLocalDevToast(ctx, displayVersion, isSisyphusEnabled).catch(() => {}) + } + deps.log("[auto-update-checker] Local development mode") + return } - deps.log("[auto-update-checker] Local development mode") - return - } - if (showStartupToast) { - deps.showVersionToast(ctx, displayVersion, getToastMessage(false)).catch(() => {}) - } + if (showStartupToast) { + deps.showVersionToast(ctx, displayVersion, getToastMessage(false)).catch(() => {}) + } - deps.runBackgroundUpdateCheck(ctx, autoUpdate, getToastMessage).catch((err) => { - deps.log("[auto-update-checker] Background update check failed:", err) - }) - }, 0) + deps.runBackgroundUpdateCheck(ctx, autoUpdate, getToastMessage).catch((err) => { + deps.log("[auto-update-checker] Background update check failed:", err) + }) + })() + }) }, } } diff --git a/src/hooks/auto-update-checker/hook/deferred-startup-check.ts b/src/hooks/auto-update-checker/hook/deferred-startup-check.ts new file mode 100644 index 000000000..2e1066424 --- /dev/null +++ b/src/hooks/auto-update-checker/hook/deferred-startup-check.ts @@ -0,0 +1,4 @@ +export function scheduleDeferredStartupCheck(runCheck: () => void): void { + const timeout = setTimeout(runCheck, 5000) + timeout.unref?.() +} diff --git a/src/hooks/comment-checker/hook.lazy-init.test.ts b/src/hooks/comment-checker/hook.lazy-init.test.ts new file mode 100644 index 000000000..2598aa3c6 --- /dev/null +++ b/src/hooks/comment-checker/hook.lazy-init.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it, mock, afterAll } from "bun:test" + +const startPendingCallCleanup = mock(() => {}) +const initializeCommentCheckerCli = mock(() => {}) + +mock.module("./cli-runner", () => ({ + initializeCommentCheckerCli, + getCommentCheckerCliPathPromise: () => Promise.resolve("/tmp/fake-comment-checker"), + isCliPathUsable: () => true, + processWithCli: async () => {}, + processApplyPatchEditsWithCli: async () => {}, +})) + +mock.module("./pending-calls", () => ({ + registerPendingCall: () => {}, + startPendingCallCleanup, + stopPendingCallCleanup: () => {}, + takePendingCall: () => undefined, +})) + +afterAll(() => { + mock.restore() +}) + +const { createCommentCheckerHooks } = await import("./hook") + +describe("comment-checker lazy initialization", () => { + it("initializes CLI and cleanup on first tool hook call only", async () => { + // given + const hooks = createCommentCheckerHooks() + const beforeHook = hooks["tool.execute.before"] + const input = { tool: "write", sessionID: "ses_test", callID: "call_test" } + const output = { args: { filePath: "src/a.ts" } } + + // when + expect(startPendingCallCleanup).toHaveBeenCalledTimes(0) + expect(initializeCommentCheckerCli).toHaveBeenCalledTimes(0) + + // then + await beforeHook(input, output) + expect(startPendingCallCleanup).toHaveBeenCalledTimes(1) + expect(initializeCommentCheckerCli).toHaveBeenCalledTimes(1) + + // when + await beforeHook(input, output) + + // then + expect(startPendingCallCleanup).toHaveBeenCalledTimes(1) + expect(initializeCommentCheckerCli).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/hooks/comment-checker/hook.ts b/src/hooks/comment-checker/hook.ts index 56632b1f9..089aca2e9 100644 --- a/src/hooks/comment-checker/hook.ts +++ b/src/hooks/comment-checker/hook.ts @@ -28,6 +28,7 @@ import { stopPendingCallCleanup, takePendingCall, } from "./pending-calls" +import { ensureCommentCheckerInitialization } from "./initialization-gate" import * as fs from "fs" import { tmpdir } from "os" @@ -48,14 +49,16 @@ function debugLog(...args: unknown[]) { export function createCommentCheckerHooks(config?: CommentCheckerConfig) { debugLog("createCommentCheckerHooks called", { config }) - startPendingCallCleanup() - initializeCommentCheckerCli(debugLog) - return { "tool.execute.before": async ( input: { tool: string; sessionID: string; callID: string }, output: { args: Record }, ): Promise => { + ensureCommentCheckerInitialization(() => { + startPendingCallCleanup() + initializeCommentCheckerCli(debugLog) + }) + debugLog("tool.execute.before:", { tool: input.tool, callID: input.callID, diff --git a/src/hooks/comment-checker/initialization-gate.ts b/src/hooks/comment-checker/initialization-gate.ts new file mode 100644 index 000000000..da9759a47 --- /dev/null +++ b/src/hooks/comment-checker/initialization-gate.ts @@ -0,0 +1,7 @@ +let initialized = false + +export function ensureCommentCheckerInitialization(initializer: () => void): void { + if (initialized) return + initialized = true + initializer() +} diff --git a/src/hooks/directory-agents-injector/finder.ts b/src/hooks/directory-agents-injector/finder.ts index 8ac8a1463..e04cfab74 100644 --- a/src/hooks/directory-agents-injector/finder.ts +++ b/src/hooks/directory-agents-injector/finder.ts @@ -1,4 +1,4 @@ -import { existsSync } from "node:fs"; +import { constants, promises as fsPromises } from "node:fs"; import { dirname, isAbsolute, join, resolve } from "node:path"; import { AGENTS_FILENAME } from "./constants"; @@ -9,10 +9,10 @@ export function resolveFilePath(rootDirectory: string, path: string): string | n return resolve(rootDirectory, path); } -export function findAgentsMdUp(input: { +export async function findAgentsMdUp(input: { startDir: string; rootDir: string; -}): string[] { +}): Promise { const found: string[] = []; let current = input.startDir; @@ -22,7 +22,11 @@ export function findAgentsMdUp(input: { const isRootDir = current === input.rootDir; if (!isRootDir) { const agentsPath = join(current, AGENTS_FILENAME); - if (existsSync(agentsPath)) { + const exists = await fsPromises + .access(agentsPath, constants.F_OK) + .then(() => true) + .catch(() => false); + if (exists) { found.push(agentsPath); } } diff --git a/src/hooks/directory-agents-injector/injector.test.ts b/src/hooks/directory-agents-injector/injector.test.ts index 8f5701645..822381a69 100644 --- a/src/hooks/directory-agents-injector/injector.test.ts +++ b/src/hooks/directory-agents-injector/injector.test.ts @@ -84,6 +84,23 @@ describe("processFilePathForAgentsInjection", () => { expect(output.output).toContain(srcAgentsContent) }) + it("finds AGENTS.md files while walking up directories", async () => { + // given + const { findAgentsMdUp } = await import("./finder") + + // when + const agentsPaths = await findAgentsMdUp({ + startDir: componentsDirectory, + rootDir: testRoot, + }) + + // then + expect(agentsPaths).toEqual([ + join(srcDirectory, "AGENTS.md"), + join(componentsDirectory, "AGENTS.md"), + ]) + }) + it("skips root-level AGENTS.md", async () => { // given rmSync(join(srcDirectory, "AGENTS.md"), { force: true }) diff --git a/src/hooks/directory-agents-injector/injector.ts b/src/hooks/directory-agents-injector/injector.ts index 28d0be943..3ff40784d 100644 --- a/src/hooks/directory-agents-injector/injector.ts +++ b/src/hooks/directory-agents-injector/injector.ts @@ -1,5 +1,5 @@ import type { PluginInput } from "@opencode-ai/plugin"; -import { readFileSync } from "node:fs"; +import { promises as fsPromises } from "node:fs"; import { dirname } from "node:path"; import type { createDynamicTruncator } from "../../shared/dynamic-truncator"; @@ -31,7 +31,7 @@ export async function processFilePathForAgentsInjection(input: { const dir = dirname(resolved); const cache = getSessionCache(input.sessionCaches, input.sessionID); - const agentsPaths = findAgentsMdUp({ startDir: dir, rootDir: input.ctx.directory }); + const agentsPaths = await findAgentsMdUp({ startDir: dir, rootDir: input.ctx.directory }); let dirty = false; for (const agentsPath of agentsPaths) { @@ -39,7 +39,8 @@ export async function processFilePathForAgentsInjection(input: { if (cache.has(agentsDir)) continue; try { - const content = readFileSync(agentsPath, "utf-8"); + const content = await fsPromises.readFile(agentsPath, "utf-8"); + cache.add(agentsDir); const { result, truncated } = await input.truncator.truncate( input.sessionID, content, @@ -48,7 +49,6 @@ export async function processFilePathForAgentsInjection(input: { ? `\n\n[Note: Content was truncated to save context window space. For full context, please read the file directly: ${agentsPath}]` : ""; input.output.output += `\n\n[Directory Context: ${agentsPath}]\n${result}${truncationNotice}`; - cache.add(agentsDir); dirty = true; } catch {} } diff --git a/src/hooks/directory-readme-injector/finder.ts b/src/hooks/directory-readme-injector/finder.ts index 70e0ba04d..904ef000c 100644 --- a/src/hooks/directory-readme-injector/finder.ts +++ b/src/hooks/directory-readme-injector/finder.ts @@ -1,4 +1,4 @@ -import { existsSync } from "node:fs"; +import { access } from "node:fs/promises"; import { dirname, isAbsolute, join, resolve } from "node:path"; import { README_FILENAME } from "./constants"; @@ -9,17 +9,19 @@ export function resolveFilePath(rootDirectory: string, path: string): string | n return resolve(rootDirectory, path); } -export function findReadmeMdUp(input: { +export async function findReadmeMdUp(input: { startDir: string; rootDir: string; -}): string[] { +}): Promise { const found: string[] = []; let current = input.startDir; while (true) { const readmePath = join(current, README_FILENAME); - if (existsSync(readmePath)) { + try { + await access(readmePath); found.push(readmePath); + } catch { } if (current === input.rootDir) break; diff --git a/src/hooks/directory-readme-injector/injector.test.ts b/src/hooks/directory-readme-injector/injector.test.ts index 74294fd7c..c8e704121 100644 --- a/src/hooks/directory-readme-injector/injector.test.ts +++ b/src/hooks/directory-readme-injector/injector.test.ts @@ -133,6 +133,32 @@ describe("processFilePathForReadmeInjection", () => { expect(output.output).toContain("# Components README") }) + it("returns a promise and finds README.md files from temp fixtures", async () => { + // given + const sourceDirectory = join(testRoot, "src") + const componentsDirectory = join(sourceDirectory, "components") + mkdirSync(componentsDirectory, { recursive: true }) + writeFileSync(join(testRoot, "README.md"), "# Root README") + writeFileSync(join(sourceDirectory, "README.md"), "# Src README") + writeFileSync(join(componentsDirectory, "README.md"), "# Components README") + + const { findReadmeMdUp } = await import("./finder") + + // when + const promise = findReadmeMdUp({ + startDir: componentsDirectory, + rootDir: testRoot, + }) + + // then + expect(promise).toBeInstanceOf(Promise) + await expect(promise).resolves.toEqual([ + join(testRoot, "README.md"), + join(sourceDirectory, "README.md"), + join(componentsDirectory, "README.md"), + ]) + }) + it("does not re-inject already cached directories", async () => { // given const sourceDirectory = join(testRoot, "src") diff --git a/src/hooks/directory-readme-injector/injector.ts b/src/hooks/directory-readme-injector/injector.ts index bfeae7d44..ce3ff7212 100644 --- a/src/hooks/directory-readme-injector/injector.ts +++ b/src/hooks/directory-readme-injector/injector.ts @@ -1,5 +1,5 @@ import type { PluginInput } from "@opencode-ai/plugin"; -import { readFileSync } from "node:fs"; +import { readFile } from "node:fs/promises"; import { dirname } from "node:path"; import type { createDynamicTruncator } from "../../shared/dynamic-truncator"; @@ -31,7 +31,7 @@ export async function processFilePathForReadmeInjection(input: { const dir = dirname(resolved); const cache = getSessionCache(input.sessionCaches, input.sessionID); - const readmePaths = findReadmeMdUp({ startDir: dir, rootDir: input.ctx.directory }); + const readmePaths = await findReadmeMdUp({ startDir: dir, rootDir: input.ctx.directory }); let dirty = false; for (const readmePath of readmePaths) { @@ -39,7 +39,7 @@ export async function processFilePathForReadmeInjection(input: { if (cache.has(readmeDir)) continue; try { - const content = readFileSync(readmePath, "utf-8"); + const content = await readFile(readmePath, "utf-8"); const { result, truncated } = await input.truncator.truncate( input.sessionID, content, diff --git a/src/hooks/rules-injector/cache.test.ts b/src/hooks/rules-injector/cache.test.ts new file mode 100644 index 000000000..39a5bb472 --- /dev/null +++ b/src/hooks/rules-injector/cache.test.ts @@ -0,0 +1,74 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import { randomUUID } from "node:crypto"; +import { existsSync } from "node:fs"; +import { join } from "node:path"; +import { createSessionCacheStore } from "./cache"; +import { RULES_INJECTOR_STORAGE } from "./constants"; +import { clearInjectedRules, saveInjectedRules } from "./storage"; + +const trackedSessionIDs: string[] = []; + +function createSessionID(prefix: string): string { + const sessionID = `${prefix}-${randomUUID()}`; + trackedSessionIDs.push(sessionID); + return sessionID; +} + +function getStoragePath(sessionID: string): string { + return join(RULES_INJECTOR_STORAGE, `${sessionID}.json`); +} + +afterEach(() => { + for (const sessionID of trackedSessionIDs.splice(0)) { + clearInjectedRules(sessionID); + } +}); + +describe("createSessionCacheStore", () => { + it("keeps factory instances isolated for the same session", () => { + // given + const sessionID = createSessionID("cache-isolation"); + const firstStore = createSessionCacheStore(); + const secondStore = createSessionCacheStore(); + const firstCache = firstStore.getSessionCache(sessionID); + + // when + firstCache.contentHashes.add("hash:first"); + firstCache.realPaths.add("/tmp/first-rule.md"); + const secondCache = secondStore.getSessionCache(sessionID); + + // then + expect([...secondCache.contentHashes]).toEqual([]); + expect([...secondCache.realPaths]).toEqual([]); + }); + + it("clears only the targeted session cache and persisted state", () => { + // given + const deletedSessionID = createSessionID("deleted-session"); + const retainedSessionID = createSessionID("retained-session"); + + saveInjectedRules(deletedSessionID, { + contentHashes: new Set(["hash:deleted"]), + realPaths: new Set(["/tmp/deleted-rule.md"]), + }); + saveInjectedRules(retainedSessionID, { + contentHashes: new Set(["hash:retained"]), + realPaths: new Set(["/tmp/retained-rule.md"]), + }); + + const store = createSessionCacheStore(); + store.getSessionCache(deletedSessionID); + const retainedCache = store.getSessionCache(retainedSessionID); + + // when + store.clearSessionCache(deletedSessionID); + const reloadedRetainedCache = store.getSessionCache(retainedSessionID); + + // then + expect(existsSync(getStoragePath(deletedSessionID))).toBe(false); + expect(existsSync(getStoragePath(retainedSessionID))).toBe(true); + expect(reloadedRetainedCache).toBe(retainedCache); + expect([...reloadedRetainedCache.contentHashes]).toEqual(["hash:retained"]); + expect([...reloadedRetainedCache.realPaths]).toEqual(["/tmp/retained-rule.md"]); + }); +}); diff --git a/src/hooks/rules-injector/cache.ts b/src/hooks/rules-injector/cache.ts index b23273144..43d64565c 100644 --- a/src/hooks/rules-injector/cache.ts +++ b/src/hooks/rules-injector/cache.ts @@ -1,4 +1,6 @@ import { clearInjectedRules, loadInjectedRules } from "./storage"; +import { createRuleScanCache } from "./rule-scan-cache"; +import type { RuleScanCache } from "./rule-scan-cache"; export type SessionInjectedRulesCache = { contentHashes: Set; @@ -25,3 +27,29 @@ export function createSessionCacheStore(): { return { getSessionCache, clearSessionCache }; } + +export function createSessionRuleScanCacheStore(): { + getSessionRuleScanCache: (sessionID: string) => RuleScanCache; + clearSessionRuleScanCache: (sessionID: string) => void; +} { + const sessionCaches = new Map(); + + function getSessionRuleScanCache(sessionID: string): RuleScanCache { + const existingCache = sessionCaches.get(sessionID); + if (existingCache) { + return existingCache; + } + + const cache = createRuleScanCache(); + sessionCaches.set(sessionID, cache); + return cache; + } + + function clearSessionRuleScanCache(sessionID: string): void { + const cache = sessionCaches.get(sessionID); + cache?.clear(); + sessionCaches.delete(sessionID); + } + + return { getSessionRuleScanCache, clearSessionRuleScanCache }; +} diff --git a/src/hooks/rules-injector/hook.ts b/src/hooks/rules-injector/hook.ts index f46af4570..b3a09ca12 100644 --- a/src/hooks/rules-injector/hook.ts +++ b/src/hooks/rules-injector/hook.ts @@ -1,8 +1,9 @@ import type { PluginInput } from "@opencode-ai/plugin"; import { createDynamicTruncator } from "../../shared/dynamic-truncator"; import { getRuleInjectionFilePath } from "./output-path"; -import { createSessionCacheStore } from "./cache"; +import { createSessionCacheStore, createSessionRuleScanCacheStore } from "./cache"; import { createRuleInjectionProcessor } from "./injector"; +import { clearProjectRootCache } from "./project-root-finder"; interface ToolExecuteInput { tool: string; @@ -36,15 +37,23 @@ export function createRulesInjectorHook( ) { const truncator = createDynamicTruncator(ctx, modelCacheState); const { getSessionCache, clearSessionCache } = createSessionCacheStore(); + const { getSessionRuleScanCache, clearSessionRuleScanCache } = + createSessionRuleScanCacheStore(); const { processFilePathForInjection } = createRuleInjectionProcessor({ workspaceDirectory: ctx.directory, truncator, getSessionCache, + getSessionRuleScanCache, ruleFinderOptions: options?.skipClaudeUserRules ? { skipClaudeUserRules: true } : undefined, }); + function clearSessionState(sessionID: string): void { + clearSessionCache(sessionID); + clearSessionRuleScanCache(sessionID); + } + const toolExecuteAfter = async ( input: ToolExecuteInput, output: ToolExecuteOutput @@ -73,16 +82,18 @@ export function createRulesInjectorHook( if (event.type === "session.deleted") { const sessionInfo = props?.info as { id?: string } | undefined; if (sessionInfo?.id) { - clearSessionCache(sessionInfo.id); + clearSessionState(sessionInfo.id); } + clearProjectRootCache(); } if (event.type === "session.compacted") { const sessionID = (props?.sessionID ?? (props?.info as { id?: string } | undefined)?.id) as string | undefined; if (sessionID) { - clearSessionCache(sessionID); + clearSessionState(sessionID); } + clearProjectRootCache(); } }; diff --git a/src/hooks/rules-injector/injector.ts b/src/hooks/rules-injector/injector.ts index dc4e9fe29..0cd64be5b 100644 --- a/src/hooks/rules-injector/injector.ts +++ b/src/hooks/rules-injector/injector.ts @@ -12,6 +12,7 @@ import { import { parseRuleFrontmatter } from "./parser"; import { saveInjectedRules } from "./storage"; import type { SessionInjectedRulesCache } from "./cache"; +import type { RuleScanCache } from "./rule-scan-cache"; import type { RuleMetadata } from "./types"; type ToolExecuteOutput = { @@ -56,6 +57,7 @@ export function createRuleInjectionProcessor(deps: { workspaceDirectory: string; truncator: DynamicTruncator; getSessionCache: (sessionID: string) => SessionInjectedRulesCache; + getSessionRuleScanCache?: (sessionID: string) => RuleScanCache; ruleFinderOptions?: FindRuleFilesOptions; readFileSync?: typeof readFileSync; statSync?: typeof statSync; @@ -76,6 +78,7 @@ export function createRuleInjectionProcessor(deps: { workspaceDirectory, truncator, getSessionCache, + getSessionRuleScanCache, ruleFinderOptions, readFileSync: readRuleFileSync = readFileSync, statSync: statRuleSync = statSync, @@ -121,9 +124,16 @@ export function createRuleInjectionProcessor(deps: { const projectRoot = findProjectRoot(resolved); const cache = getSessionCache(sessionID); + const ruleScanCache = getSessionRuleScanCache?.(sessionID); const home = getHomeDir(); - const ruleFileCandidates = findRuleFiles(projectRoot, home, resolved, ruleFinderOptions); + const ruleFileCandidates = findRuleFiles( + projectRoot, + home, + resolved, + ruleFinderOptions, + ruleScanCache, + ); const toInject: RuleToInject[] = []; let dirty = false; diff --git a/src/hooks/rules-injector/project-root-finder.test.ts b/src/hooks/rules-injector/project-root-finder.test.ts new file mode 100644 index 000000000..35d442290 --- /dev/null +++ b/src/hooks/rules-injector/project-root-finder.test.ts @@ -0,0 +1,47 @@ +import { afterEach, describe, expect, it, mock } from "bun:test"; + +describe("findProjectRoot", () => { + afterEach(async () => { + const actualFileSystem = await import("node:fs"); + mock.module("node:fs", () => actualFileSystem); + }); + + it("memoizes repeated lookups for the same start path and resets on cache clear", async () => { + // given + const actualFileSystem = await import("node:fs"); + const projectRoot = "/workspace/project"; + const startPath = `${projectRoot}/src/file.ts`; + const packageJsonPath = `${projectRoot}/package.json`; + + const existsSyncSpy = mock((path: string) => path === packageJsonPath); + const statSyncSpy = mock(() => ({ isDirectory: () => false })); + + mock.module("node:fs", () => ({ + ...actualFileSystem, + existsSync: existsSyncSpy, + statSync: statSyncSpy, + })); + + const { clearProjectRootCache, findProjectRoot } = await import( + `./project-root-finder.ts?memoization=${Date.now()}` + ); + + // when + const firstResult = findProjectRoot(startPath); + const firstExistsSyncCallCount = existsSyncSpy.mock.calls.length; + + const secondResult = findProjectRoot(startPath); + const secondExistsSyncCallCount = existsSyncSpy.mock.calls.length; + + clearProjectRootCache(); + const thirdResult = findProjectRoot(startPath); + + // then + expect(firstResult).toBe(projectRoot); + expect(secondResult).toBe(projectRoot); + expect(thirdResult).toBe(projectRoot); + expect(firstExistsSyncCallCount).toBeGreaterThan(0); + expect(secondExistsSyncCallCount).toBe(firstExistsSyncCallCount); + expect(existsSyncSpy).toHaveBeenCalledTimes(firstExistsSyncCallCount * 2); + }); +}); diff --git a/src/hooks/rules-injector/project-root-finder.ts b/src/hooks/rules-injector/project-root-finder.ts index da697f0d9..ea552e0c9 100644 --- a/src/hooks/rules-injector/project-root-finder.ts +++ b/src/hooks/rules-injector/project-root-finder.ts @@ -2,6 +2,12 @@ import { existsSync, statSync } from "node:fs"; import { dirname, join } from "node:path"; import { PROJECT_MARKERS } from "./constants"; +const projectRootCache = new Map(); + +export function clearProjectRootCache(): void { + projectRootCache.clear(); +} + /** * Find project root by walking up from startPath. * Checks for PROJECT_MARKERS (.git, pyproject.toml, package.json, etc.) @@ -10,6 +16,16 @@ import { PROJECT_MARKERS } from "./constants"; * @returns Project root path or null if not found */ export function findProjectRoot(startPath: string): string | null { + if (projectRootCache.has(startPath)) { + return projectRootCache.get(startPath) ?? null; + } + + const projectRoot = findProjectRootWithoutCache(startPath); + projectRootCache.set(startPath, projectRoot); + return projectRoot; +} + +function findProjectRootWithoutCache(startPath: string): string | null { let current: string; try { diff --git a/src/hooks/rules-injector/rule-file-finder.ts b/src/hooks/rules-injector/rule-file-finder.ts index 98bd6942b..7059804d4 100644 --- a/src/hooks/rules-injector/rule-file-finder.ts +++ b/src/hooks/rules-injector/rule-file-finder.ts @@ -1,51 +1,108 @@ import { existsSync, statSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { dirname, join, sep } from "node:path"; import { + OPENCODE_USER_RULE_DIRS, PROJECT_RULE_FILES, PROJECT_RULE_SUBDIRS, USER_RULE_DIR, - OPENCODE_USER_RULE_DIRS, } from "./constants"; -import type { RuleFileCandidate } from "./types"; +import type { RuleScanCache } from "./rule-scan-cache"; import { findRuleFilesRecursive, safeRealpathSync } from "./rule-file-scanner"; +import type { RuleFileCandidate } from "./types"; export interface FindRuleFilesOptions { - /** - * When true, skip loading rules from ~/.claude/rules/. - * Use when claude_code integration is disabled to prevent - * Claude Code-specific instructions from leaking into non-Claude agents. - */ skipClaudeUserRules?: boolean; } -/** - * Find all rule files for a given context. - * Searches from currentFile upward to projectRoot for rule directories, - * then user-level directory (~/.claude/rules). - * - * IMPORTANT: This searches EVERY directory from file to project root. - * Not just the project root itself. - * - * @param projectRoot - Project root path (or null if outside any project) - * @param homeDir - User home directory - * @param currentFile - Current file being edited (for distance calculation) - * @returns Array of rule file candidates sorted by distance - */ +function getUserRuleDirs(homeDir: string, skipClaudeUserRules: boolean): string[] { + const userRuleDirs = OPENCODE_USER_RULE_DIRS.map((dir) => join(homeDir, dir)); + if (!skipClaudeUserRules) { + userRuleDirs.push(join(homeDir, USER_RULE_DIR)); + } + return userRuleDirs; +} + +function createCacheKey( + projectRoot: string | null, + startDir: string, + skipClaudeUserRules: boolean, +): string { + return `${projectRoot ?? ""}|${startDir}|${skipClaudeUserRules ? "1" : "0"}`; +} + +function createCachedCandidate( + filePath: string, + projectRoot: string | null, + startDir: string, + userRuleDirs: string[], +): RuleFileCandidate | undefined { + const realPath = safeRealpathSync(filePath); + + for (const userRuleDir of userRuleDirs) { + if (filePath.startsWith(`${userRuleDir}${sep}`)) { + return { path: filePath, realPath, isGlobal: true, distance: 9999 }; + } + } + + if (projectRoot) { + for (const ruleFile of PROJECT_RULE_FILES) { + if (filePath === join(projectRoot, ruleFile)) { + return { + path: filePath, + realPath, + isGlobal: false, + distance: 0, + isSingleFile: true, + }; + } + } + } + + let currentDir = startDir; + let distance = 0; + while (true) { + for (const [parent, subdir] of PROJECT_RULE_SUBDIRS) { + const ruleDir = join(currentDir, parent, subdir); + if (filePath.startsWith(`${ruleDir}${sep}`)) { + return { path: filePath, realPath, isGlobal: false, distance }; + } + } + + if (projectRoot && currentDir === projectRoot) break; + const parentDir = dirname(currentDir); + if (parentDir === currentDir) break; + currentDir = parentDir; + distance += 1; + } + + return undefined; +} + export function findRuleFiles( projectRoot: string | null, homeDir: string, currentFile: string, options?: FindRuleFilesOptions, + cache?: RuleScanCache, ): RuleFileCandidate[] { + const startDir = dirname(currentFile); + const skipClaudeUserRules = options?.skipClaudeUserRules ?? false; + const userRuleDirs = getUserRuleDirs(homeDir, skipClaudeUserRules); + const cacheKey = createCacheKey(projectRoot, startDir, skipClaudeUserRules); + const cachedPaths = cache?.get(cacheKey); + + if (cachedPaths) { + return cachedPaths + .map((filePath) => createCachedCandidate(filePath, projectRoot, startDir, userRuleDirs)) + .filter((candidate): candidate is RuleFileCandidate => candidate !== undefined); + } + const candidates: RuleFileCandidate[] = []; const seenRealPaths = new Set(); - - // Search from current file's directory up to project root - let currentDir = dirname(currentFile); + let currentDir = startDir; let distance = 0; while (true) { - // Search rule directories in current directory for (const [parent, subdir] of PROJECT_RULE_SUBDIRS) { const ruleDir = join(currentDir, parent, subdir); const files: string[] = []; @@ -55,60 +112,41 @@ export function findRuleFiles( const realPath = safeRealpathSync(filePath); if (seenRealPaths.has(realPath)) continue; seenRealPaths.add(realPath); - - candidates.push({ - path: filePath, - realPath, - isGlobal: false, - distance, - }); + candidates.push({ path: filePath, realPath, isGlobal: false, distance }); } } - // Stop at project root or filesystem root if (projectRoot && currentDir === projectRoot) break; const parentDir = dirname(currentDir); if (parentDir === currentDir) break; currentDir = parentDir; - distance++; + distance += 1; } - // Check for single-file rules at project root (e.g., .github/copilot-instructions.md) if (projectRoot) { for (const ruleFile of PROJECT_RULE_FILES) { const filePath = join(projectRoot, ruleFile); - if (existsSync(filePath)) { - try { - const stat = statSync(filePath); - if (stat.isFile()) { - const realPath = safeRealpathSync(filePath); - if (!seenRealPaths.has(realPath)) { - seenRealPaths.add(realPath); - candidates.push({ - path: filePath, - realPath, - isGlobal: false, - distance: 0, - isSingleFile: true, - }); - } - } - } catch { - // Skip if file can't be read - } + if (!existsSync(filePath)) continue; + + try { + const stat = statSync(filePath); + if (!stat.isFile()) continue; + const realPath = safeRealpathSync(filePath); + if (seenRealPaths.has(realPath)) continue; + seenRealPaths.add(realPath); + candidates.push({ + path: filePath, + realPath, + isGlobal: false, + distance: 0, + isSingleFile: true, + }); + } catch { + continue; } } } - // Search user-level rule directories - // Always search OpenCode-native dirs (~/.sisyphus/rules, ~/.opencode/rules) - const userRuleDirs: string[] = OPENCODE_USER_RULE_DIRS.map((dir) => join(homeDir, dir)); - - // Only search ~/.claude/rules when claude_code integration is not disabled - if (!options?.skipClaudeUserRules) { - userRuleDirs.push(join(homeDir, USER_RULE_DIR)); - } - for (const userRuleDir of userRuleDirs) { const userFiles: string[] = []; findRuleFilesRecursive(userRuleDir, userFiles); @@ -117,23 +155,21 @@ export function findRuleFiles( const realPath = safeRealpathSync(filePath); if (seenRealPaths.has(realPath)) continue; seenRealPaths.add(realPath); - - candidates.push({ - path: filePath, - realPath, - isGlobal: true, - distance: 9999, // Global rules always have max distance - }); + candidates.push({ path: filePath, realPath, isGlobal: true, distance: 9999 }); } } - // Sort by distance (closest first, then global rules last) - candidates.sort((a, b) => { - if (a.isGlobal !== b.isGlobal) { - return a.isGlobal ? 1 : -1; + candidates.sort((left, right) => { + if (left.isGlobal !== right.isGlobal) { + return left.isGlobal ? 1 : -1; } - return a.distance - b.distance; + return left.distance - right.distance; }); + cache?.set( + cacheKey, + candidates.map((candidate) => candidate.path), + ); + return candidates; } diff --git a/src/hooks/rules-injector/rule-file-scanner.test.ts b/src/hooks/rules-injector/rule-file-scanner.test.ts new file mode 100644 index 000000000..cadf4c3f1 --- /dev/null +++ b/src/hooks/rules-injector/rule-file-scanner.test.ts @@ -0,0 +1,42 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { randomUUID } from "node:crypto"; +import { findRuleFilesRecursive } from "./rule-file-scanner"; + +const createdDirectories: string[] = []; + +afterEach(() => { + for (const directory of createdDirectories.splice(0)) { + if (existsSync(directory)) { + rmSync(directory, { recursive: true, force: true }); + } + } +}); + +describe("findRuleFilesRecursive", () => { + test("returns rule files outside excluded nested directories", () => { + // given + const temporaryDirectory = join(tmpdir(), `perf-d01-${randomUUID()}`); + createdDirectories.push(temporaryDirectory); + + const rulesDirectory = join(temporaryDirectory, ".sisyphus", "rules"); + mkdirSync(join(rulesDirectory, "node_modules", "fake"), { recursive: true }); + mkdirSync(join(rulesDirectory, ".git"), { recursive: true }); + writeFileSync(join(rulesDirectory, "foo.md"), "root rule"); + writeFileSync( + join(rulesDirectory, "node_modules", "fake", "x.md"), + "ignored node_modules rule", + ); + writeFileSync(join(rulesDirectory, ".git", "x.md"), "ignored git rule"); + + const results: string[] = []; + + // when + findRuleFilesRecursive(rulesDirectory, results); + + // then + expect(results).toEqual([join(rulesDirectory, "foo.md")]); + }); +}); diff --git a/src/hooks/rules-injector/rule-file-scanner.ts b/src/hooks/rules-injector/rule-file-scanner.ts index ffd87d8a9..2cd853d07 100644 --- a/src/hooks/rules-injector/rule-file-scanner.ts +++ b/src/hooks/rules-injector/rule-file-scanner.ts @@ -1,5 +1,6 @@ import { existsSync, readdirSync, realpathSync } from "node:fs"; import { join } from "node:path"; +import { EXCLUDED_DIRS } from "../../shared"; import { GITHUB_INSTRUCTIONS_PATTERN, RULE_EXTENSIONS } from "./constants"; function isGitHubInstructionsDir(dir: string): boolean { @@ -28,6 +29,7 @@ export function findRuleFilesRecursive(dir: string, results: string[]): void { const fullPath = join(dir, entry.name); if (entry.isDirectory()) { + if (EXCLUDED_DIRS.has(entry.name)) continue; findRuleFilesRecursive(fullPath, results); } else if (entry.isFile()) { if (isValidRuleFile(entry.name, dir)) { diff --git a/src/hooks/rules-injector/rule-scan-cache.test.ts b/src/hooks/rules-injector/rule-scan-cache.test.ts new file mode 100644 index 000000000..778cf341a --- /dev/null +++ b/src/hooks/rules-injector/rule-scan-cache.test.ts @@ -0,0 +1,91 @@ +import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test"; +import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +function createImportSuffix(): string { + return `?test=${Date.now()}-${Math.random()}`; +} + +describe("createRuleScanCache", () => { + afterEach(() => { + mock.restore(); + }); + + it("returns undefined before set, returns stored value, and clears entries", async () => { + // given + const { createRuleScanCache } = await import(`./rule-scan-cache${createImportSuffix()}`); + const cache = createRuleScanCache(); + const value = ["a", "b"]; + + // when + const initialValue = cache.get("k1"); + cache.set("k1", value); + const storedValue = cache.get("k1"); + cache.clear(); + const clearedValue = cache.get("k1"); + + // then + expect(initialValue).toBeUndefined(); + expect(storedValue).toEqual(value); + expect(clearedValue).toBeUndefined(); + }); +}); + +describe("findRuleFiles with scan cache", () => { + let testRoot = ""; + let homeDir = ""; + let projectRoot = ""; + let currentFile = ""; + let expectedRuleFile = ""; + let expectedRuleDir = ""; + + beforeEach(() => { + testRoot = join(tmpdir(), `rule-scan-cache-test-${Date.now()}`); + homeDir = join(testRoot, "home"); + projectRoot = join(testRoot, "project"); + currentFile = join(projectRoot, "src", "index.ts"); + expectedRuleDir = join(projectRoot, ".github", "instructions"); + expectedRuleFile = join(expectedRuleDir, "typescript.instructions.md"); + + mkdirSync(join(projectRoot, ".git"), { recursive: true }); + mkdirSync(join(projectRoot, "src"), { recursive: true }); + mkdirSync(homeDir, { recursive: true }); + writeFileSync(currentFile, "export const value = 1;\n"); + }); + + afterEach(() => { + mock.restore(); + if (existsSync(testRoot)) { + rmSync(testRoot, { recursive: true, force: true }); + } + }); + + it("reuses cached directory scan results for identical inputs", async () => { + // given + const findRuleFilesRecursive = mock((directoryPath: string, results: string[]) => { + if (directoryPath === expectedRuleDir) { + results.push(expectedRuleFile); + } + }); + + mock.module("./rule-file-scanner", () => ({ + findRuleFilesRecursive, + safeRealpathSync: (filePath: string) => filePath, + })); + + const { createRuleScanCache } = await import(`./rule-scan-cache${createImportSuffix()}`); + const { findRuleFiles } = await import(`./rule-file-finder${createImportSuffix()}`); + const cache = createRuleScanCache(); + + // when + const firstCandidates = findRuleFiles(projectRoot, homeDir, currentFile, undefined, cache); + const firstInvocationCount = findRuleFilesRecursive.mock.calls.length; + const secondCandidates = findRuleFiles(projectRoot, homeDir, currentFile, undefined, cache); + + // then + expect(firstCandidates).toEqual(secondCandidates); + expect(firstInvocationCount).toBeGreaterThan(0); + expect(findRuleFilesRecursive).toHaveBeenCalledTimes(firstInvocationCount); + }); +}); diff --git a/src/hooks/rules-injector/rule-scan-cache.ts b/src/hooks/rules-injector/rule-scan-cache.ts new file mode 100644 index 000000000..fc8ff1a20 --- /dev/null +++ b/src/hooks/rules-injector/rule-scan-cache.ts @@ -0,0 +1,21 @@ +export type RuleScanCache = { + get: (key: string) => string[] | undefined; + set: (key: string, value: string[]) => void; + clear: () => void; +}; + +export function createRuleScanCache(): RuleScanCache { + const cache = new Map(); + + return { + get(key: string): string[] | undefined { + return cache.get(key); + }, + set(key: string, value: string[]): void { + cache.set(key, value); + }, + clear(): void { + cache.clear(); + }, + }; +} diff --git a/src/hooks/rules-injector/storage.test.ts b/src/hooks/rules-injector/storage.test.ts new file mode 100644 index 000000000..e12c4a45e --- /dev/null +++ b/src/hooks/rules-injector/storage.test.ts @@ -0,0 +1,57 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import { randomUUID } from "node:crypto"; +import { existsSync } from "node:fs"; +import { join } from "node:path"; +import { RULES_INJECTOR_STORAGE } from "./constants"; +import { + clearInjectedRules, + loadInjectedRules, + saveInjectedRules, +} from "./storage"; + +const trackedSessionIDs: string[] = []; + +function createSessionID(prefix: string): string { + const sessionID = `${prefix}-${randomUUID()}`; + trackedSessionIDs.push(sessionID); + return sessionID; +} + +function getStoragePath(sessionID: string): string { + return join(RULES_INJECTOR_STORAGE, `${sessionID}.json`); +} + +afterEach(() => { + for (const sessionID of trackedSessionIDs.splice(0)) { + clearInjectedRules(sessionID); + } +}); + +describe("storage", () => { + it("reads back only the requested session data from session-scoped files", () => { + // given + const firstSessionID = createSessionID("storage-first"); + const secondSessionID = createSessionID("storage-second"); + + saveInjectedRules(firstSessionID, { + contentHashes: new Set(["hash:first"]), + realPaths: new Set(["/tmp/first-rule.md"]), + }); + saveInjectedRules(secondSessionID, { + contentHashes: new Set(["hash:second"]), + realPaths: new Set(["/tmp/second-rule.md"]), + }); + + // when + const firstLoaded = loadInjectedRules(firstSessionID); + const secondLoaded = loadInjectedRules(secondSessionID); + + // then + expect(existsSync(getStoragePath(firstSessionID))).toBe(true); + expect(existsSync(getStoragePath(secondSessionID))).toBe(true); + expect([...firstLoaded.contentHashes]).toEqual(["hash:first"]); + expect([...firstLoaded.realPaths]).toEqual(["/tmp/first-rule.md"]); + expect([...secondLoaded.contentHashes]).toEqual(["hash:second"]); + expect([...secondLoaded.realPaths]).toEqual(["/tmp/second-rule.md"]); + }); +}); diff --git a/src/hooks/runtime-fallback/dispose.test.ts b/src/hooks/runtime-fallback/dispose.test.ts index e49cb0904..643b7c8fb 100644 --- a/src/hooks/runtime-fallback/dispose.test.ts +++ b/src/hooks/runtime-fallback/dispose.test.ts @@ -107,9 +107,10 @@ describe("createRuntimeFallbackHook dispose", () => { globalThis.clearTimeout = originalClearTimeout }) - test("#given runtime-fallback hook created #when dispose() is called #then cleanup interval is cleared", () => { + test("#given runtime-fallback hook handles its first event #when dispose() is called #then cleanup interval is cleared", async () => { // given const hook = createRuntimeFallbackHook(createMockContext(), { pluginConfig: {} }) + await hook.event({ event: { type: "session.created", properties: {} } }) // when hook.dispose?.() diff --git a/src/hooks/runtime-fallback/hook.init.test.ts b/src/hooks/runtime-fallback/hook.init.test.ts new file mode 100644 index 000000000..06a7e658c --- /dev/null +++ b/src/hooks/runtime-fallback/hook.init.test.ts @@ -0,0 +1,124 @@ +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test" +import type { OhMyOpenCodeConfig } from "../../config" +import type { HookDeps, RuntimeFallbackInterval, RuntimeFallbackPluginInput } from "./types" + +type RuntimeFallbackModule = typeof import("./hook") + +const loadPluginConfigMock = mock(() => ({} satisfies OhMyOpenCodeConfig)) +const createAutoRetryHelpersMock = mock((_deps: HookDeps) => { + void _deps + + return { + abortSessionRequest: async () => {}, + clearSessionFallbackTimeout: () => {}, + scheduleSessionFallbackTimeout: () => {}, + autoRetryWithFallback: async () => {}, + resolveAgentForSessionFromContext: async () => undefined, + cleanupStaleSessions: () => {}, + } +}) +const createEventHandlerMock = mock(() => async () => {}) +const createMessageUpdateHandlerMock = mock(() => async () => {}) +const createChatMessageHandlerMock = mock(() => async () => {}) + +function registerModuleMocks(): void { + mock.module("../../plugin-config", () => ({ + loadPluginConfig: loadPluginConfigMock, + })) + + mock.module("./auto-retry", () => ({ + createAutoRetryHelpers: createAutoRetryHelpersMock, + })) + + mock.module("./event-handler", () => ({ + createEventHandler: createEventHandlerMock, + })) + + mock.module("./message-update-handler", () => ({ + createMessageUpdateHandler: createMessageUpdateHandlerMock, + })) + + mock.module("./chat-message-handler", () => ({ + createChatMessageHandler: createChatMessageHandlerMock, + })) +} + +function createMockContext(): RuntimeFallbackPluginInput { + return { + client: { + session: { + abort: async () => ({}), + messages: async () => ({}), + promptAsync: async () => ({}), + }, + tui: { + showToast: async () => ({}), + }, + }, + directory: "/test", + } +} + +function createMockInterval(): RuntimeFallbackInterval { + return { + unref: () => {}, + } +} + +describe("createRuntimeFallbackHook initialization", () => { + const originalSetInterval = globalThis.setInterval + let setIntervalCalls = 0 + let createRuntimeFallbackHook: RuntimeFallbackModule["createRuntimeFallbackHook"] + + beforeEach(async () => { + mock.restore() + registerModuleMocks() + loadPluginConfigMock.mockClear() + createAutoRetryHelpersMock.mockClear() + createEventHandlerMock.mockClear() + createMessageUpdateHandlerMock.mockClear() + createChatMessageHandlerMock.mockClear() + setIntervalCalls = 0 + + globalThis.setInterval = ((callback: Parameters[0], delay?: number) => { + void callback + void delay + setIntervalCalls += 1 + return createMockInterval() as ReturnType + }) as typeof globalThis.setInterval + + const cacheBuster = `${Date.now()}-${Math.random()}` + const runtimeFallbackModule: RuntimeFallbackModule = await import(`./hook?test=${cacheBuster}`) + createRuntimeFallbackHook = runtimeFallbackModule.createRuntimeFallbackHook + }) + + afterEach(() => { + globalThis.setInterval = originalSetInterval + mock.restore() + }) + + test("#given injected pluginConfig #when the hook factory runs #then loadPluginConfig is not called", () => { + // given + const pluginConfig = {} satisfies OhMyOpenCodeConfig + + // when + createRuntimeFallbackHook(createMockContext(), { pluginConfig }) + + // then + expect(loadPluginConfigMock).not.toHaveBeenCalled() + }) + + test("#given a fresh hook #when the first event arrives #then cleanup interval starts only once", async () => { + // given + const hook = createRuntimeFallbackHook(createMockContext(), { pluginConfig: {} }) + + // when + expect(setIntervalCalls).toBe(0) + await hook.event({ event: { type: "session.created", properties: {} } }) + expect(setIntervalCalls).toBe(1) + await hook.event({ event: { type: "session.error", properties: {} } }) + + // then + expect(setIntervalCalls).toBe(1) + }) +}) diff --git a/src/hooks/runtime-fallback/hook.ts b/src/hooks/runtime-fallback/hook.ts index 2a13d507e..f3509ab0a 100644 --- a/src/hooks/runtime-fallback/hook.ts +++ b/src/hooks/runtime-fallback/hook.ts @@ -1,7 +1,5 @@ import type { HookDeps, RuntimeFallbackHook, RuntimeFallbackInterval, RuntimeFallbackOptions, RuntimeFallbackPluginInput, RuntimeFallbackTimeout } from "./types" -import { DEFAULT_CONFIG, HOOK_NAME } from "./constants" -import { log } from "../../shared/logger" -import { loadPluginConfig } from "../../plugin-config" +import { DEFAULT_CONFIG } from "./constants" import { createAutoRetryHelpers } from "./auto-retry" import { createEventHandler } from "./event-handler" import { createMessageUpdateHandler } from "./message-update-handler" @@ -24,20 +22,11 @@ export function createRuntimeFallbackHook( notify_on_fallback: options?.config?.notify_on_fallback ?? DEFAULT_CONFIG.notify_on_fallback, } - let pluginConfig = options?.pluginConfig - if (!pluginConfig) { - try { - pluginConfig = loadPluginConfig(ctx.directory, ctx) - } catch { - log(`[${HOOK_NAME}] Plugin config not available`) - } - } - const deps: HookDeps = { ctx, config, options, - pluginConfig, + pluginConfig: options?.pluginConfig, sessionStates: new Map(), sessionLastAccess: new Map(), sessionRetryInFlight: new Set(), @@ -51,10 +40,23 @@ export function createRuntimeFallbackHook( const messageUpdateHandler = createMessageUpdateHandler(deps, helpers) const chatMessageHandler = createChatMessageHandler(deps) - const cleanupInterval = setInterval(helpers.cleanupStaleSessions, 5 * 60 * 1000) - cleanupInterval.unref() + let cleanupInterval: RuntimeFallbackInterval | null = null + let intervalStarted = false + + const ensureInterval = (): void => { + if (intervalStarted) return + + intervalStarted = true + cleanupInterval = setInterval(helpers.cleanupStaleSessions, 5 * 60 * 1000) + + if (typeof cleanupInterval.unref === "function") { + cleanupInterval.unref() + } + } const eventHandler = async ({ event }: { event: { type: string; properties?: unknown } }) => { + ensureInterval() + if (event.type === "message.updated") { if (!config.enabled) return const props = event.properties as Record | undefined @@ -65,7 +67,9 @@ export function createRuntimeFallbackHook( } const dispose = () => { - clearInterval(cleanupInterval) + if (cleanupInterval) { + clearInterval(cleanupInterval) + } for (const fallbackTimeout of deps.sessionFallbackTimeouts.values()) { clearTimeout(fallbackTimeout) diff --git a/src/hooks/session-notification-init.ts b/src/hooks/session-notification-init.ts new file mode 100644 index 000000000..3dab42ea6 --- /dev/null +++ b/src/hooks/session-notification-init.ts @@ -0,0 +1,31 @@ +import type { Platform } from "./session-notification-sender" +import * as sessionNotificationSender from "./session-notification-sender" +import { startBackgroundCheck } from "./session-notification-utils" + +export function createSessionNotificationInit() { + let platform: Platform | null = null + let defaultSoundPath: string | null = null + let started = false + + function initialize(): { platform: Platform; defaultSoundPath: string } { + if (!platform) { + platform = sessionNotificationSender.detectPlatform() + } + if (!defaultSoundPath) { + defaultSoundPath = sessionNotificationSender.getDefaultSoundPath(platform) + } + if (!started) { + startBackgroundCheck(platform) + started = true + } + + return { + platform, + defaultSoundPath, + } + } + + return { + initialize, + } +} diff --git a/src/hooks/session-notification-input-needed.test.ts b/src/hooks/session-notification-input-needed.test.ts index ee1614b88..f85d9154d 100644 --- a/src/hooks/session-notification-input-needed.test.ts +++ b/src/hooks/session-notification-input-needed.test.ts @@ -93,6 +93,53 @@ describe("session-notification input-needed events", () => { expect(notificationCalls).toHaveLength(1) expect(notificationCalls[0]).toContain("Agent needs permission to continue") }) + + test("lazily detects platform and starts background checks on first idle event", async () => { + const sessionID = "main-idle" + setMainSession(sessionID) + + const detectPlatformSpy = spyOn(sender, "detectPlatform") + detectPlatformSpy.mockReturnValue("darwin") + + const getDefaultSoundPathSpy = spyOn(sender, "getDefaultSoundPath") + getDefaultSoundPathSpy.mockReturnValue("/System/Library/Sounds/Glass.aiff") + + const startBackgroundCheckSpy = spyOn(utils, "startBackgroundCheck") + startBackgroundCheckSpy.mockImplementation(() => {}) + + // given + const hook = createSessionNotification(createMockPluginInput(), { enforceMainSessionFilter: false }) + + // when + await hook({ + event: { + type: "session.idle", + properties: { + sessionID, + }, + }, + }) + + // then + expect(detectPlatformSpy).toHaveBeenCalledTimes(1) + expect(getDefaultSoundPathSpy).toHaveBeenCalledTimes(1) + expect(startBackgroundCheckSpy).toHaveBeenCalledTimes(1) + + // when + await hook({ + event: { + type: "session.idle", + properties: { + sessionID, + }, + }, + }) + + // then + expect(detectPlatformSpy).toHaveBeenCalledTimes(1) + expect(getDefaultSoundPathSpy).toHaveBeenCalledTimes(1) + expect(startBackgroundCheckSpy).toHaveBeenCalledTimes(1) + }) }) export {} diff --git a/src/hooks/session-notification.ts b/src/hooks/session-notification.ts index dc83d3643..f9a40f56d 100644 --- a/src/hooks/session-notification.ts +++ b/src/hooks/session-notification.ts @@ -1,20 +1,12 @@ import type { PluginInput } from "@opencode-ai/plugin" import { subagentSessions, getMainSessionID } from "../features/claude-code-session-state" -import { - startBackgroundCheck, -} from "./session-notification-utils" import { buildReadyNotificationContent } from "./session-notification-content" -import { - type Platform, -} from "./session-notification-sender" +import { type Platform } from "./session-notification-sender" import * as sessionNotificationSender from "./session-notification-sender" -import { - getEventToolName, - getQuestionText, - getSessionID, -} from "./session-notification-event-properties" +import { getEventToolName, getQuestionText, getSessionID } from "./session-notification-event-properties" import { hasIncompleteTodos } from "./session-todo-status" import { createIdleNotificationScheduler } from "./session-notification-scheduler" +import { createSessionNotificationInit } from "./session-notification-init" interface SessionNotificationConfig { title?: string @@ -33,22 +25,15 @@ interface SessionNotificationConfig { /** Grace period in ms to ignore late-arriving activity events after scheduling (default: 100) */ activityGracePeriodMs?: number } -export function createSessionNotification( - ctx: PluginInput, - config: SessionNotificationConfig = {} -) { - const currentPlatform: Platform = sessionNotificationSender.detectPlatform() - const defaultSoundPath = sessionNotificationSender.getDefaultSoundPath(currentPlatform) - - startBackgroundCheck(currentPlatform) +export function createSessionNotification(ctx: PluginInput, config: SessionNotificationConfig = {}) { const mergedConfig = { title: "OpenCode", message: "Agent is ready for input", questionMessage: "Agent is asking a question", permissionMessage: "Agent needs permission to continue", playSound: false, - soundPath: defaultSoundPath, + soundPath: "", idleConfirmationDelay: 1500, skipIfIncompleteTodos: true, maxTrackedSessions: 100, @@ -56,22 +41,18 @@ export function createSessionNotification( ...config, } + const sessionNotificationInit = createSessionNotificationInit() + let currentPlatform: Platform | null = null + let defaultSoundPath = mergedConfig.soundPath + const scheduler = createIdleNotificationScheduler({ ctx, - platform: currentPlatform, + platform: "unsupported", config: mergedConfig, hasIncompleteTodos, send: async (hookCtx, platform, sessionID) => { - if ( - typeof hookCtx.client.session.get !== "function" - && typeof hookCtx.client.session.messages !== "function" - ) { - await sessionNotificationSender.sendSessionNotification( - hookCtx, - platform, - mergedConfig.title, - mergedConfig.message, - ) + if (typeof hookCtx.client.session.get !== "function" && typeof hookCtx.client.session.messages !== "function") { + await sessionNotificationSender.sendSessionNotification(hookCtx, platform, mergedConfig.title, mergedConfig.message) return } @@ -90,6 +71,15 @@ export function createSessionNotification( const PERMISSION_EVENTS = new Set(["permission.ask", "permission.asked", "permission.updated", "permission.requested"]) const PERMISSION_HINT_PATTERN = /\b(permission|approve|approval|allow|deny|consent)\b/i + const ensureNotificationPlatform = (): Platform => { + if (currentPlatform) return currentPlatform + + const initialized = sessionNotificationInit.initialize() + currentPlatform = initialized.platform + defaultSoundPath = initialized.defaultSoundPath || mergedConfig.soundPath + return currentPlatform + } + const shouldNotifyForSession = (sessionID: string): boolean => { if (subagentSessions.has(sessionID)) return false @@ -102,16 +92,12 @@ export function createSessionNotification( } return async ({ event }: { event: { type: string; properties?: unknown } }) => { - if (currentPlatform === "unsupported") return - const props = event.properties as Record | undefined if (event.type === "session.created") { const info = props?.info as Record | undefined const sessionID = info?.id as string | undefined - if (sessionID) { - scheduler.markSessionActivity(sessionID) - } + if (sessionID) scheduler.markSessionActivity(sessionID) return } @@ -119,6 +105,8 @@ export function createSessionNotification( const sessionID = getSessionID(props) if (!sessionID) return + const platform = ensureNotificationPlatform() + if (platform === "unsupported") return if (!shouldNotifyForSession(sessionID)) return scheduler.scheduleIdleNotification(sessionID) @@ -128,26 +116,22 @@ export function createSessionNotification( if (event.type === "message.updated") { const info = props?.info as Record | undefined const sessionID = getSessionID({ ...props, info }) - if (sessionID) { - scheduler.markSessionActivity(sessionID) - } + if (sessionID) scheduler.markSessionActivity(sessionID) return } if (PERMISSION_EVENTS.has(event.type)) { const sessionID = getSessionID(props) if (!sessionID) return + + const platform = ensureNotificationPlatform() + if (platform === "unsupported") return if (!shouldNotifyForSession(sessionID)) return scheduler.markSessionActivity(sessionID) - await sessionNotificationSender.sendSessionNotification( - ctx, - currentPlatform, - mergedConfig.title, - mergedConfig.permissionMessage, - ) - if (mergedConfig.playSound && mergedConfig.soundPath) { - await sessionNotificationSender.playSessionNotificationSound(ctx, currentPlatform, mergedConfig.soundPath) + await sessionNotificationSender.sendSessionNotification(ctx, platform, mergedConfig.title, mergedConfig.permissionMessage) + if (mergedConfig.playSound && defaultSoundPath) { + await sessionNotificationSender.playSessionNotificationSound(ctx, platform, defaultSoundPath) } return } @@ -160,16 +144,16 @@ export function createSessionNotification( if (event.type === "tool.execute.before") { const toolName = getEventToolName(props)?.toLowerCase() if (toolName && QUESTION_TOOLS.has(toolName)) { + const platform = ensureNotificationPlatform() + if (platform === "unsupported") return if (!shouldNotifyForSession(sessionID)) return const questionText = getQuestionText(props) - const message = PERMISSION_HINT_PATTERN.test(questionText) - ? mergedConfig.permissionMessage - : mergedConfig.questionMessage + const message = PERMISSION_HINT_PATTERN.test(questionText) ? mergedConfig.permissionMessage : mergedConfig.questionMessage - await sessionNotificationSender.sendSessionNotification(ctx, currentPlatform, mergedConfig.title, message) - if (mergedConfig.playSound && mergedConfig.soundPath) { - await sessionNotificationSender.playSessionNotificationSound(ctx, currentPlatform, mergedConfig.soundPath) + await sessionNotificationSender.sendSessionNotification(ctx, platform, mergedConfig.title, message) + if (mergedConfig.playSound && defaultSoundPath) { + await sessionNotificationSender.playSessionNotificationSound(ctx, platform, defaultSoundPath) } } } @@ -179,9 +163,7 @@ export function createSessionNotification( if (event.type === "session.deleted") { const sessionInfo = props?.info as { id?: string } | undefined - if (sessionInfo?.id) { - scheduler.deleteSession(sessionInfo.id) - } + if (sessionInfo?.id) scheduler.deleteSession(sessionInfo.id) } } } diff --git a/src/hooks/todo-continuation-enforcer/dispose.test.ts b/src/hooks/todo-continuation-enforcer/dispose.test.ts index 5423c8068..37971bc6d 100644 --- a/src/hooks/todo-continuation-enforcer/dispose.test.ts +++ b/src/hooks/todo-continuation-enforcer/dispose.test.ts @@ -8,6 +8,7 @@ declare module "bun:test" { import { afterAll, afterEach, describe, expect, it, mock } from "bun:test" +import type { BackgroundManager } from "../../features/background-agent" import * as actualSessionStateModule from "./session-state" import type { SessionStateStore } from "./session-state" @@ -37,6 +38,12 @@ function createMockPluginInput(): PluginInput { } as PluginInput } +function createMockBackgroundManager(): BackgroundManager { + return { + getTasksByParentSession: () => [{ status: "running" }], + } as BackgroundManager +} + function getCreatedSessionStateStore(): SessionStateStore { if (!createdSessionStateStore) { throw new Error("expected session state store to be created") @@ -68,7 +75,7 @@ describe("todo-continuation-enforcer dispose", () => { enforcer.dispose() }) - it("#given enforcer with active session states #when dispose is called #then internal session state store is shut down", () => { + it("#given enforcer with active session states #when dispose is called #then internal session state store is shut down", async () => { // given const originalClearInterval = globalThis.clearInterval const clearIntervalCalls: Array[0]> = [] @@ -78,9 +85,13 @@ describe("todo-continuation-enforcer dispose", () => { }) as typeof clearInterval try { - const enforcer = createTodoContinuationEnforcer(createMockPluginInput()) + const enforcer = createTodoContinuationEnforcer(createMockPluginInput(), { + backgroundManager: createMockBackgroundManager(), + }) const sessionStateStore = getCreatedSessionStateStore() + await enforcer.handler({ event: { type: "session.idle", properties: { sessionID: "session-1" } } }) + enforcer.markRecovering("session-1") enforcer.markRecovering("session-2") diff --git a/src/hooks/todo-continuation-enforcer/handler.ts b/src/hooks/todo-continuation-enforcer/handler.ts index 3347ee666..7136dda44 100644 --- a/src/hooks/todo-continuation-enforcer/handler.ts +++ b/src/hooks/todo-continuation-enforcer/handler.ts @@ -61,6 +61,7 @@ export function createTodoContinuationHandler(args: { const sessionID = props?.sessionID as string | undefined if (!sessionID) return + sessionStateStore.startPruneInterval() await handleSessionIdle({ ctx, sessionID, diff --git a/src/hooks/todo-continuation-enforcer/session-state.ts b/src/hooks/todo-continuation-enforcer/session-state.ts index a87472b7a..dcd88629e 100644 --- a/src/hooks/todo-continuation-enforcer/session-state.ts +++ b/src/hooks/todo-continuation-enforcer/session-state.ts @@ -31,6 +31,7 @@ export interface ContinuationProgressUpdate { export interface SessionStateStore { getState: (sessionID: string) => SessionState getExistingState: (sessionID: string) => SessionState | undefined + startPruneInterval: () => void recordActivity: (sessionID: string) => void trackContinuationProgress: ( sessionID: string, @@ -76,18 +77,26 @@ export function createSessionStateStore(): SessionStateStore { // Periodic pruning of stale session states to prevent unbounded Map growth let pruneInterval: TimerHandle | undefined - pruneInterval = setInterval(() => { - const now = Date.now() - for (const [sessionID, tracked] of sessions.entries()) { - if (now - tracked.lastAccessedAt > SESSION_STATE_TTL_MS) { - cancelCountdown(sessionID) - sessions.delete(sessionID) - } + let pruneIntervalStarted = false + + function startPruneInterval(): void { + if (pruneIntervalStarted) { + return + } + + pruneIntervalStarted = true + pruneInterval = setInterval(() => { + const now = Date.now() + for (const [sessionID, tracked] of sessions.entries()) { + if (now - tracked.lastAccessedAt > SESSION_STATE_TTL_MS) { + cancelCountdown(sessionID) + sessions.delete(sessionID) + } + } + }, SESSION_STATE_PRUNE_INTERVAL_MS) + if (typeof pruneInterval === "object" && typeof pruneInterval.unref === "function") { + pruneInterval.unref() } - }, SESSION_STATE_PRUNE_INTERVAL_MS) - // Allow process to exit naturally even if interval is running - if (typeof pruneInterval === "object" && typeof pruneInterval.unref === "function") { - pruneInterval.unref() } function getTrackedSession(sessionID: string): TrackedSessionState { @@ -272,6 +281,7 @@ export function createSessionStateStore(): SessionStateStore { return { getState, getExistingState, + startPruneInterval, recordActivity, trackContinuationProgress, resetContinuationProgress, diff --git a/src/hooks/todo-continuation-enforcer/todo-continuation-enforcer.test.ts b/src/hooks/todo-continuation-enforcer/todo-continuation-enforcer.test.ts index 9c5a35f5c..5315b0842 100644 --- a/src/hooks/todo-continuation-enforcer/todo-continuation-enforcer.test.ts +++ b/src/hooks/todo-continuation-enforcer/todo-continuation-enforcer.test.ts @@ -249,6 +249,33 @@ describe("todo-continuation-enforcer", () => { _resetForTesting() }) + test("given the first idle event, starts the prune interval lazily", async () => { + // given + const originalSetInterval = globalThis.setInterval + let setIntervalCalls = 0 + globalThis.setInterval = ((callback: TimerCallback, delay?: number, ...args: any[]) => { + setIntervalCalls += 1 + return originalSetInterval(callback, delay, ...args) + }) as typeof setInterval + + try { + const sessionID = "main-lazy-prune" + setMainSession(sessionID) + const hook = createTodoContinuationEnforcer(createMockPluginInput(), { + backgroundManager: createMockBackgroundManager(true), + }) + + // when + await hook.handler({ event: { type: "session.idle", properties: { sessionID } } }) + await hook.handler({ event: { type: "session.idle", properties: { sessionID } } }) + + // then + expect(setIntervalCalls).toBe(1) + } finally { + globalThis.setInterval = originalSetInterval + } + }) + test("should inject continuation when idle with incomplete todos", async () => { fakeTimers.restore() // given - main session with incomplete todos diff --git a/src/hooks/write-existing-file-guard/hook.ts b/src/hooks/write-existing-file-guard/hook.ts index bdaf5cad8..ab7bd9aef 100644 --- a/src/hooks/write-existing-file-guard/hook.ts +++ b/src/hooks/write-existing-file-guard/hook.ts @@ -76,7 +76,15 @@ export function isOverwriteEnabled(value: boolean | string | undefined): boolean export function createWriteExistingFileGuardHook(ctx: PluginInput): Hooks { const readPermissionsBySession = new Map>() const sessionLastAccess = new Map() - const canonicalSessionRoot = toCanonicalPath(resolveInputPath(ctx, ctx.directory)) + let canonicalSessionRoot: string | undefined + + function getCanonicalSessionRoot(): string { + if (!canonicalSessionRoot) { + canonicalSessionRoot = toCanonicalPath(resolveInputPath(ctx, ctx.directory)) + } + + return canonicalSessionRoot + } return { "tool.execute.before": async (input, output) => { @@ -86,7 +94,7 @@ export function createWriteExistingFileGuardHook(ctx: PluginInput): Hooks { output, readPermissionsBySession, sessionLastAccess, - canonicalSessionRoot, + getCanonicalSessionRoot, maxTrackedSessions: MAX_TRACKED_SESSIONS, }) }, diff --git a/src/hooks/write-existing-file-guard/lazy-canonical-path-init.test.ts b/src/hooks/write-existing-file-guard/lazy-canonical-path-init.test.ts new file mode 100644 index 000000000..0f1d4bb88 --- /dev/null +++ b/src/hooks/write-existing-file-guard/lazy-canonical-path-init.test.ts @@ -0,0 +1,65 @@ +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test" +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" + +const realFs = await import("node:fs") + +const existsSyncMock = mock(realFs.existsSync) +const realpathNativeMock = mock(realFs.realpathSync.native) + +mock.module("fs", () => ({ + ...realFs, + existsSync: existsSyncMock, + realpathSync: { + ...realFs.realpathSync, + native: realpathNativeMock, + }, +})) + +const { createWriteExistingFileGuardHook } = await import("./index") + +describe("createWriteExistingFileGuardHook", () => { + let tempDir = "" + + beforeEach(() => { + // given + tempDir = mkdtempSync(join(tmpdir(), "write-existing-file-guard-lazy-")) + mkdirSync(tempDir, { recursive: true }) + existsSyncMock.mockClear() + realpathNativeMock.mockClear() + }) + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }) + }) + + test("#given hook factory #when created #then defers fs canonical path calls until first tool invocation", async () => { + // given + const existingFile = join(tempDir, "existing.txt") + writeFileSync(existingFile, "content") + + // when + const hook = createWriteExistingFileGuardHook({ directory: tempDir } as never) + + // then + expect(existsSyncMock).toHaveBeenCalledTimes(0) + expect(realpathNativeMock).toHaveBeenCalledTimes(0) + + // when + await expect( + hook["tool.execute.before"]?.( + { + tool: "write", + sessionID: "ses_lazy", + callID: "call_lazy", + } as never, + { args: { filePath: existingFile, content: "updated" } } as never, + ), + ).rejects.toThrow("File already exists. Use edit tool instead.") + + // then + expect(existsSyncMock).toHaveBeenCalledTimes(3) + expect(realpathNativeMock).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/hooks/write-existing-file-guard/tool-execute-before-handler.ts b/src/hooks/write-existing-file-guard/tool-execute-before-handler.ts index 25eebbda3..848238a8a 100644 --- a/src/hooks/write-existing-file-guard/tool-execute-before-handler.ts +++ b/src/hooks/write-existing-file-guard/tool-execute-before-handler.ts @@ -90,10 +90,10 @@ export async function handleWriteExistingFileGuardToolExecuteBefore(params: { output: { args?: unknown } readPermissionsBySession: Map> sessionLastAccess: Map - canonicalSessionRoot: string + getCanonicalSessionRoot: () => string maxTrackedSessions: number }): Promise { - const { ctx, input, output, readPermissionsBySession, sessionLastAccess, canonicalSessionRoot, maxTrackedSessions } = params + const { ctx, input, output, readPermissionsBySession, sessionLastAccess, getCanonicalSessionRoot, maxTrackedSessions } = params const toolName = input.tool?.toLowerCase() if (toolName !== "write" && toolName !== "read") { return @@ -107,6 +107,7 @@ export async function handleWriteExistingFileGuardToolExecuteBefore(params: { } const resolvedPath = resolveInputPath(ctx, filePath) + const canonicalSessionRoot = getCanonicalSessionRoot() const canonicalPath = toCanonicalPath(resolvedPath) if (!isPathInsideDirectory(canonicalPath, canonicalSessionRoot)) { return diff --git a/src/hooks/zauc-mocks-hook/hook.test.ts b/src/hooks/zauc-mocks-hook/hook.test.ts index 2c291b1f0..de0e4d3d2 100644 --- a/src/hooks/zauc-mocks-hook/hook.test.ts +++ b/src/hooks/zauc-mocks-hook/hook.test.ts @@ -1,5 +1,13 @@ import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test" -import { createAutoUpdateCheckerHook } from "../auto-update-checker/hook" + +let scheduledDeferredCheck: (() => void) | null = null +mock.module("../auto-update-checker/hook/deferred-startup-check", () => ({ + scheduleDeferredStartupCheck: (runCheck: () => void) => { + scheduledDeferredCheck = runCheck + }, +})) + +const { createAutoUpdateCheckerHook } = await import("../auto-update-checker/hook") const mockShowConfigErrorsIfAny = mock(async () => {}) const mockShowModelCacheWarningIfNeeded = mock(async () => {}) @@ -38,6 +46,12 @@ function runSessionCreatedEvent( }) } +function drainDeferredCheck(): void { + const run = scheduledDeferredCheck + scheduledDeferredCheck = null + run?.() +} + beforeEach(() => { mockShowConfigErrorsIfAny.mockClear() mockShowModelCacheWarningIfNeeded.mockClear() @@ -51,6 +65,8 @@ beforeEach(() => { mockGetCachedVersion.mockReturnValue("3.6.0") mockGetLocalDevVersion.mockReturnValue(null) + + scheduledDeferredCheck = null }) afterEach(() => { @@ -108,8 +124,9 @@ describe("createAutoUpdateCheckerHook", () => { log: () => {}, }) - //#when - session.created event arrives on primary session + //#when - session.created schedules work and deferred check drains it runSessionCreatedEvent(hook) + drainDeferredCheck() await flushScheduledWork() //#then - startup checks, toast, and background check run @@ -165,9 +182,10 @@ describe("createAutoUpdateCheckerHook", () => { log: () => {}, }) - //#when - session.created event is fired twice + //#when - session.created fires twice and deferred check drains once runSessionCreatedEvent(hook) runSessionCreatedEvent(hook) + drainDeferredCheck() await flushScheduledWork() //#then - side effects execute only once @@ -195,8 +213,9 @@ describe("createAutoUpdateCheckerHook", () => { log: () => {}, }) - //#when - session.created event arrives + //#when - session.created schedules and deferred check drains runSessionCreatedEvent(hook) + drainDeferredCheck() await flushScheduledWork() //#then - local dev toast is shown and background check is skipped @@ -259,8 +278,9 @@ describe("createAutoUpdateCheckerHook", () => { log: () => {}, }) - //#when - session.created event arrives + //#when - session.created schedules and deferred check drains runSessionCreatedEvent(hook) + drainDeferredCheck() await flushScheduledWork() //#then - startup toast includes sisyphus wording diff --git a/src/shared/excluded-dirs.test.ts b/src/shared/excluded-dirs.test.ts new file mode 100644 index 000000000..21a488907 --- /dev/null +++ b/src/shared/excluded-dirs.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, test } from "bun:test" +import { EXCLUDED_DIRS } from "./excluded-dirs" +import { EXCLUDED_DIRS as EXCLUDED_DIRS_FROM_BARREL } from "." + +describe("EXCLUDED_DIRS", () => { + test("contains the well-known junk directories we never want to recurse into", () => { + // given + const expected = [ + "node_modules", + ".git", + "dist", + "build", + ".next", + ".sisyphus", + ".omx", + ".turbo", + "coverage", + "out", + ".cache", + ".vscode-test", + "target", + ".local-ignore", + ] + + // when / then + for (const name of expected) { + expect(EXCLUDED_DIRS.has(name)).toBe(true) + } + }) + + test("does not contain commonly-wanted project directories", () => { + // given + const shouldBeAllowed = ["src", "lib", "tests", "test", "docs", ".github", ".cursor", ".claude", ".opencode"] + + // when / then + for (const name of shouldBeAllowed) { + expect(EXCLUDED_DIRS.has(name)).toBe(false) + } + }) + + test("is frozen so consumers cannot mutate shared state", () => { + // given / when / then + expect(Object.isFrozen(EXCLUDED_DIRS)).toBe(true) + }) + + test("is re-exported from the shared barrel", () => { + // given / when / then + expect(EXCLUDED_DIRS_FROM_BARREL).toBe(EXCLUDED_DIRS) + }) +}) diff --git a/src/shared/excluded-dirs.ts b/src/shared/excluded-dirs.ts new file mode 100644 index 000000000..059a01406 --- /dev/null +++ b/src/shared/excluded-dirs.ts @@ -0,0 +1,18 @@ +const EXCLUDED_DIR_NAMES = [ + "node_modules", + ".git", + "dist", + "build", + ".next", + ".sisyphus", + ".omx", + ".turbo", + "coverage", + "out", + ".cache", + ".vscode-test", + "target", + ".local-ignore", +] as const + +export const EXCLUDED_DIRS: ReadonlySet = Object.freeze(new Set(EXCLUDED_DIR_NAMES)) diff --git a/src/shared/index.ts b/src/shared/index.ts index 140f88192..e99234c33 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -79,3 +79,4 @@ export * from "./log-legacy-plugin-startup-warning" export * from "./task-system-enabled" export * from "./parse-tools-config" export { parseModelString } from "./model-string-parser" +export { EXCLUDED_DIRS } from "./excluded-dirs" diff --git a/src/shared/jsonc-parser.memoization.test.ts b/src/shared/jsonc-parser.memoization.test.ts new file mode 100644 index 000000000..c4cd1f5b8 --- /dev/null +++ b/src/shared/jsonc-parser.memoization.test.ts @@ -0,0 +1,54 @@ +import { afterEach, describe, expect, mock, spyOn, test } from "bun:test" +import * as fs from "node:fs" +import { join } from "node:path" + +describe("detectPluginConfigFile memoization", () => { + const testDir = join(__dirname, ".test-detect-plugin-memoization") + + afterEach(() => { + mock.restore() + }) + + test("returns cached result on repeated calls for the same directory", async () => { + // given + const existsSync = spyOn(fs, "existsSync").mockImplementation((filePath: fs.PathLike) => { + return String(filePath).endsWith("oh-my-openagent.jsonc") + }) + const readdirSync = spyOn(fs, "readdirSync").mockImplementation(() => []) + spyOn(fs, "readFileSync").mockImplementation(() => "") + + const parserModule = await import(`./jsonc-parser?memoization=${Date.now()}-${Math.random()}`) + + // when + const firstResult = parserModule.detectPluginConfigFile(testDir) + const callsAfterFirstResult = existsSync.mock.calls.length + const secondResult = parserModule.detectPluginConfigFile(testDir) + + // then + expect(firstResult).toEqual(secondResult) + expect(existsSync.mock.calls.length).toBe(callsAfterFirstResult) + expect(readdirSync).toHaveBeenCalledTimes(0) + }) + + test("clears cached result when requested", async () => { + // given + const existsSync = spyOn(fs, "existsSync").mockImplementation((filePath: fs.PathLike) => { + return String(filePath).endsWith("oh-my-openagent.jsonc") + }) + const readdirSync = spyOn(fs, "readdirSync").mockImplementation(() => []) + spyOn(fs, "readFileSync").mockImplementation(() => "") + + const parserModule = await import(`./jsonc-parser?memoization=${Date.now()}-${Math.random()}`) + + parserModule.detectPluginConfigFile(testDir) + parserModule.clearPluginConfigFileDetectionCache() + const callsAfterClear = existsSync.mock.calls.length + + // when + parserModule.detectPluginConfigFile(testDir) + + // then + expect(existsSync.mock.calls.length).toBeGreaterThan(callsAfterClear) + expect(readdirSync).toHaveBeenCalledTimes(0) + }) +}) diff --git a/src/shared/jsonc-parser.test.ts b/src/shared/jsonc-parser.test.ts index 279db1fc5..c06e36353 100644 --- a/src/shared/jsonc-parser.test.ts +++ b/src/shared/jsonc-parser.test.ts @@ -1,5 +1,5 @@ -import { describe, expect, test } from "bun:test" -import { detectConfigFile, detectPluginConfigFile, parseJsonc, parseJsoncSafe, readJsoncFile } from "./jsonc-parser" +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import { clearPluginConfigFileDetectionCache, detectConfigFile, detectPluginConfigFile, parseJsonc, parseJsoncSafe, readJsoncFile } from "./jsonc-parser" import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs" import { join } from "node:path" @@ -330,6 +330,14 @@ describe("detectConfigFile", () => { describe("detectPluginConfigFile", () => { const testDir = join(__dirname, ".test-detect-plugin") + beforeEach(() => { + clearPluginConfigFileDetectionCache() + }) + + afterEach(() => { + clearPluginConfigFileDetectionCache() + }) + test("prefers oh-my-openagent over oh-my-opencode when both jsonc files exist", () => { // given if (!existsSync(testDir)) mkdirSync(testDir, { recursive: true }) diff --git a/src/shared/jsonc-parser.ts b/src/shared/jsonc-parser.ts index da1e0d98c..bb7148983 100644 --- a/src/shared/jsonc-parser.ts +++ b/src/shared/jsonc-parser.ts @@ -9,6 +9,14 @@ export interface JsoncParseResult { errors: Array<{ message: string; offset: number; length: number }> } +type DetectPluginConfigResult = { + format: "json" | "jsonc" | "none" + path: string + legacyPath?: string +} + +const pluginConfigFileDetectionCache = new Map() + function stripBom(content: string): string { return content.charCodeAt(0) === 0xfeff ? content.slice(1) : content } @@ -75,24 +83,34 @@ export function detectConfigFile(basePath: string): { return { format: "none", path: jsonPath } } -export function detectPluginConfigFile(dir: string): { - format: "json" | "jsonc" | "none" - path: string - legacyPath?: string -} { +export function clearPluginConfigFileDetectionCache(): void { + pluginConfigFileDetectionCache.clear() +} + +export function detectPluginConfigFile(dir: string): DetectPluginConfigResult { + const cachedResult = pluginConfigFileDetectionCache.get(dir) + + if (cachedResult !== undefined) { + return cachedResult + } + const canonicalResult = detectConfigFile(join(dir, CONFIG_BASENAME)) const legacyResult = detectConfigFile(join(dir, LEGACY_CONFIG_BASENAME)) + let detectionResult: DetectPluginConfigResult + if (canonicalResult.format !== "none") { - return { + detectionResult = { ...canonicalResult, legacyPath: legacyResult.format !== "none" ? legacyResult.path : undefined, } + } else if (legacyResult.format !== "none") { + detectionResult = legacyResult + } else { + detectionResult = { format: "none", path: join(dir, `${CONFIG_BASENAME}.json`) } } - if (legacyResult.format !== "none") { - return legacyResult - } + pluginConfigFileDetectionCache.set(dir, detectionResult) - return { format: "none", path: join(dir, `${CONFIG_BASENAME}.json`) } + return detectionResult } diff --git a/src/shared/load-opencode-plugins.test.ts b/src/shared/load-opencode-plugins.test.ts new file mode 100644 index 000000000..9723c1cd3 --- /dev/null +++ b/src/shared/load-opencode-plugins.test.ts @@ -0,0 +1,89 @@ +/// + +import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test" +import * as fs from "node:fs" + +type LoadOpencodePluginsModule = { + loadOpencodePlugins: (directory: string) => string[] + clearOpencodePluginsCache?: () => void +} + +const existsSyncMock = mock((_path: string) => true) +const readFileSyncMock = mock((_path: string, _encoding?: string) => `{ + "plugin": ["plugin-a", "plugin-b"] +}`) + +async function importFreshLoadOpencodePluginsModule(): Promise { + const modulePath = `${new URL("./load-opencode-plugins.ts", import.meta.url).pathname}?test=${Date.now()}-${Math.random()}` + return import(modulePath) +} + +describe("loadOpencodePlugins", () => { + beforeEach(() => { + existsSyncMock.mockReset() + existsSyncMock.mockImplementation((_path: string) => true) + readFileSyncMock.mockReset() + readFileSyncMock.mockImplementation((_path: string, _encoding?: string) => `{ + "plugin": ["plugin-a", "plugin-b"] +}`) + + mock.module("node:fs", () => ({ + ...fs, + existsSync: existsSyncMock, + readFileSync: readFileSyncMock, + })) + }) + + afterEach(() => { + mock.restore() + }) + + describe("#given the same directory is loaded twice", () => { + describe("#when loading plugins repeatedly", () => { + it("#then does not call readFileSync on the second load", async () => { + // given + const { loadOpencodePlugins } = await importFreshLoadOpencodePluginsModule() + + // when + const firstResult = loadOpencodePlugins("/some/fake/dir") + const readCountAfterFirstLoad = readFileSyncMock.mock.calls.length + const secondResult = loadOpencodePlugins("/some/fake/dir") + const readCountAfterSecondLoad = readFileSyncMock.mock.calls.length + + // then + expect(firstResult).toEqual(["plugin-a", "plugin-b"]) + expect(secondResult).toEqual(["plugin-a", "plugin-b"]) + expect(readCountAfterFirstLoad).toBeGreaterThan(0) + expect(readCountAfterSecondLoad - readCountAfterFirstLoad).toBe(0) + }) + }) + }) + + describe("#given the plugin cache was cleared", () => { + describe("#when loading the same directory again", () => { + it("#then re-reads plugin config files from disk", async () => { + // given + const { loadOpencodePlugins, clearOpencodePluginsCache } = await importFreshLoadOpencodePluginsModule() + + if (typeof clearOpencodePluginsCache !== "function") { + throw new Error("clearOpencodePluginsCache export is missing") + } + + // when + const firstResult = loadOpencodePlugins("/some/fake/dir") + const readCountAfterFirstLoad = readFileSyncMock.mock.calls.length + loadOpencodePlugins("/some/fake/dir") + const readCountAfterSecondLoad = readFileSyncMock.mock.calls.length + clearOpencodePluginsCache() + const thirdResult = loadOpencodePlugins("/some/fake/dir") + const readCountAfterThirdLoad = readFileSyncMock.mock.calls.length + + // then + expect(firstResult).toEqual(["plugin-a", "plugin-b"]) + expect(thirdResult).toEqual(["plugin-a", "plugin-b"]) + expect(readCountAfterSecondLoad - readCountAfterFirstLoad).toBe(0) + expect(readCountAfterThirdLoad - readCountAfterSecondLoad).toBeGreaterThan(0) + }) + }) + }) +}) diff --git a/src/shared/load-opencode-plugins.ts b/src/shared/load-opencode-plugins.ts index 5517c74b1..a6beffdcf 100644 --- a/src/shared/load-opencode-plugins.ts +++ b/src/shared/load-opencode-plugins.ts @@ -8,6 +8,8 @@ interface OpencodeConfig { plugin?: (string | [string, ...unknown[]])[] } +const opencodePluginsCache = new Map() + function getWindowsAppdataDir(): string | null { return process.env.APPDATA || null } @@ -33,6 +35,11 @@ function getConfigPaths(directory: string): string[] { } export function loadOpencodePlugins(directory: string): string[] { + const cachedPluginEntries = opencodePluginsCache.get(directory) + if (cachedPluginEntries) { + return cachedPluginEntries + } + const pluginEntries: string[] = [] const seenPluginEntries = new Set() @@ -56,5 +63,10 @@ export function loadOpencodePlugins(directory: string): string[] { } } + opencodePluginsCache.set(directory, pluginEntries) return pluginEntries } + +export function clearOpencodePluginsCache(): void { + opencodePluginsCache.clear() +} diff --git a/src/shared/project-discovery-dirs.test.ts b/src/shared/project-discovery-dirs.test.ts index 39ba5dc13..d2904bc72 100644 --- a/src/shared/project-discovery-dirs.test.ts +++ b/src/shared/project-discovery-dirs.test.ts @@ -1,15 +1,10 @@ -import { afterEach, beforeEach, describe, expect, it } from "bun:test" +import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test" import { mkdirSync, realpathSync, rmSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" -import { - findProjectAgentsSkillDirs, - findProjectClaudeSkillDirs, - findProjectOpencodeCommandDirs, - findProjectOpencodeSkillDirs, -} from "./project-discovery-dirs" const TEST_DIR = join(tmpdir(), `project-discovery-dirs-${Date.now()}`) +let worktreeSpawnCount = 0 function canonicalPath(path: string): string { return realpathSync(path) @@ -24,7 +19,35 @@ describe("project-discovery-dirs", () => { rmSync(TEST_DIR, { recursive: true, force: true }) }) - it("#given nested .opencode skill directories #when finding project opencode skill dirs #then returns nearest-first with aliases", () => { + it("#given repeated worktree detection #when detecting twice #then reuses the cached result", async () => { + // given + worktreeSpawnCount = 0 + + mock.module("node:child_process", () => ({ + execFileSync: () => { + worktreeSpawnCount += 1 + return TEST_DIR + }, + })) + + const { clearWorktreeCache, detectWorktreePath } = await import("./project-discovery-dirs") + + clearWorktreeCache() + + // when + const firstPath = detectWorktreePath("/some/dir") + const secondPath = detectWorktreePath("/some/dir") + clearWorktreeCache() + const thirdPath = detectWorktreePath("/some/dir") + + // then + expect(firstPath).toBe(TEST_DIR) + expect(secondPath).toBe(TEST_DIR) + expect(thirdPath).toBe(TEST_DIR) + expect(worktreeSpawnCount).toBe(2) + }) + + it("#given nested .opencode skill directories #when finding project opencode skill dirs #then returns nearest-first with aliases", async () => { // given const projectDir = join(TEST_DIR, "project") const childDir = join(projectDir, "apps", "cli") @@ -32,6 +55,8 @@ describe("project-discovery-dirs", () => { mkdirSync(join(projectDir, ".opencode", "skills"), { recursive: true }) mkdirSync(join(TEST_DIR, ".opencode", "skills"), { recursive: true }) + const { findProjectOpencodeSkillDirs } = await import("./project-discovery-dirs") + // when const directories = findProjectOpencodeSkillDirs(childDir) @@ -43,13 +68,15 @@ describe("project-discovery-dirs", () => { ]) }) - it("#given nested .opencode command directories #when finding project opencode command dirs #then returns nearest-first with aliases", () => { + it("#given nested .opencode command directories #when finding project opencode command dirs #then returns nearest-first with aliases", async () => { // given const projectDir = join(TEST_DIR, "project") const childDir = join(projectDir, "packages", "tool") mkdirSync(join(projectDir, ".opencode", "commands"), { recursive: true }) mkdirSync(join(TEST_DIR, ".opencode", "command"), { recursive: true }) + const { findProjectOpencodeCommandDirs } = await import("./project-discovery-dirs") + // when const directories = findProjectOpencodeCommandDirs(childDir) @@ -60,13 +87,15 @@ describe("project-discovery-dirs", () => { ]) }) - it("#given ancestor claude and agents skill directories #when finding project compatibility dirs #then discovers both scopes", () => { + it("#given ancestor claude and agents skill directories #when finding project compatibility dirs #then discovers both scopes", async () => { // given const projectDir = join(TEST_DIR, "project") const childDir = join(projectDir, "src", "nested") mkdirSync(join(projectDir, ".claude", "skills"), { recursive: true }) mkdirSync(join(TEST_DIR, ".agents", "skills"), { recursive: true }) + const { findProjectAgentsSkillDirs, findProjectClaudeSkillDirs } = await import("./project-discovery-dirs") + // when const claudeDirectories = findProjectClaudeSkillDirs(childDir) const agentsDirectories = findProjectAgentsSkillDirs(childDir) @@ -76,17 +105,20 @@ describe("project-discovery-dirs", () => { expect(agentsDirectories).toEqual([canonicalPath(join(TEST_DIR, ".agents", "skills"))]) }) - it("#given a stop directory #when finding ancestor dirs #then it does not scan beyond the stop boundary", () => { + it("#given a stop directory #when finding ancestor dirs #then it does not scan beyond the stop boundary", async () => { // given const projectDir = join(TEST_DIR, "project") const childDir = join(projectDir, "apps", "cli") mkdirSync(join(projectDir, ".opencode", "skills"), { recursive: true }) mkdirSync(join(TEST_DIR, ".opencode", "skills"), { recursive: true }) + const { findProjectOpencodeSkillDirs } = await import("./project-discovery-dirs") + // when const directories = findProjectOpencodeSkillDirs(childDir, projectDir) // then expect(directories).toEqual([canonicalPath(join(projectDir, ".opencode", "skills"))]) }) + }) diff --git a/src/shared/project-discovery-dirs.ts b/src/shared/project-discovery-dirs.ts index 4e22b66f6..5e243df5a 100644 --- a/src/shared/project-discovery-dirs.ts +++ b/src/shared/project-discovery-dirs.ts @@ -2,6 +2,8 @@ import { execFileSync } from "node:child_process" import { existsSync, realpathSync } from "node:fs" import { dirname, join, resolve } from "node:path" +const worktreePathCache = new Map() + function normalizePath(path: string): string { const resolvedPath = resolve(path) if (!existsSync(resolvedPath)) { @@ -49,15 +51,28 @@ function findAncestorDirectories( } } -function detectWorktreePath(directory: string): string | undefined { +export function clearWorktreeCache(): void { + worktreePathCache.clear() +} + +export function detectWorktreePath(directory: string): string | undefined { + const resolvedDirectory = resolve(directory) + if (worktreePathCache.has(resolvedDirectory)) { + return worktreePathCache.get(resolvedDirectory) + } + try { - return execFileSync("git", ["rev-parse", "--show-toplevel"], { - cwd: directory, + const worktreePath = execFileSync("git", ["rev-parse", "--show-toplevel"], { + cwd: resolvedDirectory, encoding: "utf-8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"], }).trim() + + worktreePathCache.set(resolvedDirectory, worktreePath) + return worktreePath } catch { + worktreePathCache.set(resolvedDirectory, undefined) return undefined } } diff --git a/src/tools/skill/description-formatter.ts b/src/tools/skill/description-formatter.ts index fb8dd87c5..20907cda1 100644 --- a/src/tools/skill/description-formatter.ts +++ b/src/tools/skill/description-formatter.ts @@ -38,14 +38,17 @@ function formatSlashCommand(command: CommandInfo): string { return lines.join("\n") } -export function formatCombinedDescription(skills: SkillInfo[], commands: CommandInfo[]): string { - if (skills.length === 0 && commands.length === 0) { +export function formatCombinedDescription(skills?: SkillInfo[], commands?: CommandInfo[]): string { + const availableSkills = skills ?? [] + const availableCommands = commands ?? [] + + if (availableSkills.length === 0 && availableCommands.length === 0) { return TOOL_DESCRIPTION_NO_SKILLS } const availableItems = [ - ...sortByScopePriority(skills).map(formatSkillCommand), - ...sortByScopePriority(commands).map(formatSlashCommand), + ...sortByScopePriority(availableSkills).map(formatSkillCommand), + ...sortByScopePriority(availableCommands).map(formatSlashCommand), ] if (availableItems.length === 0) { diff --git a/src/tools/skill/session-skill-cache.ts b/src/tools/skill/session-skill-cache.ts new file mode 100644 index 000000000..040979ddb --- /dev/null +++ b/src/tools/skill/session-skill-cache.ts @@ -0,0 +1,10 @@ +const seenSessionIDs = new Set() + +export function shouldInvalidateSkillCacheForSession(sessionID?: string): boolean { + if (!sessionID || seenSessionIDs.has(sessionID)) { + return false + } + + seenSessionIDs.add(sessionID) + return true +} diff --git a/src/tools/skill/tools.factory.test.ts b/src/tools/skill/tools.factory.test.ts new file mode 100644 index 000000000..5b9a5ba30 --- /dev/null +++ b/src/tools/skill/tools.factory.test.ts @@ -0,0 +1,132 @@ +/// + +import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test" +import type { ToolContext } from "@opencode-ai/plugin/tool" +import type { LoadedSkill } from "../../features/opencode-skill-loader/types" +import * as skillContent from "../../features/opencode-skill-loader/skill-content" + +const discoverCommandsSync = mock(() => []) + +mock.module("../slashcommand/command-discovery", () => ({ + discoverCommandsSync, +})) + +function createMockSkill(name: string): LoadedSkill { + return { + name, + definition: { + name, + description: `Test skill ${name}`, + template: `Test skill template for ${name}`, + }, + scope: "config", + } +} + +async function flushMicrotasks(): Promise { + await Promise.resolve() + await Promise.resolve() +} + +const loadedSkill = createMockSkill("lazy-skill") +const getAllSkills = mock(async () => [loadedSkill]) +const clearSkillCache = mock(() => {}) +const mockContext: ToolContext = { + sessionID: "test-session", + messageID: "msg-1", + agent: "test-agent", + directory: "/test", + worktree: "/test", + abort: new AbortController().signal, + metadata: () => {}, + ask: async () => {}, +} + +function createMockContext(sessionID: string): ToolContext { + return { + ...mockContext, + sessionID, + } +} + +beforeEach(() => { + spyOn(skillContent, "getAllSkills").mockImplementation(getAllSkills) + spyOn(skillContent, "clearSkillCache").mockImplementation(clearSkillCache) +}) + +afterEach(async () => { + await flushMicrotasks() + mock.restore() +}) + +describe("createSkillTool", () => { + it("delays command discovery until the description getter is accessed", async () => { + // given + const baselineDiscoverCommandsSyncCalls = discoverCommandsSync.mock.calls.length + + // when + const { createSkillTool } = await import("./tools") + const skillTool = createSkillTool({}) + + // then + expect(discoverCommandsSync.mock.calls.length).toBe(baselineDiscoverCommandsSyncCalls) + + void skillTool.description + await flushMicrotasks() + + expect(discoverCommandsSync.mock.calls.length).toBe(baselineDiscoverCommandsSyncCalls + 1) + }) + + it("delays skill loading until execute is invoked", async () => { + // given + const baselineGetAllSkillsCalls = getAllSkills.mock.calls.length + + // when + const { createSkillTool } = await import("./tools") + const skillTool = createSkillTool({}) + + // then + expect(getAllSkills.mock.calls.length).toBe(baselineGetAllSkillsCalls) + + await skillTool.execute({ name: "lazy-skill" }, mockContext) + + expect(getAllSkills.mock.calls.length).toBe(baselineGetAllSkillsCalls + 1) + }) + + it("clears the shared skill cache once on first execute in a session", async () => { + // given + const baselineClearSkillCacheCalls = clearSkillCache.mock.calls.length + const sessionContext = createMockContext("session-clear-once") + + // when + const { createSkillTool } = await import("./tools") + const skillTool = createSkillTool({}) + void skillTool.description + await flushMicrotasks() + await skillTool.execute({ name: "lazy-skill" }, sessionContext) + await skillTool.execute({ name: "lazy-skill" }, sessionContext) + + // then + expect(clearSkillCache.mock.calls.length).toBe(baselineClearSkillCacheCalls + 1) + }) + + it("clears the skill discovery cache once per session", async () => { + // given + const baselineClearSkillCacheCalls = clearSkillCache.mock.calls.length + const baselineGetAllSkillsCalls = getAllSkills.mock.calls.length + const sessionAContext = createMockContext("session-a") + const sessionBContext = createMockContext("session-b") + const { createSkillTool } = await import("./tools") + const skillTool = createSkillTool({}) + + // when + await skillTool.execute({ name: "lazy-skill" }, sessionAContext) + await skillTool.execute({ name: "lazy-skill" }, sessionAContext) + await skillTool.execute({ name: "lazy-skill" }, sessionBContext) + await skillTool.execute({ name: "lazy-skill" }, sessionBContext) + + // then + expect(clearSkillCache.mock.calls.length).toBe(baselineClearSkillCacheCalls + 2) + expect(getAllSkills.mock.calls.length).toBe(baselineGetAllSkillsCalls + 4) + }) +}) diff --git a/src/tools/skill/tools.ts b/src/tools/skill/tools.ts index 0fada5607..d49936f95 100644 --- a/src/tools/skill/tools.ts +++ b/src/tools/skill/tools.ts @@ -2,9 +2,10 @@ import { dirname } from "node:path" import { tool, type ToolDefinition } from "@opencode-ai/plugin" import type { ToolContext } from "@opencode-ai/plugin/tool" import { TOOL_DESCRIPTION_PREFIX } from "./constants" +import { shouldInvalidateSkillCacheForSession } from "./session-skill-cache" import type { SkillArgs, SkillLoadOptions } from "./types" import type { LoadedSkill } from "../../features/opencode-skill-loader" -import { getAllSkills, clearSkillCache } from "../../features/opencode-skill-loader/skill-content" +import { clearSkillCache, getAllSkills } from "../../features/opencode-skill-loader/skill-content" import { injectGitMasterConfig } from "../../features/opencode-skill-loader/skill-content" import { discoverCommandsSync } from "../slashcommand/command-discovery" import type { CommandInfo } from "../slashcommand/types" @@ -27,12 +28,15 @@ import { export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition { let cachedDescription: string | null = null - const getSkills = async (): Promise => { - clearSkillCache() - const discovered = await getAllSkills({ + const getSkills = async (context?: ToolContext): Promise => { + if (shouldInvalidateSkillCacheForSession(context?.sessionID)) { + clearSkillCache() + } + + const discovered = (await getAllSkills({ disabledSkills: options?.disabledSkills, browserProvider: options?.browserProvider, - }) + })) ?? [] const allSkills = !options.skills ? discovered : [ @@ -57,7 +61,7 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition return discoverCommandsSync(undefined, { pluginsEnabled: options.pluginsEnabled, enabledPluginsOverride: options.enabledPluginsOverride, - }) + }) ?? [] } const buildDescription = async (force = false): Promise => { @@ -92,8 +96,6 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition } } else if (options.commands !== undefined) { cachedDescription = formatCombinedDescription([], options.commands) - } else { - void buildDescription() } return tool({ @@ -111,7 +113,7 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition .describe("Optional arguments or context for command invocation. Example: name='publish', user_message='patch'"), }, async execute(args: SkillArgs, ctx?: ToolContext) { - const skills = await getSkills() + const skills = await getSkills(ctx) const commands = getCommands() cachedDescription = formatCombinedDescription(skills.map(loadedSkillToInfo), commands) diff --git a/src/tools/skill/zauc-mocks-skill-tools/tools.test.ts b/src/tools/skill/zauc-mocks-skill-tools/tools.test.ts index 5dac1e7d9..32dd83bde 100644 --- a/src/tools/skill/zauc-mocks-skill-tools/tools.test.ts +++ b/src/tools/skill/zauc-mocks-skill-tools/tools.test.ts @@ -1,7 +1,14 @@ +/// + +declare const require: NodeJS.Require + import { afterAll, beforeEach, describe, expect, it, mock, spyOn } from "bun:test" import type { ToolContext } from "@opencode-ai/plugin/tool" import * as fs from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" import { SkillMcpManager } from "../../../features/skill-mcp-manager" +import { clearSkillCache } from "../../../features/opencode-skill-loader/skill-content" import type { LoadedSkill } from "../../../features/opencode-skill-loader/types" import type { CommandInfo } from "../../slashcommand/types" import type { Tool as McpTool } from "@modelcontextprotocol/sdk/types.js" @@ -10,7 +17,24 @@ const originalReadFileSync = fs.readFileSync.bind(fs) let createSkillTool: typeof import("../tools").createSkillTool -beforeEach(async () => { +function clearRequireCache(modulePath: string): void { + const resolvedPath = require.resolve(modulePath) + if (require.cache?.[resolvedPath]) { + delete require.cache[resolvedPath] + } +} + +function requireFresh(modulePath: string): TModule { + clearRequireCache(modulePath) + return require(modulePath) as TModule +} + +beforeEach(() => { + mock.restore() + clearRequireCache("../tools") + clearRequireCache("../../../features/opencode-skill-loader/skill-content") + clearRequireCache("../../slashcommand/command-discovery") + mock.module("node:fs", () => ({ ...fs, readFileSync: (path: string, encoding?: string) => { @@ -23,9 +47,8 @@ Test skill body content` return originalReadFileSync(path, encoding as BufferEncoding) }, })) - - const module = await import("../tools") - createSkillTool = module.createSkillTool + + createSkillTool = requireFresh("../tools").createSkillTool }) afterAll(() => { @@ -548,16 +571,43 @@ describe("skill tool - ordering and priority", () => { }) describe("skill tool - dynamic discovery", () => { - it("discovers skills from disk on every invocation instead of caching", async () => { - // given: tool created with initial skills - const initialSkills = [createMockSkill("initial-skill")] - const tool = createSkillTool({ skills: initialSkills }) + it("caches discovered skills across tool instances until the shared cache resets", async () => { + // given + clearSkillCache() + const originalDirectory = process.cwd() + const temporaryDirectory = fs.mkdtempSync(join(tmpdir(), "skill-tool-cache-")) + const initialSkillDirectory = join(temporaryDirectory, ".opencode", "skills", "initial-skill") + const secondSkillDirectory = join(temporaryDirectory, ".opencode", "skills", "second-skill") - // when: executing with the initial skill name - const result = await tool.execute({ name: "initial-skill" }, mockContext) + fs.mkdirSync(initialSkillDirectory, { recursive: true }) + fs.writeFileSync(join(initialSkillDirectory, "SKILL.md"), "---\ndescription: Initial skill\n---\nInitial skill body") + process.chdir(temporaryDirectory) - // then: initial skill found (merged from options.skills since not on disk) - expect(result).toContain("Skill: initial-skill") + try { + const firstTool = createSkillTool({}) + + // when + const initialResult = await firstTool.execute({ name: "initial-skill" }, mockContext) + + fs.mkdirSync(secondSkillDirectory, { recursive: true }) + fs.writeFileSync(join(secondSkillDirectory, "SKILL.md"), "---\ndescription: Second skill\n---\nSecond skill body") + + const cachedTool = createSkillTool({}) + + // then + expect(initialResult).toContain("Skill: initial-skill") + let cachedError: Error | undefined + try { + await cachedTool.execute({ name: "second-skill" }, mockContext) + } catch (error) { + cachedError = error instanceof Error ? error : new Error(String(error)) + } + expect(cachedError?.message).toContain('Skill or command "second-skill" not found.') + } finally { + process.chdir(originalDirectory) + clearSkillCache() + fs.rmSync(temporaryDirectory, { recursive: true, force: true }) + } }) it("merges pre-provided skills with dynamically discovered ones", async () => { @@ -586,59 +636,66 @@ describe("skill tool - dynamic discovery", () => { }) }) describe("skill tool - dynamic description cache invalidation", () => { - it("rebuilds description after execute() discovers new skills", async () => { - // given: tool created with initial skills (no pre-provided skills) - // This triggers lazy description building + it("keeps description available after execute misses a skill", async () => { + // given const tool = createSkillTool({}) - - // Get initial description - it will build from empty or disk skills + + // when const initialDescription = tool.description expect(initialDescription).toBeString() - - // when: execute() is called, which clears cache AND gets fresh skills - // Note: In real scenario, execute() would discover new skills from disk - // For testing, we verify the mechanism: execute() should invalidate cachedDescription - - // Execute any skill to trigger the cache clear + getSkills flow - // Using a non-existent skill name to trigger the error path which still goes through getSkills() + try { await tool.execute({ name: "nonexistent-skill-12345" }, mockContext) - } catch (e) { - // Expected to fail - skill doesn't exist + } catch { } - - // then: cachedDescription should be invalidated, so next description access should rebuild - // We verify by checking that the description getter triggers a rebuild - // Since we can't easily mock getAllSkills in this test, we verify the cache invalidation mechanism - - // The key assertion: after execute(), the description should be rebuildable - // If cachedDescription wasn't invalidated, it would still return old value - // We verify by checking that the tool still has valid description structure + + // then expect(tool.description).toBeDefined() expect(typeof tool.description).toBe("string") }) - it("description reflects fresh skills after execute() clears cache", async () => { - // given: tool created without pre-provided skills (will use disk discovery) - const tool = createSkillTool({}) - - // when: execute() is called with a skill that exists on disk (via mock) - // This simulates the real scenario: execute() discovers skills, cache should be invalidated - - // Execute to trigger the cache invalidation path + it("picks up new disk skills only after the shared skill cache resets", async () => { + // given + clearSkillCache() + const originalDirectory = process.cwd() + const temporaryDirectory = fs.mkdtempSync(join(tmpdir(), "skill-tool-refresh-")) + const initialSkillDirectory = join(temporaryDirectory, ".opencode", "skills", "initial-skill") + const secondSkillDirectory = join(temporaryDirectory, ".opencode", "skills", "second-skill") + + fs.mkdirSync(initialSkillDirectory, { recursive: true }) + fs.writeFileSync(join(initialSkillDirectory, "SKILL.md"), "---\ndescription: Initial skill\n---\nInitial skill body") + process.chdir(temporaryDirectory) + try { - // This will call getSkills() which clears cache - await tool.execute({ name: "nonexistent" }, mockContext) - } catch (e) { - // Expected + const initialTool = createSkillTool({}) + await initialTool.execute({ name: "initial-skill" }, mockContext) + + fs.mkdirSync(secondSkillDirectory, { recursive: true }) + fs.writeFileSync(join(secondSkillDirectory, "SKILL.md"), "---\ndescription: Second skill\n---\nSecond skill body") + + const cachedTool = createSkillTool({}) + let cachedError: Error | undefined + try { + await cachedTool.execute({ name: "second-skill" }, mockContext) + } catch (error) { + cachedError = error instanceof Error ? error : new Error(String(error)) + } + expect(cachedError?.message).toContain('Skill or command "second-skill" not found.') + + clearSkillCache() + const refreshedTool = createSkillTool({}) + + // when + const refreshedResult = await refreshedTool.execute({ name: "second-skill" }, mockContext) + + // then + expect(refreshedResult).toContain("Skill: second-skill") + expect(refreshedTool.description).toContain("second-skill") + } finally { + process.chdir(originalDirectory) + clearSkillCache() + fs.rmSync(temporaryDirectory, { recursive: true, force: true }) } - - // then: description should still work and not be stale - // The bug would cause it to return old cached value forever - const desc = tool.description - - // Verify description is a valid string (not stale/old) - expect(desc).toContain("skill") }) }) diff --git a/src/tools/slashcommand/command-discovery.test.ts b/src/tools/slashcommand/command-discovery.test.ts index e82cd7653..fc193b61f 100644 --- a/src/tools/slashcommand/command-discovery.test.ts +++ b/src/tools/slashcommand/command-discovery.test.ts @@ -326,4 +326,40 @@ describe("non-directory commands path", () => { expect(testCmd).toBeDefined() expect(testCmd?.content).toContain("Test command content.") }) + + it("#given excluded subdirectories under .claude/commands #when discoverCommandsSync runs #then prunes commands beneath them", () => { + // given + const projectDir = join(testDir, "project") + const commandsDir = join(projectDir, ".claude", "commands") + + mkdirSync(join(commandsDir, "node_modules", "fake-pkg"), { recursive: true }) + mkdirSync(join(commandsDir, ".git", "branches"), { recursive: true }) + mkdirSync(join(commandsDir, "dist"), { recursive: true }) + writeFileSync( + join(commandsDir, "real-cmd.md"), + "---\ndescription: Real command\n---\nRun real command.\n", + ) + writeFileSync( + join(commandsDir, "node_modules", "fake-pkg", "cmd.md"), + "---\ndescription: Nested command\n---\nRun nested command.\n", + ) + writeFileSync( + join(commandsDir, ".git", "branches", "cmd.md"), + "---\ndescription: Git command\n---\nRun git command.\n", + ) + writeFileSync( + join(commandsDir, "dist", "bundled-cmd.md"), + "---\ndescription: Bundled command\n---\nRun bundled command.\n", + ) + + // when + const commands = discoverCommandsSync(projectDir) + const names = commands.map((command) => command.name) + + // then + expect(names).toContain("real-cmd") + expect(names).not.toContain("node_modules/fake-pkg/cmd") + expect(names).not.toContain(".git/branches/cmd") + expect(names).not.toContain("dist/bundled-cmd") + }) }) diff --git a/src/tools/slashcommand/command-discovery.ts b/src/tools/slashcommand/command-discovery.ts index 7d220ab4f..855f6dc28 100644 --- a/src/tools/slashcommand/command-discovery.ts +++ b/src/tools/slashcommand/command-discovery.ts @@ -6,6 +6,7 @@ import { findProjectOpencodeCommandDirs, getOpenCodeCommandDirs, discoverPluginCommandDefinitions, + EXCLUDED_DIRS, } from "../../shared" import type { CommandFrontmatter } from "../../features/claude-code-command-loader/types" import { isMarkdownFile } from "../../shared/file-utils" @@ -36,6 +37,7 @@ function discoverCommandsFromDir( for (const entry of entries) { if (entry.isDirectory()) { + if (EXCLUDED_DIRS.has(entry.name)) continue if (entry.name.startsWith(".")) continue const nestedPrefix = prefix ? `${prefix}${NESTED_COMMAND_SEPARATOR}${entry.name}` diff --git a/src/tools/slashcommand/execution-compatibility.test.ts b/src/tools/slashcommand/execution-compatibility.test.ts index 6d63bd678..a33b4bcc4 100644 --- a/src/tools/slashcommand/execution-compatibility.test.ts +++ b/src/tools/slashcommand/execution-compatibility.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test" import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" +import { clearCommandLoaderCache } from "../../features/claude-code-command-loader" function requireFresh(modulePath: string): T { const resolvedPath = require.resolve(modulePath) @@ -25,12 +26,14 @@ describe("slashcommand discovery and execution compatibility", () => { let originalOpencodeConfigDir: string | undefined beforeEach(() => { + clearCommandLoaderCache() tempDir = mkdtempSync(join(tmpdir(), "omo-slashcommand-compat-test-")) originalWorkingDirectory = process.cwd() originalOpencodeConfigDir = process.env.OPENCODE_CONFIG_DIR }) afterEach(() => { + clearCommandLoaderCache() process.chdir(originalWorkingDirectory) if (originalOpencodeConfigDir === undefined) {