From 32a296bf1e16239376b3f00117e7146f0154d90f Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 16 Mar 2026 13:53:12 +0900 Subject: [PATCH] fix(auto-slash-command): use event-ID dedup, align precedence, enforce skill agent gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-openagent) --- .../auto-slash-command-leak.test.ts | 85 ++++++++++------ .../executor-resolution.test.ts | 98 +++++++++++++++++++ src/hooks/auto-slash-command/executor.ts | 20 +++- src/hooks/auto-slash-command/hook.ts | 53 +++++++++- .../processed-command-store.ts | 6 +- src/hooks/auto-slash-command/types.ts | 9 ++ 6 files changed, 231 insertions(+), 40 deletions(-) create mode 100644 src/hooks/auto-slash-command/executor-resolution.test.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 index be35be92c..d402d9466 100644 --- a/src/hooks/auto-slash-command/auto-slash-command-leak.test.ts +++ b/src/hooks/auto-slash-command/auto-slash-command-leak.test.ts @@ -58,8 +58,8 @@ describe("createAutoSlashCommandHook leak prevention", () => { }) describe("#given hook with sessionProcessedCommandExecutions", () => { - describe("#when same command executed twice within TTL for same session", () => { - it("#then second execution is deduplicated", async () => { + describe("#when same command executed twice after fallback dedup window", () => { + it("#then second execution is treated as intentional rerun", async () => { //#given const nowSpy = spyOn(Date, "now") try { @@ -68,6 +68,61 @@ describe("createAutoSlashCommandHook leak prevention", () => { const firstOutput = createCommandOutput("first") const secondOutput = createCommandOutput("second") + //#when + nowSpy.mockReturnValue(0) + await hook["command.execute.before"](input, firstOutput) + nowSpy.mockReturnValue(101) + await hook["command.execute.before"](input, secondOutput) + + //#then + expect(executeSlashCommandMock).toHaveBeenCalledTimes(2) + expect(firstOutput.parts[0].text).toContain(AUTO_SLASH_COMMAND_TAG_OPEN) + expect(secondOutput.parts[0].text).toContain(AUTO_SLASH_COMMAND_TAG_OPEN) + } finally { + nowSpy.mockRestore() + } + }) + }) + + describe("#when same command is repeated within fallback dedup window", () => { + it("#then duplicate dispatch is suppressed", async () => { + //#given + const nowSpy = spyOn(Date, "now") + try { + const hook = createAutoSlashCommandHook() + const input = createCommandInput("session-dedup", "leak-test-command") + const firstOutput = createCommandOutput("first") + const secondOutput = createCommandOutput("second") + + //#when + nowSpy.mockReturnValue(0) + await hook["command.execute.before"](input, firstOutput) + nowSpy.mockReturnValue(99) + await hook["command.execute.before"](input, secondOutput) + + //#then + expect(executeSlashCommandMock).toHaveBeenCalledTimes(1) + expect(firstOutput.parts[0].text).toContain(AUTO_SLASH_COMMAND_TAG_OPEN) + expect(secondOutput.parts[0].text).toBe("second") + } finally { + nowSpy.mockRestore() + } + }) + }) + + describe("#when same event identifier is dispatched twice", () => { + it("#then second dispatch is deduplicated regardless of elapsed seconds", async () => { + //#given + const nowSpy = spyOn(Date, "now") + try { + const hook = createAutoSlashCommandHook() + const input: CommandExecuteBeforeInput = { + ...createCommandInput("session-dedup", "leak-test-command"), + eventID: "event-1", + } + const firstOutput = createCommandOutput("first") + const secondOutput = createCommandOutput("second") + //#when nowSpy.mockReturnValue(0) await hook["command.execute.before"](input, firstOutput) @@ -83,32 +138,6 @@ describe("createAutoSlashCommandHook leak prevention", () => { } }) }) - - describe("#when same command is repeated after TTL expires", () => { - it("#then command executes again", async () => { - //#given - const nowSpy = spyOn(Date, "now") - try { - const hook = createAutoSlashCommandHook() - const input = createCommandInput("session-dedup", "leak-test-command") - const firstOutput = createCommandOutput("first") - const secondOutput = createCommandOutput("second") - - //#when - nowSpy.mockReturnValue(0) - await hook["command.execute.before"](input, firstOutput) - nowSpy.mockReturnValue(30_001) - await hook["command.execute.before"](input, secondOutput) - - //#then - expect(executeSlashCommandMock).toHaveBeenCalledTimes(2) - expect(firstOutput.parts[0].text).toContain(AUTO_SLASH_COMMAND_TAG_OPEN) - expect(secondOutput.parts[0].text).toContain(AUTO_SLASH_COMMAND_TAG_OPEN) - } finally { - nowSpy.mockRestore() - } - }) - }) }) describe("#given hook with entries from multiple sessions", () => { diff --git a/src/hooks/auto-slash-command/executor-resolution.test.ts b/src/hooks/auto-slash-command/executor-resolution.test.ts new file mode 100644 index 000000000..70956546f --- /dev/null +++ b/src/hooks/auto-slash-command/executor-resolution.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it, mock } from "bun:test" +import type { LoadedSkill } from "../../features/opencode-skill-loader" + +mock.module("../../shared", () => ({ + resolveCommandsInText: async (content: string) => content, + resolveFileReferencesInText: async (content: string) => content, +})) + +mock.module("../../tools/slashcommand", () => ({ + discoverCommandsSync: () => [ + { + name: "shadowed", + metadata: { name: "shadowed", description: "builtin" }, + content: "builtin template", + scope: "builtin", + }, + { + name: "shadowed", + metadata: { name: "shadowed", description: "project" }, + content: "project template", + scope: "project", + }, + ], +})) + +mock.module("../../features/opencode-skill-loader", () => ({ + discoverAllSkills: async (): Promise => [], +})) + +const { executeSlashCommand } = await import("./executor") + +function createRestrictedSkill(): LoadedSkill { + return { + name: "restricted-skill", + definition: { + name: "restricted-skill", + description: "restricted", + template: "restricted template", + agent: "hephaestus", + }, + scope: "user", + } +} + +describe("executeSlashCommand resolution semantics", () => { + it("returns project command when project and builtin names collide", async () => { + //#given + const parsed = { + command: "shadowed", + args: "", + raw: "/shadowed", + } + + //#when + const result = await executeSlashCommand(parsed, { skills: [] }) + + //#then + expect(result.success).toBe(true) + expect(result.replacementText).toContain("**Scope**: project") + expect(result.replacementText).toContain("project template") + expect(result.replacementText).not.toContain("builtin template") + }) + + it("blocks slash skill invocation when invoking agent is missing", async () => { + //#given + const parsed = { + command: "restricted-skill", + args: "", + raw: "/restricted-skill", + } + + //#when + const result = await executeSlashCommand(parsed, { skills: [createRestrictedSkill()] }) + + //#then + expect(result.success).toBe(false) + expect(result.error).toBe('Skill "restricted-skill" is restricted to agent "hephaestus"') + }) + + it("allows slash skill invocation when invoking agent matches restriction", async () => { + //#given + const parsed = { + command: "restricted-skill", + args: "", + raw: "/restricted-skill", + } + + //#when + const result = await executeSlashCommand(parsed, { + skills: [createRestrictedSkill()], + agent: "hephaestus", + }) + + //#then + expect(result.success).toBe(true) + expect(result.replacementText).toContain("restricted template") + }) +}) diff --git a/src/hooks/auto-slash-command/executor.ts b/src/hooks/auto-slash-command/executor.ts index 8a7536ad9..cea80eb2a 100644 --- a/src/hooks/auto-slash-command/executor.ts +++ b/src/hooks/auto-slash-command/executor.ts @@ -41,6 +41,7 @@ export interface ExecutorOptions { skills?: LoadedSkill[] pluginsEnabled?: boolean enabledPluginsOverride?: Record + agent?: string } function filterDiscoveredCommandsByScope( @@ -60,12 +61,12 @@ async function discoverAllCommands(options?: ExecutorOptions): Promise { return typeof value === "object" && value !== null } @@ -35,6 +37,33 @@ function getDeletedSessionID(properties: unknown): string | null { return typeof info.id === "string" ? info.id : null } +function getCommandExecutionEventID(input: CommandExecuteBeforeInput): string | null { + const candidateKeys = [ + "messageID", + "messageId", + "eventID", + "eventId", + "invocationID", + "invocationId", + "commandID", + "commandId", + ] + + const recordInput = input as unknown + if (!isRecord(recordInput)) { + return null + } + + for (const key of candidateKeys) { + const candidateValue = recordInput[key] + if (typeof candidateValue === "string" && candidateValue.length > 0) { + return candidateValue + } + } + + return null +} + export interface AutoSlashCommandHookOptions { skills?: LoadedSkill[] pluginsEnabled?: boolean @@ -96,7 +125,12 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions args: parsed.args, }) - const result = await executeSlashCommand(parsed, executorOptions) + const executionOptions: ExecutorOptions = { + ...executorOptions, + agent: input.agent, + } + + const result = await executeSlashCommand(parsed, executionOptions) const idx = findSlashCommandPartIndex(output.parts) if (idx < 0) { @@ -125,7 +159,10 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions input: CommandExecuteBeforeInput, output: CommandExecuteBeforeOutput ): Promise => { - const commandKey = `${input.sessionID}:${input.command.toLowerCase()}:${input.arguments || ""}` + const eventID = getCommandExecutionEventID(input) + const commandKey = eventID + ? `${input.sessionID}:event:${eventID}` + : `${input.sessionID}:fallback:${input.command.toLowerCase()}:${input.arguments || ""}` if (sessionProcessedCommandExecutions.has(commandKey)) { return } @@ -142,7 +179,12 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions raw: `/${input.command}${input.arguments ? " " + input.arguments : ""}`, } - const result = await executeSlashCommand(parsed, executorOptions) + const executionOptions: ExecutorOptions = { + ...executorOptions, + agent: input.agent, + } + + const result = await executeSlashCommand(parsed, executionOptions) if (!result.success || !result.replacementText) { log(`[auto-slash-command] command.execute.before - command not found in our executor`, { @@ -153,7 +195,10 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions return } - sessionProcessedCommandExecutions.add(commandKey) + sessionProcessedCommandExecutions.add( + commandKey, + eventID ? undefined : COMMAND_EXECUTE_FALLBACK_DEDUP_TTL_MS + ) const taggedContent = `${AUTO_SLASH_COMMAND_TAG_OPEN}\n${result.replacementText}\n${AUTO_SLASH_COMMAND_TAG_CLOSE}` diff --git a/src/hooks/auto-slash-command/processed-command-store.ts b/src/hooks/auto-slash-command/processed-command-store.ts index 11ed3c86b..2f6d86949 100644 --- a/src/hooks/auto-slash-command/processed-command-store.ts +++ b/src/hooks/auto-slash-command/processed-command-store.ts @@ -24,7 +24,7 @@ function removeSessionEntries(entries: Map, sessionID: string): export interface ProcessedCommandStore { has(commandKey: string): boolean - add(commandKey: string): void + add(commandKey: string, ttlMs?: number): void cleanupSession(sessionID: string): void clear(): void } @@ -38,11 +38,11 @@ export function createProcessedCommandStore(): ProcessedCommandStore { entries = pruneExpiredEntries(entries, now) return entries.has(commandKey) }, - add(commandKey: string): void { + add(commandKey: string, ttlMs = PROCESSED_COMMAND_TTL_MS): void { const now = Date.now() entries = pruneExpiredEntries(entries, now) entries.delete(commandKey) - entries.set(commandKey, now + PROCESSED_COMMAND_TTL_MS) + entries.set(commandKey, now + ttlMs) entries = trimProcessedEntries(entries) }, cleanupSession(sessionID: string): void { diff --git a/src/hooks/auto-slash-command/types.ts b/src/hooks/auto-slash-command/types.ts index f2bc8617a..d23b1ed6b 100644 --- a/src/hooks/auto-slash-command/types.ts +++ b/src/hooks/auto-slash-command/types.ts @@ -26,6 +26,15 @@ export interface CommandExecuteBeforeInput { command: string sessionID: string arguments: string + agent?: string + messageID?: string + messageId?: string + eventID?: string + eventId?: string + invocationID?: string + invocationId?: string + commandID?: string + commandId?: string } export interface CommandExecuteBeforeOutput {