fix(auto-slash-command): use event-ID dedup, align precedence, enforce skill agent gate
🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-openagent)
This commit is contained in:
@@ -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", () => {
|
||||
|
||||
98
src/hooks/auto-slash-command/executor-resolution.test.ts
Normal file
98
src/hooks/auto-slash-command/executor-resolution.test.ts
Normal file
@@ -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<LoadedSkill[]> => [],
|
||||
}))
|
||||
|
||||
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")
|
||||
})
|
||||
})
|
||||
@@ -41,6 +41,7 @@ export interface ExecutorOptions {
|
||||
skills?: LoadedSkill[]
|
||||
pluginsEnabled?: boolean
|
||||
enabledPluginsOverride?: Record<string, boolean>
|
||||
agent?: string
|
||||
}
|
||||
|
||||
function filterDiscoveredCommandsByScope(
|
||||
@@ -60,12 +61,12 @@ async function discoverAllCommands(options?: ExecutorOptions): Promise<CommandIn
|
||||
const skillCommands = skills.map(skillToCommandInfo)
|
||||
|
||||
return [
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "builtin"),
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "opencode-project"),
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "project"),
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "opencode"),
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "user"),
|
||||
...skillCommands,
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "project"),
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "user"),
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "opencode-project"),
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "opencode"),
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "builtin"),
|
||||
...filterDiscoveredCommandsByScope(discoveredCommands, "plugin"),
|
||||
]
|
||||
}
|
||||
@@ -141,6 +142,15 @@ export async function executeSlashCommand(parsed: ParsedSlashCommand, options?:
|
||||
}
|
||||
}
|
||||
|
||||
if (command.scope === "skill" && command.metadata.agent) {
|
||||
if (!options?.agent || command.metadata.agent !== options.agent) {
|
||||
return {
|
||||
success: false,
|
||||
error: `Skill "${command.name}" is restricted to agent "${command.metadata.agent}"`,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
const template = await formatCommandTemplate(command, parsed.args)
|
||||
return {
|
||||
|
||||
@@ -18,6 +18,8 @@ import type {
|
||||
} from "./types"
|
||||
import type { LoadedSkill } from "../../features/opencode-skill-loader"
|
||||
|
||||
const COMMAND_EXECUTE_FALLBACK_DEDUP_TTL_MS = 100
|
||||
|
||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||
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<void> => {
|
||||
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}`
|
||||
|
||||
|
||||
@@ -24,7 +24,7 @@ function removeSessionEntries(entries: Map<string, number>, 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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user