From 20f78c4693d44f552d49fa9b76e07fbac855e87e Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 16 Jun 2026 19:44:28 +0000 Subject: [PATCH 1/2] fix(coding-agent): loaded discovered hook factories Included JS and TS hookCapability results in extension path discovery so hooks under hooks/pre and hooks/post bind to the extension runner without explicit settings entries. Fixes #2796 --- docs/hooks.md | 20 ++++++++-------- packages/coding-agent/CHANGELOG.md | 4 ++++ .../src/extensibility/extensions/loader.ts | 23 ++++++++++++++----- .../test/extensions-discovery.test.ts | 21 +++++++++++++++++ 4 files changed, 53 insertions(+), 15 deletions(-) diff --git a/docs/hooks.md b/docs/hooks.md index 956e478c3..d51247e89 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -4,13 +4,14 @@ This document describes the **current hook subsystem code** in `src/extensibilit ## Current status in runtime -The hook package (`src/extensibility/hooks/`) is still exported and usable as an API surface, but the default CLI runtime now initializes the **extension runner** path. In current startup flow: +The default CLI runtime initializes the **extension runner** path. In current startup flow: - `--hook` is treated as an alias for `--extension` (CLI paths are merged into `additionalExtensionPaths`) +- JS/TS hook factories discovered through `hookCapability` (for example `.omp/hooks/pre/*.ts`) are loaded as extension modules so their `pi.on(...)` handlers bind to the runtime event bus - tools are wrapped by `ExtensionToolWrapper`, not `HookToolWrapper` - context transforms and lifecycle emissions go through `ExtensionRunner` -So this file documents the hook subsystem implementation itself (types/loader/runner/wrapper), including legacy behavior and constraints. +So this file documents the legacy hook subsystem implementation itself (types/loader/runner/wrapper), plus the factory shape still accepted when a discovered hook path is loaded by the extension runner. ## Key files @@ -51,7 +52,14 @@ The factory can: ## Discovery and loading -`discoverAndLoadHooks(configuredPaths, cwd)` does: +Default sessions load JS/TS hook factories discovered by `hookCapability` through the extension runner. `discoverExtensionPaths(configuredPaths, cwd)` does: + +1. Load native extension modules from the capability registry +2. Load importable `.ts`/`.js` hook factories from the hook capability registry +3. Append plugin extension entry points +4. Append explicitly configured paths + +The legacy `discoverAndLoadHooks(configuredPaths, cwd)` helper still exists and does: 1. Load discovered hooks from capability registry (`loadCapability("hooks")`) 2. Append explicitly configured paths (deduped by absolute path) @@ -67,12 +75,6 @@ The factory can: - `~` path: expanded - relative path: resolved against `cwd` -### Important legacy mismatch - -Discovery providers for `hookCapability` still model pre/post shell-style hook files (for example `.claude/hooks/pre/*`, `.omp/.../hooks/pre/*`). - -The hook loader here uses dynamic module import and requires a default JS/TS hook factory. If a discovered hook path is not importable as a module, load fails and is reported in `LoadHooksResult.errors`. - ## Event surfaces Hook events are strongly typed in `types.ts`. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 80c6e5cf8..6966a2450 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `hooks/pre/*.ts` and `hooks/post/*.ts` files discovered through `hookCapability` being registered in discovery but never loaded into the extension runner, so their `tool_call` handlers now run without a manual `settings.json` `extensions` entry ([#2796](https://github.com/can1357/oh-my-pi/issues/2796)). + ## [16.0.2] - 2026-06-16 ### Added diff --git a/packages/coding-agent/src/extensibility/extensions/loader.ts b/packages/coding-agent/src/extensibility/extensions/loader.ts index 2ef194a37..6dae41751 100644 --- a/packages/coding-agent/src/extensibility/extensions/loader.ts +++ b/packages/coding-agent/src/extensibility/extensions/loader.ts @@ -10,6 +10,7 @@ import type { KeyId } from "@oh-my-pi/pi-tui"; import { hasFsCode, isEacces, isEnoent, logger } from "@oh-my-pi/pi-utils"; import { z } from "zod/v4"; import { type ExtensionModule, extensionModuleCapability } from "../../capability/extension-module"; +import { type Hook, hookCapability } from "../../capability/hook"; import { loadCapability } from "../../discovery"; import { getExtensionNameFromPath } from "../../discovery/helpers"; import type { ExecOptions } from "../../exec/exec"; @@ -479,8 +480,8 @@ async function discoverExtensionsInDir(dir: string): Promise { /** * Discover absolute paths of extensions to load, without importing or * binding factories. Hot path on session startup — the scan walks native - * `.omp`/`.pi` extension capabilities, the installed-plugin tree, and any - * configured paths. + * `.omp`/`.pi` extension capabilities, JS/TS hook factories, the + * installed-plugin tree, and any configured paths. * * Subagents reuse the parent's collected paths via the SDK's * `preloadedExtensionPaths` option, then call {@link loadExtensions} themselves @@ -516,17 +517,27 @@ export async function discoverExtensionPaths( }; // 1. Discover extension modules via capability API (native .omp/.pi only) - const discovered = await loadCapability(extensionModuleCapability.id, { cwd }); + const discovered = await loadCapability(extensionModuleCapability.id, { + cwd, + disabledExtensions: disabledExtensionIds, + }); for (const ext of discovered.items) { if (ext._source.provider !== "native") continue; - if (isDisabledName(ext.name)) continue; addPath(ext.path); } - // 2. Discover extension entry points from installed plugins + // 2. Discover JS/TS hook factories from hookCapability and bind them through + // the extension runner, which owns the current runtime event bus. + const hooks = await loadCapability(hookCapability.id, { + cwd, + disabledExtensions: disabledExtensionIds, + }); + addPaths(hooks.items.map(hook => hook.path).filter(hookPath => isExtensionFile(path.basename(hookPath)))); + + // 3. Discover extension entry points from installed plugins addPaths(await getAllPluginExtensionPaths(cwd)); - // 3. Explicitly configured paths + // 4. Explicitly configured paths for (const configuredPath of configuredPaths) { const resolved = resolvePath(configuredPath, cwd); diff --git a/packages/coding-agent/test/extensions-discovery.test.ts b/packages/coding-agent/test/extensions-discovery.test.ts index 3f6ab9362..d31f8d2f7 100644 --- a/packages/coding-agent/test/extensions-discovery.test.ts +++ b/packages/coding-agent/test/extensions-discovery.test.ts @@ -591,6 +591,27 @@ describe("extensions discovery", () => { expect(result.extensions[0].handlers.has("agent_end")).toBe(true); }); + it("loads hookCapability JS factories as extension handlers", async () => { + const hookDir = path.join(getProjectAgentDir(tempDir.path()), "hooks", "pre"); + fs.mkdirSync(hookDir, { recursive: true }); + const hookPath = path.join(hookDir, "guard-test.ts"); + fs.writeFileSync( + hookPath, + ` + export default function(pi) { + pi.on("tool_call", async () => ({ block: true, reason: "blocked by hook" })); + } + `, + ); + + const result = await discoverForTest(); + const loadedHook = result.extensions.find(extension => extension.path === hookPath); + + expect(result.errors).toHaveLength(0); + expect(loadedHook).toBeDefined(); + expect(loadedHook?.handlers.has("tool_call")).toBe(true); + }); + it("loads extension with shortcuts", async () => { const extCode = ` export default function(pi) { From 205413860bbaf0f203098d890c06a296e9b16107 Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 16 Jun 2026 20:11:09 +0000 Subject: [PATCH 2/2] fix(coding-agent): preserved hook disabled filters Preserved Settings.disabledExtensions fallback when extension discovery is called without an explicit disabled list and kept hookCapability paths out of extension-module basename filtering. Fixes #2796 --- .../src/extensibility/extensions/loader.ts | 27 +++++++------- .../test/extensions-discovery.test.ts | 36 +++++++++++++++++++ 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/packages/coding-agent/src/extensibility/extensions/loader.ts b/packages/coding-agent/src/extensibility/extensions/loader.ts index 6dae41751..41947c630 100644 --- a/packages/coding-agent/src/extensibility/extensions/loader.ts +++ b/packages/coding-agent/src/extensibility/extensions/loader.ts @@ -493,11 +493,12 @@ async function discoverExtensionsInDir(dir: string): Promise { export async function discoverExtensionPaths( configuredPaths: string[], cwd: string, - disabledExtensionIds: string[] = [], + disabledExtensionIds?: string[], ): Promise { const allPaths: string[] = []; const seen = new Set(); - const disabled = new Set(disabledExtensionIds); + const disabled = new Set(disabledExtensionIds ?? []); + const loadOptions = disabledExtensionIds ? { cwd, disabledExtensions: disabledExtensionIds } : { cwd }; const isDisabledName = (name: string): boolean => disabled.has(`extension-module:${name}`); @@ -517,22 +518,22 @@ export async function discoverExtensionPaths( }; // 1. Discover extension modules via capability API (native .omp/.pi only) - const discovered = await loadCapability(extensionModuleCapability.id, { - cwd, - disabledExtensions: disabledExtensionIds, - }); + const discovered = await loadCapability(extensionModuleCapability.id, loadOptions); for (const ext of discovered.items) { if (ext._source.provider !== "native") continue; addPath(ext.path); } // 2. Discover JS/TS hook factories from hookCapability and bind them through - // the extension runner, which owns the current runtime event bus. - const hooks = await loadCapability(hookCapability.id, { - cwd, - disabledExtensions: disabledExtensionIds, - }); - addPaths(hooks.items.map(hook => hook.path).filter(hookPath => isExtensionFile(path.basename(hookPath)))); + // the extension runner, which owns the current runtime event bus. Hook + // capability loading already applies hook-specific disabled ids; do not also + // filter them through extension-module names. + const hooks = await loadCapability(hookCapability.id, loadOptions); + for (const hookPath of hooks.items + .map(hook => hook.path) + .filter(hookPath => isExtensionFile(path.basename(hookPath)))) { + addPath(hookPath); + } // 3. Discover extension entry points from installed plugins addPaths(await getAllPluginExtensionPaths(cwd)); @@ -577,7 +578,7 @@ export async function discoverAndLoadExtensions( configuredPaths: string[], cwd: string, eventBus?: EventBus, - disabledExtensionIds: string[] = [], + disabledExtensionIds?: string[], ): Promise { const paths = await discoverExtensionPaths(configuredPaths, cwd, disabledExtensionIds); return loadExtensions(paths, cwd, eventBus); diff --git a/packages/coding-agent/test/extensions-discovery.test.ts b/packages/coding-agent/test/extensions-discovery.test.ts index d31f8d2f7..270bd5183 100644 --- a/packages/coding-agent/test/extensions-discovery.test.ts +++ b/packages/coding-agent/test/extensions-discovery.test.ts @@ -1,6 +1,8 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as path from "node:path"; +import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { initializeWithSettings } from "@oh-my-pi/pi-coding-agent/discovery"; import { discoverAndLoadExtensions, loadExtensions } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/loader"; import { getProjectAgentDir, TempDir } from "@oh-my-pi/pi-utils"; import { filterUserScoped } from "./utils/filter-user-extensions"; @@ -13,8 +15,10 @@ describe("extensions discovery", () => { tempDir = TempDir.createSync("@pi-ext-test-"); extensionsDir = path.join(getProjectAgentDir(tempDir.path()), "extensions"); fs.mkdirSync(extensionsDir, { recursive: true }); + resetSettingsForTest(); }); afterEach(() => { + resetSettingsForTest(); tempDir.removeSync(); }); @@ -612,6 +616,38 @@ describe("extensions discovery", () => { expect(loadedHook?.handlers.has("tool_call")).toBe(true); }); + it("keeps discovered hooks separate from disabled extension-module ids", async () => { + const extensionPath = path.join(extensionsDir, "guard.ts"); + fs.writeFileSync(extensionPath, extensionCode); + + const hookDir = path.join(getProjectAgentDir(tempDir.path()), "hooks", "pre"); + fs.mkdirSync(hookDir, { recursive: true }); + const hookPath = path.join(hookDir, "guard.ts"); + fs.writeFileSync( + hookPath, + ` + export default function(pi) { + pi.on("tool_call", async () => ({ block: true, reason: "blocked by hook" })); + } + `, + ); + + const settings = await Settings.init({ + inMemory: true, + cwd: tempDir.path(), + overrides: { disabledExtensions: ["extension-module:guard"] }, + }); + initializeWithSettings(settings); + + const result = await discoverForTest(); + const loadedHook = result.extensions.find(extension => extension.path === hookPath); + + expect(result.errors).toHaveLength(0); + expect(result.extensions.find(extension => extension.path === extensionPath)).toBeUndefined(); + expect(loadedHook).toBeDefined(); + expect(loadedHook?.handlers.has("tool_call")).toBe(true); + }); + it("loads extension with shortcuts", async () => { const extCode = ` export default function(pi) {