diff --git a/docs/examples/coding-focused.jsonc b/docs/examples/coding-focused.jsonc index 0291a589b..631e50ccc 100644 --- a/docs/examples/coding-focused.jsonc +++ b/docs/examples/coding-focused.jsonc @@ -1,50 +1,88 @@ -// oh-my-openagent coding-focused configuration -// Optimized for hands-on coding: Sisyphus + Hephaestus as primary workers. -// Uses stronger models for implementation, lighter models for support agents. { + "$schema": "https://raw.githubusercontent.com/code-yeongyu/oh-my-opencode/dev/assets/oh-my-opencode.schema.json", + + // Optimized for intensive coding sessions. + // Prioritizes deep implementation agents and fast feedback loops. + "agents": { - // Sisyphus with GPT 5.4 — strong coding performance + // Primary orchestrator: aggressive parallel delegation "sisyphus": { - "model": "openai/gpt-5.4", - "variant": "high" + "model": "kimi-for-coding/k2p5", + "ultrawork": { "model": "anthropic/claude-opus-4-6", "variant": "max" }, + "prompt_append": "Delegate heavily to hephaestus for implementation. Parallelize exploration.", }, - // Hephaestus with GPT-5.3-codex — deep autonomous coding + // Heavy lifter: maximum autonomy for coding tasks "hephaestus": { "model": "openai/gpt-5.3-codex", - "variant": "max" + "prompt_append": "You are the primary implementation agent. Own the codebase. Explore, decide, execute. Use LSP and AST-grep aggressively.", + "permission": { "edit": "allow", "bash": { "git": "allow", "test": "allow" } }, }, - // Atlas for orchestration when using /start-work - "atlas": { - "model": "anthropic/claude-sonnet-4-6", - "variant": "max" - }, - - // Prometheus for planning (Opus for best results) + // Lightweight planner: quick planning for coding tasks "prometheus": { - "model": "anthropic/claude-opus-4-6", - "variant": "max" + "model": "opencode/gpt-5-nano", + "prompt_append": "Keep plans concise. Focus on file structure and key decisions.", }, - // Lightweight agents for support tasks - "explore": { - "model": "anthropic/claude-haiku-4-5" - }, - "librarian": { - "model": "opencode-go/kimi-k2.5" - } + // Debugging and architecture + "oracle": { "model": "openai/gpt-5.4", "variant": "high" }, + + // Fast docs lookup + "librarian": { "model": "github-copilot/grok-code-fast-1" }, + + // Rapid codebase navigation + "explore": { "model": "github-copilot/grok-code-fast-1" }, + + // Frontend and visual work + "multimodal-looker": { "model": "google/gemini-3.1-pro" }, + + // Plan review: minimal overhead + "metis": { "model": "opencode/gpt-5-nano" }, + + // Code review focus + "momus": { "prompt_append": "Focus on code quality, edge cases, and test coverage." }, + + // Long-running coding sessions + "atlas": {}, + + // Quick fixes and small tasks + "sisyphus-junior": { "model": "opencode/gpt-5-nano" }, }, "categories": { - "quick": { - "model": "anthropic/claude-sonnet-4-6", - "description": "Fast implementation tasks" + // Trivial changes: fastest possible + "quick": { "model": "opencode/gpt-5-nano" }, + + // Standard coding tasks: good quality, fast + "unspecified-low": { "model": "anthropic/claude-sonnet-4-6" }, + + // Complex refactors: best quality + "unspecified-high": { "model": "openai/gpt-5.3-codex" }, + + // Visual work + "visual-engineering": { "model": "google/gemini-3.1-pro", "variant": "high" }, + + // Deep autonomous work + "deep": { "model": "openai/gpt-5.3-codex" }, + + // Architecture decisions + "ultrabrain": { "model": "openai/gpt-5.4", "variant": "xhigh" }, + }, + + // High concurrency for parallel agent work + "background_task": { + "defaultConcurrency": 8, + "providerConcurrency": { + "anthropic": 5, + "openai": 5, + "google": 10, + "github-copilot": 10, + "opencode": 15, }, - "deep": { - "model": "openai/gpt-5.4", - "variant": "high", - "description": "Complex multi-file changes" - } - } + }, + + // Enable all coding aids + "hashline_edit": true, + "experimental": { "aggressive_truncation": true, "task_system": true }, } diff --git a/docs/examples/default.jsonc b/docs/examples/default.jsonc index 6c654dbbf..2f357e2e4 100644 --- a/docs/examples/default.jsonc +++ b/docs/examples/default.jsonc @@ -1,59 +1,71 @@ -// oh-my-openagent default configuration -// Copy this file to your project root as .opencode/oh-my-openagent.jsonc -// or to ~/.config/opencode/oh-my-openagent.jsonc for global config. -// -// The legacy name oh-my-opencode.jsonc is also supported. { - // Agent model overrides - // Each agent can be configured with a specific model, variant, and prompt. + "$schema": "https://raw.githubusercontent.com/code-yeongyu/oh-my-opencode/dev/assets/oh-my-opencode.schema.json", + + // Balanced defaults for general development. + // Tuned for reliability across diverse tasks without overspending. + "agents": { - // Sisyphus: Main worker agent. Handles coding, debugging, refactoring. - // Best with: Opus (strongest), Sonnet (good), GPT 5.4 (officially supported) + // Main orchestrator: handles delegation and drives tasks to completion "sisyphus": { - "model": "anthropic/claude-sonnet-4-6" + "model": "anthropic/claude-opus-4-6", + "ultrawork": { "model": "anthropic/claude-opus-4-6", "variant": "max" }, }, - // Hephaestus: Deep autonomous worker, optimized for GPT models. - // Best with: GPT-5.3-codex (primary), GPT 5.4 + // Deep autonomous worker: end-to-end implementation "hephaestus": { - "model": "openai/gpt-5.3-codex" + "model": "openai/gpt-5.3-codex", + "prompt_append": "Explore thoroughly, then implement. Prefer small, testable changes.", }, - // Atlas: Orchestrator agent. Reads plans and delegates tasks to Sisyphus-Junior. - // Best with: Opus (recommended), GPT (has optimized prompt) - "atlas": { - "model": "anthropic/claude-sonnet-4-6" - }, - - // Prometheus: Strategic planner. Creates detailed work plans. - // Best with: Opus (recommended) + // Strategic planner: interview mode before execution "prometheus": { - "model": "anthropic/claude-opus-4-6" + "prompt_append": "Always interview first. Validate scope before planning.", }, - // Explore: Codebase explorer (grep, file listing). Lightweight and fast. - "explore": { - "model": "anthropic/claude-haiku-4-5" - }, + // Architecture consultant: complex design and debugging + "oracle": { "model": "openai/gpt-5.4", "variant": "high" }, - // Librarian: External documentation and code search. - "librarian": { - "model": "anthropic/claude-haiku-4-5" - } + // Documentation and code search + "librarian": { "model": "google/gemini-3-flash" }, + + // Fast codebase exploration + "explore": { "model": "github-copilot/grok-code-fast-1" }, + + // Visual tasks: UI/UX, images, diagrams + "multimodal-looker": { "model": "google/gemini-3.1-pro" }, + + // Plan consultant: reviews and improves plans + "metis": {}, + + // Critic and reviewer + "momus": {}, + + // Continuation and long-running task handler + "atlas": {}, + + // Lightweight task executor for simple jobs + "sisyphus-junior": { "model": "opencode/gpt-5-nano" }, }, - // Category configurations for Sisyphus-Junior tasks - // Categories control which model is used when Atlas delegates work. "categories": { - "quick": { - "model": "anthropic/claude-sonnet-4-6", - "variant": "normal", - "description": "Fast tasks: scaffolding, simple fixes, file moves" + "quick": { "model": "opencode/gpt-5-nano" }, + "unspecified-low": { "model": "anthropic/claude-sonnet-4-6" }, + "unspecified-high": { "model": "anthropic/claude-opus-4-6", "variant": "max" }, + "writing": { "model": "google/gemini-3-flash" }, + "visual-engineering": { "model": "google/gemini-3.1-pro", "variant": "high" }, + "deep": { "model": "openai/gpt-5.3-codex" }, + "ultrabrain": { "model": "openai/gpt-5.4", "variant": "xhigh" }, + }, + + // Conservative concurrency for cost control + "background_task": { + "providerConcurrency": { + "anthropic": 3, + "openai": 3, + "google": 5, + "opencode": 10, }, - "deep": { - "model": "anthropic/claude-sonnet-4-6", - "variant": "max", - "description": "Complex tasks: architecture, multi-file refactoring" - } - } + }, + + "experimental": { "aggressive_truncation": true }, } diff --git a/docs/examples/planning-focused.jsonc b/docs/examples/planning-focused.jsonc index f01871292..126ae10fc 100644 --- a/docs/examples/planning-focused.jsonc +++ b/docs/examples/planning-focused.jsonc @@ -1,56 +1,112 @@ -// oh-my-openagent planning-focused configuration -// Optimized for large projects: Prometheus planning → Atlas orchestration. -// Uses Opus for planning and review, Sonnet for implementation. { + "$schema": "https://raw.githubusercontent.com/code-yeongyu/oh-my-opencode/dev/assets/oh-my-opencode.schema.json", + + // Optimized for strategic planning, architecture, and complex project design. + // Prioritizes deep thinking agents and thorough analysis before execution. + "agents": { - // Sisyphus with Sonnet — reliable implementation + // Orchestrator: delegates to planning agents first "sisyphus": { - "model": "anthropic/claude-sonnet-4-6", - "variant": "max" - }, - - // Hephaestus as alternative worker - "hephaestus": { - "model": "openai/gpt-5.3-codex" - }, - - // Atlas with Opus — strong orchestration and task decomposition - "atlas": { "model": "anthropic/claude-opus-4-6", - "variant": "max", - "prompt_append": "Leverage quick & deep agents in parallel when tasks are independent." + "ultrawork": { "model": "anthropic/claude-opus-4-6", "variant": "max" }, + "prompt_append": "Always consult prometheus and atlas for planning. Never rush to implementation.", }, - // Prometheus with Opus — best planning quality + // Implementation: uses planning outputs + "hephaestus": { + "model": "openai/gpt-5.3-codex", + "prompt_append": "Follow established plans precisely. Ask for clarification when plans are ambiguous.", + }, + + // Primary planner: deep interview mode "prometheus": { "model": "anthropic/claude-opus-4-6", - "variant": "max", - "prompt_append": "Leverage quick & deep agents in parallel when tasks are independent." + "thinking": { "type": "enabled", "budgetTokens": 160000 }, + "prompt_append": "Interview extensively. Question assumptions. Build exhaustive plans with milestones, risks, and contingencies. Use deep & quick agents heavily in parallel for research.", }, - // Support agents - "explore": { - "model": "anthropic/claude-haiku-4-5" + // Architecture consultant + "oracle": { + "model": "openai/gpt-5.4", + "variant": "xhigh", + "thinking": { "type": "enabled", "budgetTokens": 120000 }, }, - "librarian": { - "model": "anthropic/claude-haiku-4-5" - } + + // Research and documentation + "librarian": { "model": "google/gemini-3-flash" }, + + // Exploration for research phase + "explore": { "model": "github-copilot/grok-code-fast-1" }, + + // Visual planning and diagrams + "multimodal-looker": { "model": "google/gemini-3.1-pro", "variant": "high" }, + + // Plan review and refinement: heavily utilized + "metis": { + "model": "anthropic/claude-opus-4-6", + "prompt_append": "Critically evaluate plans. Identify gaps, risks, and improvements. Be thorough.", + }, + + // Critic: challenges assumptions + "momus": { + "model": "openai/gpt-5.4", + "prompt_append": "Challenge all assumptions in plans. Look for edge cases, failure modes, and overlooked requirements.", + }, + + // Long-running planning sessions + "atlas": { + "prompt_append": "Preserve context across long planning sessions. Track evolving decisions.", + }, + + // Quick research tasks + "sisyphus-junior": { "model": "opencode/gpt-5-nano" }, }, "categories": { - "quick": { - "model": "anthropic/claude-sonnet-4-6", - "description": "Scaffolding, config changes, simple fixes" - }, - "deep": { - "model": "anthropic/claude-sonnet-4-6", - "variant": "max", - "description": "Core module implementation, refactoring" - }, + "quick": { "model": "opencode/gpt-5-nano" }, + + "unspecified-low": { "model": "anthropic/claude-sonnet-4-6" }, + + // High-effort planning tasks: maximum reasoning "unspecified-high": { - "model": "anthropic/claude-opus-4-6", - "variant": "max", - "description": "High-stakes tasks requiring maximum quality" - } - } + "model": "openai/gpt-5.4", + "variant": "xhigh", + }, + + // Documentation from plans + "writing": { "model": "google/gemini-3-flash" }, + + // Visual architecture + "visual-engineering": { "model": "google/gemini-3.1-pro", "variant": "high" }, + + // Deep research and analysis + "deep": { "model": "openai/gpt-5.3-codex" }, + + // Strategic reasoning + "ultrabrain": { "model": "openai/gpt-5.4", "variant": "xhigh" }, + + // Creative approaches to problems + "artistry": { "model": "google/gemini-3.1-pro", "variant": "high" }, + }, + + // Moderate concurrency: planning is sequential by nature + "background_task": { + "defaultConcurrency": 5, + "staleTimeoutMs": 300000, + "providerConcurrency": { + "anthropic": 3, + "openai": 3, + }, + "modelConcurrency": { + "anthropic/claude-opus-4-6": 2, + "openai/gpt-5.4": 2, + }, + }, + + "sisyphus_agent": { + "planner_enabled": true, + "replace_plan": true, + }, + + "experimental": { "aggressive_truncation": true }, } diff --git a/src/cli/config-manager/add-plugin-to-opencode-config.ts b/src/cli/config-manager/add-plugin-to-opencode-config.ts index 90a78f0ae..19b265ec5 100644 --- a/src/cli/config-manager/add-plugin-to-opencode-config.ts +++ b/src/cli/config-manager/add-plugin-to-opencode-config.ts @@ -41,37 +41,39 @@ export async function addPluginToOpenCodeConfig(currentVersion: string): Promise const config = parseResult.config const plugins = config.plugin ?? [] - // Check for existing plugin (either current or legacy name) - const currentNameIndex = plugins.findIndex( + const canonicalEntries = plugins.filter( (plugin) => plugin === PLUGIN_NAME || plugin.startsWith(`${PLUGIN_NAME}@`) ) - const legacyNameIndex = plugins.findIndex( + const legacyEntries = plugins.filter( (plugin) => plugin === LEGACY_PLUGIN_NAME || plugin.startsWith(`${LEGACY_PLUGIN_NAME}@`) ) + const otherPlugins = plugins.filter( + (plugin) => !(plugin === PLUGIN_NAME || plugin.startsWith(`${PLUGIN_NAME}@`)) + && !(plugin === LEGACY_PLUGIN_NAME || plugin.startsWith(`${LEGACY_PLUGIN_NAME}@`)) + ) - // If either name exists, update to new name - if (currentNameIndex !== -1) { - if (plugins[currentNameIndex] === pluginEntry) { - return { success: true, configPath: path } - } - plugins[currentNameIndex] = pluginEntry - } else if (legacyNameIndex !== -1) { - // Upgrade legacy name to new name - plugins[legacyNameIndex] = pluginEntry + const normalizedPlugins = [...otherPlugins] + + if (canonicalEntries.length > 0) { + normalizedPlugins.push(canonicalEntries[0]) + } else if (legacyEntries.length > 0) { + const versionMatch = legacyEntries[0].match(/@(.+)$/) + const preservedVersion = versionMatch ? versionMatch[1] : null + normalizedPlugins.push(preservedVersion ? `${PLUGIN_NAME}@${preservedVersion}` : pluginEntry) } else { - plugins.push(pluginEntry) + normalizedPlugins.push(pluginEntry) } - config.plugin = plugins + config.plugin = normalizedPlugins if (format === "jsonc") { const content = readFileSync(path, "utf-8") - const pluginArrayRegex = /"plugin"\s*:\s*\[([\s\S]*?)\]/ + const pluginArrayRegex = /((?:"plugin"|plugin)\s*:\s*)\[([\s\S]*?)\]/ const match = content.match(pluginArrayRegex) if (match) { - const formattedPlugins = plugins.map((p) => `"${p}"`).join(",\n ") - const newContent = content.replace(pluginArrayRegex, `"plugin": [\n ${formattedPlugins}\n ]`) + const formattedPlugins = normalizedPlugins.map((p) => `"${p}"`).join(",\n ") + const newContent = content.replace(pluginArrayRegex, `$1[\n ${formattedPlugins}\n ]`) writeFileSync(path, newContent) } else { const newContent = content.replace(/(\{)/, `$1\n "plugin": ["${pluginEntry}"],`) diff --git a/src/cli/config-manager/plugin-detection.test.ts b/src/cli/config-manager/plugin-detection.test.ts index 2d4d69945..e03e63357 100644 --- a/src/cli/config-manager/plugin-detection.test.ts +++ b/src/cli/config-manager/plugin-detection.test.ts @@ -28,10 +28,9 @@ describe("detectCurrentConfig - single package detection", () => { delete process.env.OPENCODE_CONFIG_DIR }) - it("detects oh-my-opencode in plugin array", () => { + it("detects both legacy and canonical plugin entries", () => { // given - const config = { plugin: ["oh-my-opencode"] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + writeFileSync(testConfigPath, JSON.stringify({ plugin: ["oh-my-opencode", "oh-my-openagent@3.11.0"] }, null, 2) + "\n", "utf-8") // when const result = detectCurrentConfig() @@ -40,58 +39,9 @@ describe("detectCurrentConfig - single package detection", () => { expect(result.isInstalled).toBe(true) }) - it("detects oh-my-opencode with version pin", () => { + it("returns false when plugin not present with similar name", () => { // given - const config = { plugin: ["oh-my-opencode@3.11.0"] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") - - // when - const result = detectCurrentConfig() - - // then - expect(result.isInstalled).toBe(true) - }) - - it("detects oh-my-openagent as installed (legacy name)", () => { - // given - const config = { plugin: ["oh-my-openagent"] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") - - // when - const result = detectCurrentConfig() - - // then - expect(result.isInstalled).toBe(true) - }) - - it("detects oh-my-openagent with version pin as installed (legacy name)", () => { - // given - const config = { plugin: ["oh-my-openagent@3.11.0"] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") - - // when - const result = detectCurrentConfig() - - // then - expect(result.isInstalled).toBe(true) - }) - - it("returns false when plugin not present", () => { - // given - const config = { plugin: ["some-other-plugin"] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") - - // when - const result = detectCurrentConfig() - - // then - expect(result.isInstalled).toBe(false) - }) - - it("returns false when plugin not present (even with similar name)", () => { - // given - not exactly oh-my-openagent - const config = { plugin: ["oh-my-openagent-extra"] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + writeFileSync(testConfigPath, JSON.stringify({ plugin: ["oh-my-openagent-extra"] }, null, 2) + "\n", "utf-8") // when const result = detectCurrentConfig() @@ -103,11 +53,7 @@ describe("detectCurrentConfig - single package detection", () => { it("detects OpenCode Go from the existing omo config", () => { // given writeFileSync(testConfigPath, JSON.stringify({ plugin: ["oh-my-opencode"] }, null, 2) + "\n", "utf-8") - writeFileSync( - testOmoConfigPath, - JSON.stringify({ agents: { atlas: { model: "opencode-go/kimi-k2.5" } } }, null, 2) + "\n", - "utf-8", - ) + writeFileSync(testOmoConfigPath, JSON.stringify({ agents: { atlas: { model: "opencode-go/kimi-k2.5" } } }, null, 2) + "\n", "utf-8") // when const result = detectCurrentConfig() @@ -137,10 +83,9 @@ describe("addPluginToOpenCodeConfig - single package writes", () => { delete process.env.OPENCODE_CONFIG_DIR }) - it("keeps oh-my-opencode when it already exists", async () => { + it("writes canonical plugin entry for new installs", async () => { // given - const config = { plugin: ["oh-my-opencode"] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + writeFileSync(testConfigPath, JSON.stringify({}, null, 2) + "\n", "utf-8") // when const result = await addPluginToOpenCodeConfig("3.11.0") @@ -148,13 +93,12 @@ describe("addPluginToOpenCodeConfig - single package writes", () => { // then expect(result.success).toBe(true) const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) - expect(savedConfig.plugin).toContain("oh-my-opencode") + expect(savedConfig.plugin).toEqual(["oh-my-openagent"]) }) - it("replaces version-pinned oh-my-opencode@X.Y.Z", async () => { + it("upgrades a bare legacy plugin entry to canonical", async () => { // given - const config = { plugin: ["oh-my-opencode@3.10.0"] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + writeFileSync(testConfigPath, JSON.stringify({ plugin: ["oh-my-opencode"] }, null, 2) + "\n", "utf-8") // when const result = await addPluginToOpenCodeConfig("3.11.0") @@ -162,14 +106,12 @@ describe("addPluginToOpenCodeConfig - single package writes", () => { // then expect(result.success).toBe(true) const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) - expect(savedConfig.plugin).toContain("oh-my-opencode") - expect(savedConfig.plugin).not.toContain("oh-my-opencode@3.10.0") + expect(savedConfig.plugin).toEqual(["oh-my-openagent"]) }) - it("recognizes oh-my-openagent as already installed (legacy name)", async () => { + it("upgrades a version-pinned legacy entry to canonical", async () => { // given - const config = { plugin: ["oh-my-openagent"] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + writeFileSync(testConfigPath, JSON.stringify({ plugin: ["oh-my-opencode@3.10.0"] }, null, 2) + "\n", "utf-8") // when const result = await addPluginToOpenCodeConfig("3.11.0") @@ -177,15 +119,12 @@ describe("addPluginToOpenCodeConfig - single package writes", () => { // then expect(result.success).toBe(true) const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) - // Should upgrade to new name - expect(savedConfig.plugin).toContain("oh-my-opencode") - expect(savedConfig.plugin).not.toContain("oh-my-openagent") + expect(savedConfig.plugin).toEqual(["oh-my-openagent@3.10.0"]) }) - it("replaces version-pinned oh-my-openagent@X.Y.Z with new name", async () => { + it("removes stale legacy entry when canonical and legacy entries both exist", async () => { // given - const config = { plugin: ["oh-my-openagent@3.10.0"] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + writeFileSync(testConfigPath, JSON.stringify({ plugin: ["oh-my-openagent", "oh-my-opencode"] }, null, 2) + "\n", "utf-8") // when const result = await addPluginToOpenCodeConfig("3.11.0") @@ -193,15 +132,12 @@ describe("addPluginToOpenCodeConfig - single package writes", () => { // then expect(result.success).toBe(true) const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) - // Legacy should be replaced with new name - expect(savedConfig.plugin).toContain("oh-my-opencode") - expect(savedConfig.plugin).not.toContain("oh-my-openagent") + expect(savedConfig.plugin).toEqual(["oh-my-openagent"]) }) - it("adds new plugin when none exists", async () => { + it("preserves a canonical entry when it already exists", async () => { // given - const config = {} - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + writeFileSync(testConfigPath, JSON.stringify({ plugin: ["oh-my-openagent@3.10.0"] }, null, 2) + "\n", "utf-8") // when const result = await addPluginToOpenCodeConfig("3.11.0") @@ -209,20 +145,21 @@ describe("addPluginToOpenCodeConfig - single package writes", () => { // then expect(result.success).toBe(true) const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) - expect(savedConfig.plugin).toContain("oh-my-opencode") + expect(savedConfig.plugin).toEqual(["oh-my-openagent@3.10.0"]) }) - it("adds plugin when plugin array is empty", async () => { + it("rewrites quoted jsonc plugin field in place", async () => { // given - const config = { plugin: [] } - writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + testConfigPath = join(testConfigDir, "opencode.jsonc") + writeFileSync(testConfigPath, '{\n "plugin": ["oh-my-opencode"]\n}\n', "utf-8") // when const result = await addPluginToOpenCodeConfig("3.11.0") // then expect(result.success).toBe(true) - const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) - expect(savedConfig.plugin).toContain("oh-my-opencode") + const savedContent = readFileSync(testConfigPath, "utf-8") + expect(savedContent.includes('"plugin": [\n "oh-my-openagent"\n ]')).toBe(true) + expect(savedContent.includes("oh-my-opencode")).toBe(false) }) }) diff --git a/src/cli/doctor/checks/system.test.ts b/src/cli/doctor/checks/system.test.ts index 74e34038f..163031f13 100644 --- a/src/cli/doctor/checks/system.test.ts +++ b/src/cli/doctor/checks/system.test.ts @@ -1,9 +1,19 @@ +/// + import { beforeEach, describe, expect, it, mock } from "bun:test" +import { PLUGIN_NAME } from "../../../shared" +import type { PluginInfo } from "./system-plugin" + +type SystemModule = typeof import("./system") + +async function importFreshSystemModule(): Promise { + return import(`./system?test=${Date.now()}-${Math.random()}`) +} const mockFindOpenCodeBinary = mock(async () => ({ path: "/usr/local/bin/opencode" })) const mockGetOpenCodeVersion = mock(async () => "1.0.200") -const mockCompareVersions = mock(() => true) -const mockGetPluginInfo = mock(() => ({ +const mockCompareVersions = mock((_leftVersion?: string, _rightVersion?: string) => true) +const mockGetPluginInfo = mock((): PluginInfo => ({ registered: true, entry: "oh-my-opencode", isPinned: false, @@ -18,7 +28,8 @@ const mockGetLoadedPluginVersion = mock(() => ({ expectedVersion: "3.0.0", loadedVersion: "3.1.0", })) -const mockGetLatestPluginVersion = mock(async () => null) +const mockGetLatestPluginVersion = mock(async (_currentVersion: string | null) => null as string | null) +const mockGetSuggestedInstallTag = mock(() => "latest") mock.module("./system-binary", () => ({ findOpenCodeBinary: mockFindOpenCodeBinary, @@ -33,10 +44,9 @@ mock.module("./system-plugin", () => ({ mock.module("./system-loaded-version", () => ({ getLoadedPluginVersion: mockGetLoadedPluginVersion, getLatestPluginVersion: mockGetLatestPluginVersion, + getSuggestedInstallTag: mockGetSuggestedInstallTag, })) -const { checkSystem } = await import("./system?test") - describe("system check", () => { beforeEach(() => { mockFindOpenCodeBinary.mockReset() @@ -45,6 +55,7 @@ describe("system check", () => { mockGetPluginInfo.mockReset() mockGetLoadedPluginVersion.mockReset() mockGetLatestPluginVersion.mockReset() + mockGetSuggestedInstallTag.mockReset() mockFindOpenCodeBinary.mockResolvedValue({ path: "/usr/local/bin/opencode" }) mockGetOpenCodeVersion.mockResolvedValue("1.0.200") @@ -65,10 +76,14 @@ describe("system check", () => { loadedVersion: "3.1.0", }) mockGetLatestPluginVersion.mockResolvedValue(null) + mockGetSuggestedInstallTag.mockReturnValue("latest") }) describe("#given cache directory contains spaces", () => { it("uses a quoted cache directory in mismatch fix command", async () => { + //#given + const { checkSystem } = await importFreshSystemModule() + //#when const result = await checkSystem() @@ -87,9 +102,11 @@ describe("system check", () => { loadedVersion: "3.0.0-canary.1", }) mockGetLatestPluginVersion.mockResolvedValue("3.0.0-canary.2") - mockCompareVersions.mockImplementation((leftVersion: string, rightVersion: string) => { + mockGetSuggestedInstallTag.mockReturnValue("canary") + mockCompareVersions.mockImplementation((leftVersion?: string, rightVersion?: string) => { return !(leftVersion === "3.0.0-canary.1" && rightVersion === "3.0.0-canary.2") }) + const { checkSystem } = await importFreshSystemModule() //#when const result = await checkSystem() @@ -97,8 +114,94 @@ describe("system check", () => { //#then const outdatedIssue = result.issues.find((issue) => issue.title === "Loaded plugin is outdated") expect(outdatedIssue?.fix).toBe( - 'Update: cd "/Users/test/Library/Caches/opencode with spaces" && bun add oh-my-opencode@canary' + `Update: cd "/Users/test/Library/Caches/opencode with spaces" && bun add ${PLUGIN_NAME}@canary` ) }) }) + + describe("#given OpenCode plugin entry uses legacy package name", () => { + it("adds a warning for a bare legacy entry", async () => { + //#given + mockGetPluginInfo.mockReturnValue({ + registered: true, + entry: "oh-my-opencode", + isPinned: false, + pinnedVersion: null, + configPath: null, + isLocalDev: false, + }) + const { checkSystem } = await importFreshSystemModule() + + //#when + const result = await checkSystem() + + //#then + const legacyEntryIssue = result.issues.find((issue) => issue.title === "Using legacy package name") + expect(legacyEntryIssue?.severity).toBe("warning") + expect(legacyEntryIssue?.fix).toBe( + 'Update your opencode.json plugin entry: "oh-my-opencode" → "oh-my-openagent"' + ) + }) + + it("adds a warning for a version-pinned legacy entry", async () => { + //#given + mockGetPluginInfo.mockReturnValue({ + registered: true, + entry: "oh-my-opencode@3.0.0", + isPinned: true, + pinnedVersion: "3.0.0", + configPath: null, + isLocalDev: false, + }) + const { checkSystem } = await importFreshSystemModule() + + //#when + const result = await checkSystem() + + //#then + const legacyEntryIssue = result.issues.find((issue) => issue.title === "Using legacy package name") + expect(legacyEntryIssue?.severity).toBe("warning") + expect(legacyEntryIssue?.fix).toBe( + 'Update your opencode.json plugin entry: "oh-my-opencode@3.0.0" → "oh-my-openagent@3.0.0"' + ) + }) + + it("does not warn for a canonical plugin entry", async () => { + //#given + mockGetPluginInfo.mockReturnValue({ + registered: true, + entry: PLUGIN_NAME, + isPinned: false, + pinnedVersion: null, + configPath: null, + isLocalDev: false, + }) + const { checkSystem } = await importFreshSystemModule() + + //#when + const result = await checkSystem() + + //#then + expect(result.issues.some((issue) => issue.title === "Using legacy package name")).toBe(false) + }) + + it("does not warn for a local-dev legacy entry", async () => { + //#given + mockGetPluginInfo.mockReturnValue({ + registered: true, + entry: "oh-my-opencode", + isPinned: false, + pinnedVersion: null, + configPath: null, + isLocalDev: true, + }) + const { checkSystem } = await importFreshSystemModule() + + //#when + const result = await checkSystem() + + //#then + expect(result.issues.some((issue) => issue.title === "Using legacy package name")).toBe(false) + }) + }) }) diff --git a/src/cli/doctor/checks/system.ts b/src/cli/doctor/checks/system.ts index 32ed7f022..ed58ad04a 100644 --- a/src/cli/doctor/checks/system.ts +++ b/src/cli/doctor/checks/system.ts @@ -83,18 +83,18 @@ export async function checkSystem(): Promise { if (!pluginInfo.registered) { issues.push({ - title: "oh-my-opencode is not registered", + title: `${PLUGIN_NAME} is not registered`, description: "Plugin entry is missing from OpenCode configuration.", - fix: "Run: bunx oh-my-opencode install", + fix: `Run: bunx ${PLUGIN_NAME} install`, severity: "error", affects: ["all agents"], }) } - // Detect legacy package name in plugin config if (pluginInfo.entry && !pluginInfo.isLocalDev) { const isLegacyName = pluginInfo.entry === LEGACY_PLUGIN_NAME || pluginInfo.entry.startsWith(`${LEGACY_PLUGIN_NAME}@`) + if (isLegacyName) { const suggestedEntry = pluginInfo.entry.replace(LEGACY_PLUGIN_NAME, PLUGIN_NAME) issues.push({ @@ -125,7 +125,7 @@ export async function checkSystem(): Promise { issues.push({ title: "Loaded plugin is outdated", description: `Loaded ${systemInfo.loadedVersion}, latest ${latestVersion}.`, - fix: `Update: cd "${loadedInfo.cacheDir}" && bun add oh-my-opencode@${installTag}`, + fix: `Update: cd "${loadedInfo.cacheDir}" && bun add ${PLUGIN_NAME}@${installTag}`, severity: "warning", affects: ["plugin features"], }) diff --git a/src/cli/doctor/constants.ts b/src/cli/doctor/constants.ts index ff41f836e..9afaf5a88 100644 --- a/src/cli/doctor/constants.ts +++ b/src/cli/doctor/constants.ts @@ -1,4 +1,5 @@ import color from "picocolors" +import { PLUGIN_NAME } from "../../shared" export const SYMBOLS = { check: color.green("\u2713"), @@ -38,6 +39,6 @@ export const EXIT_CODES = { export const MIN_OPENCODE_VERSION = "1.0.150" -export const PACKAGE_NAME = "oh-my-opencode" +export const PACKAGE_NAME = PLUGIN_NAME export const OPENCODE_BINARIES = ["opencode", "opencode-desktop"] as const diff --git a/src/cli/install.test.ts b/src/cli/install.test.ts index e967ed6b4..94a891832 100644 --- a/src/cli/install.test.ts +++ b/src/cli/install.test.ts @@ -113,10 +113,10 @@ describe("install CLI - binary check behavior", () => { const configPath = join(tempDir, "opencode.json") expect(existsSync(configPath)).toBe(true) - // then opencode.json should have plugin entry const config = JSON.parse(readFileSync(configPath, "utf-8")) expect(config.plugin).toBeDefined() - expect(config.plugin.some((p: string) => p.includes("oh-my-opencode"))).toBe(true) + expect(config.plugin.some((p: string) => p.includes("oh-my-openagent"))).toBe(true) + expect(config.plugin.some((p: string) => p.includes("oh-my-opencode"))).toBe(false) // then exit code should be 0 (success) expect(exitCode).toBe(0) diff --git a/src/plugin/chat-params.test.ts b/src/plugin/chat-params.test.ts index 511394a75..622b38374 100644 --- a/src/plugin/chat-params.test.ts +++ b/src/plugin/chat-params.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, test } from "bun:test" -import { createChatParamsHandler } from "./chat-params" +import { createChatParamsHandler, type ChatParamsOutput } from "./chat-params" import { clearSessionPromptParams, getSessionPromptParams, @@ -79,7 +79,7 @@ describe("createChatParamsHandler", () => { test("applies stored prompt params for the session", async () => { //#given - setSessionPromptParams("ses_chat_params", { + setSessionPromptParams("ses_chat_params_temperature", { temperature: 0.4, topP: 0.7, options: { @@ -94,14 +94,14 @@ describe("createChatParamsHandler", () => { }) const input = { - sessionID: "ses_chat_params", + sessionID: "ses_chat_params_temperature", agent: { name: "oracle" }, model: { providerID: "openai", modelID: "gpt-5.4" }, provider: { id: "openai" }, message: {}, } - const output = { + const output: ChatParamsOutput = { temperature: 0.1, topP: 1, topK: 1, @@ -113,6 +113,7 @@ describe("createChatParamsHandler", () => { //#then expect(output).toEqual({ + temperature: 0.4, topP: 0.7, topK: 1, options: { @@ -122,7 +123,7 @@ describe("createChatParamsHandler", () => { maxTokens: 4096, }, }) - expect(getSessionPromptParams("ses_chat_params")).toEqual({ + expect(getSessionPromptParams("ses_chat_params_temperature")).toEqual({ temperature: 0.4, topP: 0.7, options: { @@ -133,9 +134,9 @@ describe("createChatParamsHandler", () => { }) }) - test("drops unsupported temperature and clamps maxTokens from bundled model capabilities", async () => { + test("preserves gpt-5.4 temperature and clamps maxTokens from bundled model capabilities", async () => { //#given - setSessionPromptParams("ses_chat_params", { + setSessionPromptParams("ses_chat_params_temperature", { temperature: 0.7, options: { maxTokens: 200_000, @@ -147,7 +148,7 @@ describe("createChatParamsHandler", () => { }) const input = { - sessionID: "ses_chat_params", + sessionID: "ses_chat_params_temperature", agent: { name: "oracle" }, model: { providerID: "openai", modelID: "gpt-5.4" }, provider: { id: "openai" }, @@ -166,6 +167,7 @@ describe("createChatParamsHandler", () => { //#then expect(output).toEqual({ + temperature: 0.7, topP: 1, topK: 1, options: { diff --git a/src/shared/legacy-plugin-warning.test.ts b/src/shared/legacy-plugin-warning.test.ts new file mode 100644 index 000000000..adbfcfa8b --- /dev/null +++ b/src/shared/legacy-plugin-warning.test.ts @@ -0,0 +1,97 @@ +import { afterEach, beforeEach, describe, expect, it } from "bun:test" +import { mkdirSync, rmSync, writeFileSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { checkForLegacyPluginEntry } from "./legacy-plugin-warning" + +describe("checkForLegacyPluginEntry", () => { + let testConfigDir = "" + let originalXdgConfigHome: string | undefined + let originalOpenCodeConfigDir: string | undefined + + beforeEach(() => { + originalXdgConfigHome = process.env.XDG_CONFIG_HOME + originalOpenCodeConfigDir = process.env.OPENCODE_CONFIG_DIR + testConfigDir = join(tmpdir(), `omo-legacy-check-${Date.now()}-${Math.random().toString(36).slice(2)}`) + mkdirSync(join(testConfigDir, "opencode"), { recursive: true }) + process.env.XDG_CONFIG_HOME = testConfigDir + delete process.env.OPENCODE_CONFIG_DIR + }) + + afterEach(() => { + if (originalXdgConfigHome === undefined) { + delete process.env.XDG_CONFIG_HOME + } else { + process.env.XDG_CONFIG_HOME = originalXdgConfigHome + } + + if (originalOpenCodeConfigDir === undefined) { + delete process.env.OPENCODE_CONFIG_DIR + } else { + process.env.OPENCODE_CONFIG_DIR = originalOpenCodeConfigDir + } + + rmSync(testConfigDir, { recursive: true, force: true }) + }) + + it("detects a bare legacy plugin entry", () => { + // given + writeFileSync(join(testConfigDir, "opencode", "opencode.json"), JSON.stringify({ plugin: ["oh-my-opencode"] }, null, 2)) + + // when + const result = checkForLegacyPluginEntry() + + // then + expect(result.hasLegacyEntry).toBe(true) + expect(result.hasCanonicalEntry).toBe(false) + expect(result.legacyEntries).toEqual(["oh-my-opencode"]) + }) + + it("detects a version-pinned legacy plugin entry", () => { + // given + writeFileSync(join(testConfigDir, "opencode", "opencode.json"), JSON.stringify({ plugin: ["oh-my-opencode@3.10.0"] }, null, 2)) + + // when + const result = checkForLegacyPluginEntry() + + // then + expect(result.hasLegacyEntry).toBe(true) + expect(result.hasCanonicalEntry).toBe(false) + expect(result.legacyEntries).toEqual(["oh-my-opencode@3.10.0"]) + }) + + it("does not flag a canonical plugin entry", () => { + // given + writeFileSync(join(testConfigDir, "opencode", "opencode.json"), JSON.stringify({ plugin: ["oh-my-openagent"] }, null, 2)) + + // when + const result = checkForLegacyPluginEntry() + + // then + expect(result.hasLegacyEntry).toBe(false) + expect(result.hasCanonicalEntry).toBe(true) + expect(result.legacyEntries).toEqual([]) + }) + + it("detects legacy entries in quoted jsonc config", () => { + // given + writeFileSync(join(testConfigDir, "opencode", "opencode.jsonc"), '{\n "plugin": ["oh-my-opencode"]\n}\n') + + // when + const result = checkForLegacyPluginEntry() + + // then + expect(result.hasLegacyEntry).toBe(true) + expect(result.legacyEntries).toEqual(["oh-my-opencode"]) + }) + + it("returns no warning data when config is missing", () => { + // when + const result = checkForLegacyPluginEntry() + + // then + expect(result.hasLegacyEntry).toBe(false) + expect(result.hasCanonicalEntry).toBe(false) + expect(result.legacyEntries).toEqual([]) + }) +}) diff --git a/src/shared/legacy-plugin-warning.ts b/src/shared/legacy-plugin-warning.ts new file mode 100644 index 000000000..5e97a764f --- /dev/null +++ b/src/shared/legacy-plugin-warning.ts @@ -0,0 +1,57 @@ +import { existsSync, readFileSync } from "node:fs" + +import { parseJsoncSafe } from "./jsonc-parser" +import { getOpenCodeConfigPaths } from "./opencode-config-dir" +import { LEGACY_PLUGIN_NAME, PLUGIN_NAME } from "./plugin-identity" + +interface OpenCodeConfig { + plugin?: string[] +} + +export interface LegacyPluginCheckResult { + hasLegacyEntry: boolean + hasCanonicalEntry: boolean + legacyEntries: string[] +} + +function getOpenCodeConfigPath(): string | null { + const { configJsonc, configJson } = getOpenCodeConfigPaths({ binary: "opencode", version: null }) + + if (existsSync(configJsonc)) return configJsonc + if (existsSync(configJson)) return configJson + return null +} + +function isLegacyPluginEntry(entry: string): boolean { + return entry === LEGACY_PLUGIN_NAME || entry.startsWith(`${LEGACY_PLUGIN_NAME}@`) +} + +function isCanonicalPluginEntry(entry: string): boolean { + return entry === PLUGIN_NAME || entry.startsWith(`${PLUGIN_NAME}@`) +} + +export function checkForLegacyPluginEntry(): LegacyPluginCheckResult { + const configPath = getOpenCodeConfigPath() + if (!configPath) { + return { hasLegacyEntry: false, hasCanonicalEntry: false, legacyEntries: [] } + } + + try { + const content = readFileSync(configPath, "utf-8") + const parseResult = parseJsoncSafe(content) + if (!parseResult.data) { + return { hasLegacyEntry: false, hasCanonicalEntry: false, legacyEntries: [] } + } + + const legacyEntries = (parseResult.data.plugin ?? []).filter(isLegacyPluginEntry) + const hasCanonicalEntry = (parseResult.data.plugin ?? []).some(isCanonicalPluginEntry) + + return { + hasLegacyEntry: legacyEntries.length > 0, + hasCanonicalEntry, + legacyEntries, + } + } catch { + return { hasLegacyEntry: false, hasCanonicalEntry: false, legacyEntries: [] } + } +} diff --git a/src/shared/model-capabilities.test.ts b/src/shared/model-capabilities.test.ts index 35dc40f8b..5bdadcb80 100644 --- a/src/shared/model-capabilities.test.ts +++ b/src/shared/model-capabilities.test.ts @@ -233,13 +233,13 @@ describe("getModelCapabilities", () => { expect(result).toMatchObject({ canonicalModelID: "gpt-5.4", - maxOutputTokens: 64_000, - supportsTemperature: false, + maxOutputTokens: 128_000, + supportsTemperature: true, }) expect(result.diagnostics).toMatchObject({ snapshot: { source: "runtime-snapshot" }, - maxOutputTokens: { source: "runtime-snapshot" }, - supportsTemperature: { source: "runtime-snapshot" }, + maxOutputTokens: { source: "runtime" }, + supportsTemperature: { source: "runtime" }, }) }) diff --git a/src/shared/plugin-identity.test.ts b/src/shared/plugin-identity.test.ts index b3bd87cba..6de3c7bdb 100644 --- a/src/shared/plugin-identity.test.ts +++ b/src/shared/plugin-identity.test.ts @@ -3,24 +3,24 @@ import { PLUGIN_NAME, CONFIG_BASENAME, LOG_FILENAME, CACHE_DIR_NAME } from "./pl describe("plugin-identity constants", () => { describe("PLUGIN_NAME", () => { - it("equals oh-my-opencode", () => { + it("equals oh-my-openagent", () => { // given // when // then - expect(PLUGIN_NAME).toBe("oh-my-opencode") + expect(PLUGIN_NAME).toBe("oh-my-openagent") }) }) describe("CONFIG_BASENAME", () => { - it("equals oh-my-opencode", () => { + it("equals oh-my-openagent", () => { // given // when // then - expect(CONFIG_BASENAME).toBe("oh-my-opencode") + expect(CONFIG_BASENAME).toBe("oh-my-openagent") }) }) diff --git a/src/shared/plugin-identity.ts b/src/shared/plugin-identity.ts index 3de14c328..4150283b2 100644 --- a/src/shared/plugin-identity.ts +++ b/src/shared/plugin-identity.ts @@ -1,5 +1,6 @@ -export const PLUGIN_NAME = "oh-my-opencode" -export const LEGACY_PLUGIN_NAME = "oh-my-openagent" -export const CONFIG_BASENAME = "oh-my-opencode" +export const PLUGIN_NAME = "oh-my-openagent" +export const LEGACY_PLUGIN_NAME = "oh-my-opencode" +export const CONFIG_BASENAME = "oh-my-openagent" +export const LEGACY_CONFIG_BASENAME = "oh-my-opencode" export const LOG_FILENAME = "oh-my-opencode.log" export const CACHE_DIR_NAME = "oh-my-opencode" diff --git a/src/tools/look-at/image-converter.test.ts b/src/tools/look-at/image-converter.test.ts index 8acab97b6..12efaacc3 100644 --- a/src/tools/look-at/image-converter.test.ts +++ b/src/tools/look-at/image-converter.test.ts @@ -1,22 +1,14 @@ -import { describe, expect, test, mock, beforeEach } from "bun:test" +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test" +import * as childProcess from "node:child_process" import { existsSync, mkdtempSync, writeFileSync, unlinkSync, rmSync } from "node:fs" import { tmpdir } from "node:os" import { dirname, join } from "node:path" -const originalChildProcess = await import("node:child_process") +type ImageConverterModule = typeof import("./image-converter") -const execFileSyncMock = mock((_command: string, _args: string[], _options?: unknown) => "") -const execSyncMock = mock(() => { - throw new Error("execSync should not be called") -}) - -mock.module("node:child_process", () => ({ - ...originalChildProcess, - execFileSync: execFileSyncMock, - execSync: execSyncMock, -})) - -const { convertImageToJpeg, cleanupConvertedImage } = await import("./image-converter") +async function loadImageConverter(): Promise { + return import(`./image-converter?test=${Date.now()}-${Math.random()}`) +} function writeConvertedOutput(command: string, args: string[]): void { if (command === "sips") { @@ -38,7 +30,10 @@ function writeConvertedOutput(command: string, args: string[]): void { } } -function withMockPlatform(platform: NodeJS.Platform, run: () => TValue): TValue { +async function withMockPlatform( + platform: NodeJS.Platform, + run: () => TValue | Promise, +): Promise { const originalPlatform = process.platform Object.defineProperty(process, "platform", { value: platform, @@ -46,7 +41,7 @@ function withMockPlatform(platform: NodeJS.Platform, run: () => TValue): }) try { - return run() + return await run() } finally { Object.defineProperty(process, "platform", { value: originalPlatform, @@ -56,34 +51,50 @@ function withMockPlatform(platform: NodeJS.Platform, run: () => TValue): } describe("image-converter command execution safety", () => { + let execFileSyncSpy: ReturnType + let execSyncSpy: ReturnType + beforeEach(() => { - execFileSyncMock.mockReset() - execSyncMock.mockReset() + execSyncSpy = spyOn(childProcess, "execSync").mockImplementation(() => { + throw new Error("execSync should not be called") + }) + + execFileSyncSpy = spyOn(childProcess, "execFileSync").mockImplementation( + ((_command: string, _args: string[], _options?: unknown) => "") as typeof childProcess.execFileSync, + ) }) - test("uses execFileSync with argument arrays for conversion commands", () => { + afterEach(() => { + execFileSyncSpy.mockRestore() + execSyncSpy.mockRestore() + }) + + test("uses execFileSync with argument arrays for conversion commands", async () => { const testDir = mkdtempSync(join(tmpdir(), "img-converter-test-")) const inputPath = join(testDir, "evil$(touch_pwn).heic") writeFileSync(inputPath, "fake-heic-data") + const { convertImageToJpeg } = await loadImageConverter() - execFileSyncMock.mockImplementation((command: string, args: string[]) => { - writeConvertedOutput(command, args) - return "" - }) + execFileSyncSpy.mockImplementation( + ((command: string, args: string[]) => { + writeConvertedOutput(command, args) + return "" + }) as typeof childProcess.execFileSync, + ) const outputPath = convertImageToJpeg(inputPath, "image/heic") - expect(execSyncMock).not.toHaveBeenCalled() - expect(execFileSyncMock).toHaveBeenCalled() + expect(execSyncSpy).not.toHaveBeenCalled() + expect(execFileSyncSpy).toHaveBeenCalled() - const [firstCommand, firstArgs] = execFileSyncMock.mock.calls[0] as [string, string[]] + const [firstCommand, firstArgs] = execFileSyncSpy.mock.calls[0] as [string, string[]] expect(typeof firstCommand).toBe("string") expect(Array.isArray(firstArgs)).toBe(true) expect(["sips", "convert", "magick"]).toContain(firstCommand) expect(firstArgs).toContain("--") expect(firstArgs).toContain(inputPath) expect(firstArgs.indexOf("--") < firstArgs.indexOf(inputPath)).toBe(true) - expect(firstArgs.join(" ")).not.toContain(`\"${inputPath}\"`) + expect(firstArgs.join(" ")).not.toContain(`"${inputPath}"`) expect(existsSync(outputPath)).toBe(true) @@ -92,15 +103,18 @@ describe("image-converter command execution safety", () => { rmSync(testDir, { recursive: true, force: true }) }) - test("removes temporary conversion directory during cleanup", () => { + test("removes temporary conversion directory during cleanup", async () => { const testDir = mkdtempSync(join(tmpdir(), "img-converter-cleanup-test-")) const inputPath = join(testDir, "photo.heic") writeFileSync(inputPath, "fake-heic-data") + const { convertImageToJpeg, cleanupConvertedImage } = await loadImageConverter() - execFileSyncMock.mockImplementation((command: string, args: string[]) => { - writeConvertedOutput(command, args) - return "" - }) + execFileSyncSpy.mockImplementation( + ((command: string, args: string[]) => { + writeConvertedOutput(command, args) + return "" + }) as typeof childProcess.execFileSync, + ) const outputPath = convertImageToJpeg(inputPath, "image/heic") const conversionDirectory = dirname(outputPath) @@ -115,22 +129,25 @@ describe("image-converter command execution safety", () => { rmSync(testDir, { recursive: true, force: true }) }) - test("uses magick command on non-darwin platforms to avoid convert.exe collision", () => { - withMockPlatform("linux", () => { + test("uses magick command on non-darwin platforms to avoid convert.exe collision", async () => { + await withMockPlatform("linux", async () => { const testDir = mkdtempSync(join(tmpdir(), "img-converter-platform-test-")) const inputPath = join(testDir, "photo.heic") writeFileSync(inputPath, "fake-heic-data") + const { convertImageToJpeg, cleanupConvertedImage } = await loadImageConverter() - execFileSyncMock.mockImplementation((command: string, args: string[]) => { - if (command === "magick") { - writeFileSync(args[2], "jpeg") - } - return "" - }) + execFileSyncSpy.mockImplementation( + ((command: string, args: string[]) => { + if (command === "magick") { + writeFileSync(args[2], "jpeg") + } + return "" + }) as typeof childProcess.execFileSync, + ) const outputPath = convertImageToJpeg(inputPath, "image/heic") - const [command, args] = execFileSyncMock.mock.calls[0] as [string, string[]] + const [command, args] = execFileSyncSpy.mock.calls[0] as [string, string[]] expect(command).toBe("magick") expect(args).toContain("--") expect(args.indexOf("--") < args.indexOf(inputPath)).toBe(true) @@ -142,19 +159,22 @@ describe("image-converter command execution safety", () => { }) }) - test("applies timeout when executing conversion commands", () => { + test("applies timeout when executing conversion commands", async () => { const testDir = mkdtempSync(join(tmpdir(), "img-converter-timeout-test-")) const inputPath = join(testDir, "photo.heic") writeFileSync(inputPath, "fake-heic-data") + const { convertImageToJpeg, cleanupConvertedImage } = await loadImageConverter() - execFileSyncMock.mockImplementation((command: string, args: string[]) => { - writeConvertedOutput(command, args) - return "" - }) + execFileSyncSpy.mockImplementation( + ((command: string, args: string[]) => { + writeConvertedOutput(command, args) + return "" + }) as typeof childProcess.execFileSync, + ) const outputPath = convertImageToJpeg(inputPath, "image/heic") - const options = execFileSyncMock.mock.calls[0]?.[2] as { timeout?: number } | undefined + const options = execFileSyncSpy.mock.calls[0]?.[2] as { timeout?: number } | undefined expect(options).toBeDefined() expect(typeof options?.timeout).toBe("number") expect((options?.timeout ?? 0) > 0).toBe(true) @@ -164,15 +184,16 @@ describe("image-converter command execution safety", () => { rmSync(testDir, { recursive: true, force: true }) }) - test("attaches temporary output path to conversion errors", () => { - withMockPlatform("linux", () => { + test("attaches temporary output path to conversion errors", async () => { + await withMockPlatform("linux", async () => { const testDir = mkdtempSync(join(tmpdir(), "img-converter-failure-test-")) const inputPath = join(testDir, "photo.heic") writeFileSync(inputPath, "fake-heic-data") + const { convertImageToJpeg } = await loadImageConverter() - execFileSyncMock.mockImplementation(() => { + execFileSyncSpy.mockImplementation((() => { throw new Error("conversion process failed") - }) + }) as typeof childProcess.execFileSync) const runConversion = () => convertImageToJpeg(inputPath, "image/heic") expect(runConversion).toThrow("No image conversion tool available") diff --git a/src/tools/look-at/image-converter.ts b/src/tools/look-at/image-converter.ts index fe3b6be05..163e35e28 100644 --- a/src/tools/look-at/image-converter.ts +++ b/src/tools/look-at/image-converter.ts @@ -1,4 +1,4 @@ -import { execFileSync } from "node:child_process" +import * as childProcess from "node:child_process" import { existsSync, mkdtempSync, readFileSync, rmSync, unlinkSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import { dirname, join } from "node:path" @@ -59,7 +59,7 @@ export function convertImageToJpeg(inputPath: string, mimeType: string): string try { if (process.platform === "darwin") { try { - execFileSync("sips", ["-s", "format", "jpeg", "--", inputPath, "--out", outputPath], { + childProcess.execFileSync("sips", ["-s", "format", "jpeg", "--", inputPath, "--out", outputPath], { stdio: "pipe", encoding: "utf-8", timeout: CONVERSION_TIMEOUT_MS, @@ -76,7 +76,7 @@ export function convertImageToJpeg(inputPath: string, mimeType: string): string try { const imagemagickCommand = process.platform === "darwin" ? "convert" : "magick" - execFileSync(imagemagickCommand, ["--", inputPath, outputPath], { + childProcess.execFileSync(imagemagickCommand, ["--", inputPath, outputPath], { stdio: "pipe", encoding: "utf-8", timeout: CONVERSION_TIMEOUT_MS,