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
This commit is contained in:
@@ -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")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -9,6 +9,7 @@ import {
|
|||||||
AUTO_SLASH_COMMAND_TAG_CLOSE,
|
AUTO_SLASH_COMMAND_TAG_CLOSE,
|
||||||
AUTO_SLASH_COMMAND_TAG_OPEN,
|
AUTO_SLASH_COMMAND_TAG_OPEN,
|
||||||
} from "./constants"
|
} from "./constants"
|
||||||
|
import { createProcessedCommandStore } from "./processed-command-store"
|
||||||
import type {
|
import type {
|
||||||
AutoSlashCommandHookInput,
|
AutoSlashCommandHookInput,
|
||||||
AutoSlashCommandHookOutput,
|
AutoSlashCommandHookOutput,
|
||||||
@@ -17,8 +18,22 @@ import type {
|
|||||||
} from "./types"
|
} from "./types"
|
||||||
import type { LoadedSkill } from "../../features/opencode-skill-loader"
|
import type { LoadedSkill } from "../../features/opencode-skill-loader"
|
||||||
|
|
||||||
const sessionProcessedCommands = new Set<string>()
|
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||||
const sessionProcessedCommandExecutions = new Set<string>()
|
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 {
|
export interface AutoSlashCommandHookOptions {
|
||||||
skills?: LoadedSkill[]
|
skills?: LoadedSkill[]
|
||||||
@@ -32,6 +47,13 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions
|
|||||||
pluginsEnabled: options?.pluginsEnabled,
|
pluginsEnabled: options?.pluginsEnabled,
|
||||||
enabledPluginsOverride: options?.enabledPluginsOverride,
|
enabledPluginsOverride: options?.enabledPluginsOverride,
|
||||||
}
|
}
|
||||||
|
const sessionProcessedCommands = createProcessedCommandStore()
|
||||||
|
const sessionProcessedCommandExecutions = createProcessedCommandStore()
|
||||||
|
|
||||||
|
const dispose = (): void => {
|
||||||
|
sessionProcessedCommands.clear()
|
||||||
|
sessionProcessedCommandExecutions.clear()
|
||||||
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"chat.message": async (
|
"chat.message": async (
|
||||||
@@ -61,7 +83,9 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions
|
|||||||
return
|
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)) {
|
if (sessionProcessedCommands.has(commandKey)) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -101,7 +125,7 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions
|
|||||||
input: CommandExecuteBeforeInput,
|
input: CommandExecuteBeforeInput,
|
||||||
output: CommandExecuteBeforeOutput
|
output: CommandExecuteBeforeOutput
|
||||||
): Promise<void> => {
|
): Promise<void> => {
|
||||||
const commandKey = `${input.sessionID}:${input.command}:${Date.now()}`
|
const commandKey = `${input.sessionID}:${input.command.toLowerCase()}`
|
||||||
if (sessionProcessedCommandExecutions.has(commandKey)) {
|
if (sessionProcessedCommandExecutions.has(commandKey)) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -145,5 +169,23 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions
|
|||||||
command: input.command,
|
command: input.command,
|
||||||
})
|
})
|
||||||
},
|
},
|
||||||
|
event: async ({
|
||||||
|
event,
|
||||||
|
}: {
|
||||||
|
event: { type: string; properties?: unknown }
|
||||||
|
}): Promise<void> => {
|
||||||
|
if (event.type !== "session.deleted") {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
const sessionID = getDeletedSessionID(event.properties)
|
||||||
|
if (!sessionID) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
sessionProcessedCommands.cleanupSession(sessionID)
|
||||||
|
sessionProcessedCommandExecutions.cleanupSession(sessionID)
|
||||||
|
},
|
||||||
|
dispose,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,41 @@
|
|||||||
|
const MAX_PROCESSED_ENTRY_COUNT = 10_000
|
||||||
|
|
||||||
|
function trimProcessedEntries(entries: Set<string>): Set<string> {
|
||||||
|
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<string>, sessionID: string): Set<string> {
|
||||||
|
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<string>()
|
||||||
|
|
||||||
|
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()
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user