From 1b10ab36d2c5de743510574dec673b72647f9071 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 01:37:46 +0900 Subject: [PATCH 01/15] refactor(plugin): migrate to V1 PluginModule format Convert the default export from the legacy callable Plugin to the V1 PluginModule shape (`{ id, server }`) documented by opencode's plugin SDK. This aligns oh-my-openagent with the canonical plugin entry format and removes plugin-format legacy debt. Drop the module-level `let activePluginDispose` cleanup guard: opencode instantiates plugins in a scope-bound Layer per server and dynamic-imports fresh modules on reload, so module-level state is not preserved across reloads. Individual managers already register their own SIGINT/SIGTERM cleanup (skill-mcp-manager, background-agent process-cleanup), so the orphaned createPluginDispose call provided no runtime value. Remove the non-standard `name` return field that opencode's Hooks interface does not include. The PluginModule's `id` now carries the plugin identity instead. Drop the unused `lspManager` import that only fed the orphaned createPluginDispose. Update src/index.test.ts and src/index.telemetry.test.ts to call `plugin.server(ctx)` instead of `plugin(ctx)`, and assert the V1 shape. --- src/index.telemetry.test.ts | 7 ++--- src/index.test.ts | 21 ++++++++++----- src/index.ts | 52 +++++++++++++++---------------------- 3 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/index.telemetry.test.ts b/src/index.telemetry.test.ts index f3026a1cb..1f552f7eb 100644 --- a/src/index.telemetry.test.ts +++ b/src/index.telemetry.test.ts @@ -125,12 +125,13 @@ describe("OhMyOpenCodePlugin telemetry isolation", () => { const { default: plugin } = await import(`./index?telemetry=${Date.now()}-${Math.random()}`) // when - const result = await plugin({ + const result = await plugin.server({ directory: "/tmp/project", client: {}, - } as Parameters[0]) + } as Parameters[0]) // then - expect(result).toMatchObject({ name: "oh-my-openagent" }) + expect(typeof result).toBe("object") + expect(result).not.toBeNull() }) }) diff --git a/src/index.test.ts b/src/index.test.ts index 7101b9f09..00af70bb9 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -257,7 +257,7 @@ const mockCreatePluginInterface = mock(() => ({})) const mockInitializeOpenClaw = mock(async () => {}) const mockStartTmuxCheck = mock(() => {}) -let OhMyOpenCodePlugin: (typeof import("./index"))["default"] +let pluginModule: (typeof import("./index"))["default"] function installIndexModuleMocks(): void { mock.module("./cli/config-manager/config-context", () => ({ @@ -337,7 +337,7 @@ describe("OhMyOpenCodePlugin", () => { beforeEach(async () => { mock.restore() installIndexModuleMocks() - ;({ default: OhMyOpenCodePlugin } = await importFreshIndexModule()) + ;({ default: pluginModule } = await importFreshIndexModule()) mockInitConfigContext.mockClear() mockDetectExternalSkillPlugin.mockClear() mockGetSkillPluginConflictWarning.mockClear() @@ -375,10 +375,10 @@ describe("OhMyOpenCodePlugin", () => { }) // when - await OhMyOpenCodePlugin({ + await pluginModule.server({ directory: "/tmp/project", client: {}, - } as Parameters[0]) + } as Parameters[0]) // then expect(mockInitializeOpenClaw).toHaveBeenCalledTimes(1) @@ -390,12 +390,21 @@ describe("OhMyOpenCodePlugin", () => { mockLoadPluginConfig.mockReturnValue({}) // when - await OhMyOpenCodePlugin({ + await pluginModule.server({ directory: "/tmp/project", client: {}, - } as Parameters[0]) + } as Parameters[0]) // then expect(mockInitializeOpenClaw).not.toHaveBeenCalled() }) + + it("exports a V1 PluginModule shape with id and server", () => { + // given the plugin module is loaded + // when inspecting the default export + // then it has the expected V1 shape + expect(typeof pluginModule).toBe("object") + expect(pluginModule.id).toBe("oh-my-openagent") + expect(typeof pluginModule.server).toBe("function") + }) }) diff --git a/src/index.ts b/src/index.ts index b36e6c4b9..b41f611e1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,5 @@ import { initConfigContext } from "./cli/config-manager/config-context" -import type { Plugin } from "@opencode-ai/plugin" +import type { Hooks, Plugin, PluginModule } from "@opencode-ai/plugin" import type { HookName } from "./config" @@ -9,7 +9,6 @@ import { createRuntimeTmuxConfig, isTmuxIntegrationEnabled } from "./create-runt import { createTools } from "./create-tools" import { initializeOpenClaw } from "./openclaw" import { createPluginInterface } from "./plugin-interface" -import { createPluginDispose, type PluginDispose } from "./plugin-dispose" import { loadPluginConfig } from "./plugin-config" import { createModelCacheState } from "./plugin-state" @@ -17,27 +16,23 @@ import { createFirstMessageVariantGate } from "./shared/first-message-variant" import { injectServerAuthIntoClient, log, logLegacyPluginStartupWarning } from "./shared" import { detectExternalSkillPlugin, getSkillPluginConflictWarning } from "./shared/external-plugin-detector" import { startBackgroundCheck as startTmuxCheck } from "./tools/interactive-bash" -import { lspManager } from "./tools/lsp/client" import { createPluginPostHog, getPostHogDistinctId } from "./shared/posthog" -let activePluginDispose: PluginDispose | null = null - -const OhMyOpenCodePlugin: Plugin = async (ctx) => { +const serverPlugin: Plugin = async (input, _options): Promise => { initConfigContext("opencode", null) log("[OhMyOpenCodePlugin] ENTRY - plugin loading", { - directory: ctx.directory, + directory: input.directory, }) logLegacyPluginStartupWarning() - const skillPluginCheck = detectExternalSkillPlugin(ctx.directory) + const skillPluginCheck = detectExternalSkillPlugin(input.directory) if (skillPluginCheck.detected && skillPluginCheck.pluginName) { console.warn(getSkillPluginConflictWarning(skillPluginCheck.pluginName)) } - injectServerAuthIntoClient(ctx.client) - await activePluginDispose?.() + injectServerAuthIntoClient(input.client) - const pluginConfig = loadPluginConfig(ctx.directory, ctx) + const pluginConfig = loadPluginConfig(input.directory, input) const posthog = createPluginPostHog() const distinctId = getPostHogDistinctId() @@ -78,7 +73,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { const modelCacheState = createModelCacheState() const managers = createManagers({ - ctx, + ctx: input, pluginConfig, tmuxConfig, modelCacheState, @@ -86,13 +81,13 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { }) const toolsResult = await createTools({ - ctx, + ctx: input, pluginConfig, managers, }) const hooks = createHooks({ - ctx, + ctx: input, pluginConfig, modelCacheState, backgroundManager: managers.backgroundManager, @@ -102,15 +97,8 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { availableSkills: toolsResult.availableSkills, }) - const dispose = createPluginDispose({ - backgroundManager: managers.backgroundManager, - skillMcpManager: managers.skillMcpManager, - lspManager, - disposeHooks: hooks.disposeHooks, - }) - const pluginInterface = createPluginInterface({ - ctx, + ctx: input, pluginConfig, firstMessageVariantGate, managers, @@ -118,30 +106,32 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { tools: toolsResult.filteredTools, }) - activePluginDispose = dispose - return { - name: "oh-my-openagent", ...pluginInterface, "experimental.session.compacting": async ( - _input: { sessionID: string }, + compactingInput: { sessionID: string }, output: { context: string[] }, ): Promise => { - await hooks.compactionContextInjector?.capture(_input.sessionID) - await hooks.compactionTodoPreserver?.capture(_input.sessionID) + await hooks.compactionContextInjector?.capture(compactingInput.sessionID) + await hooks.compactionTodoPreserver?.capture(compactingInput.sessionID) await hooks.claudeCodeHooks?.["experimental.session.compacting"]?.( - _input, + compactingInput, output, ) if (hooks.compactionContextInjector) { - output.context.push(hooks.compactionContextInjector.inject(_input.sessionID)) + output.context.push(hooks.compactionContextInjector.inject(compactingInput.sessionID)) } }, } } -export default OhMyOpenCodePlugin +const pluginModule: PluginModule = { + id: "oh-my-openagent", + server: serverPlugin, +} + +export default pluginModule export type { OhMyOpenCodeConfig, From cb8f44ed9592368cc6f380ca04c995d7960a38f3 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 01:42:01 +0900 Subject: [PATCH 02/15] refactor(ralph-loop test): clarify race-condition predicate naming Rename the local wait predicate to avoid confusion with deprecated auth-prompt condition fields. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/hooks/ralph-loop/reset-strategy-race-condition.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks/ralph-loop/reset-strategy-race-condition.test.ts b/src/hooks/ralph-loop/reset-strategy-race-condition.test.ts index 5fcd35a2e..8f31f8ec2 100644 --- a/src/hooks/ralph-loop/reset-strategy-race-condition.test.ts +++ b/src/hooks/ralph-loop/reset-strategy-race-condition.test.ts @@ -21,9 +21,9 @@ function createDeferred(): { } } -async function waitUntil(condition: () => boolean): Promise { +async function waitUntil(shouldTrigger: () => boolean): Promise { for (let index = 0; index < 100; index++) { - if (condition()) { + if (shouldTrigger()) { return } From f94632ce649dfd296bc11b671d89723710a38901 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 01:43:08 +0900 Subject: [PATCH 03/15] refactor(skill-mcp): split tools.ts to comply with 200 LOC module rule Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../skill-mcp/parse-skill-mcp-arguments.ts | 25 +++++++++++++++++ src/tools/skill-mcp/tools.ts | 27 ++----------------- 2 files changed, 27 insertions(+), 25 deletions(-) create mode 100644 src/tools/skill-mcp/parse-skill-mcp-arguments.ts diff --git a/src/tools/skill-mcp/parse-skill-mcp-arguments.ts b/src/tools/skill-mcp/parse-skill-mcp-arguments.ts new file mode 100644 index 000000000..20e0b8158 --- /dev/null +++ b/src/tools/skill-mcp/parse-skill-mcp-arguments.ts @@ -0,0 +1,25 @@ +export function parseSkillMcpArguments( + argsJson: string | Record | undefined, +): Record { + if (!argsJson) return {} + if (typeof argsJson === "object" && argsJson !== null) { + return argsJson + } + + try { + const jsonString = argsJson.startsWith("'") && argsJson.endsWith("'") ? argsJson.slice(1, -1) : argsJson + const parsed = JSON.parse(jsonString) + if (typeof parsed !== "object" || parsed === null) { + throw new Error("Arguments must be a JSON object") + } + + return parsed as Record + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error) + throw new Error( + `Invalid arguments JSON: ${errorMessage}\n\n` + + `Expected a valid JSON object, e.g.: '{"key": "value"}'\n` + + `Received: ${argsJson}`, + ) + } +} diff --git a/src/tools/skill-mcp/tools.ts b/src/tools/skill-mcp/tools.ts index 2e1876575..25720baf8 100644 --- a/src/tools/skill-mcp/tools.ts +++ b/src/tools/skill-mcp/tools.ts @@ -1,6 +1,7 @@ import { tool, type ToolDefinition } from "@opencode-ai/plugin" import type { ToolContext } from "@opencode-ai/plugin/tool" import { BUILTIN_MCP_TOOL_HINTS, SKILL_MCP_DESCRIPTION } from "./constants" +import { parseSkillMcpArguments } from "./parse-skill-mcp-arguments" import type { SkillMcpArgs } from "./types" import type { SkillMcpManager, SkillMcpClientInfo, SkillMcpServerContext } from "../../features/skill-mcp-manager" import type { LoadedSkill } from "../../features/opencode-skill-loader/types" @@ -82,30 +83,6 @@ function formatBuiltinMcpHint(mcpName: string): string | null { ) } -function parseArguments(argsJson: string | Record | undefined): Record { - if (!argsJson) return {} - if (typeof argsJson === "object" && argsJson !== null) { - return argsJson - } - try { - // Strip outer single quotes if present (common in LLM output) - const jsonStr = argsJson.startsWith("'") && argsJson.endsWith("'") ? argsJson.slice(1, -1) : argsJson - - const parsed = JSON.parse(jsonStr) - if (typeof parsed !== "object" || parsed === null) { - throw new Error("Arguments must be a JSON object") - } - return parsed as Record - } catch (error) { - const errorMessage = error instanceof Error ? error.message : String(error) - throw new Error( - `Invalid arguments JSON: ${errorMessage}\n\n` + - `Expected a valid JSON object, e.g.: '{"key": "value"}'\n` + - `Received: ${argsJson}`, - ) - } -} - export function applyGrepFilter(output: string, pattern: string | undefined): string { if (!pattern) return output try { @@ -174,7 +151,7 @@ export function createSkillMcpTool(options: SkillMcpToolOptions): ToolDefinition skillName: found.skill.name, } - const parsedArgs = parseArguments(args.arguments) + const parsedArgs = parseSkillMcpArguments(args.arguments) let output: string switch (operation.type) { From 963355d2414d5431b4de100e70ffa483e9fda737 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 01:45:30 +0900 Subject: [PATCH 04/15] refactor(look-at): split tools.ts to comply with 200 LOC module rule Extract input preparation and image conversion handling into look-at-input-preparer.ts. Extract prompt construction and multimodal session execution into look-at-prompt.ts and look-at-session-runner.ts while keeping createLookAt stable. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/tools/look-at/look-at-input-preparer.ts | 154 +++++++++++++ src/tools/look-at/look-at-prompt.ts | 18 ++ src/tools/look-at/look-at-session-runner.ts | 107 +++++++++ src/tools/look-at/tools.ts | 227 ++------------------ 4 files changed, 297 insertions(+), 209 deletions(-) create mode 100644 src/tools/look-at/look-at-input-preparer.ts create mode 100644 src/tools/look-at/look-at-prompt.ts create mode 100644 src/tools/look-at/look-at-session-runner.ts diff --git a/src/tools/look-at/look-at-input-preparer.ts b/src/tools/look-at/look-at-input-preparer.ts new file mode 100644 index 000000000..e0eef0099 --- /dev/null +++ b/src/tools/look-at/look-at-input-preparer.ts @@ -0,0 +1,154 @@ +import { basename } from "node:path" +import { pathToFileURL } from "node:url" +import type { LookAtArgs } from "./types" +import { + extractBase64Data, + inferMimeTypeFromBase64, + inferMimeTypeFromFilePath, +} from "./mime-type-inference" +import { + needsConversion, + convertImageToJpeg, + convertBase64ImageToJpeg, + cleanupConvertedImage, +} from "./image-converter" +import { log } from "../../shared" + +export interface LookAtFilePart { + type: "file" + mime: string + url: string + filename: string +} + +export interface PreparedLookAtInput { + readonly filePart: LookAtFilePart + readonly isBase64Input: boolean + readonly sourceDescription: string + cleanup(): void +} + +type PrepareLookAtInputResult = + | { ok: true; value: PreparedLookAtInput } + | { ok: false; error: string } + +function getTemporaryConversionPath(error: unknown): string | null { + if (!(error instanceof Error)) { + return null + } + + const temporaryOutputPath = Reflect.get(error, "temporaryOutputPath") + if (typeof temporaryOutputPath === "string" && temporaryOutputPath.length > 0) { + return temporaryOutputPath + } + + const temporaryDirectory = Reflect.get(error, "temporaryDirectory") + if (typeof temporaryDirectory === "string" && temporaryDirectory.length > 0) { + return temporaryDirectory + } + + return null +} + +export function prepareLookAtInput(args: LookAtArgs): PrepareLookAtInputResult { + const imageData = args.image_data + const filePath = args.file_path + + if (imageData) { + const mimeType = inferMimeTypeFromBase64(imageData) + + let finalBase64Data = extractBase64Data(imageData) + let finalMimeType = mimeType + let tempFilesToCleanup: string[] = [] + + if (needsConversion(mimeType)) { + log(`[look_at] Detected unsupported Base64 format: ${mimeType}, converting to JPEG...`) + try { + const { base64, tempFiles } = convertBase64ImageToJpeg(finalBase64Data, mimeType) + finalBase64Data = base64 + finalMimeType = "image/jpeg" + tempFilesToCleanup = tempFiles + log("[look_at] Base64 conversion successful") + } catch (conversionError) { + log(`[look_at] Base64 conversion failed: ${conversionError}`) + return { + ok: false, + error: `Error: Failed to convert Base64 image format. ${conversionError}`, + } + } + } + + return { + ok: true, + value: { + isBase64Input: true, + sourceDescription: "clipboard/pasted image", + filePart: { + type: "file", + mime: finalMimeType, + url: `data:${finalMimeType};base64,${finalBase64Data}`, + filename: `clipboard-image.${finalMimeType.split("/")[1] || "png"}`, + }, + cleanup() { + for (const temporaryFile of tempFilesToCleanup) { + cleanupConvertedImage(temporaryFile) + } + }, + }, + } + } + + if (filePath) { + let mimeType = inferMimeTypeFromFilePath(filePath) + let actualFilePath = filePath + let tempFilePath: string | null = null + let tempConversionPath: string | null = null + + if (needsConversion(mimeType)) { + log(`[look_at] Detected unsupported format: ${mimeType}, converting to JPEG...`) + try { + tempFilePath = convertImageToJpeg(filePath, mimeType) + tempConversionPath = tempFilePath + actualFilePath = tempFilePath + mimeType = "image/jpeg" + log(`[look_at] Conversion successful: ${tempFilePath}`) + } catch (conversionError) { + const failedConversionPath = getTemporaryConversionPath(conversionError) + if (failedConversionPath) { + tempConversionPath = failedConversionPath + } + log(`[look_at] Conversion failed: ${conversionError}`) + return { + ok: false, + error: `Error: Failed to convert image format. ${conversionError}`, + } + } + } + + return { + ok: true, + value: { + isBase64Input: false, + sourceDescription: filePath, + filePart: { + type: "file", + mime: mimeType, + url: pathToFileURL(actualFilePath).href, + filename: basename(actualFilePath), + }, + cleanup() { + if (tempConversionPath) { + cleanupConvertedImage(tempConversionPath) + } else if (tempFilePath) { + cleanupConvertedImage(tempFilePath) + } + }, + }, + } + } + + return { + ok: false, + error: "Error: Must provide either 'file_path' or 'image_data'.", + } +} diff --git a/src/tools/look-at/look-at-prompt.ts b/src/tools/look-at/look-at-prompt.ts new file mode 100644 index 000000000..585a8c38f --- /dev/null +++ b/src/tools/look-at/look-at-prompt.ts @@ -0,0 +1,18 @@ +export const READ_ENABLED = false + +export function buildLookAtPrompt(goal: string, isBase64Input: boolean): string { + const subjectNoun = isBase64Input ? "image" : "file" + const sourceClause = READ_ENABLED + ? "Use the Read tool on the provided file path to load its contents, then analyze it." + : `The ${subjectNoun} is already attached to this message. Analyze it directly from the attachment. Do NOT attempt to use the Read tool. The Read tool is disabled for this invocation and the ${subjectNoun} cannot be loaded by path.` + + return `Analyze the attached ${subjectNoun} and extract the requested information. + +${sourceClause} + +Goal: ${goal} + +Provide ONLY the extracted information that matches the goal. +Be thorough on what was requested, concise on everything else. +If the requested information is not found, clearly state what is missing.` +} diff --git a/src/tools/look-at/look-at-session-runner.ts b/src/tools/look-at/look-at-session-runner.ts new file mode 100644 index 000000000..5b87a7852 --- /dev/null +++ b/src/tools/look-at/look-at-session-runner.ts @@ -0,0 +1,107 @@ +import type { PluginInput } from "@opencode-ai/plugin" +import type { ToolContext } from "@opencode-ai/plugin/tool" +import { log, promptSyncWithModelSuggestionRetry } from "../../shared" +import { extractLatestAssistantText } from "./assistant-message-extractor" +import { MULTIMODAL_LOOKER_AGENT } from "./constants" +import { READ_ENABLED, buildLookAtPrompt } from "./look-at-prompt" +import type { LookAtFilePart } from "./look-at-input-preparer" +import { resolveMultimodalLookerAgentMetadata } from "./multimodal-agent-metadata" + +interface RunLookAtSessionInput { + ctx: PluginInput + toolContext: ToolContext + goal: string + filePart: LookAtFilePart + isBase64Input: boolean +} + +export async function runLookAtSession({ + ctx, + toolContext, + goal, + filePart, + isBase64Input, +}: RunLookAtSessionInput): Promise { + const prompt = buildLookAtPrompt(goal, isBase64Input) + const { agentModel, agentVariant } = await resolveMultimodalLookerAgentMetadata(ctx) + + log(`[look_at] Creating session with parent: ${toolContext.sessionID}`) + const parentSession = await ctx.client.session.get({ + path: { id: toolContext.sessionID }, + }).catch(() => null) + const parentDirectory = parentSession?.data?.directory ?? ctx.directory + + const createResult = await ctx.client.session.create({ + body: { + parentID: toolContext.sessionID, + title: `look_at: ${goal.substring(0, 50)}`, + }, + query: { directory: parentDirectory }, + }) + + if (createResult.error) { + log("[look_at] Session create error:", createResult.error) + const errorString = String(createResult.error) + if (errorString.toLowerCase().includes("unauthorized")) { + return `Error: Failed to create session (Unauthorized). This may be due to: +1. OAuth token restrictions (e.g., Claude Code credentials are restricted to Claude Code only) +2. Provider authentication issues +3. Session permission inheritance problems + +Try using a different provider or API key authentication. + +Original error: ${createResult.error}` + } + + return `Error: Failed to create session: ${createResult.error}` + } + + const sessionID = createResult.data.id + log(`[look_at] Created session: ${sessionID}`) + + log(`[look_at] Sending prompt with ${isBase64Input ? "base64 image" : "file"} to session ${sessionID}`) + try { + await promptSyncWithModelSuggestionRetry(ctx.client, { + path: { id: sessionID }, + body: { + agent: MULTIMODAL_LOOKER_AGENT, + tools: { + task: false, + call_omo_agent: false, + look_at: false, + read: READ_ENABLED, + }, + parts: [ + { type: "text", text: prompt }, + filePart, + ], + ...(agentModel ? { model: { providerID: agentModel.providerID, modelID: agentModel.modelID } } : {}), + ...(agentVariant ? { variant: agentVariant } : {}), + }, + }) + } catch (promptError) { + log("[look_at] Prompt error (ignored, will still fetch messages):", promptError) + } + + log(`[look_at] Fetching messages from session ${sessionID}...`) + const messagesResult = await ctx.client.session.messages({ + path: { id: sessionID }, + }) + + if (messagesResult.error) { + log("[look_at] Messages error:", messagesResult.error) + return `Error: Failed to get messages: ${messagesResult.error}` + } + + const messages = messagesResult.data + log(`[look_at] Got ${messages.length} messages`) + + const responseText = extractLatestAssistantText(messages) + if (!responseText) { + log("[look_at] No assistant message found") + return "Error: No response from multimodal-looker agent" + } + + log(`[look_at] Got response, length: ${responseText.length}`) + return responseText +} diff --git a/src/tools/look-at/tools.ts b/src/tools/look-at/tools.ts index 1296afd29..d6fbb3b01 100644 --- a/src/tools/look-at/tools.ts +++ b/src/tools/look-at/tools.ts @@ -1,43 +1,11 @@ -import { basename } from "node:path" -import { pathToFileURL } from "node:url" import { tool, type PluginInput, type ToolDefinition } from "@opencode-ai/plugin" -import { LOOK_AT_DESCRIPTION, MULTIMODAL_LOOKER_AGENT } from "./constants" +import { LOOK_AT_DESCRIPTION } from "./constants" import type { LookAtArgs } from "./types" -import { log, promptSyncWithModelSuggestionRetry } from "../../shared" -import { extractLatestAssistantText } from "./assistant-message-extractor" +import { log } from "../../shared" import type { LookAtArgsWithAlias } from "./look-at-arguments" import { normalizeArgs, validateArgs } from "./look-at-arguments" -import { - extractBase64Data, - inferMimeTypeFromBase64, - inferMimeTypeFromFilePath, -} from "./mime-type-inference" -import { resolveMultimodalLookerAgentMetadata } from "./multimodal-agent-metadata" -import { - needsConversion, - convertImageToJpeg, - convertBase64ImageToJpeg, - cleanupConvertedImage, -} from "./image-converter" - -function getTemporaryConversionPath(error: unknown): string | null { - if (!(error instanceof Error)) { - return null - } - - const temporaryOutputPath = Reflect.get(error, "temporaryOutputPath") - if (typeof temporaryOutputPath === "string" && temporaryOutputPath.length > 0) { - return temporaryOutputPath - } - - const temporaryDirectory = Reflect.get(error, "temporaryDirectory") - if (typeof temporaryDirectory === "string" && temporaryDirectory.length > 0) { - return temporaryDirectory - } - - return null -} - +import { prepareLookAtInput } from "./look-at-input-preparer" +import { runLookAtSession } from "./look-at-session-runner" export { normalizeArgs, validateArgs } from "./look-at-arguments" @@ -57,188 +25,29 @@ export function createLookAt(ctx: PluginInput): ToolDefinition { return validationError } - const isBase64Input = Boolean(args.image_data) - const sourceDescription = isBase64Input ? "clipboard/pasted image" : args.file_path + const preparedInputResult = prepareLookAtInput(args) + if (!preparedInputResult.ok) { + return preparedInputResult.error + } + + const preparedInput = preparedInputResult.value + const { isBase64Input, sourceDescription } = preparedInput log(`[look_at] Analyzing ${sourceDescription}, goal: ${args.goal}`) - const imageData = args.image_data - const filePath = args.file_path - - let mimeType: string - let filePart: { type: "file"; mime: string; url: string; filename: string } - let tempFilePath: string | null = null - let tempConversionPath: string | null = null - let tempFilesToCleanup: string[] = [] - try { - if (imageData) { - mimeType = inferMimeTypeFromBase64(imageData) - - let finalBase64Data = extractBase64Data(imageData) - let finalMimeType = mimeType - - if (needsConversion(mimeType)) { - log(`[look_at] Detected unsupported Base64 format: ${mimeType}, converting to JPEG...`) - try { - const { base64, tempFiles } = convertBase64ImageToJpeg(finalBase64Data, mimeType) - finalBase64Data = base64 - finalMimeType = "image/jpeg" - tempFilesToCleanup = tempFiles - log(`[look_at] Base64 conversion successful`) - } catch (conversionError) { - log(`[look_at] Base64 conversion failed: ${conversionError}`) - return `Error: Failed to convert Base64 image format. ${conversionError}` - } - } - - filePart = { - type: "file", - mime: finalMimeType, - url: `data:${finalMimeType};base64,${finalBase64Data}`, - filename: `clipboard-image.${finalMimeType.split("/")[1] || "png"}`, - } - } else if (filePath) { - mimeType = inferMimeTypeFromFilePath(filePath) - - let actualFilePath = filePath - if (needsConversion(mimeType)) { - log(`[look_at] Detected unsupported format: ${mimeType}, converting to JPEG...`) - try { - tempFilePath = convertImageToJpeg(filePath, mimeType) - tempConversionPath = tempFilePath - actualFilePath = tempFilePath - mimeType = "image/jpeg" - log(`[look_at] Conversion successful: ${tempFilePath}`) - } catch (conversionError) { - const failedConversionPath = getTemporaryConversionPath(conversionError) - if (failedConversionPath) { - tempConversionPath = failedConversionPath - } - log(`[look_at] Conversion failed: ${conversionError}`) - return `Error: Failed to convert image format. ${conversionError}` - } - } - - filePart = { - type: "file", - mime: mimeType, - url: pathToFileURL(actualFilePath).href, - filename: basename(actualFilePath), - } - } else { - return "Error: Must provide either 'file_path' or 'image_data'." - } - - const readEnabled = false - const subjectNoun = isBase64Input ? "image" : "file" - const sourceClause = readEnabled - ? `Use the Read tool on the provided file path to load its contents, then analyze it.` - : `The ${subjectNoun} is already attached to this message. Analyze it directly from the attachment. Do NOT attempt to use the Read tool. The Read tool is disabled for this invocation and the ${subjectNoun} cannot be loaded by path.` - - const prompt = `Analyze the attached ${subjectNoun} and extract the requested information. - -${sourceClause} - -Goal: ${args.goal} - -Provide ONLY the extracted information that matches the goal. -Be thorough on what was requested, concise on everything else. -If the requested information is not found, clearly state what is missing.` - - const { agentModel, agentVariant } = await resolveMultimodalLookerAgentMetadata(ctx) - - log(`[look_at] Creating session with parent: ${toolContext.sessionID}`) - const parentSession = await ctx.client.session.get({ - path: { id: toolContext.sessionID }, - }).catch(() => null) - const parentDirectory = parentSession?.data?.directory ?? ctx.directory - - const createResult = await ctx.client.session.create({ - body: { - parentID: toolContext.sessionID, - title: `look_at: ${args.goal.substring(0, 50)}`, - }, - query: { directory: parentDirectory }, - }) - - if (createResult.error) { - log(`[look_at] Session create error:`, createResult.error) - const errorStr = String(createResult.error) - if (errorStr.toLowerCase().includes("unauthorized")) { - return `Error: Failed to create session (Unauthorized). This may be due to: -1. OAuth token restrictions (e.g., Claude Code credentials are restricted to Claude Code only) -2. Provider authentication issues -3. Session permission inheritance problems - -Try using a different provider or API key authentication. - -Original error: ${createResult.error}` - } - return `Error: Failed to create session: ${createResult.error}` - } - - const sessionID = createResult.data.id - log(`[look_at] Created session: ${sessionID}`) - - log(`[look_at] Sending prompt with ${isBase64Input ? "base64 image" : "file"} to session ${sessionID}`) - try { - await promptSyncWithModelSuggestionRetry(ctx.client, { - path: { id: sessionID }, - body: { - agent: MULTIMODAL_LOOKER_AGENT, - tools: { - task: false, - call_omo_agent: false, - look_at: false, - read: readEnabled, - }, - parts: [ - { type: "text", text: prompt }, - filePart, - ], - ...(agentModel ? { model: { providerID: agentModel.providerID, modelID: agentModel.modelID } } : {}), - ...(agentVariant ? { variant: agentVariant } : {}), - }, + return await runLookAtSession({ + ctx, + toolContext, + goal: args.goal, + filePart: preparedInput.filePart, + isBase64Input, }) - } catch (promptError) { - log(`[look_at] Prompt error (ignored, will still fetch messages):`, promptError) - } - - log(`[look_at] Fetching messages from session ${sessionID}...`) - - const messagesResult = await ctx.client.session.messages({ - path: { id: sessionID }, - }) - - if (messagesResult.error) { - log(`[look_at] Messages error:`, messagesResult.error) - return `Error: Failed to get messages: ${messagesResult.error}` - } - - const messages = messagesResult.data - log(`[look_at] Got ${messages.length} messages`) - - const responseText = extractLatestAssistantText(messages) - if (!responseText) { - log("[look_at] No assistant message found") - return "Error: No response from multimodal-looker agent" - } - - log(`[look_at] Got response, length: ${responseText.length}`) - return responseText } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error) log(`[look_at] Unexpected error analyzing ${sourceDescription}:`, error) return `Error: Failed to analyze ${sourceDescription}: ${errorMessage}` } finally { - if (tempConversionPath) { - cleanupConvertedImage(tempConversionPath) - } else if (tempFilePath) { - cleanupConvertedImage(tempFilePath) - } - tempFilesToCleanup.forEach(file => { - cleanupConvertedImage(file) - }) + preparedInput.cleanup() } }, }) From 1aebf39d23d6131a3389b7c5e57c87098f0ddcf7 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 01:50:41 +0900 Subject: [PATCH 05/15] refactor(hooks): split preemptive-compaction.ts to comply with 200 LOC module rule Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/hooks/preemptive-compaction-trigger.ts | 131 ++++++++++++++++++ src/hooks/preemptive-compaction-types.ts | 41 ++++++ src/hooks/preemptive-compaction.ts | 149 +++------------------ 3 files changed, 189 insertions(+), 132 deletions(-) create mode 100644 src/hooks/preemptive-compaction-trigger.ts create mode 100644 src/hooks/preemptive-compaction-types.ts diff --git a/src/hooks/preemptive-compaction-trigger.ts b/src/hooks/preemptive-compaction-trigger.ts new file mode 100644 index 000000000..bbab74f76 --- /dev/null +++ b/src/hooks/preemptive-compaction-trigger.ts @@ -0,0 +1,131 @@ +import type { OhMyOpenCodeConfig } from "../config" +import { + resolveActualContextLimit, + type ContextLimitModelCacheState, +} from "../shared/context-limit-resolver" +import { log } from "../shared/logger" + +import { resolveCompactionModel } from "./shared/compaction-model-resolver" +import type { + CachedCompactionState, + PreemptiveCompactionContext, +} from "./preemptive-compaction-types" + +const PREEMPTIVE_COMPACTION_TIMEOUT_MS = 60_000 +const PREEMPTIVE_COMPACTION_THRESHOLD = 0.78 +const PREEMPTIVE_COMPACTION_COOLDOWN_MS = 60_000 + +declare function setTimeout(handler: () => void, timeout?: number): unknown +declare function clearTimeout(timeoutID: unknown): void + +async function withTimeout( + promise: Promise, + timeoutMs: number, + errorMessage: string, +): Promise { + let timeoutID: unknown + + const timeoutPromise = new Promise((_, reject) => { + timeoutID = setTimeout(() => { + reject(new Error(errorMessage)) + }, timeoutMs) + }) + + return await Promise.race([promise, timeoutPromise]).finally(() => { + clearTimeout(timeoutID) + }) +} + +export async function runPreemptiveCompactionIfNeeded(args: { + ctx: PreemptiveCompactionContext + pluginConfig: OhMyOpenCodeConfig + modelCacheState?: ContextLimitModelCacheState + sessionID: string + tokenCache: Map + compactionInProgress: Set + compactedSessions: Set + lastCompactionTime: Map +}): Promise { + const { + ctx, + pluginConfig, + modelCacheState, + sessionID, + tokenCache, + compactionInProgress, + compactedSessions, + lastCompactionTime, + } = args + + if (compactedSessions.has(sessionID) || compactionInProgress.has(sessionID)) return + + const lastTime = lastCompactionTime.get(sessionID) + if (lastTime && Date.now() - lastTime < PREEMPTIVE_COMPACTION_COOLDOWN_MS) return + + const cached = tokenCache.get(sessionID) + if (!cached) return + + const actualLimit = resolveActualContextLimit( + cached.providerID, + cached.modelID, + modelCacheState, + ) + + if (actualLimit === null) { + log("[preemptive-compaction] Skipping preemptive compaction: unknown context limit for model", { + providerID: cached.providerID, + modelID: cached.modelID, + }) + return + } + + const totalInputTokens = (cached.tokens.input ?? 0) + (cached.tokens.cache?.read ?? 0) + const usageRatio = totalInputTokens / actualLimit + if (usageRatio < PREEMPTIVE_COMPACTION_THRESHOLD || !cached.modelID) return + + compactionInProgress.add(sessionID) + lastCompactionTime.set(sessionID, Date.now()) + + try { + const { providerID: targetProviderID, modelID: targetModelID } = resolveCompactionModel( + pluginConfig, + sessionID, + cached.providerID, + cached.modelID, + ) + + await withTimeout( + ctx.client.session.summarize({ + path: { id: sessionID }, + body: { providerID: targetProviderID, modelID: targetModelID, auto: true }, + query: { directory: ctx.directory }, + }), + PREEMPTIVE_COMPACTION_TIMEOUT_MS, + `Compaction summarize timed out after ${PREEMPTIVE_COMPACTION_TIMEOUT_MS}ms`, + ) + + compactedSessions.add(sessionID) + } catch (error) { + log("[preemptive-compaction] Compaction failed", { + sessionID, + providerID: cached.providerID, + modelID: cached.modelID, + error: String(error), + }) + ctx.client.tui.showToast({ + body: { + title: "Preemptive compaction failed", + message: `Context window is above ${Math.round(PREEMPTIVE_COMPACTION_THRESHOLD * 100)}% and auto-compaction could not run. The session may grow large. Error: ${String(error)}`, + variant: "warning", + duration: 10000, + }, + }).catch((toastError: unknown) => { + log("[preemptive-compaction] Failed to show toast", { + sessionID, + toastError: String(toastError), + }) + }) + } finally { + compactionInProgress.delete(sessionID) + } +} diff --git a/src/hooks/preemptive-compaction-types.ts b/src/hooks/preemptive-compaction-types.ts new file mode 100644 index 000000000..77efed575 --- /dev/null +++ b/src/hooks/preemptive-compaction-types.ts @@ -0,0 +1,41 @@ +export interface TokenInfo { + input: number + output: number + reasoning: number + cache: { read: number; write: number } +} + +export interface CachedCompactionState { + providerID: string + modelID: string + tokens: TokenInfo +} + +export interface PreemptiveCompactionClient { + session: { + messages: (input: { + path: { id: string } + query?: { directory: string } + }) => Promise + summarize: (input: { + path: { id: string } + body: { providerID: string; modelID: string; auto?: boolean } + query: { directory: string } + }) => Promise + } + tui: { + showToast: (input: { + body: { + title: string + message: string + variant: "warning" + duration: number + } + }) => Promise + } +} + +export interface PreemptiveCompactionContext { + client: PreemptiveCompactionClient + directory: string +} diff --git a/src/hooks/preemptive-compaction.ts b/src/hooks/preemptive-compaction.ts index ecab70676..7b4828dcb 100644 --- a/src/hooks/preemptive-compaction.ts +++ b/src/hooks/preemptive-compaction.ts @@ -1,69 +1,16 @@ -import { log } from "../shared/logger" import type { OhMyOpenCodeConfig } from "../config" -import { - resolveActualContextLimit, - type ContextLimitModelCacheState, -} from "../shared/context-limit-resolver" +import type { ContextLimitModelCacheState } from "../shared/context-limit-resolver" -import { resolveCompactionModel } from "./shared/compaction-model-resolver" import { createPostCompactionDegradationMonitor } from "./preemptive-compaction-degradation-monitor" - -const PREEMPTIVE_COMPACTION_TIMEOUT_MS = 60_000 -const PREEMPTIVE_COMPACTION_THRESHOLD = 0.78 -const PREEMPTIVE_COMPACTION_COOLDOWN_MS = 60_000 - -declare function setTimeout(handler: () => void, timeout?: number): unknown -declare function clearTimeout(timeoutID: unknown): void - -interface TokenInfo { - input: number - output: number - reasoning: number - cache: { read: number; write: number } -} - -interface CachedCompactionState { - providerID: string - modelID: string - tokens: TokenInfo -} - -async function withTimeout( - promise: Promise, - timeoutMs: number, - errorMessage: string, -): Promise { - let timeoutID: unknown - - const timeoutPromise = new Promise((_, reject) => { - timeoutID = setTimeout(() => { - reject(new Error(errorMessage)) - }, timeoutMs) - }) - - return await Promise.race([promise, timeoutPromise]).finally(() => { - clearTimeout(timeoutID) - }) -} - -type PluginInput = { - client: { - session: { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - messages: (...args: any[]) => any - // eslint-disable-next-line @typescript-eslint/no-explicit-any - summarize: (...args: any[]) => any - } - tui: { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - showToast: (...args: any[]) => any - } - } - directory: string -} +import { runPreemptiveCompactionIfNeeded } from "./preemptive-compaction-trigger" +import type { + CachedCompactionState, + PreemptiveCompactionContext, + TokenInfo, +} from "./preemptive-compaction-types" export function createPreemptiveCompactionHook( - ctx: PluginInput, + ctx: PreemptiveCompactionContext, pluginConfig: OhMyOpenCodeConfig, modelCacheState?: ContextLimitModelCacheState, ) { @@ -84,78 +31,16 @@ export function createPreemptiveCompactionHook( input: { tool: string; sessionID: string; callID: string }, _output: { title: string; output: string; metadata: unknown } ) => { - const { sessionID } = input - if (compactedSessions.has(sessionID) || compactionInProgress.has(sessionID)) return - - const lastTime = lastCompactionTime.get(sessionID) - if (lastTime && Date.now() - lastTime < PREEMPTIVE_COMPACTION_COOLDOWN_MS) return - - const cached = tokenCache.get(sessionID) - if (!cached) return - - const actualLimit = resolveActualContextLimit( - cached.providerID, - cached.modelID, + await runPreemptiveCompactionIfNeeded({ + ctx, + pluginConfig, modelCacheState, - ) - - if (actualLimit === null) { - log("[preemptive-compaction] Skipping preemptive compaction: unknown context limit for model", { - providerID: cached.providerID, - modelID: cached.modelID, - }) - return - } - - const totalInputTokens = (cached.tokens.input ?? 0) + (cached.tokens.cache?.read ?? 0) - const usageRatio = totalInputTokens / actualLimit - if (usageRatio < PREEMPTIVE_COMPACTION_THRESHOLD || !cached.modelID) return - - compactionInProgress.add(sessionID) - lastCompactionTime.set(sessionID, Date.now()) - - try { - const { providerID: targetProviderID, modelID: targetModelID } = resolveCompactionModel( - pluginConfig, - sessionID, - cached.providerID, - cached.modelID, - ) - - await withTimeout( - ctx.client.session.summarize({ - path: { id: sessionID }, - body: { providerID: targetProviderID, modelID: targetModelID, auto: true } as never, - query: { directory: ctx.directory }, - }), - PREEMPTIVE_COMPACTION_TIMEOUT_MS, - `Compaction summarize timed out after ${PREEMPTIVE_COMPACTION_TIMEOUT_MS}ms`, - ) - - compactedSessions.add(sessionID) - } catch (error) { - log("[preemptive-compaction] Compaction failed", { - sessionID, - providerID: cached.providerID, - modelID: cached.modelID, - error: String(error), - }) - ctx.client.tui.showToast({ - body: { - title: "Preemptive compaction failed", - message: `Context window is above ${Math.round(PREEMPTIVE_COMPACTION_THRESHOLD * 100)}% and auto-compaction could not run. The session may grow large. Error: ${String(error)}`, - variant: "warning", - duration: 10000, - }, - }).catch((toastError: unknown) => { - log("[preemptive-compaction] Failed to show toast", { - sessionID, - toastError: String(toastError), - }) - }) - } finally { - compactionInProgress.delete(sessionID) - } + sessionID: input.sessionID, + tokenCache, + compactionInProgress, + compactedSessions, + lastCompactionTime, + }) } const eventHandler = async ({ event }: { event: { type: string; properties?: unknown } }) => { From db056346d2a6905b43b68d671ea5238048e11d60 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 01:51:26 +0900 Subject: [PATCH 06/15] refactor(shared): move parseModelString out of delegate-task to break cross-tool coupling Move parseModelString into src/shared so callers can depend on a neutral module instead of reaching into delegate-task internals. Cross-tool coupling violates module boundaries, and this keeps call-omo-agent plus runtime-fallback from importing through a sibling tool. --- src/hooks/runtime-fallback/retry-model-payload.ts | 2 +- src/shared/index.ts | 1 + src/{tools/delegate-task => shared}/model-string-parser.ts | 0 src/tools/call-omo-agent/tools.ts | 4 ++-- src/tools/delegate-task/category-resolver.ts | 2 +- src/tools/delegate-task/model-selection.ts | 2 +- 6 files changed, 6 insertions(+), 5 deletions(-) rename src/{tools/delegate-task => shared}/model-string-parser.ts (100%) diff --git a/src/hooks/runtime-fallback/retry-model-payload.ts b/src/hooks/runtime-fallback/retry-model-payload.ts index 0c9ed0c9a..d5f59b74c 100644 --- a/src/hooks/runtime-fallback/retry-model-payload.ts +++ b/src/hooks/runtime-fallback/retry-model-payload.ts @@ -1,4 +1,4 @@ -import { parseModelString } from "../../tools/delegate-task/model-string-parser" +import { parseModelString } from "../../shared/model-string-parser" export function buildRetryModelPayload( model: string, diff --git a/src/shared/index.ts b/src/shared/index.ts index 80ffa751b..140f88192 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -78,3 +78,4 @@ export * from "./plugin-identity" export * from "./log-legacy-plugin-startup-warning" export * from "./task-system-enabled" export * from "./parse-tools-config" +export { parseModelString } from "./model-string-parser" diff --git a/src/tools/delegate-task/model-string-parser.ts b/src/shared/model-string-parser.ts similarity index 100% rename from src/tools/delegate-task/model-string-parser.ts rename to src/shared/model-string-parser.ts diff --git a/src/tools/call-omo-agent/tools.ts b/src/tools/call-omo-agent/tools.ts index 802ac0ba0..839f5abe8 100644 --- a/src/tools/call-omo-agent/tools.ts +++ b/src/tools/call-omo-agent/tools.ts @@ -1,6 +1,6 @@ import { tool, type PluginInput, type ToolDefinition } from "@opencode-ai/plugin" import { ALLOWED_AGENTS, CALL_OMO_AGENT_DESCRIPTION } from "./constants" -import type { AllowedAgentType, CallOmoAgentArgs, ToolContextWithMetadata } from "./types" +import type { CallOmoAgentArgs, ToolContextWithMetadata } from "./types" import type { BackgroundManager } from "../../features/background-agent" import type { CategoriesConfig, AgentOverrides } from "../../config/schema" import type { DelegatedModelConfig } from "../../shared/model-resolution-types" @@ -11,7 +11,7 @@ import { normalizeFallbackModels } from "../../shared/model-resolver" import { buildFallbackChainFromModels } from "../../shared/fallback-chain-from-models" import { log } from "../../shared" import { CONFIG_BASENAME } from "../../shared/plugin-identity" -import { parseModelString } from "../delegate-task/model-string-parser" +import { parseModelString } from "../../shared" import { executeBackground } from "./background-executor" import { executeSync } from "./sync-executor" import { resolveCallableAgents } from "./agent-resolver" diff --git a/src/tools/delegate-task/category-resolver.ts b/src/tools/delegate-task/category-resolver.ts index bfc4d1896..25f4e8a37 100644 --- a/src/tools/delegate-task/category-resolver.ts +++ b/src/tools/delegate-task/category-resolver.ts @@ -5,7 +5,7 @@ import type { FallbackEntry } from "../../shared/model-requirements" import { mergeCategories } from "../../shared/merge-categories" import { SISYPHUS_JUNIOR_AGENT } from "./sisyphus-junior-agent" import { resolveCategoryConfig } from "./categories" -import { parseModelString } from "./model-string-parser" +import { parseModelString } from "../../shared/model-string-parser" import { CATEGORY_MODEL_REQUIREMENTS } from "../../shared/model-requirements" import { normalizeFallbackModels, flattenToFallbackModelStrings } from "../../shared/model-resolver" import { buildFallbackChainFromModels, findMostSpecificFallbackEntry } from "../../shared/fallback-chain-from-models" diff --git a/src/tools/delegate-task/model-selection.ts b/src/tools/delegate-task/model-selection.ts index cef7df752..43fa4741b 100644 --- a/src/tools/delegate-task/model-selection.ts +++ b/src/tools/delegate-task/model-selection.ts @@ -4,7 +4,7 @@ import { fuzzyMatchModel } from "../../shared/model-availability" import { transformModelForProvider } from "../../shared/provider-model-id-transform" import { hasConnectedProvidersCache, hasProviderModelsCache, readConnectedProvidersCache } from "../../shared/connected-providers-cache" import { log } from "../../shared/logger" -import { parseModelString, parseVariantFromModelID } from "./model-string-parser" +import { parseModelString, parseVariantFromModelID } from "../../shared/model-string-parser" function isExplicitHighModel(model: string): boolean { return /(?:^|\/)[^/]+-high$/.test(model) From 0f1b16567af737067466577005cdb858f76a5de4 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 01:52:12 +0900 Subject: [PATCH 07/15] refactor(delegate-task): split tools.ts to comply with 200 LOC module rule Extract the tool description/category metadata into tool-description.ts and move argument normalization plus validation into tool-argument-preparation.ts. This keeps createDelegateTask focused on orchestration while preserving behavior and bringing tools.ts under the module LOC rule. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../tool-argument-preparation.ts | 80 ++++++++ src/tools/delegate-task/tool-description.ts | 88 +++++++++ src/tools/delegate-task/tools.ts | 175 ++++-------------- 3 files changed, 206 insertions(+), 137 deletions(-) create mode 100644 src/tools/delegate-task/tool-argument-preparation.ts create mode 100644 src/tools/delegate-task/tool-description.ts diff --git a/src/tools/delegate-task/tool-argument-preparation.ts b/src/tools/delegate-task/tool-argument-preparation.ts new file mode 100644 index 000000000..d54b12ca6 --- /dev/null +++ b/src/tools/delegate-task/tool-argument-preparation.ts @@ -0,0 +1,80 @@ +import type { DelegateTaskArgs, ToolContextWithMetadata } from "./types" +import { SISYPHUS_JUNIOR_AGENT } from "./sisyphus-junior-agent" +import { log } from "../../shared/logger" + +export async function prepareDelegateTaskArgs(args: Record, ctx: ToolContextWithMetadata): Promise { + const category = typeof args.category === "string" ? args.category : undefined + const prompt = typeof args.prompt === "string" ? args.prompt : "" + const originalSubagentType = typeof args.subagent_type === "string" ? args.subagent_type : undefined + let subagentType = originalSubagentType + + if (category) { + if (subagentType && subagentType !== SISYPHUS_JUNIOR_AGENT) { + log("[task] category provided - overriding subagent_type to sisyphus-junior", { + category, + subagent_type: subagentType, + }) + } + subagentType = SISYPHUS_JUNIOR_AGENT + } + + let description = typeof args.description === "string" ? args.description : undefined + if (!description || description.trim() === "") { + const words = prompt.trim().split(/\s+/) + description = words.slice(0, 4).join(" ") || "Delegated task" + } + + await ctx.metadata?.({ + title: description, + }) + + const runInBackground = args.run_in_background + if (runInBackground === undefined) { + throw new Error("Invalid arguments: 'run_in_background' parameter is REQUIRED. Specify run_in_background=false for task delegation, or run_in_background=true for parallel exploration.") + } + + let loadSkills = args.load_skills + if (typeof loadSkills === "string") { + try { + const parsed = JSON.parse(loadSkills) + loadSkills = Array.isArray(parsed) ? parsed : [] + } catch { + loadSkills = [] + } + } + + if (loadSkills === undefined) { + throw new Error("Invalid arguments: 'load_skills' parameter is REQUIRED. Pass [] if no skills needed.") + } + + if (loadSkills === null) { + throw new Error("Invalid arguments: load_skills=null is not allowed. Pass [] if no skills needed.") + } + + const normalizedLoadSkills = Array.isArray(loadSkills) + ? loadSkills.filter((value): value is string => typeof value === "string") + : [] + + const taskID = typeof args.task_id === "string" ? args.task_id : undefined + const command = typeof args.command === "string" ? args.command : undefined + + args.category = category + args.subagent_type = subagentType + args.description = description + args.prompt = prompt + args.run_in_background = runInBackground + args.task_id = taskID + args.command = command + args.load_skills = normalizedLoadSkills + + return { + category, + subagent_type: subagentType, + description, + prompt, + run_in_background: runInBackground === true, + task_id: taskID, + command, + load_skills: normalizedLoadSkills, + } +} diff --git a/src/tools/delegate-task/tool-description.ts b/src/tools/delegate-task/tool-description.ts new file mode 100644 index 000000000..48bebc58d --- /dev/null +++ b/src/tools/delegate-task/tool-description.ts @@ -0,0 +1,88 @@ +import type { AvailableCategory, AvailableSkill } from "../../agents/dynamic-agent-prompt-builder" +import { mergeCategories } from "../../shared/merge-categories" +import { CATEGORY_DESCRIPTIONS } from "./constants" +import type { DelegateTaskToolOptions } from "./types" + +export interface DelegateTaskPresentation { + availableCategories: AvailableCategory[] + availableSkills: AvailableSkill[] + categoryExamples: string + description: string +} + +export function createDelegateTaskPresentation(options: DelegateTaskToolOptions): DelegateTaskPresentation { + const { userCategories } = options + const allCategories = mergeCategories(userCategories) + const categoryNames = Object.keys(allCategories) + const categoryExamples = categoryNames.join(", ") + + const availableCategories: AvailableCategory[] = options.availableCategories + ?? Object.entries(allCategories).map(([name, categoryConfig]) => { + const userDescription = userCategories?.[name]?.description + const builtinDescription = CATEGORY_DESCRIPTIONS[name] + const description = userDescription || builtinDescription || "General tasks" + + return { + name, + description, + model: categoryConfig.model, + } + }) + + const availableSkills: AvailableSkill[] = options.availableSkills ?? [] + + const categoryList = categoryNames.map(name => { + const userDescription = userCategories?.[name]?.description + const builtinDescription = CATEGORY_DESCRIPTIONS[name] + const description = userDescription || builtinDescription + return description ? ` - ${name}: ${description}` : ` - ${name}` + }).join("\n") + + const description = `Spawn agent task with category-based or direct agent selection. + + ⚠️ CRITICAL: You MUST provide EITHER category OR subagent_type. Omitting BOTH will FAIL. + + **COMMON MISTAKE (DO NOT DO THIS):** + \`\`\` + task(description="...", prompt="...", run_in_background=false) // ❌ FAILS - missing category AND subagent_type + \`\`\` + + **CORRECT - Using category:** + \`\`\` + task(category="quick", load_skills=[], description="Fix type error", prompt="...", run_in_background=false) + \`\`\` + + **CORRECT - Using subagent_type:** + \`\`\` + task(subagent_type="explore", load_skills=[], description="Find patterns", prompt="...", run_in_background=true) + \`\`\` + + REQUIRED: Provide ONE of: + - category: For task delegation (uses Sisyphus-Junior with category-optimized model) + - subagent_type: For direct agent invocation (explore, librarian, oracle, etc.) + + **DO NOT provide both.** If category is provided, subagent_type is ignored. + + - load_skills: ALWAYS REQUIRED. Pass [] if no skills needed, or ["skill-1", "skill-2"] for category tasks. + - category: Use predefined category → Spawns Sisyphus-Junior with category config + Available categories: + ${categoryList} + - subagent_type: Use specific agent directly (explore, librarian, oracle, metis, momus) + - run_in_background: REQUIRED. true=async (returns task_id), false=sync (waits). Use background=true ONLY for parallel exploration with 5+ independent queries. + - task_id: Existing task to continue (from previous task output). Continues the same subagent session with FULL CONTEXT PRESERVED. + - command: The command that triggered this task (optional, for slash command tracking). + + **WHEN TO USE task_id:** + - Task failed/incomplete → task_id with "fix: [specific issue]" + - Need follow-up on previous result → task_id with additional question + - Multi-turn conversation with same agent → always task_id instead of new task + + Prompts MUST be in English.` + + return { + availableCategories, + availableSkills, + categoryExamples, + description, + } +} diff --git a/src/tools/delegate-task/tools.ts b/src/tools/delegate-task/tools.ts index 397048aa1..b0820f73b 100644 --- a/src/tools/delegate-task/tools.ts +++ b/src/tools/delegate-task/tools.ts @@ -1,14 +1,7 @@ import { tool, type ToolDefinition } from "@opencode-ai/plugin" -import type { DelegateTaskArgs, DelegatedModelConfig, ToolContextWithMetadata, DelegateTaskToolOptions } from "./types" -import { CATEGORY_DESCRIPTIONS } from "./constants" -import { SISYPHUS_JUNIOR_AGENT } from "./sisyphus-junior-agent" -import { mergeCategories } from "../../shared/merge-categories" +import type { DelegatedModelConfig, ToolContextWithMetadata, DelegateTaskToolOptions } from "./types" import { log } from "../../shared/logger" import { buildSystemContent } from "./prompt-builder" -import type { - AvailableCategory, - AvailableSkill, -} from "../../agents/dynamic-agent-prompt-builder" import { resolveSkillContent, resolveParentContext, @@ -20,133 +13,37 @@ import { executeBackgroundTask, executeSyncTask, } from "./executor" +import { prepareDelegateTaskArgs } from "./tool-argument-preparation" +import { createDelegateTaskPresentation } from "./tool-description" export { resolveCategoryConfig } from "./categories" export type { SyncSessionCreatedEvent, DelegateTaskToolOptions, BuildSystemContentInput } from "./types" export { buildSystemContent, buildTaskPrompt } from "./prompt-builder" +const delegateTaskArgsSchema = { + load_skills: tool.schema.array(tool.schema.string()).describe("Skill names to inject. REQUIRED - pass [] if no skills needed."), + description: tool.schema.string().optional().describe("Short task description (3-5 words). Auto-generated from prompt if omitted."), + prompt: tool.schema.string().describe("Full detailed prompt for the agent"), + run_in_background: tool.schema.boolean().describe("REQUIRED. true=async (returns task_id), false=sync (waits). Use false for task delegation, true ONLY for parallel exploration."), + category: tool.schema.string().optional().describe("REQUIRED if subagent_type not provided. Do NOT provide both category and subagent_type."), + subagent_type: tool.schema.string().optional().describe("REQUIRED if category not provided. Do NOT provide both category and subagent_type."), + task_id: tool.schema.string().optional().describe("Existing task to continue. Canonical resume identifier."), + command: tool.schema.string().optional().describe("The command that triggered this task"), +} + export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefinition { - const { userCategories } = options - - const allCategories = mergeCategories(userCategories) - const categoryNames = Object.keys(allCategories) - const categoryExamples = categoryNames.join(", ") - - const availableCategories: AvailableCategory[] = options.availableCategories - ?? Object.entries(allCategories).map(([name, categoryConfig]) => { - const userDesc = userCategories?.[name]?.description - const builtinDesc = CATEGORY_DESCRIPTIONS[name] - const description = userDesc || builtinDesc || "General tasks" - return { - name, - description, - model: categoryConfig.model, - } - }) - - const availableSkills: AvailableSkill[] = options.availableSkills ?? [] - - const categoryList = categoryNames.map(name => { - const userDesc = userCategories?.[name]?.description - const builtinDesc = CATEGORY_DESCRIPTIONS[name] - const desc = userDesc || builtinDesc - return desc ? ` - ${name}: ${desc}` : ` - ${name}` - }).join("\n") - - const description = `Spawn agent task with category-based or direct agent selection. - - ⚠️ CRITICAL: You MUST provide EITHER category OR subagent_type. Omitting BOTH will FAIL. - - **COMMON MISTAKE (DO NOT DO THIS):** - \`\`\` - task(description="...", prompt="...", run_in_background=false) // ❌ FAILS - missing category AND subagent_type - \`\`\` - - **CORRECT - Using category:** - \`\`\` - task(category="quick", load_skills=[], description="Fix type error", prompt="...", run_in_background=false) - \`\`\` - - **CORRECT - Using subagent_type:** - \`\`\` - task(subagent_type="explore", load_skills=[], description="Find patterns", prompt="...", run_in_background=true) - \`\`\` - - REQUIRED: Provide ONE of: - - category: For task delegation (uses Sisyphus-Junior with category-optimized model) - - subagent_type: For direct agent invocation (explore, librarian, oracle, etc.) - - **DO NOT provide both.** If category is provided, subagent_type is ignored. - - - load_skills: ALWAYS REQUIRED. Pass [] if no skills needed, or ["skill-1", "skill-2"] for category tasks. - - category: Use predefined category → Spawns Sisyphus-Junior with category config - Available categories: - ${categoryList} - - subagent_type: Use specific agent directly (explore, librarian, oracle, metis, momus) - - run_in_background: REQUIRED. true=async (returns task_id), false=sync (waits). Use background=true ONLY for parallel exploration with 5+ independent queries. - - task_id: Existing task to continue (from previous task output). Continues the same subagent session with FULL CONTEXT PRESERVED. - - command: The command that triggered this task (optional, for slash command tracking). - - **WHEN TO USE task_id:** - - Task failed/incomplete → task_id with "fix: [specific issue]" - - Need follow-up on previous result → task_id with additional question - - Multi-turn conversation with same agent → always task_id instead of new task - - Prompts MUST be in English.` + const { availableCategories, availableSkills, categoryExamples, description } = createDelegateTaskPresentation(options) return tool({ description, - args: { - load_skills: tool.schema.array(tool.schema.string()).describe("Skill names to inject. REQUIRED - pass [] if no skills needed."), - description: tool.schema.string().optional().describe("Short task description (3-5 words). Auto-generated from prompt if omitted."), - prompt: tool.schema.string().describe("Full detailed prompt for the agent"), - run_in_background: tool.schema.boolean().describe("REQUIRED. true=async (returns task_id), false=sync (waits). Use false for task delegation, true ONLY for parallel exploration."), - category: tool.schema.string().optional().describe(`REQUIRED if subagent_type not provided. Do NOT provide both category and subagent_type.`), - subagent_type: tool.schema.string().optional().describe("REQUIRED if category not provided. Do NOT provide both category and subagent_type."), - task_id: tool.schema.string().optional().describe("Existing task to continue. Canonical resume identifier."), - command: tool.schema.string().optional().describe("The command that triggered this task"), - }, - async execute(args: DelegateTaskArgs, toolContext) { + args: delegateTaskArgsSchema, + async execute(args, toolContext) { const ctx = toolContext as ToolContextWithMetadata + const delegateTaskArgs = await prepareDelegateTaskArgs(args, ctx) - if (args.category) { - if (args.subagent_type && args.subagent_type !== SISYPHUS_JUNIOR_AGENT) { - log("[task] category provided - overriding subagent_type to sisyphus-junior", { - category: args.category, - subagent_type: args.subagent_type, - }) - } - args.subagent_type = SISYPHUS_JUNIOR_AGENT - } - // Auto-generate description from prompt when missing or empty - if (!args.description || typeof args.description !== "string" || args.description.trim() === "") { - const words = (args.prompt || "").trim().split(/\s+/) - args.description = words.slice(0, 4).join(" ") || "Delegated task" - } - await ctx.metadata?.({ - title: args.description, - }) - if (args.run_in_background === undefined) { - throw new Error(`Invalid arguments: 'run_in_background' parameter is REQUIRED. Specify run_in_background=false for task delegation, or run_in_background=true for parallel exploration.`) - } - if (typeof args.load_skills === "string") { - try { - const parsed = JSON.parse(args.load_skills) - args.load_skills = Array.isArray(parsed) ? parsed : [] - } catch { - args.load_skills = [] - } - } - if (args.load_skills === undefined) { - throw new Error(`Invalid arguments: 'load_skills' parameter is REQUIRED. Pass [] if no skills needed.`) - } - if (args.load_skills === null) { - throw new Error(`Invalid arguments: load_skills=null is not allowed. Pass [] if no skills needed.`) - } + const runInBackground = delegateTaskArgs.run_in_background === true - const runInBackground = args.run_in_background === true - - const { content: skillContent, contents: skillContents, error: skillError } = await resolveSkillContent(args.load_skills, { + const { content: skillContent, contents: skillContents, error: skillError } = await resolveSkillContent(delegateTaskArgs.load_skills, { gitMasterConfig: options.gitMasterConfig, browserProvider: options.browserProvider, disabledSkills: options.disabledSkills, @@ -158,14 +55,14 @@ export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefini const parentContext = await resolveParentContext(ctx, options.client) - if (args.task_id) { + if (delegateTaskArgs.task_id) { if (runInBackground) { - return executeBackgroundContinuation(args, ctx, options, parentContext) + return executeBackgroundContinuation(delegateTaskArgs, ctx, options, parentContext) } - return executeSyncContinuation(args, ctx, options, parentContext) + return executeSyncContinuation(delegateTaskArgs, ctx, options, parentContext) } - if (!args.category && !args.subagent_type) { + if (!delegateTaskArgs.category && !delegateTaskArgs.subagent_type) { return `Invalid arguments: Must provide either category or subagent_type.` } @@ -190,8 +87,8 @@ export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefini let fallbackChain: import("../../shared/model-requirements").FallbackEntry[] | undefined let maxPromptTokens: number | undefined - if (args.category) { - const resolution = await resolveCategoryExecution(args, options, inheritedModel, systemDefaultModel) + if (delegateTaskArgs.category) { + const resolution = await resolveCategoryExecution(delegateTaskArgs, options, inheritedModel, systemDefaultModel) if (resolution.error) { return resolution.error } @@ -204,14 +101,14 @@ export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefini fallbackChain = resolution.fallbackChain maxPromptTokens = resolution.maxPromptTokens - const isRunInBackgroundExplicitlyFalse = args.run_in_background === false || args.run_in_background === "false" as unknown as boolean + const isRunInBackgroundExplicitlyFalse = isExplicitSyncRun(delegateTaskArgs.run_in_background) log("[task] unstable agent detection", { - category: args.category, + category: delegateTaskArgs.category, actualModel, isUnstableAgent, - run_in_background_value: args.run_in_background, - run_in_background_type: typeof args.run_in_background, + run_in_background_value: delegateTaskArgs.run_in_background, + run_in_background_type: typeof delegateTaskArgs.run_in_background, isRunInBackgroundExplicitlyFalse, willForceBackground: isUnstableAgent && isRunInBackgroundExplicitlyFalse, }) @@ -227,10 +124,10 @@ export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefini availableCategories, availableSkills, }) - return executeUnstableAgentTask(args, ctx, options, parentContext, agentToUse, categoryModel, systemContent, actualModel) + return executeUnstableAgentTask(delegateTaskArgs, ctx, options, parentContext, agentToUse, categoryModel, systemContent, actualModel) } } else { - const resolution = await resolveSubagentExecution(args, options, parentContext.agent, categoryExamples) + const resolution = await resolveSubagentExecution(delegateTaskArgs, options, parentContext.agent, categoryExamples) if (resolution.error) { return resolution.error } @@ -251,10 +148,14 @@ export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefini }) if (runInBackground) { - return executeBackgroundTask(args, ctx, options, parentContext, agentToUse, categoryModel, systemContent, fallbackChain) + return executeBackgroundTask(delegateTaskArgs, ctx, options, parentContext, agentToUse, categoryModel, systemContent, fallbackChain) } - return executeSyncTask(args, ctx, options, parentContext, agentToUse, categoryModel, systemContent, modelInfo, fallbackChain) + return executeSyncTask(delegateTaskArgs, ctx, options, parentContext, agentToUse, categoryModel, systemContent, modelInfo, fallbackChain) }, }) } + +function isExplicitSyncRun(runInBackground: unknown): boolean { + return runInBackground === false || runInBackground === "false" +} From 0d10498a1188537820498ee0321ceb2d0720e83b Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 01:52:26 +0900 Subject: [PATCH 08/15] refactor(hooks): split session-notification.ts to comply with 200 LOC module rule Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../session-notification-event-properties.ts | 51 +++++++++++++++++++ src/hooks/session-notification.ts | 42 ++------------- 2 files changed, 56 insertions(+), 37 deletions(-) create mode 100644 src/hooks/session-notification-event-properties.ts diff --git a/src/hooks/session-notification-event-properties.ts b/src/hooks/session-notification-event-properties.ts new file mode 100644 index 000000000..b51edf81b --- /dev/null +++ b/src/hooks/session-notification-event-properties.ts @@ -0,0 +1,51 @@ +type EventProperties = Record | undefined + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null +} + +function getEventInfo(properties: EventProperties): Record | undefined { + const info = properties?.info + return isRecord(info) ? info : undefined +} + +export function getSessionID(properties: EventProperties): string | undefined { + const sessionID = properties?.sessionID + if (typeof sessionID === "string" && sessionID.length > 0) return sessionID + + const sessionId = properties?.sessionId + if (typeof sessionId === "string" && sessionId.length > 0) return sessionId + + const info = getEventInfo(properties) + const infoSessionID = info?.sessionID + if (typeof infoSessionID === "string" && infoSessionID.length > 0) return infoSessionID + + const infoSessionId = info?.sessionId + if (typeof infoSessionId === "string" && infoSessionId.length > 0) return infoSessionId + + return undefined +} + +export function getEventToolName(properties: EventProperties): string | undefined { + const tool = properties?.tool + if (typeof tool === "string" && tool.length > 0) return tool + + const name = properties?.name + if (typeof name === "string" && name.length > 0) return name + + return undefined +} + +export function getQuestionText(properties: EventProperties): string { + const args = properties?.args + if (!isRecord(args)) return "" + + const questions = args.questions + if (!Array.isArray(questions) || questions.length === 0) return "" + + const firstQuestion = questions[0] + if (!isRecord(firstQuestion)) return "" + + const questionText = firstQuestion.question + return typeof questionText === "string" ? questionText : "" +} diff --git a/src/hooks/session-notification.ts b/src/hooks/session-notification.ts index d54d38c61..dc83d3643 100644 --- a/src/hooks/session-notification.ts +++ b/src/hooks/session-notification.ts @@ -8,6 +8,11 @@ import { type Platform, } from "./session-notification-sender" import * as sessionNotificationSender from "./session-notification-sender" +import { + getEventToolName, + getQuestionText, + getSessionID, +} from "./session-notification-event-properties" import { hasIncompleteTodos } from "./session-todo-status" import { createIdleNotificationScheduler } from "./session-notification-scheduler" @@ -85,23 +90,6 @@ export function createSessionNotification( const PERMISSION_EVENTS = new Set(["permission.ask", "permission.asked", "permission.updated", "permission.requested"]) const PERMISSION_HINT_PATTERN = /\b(permission|approve|approval|allow|deny|consent)\b/i - const getSessionID = (properties: Record | undefined): string | undefined => { - const sessionID = properties?.sessionID - if (typeof sessionID === "string" && sessionID.length > 0) return sessionID - - const sessionId = properties?.sessionId - if (typeof sessionId === "string" && sessionId.length > 0) return sessionId - - const info = properties?.info as Record | undefined - const infoSessionID = info?.sessionID - if (typeof infoSessionID === "string" && infoSessionID.length > 0) return infoSessionID - - const infoSessionId = info?.sessionId - if (typeof infoSessionId === "string" && infoSessionId.length > 0) return infoSessionId - - return undefined - } - const shouldNotifyForSession = (sessionID: string): boolean => { if (subagentSessions.has(sessionID)) return false @@ -113,26 +101,6 @@ export function createSessionNotification( return true } - const getEventToolName = (properties: Record | undefined): string | undefined => { - const tool = properties?.tool - if (typeof tool === "string" && tool.length > 0) return tool - - const name = properties?.name - if (typeof name === "string" && name.length > 0) return name - - return undefined - } - - const getQuestionText = (properties: Record | undefined): string => { - const args = properties?.args as Record | undefined - const questions = args?.questions - if (!Array.isArray(questions) || questions.length === 0) return "" - - const firstQuestion = questions[0] as Record | undefined - const questionText = firstQuestion?.question - return typeof questionText === "string" ? questionText : "" - } - return async ({ event }: { event: { type: string; properties?: unknown } }) => { if (currentPlatform === "unsupported") return From 81b68d828cfa50910750efdc6180b2c75f314c1f Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 01:53:03 +0900 Subject: [PATCH 09/15] refactor(model-fallback): move fallback state into factory closure and split hook.ts Move the pending fallback, toast, and session-chain maps behind a shared controller initialized from the hook factory. This preserves the existing singleton semantics because exported helpers and hook instances still resolve the same lazily initialized controller while hook.ts stays under the 200-line limit. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../fallback-state-controller.ts | 135 +++++++++++++++++ src/hooks/model-fallback/hook.ts | 137 ++++++------------ 2 files changed, 179 insertions(+), 93 deletions(-) create mode 100644 src/hooks/model-fallback/fallback-state-controller.ts diff --git a/src/hooks/model-fallback/fallback-state-controller.ts b/src/hooks/model-fallback/fallback-state-controller.ts new file mode 100644 index 000000000..b2e6831a0 --- /dev/null +++ b/src/hooks/model-fallback/fallback-state-controller.ts @@ -0,0 +1,135 @@ +import type { FallbackEntry } from "../../shared/model-requirements" +import { getAgentConfigKey } from "../../shared/agent-display-names" +import { AGENT_MODEL_REQUIREMENTS } from "../../shared/model-requirements" +import { log } from "../../shared/logger" +import { getNextReachableFallback } from "./next-fallback" + +type ModelFallbackStateLike = { + providerID: string + modelID: string + fallbackChain: FallbackEntry[] + attemptCount: number + pending: boolean +} + +export type ModelFallbackStateController = { + lastToastKey: Map + setSessionFallbackChain: (sessionID: string, fallbackChain: FallbackEntry[] | undefined) => void + clearSessionFallbackChain: (sessionID: string) => void + setPendingModelFallback: ( + sessionID: string, + agentName: string, + currentProviderID: string, + currentModelID: string, + ) => boolean + getNextFallback: (sessionID: string) => ReturnType + clearPendingModelFallback: (sessionID: string) => void + hasPendingModelFallback: (sessionID: string) => boolean + getFallbackState: (sessionID: string) => ModelFallbackStateLike | undefined + reset: () => void +} + +export function createModelFallbackStateController(input: { + pendingModelFallbacks: Map + lastToastKey: Map + sessionFallbackChains: Map +}): ModelFallbackStateController { + const { pendingModelFallbacks, lastToastKey, sessionFallbackChains } = input + + function setSessionFallbackChain(sessionID: string, fallbackChain: FallbackEntry[] | undefined): void { + if (!sessionID) return + sessionFallbackChains.set(sessionID, fallbackChain?.length ? fallbackChain : []) + } + + function clearSessionFallbackChain(sessionID: string): void { + sessionFallbackChains.delete(sessionID) + } + + function setPendingModelFallback( + sessionID: string, + agentName: string, + currentProviderID: string, + currentModelID: string, + ): boolean { + const agentKey = getAgentConfigKey(agentName) + const requirements = AGENT_MODEL_REQUIREMENTS[agentKey] + const fallbackChain = sessionFallbackChains.has(sessionID) + ? sessionFallbackChains.get(sessionID) + : requirements?.fallbackChain + + if (!fallbackChain?.length) { + log("[model-fallback] No fallback chain for agent: " + agentName + " (key: " + agentKey + ")") + return false + } + + const existing = pendingModelFallbacks.get(sessionID) + if (existing) { + if (existing.pending) { + log("[model-fallback] Pending fallback already armed for session: " + sessionID) + return false + } + existing.providerID = currentProviderID + existing.modelID = currentModelID + existing.pending = true + if (existing.attemptCount >= existing.fallbackChain.length) { + log("[model-fallback] Fallback chain exhausted for session: " + sessionID) + return false + } + log("[model-fallback] Re-armed pending fallback for session: " + sessionID) + return true + } + + pendingModelFallbacks.set(sessionID, { + providerID: currentProviderID, + modelID: currentModelID, + fallbackChain, + attemptCount: 0, + pending: true, + }) + log("[model-fallback] Set pending fallback for session: " + sessionID + ", agent: " + agentName) + return true + } + + function getNextFallback(sessionID: string): ReturnType { + const state = pendingModelFallbacks.get(sessionID) + if (!state?.pending) return null + + const fallback = getNextReachableFallback(sessionID, state) + if (fallback) return fallback + + log("[model-fallback] No more fallbacks for session: " + sessionID) + pendingModelFallbacks.delete(sessionID) + return null + } + + function clearPendingModelFallback(sessionID: string): void { + pendingModelFallbacks.delete(sessionID) + lastToastKey.delete(sessionID) + } + + function hasPendingModelFallback(sessionID: string): boolean { + return pendingModelFallbacks.get(sessionID)?.pending === true + } + + function getFallbackState(sessionID: string): ModelFallbackStateLike | undefined { + return pendingModelFallbacks.get(sessionID) + } + + function reset(): void { + pendingModelFallbacks.clear() + lastToastKey.clear() + sessionFallbackChains.clear() + } + + return { + lastToastKey, + setSessionFallbackChain, + clearSessionFallbackChain, + setPendingModelFallback, + getNextFallback, + clearPendingModelFallback, + hasPendingModelFallback, + getFallbackState, + reset, + } +} diff --git a/src/hooks/model-fallback/hook.ts b/src/hooks/model-fallback/hook.ts index b188bd48d..191a58e3a 100644 --- a/src/hooks/model-fallback/hook.ts +++ b/src/hooks/model-fallback/hook.ts @@ -1,13 +1,10 @@ import type { FallbackEntry } from "../../shared/model-requirements" -import { getAgentConfigKey } from "../../shared/agent-display-names" -import { AGENT_MODEL_REQUIREMENTS } from "../../shared/model-requirements" -import { readConnectedProvidersCache, readProviderModelsCache } from "../../shared/connected-providers-cache" -import { selectFallbackProvider } from "../../shared/model-error-classifier" -import { transformModelForProvider } from "../../shared/provider-model-id-transform" -import { log } from "../../shared/logger" import type { ChatMessageInput, ChatMessageHandlerOutput } from "../../plugin/chat-message" import { applyFallbackToChatMessage } from "./chat-message-fallback-handler" -import { getNextReachableFallback } from "./next-fallback" +import { + createModelFallbackStateController, + type ModelFallbackStateController, +} from "./fallback-state-controller" type FallbackToast = (input: { title: string @@ -31,30 +28,26 @@ export type ModelFallbackState = { pending: boolean } -/** - * Map of sessionID -> pending model fallback state - * When a model error occurs, we store the fallback info here. - * The next chat.message call will use this to switch to the fallback model. - */ -const pendingModelFallbacks = new Map() -const lastToastKey = new Map() -const sessionFallbackChains = new Map() +const modelFallbackControllerRef: { current?: ModelFallbackStateController } = {} + +function getOrCreateModelFallbackController(): ModelFallbackStateController { + if (!modelFallbackControllerRef.current) { + createModelFallbackHook() + } + + const controller = modelFallbackControllerRef.current + if (!controller) { + throw new Error("Model fallback controller should be initialized") + } + return controller +} export function setSessionFallbackChain(sessionID: string, fallbackChain: FallbackEntry[] | undefined): void { - if (!sessionID) return - if (!fallbackChain) { - sessionFallbackChains.set(sessionID, []) - return - } - if (fallbackChain.length === 0) { - sessionFallbackChains.set(sessionID, []) - return - } - sessionFallbackChains.set(sessionID, fallbackChain) + getOrCreateModelFallbackController().setSessionFallbackChain(sessionID, fallbackChain) } export function clearSessionFallbackChain(sessionID: string): void { - sessionFallbackChains.delete(sessionID) + getOrCreateModelFallbackController().clearSessionFallbackChain(sessionID) } /** @@ -67,51 +60,12 @@ export function setPendingModelFallback( currentProviderID: string, currentModelID: string, ): boolean { - const agentKey = getAgentConfigKey(agentName) - const requirements = AGENT_MODEL_REQUIREMENTS[agentKey] - const hasSessionFallback = sessionFallbackChains.has(sessionID) - const sessionFallback = sessionFallbackChains.get(sessionID) - const fallbackChain = hasSessionFallback - ? sessionFallback - : requirements?.fallbackChain - - if (!fallbackChain || fallbackChain.length === 0) { - log("[model-fallback] No fallback chain for agent: " + agentName + " (key: " + agentKey + ")") - return false - } - - const existing = pendingModelFallbacks.get(sessionID) - - if (existing) { - if (existing.pending) { - log("[model-fallback] Pending fallback already armed for session: " + sessionID) - return false - } - - // Preserve progression across repeated session.error retries in same session. - // We only mark the next turn as pending fallback application. - existing.providerID = currentProviderID - existing.modelID = currentModelID - existing.pending = true - if (existing.attemptCount >= existing.fallbackChain.length) { - log("[model-fallback] Fallback chain exhausted for session: " + sessionID) - return false - } - log("[model-fallback] Re-armed pending fallback for session: " + sessionID) - return true - } - - const state: ModelFallbackState = { - providerID: currentProviderID, - modelID: currentModelID, - fallbackChain, - attemptCount: 0, - pending: true, - } - - pendingModelFallbacks.set(sessionID, state) - log("[model-fallback] Set pending fallback for session: " + sessionID + ", agent: " + agentName) - return true + return getOrCreateModelFallbackController().setPendingModelFallback( + sessionID, + agentName, + currentProviderID, + currentModelID, + ) } /** @@ -121,19 +75,7 @@ export function setPendingModelFallback( export function getNextFallback( sessionID: string, ): { providerID: string; modelID: string; variant?: string } | null { - const state = pendingModelFallbacks.get(sessionID) - if (!state) return null - - if (!state.pending) return null - - const fallback = getNextReachableFallback(sessionID, state) - if (fallback) { - return fallback - } - - log("[model-fallback] No more fallbacks for session: " + sessionID) - pendingModelFallbacks.delete(sessionID) - return null + return getOrCreateModelFallbackController().getNextFallback(sessionID) } /** @@ -141,29 +83,40 @@ export function getNextFallback( * Called after fallback is successfully applied. */ export function clearPendingModelFallback(sessionID: string): void { - pendingModelFallbacks.delete(sessionID) - lastToastKey.delete(sessionID) + getOrCreateModelFallbackController().clearPendingModelFallback(sessionID) } /** * Checks if there's a pending fallback for a session. */ export function hasPendingModelFallback(sessionID: string): boolean { - const state = pendingModelFallbacks.get(sessionID) - return state?.pending === true + return getOrCreateModelFallbackController().hasPendingModelFallback(sessionID) } /** * Gets the current fallback state for a session (for debugging). */ export function getFallbackState(sessionID: string): ModelFallbackState | undefined { - return pendingModelFallbacks.get(sessionID) + return getOrCreateModelFallbackController().getFallbackState(sessionID) } /** * Creates a chat.message hook that applies model fallbacks when pending. */ export function createModelFallbackHook(args?: { toast?: FallbackToast; onApplied?: FallbackCallback }) { + if (!modelFallbackControllerRef.current) { + const pendingModelFallbacks = new Map() + const lastToastKey = new Map() + const sessionFallbackChains = new Map() + + modelFallbackControllerRef.current = createModelFallbackStateController({ + pendingModelFallbacks, + lastToastKey, + sessionFallbackChains, + }) + } + + const controller = getOrCreateModelFallbackController() const toast = args?.toast const onApplied = args?.onApplied @@ -184,7 +137,7 @@ export function createModelFallbackHook(args?: { toast?: FallbackToast; onApplie fallback, toast, onApplied, - lastToastKey, + lastToastKey: controller.lastToastKey, }) }, } @@ -195,7 +148,5 @@ export function createModelFallbackHook(args?: { toast?: FallbackToast; onApplie * Clears pending fallbacks, toast keys, and session chains. */ export function _resetForTesting(): void { - pendingModelFallbacks.clear() - lastToastKey.clear() - sessionFallbackChains.clear() + getOrCreateModelFallbackController().reset() } From f56e3934d8767124778aef1bd2e39f7daefc65ab Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 02:17:25 +0900 Subject: [PATCH 10/15] docs: update plugin entry references to V1 PluginModule shape Sync the stale plugin entry docs with the shipped V1 PluginModule default export and remove the removed callable symbol references.\n\nUltraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)\nCo-authored-by: Sisyphus --- AGENTS.md | 4 ++-- CONTRIBUTING.md | 2 +- src/AGENTS.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 64bed7618..458e7a9f0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,7 +11,7 @@ OpenCode plugin (npm: `oh-my-opencode`) extending Claude Code with multi-agent o ``` oh-my-opencode/ ├── src/ -│ ├── index.ts # Plugin entry: loadConfig → createManagers → createTools → createHooks → createPluginInterface +│ ├── index.ts # Plugin entry: default export `pluginModule`, shape `{ id, server }` │ ├── plugin-config.ts # JSONC multi-level config: user → project → defaults (Zod v4) │ ├── agents/ # 11 agents (Sisyphus, Hephaestus, Oracle, Librarian, Explore, Atlas, Prometheus, Metis, Momus, Multimodal-Looker, Sisyphus-Junior) │ ├── hooks/ # 52 lifecycle hooks across dedicated modules and standalone files @@ -33,7 +33,7 @@ oh-my-opencode/ ## INITIALIZATION FLOW ``` -OhMyOpenCodePlugin(ctx) +pluginModule.server(input, options) ├─→ loadPluginConfig() # JSONC parse → project/user merge → Zod validate → migrate ├─→ createManagers() # TmuxSessionManager, BackgroundManager, SkillMcpManager, ConfigHandler ├─→ createTools() # SkillContext + AvailableCategories + ToolRegistry (26 tools) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f1ae6e419..f1ded421d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -109,7 +109,7 @@ After making changes, you can test your local build in OpenCode: ``` oh-my-opencode/ ├── src/ -│ ├── index.ts # Plugin entry (OhMyOpenCodePlugin) +│ ├── index.ts # Plugin entry (V1 PluginModule, default export) │ ├── plugin-config.ts # JSONC multi-level config (Zod v4) │ ├── agents/ # 11 agents (Sisyphus, Hephaestus, Oracle, Librarian, Explore, Atlas, Prometheus, Metis, Momus, Multimodal-Looker, Sisyphus-Junior) │ ├── hooks/ # 52 lifecycle hooks across 55 dedicated modules diff --git a/src/AGENTS.md b/src/AGENTS.md index 255bd8ea6..aa0bbd6f3 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -10,7 +10,7 @@ Entry point `index.ts` orchestrates 5-step initialization: loadConfig → create | File | Purpose | |------|---------| -| `index.ts` | Plugin entry, exports `OhMyOpenCodePlugin` | +| `index.ts` | Plugin entry, default-exports `pluginModule: PluginModule` with `{ id, server }` | | `plugin-config.ts` | JSONC parse, multi-level merge, Zod v4 validation | | `create-managers.ts` | TmuxSessionManager, BackgroundManager, SkillMcpManager, ConfigHandler | | `create-tools.ts` | SkillContext + AvailableCategories + ToolRegistry (26 tools) | From e2f5c0d3616b4c227da76f775d4b0c8b95d42fb7 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 02:23:55 +0900 Subject: [PATCH 11/15] refactor(plugin): remove orphaned createPluginDispose + stale test mocks Remove the dead plugin-dispose module and its dedicated test now that V1 plugin migration removed the last production call site. Clean the remaining bootstrap test mocks so src no longer references createPluginDispose. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/index.compacting.test.ts | 136 ++++++++++++++++ src/index.conditional-tools.test.ts | 85 ++++++++++ src/index.telemetry.test.ts | 4 - src/index.test.ts | 224 -------------------------- src/plugin-dispose.test.ts | 237 ---------------------------- src/plugin-dispose.ts | 51 ------ 6 files changed, 221 insertions(+), 516 deletions(-) create mode 100644 src/index.compacting.test.ts create mode 100644 src/index.conditional-tools.test.ts delete mode 100644 src/plugin-dispose.test.ts delete mode 100644 src/plugin-dispose.ts diff --git a/src/index.compacting.test.ts b/src/index.compacting.test.ts new file mode 100644 index 000000000..46434d8cb --- /dev/null +++ b/src/index.compacting.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it, mock } from "bun:test" + +function createCompactingHandler(hooks: { + compactionContextInjector?: { + capture: (sessionID: string) => Promise + inject: (sessionID: string) => string + } + compactionTodoPreserver?: { capture: (sessionID: string) => Promise } + claudeCodeHooks?: { + "experimental.session.compacting"?: ( + input: { sessionID: string }, + output: { context: string[] }, + ) => Promise + } +}) { + return async ( + input: { sessionID: string }, + output: { context: string[] }, + ): Promise => { + await hooks.compactionContextInjector?.capture(input.sessionID) + await hooks.compactionTodoPreserver?.capture(input.sessionID) + await hooks.claudeCodeHooks?.["experimental.session.compacting"]?.( + input, + output, + ) + if (hooks.compactionContextInjector) { + output.context.push(hooks.compactionContextInjector.inject(input.sessionID)) + } + } +} + +describe("experimental.session.compacting handler", () => { + //#given all three hooks are present + //#when compacting handler is invoked + //#then all hooks are called in order: capture → PreCompact → contextInjector + it("calls claudeCodeHooks PreCompact alongside other hooks", async () => { + const callOrder: string[] = [] + + const handler = createCompactingHandler({ + compactionContextInjector: { + capture: mock(async () => { + callOrder.push("checkpointCapture") + }), + inject: mock((sessionID: string) => { + callOrder.push("contextInjector") + return `context-for-${sessionID}` + }), + }, + compactionTodoPreserver: { + capture: mock(async () => { + callOrder.push("capture") + }), + }, + claudeCodeHooks: { + "experimental.session.compacting": mock(async () => { + callOrder.push("preCompact") + }), + }, + }) + + const output = { context: [] as string[] } + await handler({ sessionID: "ses_test" }, output) + + expect(callOrder).toEqual([ + "checkpointCapture", + "capture", + "preCompact", + "contextInjector", + ]) + expect(output.context).toEqual(["context-for-ses_test"]) + }) + + //#given claudeCodeHooks injects context during PreCompact + //#when compacting handler is invoked + //#then injected context from PreCompact is preserved in output + it("preserves context injected by PreCompact hooks", async () => { + const handler = createCompactingHandler({ + claudeCodeHooks: { + "experimental.session.compacting": async (_input, output) => { + output.context.push("precompact-injected-context") + }, + }, + }) + + const output = { context: [] as string[] } + await handler({ sessionID: "ses_test" }, output) + + expect(output.context).toContain("precompact-injected-context") + }) + + //#given claudeCodeHooks is null (no claude code hooks configured) + //#when compacting handler is invoked + //#then handler completes without error and other hooks still run + it("handles null claudeCodeHooks gracefully", async () => { + const captureMock = mock(async () => {}) + const checkpointCaptureMock = mock(async () => {}) + const contextMock = mock(() => "injected-context") + + const handler = createCompactingHandler({ + compactionContextInjector: { + capture: checkpointCaptureMock, + inject: contextMock, + }, + compactionTodoPreserver: { capture: captureMock }, + claudeCodeHooks: undefined, + }) + + const output = { context: [] as string[] } + await handler({ sessionID: "ses_test" }, output) + + expect(checkpointCaptureMock).toHaveBeenCalledWith("ses_test") + expect(captureMock).toHaveBeenCalledWith("ses_test") + expect(contextMock).toHaveBeenCalledWith("ses_test") + expect(output.context).toEqual(["injected-context"]) + }) + + //#given compactionContextInjector is null + //#when compacting handler is invoked + //#then handler does not early-return, PreCompact hooks still execute + it("does not early-return when compactionContextInjector is null", async () => { + const preCompactMock = mock(async () => {}) + + const handler = createCompactingHandler({ + claudeCodeHooks: { + "experimental.session.compacting": preCompactMock, + }, + compactionContextInjector: undefined, + }) + + const output = { context: [] as string[] } + await handler({ sessionID: "ses_test" }, output) + + expect(preCompactMock).toHaveBeenCalled() + expect(output.context).toEqual([]) + }) +}) diff --git a/src/index.conditional-tools.test.ts b/src/index.conditional-tools.test.ts new file mode 100644 index 000000000..96c955c4b --- /dev/null +++ b/src/index.conditional-tools.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it } from "bun:test" + +describe("look_at tool conditional registration", () => { + describe("isMultimodalLookerEnabled logic", () => { + // given multimodal-looker is in disabled_agents + // when checking if agent is enabled + // then should return false (disabled) + it("returns false when multimodal-looker is disabled (exact case)", () => { + const disabledAgents: string[] = ["multimodal-looker"] + const isEnabled = !disabledAgents.some( + (agent) => agent.toLowerCase() === "multimodal-looker", + ) + expect(isEnabled).toBe(false) + }) + + // given multimodal-looker is in disabled_agents with different case + // when checking if agent is enabled + // then should return false (case-insensitive match) + it("returns false when multimodal-looker is disabled (case-insensitive)", () => { + const disabledAgents: string[] = ["Multimodal-Looker"] + const isEnabled = !disabledAgents.some( + (agent) => agent.toLowerCase() === "multimodal-looker", + ) + expect(isEnabled).toBe(false) + }) + + // given multimodal-looker is NOT in disabled_agents + // when checking if agent is enabled + // then should return true (enabled) + it("returns true when multimodal-looker is not disabled", () => { + const disabledAgents: string[] = ["oracle", "librarian"] + const isEnabled = !disabledAgents.some( + (agent) => agent.toLowerCase() === "multimodal-looker", + ) + expect(isEnabled).toBe(true) + }) + + // given disabled_agents is empty + // when checking if agent is enabled + // then should return true (enabled by default) + it("returns true when disabled_agents is empty", () => { + const disabledAgents: string[] = [] + const isEnabled = !disabledAgents.some( + (agent) => agent.toLowerCase() === "multimodal-looker", + ) + expect(isEnabled).toBe(true) + }) + + // given disabled_agents is undefined (simulated as empty array) + // when checking if agent is enabled + // then should return true (enabled by default) + it("returns true when disabled_agents is undefined (fallback to empty)", () => { + const disabledAgents: string[] | undefined = undefined + const list: string[] = disabledAgents ?? [] + const isEnabled = !list.some( + (agent) => agent.toLowerCase() === "multimodal-looker", + ) + expect(isEnabled).toBe(true) + }) + }) + + describe("conditional tool spread pattern", () => { + // given lookAt is not null (agent enabled) + // when spreading into tool object + // then look_at should be included + it("includes look_at when lookAt is not null", () => { + const lookAt = { execute: () => {} } + const tools = { + ...(lookAt ? { look_at: lookAt } : {}), + } + expect(tools).toHaveProperty("look_at") + }) + + // given lookAt is null (agent disabled) + // when spreading into tool object + // then look_at should NOT be included + it("excludes look_at when lookAt is null", () => { + const lookAt = null + const tools = { + ...(lookAt ? { look_at: lookAt } : {}), + } + expect(tools).not.toHaveProperty("look_at") + }) + }) +}) diff --git a/src/index.telemetry.test.ts b/src/index.telemetry.test.ts index 1f552f7eb..7f751f594 100644 --- a/src/index.telemetry.test.ts +++ b/src/index.telemetry.test.ts @@ -29,7 +29,6 @@ const mockCreateHooks = mock(() => ({ compactionTodoPreserver: undefined, claudeCodeHooks: undefined, })) -const mockCreatePluginDispose = mock(() => async () => {}) const mockCreatePluginInterface = mock(() => ({})) const mockCreatePluginPostHog = mock(() => ({ trackActive: () => { @@ -70,9 +69,6 @@ function installModuleMocks(): void { mock.module("./create-hooks", () => ({ createHooks: mockCreateHooks, })) - mock.module("./plugin-dispose", () => ({ - createPluginDispose: mockCreatePluginDispose, - })) mock.module("./plugin-interface", () => ({ createPluginInterface: mockCreatePluginInterface, })) diff --git a/src/index.test.ts b/src/index.test.ts index 00af70bb9..335562cd0 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -1,223 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test" -describe("experimental.session.compacting handler", () => { - function createCompactingHandler(hooks: { - compactionContextInjector?: { - capture: (sessionID: string) => Promise - inject: (sessionID: string) => string - } - compactionTodoPreserver?: { capture: (sessionID: string) => Promise } - claudeCodeHooks?: { - "experimental.session.compacting"?: ( - input: { sessionID: string }, - output: { context: string[] }, - ) => Promise - } - }) { - return async ( - _input: { sessionID: string }, - output: { context: string[] }, - ): Promise => { - await hooks.compactionContextInjector?.capture(_input.sessionID) - await hooks.compactionTodoPreserver?.capture(_input.sessionID) - await hooks.claudeCodeHooks?.["experimental.session.compacting"]?.( - _input, - output, - ) - if (hooks.compactionContextInjector) { - output.context.push(hooks.compactionContextInjector.inject(_input.sessionID)) - } - } - } - - //#given all three hooks are present - //#when compacting handler is invoked - //#then all hooks are called in order: capture → PreCompact → contextInjector - it("calls claudeCodeHooks PreCompact alongside other hooks", async () => { - const callOrder: string[] = [] - - const handler = createCompactingHandler({ - compactionContextInjector: { - capture: mock(async () => { - callOrder.push("checkpointCapture") - }), - inject: mock((sessionID: string) => { - callOrder.push("contextInjector") - return `context-for-${sessionID}` - }), - }, - compactionTodoPreserver: { - capture: mock(async () => { callOrder.push("capture") }), - }, - claudeCodeHooks: { - "experimental.session.compacting": mock(async () => { - callOrder.push("preCompact") - }), - }, - }) - - const output = { context: [] as string[] } - await handler({ sessionID: "ses_test" }, output) - - expect(callOrder).toEqual(["checkpointCapture", "capture", "preCompact", "contextInjector"]) - expect(output.context).toEqual(["context-for-ses_test"]) - }) - - //#given claudeCodeHooks injects context during PreCompact - //#when compacting handler is invoked - //#then injected context from PreCompact is preserved in output - it("preserves context injected by PreCompact hooks", async () => { - const handler = createCompactingHandler({ - claudeCodeHooks: { - "experimental.session.compacting": async (_input, output) => { - output.context.push("precompact-injected-context") - }, - }, - }) - - const output = { context: [] as string[] } - await handler({ sessionID: "ses_test" }, output) - - expect(output.context).toContain("precompact-injected-context") - }) - - //#given claudeCodeHooks is null (no claude code hooks configured) - //#when compacting handler is invoked - //#then handler completes without error and other hooks still run - it("handles null claudeCodeHooks gracefully", async () => { - const captureMock = mock(async () => {}) - const checkpointCaptureMock = mock(async () => {}) - const contextMock = mock(() => "injected-context") - - const handler = createCompactingHandler({ - compactionContextInjector: { - capture: checkpointCaptureMock, - inject: contextMock, - }, - compactionTodoPreserver: { capture: captureMock }, - claudeCodeHooks: undefined, - }) - - const output = { context: [] as string[] } - await handler({ sessionID: "ses_test" }, output) - - expect(checkpointCaptureMock).toHaveBeenCalledWith("ses_test") - expect(captureMock).toHaveBeenCalledWith("ses_test") - expect(contextMock).toHaveBeenCalledWith("ses_test") - expect(output.context).toEqual(["injected-context"]) - }) - - //#given compactionContextInjector is null - //#when compacting handler is invoked - //#then handler does not early-return, PreCompact hooks still execute - it("does not early-return when compactionContextInjector is null", async () => { - const preCompactMock = mock(async () => {}) - - const handler = createCompactingHandler({ - claudeCodeHooks: { - "experimental.session.compacting": preCompactMock, - }, - compactionContextInjector: undefined, - }) - - const output = { context: [] as string[] } - await handler({ sessionID: "ses_test" }, output) - - expect(preCompactMock).toHaveBeenCalled() - expect(output.context).toEqual([]) - }) -}) - -/** - * Tests for conditional tool registration logic in index.ts - * - * The actual plugin initialization is complex to test directly, - * so we test the underlying logic that determines tool registration. - */ -describe("look_at tool conditional registration", () => { - describe("isMultimodalLookerEnabled logic", () => { - // given multimodal-looker is in disabled_agents - // when checking if agent is enabled - // then should return false (disabled) - it("returns false when multimodal-looker is disabled (exact case)", () => { - const disabledAgents: string[] = ["multimodal-looker"] - const isEnabled = !disabledAgents.some( - (agent) => agent.toLowerCase() === "multimodal-looker" - ) - expect(isEnabled).toBe(false) - }) - - // given multimodal-looker is in disabled_agents with different case - // when checking if agent is enabled - // then should return false (case-insensitive match) - it("returns false when multimodal-looker is disabled (case-insensitive)", () => { - const disabledAgents: string[] = ["Multimodal-Looker"] - const isEnabled = !disabledAgents.some( - (agent) => agent.toLowerCase() === "multimodal-looker" - ) - expect(isEnabled).toBe(false) - }) - - // given multimodal-looker is NOT in disabled_agents - // when checking if agent is enabled - // then should return true (enabled) - it("returns true when multimodal-looker is not disabled", () => { - const disabledAgents: string[] = ["oracle", "librarian"] - const isEnabled = !disabledAgents.some( - (agent) => agent.toLowerCase() === "multimodal-looker" - ) - expect(isEnabled).toBe(true) - }) - - // given disabled_agents is empty - // when checking if agent is enabled - // then should return true (enabled by default) - it("returns true when disabled_agents is empty", () => { - const disabledAgents: string[] = [] - const isEnabled = !disabledAgents.some( - (agent) => agent.toLowerCase() === "multimodal-looker" - ) - expect(isEnabled).toBe(true) - }) - - // given disabled_agents is undefined (simulated as empty array) - // when checking if agent is enabled - // then should return true (enabled by default) - it("returns true when disabled_agents is undefined (fallback to empty)", () => { - const disabledAgents: string[] | undefined = undefined - const list: string[] = disabledAgents ?? [] - const isEnabled = !list.some( - (agent) => agent.toLowerCase() === "multimodal-looker" - ) - expect(isEnabled).toBe(true) - }) - }) - - describe("conditional tool spread pattern", () => { - // given lookAt is not null (agent enabled) - // when spreading into tool object - // then look_at should be included - it("includes look_at when lookAt is not null", () => { - const lookAt = { execute: () => {} } // mock tool - const tools = { - ...(lookAt ? { look_at: lookAt } : {}), - } - expect(tools).toHaveProperty("look_at") - }) - - // given lookAt is null (agent disabled) - // when spreading into tool object - // then look_at should NOT be included - it("excludes look_at when lookAt is null", () => { - const lookAt = null - const tools = { - ...(lookAt ? { look_at: lookAt } : {}), - } - expect(tools).not.toHaveProperty("look_at") - }) - }) -}) - const mockInitConfigContext = mock(() => {}) const mockDetectExternalSkillPlugin = mock(() => ({ detected: false, pluginName: null })) const mockGetSkillPluginConflictWarning = mock(() => "") @@ -252,7 +34,6 @@ const mockCreateHooks = mock(() => ({ compactionTodoPreserver: undefined, claudeCodeHooks: undefined, })) -const mockCreatePluginDispose = mock(() => async () => {}) const mockCreatePluginInterface = mock(() => ({})) const mockInitializeOpenClaw = mock(async () => {}) const mockStartTmuxCheck = mock(() => {}) @@ -297,10 +78,6 @@ function installIndexModuleMocks(): void { createHooks: mockCreateHooks, })) - mock.module("./plugin-dispose", () => ({ - createPluginDispose: mockCreatePluginDispose, - })) - mock.module("./plugin-interface", () => ({ createPluginInterface: mockCreatePluginInterface, })) @@ -350,7 +127,6 @@ describe("OhMyOpenCodePlugin", () => { mockCreateManagers.mockClear() mockCreateTools.mockClear() mockCreateHooks.mockClear() - mockCreatePluginDispose.mockClear() mockCreatePluginInterface.mockClear() mockInitializeOpenClaw.mockClear() mockStartTmuxCheck.mockClear() diff --git a/src/plugin-dispose.test.ts b/src/plugin-dispose.test.ts deleted file mode 100644 index d0dd0285b..000000000 --- a/src/plugin-dispose.test.ts +++ /dev/null @@ -1,237 +0,0 @@ -import { describe, expect, spyOn, test } from "bun:test" - -import { disposeCreatedHooks } from "./create-hooks" -import { createPluginDispose } from "./plugin-dispose" - -describe("createPluginDispose", () => { - test("#given plugin with active managers and hooks #when dispose() is called #then backgroundManager.shutdown() is called", async () => { - // given - const backgroundManager = { - shutdown: async (): Promise => {}, - } - const skillMcpManager = { - disconnectAll: async (): Promise => {}, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - const shutdownSpy = spyOn(backgroundManager, "shutdown") - const dispose = createPluginDispose({ - backgroundManager, - skillMcpManager, - lspManager, - disposeHooks: (): void => {}, - }) - - // when - await dispose() - - // then - expect(shutdownSpy).toHaveBeenCalledTimes(1) - }) - - test("#given plugin with active MCP connections #when dispose() is called #then skillMcpManager.disconnectAll() is called", async () => { - // given - const backgroundManager = { - shutdown: async (): Promise => {}, - } - const skillMcpManager = { - disconnectAll: async (): Promise => {}, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - const disconnectAllSpy = spyOn(skillMcpManager, "disconnectAll") - const dispose = createPluginDispose({ - backgroundManager, - skillMcpManager, - lspManager, - disposeHooks: (): void => {}, - }) - - // when - await dispose() - - // then - expect(disconnectAllSpy).toHaveBeenCalledTimes(1) - }) - - test("#given plugin with hooks that have dispose #when dispose() is called #then each hook's dispose is called", async () => { - // given - const claudeCodeHooks = { - dispose: (): void => {}, - } - const commentChecker = { - dispose: (): void => {}, - } - const runtimeFallback = { - dispose: (): void => {}, - } - const todoContinuationEnforcer = { - dispose: (): void => {}, - } - const autoSlashCommand = { - dispose: (): void => {}, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - const claudeCodeHooksDisposeSpy = spyOn(claudeCodeHooks, "dispose") - const commentCheckerDisposeSpy = spyOn(commentChecker, "dispose") - const runtimeFallbackDisposeSpy = spyOn(runtimeFallback, "dispose") - const todoContinuationEnforcerDisposeSpy = spyOn(todoContinuationEnforcer, "dispose") - const autoSlashCommandDisposeSpy = spyOn(autoSlashCommand, "dispose") - const dispose = createPluginDispose({ - backgroundManager: { - shutdown: async (): Promise => {}, - }, - skillMcpManager: { - disconnectAll: async (): Promise => {}, - }, - lspManager, - disposeHooks: (): void => { - disposeCreatedHooks({ - claudeCodeHooks, - commentChecker, - runtimeFallback, - todoContinuationEnforcer, - autoSlashCommand, - }) - }, - }) - - // when - await dispose() - - // then - expect(claudeCodeHooksDisposeSpy).toHaveBeenCalledTimes(1) - expect(commentCheckerDisposeSpy).toHaveBeenCalledTimes(1) - expect(runtimeFallbackDisposeSpy).toHaveBeenCalledTimes(1) - expect(todoContinuationEnforcerDisposeSpy).toHaveBeenCalledTimes(1) - expect(autoSlashCommandDisposeSpy).toHaveBeenCalledTimes(1) - }) - - test("#given dispose already called #when dispose() called again #then no errors", async () => { - // given - const backgroundManager = { - shutdown: async (): Promise => {}, - } - const skillMcpManager = { - disconnectAll: async (): Promise => {}, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - const disposeHooks = { - run: (): void => {}, - } - const shutdownSpy = spyOn(backgroundManager, "shutdown") - const disconnectAllSpy = spyOn(skillMcpManager, "disconnectAll") - const stopAllSpy = spyOn(lspManager, "stopAll") - const disposeHooksSpy = spyOn(disposeHooks, "run") - const dispose = createPluginDispose({ - backgroundManager, - skillMcpManager, - lspManager, - disposeHooks: disposeHooks.run, - }) - - // when - await dispose() - await dispose() - - // then - expect(shutdownSpy).toHaveBeenCalledTimes(1) - expect(disconnectAllSpy).toHaveBeenCalledTimes(1) - expect(stopAllSpy).toHaveBeenCalledTimes(1) - expect(disposeHooksSpy).toHaveBeenCalledTimes(1) - }) - - test("#given backgroundManager.shutdown() throws #when dispose() is called #then skillMcpManager.disconnectAll() and disposeHooks() are still called", async () => { - // given - const backgroundManager = { - shutdown: async (): Promise => { - throw new Error("shutdown failed") - }, - } - const skillMcpManager = { - disconnectAll: async (): Promise => {}, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - const disposeHooksCalls: number[] = [] - const disconnectAllSpy = spyOn(skillMcpManager, "disconnectAll") - const dispose = createPluginDispose({ - backgroundManager, - skillMcpManager, - lspManager, - disposeHooks: (): void => { - disposeHooksCalls.push(1) - }, - }) - - // when - await dispose() - - // then - expect(disconnectAllSpy).toHaveBeenCalledTimes(1) - expect(disposeHooksCalls).toHaveLength(1) - }) - - test("#given skillMcpManager.disconnectAll() throws #when dispose() is called #then disposeHooks() is still called", async () => { - // given - const backgroundManager = { - shutdown: async (): Promise => {}, - } - const skillMcpManager = { - disconnectAll: async (): Promise => { - throw new Error("disconnectAll failed") - }, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - const disposeHooksCalls: number[] = [] - const shutdownSpy = spyOn(backgroundManager, "shutdown") - const dispose = createPluginDispose({ - backgroundManager, - skillMcpManager, - lspManager, - disposeHooks: (): void => { - disposeHooksCalls.push(1) - }, - }) - - // when - await dispose() - - // then - expect(shutdownSpy).toHaveBeenCalledTimes(1) - expect(disposeHooksCalls).toHaveLength(1) - }) - - test("#given active LSP clients #when dispose runs #then lsp manager is stopped", async () => { - // given - const lspManager = { - stopAll: async (): Promise => {}, - } - const stopAllSpy = spyOn(lspManager, "stopAll") - const dispose = createPluginDispose({ - backgroundManager: { - shutdown: async (): Promise => {}, - }, - skillMcpManager: { - disconnectAll: async (): Promise => {}, - }, - lspManager, - disposeHooks: (): void => {}, - }) - - // when - await dispose() - - // then - expect(stopAllSpy).toHaveBeenCalledTimes(1) - }) -}) diff --git a/src/plugin-dispose.ts b/src/plugin-dispose.ts deleted file mode 100644 index 998fd28eb..000000000 --- a/src/plugin-dispose.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { log } from "./shared" - -export type PluginDispose = () => Promise - -export function createPluginDispose(args: { - backgroundManager: { - shutdown: () => void | Promise - } - skillMcpManager: { - disconnectAll: () => Promise - } - lspManager: { - stopAll: () => Promise - } - disposeHooks: () => void -}): PluginDispose { - const { backgroundManager, skillMcpManager, lspManager, disposeHooks } = args - let disposePromise: Promise | null = null - - return async (): Promise => { - if (disposePromise) { - await disposePromise - return - } - - disposePromise = (async (): Promise => { - try { - await backgroundManager.shutdown() - } catch (error) { - log("[plugin-dispose] backgroundManager.shutdown() error:", error) - } - try { - await skillMcpManager.disconnectAll() - } catch (error) { - log("[plugin-dispose] skillMcpManager.disconnectAll() error:", error) - } - try { - await lspManager.stopAll() - } catch (error) { - log("[plugin-dispose] lspManager.stopAll() error:", error) - } - try { - disposeHooks() - } catch (error) { - log("[plugin-dispose] disposeHooks() error:", error) - } - })() - - await disposePromise - } -} From 5e4102566cd7a53c3cc7fed49ee2a85e90211595 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 02:35:46 +0900 Subject: [PATCH 12/15] refactor(model-fallback): fully encapsulate session state in factory closure Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/create-hooks.ts | 4 + src/create-managers.ts | 5 + src/create-tools.ts | 2 +- src/hooks/index.ts | 8 +- .../model-fallback/controller-accessor.ts | 30 +++++ src/hooks/model-fallback/hook.test.ts | 77 +++++++----- src/hooks/model-fallback/hook.ts | 117 ++++++++++++------ src/hooks/model-fallback/index.ts | 2 + src/index.ts | 1 + src/plugin/event.model-fallback-2941.test.ts | 21 ++-- src/plugin/event.model-fallback.test.ts | 13 +- src/plugin/event.test.ts | 3 +- src/plugin/event.ts | 31 +++-- .../fallback.cliproxyapi-matrix.test.ts | 2 - src/plugin/hooks/create-core-hooks.ts | 5 +- src/plugin/hooks/create-session-hooks.ts | 5 +- src/plugin/tool-registry.ts | 4 +- src/tools/call-omo-agent/sync-executor.ts | 9 +- src/tools/call-omo-agent/tools.ts | 39 +++++- src/tools/delegate-task/background-task.ts | 7 +- src/tools/delegate-task/executor-types.ts | 2 + src/tools/delegate-task/sync-task.ts | 5 +- src/tools/delegate-task/types.ts | 2 + 23 files changed, 271 insertions(+), 123 deletions(-) create mode 100644 src/hooks/model-fallback/controller-accessor.ts create mode 100644 src/hooks/model-fallback/index.ts diff --git a/src/create-hooks.ts b/src/create-hooks.ts index 0e40ad480..436f8e2b9 100644 --- a/src/create-hooks.ts +++ b/src/create-hooks.ts @@ -2,6 +2,7 @@ import type { AvailableSkill } from "./agents/dynamic-agent-prompt-builder" import type { HookName, OhMyOpenCodeConfig } from "./config" import type { LoadedSkill } from "./features/opencode-skill-loader/types" import type { BackgroundManager } from "./features/background-agent" +import type { ModelFallbackControllerAccessor } from "./hooks/model-fallback" import type { PluginContext } from "./plugin/types" import type { ModelCacheState } from "./plugin-state" @@ -36,6 +37,7 @@ export function createHooks(args: { pluginConfig: OhMyOpenCodeConfig modelCacheState: ModelCacheState backgroundManager: BackgroundManager + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor isHookEnabled: (hookName: HookName) => boolean safeHookEnabled: boolean mergedSkills: LoadedSkill[] @@ -46,6 +48,7 @@ export function createHooks(args: { pluginConfig, modelCacheState, backgroundManager, + modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled, mergedSkills, @@ -56,6 +59,7 @@ export function createHooks(args: { ctx, pluginConfig, modelCacheState, + modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled, }) diff --git a/src/create-managers.ts b/src/create-managers.ts index d40896343..9c0013fd4 100644 --- a/src/create-managers.ts +++ b/src/create-managers.ts @@ -5,6 +5,7 @@ import type { PluginContext, TmuxConfig } from "./plugin/types" import type { SubagentSessionCreatedEvent } from "./features/background-agent" import { BackgroundManager } from "./features/background-agent" import { SkillMcpManager } from "./features/skill-mcp-manager" +import { createModelFallbackControllerAccessor } from "./hooks/model-fallback" import { initTaskToastManager } from "./features/task-toast-manager" import { TmuxSessionManager } from "./features/tmux-subagent" import * as openclawRuntimeDispatch from "./openclaw/runtime-dispatch" @@ -12,6 +13,7 @@ import { registerManagerForCleanup } from "./features/background-agent/process-c import { createConfigHandler } from "./plugin-handlers" import { log } from "./shared" import { markServerRunningInProcess } from "./shared/tmux/tmux-utils/server-health" +import type { ModelFallbackControllerAccessor } from "./hooks/model-fallback" type CreateManagersDeps = { BackgroundManagerClass: typeof BackgroundManager @@ -38,6 +40,7 @@ export type Managers = { backgroundManager: BackgroundManager skillMcpManager: SkillMcpManager configHandler: ReturnType + modelFallbackControllerAccessor: ModelFallbackControllerAccessor } export function createManagers(args: { @@ -119,11 +122,13 @@ export function createManagers(args: { pluginConfig, modelCacheState, }) + const modelFallbackControllerAccessor = createModelFallbackControllerAccessor() return { tmuxSessionManager, backgroundManager, skillMcpManager, configHandler, + modelFallbackControllerAccessor, } } diff --git a/src/create-tools.ts b/src/create-tools.ts index 5ac5a7e2f..6a9bc3941 100644 --- a/src/create-tools.ts +++ b/src/create-tools.ts @@ -22,7 +22,7 @@ type CreateToolsResult = { export async function createTools(args: { ctx: PluginContext pluginConfig: OhMyOpenCodeConfig - managers: Pick + managers: Pick }): Promise { const { ctx, pluginConfig, managers } = args diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 051cbd12a..8fd15af2f 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -14,7 +14,13 @@ export { createEmptyTaskResponseDetectorHook } from "./empty-task-response-detec export { createAnthropicContextWindowLimitRecoveryHook, type AnthropicContextWindowLimitRecoveryOptions } from "./anthropic-context-window-limit-recovery"; export { createThinkModeHook } from "./think-mode"; -export { createModelFallbackHook, setPendingModelFallback, clearPendingModelFallback, type ModelFallbackState } from "./model-fallback/hook"; +export { + createModelFallbackHook, + setPendingModelFallback, + clearPendingModelFallback, + type ModelFallbackHook, + type ModelFallbackState, +} from "./model-fallback/hook"; export { createClaudeCodeHooksHook } from "./claude-code-hooks"; export { createRulesInjectorHook } from "./rules-injector"; export { createBackgroundNotificationHook } from "./background-notification" diff --git a/src/hooks/model-fallback/controller-accessor.ts b/src/hooks/model-fallback/controller-accessor.ts new file mode 100644 index 000000000..281ae9931 --- /dev/null +++ b/src/hooks/model-fallback/controller-accessor.ts @@ -0,0 +1,30 @@ +import type { FallbackEntry } from "../../shared/model-requirements" +import type { ModelFallbackStateController } from "./fallback-state-controller" + +export type ModelFallbackControllerAccessor = { + register: (controller: ModelFallbackStateController) => void + setSessionFallbackChain: (sessionID: string, fallbackChain: FallbackEntry[] | undefined) => void + clearSessionFallbackChain: (sessionID: string) => void +} + +export function createModelFallbackControllerAccessor(): ModelFallbackControllerAccessor { + let controller: ModelFallbackStateController | null = null + + function register(nextController: ModelFallbackStateController): void { + controller = nextController + } + + function setSessionFallbackChain(sessionID: string, fallbackChain: FallbackEntry[] | undefined): void { + controller?.setSessionFallbackChain(sessionID, fallbackChain) + } + + function clearSessionFallbackChain(sessionID: string): void { + controller?.clearSessionFallbackChain(sessionID) + } + + return { + register, + setSessionFallbackChain, + clearSessionFallbackChain, + } +} diff --git a/src/hooks/model-fallback/hook.test.ts b/src/hooks/model-fallback/hook.test.ts index 14b3ff6bb..de9e66fd7 100644 --- a/src/hooks/model-fallback/hook.test.ts +++ b/src/hooks/model-fallback/hook.test.ts @@ -70,22 +70,23 @@ const { setPendingModelFallback, } = await importFreshModelFallbackHookModule() +type ModelFallbackHook = ReturnType + describe("model fallback hook", () => { + let modelFallback: ModelFallbackHook + beforeEach(() => { + modelFallback = createModelFallbackHook() readConnectedProvidersCacheMock.mockReturnValue(null) readProviderModelsCacheMock.mockReturnValue(null) readConnectedProvidersCacheMock.mockClear() readProviderModelsCacheMock.mockClear() selectFallbackProviderMock.mockClear() - - clearPendingModelFallback("ses_model_fallback_main") - clearPendingModelFallback("ses_model_fallback_ghcp") - clearPendingModelFallback("ses_model_fallback_google") }) test("applies pending fallback on chat.message by overriding model", async () => { //#given - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, @@ -93,6 +94,7 @@ describe("model fallback hook", () => { } const set = setPendingModelFallback( + modelFallback, "ses_model_fallback_main", "Sisyphus - Ultraworker", "anthropic", @@ -123,7 +125,7 @@ describe("model fallback hook", () => { test("preserves fallback progression across repeated session.error retries", async () => { //#given - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, @@ -132,7 +134,7 @@ describe("model fallback hook", () => { const sessionID = "ses_model_fallback_main" expect( - setPendingModelFallback(sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7-thinking"), + setPendingModelFallback(modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7-thinking"), ).toBe(true) const firstOutput = { @@ -154,7 +156,7 @@ describe("model fallback hook", () => { //#when - second error re-arms fallback and should advance to next entry expect( - setPendingModelFallback(sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7"), + setPendingModelFallback(modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7"), ).toBe(true) const secondOutput = { @@ -176,16 +178,18 @@ describe("model fallback hook", () => { test("does not re-arm fallback when one is already pending", () => { //#given const sessionID = "ses_model_fallback_pending_guard" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) //#when const firstSet = setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7-thinking", ) const secondSet = setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", @@ -195,28 +199,29 @@ describe("model fallback hook", () => { //#then expect(firstSet).toBe(true) expect(secondSet).toBe(false) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("skips no-op fallback entries that resolve to same provider/model", async () => { //#given const sessionID = "ses_model_fallback_noop_skip" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, ) => Promise } - setSessionFallbackChain(sessionID, [ + setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["anthropic"], model: "claude-opus-4-7" }, { providers: ["opencode"], model: "kimi-k2.5-free" }, ]) expect( setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", @@ -239,28 +244,29 @@ describe("model fallback hook", () => { providerID: "opencode", modelID: "kimi-k2.5-free", }) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("skips no-op fallback entries even when variant differs", async () => { //#given const sessionID = "ses_model_fallback_noop_variant_skip" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, ) => Promise } - setSessionFallbackChain(sessionID, [ + setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["quotio"], model: "claude-opus-4-7", variant: "max" }, { providers: ["quotio"], model: "gpt-5.2" }, ]) expect( setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Ultraworker", "quotio", @@ -285,28 +291,29 @@ describe("model fallback hook", () => { modelID: "gpt-5.2", }) expect(output.message["variant"]).toBeUndefined() - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("uses connected preferred provider when fallback entry providers are disconnected", async () => { //#given const sessionID = "ses_model_fallback_preferred_provider" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) readConnectedProvidersCacheMock.mockReturnValue(["provider-x"]) - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, ) => Promise } - setSessionFallbackChain(sessionID, [ + setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["provider-y"], model: "fallback-model" }, ]) expect( setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Ultraworker", "provider-x", @@ -329,17 +336,18 @@ describe("model fallback hook", () => { providerID: "provider-x", modelID: "fallback-model", }) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("does not fall back to hardcoded agent chain when session explicitly stores no fallback chain [regression #2941]", () => { //#given const sessionID = "ses_model_fallback_explicit_none" - clearPendingModelFallback(sessionID) - setSessionFallbackChain(sessionID, undefined) + clearPendingModelFallback(modelFallback, sessionID) + setSessionFallbackChain(modelFallback, sessionID, undefined) //#when const set = setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Junior", "anthropic", @@ -348,7 +356,7 @@ describe("model fallback hook", () => { //#then expect(set).toBe(false) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("shows toast when fallback is applied", async () => { @@ -366,6 +374,7 @@ describe("model fallback hook", () => { } const set = setPendingModelFallback( + hook, "ses_model_fallback_toast", "Sisyphus - Ultraworker", "anthropic", @@ -392,9 +401,9 @@ describe("model fallback hook", () => { test("transforms model names for github-copilot provider via fallback chain", async () => { //#given const sessionID = "ses_model_fallback_ghcp" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, @@ -402,11 +411,12 @@ describe("model fallback hook", () => { } // Set a custom fallback chain that routes through github-copilot - setSessionFallbackChain(sessionID, [ + setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["github-copilot"], model: "claude-sonnet-4-6" }, ]) const set = setPendingModelFallback( + modelFallback, sessionID, "Atlas - Plan Executor", "github-copilot", @@ -430,15 +440,15 @@ describe("model fallback hook", () => { modelID: "claude-sonnet-4.6", }) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("preserves canonical google preview model names via fallback chain", async () => { //#given const sessionID = "ses_model_fallback_google" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, @@ -446,11 +456,12 @@ describe("model fallback hook", () => { } // Set a custom fallback chain that routes through google - setSessionFallbackChain(sessionID, [ + setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["google"], model: "gemini-3.1-pro-preview" }, ]) const set = setPendingModelFallback( + modelFallback, sessionID, "Oracle", "google", @@ -474,7 +485,7 @@ describe("model fallback hook", () => { modelID: "gemini-3.1-pro-preview", }) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) }) diff --git a/src/hooks/model-fallback/hook.ts b/src/hooks/model-fallback/hook.ts index 191a58e3a..fee130ed8 100644 --- a/src/hooks/model-fallback/hook.ts +++ b/src/hooks/model-fallback/hook.ts @@ -5,6 +5,7 @@ import { createModelFallbackStateController, type ModelFallbackStateController, } from "./fallback-state-controller" +import type { ModelFallbackControllerAccessor } from "./controller-accessor" type FallbackToast = (input: { title: string @@ -28,26 +29,45 @@ export type ModelFallbackState = { pending: boolean } -const modelFallbackControllerRef: { current?: ModelFallbackStateController } = {} +type ModelFallbackControllerWithState = Pick< + ModelFallbackStateController, + | "lastToastKey" + | "setSessionFallbackChain" + | "clearSessionFallbackChain" + | "setPendingModelFallback" + | "getNextFallback" + | "clearPendingModelFallback" + | "hasPendingModelFallback" + | "getFallbackState" + | "reset" +> -function getOrCreateModelFallbackController(): ModelFallbackStateController { - if (!modelFallbackControllerRef.current) { - createModelFallbackHook() - } - - const controller = modelFallbackControllerRef.current - if (!controller) { - throw new Error("Model fallback controller should be initialized") - } - return controller +export type ModelFallbackHook = ModelFallbackControllerWithState & { + "chat.message": ( + input: ChatMessageInput, + output: ChatMessageHandlerOutput, + ) => Promise } -export function setSessionFallbackChain(sessionID: string, fallbackChain: FallbackEntry[] | undefined): void { - getOrCreateModelFallbackController().setSessionFallbackChain(sessionID, fallbackChain) +type ModelFallbackHookArgs = { + toast?: FallbackToast + onApplied?: FallbackCallback + controllerAccessor?: ModelFallbackControllerAccessor } -export function clearSessionFallbackChain(sessionID: string): void { - getOrCreateModelFallbackController().clearSessionFallbackChain(sessionID) +export function setSessionFallbackChain( + controller: Pick, + sessionID: string, + fallbackChain: FallbackEntry[] | undefined, +): void { + controller.setSessionFallbackChain(sessionID, fallbackChain) +} + +export function clearSessionFallbackChain( + controller: Pick, + sessionID: string, +): void { + controller.clearSessionFallbackChain(sessionID) } /** @@ -55,12 +75,13 @@ export function clearSessionFallbackChain(sessionID: string): void { * Called when a model error is detected in session.error handler. */ export function setPendingModelFallback( + controller: Pick, sessionID: string, agentName: string, currentProviderID: string, currentModelID: string, ): boolean { - return getOrCreateModelFallbackController().setPendingModelFallback( + return controller.setPendingModelFallback( sessionID, agentName, currentProviderID, @@ -73,54 +94,71 @@ export function setPendingModelFallback( * Increments attemptCount each time called. */ export function getNextFallback( + controller: Pick, sessionID: string, ): { providerID: string; modelID: string; variant?: string } | null { - return getOrCreateModelFallbackController().getNextFallback(sessionID) + return controller.getNextFallback(sessionID) } /** * Clears the pending fallback for a session. * Called after fallback is successfully applied. */ -export function clearPendingModelFallback(sessionID: string): void { - getOrCreateModelFallbackController().clearPendingModelFallback(sessionID) +export function clearPendingModelFallback( + controller: Pick, + sessionID: string, +): void { + controller.clearPendingModelFallback(sessionID) } /** * Checks if there's a pending fallback for a session. */ -export function hasPendingModelFallback(sessionID: string): boolean { - return getOrCreateModelFallbackController().hasPendingModelFallback(sessionID) +export function hasPendingModelFallback( + controller: Pick, + sessionID: string, +): boolean { + return controller.hasPendingModelFallback(sessionID) } /** * Gets the current fallback state for a session (for debugging). */ -export function getFallbackState(sessionID: string): ModelFallbackState | undefined { - return getOrCreateModelFallbackController().getFallbackState(sessionID) +export function getFallbackState( + controller: Pick, + sessionID: string, +): ModelFallbackState | undefined { + return controller.getFallbackState(sessionID) } /** * Creates a chat.message hook that applies model fallbacks when pending. */ -export function createModelFallbackHook(args?: { toast?: FallbackToast; onApplied?: FallbackCallback }) { - if (!modelFallbackControllerRef.current) { - const pendingModelFallbacks = new Map() - const lastToastKey = new Map() - const sessionFallbackChains = new Map() +export function createModelFallbackHook(args?: ModelFallbackHookArgs): ModelFallbackHook { + const pendingModelFallbacks = new Map() + const lastToastKey = new Map() + const sessionFallbackChains = new Map() + const controller = createModelFallbackStateController({ + pendingModelFallbacks, + lastToastKey, + sessionFallbackChains, + }) - modelFallbackControllerRef.current = createModelFallbackStateController({ - pendingModelFallbacks, - lastToastKey, - sessionFallbackChains, - }) - } + args?.controllerAccessor?.register(controller) - const controller = getOrCreateModelFallbackController() const toast = args?.toast const onApplied = args?.onApplied return { + lastToastKey: controller.lastToastKey, + setSessionFallbackChain: controller.setSessionFallbackChain, + clearSessionFallbackChain: controller.clearSessionFallbackChain, + setPendingModelFallback: controller.setPendingModelFallback, + getNextFallback: controller.getNextFallback, + clearPendingModelFallback: controller.clearPendingModelFallback, + hasPendingModelFallback: controller.hasPendingModelFallback, + getFallbackState: controller.getFallbackState, + reset: controller.reset, "chat.message": async ( input: ChatMessageInput, output: ChatMessageHandlerOutput, @@ -128,7 +166,7 @@ export function createModelFallbackHook(args?: { toast?: FallbackToast; onApplie const { sessionID } = input if (!sessionID) return - const fallback = getNextFallback(sessionID) + const fallback = getNextFallback(controller, sessionID) if (!fallback) return await applyFallbackToChatMessage({ @@ -144,9 +182,8 @@ export function createModelFallbackHook(args?: { toast?: FallbackToast; onApplie } /** - * Resets all module-global state for testing. - * Clears pending fallbacks, toast keys, and session chains. + * Resets hook-owned state for testing. */ -export function _resetForTesting(): void { - getOrCreateModelFallbackController().reset() +export function _resetForTesting(controller?: Pick): void { + controller?.reset() } diff --git a/src/hooks/model-fallback/index.ts b/src/hooks/model-fallback/index.ts new file mode 100644 index 000000000..08e4c0dd7 --- /dev/null +++ b/src/hooks/model-fallback/index.ts @@ -0,0 +1,2 @@ +export { createModelFallbackControllerAccessor } from "./controller-accessor" +export type { ModelFallbackControllerAccessor } from "./controller-accessor" diff --git a/src/index.ts b/src/index.ts index b41f611e1..2c24e003e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -91,6 +91,7 @@ const serverPlugin: Plugin = async (input, _options): Promise => { pluginConfig, modelCacheState, backgroundManager: managers.backgroundManager, + modelFallbackControllerAccessor: managers.modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled, mergedSkills: toolsResult.mergedSkills, diff --git a/src/plugin/event.model-fallback-2941.test.ts b/src/plugin/event.model-fallback-2941.test.ts index 46765a5d9..2b97d2cb7 100644 --- a/src/plugin/event.model-fallback-2941.test.ts +++ b/src/plugin/event.model-fallback-2941.test.ts @@ -65,13 +65,13 @@ function createChatMessageHandlerHooks(modelFallback: ReturnType void } | undefined let readProviderModelsCacheSpy: { mockRestore: () => void } | undefined -afterEach(() => { - readConnectedProvidersCacheSpy?.mockRestore() - readProviderModelsCacheSpy?.mockRestore() - readConnectedProvidersCacheSpy = undefined - readProviderModelsCacheSpy = undefined - _resetForTesting() -}) + afterEach(() => { + readConnectedProvidersCacheSpy?.mockRestore() + readProviderModelsCacheSpy?.mockRestore() + readConnectedProvidersCacheSpy = undefined + readProviderModelsCacheSpy = undefined + _resetForTesting() + }) describe("createEventHandler - category runtime fallback suppression", () => { test("does not arm retry fallback when category session explicitly stores no fallback chain [regression #2941]", async () => { @@ -83,11 +83,10 @@ describe("createEventHandler - category runtime fallback suppression", () => { readConnectedProvidersCacheSpy = spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(null) readProviderModelsCacheSpy = spyOn(connectedProvidersCache, "readProviderModelsCache").mockReturnValue(null) - clearPendingModelFallback(sessionID) - setSessionAgent(sessionID, "sisyphus-junior") - setSessionFallbackChain(sessionID, undefined) - const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) + setSessionAgent(sessionID, "sisyphus-junior") + setSessionFallbackChain(modelFallback, sessionID, undefined) const eventHandler = createEventHandler({ ctx: asEventHandlerContext({ directory: "/tmp", diff --git a/src/plugin/event.model-fallback.test.ts b/src/plugin/event.model-fallback.test.ts index 3e82817ff..967608f09 100644 --- a/src/plugin/event.model-fallback.test.ts +++ b/src/plugin/event.model-fallback.test.ts @@ -142,9 +142,8 @@ describe("createEventHandler - model fallback", () => { //#given const sessionID = "ses_status_retry_fallback" setMainSession(sessionID) - clearPendingModelFallback(sessionID) - const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) const { handler, abortCalls, promptCalls } = createHandler({ hooks: { modelFallback } }) @@ -232,8 +231,8 @@ describe("createEventHandler - model fallback", () => { //#given const sessionID = "ses_status_retry_dedup" setMainSession(sessionID) - clearPendingModelFallback(sessionID) const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) const { handler, abortCalls, promptCalls } = createHandler({ hooks: { modelFallback } }) await handler({ @@ -293,8 +292,8 @@ describe("createEventHandler - model fallback", () => { //#given const sessionID = "ses_status_retry_runtime_enabled" setMainSession(sessionID) - clearPendingModelFallback(sessionID) const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) const runtimeFallback = { event: async () => {}, "chat.message": async () => {}, @@ -346,9 +345,8 @@ describe("createEventHandler - model fallback", () => { //#given const sessionID = "ses_status_retry_user_fallback" setMainSession(sessionID) - clearPendingModelFallback(sessionID) - const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) const pluginConfig = { agents: { sisyphus: { @@ -446,9 +444,8 @@ describe("createEventHandler - model fallback", () => { const toastCalls: string[] = [] const sessionID = "ses_main_fallback_chain" setMainSession(sessionID) - clearPendingModelFallback(sessionID) - const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) setupConnectedProviderCacheMocks() const eventHandler = createEventHandler({ diff --git a/src/plugin/event.test.ts b/src/plugin/event.test.ts index cb87efff4..ea880c145 100644 --- a/src/plugin/event.test.ts +++ b/src/plugin/event.test.ts @@ -761,11 +761,10 @@ describe("createEventHandler - retry dedupe lifecycle", () => { //#given const sessionID = "ses_retry_recovery_rearm" setMainSession(sessionID) - clearPendingModelFallback(sessionID) - const abortCalls: string[] = [] const promptCalls: string[] = [] const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) const eventHandler = createEventHandler({ ctx: asEventHandlerContext({ diff --git a/src/plugin/event.ts b/src/plugin/event.ts index 6d70d7951..5a5f177b6 100644 --- a/src/plugin/event.ts +++ b/src/plugin/event.ts @@ -15,6 +15,7 @@ import { clearSessionFallbackChain, setSessionFallbackChain, setPendingModelFallback, + type ModelFallbackHook, } from "../hooks/model-fallback/hook"; import { getRawFallbackModels } from "../hooks/runtime-fallback/fallback-models"; import { @@ -111,6 +112,7 @@ function extractProviderModelFromErrorMessage(message: string): { providerID?: s return {}; } function applyUserConfiguredFallbackChain( + modelFallback: Pick | null | undefined, sessionID: string, agentName: string, currentProviderID: string, @@ -123,7 +125,9 @@ function applyUserConfiguredFallbackChain( const fallbackChain = buildFallbackChainFromModels(rawFallbackModels, currentProviderID); if (fallbackChain && fallbackChain.length > 0) { - setSessionFallbackChain(sessionID, fallbackChain); + if (modelFallback) { + setSessionFallbackChain(modelFallback, sessionID, fallbackChain); + } } } @@ -170,6 +174,7 @@ export function createEventHandler(args: { const isModelFallbackEnabled = hooks.modelFallback !== null && hooks.modelFallback !== undefined; + const modelFallback = hooks.modelFallback; // Avoid triggering multiple abort+continue cycles for the same failing assistant message. const lastHandledModelErrorMessageID = new Map(); @@ -408,8 +413,10 @@ export function createEventHandler(args: { lastHandledModelErrorMessageID.delete(sessionInfo.id); lastHandledRetryStatusKey.delete(sessionInfo.id); lastKnownModelBySession.delete(sessionInfo.id); - clearPendingModelFallback(sessionInfo.id); - clearSessionFallbackChain(sessionInfo.id); + if (modelFallback) { + clearPendingModelFallback(modelFallback, sessionInfo.id); + clearSessionFallbackChain(modelFallback, sessionInfo.id); + } resetMessageCursor(sessionInfo.id); clearBackgroundOutputConsumptionsForParentSession(sessionInfo.id); clearBackgroundOutputConsumptionsForTaskSession(sessionInfo.id); @@ -517,9 +524,11 @@ export function createEventHandler(args: { ); const rawModel = (info?.modelID as string | undefined) ?? "claude-opus-4-7"; const currentModel = normalizeFallbackModelID(rawModel); - applyUserConfiguredFallbackChain(sessionID, agentName, currentProvider, args.pluginConfig); + applyUserConfiguredFallbackChain(modelFallback, sessionID, agentName, currentProvider, args.pluginConfig); - const setFallback = setPendingModelFallback(sessionID, agentName, currentProvider, currentModel); + const setFallback = modelFallback + ? setPendingModelFallback(modelFallback, sessionID, agentName, currentProvider, currentModel) + : false; if ( setFallback && @@ -580,9 +589,11 @@ export function createEventHandler(args: { const currentProvider = resolveFallbackProviderID(sessionID, parsed.providerID); let currentModel = parsed.modelID ?? lastKnown?.modelID ?? "claude-opus-4-7"; currentModel = normalizeFallbackModelID(currentModel); - applyUserConfiguredFallbackChain(sessionID, agentName, currentProvider, args.pluginConfig); + applyUserConfiguredFallbackChain(modelFallback, sessionID, agentName, currentProvider, args.pluginConfig); - const setFallback = setPendingModelFallback(sessionID, agentName, currentProvider, currentModel); + const setFallback = modelFallback + ? setPendingModelFallback(modelFallback, sessionID, agentName, currentProvider, currentModel) + : false; if ( setFallback && @@ -666,9 +677,11 @@ export function createEventHandler(args: { ); let currentModel = (props?.modelID as string) || parsed.modelID || "claude-opus-4-7"; currentModel = normalizeFallbackModelID(currentModel); - applyUserConfiguredFallbackChain(sessionID, agentName, currentProvider, args.pluginConfig); + applyUserConfiguredFallbackChain(modelFallback, sessionID, agentName, currentProvider, args.pluginConfig); - const setFallback = setPendingModelFallback(sessionID, agentName, currentProvider, currentModel); + const setFallback = modelFallback + ? setPendingModelFallback(modelFallback, sessionID, agentName, currentProvider, currentModel) + : false; if ( setFallback && diff --git a/src/plugin/fallback.cliproxyapi-matrix.test.ts b/src/plugin/fallback.cliproxyapi-matrix.test.ts index d5e810745..3d1b5fd4c 100644 --- a/src/plugin/fallback.cliproxyapi-matrix.test.ts +++ b/src/plugin/fallback.cliproxyapi-matrix.test.ts @@ -9,7 +9,6 @@ import { createModelFallbackHook } from "../hooks/model-fallback/hook" import { createRuntimeFallbackHook } from "../hooks/runtime-fallback" import type { RuntimeFallbackPluginInput } from "../hooks/runtime-fallback/types" import { _resetForTesting } from "../features/claude-code-session-state" -import { _resetForTesting as _resetModelFallbackForTesting } from "../hooks/model-fallback/hook" import { SessionCategoryRegistry } from "../shared/session-category-registry" import * as connectedProvidersCache from "../shared/connected-providers-cache" @@ -369,7 +368,6 @@ function setupConnectedProviderCacheMocks(): void { afterEach(() => { _resetForTesting() - _resetModelFallbackForTesting() SessionCategoryRegistry.clear() }) diff --git a/src/plugin/hooks/create-core-hooks.ts b/src/plugin/hooks/create-core-hooks.ts index 4da2b5085..5a36aa026 100644 --- a/src/plugin/hooks/create-core-hooks.ts +++ b/src/plugin/hooks/create-core-hooks.ts @@ -1,4 +1,5 @@ import type { HookName, OhMyOpenCodeConfig } from "../../config" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import type { PluginContext } from "../types" import type { ModelCacheState } from "../../plugin-state" @@ -10,15 +11,17 @@ export function createCoreHooks(args: { ctx: PluginContext pluginConfig: OhMyOpenCodeConfig modelCacheState: ModelCacheState + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor isHookEnabled: (hookName: HookName) => boolean safeHookEnabled: boolean }) { - const { ctx, pluginConfig, modelCacheState, isHookEnabled, safeHookEnabled } = args + const { ctx, pluginConfig, modelCacheState, modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled } = args const session = createSessionHooks({ ctx, pluginConfig, modelCacheState, + modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled, }) diff --git a/src/plugin/hooks/create-session-hooks.ts b/src/plugin/hooks/create-session-hooks.ts index af87bd366..9d437bc75 100644 --- a/src/plugin/hooks/create-session-hooks.ts +++ b/src/plugin/hooks/create-session-hooks.ts @@ -1,4 +1,5 @@ import type { OhMyOpenCodeConfig, HookName } from "../../config" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import type { ModelCacheState } from "../../plugin-state" import type { PluginContext } from "../types" @@ -69,10 +70,11 @@ export function createSessionHooks(args: { ctx: PluginContext pluginConfig: OhMyOpenCodeConfig modelCacheState: ModelCacheState + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor isHookEnabled: (hookName: HookName) => boolean safeHookEnabled: boolean }): SessionHooks { - const { ctx, pluginConfig, modelCacheState, isHookEnabled, safeHookEnabled } = args + const { ctx, pluginConfig, modelCacheState, modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled } = args const safeHook = (hookName: HookName, factory: () => T): T | null => safeCreateHook(hookName, factory, { enabled: safeHookEnabled }) @@ -171,6 +173,7 @@ export function createSessionHooks(args: { .catch(() => {}) }, onApplied: enableFallbackTitle ? updateFallbackTitle : undefined, + controllerAccessor: modelFallbackControllerAccessor, })) : null diff --git a/src/plugin/tool-registry.ts b/src/plugin/tool-registry.ts index 6d04e7e1c..a3e46185a 100644 --- a/src/plugin/tool-registry.ts +++ b/src/plugin/tool-registry.ts @@ -144,7 +144,7 @@ export function trimToolsToCap(filteredTools: ToolsRecord, maxTools: number): vo export function createToolRegistry(args: { ctx: PluginContext pluginConfig: OhMyOpenCodeConfig - managers: Pick + managers: Pick skillContext: SkillContext availableCategories: AvailableCategory[] interactiveBashEnabled?: boolean @@ -170,6 +170,7 @@ export function createToolRegistry(args: { pluginConfig.disabled_agents ?? [], pluginConfig.agents, pluginConfig.categories, + managers.modelFallbackControllerAccessor, ) const isMultimodalLookerEnabled = !(pluginConfig.disabled_agents ?? []).some( @@ -191,6 +192,7 @@ export function createToolRegistry(args: { availableSkills: skillContext.availableSkills, sisyphusAgentConfig: pluginConfig.sisyphus_agent, syncPollTimeoutMs: pluginConfig.background_task?.syncPollTimeoutMs, + modelFallbackControllerAccessor: managers.modelFallbackControllerAccessor, onSyncSessionCreated: async (event) => { log("[index] onSyncSessionCreated callback", { sessionID: event.sessionID, diff --git a/src/tools/call-omo-agent/sync-executor.ts b/src/tools/call-omo-agent/sync-executor.ts index 23089d8ea..56e22a80a 100644 --- a/src/tools/call-omo-agent/sync-executor.ts +++ b/src/tools/call-omo-agent/sync-executor.ts @@ -1,7 +1,6 @@ import type { CallOmoAgentArgs } from "./types" import type { PluginInput } from "@opencode-ai/plugin" import { subagentSessions, syncSubagentSessions } from "../../features/claude-code-session-state" -import { clearSessionFallbackChain, setSessionFallbackChain } from "../../hooks/model-fallback/hook" import { getAgentToolRestrictions, log } from "../../shared" import { applySessionPromptParams } from "../../shared/session-prompt-params-helpers" import type { DelegatedModelConfig } from "../../shared/model-resolution-types" @@ -19,8 +18,8 @@ type ExecuteSyncDeps = { createOrGetSession: typeof createOrGetSession waitForCompletion: typeof waitForCompletion processMessages: typeof processMessages - setSessionFallbackChain: typeof setSessionFallbackChain - clearSessionFallbackChain: typeof clearSessionFallbackChain + setSessionFallbackChain: (sessionID: string, fallbackChain: FallbackEntry[] | undefined) => void + clearSessionFallbackChain: (sessionID: string) => void } type SpawnReservation = { @@ -32,8 +31,8 @@ const defaultDeps: ExecuteSyncDeps = { createOrGetSession, waitForCompletion, processMessages, - setSessionFallbackChain, - clearSessionFallbackChain, + setSessionFallbackChain: () => {}, + clearSessionFallbackChain: () => {}, } function buildPromptGenerationParams(model: DelegatedModelConfig | undefined): Record { diff --git a/src/tools/call-omo-agent/tools.ts b/src/tools/call-omo-agent/tools.ts index 839f5abe8..51ea8730c 100644 --- a/src/tools/call-omo-agent/tools.ts +++ b/src/tools/call-omo-agent/tools.ts @@ -2,6 +2,7 @@ import { tool, type PluginInput, type ToolDefinition } from "@opencode-ai/plugin import { ALLOWED_AGENTS, CALL_OMO_AGENT_DESCRIPTION } from "./constants" import type { CallOmoAgentArgs, ToolContextWithMetadata } from "./types" import type { BackgroundManager } from "../../features/background-agent" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import type { CategoriesConfig, AgentOverrides } from "../../config/schema" import type { DelegatedModelConfig } from "../../shared/model-resolution-types" import type { FallbackEntry } from "../../shared/model-requirements" @@ -15,6 +16,23 @@ import { parseModelString } from "../../shared" import { executeBackground } from "./background-executor" import { executeSync } from "./sync-executor" import { resolveCallableAgents } from "./agent-resolver" +import { createOrGetSession } from "./session-creator" +import { processMessages } from "./message-processor" +import { waitForCompletion } from "./completion-poller" + +function createSyncExecutorDeps(modelFallbackControllerAccessor?: ModelFallbackControllerAccessor) { + return { + createOrGetSession, + waitForCompletion, + processMessages, + setSessionFallbackChain: (sessionID: string, fallbackChain: FallbackEntry[] | undefined) => { + modelFallbackControllerAccessor?.setSessionFallbackChain(sessionID, fallbackChain) + }, + clearSessionFallbackChain: (sessionID: string) => { + modelFallbackControllerAccessor?.clearSessionFallbackChain(sessionID) + }, + } +} function resolveModelAndFallbackChain(args: { subagentType: string @@ -82,6 +100,7 @@ export function createCallOmoAgent( disabledAgents: string[] = [], agentOverrides?: AgentOverrides, userCategories?: CategoriesConfig, + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor, ): ToolDefinition { const agentDescriptions = ALLOWED_AGENTS.map( (name) => `- ${name}: Specialized agent for ${name} tasks`, @@ -158,14 +177,30 @@ export function createCallOmoAgent( let spawnReservation: Awaited> | undefined try { spawnReservation = await backgroundManager.reserveSubagentSpawn(toolCtx.sessionID) - return await executeSync(args, toolCtx, ctx, undefined, fallbackChain, spawnReservation, resolvedModel) + return await executeSync( + args, + toolCtx, + ctx, + createSyncExecutorDeps(modelFallbackControllerAccessor), + fallbackChain, + spawnReservation, + resolvedModel, + ) } catch (error) { spawnReservation?.rollback() return `Error: ${error instanceof Error ? error.message : String(error)}` } } - return await executeSync(args, toolCtx, ctx, undefined, fallbackChain, undefined, resolvedModel) + return await executeSync( + args, + toolCtx, + ctx, + createSyncExecutorDeps(modelFallbackControllerAccessor), + fallbackChain, + undefined, + resolvedModel, + ) }, }); } diff --git a/src/tools/delegate-task/background-task.ts b/src/tools/delegate-task/background-task.ts index 6d982992b..d5c4adf5d 100644 --- a/src/tools/delegate-task/background-task.ts +++ b/src/tools/delegate-task/background-task.ts @@ -8,7 +8,6 @@ import { formatDetailedError } from "./error-formatting" import { getSessionTools } from "../../shared/session-tools-store" import { SessionCategoryRegistry } from "../../shared/session-category-registry" import { QUESTION_DENIED_SESSION_PERMISSION } from "../../shared/question-denied-session-permission" -import { setSessionFallbackChain } from "../../hooks/model-fallback/hook" import { stripAgentListSortPrefix } from "../../shared/agent-display-names" import { buildTaskMetadataBlock } from "../../features/tool-metadata-store/task-metadata-contract" import { resolveMetadataModel } from "./resolve-metadata-model" @@ -19,6 +18,7 @@ function continueSessionSetup(args: { timing: ReturnType fallbackChain?: FallbackEntry[] category?: string + modelFallbackControllerAccessor?: ExecutorContext["modelFallbackControllerAccessor"] }): void { if (!args.fallbackChain && !args.category) { return @@ -41,7 +41,7 @@ function continueSessionSetup(args: { continue } - setSessionFallbackChain(sessionId, args.fallbackChain) + args.modelFallbackControllerAccessor?.setSessionFallbackChain(sessionId, args.fallbackChain) if (args.category) { SessionCategoryRegistry.register(sessionId, args.category) } @@ -106,6 +106,7 @@ export async function executeBackgroundTask( timing, fallbackChain, category: args.category, + modelFallbackControllerAccessor: executorCtx.modelFallbackControllerAccessor, }) break } @@ -113,7 +114,7 @@ export async function executeBackgroundTask( } if (sessionId) { - setSessionFallbackChain(sessionId, fallbackChain) + executorCtx.modelFallbackControllerAccessor?.setSessionFallbackChain(sessionId, fallbackChain) } if (args.category && sessionId) { SessionCategoryRegistry.register(sessionId, args.category) diff --git a/src/tools/delegate-task/executor-types.ts b/src/tools/delegate-task/executor-types.ts index bfa7fc70b..8b430c9ce 100644 --- a/src/tools/delegate-task/executor-types.ts +++ b/src/tools/delegate-task/executor-types.ts @@ -1,5 +1,6 @@ import type { BackgroundManager } from "../../features/background-agent" import type { CategoriesConfig, GitMasterConfig, BrowserAutomationProvider, AgentOverrides, SisyphusAgentConfig } from "../../config/schema" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import type { OpencodeClient } from "./types" export interface ExecutorContext { @@ -12,6 +13,7 @@ export interface ExecutorContext { browserProvider?: BrowserAutomationProvider agentOverrides?: AgentOverrides sisyphusAgentConfig?: SisyphusAgentConfig + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor onSyncSessionCreated?: (event: { sessionID: string; parentID: string; title: string }) => Promise syncPollTimeoutMs?: number } diff --git a/src/tools/delegate-task/sync-task.ts b/src/tools/delegate-task/sync-task.ts index 111371a51..034c0e199 100644 --- a/src/tools/delegate-task/sync-task.ts +++ b/src/tools/delegate-task/sync-task.ts @@ -9,7 +9,6 @@ import { SessionCategoryRegistry } from "../../shared/session-category-registry" import { formatDuration } from "./time-formatter" import { formatDetailedError } from "./error-formatting" import { syncTaskDeps, type SyncTaskDeps } from "./sync-task-deps" -import { setSessionFallbackChain, clearSessionFallbackChain } from "../../hooks/model-fallback/hook" import { retrySyncPromptWithFallbacks } from "./sync-task-fallback" import { buildTaskMetadataBlock } from "../../features/tool-metadata-store/task-metadata-contract" import { resolveMetadataModel } from "./resolve-metadata-model" @@ -81,7 +80,7 @@ export async function executeSyncTask( subagentSessions.add(sessionID) syncSubagentSessions.add(sessionID) setSessionAgent(sessionID, agentToUse) - setSessionFallbackChain(sessionID, fallbackChain) + executorCtx.modelFallbackControllerAccessor?.setSessionFallbackChain(sessionID, fallbackChain) if (args.category) { SessionCategoryRegistry.register(sessionID, args.category) @@ -237,7 +236,7 @@ ${buildTaskMetadataBlock({ if (syncSessionID) { subagentSessions.delete(syncSessionID) syncSubagentSessions.delete(syncSessionID) - clearSessionFallbackChain(syncSessionID) + executorCtx.modelFallbackControllerAccessor?.clearSessionFallbackChain(syncSessionID) SessionCategoryRegistry.remove(syncSessionID) } } diff --git a/src/tools/delegate-task/types.ts b/src/tools/delegate-task/types.ts index 987e821a2..9eff782ce 100644 --- a/src/tools/delegate-task/types.ts +++ b/src/tools/delegate-task/types.ts @@ -1,6 +1,7 @@ import type { PluginInput } from "@opencode-ai/plugin" import type { BackgroundManager } from "../../features/background-agent" import type { CategoriesConfig, GitMasterConfig, BrowserAutomationProvider, AgentOverrides, SisyphusAgentConfig } from "../../config/schema" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import type { AvailableCategory, AvailableSkill, @@ -68,6 +69,7 @@ export interface DelegateTaskToolOptions { availableSkills?: AvailableSkill[] agentOverrides?: AgentOverrides sisyphusAgentConfig?: SisyphusAgentConfig + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor onSyncSessionCreated?: (event: SyncSessionCreatedEvent) => Promise syncPollTimeoutMs?: number } From e6f84f713b09156885556faf615befd9c89c33bc Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 02:37:57 +0900 Subject: [PATCH 13/15] refactor(tools): break glob->grep sibling-tool coupling Hoist shared ripgrep CLI resolution helpers (resolveGrepCli, resolveGrepCliWithAutoInstall, GrepBackend, DEFAULT_RG_THREADS, ResolvedCli) out of src/tools/grep/constants.ts into src/shared/ripgrep-cli.ts so they no longer straddle two sibling tool directories. Before: src/tools/glob/constants.ts re-exported from src/tools/grep/constants.ts, violating the project's "tools should not import from sibling tools" rule enforced by .sisyphus/rules/modular-code-enforcement.md. After: both src/tools/glob/ and src/tools/grep/ consume the shared helpers from src/shared/ripgrep-cli.ts. src/tools/grep/constants.ts keeps only the grep-specific UI-exposed constants. --- src/shared/ripgrep-cli.ts | 124 ++++++++++++++++++++++++++++++++++++ src/tools/glob/constants.ts | 2 +- src/tools/grep/cli.ts | 4 +- src/tools/grep/constants.ts | 124 ------------------------------------ src/tools/grep/tools.ts | 2 +- 5 files changed, 129 insertions(+), 127 deletions(-) create mode 100644 src/shared/ripgrep-cli.ts diff --git a/src/shared/ripgrep-cli.ts b/src/shared/ripgrep-cli.ts new file mode 100644 index 000000000..38eaff703 --- /dev/null +++ b/src/shared/ripgrep-cli.ts @@ -0,0 +1,124 @@ +import { spawnSync } from "node:child_process" +import { existsSync } from "node:fs" +import { dirname, join } from "node:path" +import { downloadAndInstallRipgrep, getInstalledRipgrepPath } from "../tools/grep/downloader" +import { getDataDir } from "./data-path" +import { log } from "./logger" +import { PUBLISHED_PACKAGE_NAME } from "./plugin-identity" + +export type GrepBackend = "rg" | "grep" + +export interface ResolvedCli { + path: string + backend: GrepBackend +} + +export const DEFAULT_RG_THREADS = 4 + +let cachedCli: ResolvedCli | null = null +let autoInstallAttempted = false + +function findExecutable(name: string): string | null { + const isWindows = process.platform === "win32" + const cmd = isWindows ? "where" : "which" + + try { + const result = spawnSync(cmd, [name], { encoding: "utf-8", timeout: 5000 }) + if (result.status === 0 && result.stdout.trim()) { + return result.stdout.trim().split("\n")[0] + } + } catch { + // Command execution failed + } + return null +} + +function getOpenCodeBundledRg(): string | null { + const execPath = process.execPath + const execDir = dirname(execPath) + + const isWindows = process.platform === "win32" + const rgName = isWindows ? "rg.exe" : "rg" + + const candidates = [ + join(getDataDir(), "opencode", "bin", rgName), + join(execDir, rgName), + join(execDir, "bin", rgName), + join(execDir, "..", "bin", rgName), + join(execDir, "..", "libexec", rgName), + ] + + for (const candidate of candidates) { + if (existsSync(candidate)) { + return candidate + } + } + + return null +} + +export function resolveGrepCli(): ResolvedCli { + if (cachedCli) { + return cachedCli + } + + const bundledRg = getOpenCodeBundledRg() + if (bundledRg) { + cachedCli = { path: bundledRg, backend: "rg" } + return cachedCli + } + + const systemRg = findExecutable("rg") + if (systemRg) { + cachedCli = { path: systemRg, backend: "rg" } + return cachedCli + } + + const installedRg = getInstalledRipgrepPath() + if (installedRg) { + cachedCli = { path: installedRg, backend: "rg" } + return cachedCli + } + + const grep = findExecutable("grep") + if (grep) { + cachedCli = { path: grep, backend: "grep" } + return cachedCli + } + + cachedCli = { path: "rg", backend: "rg" } + return cachedCli +} + +export async function resolveGrepCliWithAutoInstall(): Promise { + const current = resolveGrepCli() + + if (current.backend === "rg" && current.path !== "rg") { + return current + } + + if (autoInstallAttempted) { + return current + } + + autoInstallAttempted = true + + try { + const rgPath = await downloadAndInstallRipgrep() + cachedCli = { path: rgPath, backend: "rg" } + return cachedCli + } catch (error) { + if (current.backend === "grep") { + log(`[${PUBLISHED_PACKAGE_NAME}] Failed to auto-install ripgrep. Falling back to GNU grep.`, { + error: error instanceof Error ? error.message : String(error), + grep_path: current.path, + }) + } else { + log(`[${PUBLISHED_PACKAGE_NAME}] Failed to auto-install ripgrep and GNU grep was not found.`, { + error: error instanceof Error ? error.message : String(error), + }) + } + + return current + } +} diff --git a/src/tools/glob/constants.ts b/src/tools/glob/constants.ts index 05b5f85f1..8284b3681 100644 --- a/src/tools/glob/constants.ts +++ b/src/tools/glob/constants.ts @@ -1,4 +1,4 @@ -export { resolveGrepCli, resolveGrepCliWithAutoInstall, type GrepBackend, DEFAULT_RG_THREADS } from "../grep/constants" +export { resolveGrepCli, resolveGrepCliWithAutoInstall, type GrepBackend, DEFAULT_RG_THREADS } from "../../shared/ripgrep-cli" export const DEFAULT_TIMEOUT_MS = 60_000 export const DEFAULT_LIMIT = 100 diff --git a/src/tools/grep/cli.ts b/src/tools/grep/cli.ts index bcec98aaa..9f55b1d27 100644 --- a/src/tools/grep/cli.ts +++ b/src/tools/grep/cli.ts @@ -3,13 +3,15 @@ import { resolveGrepCli, type ResolvedCli, type GrepBackend, + DEFAULT_RG_THREADS, +} from "../../shared/ripgrep-cli" +import { DEFAULT_MAX_DEPTH, DEFAULT_MAX_FILESIZE, DEFAULT_MAX_COUNT, DEFAULT_MAX_COLUMNS, DEFAULT_TIMEOUT_MS, DEFAULT_MAX_OUTPUT_BYTES, - DEFAULT_RG_THREADS, RG_SAFETY_FLAGS, GREP_SAFETY_FLAGS, } from "./constants" diff --git a/src/tools/grep/constants.ts b/src/tools/grep/constants.ts index 79db24c6b..f6c913303 100644 --- a/src/tools/grep/constants.ts +++ b/src/tools/grep/constants.ts @@ -1,126 +1,3 @@ -import { existsSync } from "node:fs" -import { join, dirname } from "node:path" -import { spawnSync } from "node:child_process" -import { getInstalledRipgrepPath, downloadAndInstallRipgrep } from "./downloader" -import { getDataDir } from "../../shared/data-path" -import { log } from "../../shared/logger" -import { PUBLISHED_PACKAGE_NAME } from "../../shared/plugin-identity" - -export type GrepBackend = "rg" | "grep" - -export interface ResolvedCli { - path: string - backend: GrepBackend -} - -let cachedCli: ResolvedCli | null = null -let autoInstallAttempted = false - -function findExecutable(name: string): string | null { - const isWindows = process.platform === "win32" - const cmd = isWindows ? "where" : "which" - - try { - const result = spawnSync(cmd, [name], { encoding: "utf-8", timeout: 5000 }) - if (result.status === 0 && result.stdout.trim()) { - return result.stdout.trim().split("\n")[0] - } - } catch { - // Command execution failed - } - return null -} - -function getOpenCodeBundledRg(): string | null { - const execPath = process.execPath - const execDir = dirname(execPath) - - const isWindows = process.platform === "win32" - const rgName = isWindows ? "rg.exe" : "rg" - - const candidates = [ - // OpenCode XDG data path (highest priority - where OpenCode installs rg) - join(getDataDir(), "opencode", "bin", rgName), - // Legacy paths relative to execPath - join(execDir, rgName), - join(execDir, "bin", rgName), - join(execDir, "..", "bin", rgName), - join(execDir, "..", "libexec", rgName), - ] - - for (const candidate of candidates) { - if (existsSync(candidate)) { - return candidate - } - } - - return null -} - -export function resolveGrepCli(): ResolvedCli { - if (cachedCli) return cachedCli - - const bundledRg = getOpenCodeBundledRg() - if (bundledRg) { - cachedCli = { path: bundledRg, backend: "rg" } - return cachedCli - } - - const systemRg = findExecutable("rg") - if (systemRg) { - cachedCli = { path: systemRg, backend: "rg" } - return cachedCli - } - - const installedRg = getInstalledRipgrepPath() - if (installedRg) { - cachedCli = { path: installedRg, backend: "rg" } - return cachedCli - } - - const grep = findExecutable("grep") - if (grep) { - cachedCli = { path: grep, backend: "grep" } - return cachedCli - } - - cachedCli = { path: "rg", backend: "rg" } - return cachedCli -} - -export async function resolveGrepCliWithAutoInstall(): Promise { - const current = resolveGrepCli() - - if (current.backend === "rg" && current.path !== "rg") { - return current - } - - if (autoInstallAttempted) { - return current - } - - autoInstallAttempted = true - - try { - const rgPath = await downloadAndInstallRipgrep() - cachedCli = { path: rgPath, backend: "rg" } - return cachedCli - } catch (error) { - if (current.backend === "grep") { - log(`[${PUBLISHED_PACKAGE_NAME}] Failed to auto-install ripgrep. Falling back to GNU grep.`, { - error: error instanceof Error ? error.message : String(error), - grep_path: current.path, - }) - } else { - log(`[${PUBLISHED_PACKAGE_NAME}] Failed to auto-install ripgrep and GNU grep was not found.`, { - error: error instanceof Error ? error.message : String(error), - }) - } - - return current - } -} - export const DEFAULT_MAX_DEPTH = 20 export const DEFAULT_MAX_FILESIZE = "10M" export const DEFAULT_MAX_COUNT = 500 @@ -128,7 +5,6 @@ export const DEFAULT_MAX_COLUMNS = 1000 export const DEFAULT_CONTEXT = 2 export const DEFAULT_TIMEOUT_MS = 60_000 export const DEFAULT_MAX_OUTPUT_BYTES = 256 * 1024 -export const DEFAULT_RG_THREADS = 4 export const RG_SAFETY_FLAGS = [ "--no-follow", diff --git a/src/tools/grep/tools.ts b/src/tools/grep/tools.ts index eaf8a3972..c40193c56 100644 --- a/src/tools/grep/tools.ts +++ b/src/tools/grep/tools.ts @@ -1,8 +1,8 @@ import { resolve } from "node:path" import type { PluginInput } from "@opencode-ai/plugin" import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool" +import { resolveGrepCliWithAutoInstall } from "../../shared/ripgrep-cli" import { runRg, runRgCount } from "./cli" -import { resolveGrepCliWithAutoInstall } from "./constants" import { formatGrepResult, formatCountResult } from "./result-formatter" export function createGrepTools(ctx: PluginInput): Record { From 81b37dd2ccf0ef92986dcdde00bef7a2ea40d4ed Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 02:52:10 +0900 Subject: [PATCH 14/15] refactor: remove cosmetic OhMyOpenCodePlugin references Post-V1-migration cleanup of the removed symbol's ghost references: - src/index.ts: log prefix '[OhMyOpenCodePlugin]' -> '[oh-my-openagent]' - src/index.test.ts: describe label 'OhMyOpenCodePlugin' -> 'oh-my-openagent plugin module' - src/index.telemetry.test.ts: describe label 'OhMyOpenCodePlugin telemetry isolation' -> 'oh-my-openagent telemetry isolation' - src/shared/log-legacy-plugin-startup-warning.ts: log prefix '[OhMyOpenCodePlugin]' -> '[legacy-migration]' (plus matching test assertion) After these renames 'grep -rn OhMyOpenCodePlugin src/' returns zero matches. Pure cosmetic rename, no behavior change. --- src/index.telemetry.test.ts | 2 +- src/index.test.ts | 2 +- src/index.ts | 2 +- src/shared/log-legacy-plugin-startup-warning.test.ts | 2 +- src/shared/log-legacy-plugin-startup-warning.ts | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/index.telemetry.test.ts b/src/index.telemetry.test.ts index 7f751f594..924a7db2c 100644 --- a/src/index.telemetry.test.ts +++ b/src/index.telemetry.test.ts @@ -106,7 +106,7 @@ function installModuleMocks(): void { })) } -describe("OhMyOpenCodePlugin telemetry isolation", () => { +describe("oh-my-openagent telemetry isolation", () => { beforeEach(() => { mock.restore() installModuleMocks() diff --git a/src/index.test.ts b/src/index.test.ts index 335562cd0..ba7be1363 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -110,7 +110,7 @@ async function importFreshIndexModule(): Promise { return import(`./index?test=${Date.now()}-${Math.random()}`) } -describe("OhMyOpenCodePlugin", () => { +describe("oh-my-openagent plugin module", () => { beforeEach(async () => { mock.restore() installIndexModuleMocks() diff --git a/src/index.ts b/src/index.ts index 2c24e003e..6778427d0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -20,7 +20,7 @@ import { createPluginPostHog, getPostHogDistinctId } from "./shared/posthog" const serverPlugin: Plugin = async (input, _options): Promise => { initConfigContext("opencode", null) - log("[OhMyOpenCodePlugin] ENTRY - plugin loading", { + log("[oh-my-openagent] ENTRY - plugin loading", { directory: input.directory, }) logLegacyPluginStartupWarning() diff --git a/src/shared/log-legacy-plugin-startup-warning.test.ts b/src/shared/log-legacy-plugin-startup-warning.test.ts index 917f40927..76ec3541f 100644 --- a/src/shared/log-legacy-plugin-startup-warning.test.ts +++ b/src/shared/log-legacy-plugin-startup-warning.test.ts @@ -63,7 +63,7 @@ describe("logLegacyPluginStartupWarning", () => { //#then expect(mockLog).toHaveBeenCalledTimes(1) expect(mockLog).toHaveBeenCalledWith( - "[OhMyOpenCodePlugin] Legacy plugin entry detected in OpenCode config", + "[legacy-migration] Legacy plugin entry detected in OpenCode config", { legacyEntries: ["oh-my-opencode", "oh-my-opencode@3.13.1"], suggestedEntries: ["oh-my-openagent", "oh-my-openagent@3.13.1"], diff --git a/src/shared/log-legacy-plugin-startup-warning.ts b/src/shared/log-legacy-plugin-startup-warning.ts index cc8be67e2..d1151b122 100644 --- a/src/shared/log-legacy-plugin-startup-warning.ts +++ b/src/shared/log-legacy-plugin-startup-warning.ts @@ -22,7 +22,7 @@ export function logLegacyPluginStartupWarning(deps: LogLegacyPluginStartupWarnin const suggestedEntries = result.legacyEntries.map(toCanonicalEntry) - logFn("[OhMyOpenCodePlugin] Legacy plugin entry detected in OpenCode config", { + logFn("[legacy-migration] Legacy plugin entry detected in OpenCode config", { legacyEntries: result.legacyEntries, suggestedEntries, hasCanonicalEntry: result.hasCanonicalEntry, From 70ddc01e1050601e4199334e6d5a68f85dfbf589 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 03:01:51 +0900 Subject: [PATCH 15/15] refactor: remove AI slop from refactored files Behavior-preserving cleanup of AI-generated code smells in 5 files authored/moved by this PR: - src/hooks/model-fallback/fallback-state-controller.ts (-47/+47 net reorganization, redundant defensiveness removed) - src/shared/model-string-parser.ts (-4 LOC obvious-comment cleanup) - src/shared/ripgrep-cli.ts (-13 LOC obvious comments + redundant defensive checks) - src/tools/delegate-task/tool-description.ts (-6 LOC) - src/tools/look-at/look-at-input-preparer.ts (-6 LOC) Targets: obvious comments that restate code, over-defensive null checks on guaranteed values, redundant existence checks. No public API signatures changed, no type hints removed, no new abstractions introduced. Full test suite still passes. --- .../fallback-state-controller.ts | 47 +++++++++---------- src/shared/model-string-parser.ts | 8 ++-- src/shared/ripgrep-cli.ts | 26 ++++------ src/tools/delegate-task/tool-description.ts | 20 ++++---- src/tools/look-at/look-at-input-preparer.ts | 11 ++--- 5 files changed, 48 insertions(+), 64 deletions(-) diff --git a/src/hooks/model-fallback/fallback-state-controller.ts b/src/hooks/model-fallback/fallback-state-controller.ts index b2e6831a0..4230bfb0a 100644 --- a/src/hooks/model-fallback/fallback-state-controller.ts +++ b/src/hooks/model-fallback/fallback-state-controller.ts @@ -53,9 +53,7 @@ export function createModelFallbackStateController(input: { ): boolean { const agentKey = getAgentConfigKey(agentName) const requirements = AGENT_MODEL_REQUIREMENTS[agentKey] - const fallbackChain = sessionFallbackChains.has(sessionID) - ? sessionFallbackChains.get(sessionID) - : requirements?.fallbackChain + const fallbackChain = sessionFallbackChains.get(sessionID) ?? requirements?.fallbackChain if (!fallbackChain?.length) { log("[model-fallback] No fallback chain for agent: " + agentName + " (key: " + agentKey + ")") @@ -63,30 +61,31 @@ export function createModelFallbackStateController(input: { } const existing = pendingModelFallbacks.get(sessionID) - if (existing) { - if (existing.pending) { - log("[model-fallback] Pending fallback already armed for session: " + sessionID) - return false - } - existing.providerID = currentProviderID - existing.modelID = currentModelID - existing.pending = true - if (existing.attemptCount >= existing.fallbackChain.length) { - log("[model-fallback] Fallback chain exhausted for session: " + sessionID) - return false - } - log("[model-fallback] Re-armed pending fallback for session: " + sessionID) + if (!existing) { + pendingModelFallbacks.set(sessionID, { + providerID: currentProviderID, + modelID: currentModelID, + fallbackChain, + attemptCount: 0, + pending: true, + }) + log("[model-fallback] Set pending fallback for session: " + sessionID + ", agent: " + agentName) return true } - pendingModelFallbacks.set(sessionID, { - providerID: currentProviderID, - modelID: currentModelID, - fallbackChain, - attemptCount: 0, - pending: true, - }) - log("[model-fallback] Set pending fallback for session: " + sessionID + ", agent: " + agentName) + if (existing.pending) { + log("[model-fallback] Pending fallback already armed for session: " + sessionID) + return false + } + + existing.providerID = currentProviderID + existing.modelID = currentModelID + existing.pending = true + if (existing.attemptCount >= existing.fallbackChain.length) { + log("[model-fallback] Fallback chain exhausted for session: " + sessionID) + return false + } + log("[model-fallback] Re-armed pending fallback for session: " + sessionID) return true } diff --git a/src/shared/model-string-parser.ts b/src/shared/model-string-parser.ts index 820bb3cc3..220bbd880 100644 --- a/src/shared/model-string-parser.ts +++ b/src/shared/model-string-parser.ts @@ -41,13 +41,13 @@ export function parseModelString( const trimmedModel = model.trim() if (!trimmedModel) return undefined - const parts = trimmedModel.split("/") - if (parts.length < 2) { + const separatorIndex = trimmedModel.indexOf("/") + if (separatorIndex === -1) { return undefined } - const providerID = parts[0]?.trim() - const rawModelID = parts.slice(1).join("/").trim() + const providerID = trimmedModel.slice(0, separatorIndex).trim() + const rawModelID = trimmedModel.slice(separatorIndex + 1).trim() if (!providerID || !rawModelID) { return undefined } diff --git a/src/shared/ripgrep-cli.ts b/src/shared/ripgrep-cli.ts index 38eaff703..5f62b3ad7 100644 --- a/src/shared/ripgrep-cli.ts +++ b/src/shared/ripgrep-cli.ts @@ -28,7 +28,7 @@ function findExecutable(name: string): string | null { return result.stdout.trim().split("\n")[0] } } catch { - // Command execution failed + return null } return null } @@ -62,21 +62,9 @@ export function resolveGrepCli(): ResolvedCli { return cachedCli } - const bundledRg = getOpenCodeBundledRg() - if (bundledRg) { - cachedCli = { path: bundledRg, backend: "rg" } - return cachedCli - } - - const systemRg = findExecutable("rg") - if (systemRg) { - cachedCli = { path: systemRg, backend: "rg" } - return cachedCli - } - - const installedRg = getInstalledRipgrepPath() - if (installedRg) { - cachedCli = { path: installedRg, backend: "rg" } + const rgPath = getOpenCodeBundledRg() ?? findExecutable("rg") ?? getInstalledRipgrepPath() + if (rgPath) { + cachedCli = { path: rgPath, backend: "rg" } return cachedCli } @@ -108,14 +96,16 @@ export async function resolveGrepCliWithAutoInstall(): Promise { cachedCli = { path: rgPath, backend: "rg" } return cachedCli } catch (error) { + const message = error instanceof Error ? error.message : String(error) + if (current.backend === "grep") { log(`[${PUBLISHED_PACKAGE_NAME}] Failed to auto-install ripgrep. Falling back to GNU grep.`, { - error: error instanceof Error ? error.message : String(error), + error: message, grep_path: current.path, }) } else { log(`[${PUBLISHED_PACKAGE_NAME}] Failed to auto-install ripgrep and GNU grep was not found.`, { - error: error instanceof Error ? error.message : String(error), + error: message, }) } diff --git a/src/tools/delegate-task/tool-description.ts b/src/tools/delegate-task/tool-description.ts index 48bebc58d..0b2717a82 100644 --- a/src/tools/delegate-task/tool-description.ts +++ b/src/tools/delegate-task/tool-description.ts @@ -13,28 +13,26 @@ export interface DelegateTaskPresentation { export function createDelegateTaskPresentation(options: DelegateTaskToolOptions): DelegateTaskPresentation { const { userCategories } = options const allCategories = mergeCategories(userCategories) - const categoryNames = Object.keys(allCategories) + const categoryEntries = Object.entries(allCategories).map(([name, categoryConfig]) => ({ + name, + categoryConfig, + description: userCategories?.[name]?.description || CATEGORY_DESCRIPTIONS[name], + })) + const categoryNames = categoryEntries.map(({ name }) => name) const categoryExamples = categoryNames.join(", ") const availableCategories: AvailableCategory[] = options.availableCategories - ?? Object.entries(allCategories).map(([name, categoryConfig]) => { - const userDescription = userCategories?.[name]?.description - const builtinDescription = CATEGORY_DESCRIPTIONS[name] - const description = userDescription || builtinDescription || "General tasks" - + ?? categoryEntries.map(({ name, categoryConfig, description }) => { return { name, - description, + description: description || "General tasks", model: categoryConfig.model, } }) const availableSkills: AvailableSkill[] = options.availableSkills ?? [] - const categoryList = categoryNames.map(name => { - const userDescription = userCategories?.[name]?.description - const builtinDescription = CATEGORY_DESCRIPTIONS[name] - const description = userDescription || builtinDescription + const categoryList = categoryEntries.map(({ name, description }) => { return description ? ` - ${name}: ${description}` : ` - ${name}` }).join("\n") diff --git a/src/tools/look-at/look-at-input-preparer.ts b/src/tools/look-at/look-at-input-preparer.ts index e0eef0099..4901eb00f 100644 --- a/src/tools/look-at/look-at-input-preparer.ts +++ b/src/tools/look-at/look-at-input-preparer.ts @@ -101,17 +101,16 @@ export function prepareLookAtInput(args: LookAtArgs): PrepareLookAtInputResult { if (filePath) { let mimeType = inferMimeTypeFromFilePath(filePath) let actualFilePath = filePath - let tempFilePath: string | null = null let tempConversionPath: string | null = null if (needsConversion(mimeType)) { log(`[look_at] Detected unsupported format: ${mimeType}, converting to JPEG...`) try { - tempFilePath = convertImageToJpeg(filePath, mimeType) - tempConversionPath = tempFilePath - actualFilePath = tempFilePath + const convertedFilePath = convertImageToJpeg(filePath, mimeType) + tempConversionPath = convertedFilePath + actualFilePath = convertedFilePath mimeType = "image/jpeg" - log(`[look_at] Conversion successful: ${tempFilePath}`) + log(`[look_at] Conversion successful: ${convertedFilePath}`) } catch (conversionError) { const failedConversionPath = getTemporaryConversionPath(conversionError) if (failedConversionPath) { @@ -139,8 +138,6 @@ export function prepareLookAtInput(args: LookAtArgs): PrepareLookAtInputResult { cleanup() { if (tempConversionPath) { cleanupConvertedImage(tempConversionPath) - } else if (tempFilePath) { - cleanupConvertedImage(tempFilePath) } }, },