From 6fffacd4fc66d4642d87a64f9274c32bb90cefd3 Mon Sep 17 00:00:00 2001 From: Kenny Date: Sun, 19 Apr 2026 14:34:13 +0800 Subject: [PATCH] fix(notify): harden bundled ownership and idle suppression --- package.json | 2 +- src/bundled-opencode-notify/index.test.ts | 46 +++++++++++--- src/bundled-opencode-notify/index.ts | 6 +- src/hooks/session-todo-status.ts | 14 +++-- src/shared/bundled-notify-ownership.test.ts | 29 +++++++++ src/shared/bundled-notify-ownership.ts | 61 +++++++++++++------ .../bundled-notify-postinstall-path.test.ts | 22 +++++++ 7 files changed, 144 insertions(+), 36 deletions(-) create mode 100644 src/shared/bundled-notify-postinstall-path.test.ts diff --git a/package.json b/package.json index bfb3c5e3b..6f5f08bc0 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "./schema.json": "./dist/oh-my-opencode.schema.json" }, "scripts": { - "build": "bun build src/index.ts --outdir dist --target bun --format esm --external @ast-grep/napi --external zod && tsc --emitDeclarationOnly && bun build src/cli/index.ts --outdir dist/cli --target bun --format esm --external @ast-grep/napi && bun build src/bundled-opencode-notify/index.ts --outdir dist/opencode-notify --target bun --format esm --external @ast-grep/napi --external zod && bun run build:bundled-notify && bun run build:schema", + "build": "bun build src/index.ts --outdir dist --target bun --format esm --external @ast-grep/napi --external zod && tsc --emitDeclarationOnly && bun build src/cli/index.ts --outdir dist/cli --target bun --format esm --external @ast-grep/napi && bun build src/bundled-opencode-notify/index.ts --outdir dist/opencode-notify --target bun --format esm --external @ast-grep/napi --external zod && bun build src/shared/bundled-notify-ownership.ts --outdir dist/shared --target bun --format esm --external @ast-grep/napi --external zod && bun run build:bundled-notify && bun run build:schema", "build:all": "bun run build && bun run build:binaries", "build:binaries": "bun run script/build-binaries.ts", "build:schema": "bun run script/build-schema.ts", diff --git a/src/bundled-opencode-notify/index.test.ts b/src/bundled-opencode-notify/index.test.ts index 94b94d6a2..641692b1b 100644 --- a/src/bundled-opencode-notify/index.test.ts +++ b/src/bundled-opencode-notify/index.test.ts @@ -6,6 +6,11 @@ interface TodoItem { status: string } +type TodoResponseMode = { + todos?: TodoItem[] + throwError?: boolean +} + function createMockShellExecutor(notificationCommands: string[]) { return (cmd: TemplateStringsArray | string, ...values: unknown[]) => { const command = typeof cmd === "string" @@ -35,12 +40,18 @@ function createMockShellExecutor(notificationCommands: string[]) { } } -function createPluginInput(todos: TodoItem[], notificationCommands: string[]) { +function createPluginInput(todoMode: TodoResponseMode, notificationCommands: string[]) { return { $: createMockShellExecutor(notificationCommands), client: { session: { - todo: async () => ({ data: todos }), + todo: async () => { + if (todoMode.throwError) { + throw new Error("todo fetch failed") + } + + return { data: todoMode.todos ?? [] } + }, }, }, } as Parameters[0] @@ -60,7 +71,7 @@ describe("bundled-opencode-notify idle suppression", () => { // given const notificationCommands: string[] = [] const hooks = await bundledNotifyPlugin.server( - createPluginInput([{ status: "in_progress" }], notificationCommands), + createPluginInput({ todos: [{ status: "in_progress" }] }, notificationCommands), ) // when @@ -77,7 +88,7 @@ describe("bundled-opencode-notify idle suppression", () => { // given const notificationCommands: string[] = [] const hooks = await bundledNotifyPlugin.server( - createPluginInput([{ status: "completed" }], notificationCommands), + createPluginInput({ todos: [{ status: "completed" }] }, notificationCommands), ) // when @@ -94,10 +105,12 @@ describe("bundled-opencode-notify idle suppression", () => { // given const notificationCommands: string[] = [] const hooks = await bundledNotifyPlugin.server( - createPluginInput([ - { status: "blocked" }, - { status: "deleted" }, - ], notificationCommands), + createPluginInput({ + todos: [ + { status: "blocked" }, + { status: "deleted" }, + ], + }, notificationCommands), ) // when @@ -109,4 +122,21 @@ describe("bundled-opencode-notify idle suppression", () => { // then expect(notificationCommands.length).toBeGreaterThan(0) }) + + test("suppresses ready notification when todo fetch state is unknown", async () => { + // given + const notificationCommands: string[] = [] + const hooks = await bundledNotifyPlugin.server( + createPluginInput({ throwError: true }, notificationCommands), + ) + + // when + await hooks.event?.({ event: { type: "session.idle", properties: { sessionID: "session-4" } } }) + jest.advanceTimersByTime(1500) + await Promise.resolve() + await Promise.resolve() + + // then + expect(notificationCommands).toHaveLength(0) + }) }) diff --git a/src/bundled-opencode-notify/index.ts b/src/bundled-opencode-notify/index.ts index 6d20bf282..511c20686 100644 --- a/src/bundled-opencode-notify/index.ts +++ b/src/bundled-opencode-notify/index.ts @@ -1,5 +1,5 @@ import type { Hooks, Plugin, PluginModule } from "@opencode-ai/plugin" -import { hasIncompleteTodos } from "../hooks/session-todo-status" +import { getSessionTodoState } from "../hooks/session-todo-status" type Platform = "darwin" | "linux" | "win32" | "unsupported" @@ -82,8 +82,8 @@ async function sendSessionNotification(ctx: Parameters[0], title: string } async function sendIdleReadyNotification(ctx: Parameters[0], sessionID: string): Promise { - const hasPendingTodos = await hasIncompleteTodos(ctx, sessionID) - if (hasPendingTodos) return + const todoState = await getSessionTodoState(ctx, sessionID) + if (todoState !== "clear") return await sendSessionNotification(ctx, "OpenCode", "Agent is ready for input") } diff --git a/src/hooks/session-todo-status.ts b/src/hooks/session-todo-status.ts index d1705ef7d..18099e934 100644 --- a/src/hooks/session-todo-status.ts +++ b/src/hooks/session-todo-status.ts @@ -9,13 +9,19 @@ interface Todo { id?: string } -export async function hasIncompleteTodos(ctx: PluginInput, sessionID: string): Promise { +export type SessionTodoState = "pending" | "clear" | "unknown" + +export async function getSessionTodoState(ctx: PluginInput, sessionID: string): Promise { try { const response = await ctx.client.session.todo({ path: { id: sessionID } }) const todos = normalizeSDKResponse(response, [] as Todo[], { preferResponseOnMissingData: true }) - if (!todos || todos.length === 0) return false - return getIncompleteCount(todos) > 0 + if (!todos || todos.length === 0) return "clear" + return getIncompleteCount(todos) > 0 ? "pending" : "clear" } catch { - return false + return "unknown" } } + +export async function hasIncompleteTodos(ctx: PluginInput, sessionID: string): Promise { + return (await getSessionTodoState(ctx, sessionID)) === "pending" +} diff --git a/src/shared/bundled-notify-ownership.test.ts b/src/shared/bundled-notify-ownership.test.ts index 4ec140302..19a7fa679 100644 --- a/src/shared/bundled-notify-ownership.test.ts +++ b/src/shared/bundled-notify-ownership.test.ts @@ -98,6 +98,35 @@ describe("ensureBundledNotifyOwnership", () => { expect(readConfig(userConfigPath).plugin).toEqual(["oh-my-openagent", canonicalEntry]) }) + test("does not classify unrelated notify-like plugin names as unsafe", () => { + // given + const userConfigPath = join(userConfigDir, "opencode.json") + writeFileSync(userConfigPath, JSON.stringify({ plugin: ["team-notify-center", "oh-my-openagent"] }, null, 2) + "\n") + + // when + const result = ensureBundledNotifyOwnership({ projectDirectory: projectDir, packageRoot }) + + // then + expect(result.changedUserConfig).toBe(true) + expect(readConfig(userConfigPath).plugin).toEqual(["team-notify-center", "oh-my-openagent", canonicalEntry]) + }) + + test("migrates stale bundled dist/opencode-notify file URL to canonical bundled entry", () => { + // given + const userConfigPath = join(userConfigDir, "opencode.json") + const stalePackageRoot = join(rootDir, "package-old") + mkdirSync(join(stalePackageRoot, "dist", "opencode-notify"), { recursive: true }) + const staleBundledEntry = getBundledNotifyCanonicalEntry(stalePackageRoot) + writeFileSync(userConfigPath, JSON.stringify({ plugin: [staleBundledEntry, "oh-my-openagent"] }, null, 2) + "\n") + + // when + const result = ensureBundledNotifyOwnership({ projectDirectory: projectDir, packageRoot }) + + // then + expect(result.changedUserConfig).toBe(true) + expect(readConfig(userConfigPath).plugin).toEqual(["oh-my-openagent", canonicalEntry]) + }) + test("removes project recognized notify and adds bundled user owner", () => { // given const projectConfigPath = join(projectDir, ".opencode", "opencode.json") diff --git a/src/shared/bundled-notify-ownership.ts b/src/shared/bundled-notify-ownership.ts index ac9189100..91b93ac5e 100644 --- a/src/shared/bundled-notify-ownership.ts +++ b/src/shared/bundled-notify-ownership.ts @@ -66,11 +66,41 @@ function isRecognizedExternalNotifyId(entry: string): boolean { return KNOWN_EXTERNAL_NOTIFY_IDS.some((base) => normalized === base || normalized.startsWith(`${base}@`)) } -function looksLikeNotifyPlugin(entry: string): boolean { - const normalized = entry.trim().toLowerCase() - return normalized.includes("kdco/notify") - || normalized.includes("opencode-notify") - || normalized.includes("notify") +function normalizePathForComparison(pathValue: string): string { + return resolve(pathValue).replace(/\\/g, "/").replace(/\/+$/, "") +} + +function tryParseFileUrlPath(entry: string): string | null { + if (!entry.startsWith("file://")) return null + + try { + return fileURLToPath(entry) + } catch { + return null + } +} + +function hasBundledNotifyArtifactPathShape(pathValue: string): boolean { + const normalizedPath = normalizePathForComparison(pathValue) + return normalizedPath.endsWith("/dist/opencode-notify") +} + +function isBundledNotifyArtifactEntry(entry: string, canonicalEntry: string): boolean { + if (entry === canonicalEntry) return true + + const filePath = tryParseFileUrlPath(entry) + if (!filePath) return false + + return hasBundledNotifyArtifactPathShape(filePath) +} + +function isPathBasedNotifyEntry(entry: string): boolean { + if (!isPathLikePluginEntry(entry)) return false + + const filePath = tryParseFileUrlPath(entry) + const pathCandidate = filePath ?? entry + const normalizedPath = normalizePathForComparison(pathCandidate).toLowerCase() + return normalizedPath.includes("/opencode-notify") } function areTupleOptionsEmptyOrDefault(options: unknown[]): boolean { @@ -86,7 +116,7 @@ function areTupleOptionsEmptyOrDefault(options: unknown[]): boolean { function classifyPluginEntry(entry: OpenCodePluginEntry, index: number, canonicalEntry: string): ClassifiedEntry { if (typeof entry === "string") { - if (entry === canonicalEntry) { + if (isBundledNotifyArtifactEntry(entry, canonicalEntry)) { return { kind: "bundled", entry, index } } @@ -94,11 +124,7 @@ function classifyPluginEntry(entry: OpenCodePluginEntry, index: number, canonica return { kind: "recognized-external", entry, index } } - if (!looksLikeNotifyPlugin(entry)) { - return { kind: "other", entry, index } - } - - if (isPathLikePluginEntry(entry)) { + if (isPathBasedNotifyEntry(entry)) { return { kind: "unsafe-external", entry, @@ -107,16 +133,11 @@ function classifyPluginEntry(entry: OpenCodePluginEntry, index: number, canonica } } - return { - kind: "unsafe-external", - entry, - index, - reason: "notify plugin entry is not an exact recognized kdco/notify identifier", - } + return { kind: "other", entry, index } } const [tupleKey, ...tupleOptions] = entry - if (tupleKey === canonicalEntry) { + if (isBundledNotifyArtifactEntry(tupleKey, canonicalEntry)) { if (areTupleOptionsEmptyOrDefault(tupleOptions)) { return { kind: "bundled", entry, index } } @@ -142,12 +163,12 @@ function classifyPluginEntry(entry: OpenCodePluginEntry, index: number, canonica } } - if (looksLikeNotifyPlugin(tupleKey)) { + if (isPathBasedNotifyEntry(tupleKey)) { return { kind: "unsafe-external", entry, index, - reason: "tuple-based notify entry is not an exact recognized kdco/notify identifier", + reason: "path-based notify plugin entries are not auto-migrated", } } diff --git a/src/shared/bundled-notify-postinstall-path.test.ts b/src/shared/bundled-notify-postinstall-path.test.ts new file mode 100644 index 000000000..741f042d8 --- /dev/null +++ b/src/shared/bundled-notify-postinstall-path.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, test } from "bun:test" +import { readFileSync } from "node:fs" +import { resolve } from "node:path" + +describe("bundled notify postinstall bootstrap path", () => { + test("build script emits ownership module consumed by postinstall", () => { + // given + const packageJsonPath = resolve(import.meta.dir, "..", "..", "package.json") + const postinstallPath = resolve(import.meta.dir, "..", "..", "postinstall.mjs") + const packageJson = JSON.parse(readFileSync(packageJsonPath, "utf-8")) as { + scripts?: { build?: string } + } + const postinstallScript = readFileSync(postinstallPath, "utf-8") + + // when + const buildScript = packageJson.scripts?.build ?? "" + + // then + expect(buildScript).toContain("src/shared/bundled-notify-ownership.ts --outdir dist/shared") + expect(postinstallScript).toContain("dist/shared/bundled-notify-ownership.js") + }) +})