fix(notify): harden bundled ownership and idle suppression

This commit is contained in:
Kenny
2026-04-19 14:34:13 +08:00
parent f5a11281f8
commit 6fffacd4fc
7 changed files with 144 additions and 36 deletions
+1 -1
View File
@@ -22,7 +22,7 @@
"./schema.json": "./dist/oh-my-opencode.schema.json" "./schema.json": "./dist/oh-my-opencode.schema.json"
}, },
"scripts": { "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:all": "bun run build && bun run build:binaries",
"build:binaries": "bun run script/build-binaries.ts", "build:binaries": "bun run script/build-binaries.ts",
"build:schema": "bun run script/build-schema.ts", "build:schema": "bun run script/build-schema.ts",
+38 -8
View File
@@ -6,6 +6,11 @@ interface TodoItem {
status: string status: string
} }
type TodoResponseMode = {
todos?: TodoItem[]
throwError?: boolean
}
function createMockShellExecutor(notificationCommands: string[]) { function createMockShellExecutor(notificationCommands: string[]) {
return (cmd: TemplateStringsArray | string, ...values: unknown[]) => { return (cmd: TemplateStringsArray | string, ...values: unknown[]) => {
const command = typeof cmd === "string" 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 { return {
$: createMockShellExecutor(notificationCommands), $: createMockShellExecutor(notificationCommands),
client: { client: {
session: { session: {
todo: async () => ({ data: todos }), todo: async () => {
if (todoMode.throwError) {
throw new Error("todo fetch failed")
}
return { data: todoMode.todos ?? [] }
},
}, },
}, },
} as Parameters<typeof bundledNotifyPlugin.server>[0] } as Parameters<typeof bundledNotifyPlugin.server>[0]
@@ -60,7 +71,7 @@ describe("bundled-opencode-notify idle suppression", () => {
// given // given
const notificationCommands: string[] = [] const notificationCommands: string[] = []
const hooks = await bundledNotifyPlugin.server( const hooks = await bundledNotifyPlugin.server(
createPluginInput([{ status: "in_progress" }], notificationCommands), createPluginInput({ todos: [{ status: "in_progress" }] }, notificationCommands),
) )
// when // when
@@ -77,7 +88,7 @@ describe("bundled-opencode-notify idle suppression", () => {
// given // given
const notificationCommands: string[] = [] const notificationCommands: string[] = []
const hooks = await bundledNotifyPlugin.server( const hooks = await bundledNotifyPlugin.server(
createPluginInput([{ status: "completed" }], notificationCommands), createPluginInput({ todos: [{ status: "completed" }] }, notificationCommands),
) )
// when // when
@@ -94,10 +105,12 @@ describe("bundled-opencode-notify idle suppression", () => {
// given // given
const notificationCommands: string[] = [] const notificationCommands: string[] = []
const hooks = await bundledNotifyPlugin.server( const hooks = await bundledNotifyPlugin.server(
createPluginInput([ createPluginInput({
{ status: "blocked" }, todos: [
{ status: "deleted" }, { status: "blocked" },
], notificationCommands), { status: "deleted" },
],
}, notificationCommands),
) )
// when // when
@@ -109,4 +122,21 @@ describe("bundled-opencode-notify idle suppression", () => {
// then // then
expect(notificationCommands.length).toBeGreaterThan(0) 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)
})
}) })
+3 -3
View File
@@ -1,5 +1,5 @@
import type { Hooks, Plugin, PluginModule } from "@opencode-ai/plugin" 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" type Platform = "darwin" | "linux" | "win32" | "unsupported"
@@ -82,8 +82,8 @@ async function sendSessionNotification(ctx: Parameters<Plugin>[0], title: string
} }
async function sendIdleReadyNotification(ctx: Parameters<Plugin>[0], sessionID: string): Promise<void> { async function sendIdleReadyNotification(ctx: Parameters<Plugin>[0], sessionID: string): Promise<void> {
const hasPendingTodos = await hasIncompleteTodos(ctx, sessionID) const todoState = await getSessionTodoState(ctx, sessionID)
if (hasPendingTodos) return if (todoState !== "clear") return
await sendSessionNotification(ctx, "OpenCode", "Agent is ready for input") await sendSessionNotification(ctx, "OpenCode", "Agent is ready for input")
} }
+10 -4
View File
@@ -9,13 +9,19 @@ interface Todo {
id?: string id?: string
} }
export async function hasIncompleteTodos(ctx: PluginInput, sessionID: string): Promise<boolean> { export type SessionTodoState = "pending" | "clear" | "unknown"
export async function getSessionTodoState(ctx: PluginInput, sessionID: string): Promise<SessionTodoState> {
try { try {
const response = await ctx.client.session.todo({ path: { id: sessionID } }) const response = await ctx.client.session.todo({ path: { id: sessionID } })
const todos = normalizeSDKResponse(response, [] as Todo[], { preferResponseOnMissingData: true }) const todos = normalizeSDKResponse(response, [] as Todo[], { preferResponseOnMissingData: true })
if (!todos || todos.length === 0) return false if (!todos || todos.length === 0) return "clear"
return getIncompleteCount(todos) > 0 return getIncompleteCount(todos) > 0 ? "pending" : "clear"
} catch { } catch {
return false return "unknown"
} }
} }
export async function hasIncompleteTodos(ctx: PluginInput, sessionID: string): Promise<boolean> {
return (await getSessionTodoState(ctx, sessionID)) === "pending"
}
@@ -98,6 +98,35 @@ describe("ensureBundledNotifyOwnership", () => {
expect(readConfig(userConfigPath).plugin).toEqual(["oh-my-openagent", canonicalEntry]) 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", () => { test("removes project recognized notify and adds bundled user owner", () => {
// given // given
const projectConfigPath = join(projectDir, ".opencode", "opencode.json") const projectConfigPath = join(projectDir, ".opencode", "opencode.json")
+41 -20
View File
@@ -66,11 +66,41 @@ function isRecognizedExternalNotifyId(entry: string): boolean {
return KNOWN_EXTERNAL_NOTIFY_IDS.some((base) => normalized === base || normalized.startsWith(`${base}@`)) return KNOWN_EXTERNAL_NOTIFY_IDS.some((base) => normalized === base || normalized.startsWith(`${base}@`))
} }
function looksLikeNotifyPlugin(entry: string): boolean { function normalizePathForComparison(pathValue: string): string {
const normalized = entry.trim().toLowerCase() return resolve(pathValue).replace(/\\/g, "/").replace(/\/+$/, "")
return normalized.includes("kdco/notify") }
|| normalized.includes("opencode-notify")
|| normalized.includes("notify") 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 { function areTupleOptionsEmptyOrDefault(options: unknown[]): boolean {
@@ -86,7 +116,7 @@ function areTupleOptionsEmptyOrDefault(options: unknown[]): boolean {
function classifyPluginEntry(entry: OpenCodePluginEntry, index: number, canonicalEntry: string): ClassifiedEntry { function classifyPluginEntry(entry: OpenCodePluginEntry, index: number, canonicalEntry: string): ClassifiedEntry {
if (typeof entry === "string") { if (typeof entry === "string") {
if (entry === canonicalEntry) { if (isBundledNotifyArtifactEntry(entry, canonicalEntry)) {
return { kind: "bundled", entry, index } return { kind: "bundled", entry, index }
} }
@@ -94,11 +124,7 @@ function classifyPluginEntry(entry: OpenCodePluginEntry, index: number, canonica
return { kind: "recognized-external", entry, index } return { kind: "recognized-external", entry, index }
} }
if (!looksLikeNotifyPlugin(entry)) { if (isPathBasedNotifyEntry(entry)) {
return { kind: "other", entry, index }
}
if (isPathLikePluginEntry(entry)) {
return { return {
kind: "unsafe-external", kind: "unsafe-external",
entry, entry,
@@ -107,16 +133,11 @@ function classifyPluginEntry(entry: OpenCodePluginEntry, index: number, canonica
} }
} }
return { return { kind: "other", entry, index }
kind: "unsafe-external",
entry,
index,
reason: "notify plugin entry is not an exact recognized kdco/notify identifier",
}
} }
const [tupleKey, ...tupleOptions] = entry const [tupleKey, ...tupleOptions] = entry
if (tupleKey === canonicalEntry) { if (isBundledNotifyArtifactEntry(tupleKey, canonicalEntry)) {
if (areTupleOptionsEmptyOrDefault(tupleOptions)) { if (areTupleOptionsEmptyOrDefault(tupleOptions)) {
return { kind: "bundled", entry, index } return { kind: "bundled", entry, index }
} }
@@ -142,12 +163,12 @@ function classifyPluginEntry(entry: OpenCodePluginEntry, index: number, canonica
} }
} }
if (looksLikeNotifyPlugin(tupleKey)) { if (isPathBasedNotifyEntry(tupleKey)) {
return { return {
kind: "unsafe-external", kind: "unsafe-external",
entry, entry,
index, index,
reason: "tuple-based notify entry is not an exact recognized kdco/notify identifier", reason: "path-based notify plugin entries are not auto-migrated",
} }
} }
@@ -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")
})
})