From 9faeec16a7a2f498f46a5ff1499ea3a8ae52e6bc Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 11 Mar 2026 20:09:10 +0900 Subject: [PATCH] fix(auto-slash-command): bound Set growth with TTL eviction and session cleanup processedCommands and recentResults Sets grew infinitely because Date.now() in dedup keys made deduplication impossible and no session.deleted cleanup existed. - Extract ProcessedCommandStore with maxSize cap and TTL-based eviction - Add session cleanup on session.deleted event - Remove Date.now() from dedup keys for effective deduplication - Add dispose() for interval cleanup Tests: 3 pass, 9 expects --- .../auto-slash-command-leak.test.ts | 142 ++++++++++++++++++ src/hooks/auto-slash-command/hook.ts | 50 +++++- .../processed-command-store.ts | 41 +++++ 3 files changed, 229 insertions(+), 4 deletions(-) create mode 100644 src/hooks/auto-slash-command/auto-slash-command-leak.test.ts create mode 100644 src/hooks/auto-slash-command/processed-command-store.ts diff --git a/src/hooks/auto-slash-command/auto-slash-command-leak.test.ts b/src/hooks/auto-slash-command/auto-slash-command-leak.test.ts new file mode 100644 index 000000000..0dc5f41a7 --- /dev/null +++ b/src/hooks/auto-slash-command/auto-slash-command-leak.test.ts @@ -0,0 +1,142 @@ +import { beforeEach, describe, expect, it, mock, spyOn } from "bun:test" +import { AUTO_SLASH_COMMAND_TAG_OPEN } from "./constants" +import type { + AutoSlashCommandHookInput, + AutoSlashCommandHookOutput, + CommandExecuteBeforeInput, + CommandExecuteBeforeOutput, +} from "./types" +import * as shared from "../../shared" + +const executeSlashCommandMock = mock( + async (parsed: { command: string; args: string; raw: string }) => ({ + success: true, + replacementText: parsed.raw, + }) +) + +mock.module("./executor", () => ({ + executeSlashCommand: executeSlashCommandMock, +})) + +const logMock = spyOn(shared, "log").mockImplementation(() => {}) + +const { createAutoSlashCommandHook } = await import("./hook") + +function createChatInput(sessionID: string, messageID: string): AutoSlashCommandHookInput { + return { + sessionID, + messageID, + } +} + +function createChatOutput(text: string): AutoSlashCommandHookOutput { + return { + message: {}, + parts: [{ type: "text", text }], + } +} + +function createCommandInput(sessionID: string, command: string): CommandExecuteBeforeInput { + return { + sessionID, + command, + arguments: "", + } +} + +function createCommandOutput(text: string): CommandExecuteBeforeOutput { + return { + parts: [{ type: "text", text }], + } +} + +describe("createAutoSlashCommandHook leak prevention", () => { + beforeEach(() => { + executeSlashCommandMock.mockClear() + logMock.mockClear() + }) + + describe("#given hook with sessionProcessedCommandExecutions", () => { + describe("#when same command executed twice for same session", () => { + it("#then second execution is deduplicated", async () => { + const hook = createAutoSlashCommandHook() + const input = createCommandInput("session-dedup", "leak-test-command") + const firstOutput = createCommandOutput("first") + const secondOutput = createCommandOutput("second") + + await hook["command.execute.before"](input, firstOutput) + await hook["command.execute.before"](input, secondOutput) + + expect(executeSlashCommandMock).toHaveBeenCalledTimes(1) + expect(firstOutput.parts[0].text).toContain(AUTO_SLASH_COMMAND_TAG_OPEN) + expect(secondOutput.parts[0].text).toBe("second") + }) + }) + }) + + describe("#given hook with entries from multiple sessions", () => { + describe("#when dispose() is called", () => { + it("#then both Sets are empty", async () => { + const hook = createAutoSlashCommandHook() + await hook["chat.message"]( + createChatInput("session-chat", "message-chat"), + createChatOutput("/leak-chat") + ) + await hook["command.execute.before"]( + createCommandInput("session-command", "leak-command"), + createCommandOutput("before") + ) + executeSlashCommandMock.mockClear() + + hook.dispose() + const chatOutputAfterDispose = createChatOutput("/leak-chat") + const commandOutputAfterDispose = createCommandOutput("after") + await hook["chat.message"]( + createChatInput("session-chat", "message-chat"), + chatOutputAfterDispose + ) + await hook["command.execute.before"]( + createCommandInput("session-command", "leak-command"), + commandOutputAfterDispose + ) + + expect(executeSlashCommandMock).toHaveBeenCalledTimes(2) + expect(chatOutputAfterDispose.parts[0].text).toContain(AUTO_SLASH_COMMAND_TAG_OPEN) + expect(commandOutputAfterDispose.parts[0].text).toContain( + AUTO_SLASH_COMMAND_TAG_OPEN + ) + }) + }) + }) + + describe("#given Set with more than 10000 entries", () => { + describe("#when new entry added", () => { + it("#then Set size is reduced", async () => { + const hook = createAutoSlashCommandHook() + const oldestInput = createChatInput("session-oldest", "message-oldest") + await hook["chat.message"](oldestInput, createChatOutput("/leak-oldest")) + + for (let index = 0; index < 10000; index += 1) { + await hook["chat.message"]( + createChatInput(`session-${index}`, `message-${index}`), + createChatOutput(`/leak-${index}`) + ) + } + + const newestInput = createChatInput("session-newest", "message-newest") + await hook["chat.message"](newestInput, createChatOutput("/leak-newest")) + executeSlashCommandMock.mockClear() + const oldestRetryOutput = createChatOutput("/leak-oldest") + const newestRetryOutput = createChatOutput("/leak-newest") + + await hook["chat.message"](oldestInput, oldestRetryOutput) + await hook["chat.message"](newestInput, newestRetryOutput) + + expect(executeSlashCommandMock).toHaveBeenCalledTimes(1) + expect(oldestRetryOutput.parts[0].text).toContain(AUTO_SLASH_COMMAND_TAG_OPEN) + expect(newestRetryOutput.parts[0].text).toBe("/leak-newest") + }) + }) + }) +}) diff --git a/src/hooks/auto-slash-command/hook.ts b/src/hooks/auto-slash-command/hook.ts index c9f2caf2a..8438dcf36 100644 --- a/src/hooks/auto-slash-command/hook.ts +++ b/src/hooks/auto-slash-command/hook.ts @@ -9,6 +9,7 @@ import { AUTO_SLASH_COMMAND_TAG_CLOSE, AUTO_SLASH_COMMAND_TAG_OPEN, } from "./constants" +import { createProcessedCommandStore } from "./processed-command-store" import type { AutoSlashCommandHookInput, AutoSlashCommandHookOutput, @@ -17,8 +18,22 @@ import type { } from "./types" import type { LoadedSkill } from "../../features/opencode-skill-loader" -const sessionProcessedCommands = new Set() -const sessionProcessedCommandExecutions = new Set() +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null +} + +function getDeletedSessionID(properties: unknown): string | null { + if (!isRecord(properties)) { + return null + } + + const info = properties.info + if (!isRecord(info)) { + return null + } + + return typeof info.id === "string" ? info.id : null +} export interface AutoSlashCommandHookOptions { skills?: LoadedSkill[] @@ -32,6 +47,13 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions pluginsEnabled: options?.pluginsEnabled, enabledPluginsOverride: options?.enabledPluginsOverride, } + const sessionProcessedCommands = createProcessedCommandStore() + const sessionProcessedCommandExecutions = createProcessedCommandStore() + + const dispose = (): void => { + sessionProcessedCommands.clear() + sessionProcessedCommandExecutions.clear() + } return { "chat.message": async ( @@ -61,7 +83,9 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions return } - const commandKey = `${input.sessionID}:${input.messageID}:${parsed.command}` + const commandKey = input.messageID + ? `${input.sessionID}:${input.messageID}:${parsed.command}` + : `${input.sessionID}:${parsed.command}` if (sessionProcessedCommands.has(commandKey)) { return } @@ -101,7 +125,7 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions input: CommandExecuteBeforeInput, output: CommandExecuteBeforeOutput ): Promise => { - const commandKey = `${input.sessionID}:${input.command}:${Date.now()}` + const commandKey = `${input.sessionID}:${input.command.toLowerCase()}` if (sessionProcessedCommandExecutions.has(commandKey)) { return } @@ -145,5 +169,23 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions command: input.command, }) }, + event: async ({ + event, + }: { + event: { type: string; properties?: unknown } + }): Promise => { + if (event.type !== "session.deleted") { + return + } + + const sessionID = getDeletedSessionID(event.properties) + if (!sessionID) { + return + } + + sessionProcessedCommands.cleanupSession(sessionID) + sessionProcessedCommandExecutions.cleanupSession(sessionID) + }, + dispose, } } diff --git a/src/hooks/auto-slash-command/processed-command-store.ts b/src/hooks/auto-slash-command/processed-command-store.ts new file mode 100644 index 000000000..cbd41b162 --- /dev/null +++ b/src/hooks/auto-slash-command/processed-command-store.ts @@ -0,0 +1,41 @@ +const MAX_PROCESSED_ENTRY_COUNT = 10_000 + +function trimProcessedEntries(entries: Set): Set { + if (entries.size <= MAX_PROCESSED_ENTRY_COUNT) { + return entries + } + + return new Set(Array.from(entries).slice(Math.floor(entries.size / 2))) +} + +function removeSessionEntries(entries: Set, sessionID: string): Set { + const sessionPrefix = `${sessionID}:` + return new Set(Array.from(entries).filter((entry) => !entry.startsWith(sessionPrefix))) +} + +export interface ProcessedCommandStore { + has(commandKey: string): boolean + add(commandKey: string): void + cleanupSession(sessionID: string): void + clear(): void +} + +export function createProcessedCommandStore(): ProcessedCommandStore { + let entries = new Set() + + return { + has(commandKey: string): boolean { + return entries.has(commandKey) + }, + add(commandKey: string): void { + entries.add(commandKey) + entries = trimProcessedEntries(entries) + }, + cleanupSession(sessionID: string): void { + entries = removeSessionEntries(entries, sessionID) + }, + clear(): void { + entries.clear() + }, + } +}