Merge remote-tracking branch 'origin/farm/cced21e6/hooks-pre-discovery'
This commit is contained in:
+11
-9
@@ -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`.
|
||||
|
||||
@@ -39,6 +39,10 @@
|
||||
|
||||
- Fixed empty CLI sessions being retained after opening `omp` and exiting without a prompt ([#2800](https://github.com/can1357/oh-my-pi/issues/2800)).
|
||||
|
||||
### 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
|
||||
|
||||
@@ -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<string[]> {
|
||||
/**
|
||||
* 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
|
||||
@@ -492,11 +493,12 @@ async function discoverExtensionsInDir(dir: string): Promise<string[]> {
|
||||
export async function discoverExtensionPaths(
|
||||
configuredPaths: string[],
|
||||
cwd: string,
|
||||
disabledExtensionIds: string[] = [],
|
||||
disabledExtensionIds?: string[],
|
||||
): Promise<string[]> {
|
||||
const allPaths: string[] = [];
|
||||
const seen = new Set<string>();
|
||||
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}`);
|
||||
|
||||
@@ -516,17 +518,27 @@ export async function discoverExtensionPaths(
|
||||
};
|
||||
|
||||
// 1. Discover extension modules via capability API (native .omp/.pi only)
|
||||
const discovered = await loadCapability<ExtensionModule>(extensionModuleCapability.id, { cwd });
|
||||
const discovered = await loadCapability<ExtensionModule>(extensionModuleCapability.id, loadOptions);
|
||||
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. Hook
|
||||
// capability loading already applies hook-specific disabled ids; do not also
|
||||
// filter them through extension-module names.
|
||||
const hooks = await loadCapability<Hook>(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));
|
||||
|
||||
// 3. Explicitly configured paths
|
||||
// 4. Explicitly configured paths
|
||||
for (const configuredPath of configuredPaths) {
|
||||
const resolved = resolvePath(configuredPath, cwd);
|
||||
|
||||
@@ -566,7 +578,7 @@ export async function discoverAndLoadExtensions(
|
||||
configuredPaths: string[],
|
||||
cwd: string,
|
||||
eventBus?: EventBus,
|
||||
disabledExtensionIds: string[] = [],
|
||||
disabledExtensionIds?: string[],
|
||||
): Promise<LoadExtensionsResult> {
|
||||
const paths = await discoverExtensionPaths(configuredPaths, cwd, disabledExtensionIds);
|
||||
return loadExtensions(paths, cwd, eventBus);
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
|
||||
@@ -591,6 +595,59 @@ 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("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) {
|
||||
|
||||
Reference in New Issue
Block a user