diff --git a/agenthook/AGENTS.md b/agenthook/AGENTS.md index 9477b48..da05767 100644 --- a/agenthook/AGENTS.md +++ b/agenthook/AGENTS.md @@ -30,6 +30,12 @@ timeout units, timeout fields, failure policy, and cross-platform command fields. Decision-bearing Cursor registrations are fail-closed because Cursor otherwise allows the operation when a hook crashes or emits invalid JSON. +- Script profiles (Pi) own one kit-named module. Edit only its delimited + registration block, refuse a file without that block, rewrite the runtime + on every write, and spawn registered argv without a shell on every OS. + The Pi module reports only from Pi's interactive terminal (`ctx.mode` is + `tui`), so print, json, and rpc runs, subagents included, stay silent. It + needs Pi 0.80.4 or later (`ctx.mode` 0.78.1, `agent_settled` 0.80.4). - Keep each harness profile in its own agent-named file (`claude.go`, `codex.go`, and so on). `profile.go` owns only the shared vocabulary, registry, and lookup behavior. diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index 898b350..3f9e3fc 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -3,10 +3,14 @@ package agenthook import ( "encoding/json" "fmt" + "io" "os" + "os/exec" + "os/signal" "path/filepath" "runtime" "strings" + "syscall" "testing" "time" @@ -24,7 +28,7 @@ func TestProfilesExposeClaudeStyleEvents(t *testing.T) { require := require.New(t) profiles := Profiles() - require.Len(profiles, 8) + require.Len(profiles, 9) assert.Equal([]Agent{ AgentClaude, AgentCodex, @@ -33,6 +37,7 @@ func TestProfilesExposeClaudeStyleEvents(t *testing.T) { AgentDroid, AgentGemini, AgentHermes, + AgentPi, AgentQwen, }, []Agent{ profiles[0].Agent, @@ -43,10 +48,15 @@ func TestProfilesExposeClaudeStyleEvents(t *testing.T) { profiles[5].Agent, profiles[6].Agent, profiles[7].Agent, + profiles[8].Agent, }) assert.Contains(profiles[6].SupportedEvents, EventPreToolUse) assert.NotContains(profiles[6].SupportedEvents, EventNotification) - assert.Contains(profiles[7].SupportedEvents, EventPermissionRequest) + assert.Equal( + []Event{EventSessionStart, EventUserPromptSubmit, EventStop, EventSessionEnd}, + profiles[7].SupportedEvents, + ) + assert.Contains(profiles[8].SupportedEvents, EventPermissionRequest) } func TestPlanInstallDefaultsToEveryProfileEvent(t *testing.T) { @@ -139,21 +149,23 @@ func TestPlanInstallBuildsCommandFromExecutable(t *testing.T) { assert.NotContains(t, handler, "args") } -func TestPlanInstallRejectsWindowsShimForClaude(t *testing.T) { +func TestPlanInstallRejectsWindowsShim(t *testing.T) { if runtime.GOOS != "windows" { - t.Skip("Claude exec form is written only on Windows") + t.Skip("shims need a shell only on Windows") } - for _, executable := range []string{`C:\tools\hook.cmd`, `C:\tools\hook.BAT`} { - t.Run(executable, func(t *testing.T) { - _, err := PlanInstall(AgentClaude, InstallOptions{ - ConfigPath: filepath.Join(t.TempDir(), "settings.json"), - Executable: executable, - Arguments: []string{"agent-hook", "run", "--source", "shared-agent-hook-test"}, - Marker: testMarker, - }) + for _, agent := range []Agent{AgentClaude, AgentPi} { + for _, executable := range []string{`C:\tools\hook.cmd`, `C:\tools\hook.BAT`} { + t.Run(string(agent)+" "+executable, func(t *testing.T) { + _, err := PlanInstall(agent, InstallOptions{ + ConfigPath: filepath.Join(t.TempDir(), "hook-config"), + Executable: executable, + Arguments: []string{"agent-hook", "run", "--source", "shared-agent-hook-test"}, + Marker: testMarker, + }) - require.ErrorContains(t, err, "pass the executable it launches") - }) + require.ErrorContains(t, err, "pass the executable it launches") + }) + } } } @@ -754,6 +766,36 @@ func TestNormalizeConvertsNativePayloadToClaudeShape(t *testing.T) { }, }, }, + { + name: "pi session start reason", + agent: AgentPi, + input: `{"session_id":"p1","hook_event_name":"session_start","reason":"startup"}`, + want: map[string]any{"session_id": "p1", "hook_event_name": "SessionStart", "source": "startup"}, + }, + { + name: "pi new session clears", + agent: AgentPi, + input: `{"session_id":"p1","hook_event_name":"session_start","reason":"new"}`, + want: map[string]any{"hook_event_name": "SessionStart", "source": "clear"}, + }, + { + name: "pi session replaced by resume", + agent: AgentPi, + input: `{"session_id":"p1","hook_event_name":"session_shutdown","reason":"resume"}`, + want: map[string]any{"hook_event_name": "SessionEnd", "reason": "resume"}, + }, + { + name: "pi session replaced by fork", + agent: AgentPi, + input: `{"session_id":"p1","hook_event_name":"session_shutdown","reason":"fork"}`, + want: map[string]any{"hook_event_name": "SessionEnd", "reason": "other"}, + }, + { + name: "pi prompt", + agent: AgentPi, + input: `{"session_id":"p1","hook_event_name":"before_agent_start","prompt":"fix it"}`, + want: map[string]any{"session_id": "p1", "hook_event_name": "UserPromptSubmit", "prompt": "fix it"}, + }, { name: "qwen shell tool", agent: AgentQwen, @@ -907,3 +949,338 @@ func TestWriteConfigRefusesLinkSwappedInForRegularConfig(t *testing.T) { require.NoError(err) assert.Equal(t, "other", string(data)) } + +func TestConfigPathNormalizesPiAgentDirAsPiDoes(t *testing.T) { + home, err := os.UserHomeDir() + require.NoError(t, err) + windows := runtime.GOOS == "windows" + pick := func(onWindows, elsewhere string) string { + if windows { + return onWindows + } + return elsewhere + } + tests := []struct { + env string + want string + }{ + {env: "~", want: home}, + {env: "~/pi-agent", want: filepath.Join(home, "pi-agent")}, + {env: `~\pi-agent`, want: pick(filepath.Join(home, "pi-agent"), `~\pi-agent`)}, + {env: "/c/Users/me/pi", want: pick(`C:\Users\me\pi`, "/c/Users/me/pi")}, + {env: "/mnt/d/pi", want: pick(`D:\pi`, "/mnt/d/pi")}, + {env: "/cygdrive/e", want: pick(`E:\`, "/cygdrive/e")}, + {env: "//server/share", want: "//server/share"}, + } + for _, tt := range tests { + t.Run(tt.env, func(t *testing.T) { + t.Setenv("PI_CODING_AGENT_DIR", tt.env) + + path, err := ConfigPath(AgentPi) + + require.NoError(t, err) + assert.Equal(t, filepath.Join(tt.want, "extensions", "agenthook.js"), path) + }) + } +} + +// piScriptHooks parses the registration block of a generated Pi extension. +func piScriptHooks(t *testing.T, path string) map[string]any { + t.Helper() + data, err := os.ReadFile(path) + require.NoError(t, err) + block, err := scriptBlock(data, path) + require.NoError(t, err) + var root map[string]any + require.NoError(t, json.Unmarshal(block, &root)) + hooks, _ := root["hooks"].(map[string]any) + return hooks +} + +func piCommands(hooks map[string]any, event string) []string { + var commands []string + entries, _ := hooks[event].([]any) + for _, entry := range entries { + handlers, _ := entry.(map[string]any)["hooks"].([]any) + for _, handler := range handlers { + fields, _ := handler.(map[string]any) + command, _ := fields["command"].(string) + argv := []string{command} + args, _ := fields["args"].([]any) + for _, arg := range args { + argv = append(argv, arg.(string)) + } + commands = append(commands, strings.Join(argv, " ")) + } + } + return commands +} + +func TestInstallPiKeepsOtherApplicationsCommands(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + path := filepath.Join(t.TempDir(), "extensions", "agenthook.js") + install := func(executable, source string, extra ...string) Result { + result, err := Install(AgentPi, InstallOptions{ + ConfigPath: path, + Executable: executable, + Arguments: append(append([]string{"agent-hook"}, extra...), "--source", source), + Marker: "--source " + source, + }) + require.NoError(err) + return result + } + // B's argument is a block delimiter, which must not end the registration block. + const bCommand = "/opt/b agent-hook " + scriptBlockEnd + " --source b-hook" + + install("/opt/a", "a-hook") + install("/opt/b", "b-hook", scriptBlockEnd) + assert.Equal( + []string{"/opt/a agent-hook --source a-hook", bCommand}, + piCommands(piScriptHooks(t, path), "session_start"), + ) + assert.False(install("/opt/b", "b-hook", scriptBlockEnd).Changed) + + install("/moved/a", "a-hook") + hooks := piScriptHooks(t, path) + assert.Equal( + []string{bCommand, "/moved/a agent-hook --source a-hook"}, + piCommands(hooks, "agent_settled"), + ) + assert.Len(piCommands(hooks, "before_agent_start"), 2) + + result, err := Uninstall(AgentPi, path, "--source a-hook") + require.NoError(err) + assert.True(result.Changed) + assert.Equal([]string{bCommand}, piCommands(piScriptHooks(t, path), "session_start")) + + result, err = Uninstall(AgentPi, path, "--source b-hook") + require.NoError(err) + assert.True(result.Changed) + assert.Empty(piScriptHooks(t, path)) + + result, err = Uninstall(AgentPi, filepath.Join(t.TempDir(), "missing.js"), "--source b-hook") + require.NoError(err) + assert.False(result.Changed) +} + +func TestPlanInstallPiRefusesForeignFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "agenthook.js") + original := []byte("export default function (pi) {}\n") + require.NoError(t, os.WriteFile(path, original, 0o600)) + + _, err := Install(AgentPi, InstallOptions{ + ConfigPath: path, + Executable: "/opt/hook", + Arguments: []string{"--source", "shared-agent-hook-test"}, + Marker: testMarker, + }) + + require.ErrorContains(t, err, "not written by agenthook") + data, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, original, data) +} + +func TestPlanInstallPiRequiresExecutableWithoutMatchers(t *testing.T) { + path := filepath.Join(t.TempDir(), "agenthook.js") + + _, err := PlanInstall(AgentPi, InstallOptions{ + ConfigPath: path, + Command: "/opt/hook " + testMarker, + Marker: testMarker, + }) + require.ErrorContains(t, err, "need Executable and Arguments") + + _, err = PlanInstall(AgentPi, InstallOptions{ + ConfigPath: path, + Executable: "/opt/hook", + Arguments: []string{"--source", "shared-agent-hook-test"}, + Marker: testMarker, + Hooks: []Hook{{Event: EventStop, Matcher: ToolBash}}, + }) + require.ErrorContains(t, err, "do not support matchers") +} + +func TestPiExtensionHelper(t *testing.T) { + out := os.Getenv("KIT_AGENTHOOK_PI_HELPER_OUT") + if out == "" { + return + } + payload, err := io.ReadAll(os.Stdin) + require.NoError(t, err) + var fields map[string]any + require.NoError(t, json.Unmarshal(payload, &fields)) + fields["helper_cwd"], err = os.Getwd() + require.NoError(t, err) + payload, err = json.Marshal(fields) + require.NoError(t, err) + file, err := os.OpenFile(out, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) + require.NoError(t, err) + _, err = file.Write(append(payload, '\n')) + require.NoError(t, err) + require.NoError(t, file.Close()) + if strings.Contains(string(payload), "agent_settled") { + // Ignore SIGTERM and outlive the 1s hook timeout, so only a forced kill + // with an unconditional deadline keeps the extension from waiting. + signal.Ignore(syscall.SIGTERM) + <-time.After(4 * time.Minute) + } +} + +const piExtensionDriver = ` +import { existsSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +const extension = await import(pathToFileURL(process.argv[2]).href); +const fileA = join(process.argv[3], "a.jsonl"); +const fileB = join(process.argv[3], "b.jsonl"); +const runtime = () => { + const handlers = {}; + extension.default({ on: (name, handler) => { handlers[name] = handler; } }); + return handlers; +}; +const ctx = (id, file, mode = "tui") => ({ + mode, + cwd: process.argv[3], + sessionManager: { getSessionId: () => id, getSessionFile: () => file }, +}); +const fire = async (pi, name, event, context) => { + try { + await pi[name]({ type: name, ...event }, context); + } catch (error) { + console.log(name + ": " + error.message); + } +}; +const nothingSent = (when) => { + if (existsSync(process.env.KIT_AGENTHOOK_PI_HELPER_OUT)) console.log("sent " + when); +}; + +// --no-session: Pi never writes a session file, so nothing is reported. +let pi = runtime(); +await fire(pi, "session_start", { reason: "startup" }, ctx("memory", undefined)); +await fire(pi, "before_agent_start", { prompt: "zero" }, ctx("memory", undefined)); +await fire(pi, "context", { messages: [] }, ctx("memory", undefined)); +await fire(pi, "agent_settled", {}, ctx("memory", undefined)); +await fire(pi, "session_shutdown", { reason: "new" }, ctx("memory", undefined)); +nothingSent("without a session file"); + +// Fresh start: SessionStart and the first prompt wait until Pi appends the user +// message, which happens before the first context event. +pi = runtime(); +await fire(pi, "session_start", { reason: "startup" }, ctx("a", fileA)); +await fire(pi, "before_agent_start", { prompt: "one" }, ctx("a", fileA)); +nothingSent("before the session file existed"); +// The header line Pi's SessionManager writes first: +// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/session-manager.ts#L1061-L1071 +writeFileSync(fileA, JSON.stringify({ type: "session", version: 3, id: "a", timestamp: new Date().toISOString(), cwd: "/work" }) + "\n"); +await fire(pi, "context", { messages: [] }, ctx("a", fileA)); +await fire(pi, "agent_settled", {}, ctx("a", fileA)); + +// /new: the replaced session ends; the new one is replaced before Pi saves it, so it sends nothing. +await fire(pi, "session_shutdown", { reason: "new" }, ctx("a", fileA)); +pi = runtime(); +await fire(pi, "session_start", { reason: "new" }, ctx("b", fileB)); +await fire(pi, "session_shutdown", { reason: "resume" }, ctx("b", fileB)); + +// Resume: the session file exists, so SessionStart reports at once; other modes and quit stay silent. +pi = runtime(); +await fire(pi, "session_start", { reason: "resume" }, ctx("a", fileA, "json")); +await fire(pi, "session_start", { reason: "resume" }, ctx("a", fileA)); +await fire(pi, "session_shutdown", { reason: "quit" }, ctx("a", fileA)); +` + +func TestPiExtensionReportsResumableSessions(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + node, err := exec.LookPath("node") + if err != nil { + t.Skip("node not available") + } + dir := t.TempDir() + path := filepath.Join(dir, "agenthook.js") + missing := filepath.Join(dir, "missing-hook") + _, err = Install(AgentPi, InstallOptions{ + ConfigPath: path, + Executable: missing, + Arguments: []string{"--source", "missing-hook"}, + Marker: "--source missing-hook", + Hooks: []Hook{{Event: EventSessionStart}}, + }) + require.NoError(err) + _, err = Install(AgentPi, InstallOptions{ + ConfigPath: path, + Executable: os.Args[0], + Arguments: []string{"-test.run=^TestPiExtensionHelper$", "--", "--source", "shared-agent-hook-test"}, + Marker: testMarker, + Hooks: []Hook{ + {Event: EventSessionStart}, + {Event: EventUserPromptSubmit}, + {Event: EventSessionEnd}, + {Event: EventStop, Timeout: time.Second}, + }, + }) + require.NoError(err) + data, err := os.ReadFile(path) + require.NoError(err) + module := filepath.Join(dir, "extension.mjs") + require.NoError(os.WriteFile(module, data, 0o600)) + driver := filepath.Join(dir, "driver.mjs") + require.NoError(os.WriteFile(driver, []byte(piExtensionDriver), 0o600)) + out := filepath.Join(dir, "payloads.jsonl") + cmd := exec.CommandContext(t.Context(), node, driver, module, dir) + cmd.Env = append(os.Environ(), "KIT_AGENTHOOK_PI_HELPER_OUT="+out) + + started := time.Now() + output, err := cmd.CombinedOutput() + + require.NoError(err, string(output)) + // Node waits for a live child, so finishing well before the helper's sleep + // ends proves the kill; a loaded runner can spend tens of seconds on re-execs. + assert.Less(time.Since(started), 3*time.Minute, "the timed-out command was not killed") + payloads, err := os.ReadFile(out) + require.NoError(err) + type report struct { + Event string `json:"hook_event_name"` + SessionID string `json:"session_id"` + Transcript string `json:"transcript_path"` + Reason string `json:"reason"` + Prompt string `json:"prompt"` + Cwd string `json:"cwd"` + HelperCwd string `json:"helper_cwd"` + } + // macOS reports the working directory through /tmp's symlink target. + realDir, err := filepath.EvalSymlinks(dir) + require.NoError(err) + var reports []report + for line := range strings.Lines(strings.TrimSpace(string(payloads))) { + var r report + require.NoError(json.Unmarshal([]byte(line), &r)) + assert.Equal(dir, r.Cwd) + helperDir, err := filepath.EvalSymlinks(r.HelperCwd) + require.NoError(err) + assert.Equal(realDir, helperDir, "the hook ran outside the session's directory") + r.Cwd, r.HelperCwd = "", "" + reports = append(reports, r) + } + fileA := filepath.Join(dir, "a.jsonl") + for _, r := range reports { + assert.FileExists(r.Transcript, "reported %s for an unsaved session", r.Event) + } + assert.Equal([]report{ + {Event: "session_start", SessionID: "a", Transcript: fileA, Reason: "startup"}, + {Event: "before_agent_start", SessionID: "a", Transcript: fileA, Prompt: "one"}, + {Event: "agent_settled", SessionID: "a", Transcript: fileA}, + {Event: "session_shutdown", SessionID: "a", Transcript: fileA, Reason: "new"}, + {Event: "session_start", SessionID: "a", Transcript: fileA, Reason: "resume"}, + }, reports) + // A failed command is reported once its event's other commands have run. + failed := "agenthook session_start commands failed: " + missing + " --source missing-hook: could not start" + failures := strings.Split(strings.TrimSpace(string(output)), "\n") + require.Len(failures, 3, string(output)) + assert.Contains(failures[0], "context: "+failed) + assert.Contains(failures[1], "agent_settled: agenthook agent_settled commands failed: ") + assert.Contains(failures[1], "timed out after 1s") + assert.Contains(failures[2], "session_start: "+failed) + assert.NotContains(string(output), "TestPiExtensionHelper$ -- --source shared-agent-hook-test: could not start") +} diff --git a/agenthook/config.go b/agenthook/config.go index d818f07..e27ec1a 100644 --- a/agenthook/config.go +++ b/agenthook/config.go @@ -90,12 +90,20 @@ func PlanInstall(agent Agent, opts InstallOptions) (Result, error) { // writes a command string on this platform. func execArgv(spec profileSpec, opts InstallOptions) ([]string, error) { executable := strings.TrimSpace(opts.Executable) - if executable == "" || spec.windowsCommandStyle != windowsCommandExec || - runtime.GOOS != "windows" { + if spec.format == formatScript && executable == "" { + return nil, fmt.Errorf( + "%s hooks need Executable and Arguments; a raw command needs a shell", + spec.profile.DisplayName, + ) + } + if spec.format != formatScript && (executable == "" || + spec.windowsCommandStyle != windowsCommandExec || runtime.GOOS != "windows") { return nil, nil } - if ext := filepath.Ext(executable); strings.EqualFold(ext, ".cmd") || - strings.EqualFold(ext, ".bat") { + // Node refuses to spawn .cmd and .bat files without a shell: + // https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2 + if ext := filepath.Ext(executable); runtime.GOOS == "windows" && + (strings.EqualFold(ext, ".cmd") || strings.EqualFold(ext, ".bat")) { return nil, fmt.Errorf( "%s hooks on Windows cannot run %s shim %s without a shell; "+ "pass the executable it launches", @@ -204,6 +212,10 @@ func prepareInstall(agent Agent, opts InstallOptions) (profileSpec, string, []na if spec.format == formatHermesYAML && hook.Timeout > 300*time.Second { return profileSpec{}, "", nil, errors.New("Hermes hook timeout must not exceed 300 seconds") } + // The generated module runs every command for its event, so a matcher would be dropped silently. + if spec.format == formatScript && strings.TrimSpace(hook.Matcher) != "" { + return profileSpec{}, "", nil, fmt.Errorf("%s hooks do not support matchers", spec.profile.DisplayName) + } matcher := nativeMatcher(spec, strings.TrimSpace(hook.Matcher)) if spec.format == formatHermesYAML && matcher != "" && hook.Event != EventPreToolUse && hook.Event != EventPostToolUse { @@ -252,6 +264,8 @@ func planConfig( ) case formatHermesYAML: return planHermesConfig(path, marker, command, hooks, uninstall) + case formatScript: + return planScriptConfig(spec, path, marker, argv, hooks, uninstall) default: return nil, false, errors.New("unsupported agent hook config format") } diff --git a/agenthook/doc.go b/agenthook/doc.go index cc9bc09..5a3e7b7 100644 --- a/agenthook/doc.go +++ b/agenthook/doc.go @@ -6,7 +6,17 @@ // and file format used by each harness. This lets applications describe one set // of lifecycle hooks while support for new agents stays centralized in kit. // Profiles are provided for Claude Code, Codex, GitHub Copilot CLI, Cursor, -// Factory Droid, Gemini CLI, Hermes Agent, and Qwen Code. +// Factory Droid, Gemini CLI, Hermes Agent, Pi, and Qwen Code. Pi has no +// command-hook config, so its profile writes a kit-owned extension module that +// runs the registered commands. That module reports only from Pi's interactive +// terminal, so print, json, and rpc runs, subagents included, stay silent. Its +// SessionEnd fires only when another session replaces the current one (new, +// resume, or fork), never on quit or reload. The module reports an event only +// once Pi has saved the session file, so every reported ID resumes: a new +// session's SessionStart and first prompt wait for that save, and a +// --no-session run reports nothing. A root index.ts, index.js, or package.json +// pi.extensions in Pi's extensions directory stops Pi from loading the module. +// It needs Pi 0.80.4 or later. // // Applications identify their hooks with a stable marker embedded in the // command. Reinstalling replaces commands carrying that marker even when the diff --git a/agenthook/handler_test.go b/agenthook/handler_test.go index f86813a..d253f02 100644 --- a/agenthook/handler_test.go +++ b/agenthook/handler_test.go @@ -627,6 +627,16 @@ func TestHandleAllowsNativeLifecyclePayloadWithoutClaudeEquivalent(t *testing.T) assert.Empty(t, handler.sessionStart.Source) }, }, + { + name: "Pi reload without Claude source", + agent: AgentPi, + payload: `{"session_id":"p1","hook_event_name":"session_start","reason":"reload"}`, + check: func(t *testing.T, handler *lifecycleHandler) { + t.Helper() + require.NotNil(t, handler.sessionStart) + assert.Empty(t, handler.sessionStart.Source) + }, + }, { name: "Hermes session end without Claude reason", agent: AgentHermes, @@ -848,3 +858,34 @@ func TestHandleRejectsOversizedPayload(t *testing.T) { require.Error(t, err) assert.ErrorContains(t, err, "hook payload exceeds") } + +func TestHandleDispatchesPiSessionEnd(t *testing.T) { + var output bytes.Buffer + handler := &lifecycleHandler{} + + err := Handle( + t.Context(), AgentPi, + strings.NewReader(`{"session_id":"pi-1","hook_event_name":"session_shutdown","reason":"new"}`), + &output, handler, + ) + + require.NoError(t, err) + require.NotNil(t, handler.sessionEnd) + assert.Equal(t, "pi-1", handler.sessionEnd.SessionID) + assert.Equal(t, SessionEndClear, handler.sessionEnd.Reason) + assert.JSONEq(t, `{}`, output.String()) +} + +func TestHandleRejectsPiControlOutput(t *testing.T) { + var output bytes.Buffer + handler := stopHandler{output: StopOutput{Decision: DecisionBlock, Reason: "work remains"}} + + err := Handle( + t.Context(), AgentPi, + strings.NewReader(`{"session_id":"pi-1","hook_event_name":"agent_settled"}`), + &output, handler, + ) + + require.ErrorContains(t, err, "does not support Stop control output") + assert.Empty(t, output.String()) +} diff --git a/agenthook/json.go b/agenthook/json.go index eec1a00..a152bbb 100644 --- a/agenthook/json.go +++ b/agenthook/json.go @@ -24,47 +24,11 @@ func planNestedJSONConfig( if err != nil { return nil, false, fmt.Errorf("encode existing agent hook config %s: %w", path, err) } - - hooksObject, err := jsonHooksObject(root, path, !uninstall) - if err != nil { + if err := applyNestedJSONHooks( + root, path, marker, command, commandWindows, argv, hooks, uninstall, + ); err != nil { return nil, false, err } - if hooksObject != nil { - if err := removeOwnedJSONHooks(hooksObject, marker, path); err != nil { - return nil, false, err - } - } - if !uninstall { - if hooksObject == nil { - return nil, false, fmt.Errorf("agent hook config %s has no hooks object", path) - } - for _, hook := range hooks { - entry := map[string]any{} - if hook.matcher != "" { - entry["matcher"] = hook.matcher - } - handler := map[string]any{ - "type": "command", - "command": command, - } - if len(argv) > 0 { - handler["command"] = argv[0] - handler["args"] = argv[1:] - } else if commandWindows != "" { - handler["commandWindows"] = commandWindows - } - if hook.timeout > 0 { - handler["timeout"] = hook.timeout - } - entry["hooks"] = []any{handler} - event := hook.name - entries, err := jsonEventEntries(hooksObject, event, path) - if err != nil { - return nil, false, err - } - hooksObject[event] = append(entries, entry) - } - } after, err := marshalJSONConfig(root) if err != nil { return nil, false, fmt.Errorf("encode agent hook config %s: %w", path, err) @@ -76,6 +40,59 @@ func planNestedJSONConfig( return after, changed, nil } +// applyNestedJSONHooks replaces the commands owned by marker in root's +// Claude-style hooks object, or removes them on uninstall. +func applyNestedJSONHooks( + root map[string]any, + path, marker, command, commandWindows string, + argv []string, + hooks []nativeHook, + uninstall bool, +) error { + hooksObject, err := jsonHooksObject(root, path, !uninstall) + if err != nil { + return err + } + if hooksObject != nil { + if err := removeOwnedJSONHooks(hooksObject, marker, path); err != nil { + return err + } + } + if uninstall { + return nil + } + if hooksObject == nil { + return fmt.Errorf("agent hook config %s has no hooks object", path) + } + for _, hook := range hooks { + entry := map[string]any{} + if hook.matcher != "" { + entry["matcher"] = hook.matcher + } + handler := map[string]any{ + "type": "command", + "command": command, + } + if len(argv) > 0 { + handler["command"] = argv[0] + handler["args"] = argv[1:] + } else if commandWindows != "" { + handler["commandWindows"] = commandWindows + } + if hook.timeout > 0 { + handler["timeout"] = hook.timeout + } + entry["hooks"] = []any{handler} + event := hook.name + entries, err := jsonEventEntries(hooksObject, event, path) + if err != nil { + return err + } + hooksObject[event] = append(entries, entry) + } + return nil +} + func readJSONConfig(path string) (map[string]any, bool, error) { data, err := os.ReadFile(path) if errors.Is(err, os.ErrNotExist) { @@ -84,14 +101,22 @@ func readJSONConfig(path string) (map[string]any, bool, error) { if err != nil { return nil, false, fmt.Errorf("read agent hook config %s: %w", path, err) } + root, err := decodeJSONConfig(path, data) + if err != nil { + return nil, false, err + } + return root, true, nil +} + +func decodeJSONConfig(path string, data []byte) (map[string]any, error) { if len(strings.TrimSpace(string(data))) == 0 { - return map[string]any{}, true, nil + return map[string]any{}, nil } decoder := json.NewDecoder(bytes.NewReader(data)) decoder.UseNumber() var root map[string]any if err := decoder.Decode(&root); err != nil { - return nil, false, fmt.Errorf("decode agent hook config %s: %w", path, err) + return nil, fmt.Errorf("decode agent hook config %s: %w", path, err) } if root == nil { root = map[string]any{} @@ -101,9 +126,9 @@ func readJSONConfig(path string) (map[string]any, bool, error) { if err == nil { err = errors.New("multiple JSON values") } - return nil, false, fmt.Errorf("decode agent hook config %s: %w", path, err) + return nil, fmt.Errorf("decode agent hook config %s: %w", path, err) } - return root, true, nil + return root, nil } func marshalJSONConfig(root map[string]any) ([]byte, error) { diff --git a/agenthook/normalize.go b/agenthook/normalize.go index 9b764e3..57edefa 100644 --- a/agenthook/normalize.go +++ b/agenthook/normalize.go @@ -47,6 +47,11 @@ func normalize(agent Agent, input io.Reader) ([]byte, error) { return nil, fmt.Errorf("normalize Hermes Agent hook payload: %w", err) } } + if agent == AgentPi { + if err := promotePiReason(payload); err != nil { + return nil, fmt.Errorf("normalize Pi hook payload: %w", err) + } + } if err := normalizePayloadString(payload, "hook_event_name", func(value string) string { return canonicalEventName(spec, value) }); err != nil { diff --git a/agenthook/pi.go b/agenthook/pi.go new file mode 100644 index 0000000..26a74ed --- /dev/null +++ b/agenthook/pi.go @@ -0,0 +1,143 @@ +package agenthook + +import ( + _ "embed" + "encoding/json" + "fmt" + "os" + "path/filepath" + "regexp" + "runtime" + "strings" +) + +//go:embed pi_extension.js +var piExtension string + +func piProfile() profileSpec { + spec := newProfileSpec( + Profile{ + Agent: AgentPi, DisplayName: "Pi", + ConfigEnvironment: "PI_CODING_AGENT_DIR", + // Pi loads top-level *.js and *.ts files from /extensions + // and skips dotfiles, so atomicfile's staging file is never loaded: + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/package-manager.ts#L603-L640 + ConfigFilename: filepath.Join("extensions", "agenthook.js"), + // SessionEnd fires only when another session replaces this one (new, + // resume, fork), never on quit or reload, so a stopped Pi stays resumable. + SupportedEvents: []Event{ + EventSessionStart, EventUserPromptSubmit, EventStop, EventSessionEnd, + }, + }, + formatScript, + "", + func() (string, error) { return userDotDir(filepath.Join(".pi", "agent")) }, + ) + spec.configEnvDir = piAgentDir + spec.eventName = piEventName + spec.script = piExtension + // Pi extension handlers run in-process; the generated extension ignores + // command output, so control decisions have nowhere to go. + spec.responseFormat = responseObservational + // session_start carries a reason that maps to source except for reload, and + // the extension reports session_shutdown only for reasons that map to one: + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/types.ts#L733-L741 + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/types.ts#L802-L808 + spec.sessionSourceRequirement = inputOptional + spec.sessionEndReasonRequirement = inputRequired + return spec +} + +// piEventName maps Claude events to Pi extension events: +// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/types.ts#L911-L922 +// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/types.ts#L997-L1000 +func piEventName(event Event) string { + switch event { + case EventSessionStart: + return "session_start" + case EventUserPromptSubmit: + return "before_agent_start" + case EventStop: + // agent_settled fires once no retry, compaction, or queued turn follows. + return "agent_settled" + case EventSessionEnd: + return "session_shutdown" + default: + return string(event) + } +} + +var piWindowsShellPath = regexp.MustCompile(`(?i)^/(?:mnt/|cygdrive/)?([a-z])(?:/(.*))?$`) + +// piAgentDir follows Pi's normalizePath for PI_CODING_AGENT_DIR: Git Bash, +// MSYS, Cygwin, and WSL drive paths on Windows, then ~: +// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/utils/paths.ts#L67-L101 +func piAgentDir(dir string) (string, error) { + if runtime.GOOS == "windows" && strings.HasPrefix(dir, "/") && + !strings.HasPrefix(dir, "//") && !strings.Contains(dir, `\`) { + if match := piWindowsShellPath.FindStringSubmatch(dir); match != nil { + return strings.ToUpper(match[1]) + `:\` + strings.ReplaceAll(match[2], "/", `\`), nil + } + } + if dir == "~" || strings.HasPrefix(dir, "~/") || + (runtime.GOOS == "windows" && strings.HasPrefix(dir, `~\`)) { + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + rest := "" + if len(dir) > 2 { + rest = dir[2:] + } + return filepath.Join(home, rest), nil + } + return dir, nil +} + +// promotePiReason maps Pi's reason field to Claude's. session_start reasons +// become source (Pi's /new starts a fresh session as Claude's /clear does; +// reload has no equivalent), and session_shutdown reasons for a replaced +// session become SessionEnd reasons. The extension reports no shutdown for +// quit or reload. +func promotePiReason(payload map[string]json.RawMessage) error { + var event, reason string + if raw, ok := payload["hook_event_name"]; ok { + if err := json.Unmarshal(raw, &event); err != nil { + return fmt.Errorf("field %q must be a string: %w", "hook_event_name", err) + } + } + if raw, ok := payload["reason"]; ok { + if err := json.Unmarshal(raw, &reason); err != nil { + return fmt.Errorf("field %q must be a string: %w", "reason", err) + } + } + var field, value string + switch event { + case "session_start": + field = "source" + value = map[string]string{ + "startup": string(SessionSourceStartup), + "resume": string(SessionSourceResume), + "fork": string(SessionSourceFork), + "new": string(SessionSourceClear), + }[reason] + case "session_shutdown": + field = "reason" + value = map[string]string{ + "new": string(SessionEndClear), + "resume": string(SessionEndResume), + "fork": string(SessionEndOther), + }[reason] + default: + return nil + } + if value == "" { + return nil + } + encoded, err := json.Marshal(value) + if err != nil { + return err + } + payload[field] = encoded + return nil +} diff --git a/agenthook/pi_extension.js b/agenthook/pi_extension.js new file mode 100644 index 0000000..c5db521 --- /dev/null +++ b/agenthook/pi_extension.js @@ -0,0 +1,98 @@ +import { existsSync } from "node:fs"; + +// Pi extension API: https://github.com/earendil-works/pi/blob/main/packages/coding-agent/docs/extensions.md +// Pi reports a handler's thrown error as an extension error and still runs the +// event and its other handlers: +// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/runner.ts#L1089-L1117 +// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/runner.ts#L1458-L1466 +const replacedSessionReasons = ["new", "resume", "fork"]; + +// Pi calls this once per session runtime and tears the runtime down when +// another session replaces it, so this state belongs to one session. +export default function (pi) { + // Events wait until the session file exists, so every reported ID resumes + // with pi --session. Pi writes the file once the session holds a user + // message, and never with --no-session: + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/session-manager.ts#L1160-L1185 + let heldStart = null; + const heldPrompts = []; + + // Pi names no file with --no-session, so nothing would ever flush. + const persisting = (ctx) => Boolean(ctx.sessionManager.getSessionFile()); + const saved = (ctx) => { + const file = ctx.sessionManager.getSessionFile(); + return Boolean(file) && existsSync(file); + }; + const payloadFor = (name, ctx, fields) => { + const payload = { hook_event_name: name, session_id: ctx.sessionManager.getSessionId(), cwd: ctx.cwd }; + const transcript = ctx.sessionManager.getSessionFile(); + if (transcript) payload.transcript_path = transcript; + return { ...payload, ...fields }; + }; + const run = (failures, name, payload) => runHooks(name, payload).catch((error) => failures.push(error.message)); + + // Sends held events, oldest first, once the session file exists, and reports + // whether it does. + const flush = async (ctx, failures) => { + if (!saved(ctx)) return false; + if (heldStart) { + const start = heldStart; + heldStart = null; + await run(failures, "session_start", start); + } + for (const prompt of heldPrompts.splice(0)) { + await run(failures, "before_agent_start", prompt); + } + return true; + }; + + // Subagents run Pi in json or print mode with global extensions loaded; + // only the interactive session is one the user can resume: + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/types.ts#L323-L335 + const on = (name, handler) => + pi.on(name, async (event, ctx) => { + if (ctx.mode !== "tui") return; + const failures = []; + await handler(event, ctx, failures); + if (failures.length > 0) throw new Error(failures.join("; ")); + }); + + on("session_start", async (event, ctx, failures) => { + if (!persisting(ctx)) return; + heldStart = payloadFor("session_start", ctx, { reason: event.reason }); + await flush(ctx, failures); + }); + + // before_agent_start runs before Pi appends the turn's user message, so a + // first prompt waits for the next event: + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/agent-session.ts#L2060-L2109 + on("before_agent_start", async (event, ctx, failures) => { + if (!persisting(ctx)) return; + if (typeof event.prompt === "string" && event.prompt !== "") { + heldPrompts.push(payloadFor("before_agent_start", ctx, { prompt: event.prompt })); + } + await flush(ctx, failures); + }); + + // context fires before each model call. Pi has appended the user message by + // the first one: extensions see message_end before Pi persists it, and the + // agent loop awaits that before streaming the response: + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/agent-session.ts#L1137-L1161 + // https://github.com/earendil-works/pi/blob/main/packages/agent/src/agent-loop.ts#L117-L124 + // https://github.com/earendil-works/pi/blob/main/packages/agent/src/agent-loop.ts#L388-L392 + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/sdk.ts#L436-L440 + on("context", async (_event, ctx, failures) => { + if (heldStart || heldPrompts.length > 0) await flush(ctx, failures); + }); + + on("agent_settled", async (_event, ctx, failures) => { + if (await flush(ctx, failures)) await run(failures, "agent_settled", payloadFor("agent_settled", ctx)); + }); + + // Report a shutdown only when another session replaces this one; quit and + // reload keep the session resumable. + on("session_shutdown", async (event, ctx, failures) => { + if (!replacedSessionReasons.includes(event.reason) || !(await flush(ctx, failures))) return; + await run(failures, "session_shutdown", payloadFor("session_shutdown", ctx, { reason: event.reason })); + }); +} diff --git a/agenthook/profile.go b/agenthook/profile.go index bc65103..07a478b 100644 --- a/agenthook/profile.go +++ b/agenthook/profile.go @@ -21,6 +21,7 @@ const ( AgentDroid Agent = "droid" AgentGemini Agent = "gemini" AgentHermes Agent = "hermes" + AgentPi Agent = "pi" AgentQwen Agent = "qwen" ) @@ -61,6 +62,8 @@ const ( formatNestedJSON configFormat = iota formatDirectJSON formatHermesYAML + // formatScript writes a kit-owned JavaScript module the harness loads. + formatScript ) type windowsCommandStyle uint8 @@ -101,6 +104,7 @@ type profileSpec struct { shellToolName string defaultDir func() (string, error) configEnvSubdir string + configEnvDir func(string) (string, error) eventName func(Event) string timeoutUnit time.Duration timeoutField string @@ -110,6 +114,7 @@ type profileSpec struct { requireVersion bool sessionSourceRequirement inputRequirement sessionEndReasonRequirement inputRequirement + script string } var profileOrder = []Agent{ @@ -120,6 +125,7 @@ var profileOrder = []Agent{ AgentDroid, AgentGemini, AgentHermes, + AgentPi, AgentQwen, } @@ -131,6 +137,7 @@ var profiles = map[Agent]profileSpec{ AgentDroid: droidProfile(), AgentGemini: geminiProfile(), AgentHermes: hermesProfile(), + AgentPi: piProfile(), AgentQwen: qwenProfile(), } @@ -190,6 +197,12 @@ func ConfigPath(agent Agent) (string, error) { if dir != "" && spec.configEnvSubdir != "" { dir = filepath.Join(dir, spec.configEnvSubdir) } + if dir != "" && spec.configEnvDir != nil { + var err error + if dir, err = spec.configEnvDir(dir); err != nil { + return "", fmt.Errorf("resolve %s config directory: %w", spec.profile.DisplayName, err) + } + } } if dir == "" { var err error diff --git a/agenthook/script.go b/agenthook/script.go new file mode 100644 index 0000000..36ba9e5 --- /dev/null +++ b/agenthook/script.go @@ -0,0 +1,95 @@ +package agenthook + +import ( + "bytes" + _ "embed" + "errors" + "fmt" + "os" + "strings" +) + +// scriptRuntime spawns registered commands for a harness that loads a +// JavaScript module; each script profile supplies the glue that calls it. +// +//go:embed script_runtime.js +var scriptRuntime string + +const ( + scriptBlockBegin = "// agenthook:begin" + scriptBlockEnd = "// agenthook:end" + scriptConfigDecl = "const config =" +) + +// planScriptConfig rewrites a kit-owned module whose registration block holds +// Claude-style nested hooks JSON. The runtime and profile glue are rewritten +// on every write, so a reinstall also upgrades them. +func planScriptConfig( + spec profileSpec, + path, marker string, + argv []string, + hooks []nativeHook, + uninstall bool, +) ([]byte, bool, error) { + existing, err := os.ReadFile(path) + exists := err == nil + if err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, false, fmt.Errorf("read agent hook config %s: %w", path, err) + } + if !exists && uninstall { + return nil, false, nil + } + root := map[string]any{} + if exists { + block, err := scriptBlock(existing, path) + if err != nil { + return nil, false, err + } + if root, err = decodeJSONConfig(path, block); err != nil { + return nil, false, err + } + } + if err := applyNestedJSONHooks(root, path, marker, "", "", argv, hooks, uninstall); err != nil { + return nil, false, err + } + encoded, err := marshalJSONConfig(root) + if err != nil { + return nil, false, fmt.Errorf("encode agent hook config %s: %w", path, err) + } + var data bytes.Buffer + data.WriteString("// Generated by go.kenn.io/kit/agenthook; Install and Uninstall rewrite this file.\n") + data.WriteString(scriptBlockBegin + "\n") + data.WriteString(scriptConfigDecl + " " + strings.TrimSuffix(string(encoded), "\n") + ";\n") + data.WriteString(scriptBlockEnd + "\n\n") + data.WriteString(scriptRuntime + "\n") + data.WriteString(spec.script) + return data.Bytes(), !bytes.Equal(existing, data.Bytes()), nil +} + +// scriptBlock returns the JSON between the registration markers and refuses a +// file agenthook did not write, so a user's module at the path is never lost. +// Markers match whole lines only; JSON escapes newlines, so a hook argument +// that contains a marker can never form one. +func scriptBlock(data []byte, path string) ([]byte, error) { + lines := strings.Split(string(data), "\n") + begin, end := -1, -1 + for i, line := range lines { + line = strings.TrimSuffix(line, "\r") + if begin < 0 && line == scriptBlockBegin { + begin = i + } else if begin >= 0 && line == scriptBlockEnd { + end = i + break + } + } + if begin < 0 || end < 0 { + return nil, fmt.Errorf("agent hook config %s was not written by agenthook", path) + } + block := strings.TrimSpace(strings.Join(lines[begin+1:end], "\n")) + block, foundDecl := strings.CutPrefix(block, scriptConfigDecl) + block, foundEnd := strings.CutSuffix(block, ";") + if !foundDecl || !foundEnd { + return nil, fmt.Errorf("agent hook config %s has a malformed registration block", path) + } + return []byte(block), nil +} diff --git a/agenthook/script_runtime.js b/agenthook/script_runtime.js new file mode 100644 index 0000000..4b5d05a --- /dev/null +++ b/agenthook/script_runtime.js @@ -0,0 +1,51 @@ +import { spawn } from "node:child_process"; + +const defaultTimeoutSeconds = 60; + +// Runs one argv without a shell in the payload's directory, writing the payload to stdin, and resolves to +// a failure description or null. At its timeout the child is force-killed and +// the wait ends even if it has not exited, so a child that ignores SIGTERM +// cannot block later commands or the harness. +function runCommand(handler, payload) { + return new Promise((resolve) => { + let child; + try { + // Pi changes ctx.cwd on a cross-project resume without changing the process directory. + child = spawn(handler.command, handler.args, { cwd: payload.cwd, stdio: ["pipe", "ignore", "ignore"] }); + } catch (error) { + resolve(`could not start: ${error.message}`); + return; + } + const seconds = handler.timeout > 0 ? handler.timeout : defaultTimeoutSeconds; + const timer = setTimeout(() => { + child.kill("SIGKILL"); + resolve(`timed out after ${seconds}s`); + }, seconds * 1000); + const finish = (failure) => { + clearTimeout(timer); + resolve(failure); + }; + child.once("error", (error) => finish(`could not start: ${error.message}`)); + child.once("close", (code, signal) => { + if (signal) finish(`killed by ${signal}`); + else finish(code === 0 ? null : `exited with status ${code}`); + }); + child.stdin.on("error", () => {}); + child.stdin.end(JSON.stringify(payload)); + }); +} + +// Runs every registered command for a native event in order, then throws one +// error naming each command that failed so the harness reports it. +async function runHooks(event, payload) { + const failures = []; + for (const entry of config.hooks?.[event] ?? []) { + for (const handler of entry.hooks) { + const failure = await runCommand(handler, payload); + if (failure) failures.push(`${[handler.command, ...handler.args].join(" ")}: ${failure}`); + } + } + if (failures.length > 0) { + throw new Error(`agenthook ${event} commands failed: ${failures.join("; ")}`); + } +} diff --git a/agentmcp/agentmcp_test.go b/agentmcp/agentmcp_test.go index b41258c..172e3dd 100644 --- a/agentmcp/agentmcp_test.go +++ b/agentmcp/agentmcp_test.go @@ -105,7 +105,10 @@ func decodeConfig(t *testing.T, agent agentmcp.Agent, data []byte) map[string]an func TestProfiles(t *testing.T) { var hooks, mcps []string for _, profile := range agenthook.Profiles() { - hooks = append(hooks, string(profile.Agent)) + // agentmcp has no Pi profile yet. + if profile.Agent != agenthook.AgentPi { + hooks = append(hooks, string(profile.Agent)) + } } for _, profile := range agentmcp.Profiles() { mcps = append(mcps, string(profile.Agent))