diff --git a/script/package-layout-exclusion.test.ts b/script/package-layout-exclusion.test.ts index 1fb0e37d3..033b7ee56 100644 --- a/script/package-layout-exclusion.test.ts +++ b/script/package-layout-exclusion.test.ts @@ -1,6 +1,6 @@ /// -import { afterAll, beforeAll, describe, expect, test } from "bun:test" +import { afterAll, beforeAll, describe, expect, setDefaultTimeout, test } from "bun:test" import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs" import { dirname, join, relative, sep } from "node:path" import { fileURLToPath } from "node:url" @@ -25,13 +25,16 @@ const fakeInternalArtifactCleanupPaths = [ ...fakeInternalSkillArtifactRootPaths, ...fakeInternalCommandArtifactPaths, ] as const +const packageLayoutTestTimeoutMs = 60_000 + +setDefaultTimeout(packageLayoutTestTimeoutMs) let originalPackageJsonText: string | null = null let packageJsonWasTemporarilyModified = false class PackDryRunError extends Error { constructor(readonly exitCode: number, readonly stderr: string) { - super(`bun pm pack --dry-run failed with exit code ${exitCode}: ${stderr}`) + super(`bun pm pack --dry-run --ignore-scripts failed with exit code ${exitCode}: ${stderr}`) this.name = "PackDryRunError" } } @@ -90,7 +93,7 @@ function parsePackedPaths(output: string): Set { async function packDryRunPaths(): Promise> { const packProcess = Bun.spawn({ - cmd: ["bun", "pm", "pack", "--dry-run"], + cmd: ["bun", "pm", "pack", "--dry-run", "--ignore-scripts"], cwd: repositoryRoot, stdout: "pipe", stderr: "pipe", diff --git a/script/package-layout.test.ts b/script/package-layout.test.ts index a4f3c18fd..5f7c759f3 100644 --- a/script/package-layout.test.ts +++ b/script/package-layout.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "bun:test" +import { describe, expect, setDefaultTimeout, test } from "bun:test" import { existsSync, readdirSync } from "node:fs" import { join, relative, sep } from "node:path" import { fileURLToPath } from "node:url" @@ -6,10 +6,13 @@ import { fileURLToPath } from "node:url" const repositoryRoot = fileURLToPath(new URL("..", import.meta.url)) const commandRoots = [".opencode/command", ".agents/command"] as const const skillRoots = [".opencode/skills", ".agents/skills"] as const +const packageLayoutTestTimeoutMs = 60_000 + +setDefaultTimeout(packageLayoutTestTimeoutMs) class PackDryRunError extends Error { constructor(readonly exitCode: number, readonly stderr: string) { - super(`bun pm pack --dry-run failed with exit code ${exitCode}: ${stderr}`) + super(`bun pm pack --dry-run --ignore-scripts failed with exit code ${exitCode}: ${stderr}`) this.name = "PackDryRunError" } } @@ -98,7 +101,7 @@ function parsePackedPaths(output: string): Set { async function packDryRunPaths(): Promise> { const packProcess = Bun.spawn({ - cmd: ["bun", "pm", "pack", "--dry-run"], + cmd: ["bun", "pm", "pack", "--dry-run", "--ignore-scripts"], cwd: repositoryRoot, stdout: "pipe", stderr: "pipe", diff --git a/src/index.telemetry.test.ts b/src/index.telemetry.test.ts index e799b8bf4..2b5f59af6 100644 --- a/src/index.telemetry.test.ts +++ b/src/index.telemetry.test.ts @@ -48,6 +48,13 @@ function createTestPluginModule(): ReturnType { createHooks: mockCreateHooks as never, createPluginInterface: mockCreatePluginInterface as never, log: mockLog, + detectDuplicateOmoPlugin: mock(() => ({ + detected: false, + pluginName: null, + duplicatePlugins: [], + allPlugins: [], + })), + getDuplicateOmoPluginWarning: mock(() => ""), detectExternalSkillPlugin: mock(() => ({ detected: false, pluginName: null })), getSkillPluginConflictWarning: mock(() => ""), initializeOpenClaw: mock(async () => {}), diff --git a/src/index.test.ts b/src/index.test.ts index 842164c17..c4271efbe 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -2,6 +2,13 @@ import { beforeEach, describe, expect, it, mock } from "bun:test" import { createPluginModule } from "./testing/create-plugin-module" const mockInitConfigContext = mock(() => {}) +const mockDetectDuplicateOmoPlugin = mock(() => ({ + detected: false, + pluginName: null, + duplicatePlugins: [], + allPlugins: [], +})) +const mockGetDuplicateOmoPluginWarning = mock(() => "") const mockDetectExternalSkillPlugin = mock(() => ({ detected: false, pluginName: null })) const mockGetSkillPluginConflictWarning = mock(() => "") const mockInjectServerAuthIntoClient = mock(() => {}) @@ -54,6 +61,8 @@ let pluginModule: ReturnType function createTestPluginModule(): ReturnType { return createPluginModule({ initConfigContext: mockInitConfigContext, + detectDuplicateOmoPlugin: mockDetectDuplicateOmoPlugin, + getDuplicateOmoPluginWarning: mockGetDuplicateOmoPluginWarning, detectExternalSkillPlugin: mockDetectExternalSkillPlugin, getSkillPluginConflictWarning: mockGetSkillPluginConflictWarning, injectServerAuthIntoClient: mockInjectServerAuthIntoClient, @@ -79,6 +88,8 @@ function createTestPluginModule(): ReturnType { describe("oh-my-openagent plugin module", () => { beforeEach(() => { mockInitConfigContext.mockClear() + mockDetectDuplicateOmoPlugin.mockClear() + mockGetDuplicateOmoPluginWarning.mockClear() mockDetectExternalSkillPlugin.mockClear() mockGetSkillPluginConflictWarning.mockClear() mockInjectServerAuthIntoClient.mockClear() diff --git a/src/shared/external-plugin-detector.test.ts b/src/shared/external-plugin-detector.test.ts index 64c27e2d3..66d8bee80 100644 --- a/src/shared/external-plugin-detector.test.ts +++ b/src/shared/external-plugin-detector.test.ts @@ -1,5 +1,11 @@ import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test" -import { detectExternalNotificationPlugin, getNotificationConflictWarning, detectExternalSkillPlugin, getSkillPluginConflictWarning } from "./external-plugin-detector" +import { + detectExternalNotificationPlugin, + detectExternalSkillPlugin, + getDuplicateOmoPluginWarning, + getNotificationConflictWarning, + getSkillPluginConflictWarning, +} from "./external-plugin-detector" import * as fs from "node:fs" import * as path from "node:path" import * as os from "node:os" @@ -11,13 +17,21 @@ async function importFreshExternalPluginDetectorModule(): Promise { let tempDir: string let tempHomeDir: string + let originalOpencodeConfigDir: string | undefined beforeEach(() => { tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "omo-test-")) tempHomeDir = fs.mkdtempSync(path.join(os.tmpdir(), "omo-home-")) + originalOpencodeConfigDir = process.env.OPENCODE_CONFIG_DIR + delete process.env.OPENCODE_CONFIG_DIR }) afterEach(() => { + if (originalOpencodeConfigDir === undefined) { + delete process.env.OPENCODE_CONFIG_DIR + } else { + process.env.OPENCODE_CONFIG_DIR = originalOpencodeConfigDir + } mock.restore() fs.rmSync(tempDir, { recursive: true, force: true }) fs.rmSync(tempHomeDir, { recursive: true, force: true }) @@ -476,6 +490,66 @@ describe("external-plugin-detector", () => { }) }) + describe("detectDuplicateOmoPlugin", () => { + test("#given a source plugin and active profile package alias #when detecting duplicates #then it reports the self-conflict", async () => { + // given + const projectConfigDir = path.join(tempDir, ".opencode") + const profileConfigDir = path.join(tempHomeDir, ".config", "opencode", "profiles", "today") + fs.mkdirSync(projectConfigDir, { recursive: true }) + fs.mkdirSync(profileConfigDir, { recursive: true }) + fs.writeFileSync( + path.join(projectConfigDir, "opencode.json"), + JSON.stringify({ plugin: ["file:///Users/yeongyu/local-workspaces/omo/src/index.ts"] }), + ) + fs.writeFileSync( + path.join(profileConfigDir, "opencode.json"), + JSON.stringify({ plugin: ["oh-my-openagent@latest"] }), + ) + process.env.OPENCODE_CONFIG_DIR = profileConfigDir + + const nodeOs = await import("node:os") + mock.module("node:os", () => ({ + ...nodeOs, + homedir: () => tempHomeDir, + })) + const { detectDuplicateOmoPlugin: detectDuplicateOmoPluginFresh } = await importFreshExternalPluginDetectorModule() + + // when + const result = detectDuplicateOmoPluginFresh(tempDir) + + // then + expect(result.detected).toBe(true) + expect(result.pluginName).toBe("oh-my-openagent") + expect(result.duplicatePlugins).toEqual([ + "file:///Users/yeongyu/local-workspaces/omo/src/index.ts", + "oh-my-openagent@latest", + ]) + }) + + test("#given both package names from the rename window #when detecting duplicates #then it treats them as the same OMO plugin", async () => { + // given + const opencodeDir = path.join(tempDir, ".opencode") + fs.mkdirSync(opencodeDir, { recursive: true }) + fs.writeFileSync( + path.join(opencodeDir, "opencode.json"), + JSON.stringify({ plugin: ["oh-my-opencode", "npm:oh-my-openagent@latest"] }), + ) + const nodeOs = await import("node:os") + mock.module("node:os", () => ({ + ...nodeOs, + homedir: () => tempHomeDir, + })) + const { detectDuplicateOmoPlugin: detectDuplicateOmoPluginFresh } = await importFreshExternalPluginDetectorModule() + + // when + const result = detectDuplicateOmoPluginFresh(tempDir) + + // then + expect(result.detected).toBe(true) + expect(result.duplicatePlugins).toEqual(["oh-my-opencode", "npm:oh-my-openagent@latest"]) + }) + }) + describe("getSkillPluginConflictWarning", () => { test("should generate warning message with plugin name", () => { // when @@ -488,4 +562,19 @@ describe("external-plugin-detector", () => { expect(warning).toContain("skills") }) }) + + describe("getDuplicateOmoPluginWarning", () => { + test("#given duplicate OMO entries #when generating a warning #then it tells the user startup is disabled", () => { + // when + const warning = getDuplicateOmoPluginWarning([ + "file:///Users/yeongyu/local-workspaces/omo/src/index.ts", + "oh-my-openagent@latest", + ]) + + // then + expect(warning).toContain("Duplicate OMO plugin entries detected") + expect(warning).toContain("startup has been disabled") + expect(warning).toContain("oh-my-openagent@latest") + }) + }) }) diff --git a/src/shared/external-plugin-detector.ts b/src/shared/external-plugin-detector.ts index 818d1c893..0bab73448 100644 --- a/src/shared/external-plugin-detector.ts +++ b/src/shared/external-plugin-detector.ts @@ -28,6 +28,13 @@ const KNOWN_SKILL_PLUGINS = [ "@opencode/skills", ] +const OMO_PACKAGE_PLUGINS = [ + "oh-my-opencode", + "oh-my-openagent", + "@code-yeongyu/oh-my-opencode", + "@code-yeongyu/oh-my-openagent", +] + function matchesKnownPlugin(entry: string, knownPlugins: readonly string[]): string | null { const normalized = entry.toLowerCase() for (const known of knownPlugins) { @@ -43,6 +50,20 @@ function matchesKnownPlugin(entry: string, knownPlugins: readonly string[]): str return null } +function isOmoFilePlugin(entry: string): boolean { + const normalized = entry.toLowerCase().replaceAll("\\", "/") + if (!normalized.startsWith("file://")) return false + + return /\/(omo(?:-[^/]*)?|oh-my-opencode|oh-my-openagent)\/(src|dist)\/index\.(ts|js)$/.test(normalized) +} + +function matchesOmoPlugin(entry: string): string | null { + const packageMatch = matchesKnownPlugin(entry, OMO_PACKAGE_PLUGINS) + if (packageMatch) return packageMatch + if (isOmoFilePlugin(entry)) return "oh-my-openagent" + return null +} + export interface ExternalNotifierResult { detected: boolean pluginName: string | null @@ -55,6 +76,13 @@ export interface ExternalSkillPluginResult { allPlugins: string[] } +export interface DuplicateOmoPluginResult { + detected: boolean + pluginName: string | null + duplicatePlugins: string[] + allPlugins: string[] +} + /** * Detect if any external notification plugin is configured. * Returns information about detected plugins for logging/warning. @@ -107,6 +135,31 @@ export function detectExternalSkillPlugin(directory: string): ExternalSkillPlugi } } +export function detectDuplicateOmoPlugin(directory: string): DuplicateOmoPluginResult { + const plugins = loadOpencodePlugins(directory) + const duplicatePlugins = plugins.filter((plugin) => matchesOmoPlugin(plugin) !== null) + + if (duplicatePlugins.length > 1) { + log("[oh-my-openagent] Duplicate OMO plugin entries detected", { + duplicatePlugins, + allPlugins: plugins, + }) + return { + detected: true, + pluginName: "oh-my-openagent", + duplicatePlugins, + allPlugins: plugins, + } + } + + return { + detected: false, + pluginName: null, + duplicatePlugins, + allPlugins: plugins, + } +} + /** * Generate a warning message for users with conflicting notification plugins. */ @@ -137,3 +190,19 @@ Both ${PLUGIN_NAME} and ${pluginName} scan ~/.config/opencode/skills/ and regist 2. Or disable ${PLUGIN_NAME}'s skill loading by setting "claude_code.skills": false in ${CONFIG_BASENAME}.json 3. Or uninstall ${PLUGIN_NAME} if you prefer ${pluginName}'s skill management` } + +export function getDuplicateOmoPluginWarning(duplicatePlugins: readonly string[]): string { + const formattedPlugins = duplicatePlugins.map((plugin) => ` - ${plugin}`).join("\n") + + return `[${PLUGIN_NAME}] Duplicate OMO plugin entries detected: +${formattedPlugins} + +Multiple ${PLUGIN_NAME} instances can inject internal prompts into the same live OpenCode session. + That can create overlapping assistant turns and corrupt session state. + + ${PLUGIN_NAME} startup has been disabled for this plugin instance. + + Keep exactly one OMO plugin entry in your OpenCode config. Check both: + 1. ~/.config/opencode/opencode.json + 2. Any active OPENCODE_CONFIG_DIR profile, such as ~/.config/opencode/profiles//opencode.json` +} diff --git a/src/shared/load-opencode-plugins.test.ts b/src/shared/load-opencode-plugins.test.ts index 9723c1cd3..97879570d 100644 --- a/src/shared/load-opencode-plugins.test.ts +++ b/src/shared/load-opencode-plugins.test.ts @@ -19,7 +19,12 @@ async function importFreshLoadOpencodePluginsModule(): Promise { + let originalOpencodeConfigDir: string | undefined + beforeEach(() => { + originalOpencodeConfigDir = process.env.OPENCODE_CONFIG_DIR + delete process.env.OPENCODE_CONFIG_DIR + existsSyncMock.mockReset() existsSyncMock.mockImplementation((_path: string) => true) readFileSyncMock.mockReset() @@ -35,6 +40,11 @@ describe("loadOpencodePlugins", () => { }) afterEach(() => { + if (originalOpencodeConfigDir === undefined) { + delete process.env.OPENCODE_CONFIG_DIR + } else { + process.env.OPENCODE_CONFIG_DIR = originalOpencodeConfigDir + } mock.restore() }) @@ -86,4 +96,36 @@ describe("loadOpencodePlugins", () => { }) }) }) + + describe("#given OPENCODE_CONFIG_DIR points at an active profile", () => { + describe("#when loading plugins for the project", () => { + it("#then includes plugin entries from the profile config directory", async () => { + // given + process.env.OPENCODE_CONFIG_DIR = "/tmp/opencode-profile" + existsSyncMock.mockImplementation((filePath: string) => ( + filePath === "/project/.opencode/opencode.json" + || filePath === "/tmp/opencode-profile/opencode.json" + )) + readFileSyncMock.mockImplementation((filePath: string, _encoding?: string) => { + if (filePath === "/project/.opencode/opencode.json") { + return JSON.stringify({ plugin: ["file:///repo/omo/src/index.ts"] }) + } + if (filePath === "/tmp/opencode-profile/opencode.json") { + return JSON.stringify({ plugin: ["oh-my-openagent@latest"] }) + } + return JSON.stringify({}) + }) + const { loadOpencodePlugins } = await importFreshLoadOpencodePluginsModule() + + // when + const result = loadOpencodePlugins("/project") + + // then + expect(result).toEqual([ + "file:///repo/omo/src/index.ts", + "oh-my-openagent@latest", + ]) + }) + }) + }) }) diff --git a/src/shared/load-opencode-plugins.ts b/src/shared/load-opencode-plugins.ts index 1d8664aa7..ff7352332 100644 --- a/src/shared/load-opencode-plugins.ts +++ b/src/shared/load-opencode-plugins.ts @@ -23,6 +23,13 @@ function getConfigPaths(directory: string): string[] { path.join(crossPlatformDir, "opencode", "opencode.jsonc"), ] + const customConfigDir = process.env.OPENCODE_CONFIG_DIR?.trim() + if (customConfigDir) { + const resolvedCustomConfigDir = path.resolve(customConfigDir) + paths.push(path.join(resolvedCustomConfigDir, "opencode.json")) + paths.push(path.join(resolvedCustomConfigDir, "opencode.jsonc")) + } + if (process.platform === "win32") { const appdataDir = getWindowsAppdataDir() if (appdataDir) { @@ -31,7 +38,7 @@ function getConfigPaths(directory: string): string[] { } } - return paths + return Array.from(new Set(paths)) } export function loadOpencodePlugins(directory: string): string[] { diff --git a/src/testing/create-plugin-module.test.ts b/src/testing/create-plugin-module.test.ts index 94163ee3f..8f3fa06ad 100644 --- a/src/testing/create-plugin-module.test.ts +++ b/src/testing/create-plugin-module.test.ts @@ -5,6 +5,13 @@ import { createPluginModule } from "./create-plugin-module" const mockInitConfigContext = mock(() => {}) const mockDetectExternalSkillPlugin = mock(() => ({ detected: false, pluginName: null, allPlugins: [] })) const mockGetSkillPluginConflictWarning = mock(() => "") +const mockDetectDuplicateOmoPlugin = mock(() => ({ + detected: false, + pluginName: null, + duplicatePlugins: [], + allPlugins: [], +})) +const mockGetDuplicateOmoPluginWarning = mock(() => "") const mockInjectServerAuthIntoClient = mock(() => {}) const mockLogLegacyPluginStartupWarning = mock(() => {}) const mockMigrateLegacyWorkspaceDirectory = mock(() => ({ migrated: false, skipped: [] })) @@ -55,6 +62,8 @@ function createTestPluginModule(): ReturnType { initConfigContext: mockInitConfigContext, detectExternalSkillPlugin: mockDetectExternalSkillPlugin, getSkillPluginConflictWarning: mockGetSkillPluginConflictWarning, + detectDuplicateOmoPlugin: mockDetectDuplicateOmoPlugin, + getDuplicateOmoPluginWarning: mockGetDuplicateOmoPluginWarning, injectServerAuthIntoClient: mockInjectServerAuthIntoClient, logLegacyPluginStartupWarning: mockLogLegacyPluginStartupWarning, migrateLegacyWorkspaceDirectory: mockMigrateLegacyWorkspaceDirectory, @@ -77,7 +86,20 @@ function createTestPluginModule(): ReturnType { describe("createPluginModule()", () => { beforeEach(() => { + mockDetectDuplicateOmoPlugin.mockClear() + mockGetDuplicateOmoPluginWarning.mockClear() + mockInjectServerAuthIntoClient.mockClear() mockLoadPluginConfig.mockClear() + mockCreateManagers.mockClear() + mockCreateTools.mockClear() + mockCreateHooks.mockClear() + mockCreatePluginInterface.mockClear() + mockDetectDuplicateOmoPlugin.mockReturnValue({ + detected: false, + pluginName: null, + duplicatePlugins: [], + allPlugins: [], + }) initI18n({ locale: "en", fallback: "en" }) }) @@ -100,4 +122,44 @@ describe("createPluginModule()", () => { expect(t("toast.task_completed")).toBe("任务完成") }) }) + + describe("#given duplicate OMO plugin entries are configured", () => { + it("#then startup warns and returns no prompt-producing hooks", async () => { + // given + const pluginModule = createTestPluginModule() + const duplicatePlugins = [ + "file:///Users/yeongyu/local-workspaces/omo/src/index.ts", + "oh-my-openagent@latest", + ] + mockDetectDuplicateOmoPlugin.mockReturnValue({ + detected: true, + pluginName: "oh-my-openagent", + duplicatePlugins, + allPlugins: duplicatePlugins, + }) + mockGetDuplicateOmoPluginWarning.mockReturnValue("duplicate OMO startup disabled") + const consoleWarn = mock(() => {}) + const originalWarn = console.warn + console.warn = consoleWarn + + try { + // when + const hooks = await pluginModule.server({ + directory: "/tmp/project", + client: {}, + } as Parameters[0]) + + // then + expect(hooks).toEqual({}) + expect(consoleWarn).toHaveBeenCalledWith("duplicate OMO startup disabled") + expect(mockInjectServerAuthIntoClient).not.toHaveBeenCalled() + expect(mockCreateManagers).not.toHaveBeenCalled() + expect(mockCreateTools).not.toHaveBeenCalled() + expect(mockCreateHooks).not.toHaveBeenCalled() + expect(mockCreatePluginInterface).not.toHaveBeenCalled() + } finally { + console.warn = originalWarn + } + }) + }) }) diff --git a/src/testing/create-plugin-module.ts b/src/testing/create-plugin-module.ts index b7f58bcda..950185899 100644 --- a/src/testing/create-plugin-module.ts +++ b/src/testing/create-plugin-module.ts @@ -16,7 +16,12 @@ import { type CompactionAutocontinueHook, } from "../plugin/session-compacting" import { installAgentSortShim, setAgentSortOrder } from "../shared/agent-sort-shim" -import { detectExternalSkillPlugin, getSkillPluginConflictWarning } from "../shared/external-plugin-detector" +import { + detectDuplicateOmoPlugin, + detectExternalSkillPlugin, + getDuplicateOmoPluginWarning, + getSkillPluginConflictWarning, +} from "../shared/external-plugin-detector" import { createFirstMessageVariantGate } from "../shared/first-message-variant" import { initI18n } from "../shared/i18n" import { log } from "../shared/logger" @@ -36,6 +41,8 @@ export type PluginModuleDeps = { log: typeof log logLegacyPluginStartupWarning: typeof logLegacyPluginStartupWarning migrateLegacyWorkspaceDirectory: typeof migrateLegacyWorkspaceDirectory + detectDuplicateOmoPlugin: typeof detectDuplicateOmoPlugin + getDuplicateOmoPluginWarning: typeof getDuplicateOmoPluginWarning detectExternalSkillPlugin: typeof detectExternalSkillPlugin getSkillPluginConflictWarning: typeof getSkillPluginConflictWarning injectServerAuthIntoClient: typeof injectServerAuthIntoClient @@ -60,6 +67,8 @@ const defaultPluginModuleDeps: PluginModuleDeps = { log, logLegacyPluginStartupWarning, migrateLegacyWorkspaceDirectory, + detectDuplicateOmoPlugin, + getDuplicateOmoPluginWarning, detectExternalSkillPlugin, getSkillPluginConflictWarning, injectServerAuthIntoClient, @@ -88,6 +97,12 @@ export function createPluginModule(overrides: Partial = {}): P deps.logLegacyPluginStartupWarning() deps.migrateLegacyWorkspaceDirectory(input.directory) + const duplicateOmoPluginCheck = deps.detectDuplicateOmoPlugin(input.directory) + if (duplicateOmoPluginCheck.detected) { + console.warn(deps.getDuplicateOmoPluginWarning(duplicateOmoPluginCheck.duplicatePlugins)) + return {} + } + const skillPluginCheck = deps.detectExternalSkillPlugin(input.directory) if (skillPluginCheck.detected && skillPluginCheck.pluginName) { console.warn(deps.getSkillPluginConflictWarning(skillPluginCheck.pluginName))