mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
feat(hook): improvement hook context injection for planning (#459)
* feat(shared): add improvement hook reader utility Adds a runtime-agnostic module for reading improvement hook files from ~/.plannotator/hooks/. Uses a hardcoded base path with an allowlist of known hook files and a 50KB size cap to prevent runaway context injection. Designed for cross-harness reuse (Claude Code, OpenCode, Pi, etc.). For provenance purposes, this commit was AI assisted. * feat(hook): wire up improve-context PreToolUse hook for EnterPlanMode Adds a PreToolUse hook on EnterPlanMode that injects corrective planning instructions into Claude's context at plan mode entry. The new `plannotator improve-context` subcommand reads the improvement hook file and returns additionalContext, or silently passes through if no file exists. For provenance purposes, this commit was AI assisted. * fix(skill): update compound skill improvement hook path Moves the improvement hook file path from ~/.plannotator/compound/ to ~/.plannotator/hooks/compound/ to namespace hook-injectable files separately from other plannotator data. For provenance purposes, this commit was AI assisted. * fix(shared): add legacy path fallback for improvement hook reader Users who ran the compound skill before the path migration have their improvement hook file at ~/.plannotator/compound/ instead of the new ~/.plannotator/hooks/compound/ path. Add a fallback that checks the legacy path only when the new path is absent — if the new path exists but is invalid (empty, oversized), no fallback occurs to prevent resurrecting stale instructions. Includes tests for new-path-wins, legacy-fallback, and invalid-new-blocks-legacy scenarios. For provenance purposes, this commit was AI assisted.
This commit is contained in:
@@ -1,5 +1,17 @@
|
||||
{
|
||||
"hooks": {
|
||||
"PreToolUse": [
|
||||
{
|
||||
"matcher": "EnterPlanMode",
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "plannotator improve-context",
|
||||
"timeout": 5
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"PermissionRequest": [
|
||||
{
|
||||
"matcher": "ExitPlanMode",
|
||||
|
||||
@@ -19,6 +19,7 @@ export function formatTopLevelHelp(): string {
|
||||
" plannotator last",
|
||||
" plannotator archive",
|
||||
" plannotator sessions",
|
||||
" plannotator improve-context",
|
||||
"",
|
||||
"Note:",
|
||||
" running 'plannotator' without arguments is for hook integration and expects JSON on stdin",
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
/**
|
||||
* Plannotator CLI for Claude Code & Copilot CLI
|
||||
*
|
||||
* Supports seven modes:
|
||||
* Supports eight modes:
|
||||
*
|
||||
* 1. Plan Review (default, no args):
|
||||
* - Spawned by ExitPlanMode hook (Claude Code)
|
||||
@@ -37,6 +37,11 @@
|
||||
* - Annotate the last assistant message from a Copilot CLI session
|
||||
* - Parses events.jsonl from session state
|
||||
*
|
||||
* 8. Improve Context (`plannotator improve-context`):
|
||||
* - Spawned by PreToolUse hook on EnterPlanMode
|
||||
* - Reads improvement hook file from ~/.plannotator/hooks/
|
||||
* - Returns additionalContext or silently passes through
|
||||
*
|
||||
* Global flags:
|
||||
* --help - Show top-level usage information
|
||||
* --browser <name> - Override which browser to open (e.g. "Google Chrome")
|
||||
@@ -68,6 +73,7 @@ import { registerSession, unregisterSession, listSessions } from "@plannotator/s
|
||||
import { openBrowser } from "@plannotator/server/browser";
|
||||
import { detectProjectName } from "@plannotator/server/project";
|
||||
import { planDenyFeedback } from "@plannotator/shared/feedback-templates";
|
||||
import { readImprovementHook } from "@plannotator/shared/improvement-hooks";
|
||||
import type { Origin } from "@plannotator/shared/agents";
|
||||
import { findSessionLogsForCwd, resolveSessionLogByPpid, findSessionLogsByAncestorWalk, getLastRenderedMessage, type RenderedMessage } from "./session-log";
|
||||
import { findCodexRolloutByThreadId, getLastCodexMessage } from "./codex-session";
|
||||
@@ -700,6 +706,37 @@ if (args[0] === "sessions") {
|
||||
console.log(result.feedback || "No feedback provided.");
|
||||
process.exit(0);
|
||||
|
||||
} else if (args[0] === "improve-context") {
|
||||
// ============================================
|
||||
// IMPROVEMENT HOOK CONTEXT INJECTION MODE
|
||||
// ============================================
|
||||
//
|
||||
// Called by PreToolUse hook on EnterPlanMode.
|
||||
// Reads the improvement hook file and returns additionalContext.
|
||||
// No file = exit 0 silently (passthrough).
|
||||
|
||||
// Must consume stdin (Claude Code hooks deliver event JSON on stdin)
|
||||
await Bun.stdin.text();
|
||||
|
||||
const hook = readImprovementHook("enterplanmode-improve");
|
||||
if (!hook) process.exit(0);
|
||||
|
||||
const context = [
|
||||
"[Plannotator Improvement Hook]",
|
||||
"The following corrective instructions were generated from analysis of previous plan denial patterns.",
|
||||
"Apply these guidelines when writing your plan:\n",
|
||||
hook.content,
|
||||
].join("\n");
|
||||
|
||||
console.log(JSON.stringify({
|
||||
hookSpecificOutput: {
|
||||
hookEventName: "PreToolUse",
|
||||
additionalContext: context,
|
||||
},
|
||||
}));
|
||||
|
||||
process.exit(0);
|
||||
|
||||
} else {
|
||||
// ============================================
|
||||
// PLAN REVIEW MODE (default)
|
||||
|
||||
@@ -473,10 +473,10 @@ every future planning session automatically.
|
||||
The hook file lives at:
|
||||
|
||||
```
|
||||
~/.plannotator/compound/enterplanmode-improve-hook.txt
|
||||
~/.plannotator/hooks/compound/enterplanmode-improve-hook.txt
|
||||
```
|
||||
|
||||
Create the `~/.plannotator/compound/` directory if it doesn't exist.
|
||||
Create the `~/.plannotator/hooks/compound/` directory if it doesn't exist.
|
||||
|
||||
The file contents should be the corrective prompt instructions from Phase 3 —
|
||||
the same numbered list that appears in section 7 of the HTML report. Write them
|
||||
|
||||
@@ -0,0 +1,135 @@
|
||||
/**
|
||||
* Tests for improvement hook reader.
|
||||
*
|
||||
* Run: bun test packages/shared/improvement-hooks.test.ts
|
||||
*/
|
||||
|
||||
import { describe, expect, test, beforeEach, afterEach } from "bun:test";
|
||||
import { mkdirSync, writeFileSync, rmSync, existsSync } from "fs";
|
||||
import { join } from "path";
|
||||
import { tmpdir } from "os";
|
||||
|
||||
// We need to override the base dirs used by readImprovementHook.
|
||||
// Since the module uses homedir() at import time, we mock it via
|
||||
// a test harness that sets HOME to a temp directory.
|
||||
|
||||
const TEST_HOME = join(tmpdir(), `improvement-hooks-test-${Date.now()}`);
|
||||
const NEW_BASE = join(TEST_HOME, ".plannotator", "hooks");
|
||||
const LEGACY_BASE = join(TEST_HOME, ".plannotator");
|
||||
const HOOK_RELATIVE = "compound/enterplanmode-improve-hook.txt";
|
||||
|
||||
function setupTestHome() {
|
||||
mkdirSync(join(NEW_BASE, "compound"), { recursive: true });
|
||||
mkdirSync(join(LEGACY_BASE, "compound"), { recursive: true });
|
||||
}
|
||||
|
||||
function cleanTestHome() {
|
||||
if (existsSync(TEST_HOME)) {
|
||||
rmSync(TEST_HOME, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
// Since the module reads homedir() at import time, we need to
|
||||
// re-import with HOME overridden. Use a helper that spawns a
|
||||
// small inline script to test each scenario in isolation.
|
||||
async function runScenario(setup: {
|
||||
newPathContent?: string | null;
|
||||
legacyPathContent?: string | null;
|
||||
}): Promise<{ content: string; filePath: string } | null> {
|
||||
setupTestHome();
|
||||
|
||||
const newPath = join(NEW_BASE, HOOK_RELATIVE);
|
||||
const legacyPath = join(LEGACY_BASE, HOOK_RELATIVE);
|
||||
|
||||
if (setup.newPathContent !== undefined && setup.newPathContent !== null) {
|
||||
writeFileSync(newPath, setup.newPathContent);
|
||||
}
|
||||
if (setup.legacyPathContent !== undefined && setup.legacyPathContent !== null) {
|
||||
writeFileSync(legacyPath, setup.legacyPathContent);
|
||||
}
|
||||
|
||||
// Run in a subprocess with HOME overridden so homedir() returns TEST_HOME
|
||||
const proc = Bun.spawn(
|
||||
[
|
||||
"bun",
|
||||
"-e",
|
||||
`
|
||||
import { readImprovementHook } from "./packages/shared/improvement-hooks";
|
||||
const result = readImprovementHook("enterplanmode-improve");
|
||||
console.log(JSON.stringify(result));
|
||||
`,
|
||||
],
|
||||
{
|
||||
env: { ...process.env, HOME: TEST_HOME },
|
||||
cwd: join(import.meta.dir, "../.."),
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
},
|
||||
);
|
||||
|
||||
const stdout = await new Response(proc.stdout).text();
|
||||
const exitCode = await proc.exited;
|
||||
|
||||
if (exitCode !== 0) {
|
||||
const stderr = await new Response(proc.stderr).text();
|
||||
throw new Error(`Subprocess failed (exit ${exitCode}): ${stderr}`);
|
||||
}
|
||||
|
||||
const parsed = JSON.parse(stdout.trim());
|
||||
return parsed;
|
||||
}
|
||||
|
||||
describe("readImprovementHook", () => {
|
||||
beforeEach(setupTestHome);
|
||||
afterEach(cleanTestHome);
|
||||
|
||||
test("returns content from new path when file exists", async () => {
|
||||
const result = await runScenario({
|
||||
newPathContent: "Focus on error handling",
|
||||
});
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.content).toBe("Focus on error handling");
|
||||
expect(result!.filePath).toContain(".plannotator/hooks/compound/");
|
||||
});
|
||||
|
||||
test("new path wins over legacy path", async () => {
|
||||
const result = await runScenario({
|
||||
newPathContent: "New instructions",
|
||||
legacyPathContent: "Old instructions",
|
||||
});
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.content).toBe("New instructions");
|
||||
expect(result!.filePath).toContain(".plannotator/hooks/compound/");
|
||||
});
|
||||
|
||||
test("falls back to legacy path when new path is absent", async () => {
|
||||
const result = await runScenario({
|
||||
legacyPathContent: "Legacy instructions",
|
||||
});
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.content).toBe("Legacy instructions");
|
||||
expect(result!.filePath).toContain(".plannotator/compound/");
|
||||
expect(result!.filePath).not.toContain(".plannotator/hooks/");
|
||||
});
|
||||
|
||||
test("returns null when new path exists but is empty (no legacy fallback)", async () => {
|
||||
const result = await runScenario({
|
||||
newPathContent: "",
|
||||
legacyPathContent: "Legacy instructions",
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
test("returns null when no files exist", async () => {
|
||||
const result = await runScenario({});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
test("returns null when new path is whitespace-only (no legacy fallback)", async () => {
|
||||
const result = await runScenario({
|
||||
newPathContent: " \n \n ",
|
||||
legacyPathContent: "Legacy instructions",
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,105 @@
|
||||
/**
|
||||
* Improvement Hook Reader
|
||||
*
|
||||
* Reads improvement hook files from ~/.plannotator/hooks/.
|
||||
* Falls back to the legacy path (~/.plannotator/) when the new-path
|
||||
* file is absent, for compatibility with files written before the
|
||||
* path migration. If the new-path file exists but is invalid (empty,
|
||||
* oversized, not a regular file), the legacy path is NOT consulted —
|
||||
* this prevents resurrecting stale instructions.
|
||||
*
|
||||
* Runtime-agnostic: uses only node:fs, node:path, node:os.
|
||||
*
|
||||
* Security model:
|
||||
* - Hardcoded base paths (no user input determines file path)
|
||||
* - KNOWN_HOOKS allowlist (only pre-registered relative paths)
|
||||
* - Size cap to prevent runaway context injection
|
||||
* - Same trust model as ~/.plannotator/config.json
|
||||
*/
|
||||
|
||||
import { homedir } from "os";
|
||||
import { join } from "path";
|
||||
import { readFileSync, statSync } from "fs";
|
||||
|
||||
/** Base directory for hook-injectable files (new path) */
|
||||
const HOOKS_BASE_DIR = join(homedir(), ".plannotator", "hooks");
|
||||
|
||||
/** Legacy base directory (pre-migration path) */
|
||||
const LEGACY_BASE_DIR = join(homedir(), ".plannotator");
|
||||
|
||||
/** Maximum file size to read (50 KB) */
|
||||
const MAX_FILE_SIZE = 50 * 1024;
|
||||
|
||||
/**
|
||||
* Known improvement hook file paths, keyed by hook name.
|
||||
* `path` is relative to HOOKS_BASE_DIR (~/.plannotator/hooks/).
|
||||
* `legacyPath` is relative to LEGACY_BASE_DIR (~/.plannotator/).
|
||||
*/
|
||||
const KNOWN_HOOKS = {
|
||||
"enterplanmode-improve": {
|
||||
path: "compound/enterplanmode-improve-hook.txt",
|
||||
legacyPath: "compound/enterplanmode-improve-hook.txt",
|
||||
},
|
||||
} as const;
|
||||
|
||||
export type ImprovementHookName = keyof typeof KNOWN_HOOKS;
|
||||
|
||||
export interface ImprovementHookResult {
|
||||
content: string;
|
||||
hookName: ImprovementHookName;
|
||||
filePath: string;
|
||||
}
|
||||
|
||||
/** Check whether a path exists on disk (any file type). */
|
||||
function fileExists(path: string): boolean {
|
||||
try {
|
||||
statSync(path);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Validate and read a hook file. Returns the result, or null if invalid. */
|
||||
function tryReadHookFile(
|
||||
filePath: string,
|
||||
hookName: ImprovementHookName,
|
||||
): ImprovementHookResult | null {
|
||||
try {
|
||||
const stat = statSync(filePath);
|
||||
if (!stat.isFile() || stat.size === 0 || stat.size > MAX_FILE_SIZE) return null;
|
||||
|
||||
const content = readFileSync(filePath, "utf-8").trim();
|
||||
if (!content) return null;
|
||||
|
||||
return { content, hookName, filePath };
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Read an improvement hook file by name.
|
||||
*
|
||||
* Lookup order:
|
||||
* 1. New path (HOOKS_BASE_DIR + path). If it exists and validates, return it.
|
||||
* 2. If the new path exists but is invalid (empty, oversized, etc.), return null.
|
||||
* 3. Only if the new path does not exist, try the legacy path (LEGACY_BASE_DIR + legacyPath).
|
||||
*/
|
||||
export function readImprovementHook(
|
||||
hookName: ImprovementHookName,
|
||||
): ImprovementHookResult | null {
|
||||
const entry = KNOWN_HOOKS[hookName];
|
||||
if (!entry) return null;
|
||||
|
||||
const newPath = join(HOOKS_BASE_DIR, entry.path);
|
||||
|
||||
// New path exists — use it exclusively (even if invalid)
|
||||
if (fileExists(newPath)) {
|
||||
return tryReadHookFile(newPath, hookName);
|
||||
}
|
||||
|
||||
// New path absent — fall back to legacy path
|
||||
const legacyFilePath = join(LEGACY_BASE_DIR, entry.legacyPath);
|
||||
return tryReadHookFile(legacyFilePath, hookName);
|
||||
}
|
||||
@@ -21,6 +21,7 @@
|
||||
"./resolve-file": "./resolve-file.ts",
|
||||
"./external-annotation": "./external-annotation.ts",
|
||||
"./agent-jobs": "./agent-jobs.ts",
|
||||
"./config": "./config.ts"
|
||||
"./config": "./config.ts",
|
||||
"./improvement-hooks": "./improvement-hooks.ts"
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user