From 6da4d2dae0ba9cceb6a2456ff911f49257b2ce0e Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 24 Mar 2026 20:30:16 +0900 Subject: [PATCH] fix(hashline-edit): scope formatter cache by directory Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../formatter-trigger-cache.test.ts | 117 ++++++++++++++++++ src/tools/hashline-edit/formatter-trigger.ts | 20 ++- 2 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 src/tools/hashline-edit/formatter-trigger-cache.test.ts diff --git a/src/tools/hashline-edit/formatter-trigger-cache.test.ts b/src/tools/hashline-edit/formatter-trigger-cache.test.ts new file mode 100644 index 000000000..7b6c37bc0 --- /dev/null +++ b/src/tools/hashline-edit/formatter-trigger-cache.test.ts @@ -0,0 +1,117 @@ +import { beforeEach, describe, expect, it, mock } from "bun:test" + +import { clearFormatterCache, resolveFormatters, type FormatterClient } from "./formatter-trigger" + +function createDirectoryAwareClient( + resolveConfig: (directory: string) => Promise | undefined>, +): FormatterClient { + return { + config: { + get: mock(async ({ query }: { query?: { directory?: string } } = {}) => ({ + data: await resolveConfig(query?.directory ?? ""), + })), + }, + } +} + +describe("resolveFormatters cache behavior", () => { + beforeEach(() => { + clearFormatterCache() + }) + + it("caches formatter resolution per directory", async () => { + //#given + const client = createDirectoryAwareClient(async (directory) => { + if (directory === "/project-a") { + return { + formatter: { + prettier: { + command: ["prettier", "--write", "$FILE"], + extensions: [".ts"], + }, + }, + } + } + + return { + formatter: { + biome: { + command: ["biome", "format", "$FILE"], + extensions: [".ts"], + }, + }, + } + }) + + //#when + const firstProjectAResult = await resolveFormatters(client, "/project-a") + const projectBResult = await resolveFormatters(client, "/project-b") + const secondProjectAResult = await resolveFormatters(client, "/project-a") + + //#then + expect(client.config.get).toHaveBeenCalledTimes(2) + expect(firstProjectAResult.get(".ts")?.[0]?.command).toEqual(["prettier", "--write", "$FILE"]) + expect(projectBResult.get(".ts")?.[0]?.command).toEqual(["biome", "format", "$FILE"]) + expect(secondProjectAResult).toBe(firstProjectAResult) + }) + + it("does not cache transient config fetch failures", async () => { + //#given + const get = mock(async () => ({ + data: { + formatter: { + prettier: { + command: ["prettier", "--write", "$FILE"], + extensions: [".ts"], + }, + }, + }, + })) + + get.mockImplementationOnce(async () => { + throw new Error("network error") + }) + + const client: FormatterClient = { + config: { get }, + } + + //#when + const firstResult = await resolveFormatters(client, "/project-a") + const secondResult = await resolveFormatters(client, "/project-a") + + //#then + expect(get).toHaveBeenCalledTimes(2) + expect(firstResult.size).toBe(0) + expect(secondResult.get(".ts")?.[0]?.command).toEqual(["prettier", "--write", "$FILE"]) + }) + + it("does not cache missing config data", async () => { + //#given + let callCount = 0 + const client = createDirectoryAwareClient(async () => { + callCount += 1 + if (callCount === 1) { + return undefined + } + + return { + formatter: { + prettier: { + command: ["prettier", "--write", "$FILE"], + extensions: [".ts"], + }, + }, + } + }) + + //#when + const firstResult = await resolveFormatters(client, "/project-a") + const secondResult = await resolveFormatters(client, "/project-a") + + //#then + expect(client.config.get).toHaveBeenCalledTimes(2) + expect(firstResult.size).toBe(0) + expect(secondResult.get(".ts")?.[0]?.command).toEqual(["prettier", "--write", "$FILE"]) + }) +}) diff --git a/src/tools/hashline-edit/formatter-trigger.ts b/src/tools/hashline-edit/formatter-trigger.ts index 98733c182..370015844 100644 --- a/src/tools/hashline-edit/formatter-trigger.ts +++ b/src/tools/hashline-edit/formatter-trigger.ts @@ -25,15 +25,24 @@ export interface FormatterClient { } } -let cachedFormatters: Map }>> | null = null +type FormatterDefinition = { command: string[]; environment: Record } +type FormatterMap = Map + +const cachedFormattersByDirectory = new Map() + +function getFormatterCacheKey(directory: string): string { + return path.resolve(directory) +} export async function resolveFormatters( client: FormatterClient, directory: string, -): Promise }>>> { +): Promise { + const cacheKey = getFormatterCacheKey(directory) + const cachedFormatters = cachedFormattersByDirectory.get(cacheKey) if (cachedFormatters) return cachedFormatters - const result = new Map }>>() + const result = new Map() try { const response = await client.config.get({ query: { directory } }) @@ -68,11 +77,12 @@ export async function resolveFormatters( result.set(normalizedExt, existing) } } + + cachedFormattersByDirectory.set(cacheKey, result) } catch (error) { log("[formatter-trigger] Failed to fetch formatter config", { error }) } - cachedFormatters = result return result } @@ -118,5 +128,5 @@ export async function runFormattersForFile( } export function clearFormatterCache(): void { - cachedFormatters = null + cachedFormattersByDirectory.clear() }